[GTK] WebKitDOM objects leaking
authorcarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Mar 2015 11:10:13 +0000 (11:10 +0000)
committercarlosgc@webkit.org <carlosgc@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 17 Mar 2015 11:10:13 +0000 (11:10 +0000)
https://bugs.webkit.org/show_bug.cgi?id=118788

Reviewed by Darin Adler and Sergio Villar Senin.

Source/WebCore:

Use a DOMwindowObserver class, derived from DOMWindowProperty to
be notified when the window object is detached from the frame to
clear the DOM objects associated to that frame in that case too.

* bindings/gobject/DOMObjectCache.cpp:

Tools:

Update DOMObjectCache unit test to check that DOM objects are also
released when new contents are loaded in the web view, and the old
document is detached from the frame.

* TestWebKitAPI/Tests/WebKit2Gtk/TestDOMNode.cpp:
(testWebKitDOMObjectCache):
* TestWebKitAPI/Tests/WebKit2Gtk/WebProcessTest.cpp:
(runTest):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@181631 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/bindings/gobject/DOMObjectCache.cpp
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestDOMNode.cpp
Tools/TestWebKitAPI/Tests/WebKit2Gtk/WebProcessTest.cpp

index 7568983..1e0a7f8 100644 (file)
@@ -1,3 +1,16 @@
+2015-03-17  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] WebKitDOM objects leaking
+        https://bugs.webkit.org/show_bug.cgi?id=118788
+
+        Reviewed by Darin Adler and Sergio Villar Senin.
+
+        Use a DOMwindowObserver class, derived from DOMWindowProperty to
+        be notified when the window object is detached from the frame to
+        clear the DOM objects associated to that frame in that case too.
+
+        * bindings/gobject/DOMObjectCache.cpp:
+
 2015-03-17  Zan Dobersek  <zdobersek@igalia.com>
 
         [CMake] Use a forwarding header for ANGLE's ShaderLang.h to avoid picking up ANGLE's EGL headers
index b7bf3e5..4710768 100644 (file)
@@ -19,7 +19,9 @@
 #include "config.h"
 #include "DOMObjectCache.h"
 
+#include "DOMWindowProperty.h"
 #include "Document.h"
+#include "Frame.h"
 #include "FrameDestructionObserver.h"
 #include "Node.h"
 #include <glib-object.h>
@@ -94,11 +96,38 @@ public:
     {
         ASSERT(!m_objects.contains(&data));
 
+        if (!m_domWindowObserver && m_frame->document()->domWindow())
+            m_domWindowObserver = std::make_unique<DOMWindowObserver>(*m_frame, *this);
+
         m_objects.append(&data);
         g_object_weak_ref(data.object, DOMObjectCacheFrameObserver::objectFinalizedCallback, this);
     }
 
 private:
+    class DOMWindowObserver final: public WebCore::DOMWindowProperty {
+        WTF_MAKE_FAST_ALLOCATED;
+    public:
+        DOMWindowObserver(WebCore::Frame& frame, DOMObjectCacheFrameObserver& frameObserver)
+            : DOMWindowProperty(&frame)
+            , m_frameObserver(frameObserver)
+        {
+        }
+
+        virtual ~DOMWindowObserver()
+        {
+        }
+
+    private:
+        virtual void willDetachGlobalObjectFromFrame() override
+        {
+            // Clear the DOMWindowProperty first, and then notify the Frame observer.
+            DOMWindowProperty::willDetachGlobalObjectFromFrame();
+            m_frameObserver.willDetachGlobalObjectFromFrame();
+        }
+
+        DOMObjectCacheFrameObserver& m_frameObserver;
+    };
+
     static void objectFinalizedCallback(gpointer userData, GObject* finalizedObject)
     {
         DOMObjectCacheFrameObserver* observer = static_cast<DOMObjectCacheFrameObserver*>(userData);
@@ -132,7 +161,14 @@ private:
         domObjectCacheFrameObservers().remove(frame);
     }
 
+    void willDetachGlobalObjectFromFrame()
+    {
+        clear();
+        m_domWindowObserver = nullptr;
+    }
+
     Vector<DOMObjectCacheData*, 8> m_objects;
+    std::unique_ptr<DOMWindowObserver> m_domWindowObserver;
 };
 
 typedef HashMap<void*, std::unique_ptr<DOMObjectCacheData>> DOMObjectMap;
index f5205a2..148c7df 100644 (file)
@@ -1,3 +1,19 @@
+2015-03-17  Carlos Garcia Campos  <cgarcia@igalia.com>
+
+        [GTK] WebKitDOM objects leaking
+        https://bugs.webkit.org/show_bug.cgi?id=118788
+
+        Reviewed by Darin Adler and Sergio Villar Senin.
+
+        Update DOMObjectCache unit test to check that DOM objects are also
+        released when new contents are loaded in the web view, and the old
+        document is detached from the frame.
+
+        * TestWebKitAPI/Tests/WebKit2Gtk/TestDOMNode.cpp:
+        (testWebKitDOMObjectCache):
+        * TestWebKitAPI/Tests/WebKit2Gtk/WebProcessTest.cpp:
+        (runTest):
+
 2015-03-17  Youenn Fablet  <youenn.fablet@crf.canon.fr>
 
         W3C test importer default import folder should be LayoutTests/imported/w3c
index 24d0c00..4b28042 100644 (file)
@@ -62,10 +62,15 @@ static void testWebKitDOMNodeTagNames(WebViewTest* test, gconstpointer)
 static void testWebKitDOMObjectCache(WebViewTest* test, gconstpointer)
 {
     static const char* testHTML = "<html><body><div id='container'><p>DOM Cache test</p><a id='link href='#'>link</a></div></body></html>";
-    test->loadHtml(testHTML, nullptr);
-    test->waitUntilLoadFinished();
 
-    g_assert(test->runWebProcessTest("WebKitDOMNode", "dom-cache"));
+    // Run the test 3 times to make sure the DOM objects are correctly released when the
+    // document is detached from the frame for every new document created.
+    for (unsigned i = 0; i < 3; ++i) {
+        test->loadHtml(testHTML, nullptr);
+        test->waitUntilLoadFinished();
+
+        g_assert(test->runWebProcessTest("WebKitDOMNode", "dom-cache"));
+    }
 }
 
 
index 643b549..82f1112 100644 (file)
@@ -64,7 +64,14 @@ static JSValueRef runTest(JSContextRef context, JSObjectRef function, JSObjectRe
 
     WebKitWebPage* webPage = WEBKIT_WEB_PAGE(JSObjectGetPrivate(thisObject));
     g_assert(WEBKIT_IS_WEB_PAGE(webPage));
-    WebProcessTest::assertObjectIsDeletedWhenTestFinishes(G_OBJECT(webPage));
+    // Test /WebKitDOMNode/dom-cache is an exception, because it's called 3 times, so
+    // the WebPage is destroyed after the third time.
+    if (g_str_equal(testPath.get(), "WebKitDOMNode/dom-cache")) {
+        static unsigned domCacheTestRunCount = 0;
+        if (++domCacheTestRunCount == 3)
+            WebProcessTest::assertObjectIsDeletedWhenTestFinishes(G_OBJECT(webPage));
+    } else
+        WebProcessTest::assertObjectIsDeletedWhenTestFinishes(G_OBJECT(webPage));
 
     std::unique_ptr<WebProcessTest> test = WebProcessTest::create(String::fromUTF8(testPath.get()));
     return JSValueMakeBoolean(context, test->runTest(g_strrstr(testPath.get(), "/") + 1, webPage));