Title: [111754] trunk
Revision
111754
Author
commit-qu...@webkit.org
Date
2012-03-22 13:45:36 -0700 (Thu, 22 Mar 2012)

Log Message

HTMLPluginElement is not destroyed on reload or navigation if getNPObject is called
https://bugs.webkit.org/show_bug.cgi?id=80428

Patch by Dave Michael <dmich...@chromium.org> on 2012-03-22
Reviewed by Eric Seidel.

.:

Test: plugins/netscape-dom-access-and-reload.html

* Source/autotools/symbols.filter: Export a symbol for InspectorCounters::counterValue.

Source/WebCore:

Make HTMLPluginElement release its m_NPObject in detach() to break a
reference-counting cycle that happens on reload or navigation. With this
change, HTMLPlugInElement::removedFromDocument is unnecessary, so it
was removed. Note that Releasing m_NPObject does not result in a call to
the plugin; it simply releases a reference count on the wrapper object
for this HTMLPlugInElement. (The plugin's NPP_Deallocate is invoked
when the render tree is destroyed, when PluginView calls
PluginPackage::unload.) Thus, it is safe to release m_NPObject in
detach, because it can not result in layout or style changes.

Also added numberOfLiveNodes() and numberOfLiveDocuments() to
window.internals to enable testing.

Test: plugins/netscape-dom-access-and-reload.html

* WebCore.exp.in:
* html/HTMLPlugInElement.cpp:
(WebCore::HTMLPlugInElement::detach):
* html/HTMLPlugInElement.h:
(HTMLPlugInElement):
* testing/Internals.cpp:
(WebCore::Internals::numberOfLiveDocuments):
(WebCore::Internals::numberOfLiveNodes):
(WebCore):
* testing/Internals.h:
(Internals):
* testing/Internals.idl:

Source/WebKit2:

Test: plugins/netscape-dom-access-and-reload.html

* win/WebKit2.def: Export a symbol for InspectorCounters::counterValue
* win/WebKit2CFLite.def: Export a symbol for InspectorCounters::counterValue

LayoutTests:

* plugins/netscape-dom-access-and-reload-expected.txt: Added.
* plugins/netscape-dom-access-and-reload.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/ChangeLog (111753 => 111754)


--- trunk/ChangeLog	2012-03-22 20:43:12 UTC (rev 111753)
+++ trunk/ChangeLog	2012-03-22 20:45:36 UTC (rev 111754)
@@ -1,3 +1,14 @@
+2012-03-22  Dave Michael  <dmich...@chromium.org>
+
+        HTMLPluginElement is not destroyed on reload or navigation if getNPObject is called
+        https://bugs.webkit.org/show_bug.cgi?id=80428
+
+        Reviewed by Eric Seidel.
+
+        Test: plugins/netscape-dom-access-and-reload.html
+
+        * Source/autotools/symbols.filter: Export a symbol for InspectorCounters::counterValue.
+
 2012-03-22  Kevin Ollivier  <kev...@theolliviers.com>
 
         [wx] Unreviewed. Adding Source/WTF to the build and updating

Modified: trunk/LayoutTests/ChangeLog (111753 => 111754)


--- trunk/LayoutTests/ChangeLog	2012-03-22 20:43:12 UTC (rev 111753)
+++ trunk/LayoutTests/ChangeLog	2012-03-22 20:45:36 UTC (rev 111754)
@@ -1,3 +1,13 @@
+2012-03-22  Dave Michael  <dmich...@chromium.org>
+
+        HTMLPluginElement is not destroyed on reload or navigation if getNPObject is called
+        https://bugs.webkit.org/show_bug.cgi?id=80428
+
+        Reviewed by Eric Seidel.
+
+        * plugins/netscape-dom-access-and-reload-expected.txt: Added.
+        * plugins/netscape-dom-access-and-reload.html: Added.
+
 2012-03-22  Sudarsana Nagineni  <sudarsana.nagin...@linux.intel.com>
 
         [EFL] Enable view mode media feature layout tests

Added: trunk/LayoutTests/plugins/netscape-dom-access-and-reload-expected.txt (0 => 111754)


--- trunk/LayoutTests/plugins/netscape-dom-access-and-reload-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/plugins/netscape-dom-access-and-reload-expected.txt	2012-03-22 20:45:36 UTC (rev 111754)
@@ -0,0 +1,3 @@
+This page tests reloading a Netscape plug-in that accesses its own DOM element. See https://bugs.webkit.org/show_bug.cgi?id=80428, "HTMLPluginElement is not destroyed on reload or navigation if getNPObject is called". If it succeeds, you should see SUCCESS below. 
+
+SUCCESS

