Changeset 180211 in webkit


Ignore:
Timestamp:
Feb 16, 2015 11:34:29 PM (9 years ago)
Author:
Carlos Garcia Campos
Message:

[GTK] WebKitFrame objects are never released
​https://bugs.webkit.org/show_bug.cgi?id=141641

Reviewed by Martin Robinson.

Source/WebKit2:

Use a FrameDestructionObserver derived class to wrap our
WebKitFrame objects and delete them when the frame is destroyed,
instead of using willDestroyFrame callback of WKBundlePageLoaderClient
that has never worked.

  • WebProcess/InjectedBundle/API/gtk/WebKitFrame.cpp:

(webkitFrameGetWebFrame):

  • WebProcess/InjectedBundle/API/gtk/WebKitFramePrivate.h:
  • WebProcess/InjectedBundle/API/gtk/WebKitWebPage.cpp:

(webkitFrameGetOrCreate):
(webkitFrameDestroy):
(webkitWebPageCreate):
(willDestroyFrame): Deleted.

Tools:

Add a way to check GObjects leaks for WebProcess tests and check
WebKitFrame objects are not leaked.

  • TestWebKitAPI/Tests/WebKit2Gtk/FrameTest.cpp:

(WebKitFrameTest::testMainFrame):
(WebKitFrameTest::testURI):
(WebKitFrameTest::testJavaScriptContext):

  • TestWebKitAPI/Tests/WebKit2Gtk/WebProcessTest.cpp:

