Changeset 181631 in webkit

Timestamp:

Mar 17, 2015 4:10:13 AM (9 years ago)

Author:

Carlos Garcia Campos

Message:

[GTK] WebKitDOM objects leaking
​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):

Location:

trunk

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/bindings/gobject/DOMObjectCache.cpp (modified) (4 diffs)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestDOMNode.cpp (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKit2Gtk/WebProcessTest.cpp (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebCore/bindings/gobject/DOMObjectCache.cpp

@@ -20 +20 @@
 #include "DOMObjectCache.h"
 
+#include "DOMWindowProperty.h"
 #include "Document.h"
+#include "Frame.h"
 #include "FrameDestructionObserver.h"
 #include "Node.h"
@@ -95 +97 @@
         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);
@@ -100 +105 @@
 
 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)
     {
@@ -133 +162 @@
     }
 
+    void willDetachGlobalObjectFromFrame()
+    {
+        clear();
+        m_domWindowObserver = nullptr;
+    }
+
     Vector<DOMObjectCacheData*, 8> m_objects;
+    std::unique_ptr<DOMWindowObserver> m_domWindowObserver;
 };
 

trunk/Tools/ChangeLog

@@ +1 @@
+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>
 

trunk/Tools/TestWebKitAPI/Tests/WebKit2Gtk/TestDOMNode.cpp

@@ -63 +63 @@
 {
     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"));
+    }
 }
 

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

@@ -65 +65 @@
     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()));