Added: trunk/LayoutTests/plugins/netscape-dom-access-and-reload.html (0 => 111754)


--- trunk/LayoutTests/plugins/netscape-dom-access-and-reload.html	                        (rev 0)
+++ trunk/LayoutTests/plugins/netscape-dom-access-and-reload.html	2012-03-22 20:45:36 UTC (rev 111754)
@@ -0,0 +1,55 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+function runTest()
+{
+    // Make the plugin retrieve a DOM element to itself. This exercises
+    // https://bugs.webkit.org/show_bug.cgi?id=80428
+    document.getElementById("testPlugin").testDOMAccess();
+
+    var callReload = true;
+    if (window.sessionStorage) {
+        if (window.sessionStorage.reloadPluginsAndPagesCalled)
+            callReload = false;
+        else
+            window.sessionStorage.reloadPluginsAndPagesCalled = 1;
+    }
+
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+    
+    if (callReload) {
+        if (window.layoutTestController)
+            layoutTestController.waitUntilDone();
+        location.reload();
+    } else {
+        window.GCController.collect();
+        // Note we could also collect the number of live nodes, but that seems
+        // more likely to give spurious failures. When an HTMLPluginElement is
+        // improperly retained, its owning Document also survives, so we'll
+        // detect an excess Document.
+        if (internals && internals.numberOfLiveDocuments) {
+            var numberOfLiveDocuments = internals.numberOfLiveDocuments();
+            if (numberOfLiveDocuments == 1) {
+                document.getElementById("result").innerHTML = "SUCCESS";
+            }
+        } else {
+              document.getElementById("result").innerHTML = "FAILED; This test is only valid in DumpRenderTree, and only when the Inspector is enabled.";
+        }
+        if (window.layoutTestController) {
+            layoutTestController.dumpAsText();
+            layoutTestController.notifyDone();
+        }
+    }
+}
+</script>
+</head>
+<body _onload_="runTest();">
+<p>This page tests reloading a Netscape plug-in that accesses its own DOM element. See https://bugs.webkit.org/show_bug.cgi?id=80428, "HTMLPluginElement is not destroyed on reload or navigation if getNPObject is called".
+
+If it succeeds, you should see SUCCESS below.
+<embed id="testPlugin" type="application/x-webkit-test-netscape" width="200" height="200"></embed>
+<div id="result">FAILURE</div>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (111753 => 111754)


--- trunk/Source/WebCore/ChangeLog	2012-03-22 20:43:12 UTC (rev 111753)
+++ trunk/Source/WebCore/ChangeLog	2012-03-22 20:45:36 UTC (rev 111754)
@@ -1,3 +1,38 @@
+2012-03-22  Dave Michael  <dmich...@chromium.org>
+
+        HTMLPluginElement is not destroyed on reload or navigation if getNPObject is called
+        https://bugs.webkit.org/show_bug.cgi?id=80428
+
+        Reviewed by Eric Seidel.
+
+        Make HTMLPluginElement release its m_NPObject in detach() to break a
+        reference-counting cycle that happens on reload or navigation. With this
+        change, HTMLPlugInElement::removedFromDocument is unnecessary, so it
+        was removed. Note that Releasing m_NPObject does not result in a call to
+        the plugin; it simply releases a reference count on the wrapper object
+        for this HTMLPlugInElement. (The plugin's NPP_Deallocate is invoked
+        when the render tree is destroyed, when PluginView calls
+        PluginPackage::unload.) Thus, it is safe to release m_NPObject in
+        detach, because it can not result in layout or style changes.
+
+        Also added numberOfLiveNodes() and numberOfLiveDocuments() to
+        window.internals to enable testing.
+
+        Test: plugins/netscape-dom-access-and-reload.html
+
+        * WebCore.exp.in:
+        * html/HTMLPlugInElement.cpp:
+        (WebCore::HTMLPlugInElement::detach):
+        * html/HTMLPlugInElement.h:
+        (HTMLPlugInElement):
+        * testing/Internals.cpp:
+        (WebCore::Internals::numberOfLiveDocuments):
+        (WebCore::Internals::numberOfLiveNodes):
+        (WebCore):
+        * testing/Internals.h:
+        (Internals):
+        * testing/Internals.idl:
+
 2012-03-22  Shawn Singh  <shawnsi...@chromium.org>
 
         [chromium] Make CCDamageTracker robust to empty layer lists

Modified: trunk/Source/WebCore/WebCore.exp.in (111753 => 111754)