(WebProcessTest::assertObjectIsDeletedWhenTestFinishes):
(runTest):

  • TestWebKitAPI/Tests/WebKit2Gtk/WebProcessTest.h:
Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebKit2/ChangeLog

    r180210 r180211  
     12015-02-16  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [GTK] WebKitFrame objects are never released
     4        https://bugs.webkit.org/show_bug.cgi?id=141641
     5
     6        Reviewed by Martin Robinson.
     7
     8        Use a FrameDestructionObserver derived class to wrap our
     9        WebKitFrame objects and delete them when the frame is destroyed,
     10        instead of using willDestroyFrame callback of WKBundlePageLoaderClient
     11        that has never worked.
     12
     13        * WebProcess/InjectedBundle/API/gtk/WebKitFrame.cpp:
     14        (webkitFrameGetWebFrame):
     15        * WebProcess/InjectedBundle/API/gtk/WebKitFramePrivate.h:
     16        * WebProcess/InjectedBundle/API/gtk/WebKitWebPage.cpp:
     17        (webkitFrameGetOrCreate):
     18        (webkitFrameDestroy):
     19        (webkitWebPageCreate):
     20        (willDestroyFrame): Deleted.
     21
    1222015-02-16  Tim Horton  <timothy_horton@apple.com>
    223

  • trunk/Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitFrame.cpp

    r165760 r180211  
    4646    frame->priv->webFrame = webFrame;
    4747    return frame;
     48}
     49
     50WebFrame* webkitFrameGetWebFrame(WebKitFrame* frame)
     51{
     52    return frame->priv->webFrame.get();
    4853}
    4954

  • trunk/Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitFramePrivate.h

    r154540 r180211  
    2525
    2626WebKitFrame* webkitFrameCreate(WebKit::WebFrame*);
     27WebKit::WebFrame* webkitFrameGetWebFrame(WebKitFrame*);
    2728
    2829#endif // WebKitFramePrivate_h

  • trunk/Source/WebKit2/WebProcess/InjectedBundle/API/gtk/WebKitWebPage.cpp

    r179409 r180211  
    4343#include <WebCore/DocumentLoader.h>
    4444#include <WebCore/Frame.h>
     45#include <WebCore/FrameDestructionObserver.h>
    4546#include <WebCore/FrameView.h>
    4647#include <WebCore/MainFrame.h>
    … …  
    7677WEBKIT_DEFINE_TYPE(WebKitWebPage, webkit_web_page, G_TYPE_OBJECT)
    7778
    78 typedef HashMap<WebFrame*, GRefPtr<WebKitFrame>> WebFrameMap;
     79static void webFrameDestroyed(WebFrame*);
     80
     81class WebKitFrameWrapper final: public FrameDestructionObserver {
     82public:
     83    WebKitFrameWrapper(WebFrame& webFrame)
     84        : FrameDestructionObserver(webFrame.coreFrame())
     85        , m_webkitFrame(adoptGRef(webkitFrameCreate(&webFrame)))
     86    {
     87    }
     88
     89    WebKitFrame* webkitFrame() const { return m_webkitFrame.get(); }
     90
     91private:
     92    virtual void frameDestroyed() override
     93    {
     94        FrameDestructionObserver::frameDestroyed();
     95        webFrameDestroyed(webkitFrameGetWebFrame(m_webkitFrame.get()));
     96    }
     97
     98    GRefPtr<WebKitFrame> m_webkitFrame;
     99};
     100
     101typedef HashMap<WebFrame*, std::unique_ptr<WebKitFrameWrapper>> WebFrameMap;
    79102
    80103static WebFrameMap& webFrameMap()
    … …  
    86109static WebKitFrame* webkitFrameGetOrCreate(WebFrame* webFrame)
    87110{
    88     GRefPtr<WebKitFrame> frame = webFrameMap().get(webFrame);
    89     if (frame)
    90         return frame.get();
    91 
    92     frame = adoptGRef(webkitFrameCreate(webFrame));
    93     webFrameMap().set(webFrame, frame);
    94 
    95     return frame.get();
     111    auto wrapperPtr = webFrameMap().get(webFrame);
     112    if (wrapperPtr)
     113        return wrapperPtr->webkitFrame();
     114
     115    std::unique_ptr<WebKitFrameWrapper> wrapper = std::make_unique<WebKitFrameWrapper>(*webFrame);
     116    wrapperPtr = wrapper.get();
     117    webFrameMap().set(webFrame, WTF::move(wrapper));
     118    return wrapperPtr->webkitFrame();
     119}
     120
     121static void webFrameDestroyed(WebFrame* webFrame)
     122{
     123    webFrameMap().remove(webFrame);
    96124}
    97125
    … …  
    144172
    145173    g_signal_emit(WEBKIT_WEB_PAGE(clientInfo), signals[DOCUMENT_LOADED], 0);
    146 }
    147 
    148 static void willDestroyFrame(WKBundlePageRef, WKBundleFrameRef frame, const void* /* clientInfo */)
    149 {
    150     webFrameMap().remove(toImpl(frame));
    151174}
    152175
    … …  
    393416        0, // didFirstLayoutForFrame
    394417        0, // didFirstVisuallyNonEmptyLayoutForFrame
    395         0, // didRemoveFrameFromHierarchy
     418        0, // didRemoveFrameFromHierarchy,
    396419        0, // didDisplayInsecureContentForFrame
    397420        0, // didRunInsecureContentForFrame
    … …  
    416439        0, // willLoadURLRequest
    417440        0, // willLoadDataRequest
    418         willDestroyFrame
     441        0, // willDestroyFrame
    419442    };
    420443    WKBundlePageSetPageLoaderClient(toAPI(webPage), &loaderClient.base);

  • trunk/Tools/ChangeLog

    r180164 r180211  
     12015-02-16  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [GTK] WebKitFrame objects are never released
     4        https://bugs.webkit.org/show_bug.cgi?id=141641
     5
     6        Reviewed by Martin Robinson.
     7
     8        Add a way to check GObjects leaks for WebProcess tests and check
     9        WebKitFrame objects are not leaked.
     10
     11        * TestWebKitAPI/Tests/WebKit2Gtk/FrameTest.cpp:
     12        (WebKitFrameTest::testMainFrame):
     13        (WebKitFrameTest::testURI):
     14        (WebKitFrameTest::testJavaScriptContext):
     15        * TestWebKitAPI/Tests/WebKit2Gtk/WebProcessTest.cpp:
     16        (WebProcessTest::assertObjectIsDeletedWhenTestFinishes):
     17        (runTest):
     18        * TestWebKitAPI/Tests/WebKit2Gtk/WebProcessTest.h:
     19
    1202015-02-16  Tim Horton  <timothy_horton@apple.com>
    221

  • trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/FrameTest.cpp

    r176563 r180211  
    3333        WebKitFrame* frame = webkit_web_page_get_main_frame(page);
    3434        g_assert(WEBKIT_IS_FRAME(frame));
     35        assertObjectIsDeletedWhenTestFinishes(G_OBJECT(frame));
    3536        g_assert(webkit_frame_is_main_frame(frame));
    3637
    … …  
    4243        WebKitFrame* frame = webkit_web_page_get_main_frame(page);
    4344        g_assert(WEBKIT_IS_FRAME(frame));
     45        assertObjectIsDeletedWhenTestFinishes(G_OBJECT(frame));
    4446        g_assert_cmpstr(webkit_web_page_get_uri(page), ==, webkit_frame_get_uri(frame));
    4547
    … …  
    5153        WebKitFrame* frame = webkit_web_page_get_main_frame(page);
    5254        g_assert(WEBKIT_IS_FRAME(frame));
     55        assertObjectIsDeletedWhenTestFinishes(G_OBJECT(frame));
    5356        g_assert(webkit_frame_get_javascript_global_context(frame));
    5457

  • trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/WebProcessTest.cpp

    r176563 r180211  
    2323#include <JavaScriptCore/JSRetainPtr.h>
    2424#include <gio/gio.h>
     25#include <wtf/HashSet.h>
    2526#include <wtf/NeverDestroyed.h>
    2627#include <wtf/gobject/GUniquePtr.h>
     28
     29static HashSet<GObject*> s_watchedObjects;
    2730
    2831typedef HashMap<String, std::function<std::unique_ptr<WebProcessTest> ()>> TestsMap;
    … …  
    3639{
    3740    testsMap().add(testName, WTF::move(closure));
     41}
     42
     43void WebProcessTest::assertObjectIsDeletedWhenTestFinishes(GObject* object)
     44{
     45    s_watchedObjects.add(object);
     46    g_object_weak_ref(object, [](gpointer, GObject* finalizedObject) {
     47        s_watchedObjects.remove(finalizedObject);
     48    }, nullptr);
    3849}
    3950
    … …  
    5465    WebKitWebPage* webPage = WEBKIT_WEB_PAGE(JSObjectGetPrivate(thisObject));
    5566    g_assert(WEBKIT_IS_WEB_PAGE(webPage));
     67    WebProcessTest::assertObjectIsDeletedWhenTestFinishes(G_OBJECT(webPage));
    5668
    5769    std::unique_ptr<WebProcessTest> test = WebProcessTest::create(String::fromUTF8(testPath.get()));
    … …  
    6880{
    6981    g_object_unref(JSObjectGetPrivate(object));
     82
     83    if (s_watchedObjects.isEmpty())
     84        return;
     85
     86    g_print("Leaked objects in WebProcess:");
     87    for (const auto object : s_watchedObjects)
     88        g_print(" %s(%p)", g_type_name_from_instance(reinterpret_cast<GTypeInstance*>(object)), object);
     89    g_print("\n");
     90
     91    g_assert(s_watchedObjects.isEmpty());
    7092}
    7193

  • trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/WebProcessTest.h

    r176563 r180211  
    2929    virtual bool runTest(const char* testName, WebKitWebPage*) = 0;
    3030
     31    static void assertObjectIsDeletedWhenTestFinishes(GObject*);
     32
    3133    static void add(const String& testName, std::function<std::unique_ptr<WebProcessTest> ()>);
    3234    static std::unique_ptr<WebProcessTest> create(const String& testName);
Note: See TracChangeset for help on using the changeset viewer.