--- trunk/Source/WebCore/WebCore.exp.in	2012-03-22 20:43:12 UTC (rev 111753)
+++ trunk/Source/WebCore/WebCore.exp.in	2012-03-22 20:45:36 UTC (rev 111754)
@@ -1748,6 +1748,7 @@
 
 #if ENABLE(INSPECTOR)
 __ZN7WebCore15InspectorClient31doDispatchMessageOnFrontendPageEPNS_4PageERKN3WTF6StringE
+__ZN7WebCore17InspectorCounters12counterValueENS0_11CounterTypeE
 __ZN7WebCore19InspectorController14enableProfilerEv
 __ZN7WebCore19InspectorController15disableProfilerEv
 __ZN7WebCore19InspectorController15profilerEnabledEv

Modified: trunk/Source/WebCore/html/HTMLPlugInElement.cpp (111753 => 111754)


--- trunk/Source/WebCore/html/HTMLPlugInElement.cpp	2012-03-22 20:43:12 UTC (rev 111753)
+++ trunk/Source/WebCore/html/HTMLPlugInElement.cpp	2012-03-22 20:45:36 UTC (rev 111754)
@@ -78,11 +78,6 @@
         m_isCapturingMouseEvents = false;
     }
 
-    HTMLFrameOwnerElement::detach();
-}
-
-void HTMLPlugInElement::removedFromDocument()
-{
 #if ENABLE(NETSCAPE_PLUGIN_API)
     if (m_NPObject) {
         _NPN_ReleaseObject(m_NPObject);
@@ -90,7 +85,7 @@
     }
 #endif
 
-    HTMLFrameOwnerElement::removedFromDocument();
+    HTMLFrameOwnerElement::detach();
 }
 
 PassScriptInstance HTMLPlugInElement::getInstance()

Modified: trunk/Source/WebCore/html/HTMLPlugInElement.h (111753 => 111754)


--- trunk/Source/WebCore/html/HTMLPlugInElement.h	2012-03-22 20:43:12 UTC (rev 111753)
+++ trunk/Source/WebCore/html/HTMLPlugInElement.h	2012-03-22 20:45:36 UTC (rev 111754)
@@ -57,7 +57,6 @@
     HTMLPlugInElement(const QualifiedName& tagName, Document*);
 
     virtual void detach();
-    virtual void removedFromDocument();
     virtual bool isPresentationAttribute(const QualifiedName&) const OVERRIDE;
     virtual void collectStyleForAttribute(Attribute*, StylePropertySet*) OVERRIDE;
 

Modified: trunk/Source/WebCore/testing/Internals.cpp (111753 => 111754)


--- trunk/Source/WebCore/testing/Internals.cpp	2012-03-22 20:43:12 UTC (rev 111753)
+++ trunk/Source/WebCore/testing/Internals.cpp	2012-03-22 20:45:36 UTC (rev 111754)
@@ -42,6 +42,7 @@
 #include "HTMLNames.h"
 #include "HTMLTextAreaElement.h"
 #include "InspectorController.h"
+#include "InspectorCounters.h"
 #include "InspectorInstrumentation.h"
 #include "InternalSettings.h"
 #include "IntRect.h"
@@ -770,4 +771,16 @@
     return document->frame()->editor()->selectionStartHasMarkerFor(DocumentMarker::Spelling, from, length);
 }
 
+#if ENABLE(INSPECTOR)
+unsigned Internals::numberOfLiveNodes() const
+{
+    return InspectorCounters::counterValue(InspectorCounters::NodeCounter);
 }
+
+unsigned Internals::numberOfLiveDocuments() const
+{
+    return InspectorCounters::counterValue(InspectorCounters::DocumentCounter);
+}
+#endif // ENABLE(INSPECTOR)
+
+}

Modified: trunk/Source/WebCore/testing/Internals.h (111753 => 111754)


--- trunk/Source/WebCore/testing/Internals.h	2012-03-22 20:43:12 UTC (rev 111753)
+++ trunk/Source/WebCore/testing/Internals.h	2012-03-22 20:45:36 UTC (rev 111754)
@@ -145,6 +145,11 @@
 
     void setBatteryStatus(Document*, const String& eventType, bool charging, double chargingTime, double dischargingTime, double level, ExceptionCode&);
 
+#if ENABLE(INSPECTOR)
+    unsigned numberOfLiveNodes() const;
+    unsigned numberOfLiveDocuments() const;
+#endif
+
 private:
     explicit Internals(Document*);
     DocumentMarker* markerAt(Node*, const String& markerType, unsigned index, ExceptionCode&);

Modified: trunk/Source/WebCore/testing/Internals.idl (111753 => 111754)


--- trunk/Source/WebCore/testing/Internals.idl	2012-03-22 20:43:12 UTC (rev 111753)
+++ trunk/Source/WebCore/testing/Internals.idl	2012-03-22 20:45:36 UTC (rev 111754)
@@ -119,7 +119,10 @@
 
 #if defined(ENABLE_BATTERY_STATUS) && ENABLE_BATTERY_STATUS
         void setBatteryStatus(in Document document, in DOMString eventType, in boolean charging, in double chargingTime, in double dischargingTime, in double level) raises (DOMException);
-#endif
+#endif 
+
+        [Conditional=INSPECTOR] unsigned long numberOfLiveNodes();
+        [Conditional=INSPECTOR] unsigned long numberOfLiveDocuments();
     };
 }
 

Modified: trunk/Source/WebKit2/ChangeLog (111753 => 111754)


--- trunk/Source/WebKit2/ChangeLog	2012-03-22 20:43:12 UTC (rev 111753)
+++ trunk/Source/WebKit2/ChangeLog	2012-03-22 20:45:36 UTC (rev 111754)
@@ -1,3 +1,15 @@
+2012-03-22  Dave Michael  <dmich...@chromium.org>
+
+        HTMLPluginElement is not destroyed on reload or navigation if getNPObject is called
+        https://bugs.webkit.org/show_bug.cgi?id=80428
+
+        Reviewed by Eric Seidel.
+
+        Test: plugins/netscape-dom-access-and-reload.html
+
+        * win/WebKit2.def: Export a symbol for InspectorCounters::counterValue
+        * win/WebKit2CFLite.def: Export a symbol for InspectorCounters::counterValue
+
 2012-03-22  Pierre Rossi  <pierre.ro...@gmail.com>
 
         Revert back the device DPI to 160.

Modified: trunk/Source/WebKit2/win/WebKit2.def (111753 => 111754)


--- trunk/Source/WebKit2/win/WebKit2.def	2012-03-22 20:43:12 UTC (rev 111753)
+++ trunk/Source/WebKit2/win/WebKit2.def	2012-03-22 20:45:36 UTC (rev 111754)
@@ -115,6 +115,7 @@
         ??1ThreadCondition@WTF@@QAE@XZ
         ?broadcast@ThreadCondition@WTF@@QAEXXZ
         ?callOnMainThread@WTF@@YAXP6AXPAX@Z0@Z
+        ?counterValue@InspectorCounters@WebCore@@SAHW4CounterType@12@@Z
         ?createThread@WTF@@YAIP6APAXPAX@Z0PBD@Z
         ?createThread@WTF@@YAIP6AXPAX@Z0PBD@Z
         ?currentThread@WTF@@YAIXZ

Modified: trunk/Source/WebKit2/win/WebKit2CFLite.def (111753 => 111754)


--- trunk/Source/WebKit2/win/WebKit2CFLite.def	2012-03-22 20:43:12 UTC (rev 111753)
+++ trunk/Source/WebKit2/win/WebKit2CFLite.def	2012-03-22 20:45:36 UTC (rev 111754)
@@ -108,6 +108,7 @@
         ??1ThreadCondition@WTF@@QAE@XZ
         ?broadcast@ThreadCondition@WTF@@QAEXXZ
         ?callOnMainThread@WTF@@YAXP6AXPAX@Z0@Z
+        ?counterValue@InspectorCounters@WebCore@@SAHW4CounterType@12@@Z
         ?createThread@WTF@@YAIP6APAXPAX@Z0PBD@Z
         ?createThread@WTF@@YAIP6AXPAX@Z0PBD@Z
         ?currentThread@WTF@@YAIXZ

Modified: trunk/Source/autotools/symbols.filter (111753 => 111754)


--- trunk/Source/autotools/symbols.filter	2012-03-22 20:43:12 UTC (rev 111753)
+++ trunk/Source/autotools/symbols.filter	2012-03-22 20:45:36 UTC (rev 111754)
@@ -59,6 +59,7 @@
 _ZN7WebCore16jsStringSlowCaseEPN3JSC9ExecStateERN3WTF7HashMapIPNS3_10StringImplENS0_4WeakINS0_8JSStringEEENS3_10StringHashENS3_10HashTraitsIS6_EENSB_IS9_EEEES6_;
 _ZN7WebCore16scriptNameToCodeERKN3WTF6StringE;
 _ZN7WebCore17cacheDOMStructureEPNS_17JSDOMGlobalObjectEPN3JSC9StructureEPKNS2_9ClassInfoE;
+_ZN7WebCore17InspectorCounters12counterValueENS0_11CounterTypeE;
 _ZN7WebCore18HTMLContentElement6createEPNS_8DocumentE;
 _ZN7WebCore19InspectorController39setResourcesDataSizeLimitsFromInternalsEii;
 _ZN7WebCore20NodeRenderingContextC1EPNS_4NodeE;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to