Crash when trying to invalidate the NPRuntimeObjectMap for a plug-in in a subframe
authorandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Jan 2012 00:29:14 +0000 (00:29 +0000)
committerandersca@apple.com <andersca@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 6 Jan 2012 00:29:14 +0000 (00:29 +0000)
https://bugs.webkit.org/show_bug.cgi?id=75667
<rdar://problem/10389454>

Reviewed by Kevin Decker.

Source/WebKit2:

NPRuntimeObjectMap::invalidate is called whenever a plug-in view is destroyed. If invalidate is called for an object map
whose plug-in has a null frame, we'd crash.

The plug-in will have a null frame if the plug-in view is destroyed because its containing frame has been removed from the document,
and if the plug-in view is being destroyed asynchronously due to the plug-in itself calling JavaScript that will remove the frame
(see PluginView::unprotectPluginFromDestruction).

The reason NPRuntimeObjectMap::invalidate will crash when the frame is null is because we were trying to access the frame's global
object, causing a null dereference. The reason we were trying to get at the frame's global object was to create a Strong handle to
a JSNPObject so we could stick the object in a vector so we could later iterate over the vector elements and call invalidate() on
each JSNPObject which will end up releasing the underlying NPObject.

However, it turns out that we don't need to stick the JSNPObject in a vector; we can just get the underlying NPObject directly and
stick that in a vector and then iterate over the NPObjects, releasing them.

* WebProcess/Plugins/Netscape/NPRuntimeObjectMap.cpp:
(WebKit::NPRuntimeObjectMap::invalidate):

Tools:

Add an evaluate method to the plug-in test scriptable object that can be used to evaluate a given JS string.

* DumpRenderTree/TestNetscapePlugIn/PluginTest.cpp:
(PluginTest::executeScript):
* DumpRenderTree/TestNetscapePlugIn/PluginTest.h:
* DumpRenderTree/TestNetscapePlugIn/Tests/NPRuntimeObjectFromDestroyedPlugin.cpp:
(NPRuntimeObjectFromDestroyedPlugin::ScriptableObject::hasMethod):
(NPRuntimeObjectFromDestroyedPlugin::ScriptableObject::invoke):
(NPRuntimeObjectFromDestroyedPlugin::ScriptableObject::hasProperty):

LayoutTests:

* plugins/npruntime/object-from-destroyed-plugin-in-subframe-expected.txt: Added.
* plugins/npruntime/object-from-destroyed-plugin-in-subframe.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/plugins/npruntime/object-from-destroyed-plugin-in-subframe-expected.txt [new file with mode: 0644]
LayoutTests/plugins/npruntime/object-from-destroyed-plugin-in-subframe.html [new file with mode: 0644]
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/Plugins/Netscape/NPRuntimeObjectMap.cpp
Tools/ChangeLog
Tools/DumpRenderTree/TestNetscapePlugIn/PluginTest.cpp
Tools/DumpRenderTree/TestNetscapePlugIn/PluginTest.h
Tools/DumpRenderTree/TestNetscapePlugIn/Tests/NPRuntimeObjectFromDestroyedPlugin.cpp

index 77bc3cc..086f359 100755 (executable)
@@ -1,3 +1,14 @@
+2012-01-05  Anders Carlsson  <andersca@apple.com>
+
+        Crash when trying to invalidate the NPRuntimeObjectMap for a plug-in in a subframe
+        https://bugs.webkit.org/show_bug.cgi?id=75667
+        <rdar://problem/10389454>
+
+        Reviewed by Kevin Decker.
+
+        * plugins/npruntime/object-from-destroyed-plugin-in-subframe-expected.txt: Added.
+        * plugins/npruntime/object-from-destroyed-plugin-in-subframe.html: Added.
+
 2012-01-05  Ojan Vafai  <ojan@chromium.org>
 
         New chromium expected results after r104208.
diff --git a/LayoutTests/plugins/npruntime/object-from-destroyed-plugin-in-subframe-expected.txt b/LayoutTests/plugins/npruntime/object-from-destroyed-plugin-in-subframe-expected.txt
new file mode 100644 (file)
index 0000000..f476f9a
--- /dev/null
@@ -0,0 +1,10 @@
+Test various operation on an NPObject whose plug-in (that lives in a subframe) has been destroyed
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+ PASS testObject.gettingProperty threw exception ReferenceError: Trying to access object from destroyed plug-in..
+PASS testObject.settingProperty = 10 threw exception ReferenceError: Trying to access object from destroyed plug-in..
+PASS testObject() threw exception TypeError: '' is not a function (evaluating 'testObject()').
+PASS new testObject(); threw exception TypeError: '' is not a constructor (evaluating 'new testObject()').
+
diff --git a/LayoutTests/plugins/npruntime/object-from-destroyed-plugin-in-subframe.html b/LayoutTests/plugins/npruntime/object-from-destroyed-plugin-in-subframe.html
new file mode 100644 (file)
index 0000000..66e0edc
--- /dev/null
@@ -0,0 +1,35 @@
+
+<script src="../../fast/js/resources/js-test-pre.js"></script>
+</head>
+<script>
+var subframe;
+var testObject;
+
+function runTest() {
+    subframe = document.getElementById('subframe');
+    subframe.contentWindow.document.documentElement.innerHTML = '<embed id="plugin" type="application/x-webkit-test-netscape" test="npruntime-object-from-destroyed-plugin"></embed>';
+
+    var plugin = subframe.contentWindow.document.getElementById('plugin');
+
+    // Get a reference to the plug-in test object.
+    testObject = plugin.testObject;
+
+    plugin.evaluate('window.top.subframe.parentNode.removeChild(window.top.subframe)')
+
+    // testObject is now a dangling object and every operation on it should throw.
+    shouldThrow('testObject.gettingProperty');
+    shouldThrow('testObject.settingProperty = 10');
+    shouldThrow('testObject()');
+    shouldThrow('new testObject();')
+}
+
+</script>
+<body onLoad="runTest()">
+<p id="description"></p>
+<iframe id="subframe"></iframe>
+<div id="console"></div>
+
+<script>
+description("Test various operation on an NPObject whose plug-in (that lives in a subframe) has been destroyed");
+
+</script>
index f58b4ac..2ecfec6 100644 (file)
@@ -1,3 +1,29 @@
+2012-01-05  Anders Carlsson  <andersca@apple.com>
+
+        Crash when trying to invalidate the NPRuntimeObjectMap for a plug-in in a subframe
+        https://bugs.webkit.org/show_bug.cgi?id=75667
+        <rdar://problem/10389454>
+
+        Reviewed by Kevin Decker.
+
+        NPRuntimeObjectMap::invalidate is called whenever a plug-in view is destroyed. If invalidate is called for an object map
+        whose plug-in has a null frame, we'd crash.
+
+        The plug-in will have a null frame if the plug-in view is destroyed because its containing frame has been removed from the document,
+        and if the plug-in view is being destroyed asynchronously due to the plug-in itself calling JavaScript that will remove the frame
+        (see PluginView::unprotectPluginFromDestruction).
+
+        The reason NPRuntimeObjectMap::invalidate will crash when the frame is null is because we were trying to access the frame's global
+        object, causing a null dereference. The reason we were trying to get at the frame's global object was to create a Strong handle to
+        a JSNPObject so we could stick the object in a vector so we could later iterate over the vector elements and call invalidate() on
+        each JSNPObject which will end up releasing the underlying NPObject.
+
+        However, it turns out that we don't need to stick the JSNPObject in a vector; we can just get the underlying NPObject directly and
+        stick that in a vector and then iterate over the NPObjects, releasing them.
+
+        * WebProcess/Plugins/Netscape/NPRuntimeObjectMap.cpp:
+        (WebKit::NPRuntimeObjectMap::invalidate):
+
 2012-01-05  Alexey Proskuryakov  <ap@apple.com>
 
         REGRESSION (r98912-r99538): Crash in WebKit::WebFrameLoaderClient::didDetectXSS
index 81263b6..5aadd13 100644 (file)
@@ -208,13 +208,15 @@ void NPRuntimeObjectMap::invalidate()
     // We shouldn't have any NPJSObjects left now.
     ASSERT(m_npJSObjects.isEmpty());
 
-    HashMap<NPObject*, JSC::Weak<JSNPObject> >::iterator end = m_jsNPObjects.end();
-    Vector<Strong<JSNPObject> > objects;
-    for (HashMap<NPObject*, JSC::Weak<JSNPObject> >::iterator ptr = m_jsNPObjects.begin(); ptr != end; ++ptr)
-        objects.append(Strong<JSNPObject>(globalObject()->globalData(), ptr->second));
+    Vector<NPObject*> objects;
+
+    for (HashMap<NPObject*, JSC::Weak<JSNPObject> >::iterator ptr = m_jsNPObjects.begin(), end = m_jsNPObjects.end(); ptr != end; ++ptr)
+        objects.append(ptr->second->leakNPObject());
+
     m_jsNPObjects.clear();
+
     for (size_t i = 0; i < objects.size(); ++i)
-        objects[i]->invalidate();
+        releaseNPObject(objects[i]);
     
     // Deal with any objects that were scheduled for delayed destruction
     if (m_npObjectsToFinalize.isEmpty())
index 83e4659..07ec5f8 100644 (file)
@@ -1,3 +1,21 @@
+2012-01-05  Anders Carlsson  <andersca@apple.com>
+
+        Crash when trying to invalidate the NPRuntimeObjectMap for a plug-in in a subframe
+        https://bugs.webkit.org/show_bug.cgi?id=75667
+        <rdar://problem/10389454>
+
+        Reviewed by Kevin Decker.
+
+        Add an evaluate method to the plug-in test scriptable object that can be used to evaluate a given JS string.
+
+        * DumpRenderTree/TestNetscapePlugIn/PluginTest.cpp:
+        (PluginTest::executeScript):
+        * DumpRenderTree/TestNetscapePlugIn/PluginTest.h:
+        * DumpRenderTree/TestNetscapePlugIn/Tests/NPRuntimeObjectFromDestroyedPlugin.cpp:
+        (NPRuntimeObjectFromDestroyedPlugin::ScriptableObject::hasMethod):
+        (NPRuntimeObjectFromDestroyedPlugin::ScriptableObject::invoke):
+        (NPRuntimeObjectFromDestroyedPlugin::ScriptableObject::hasProperty):
+
 2012-01-05  Dirk Pranke  <dpranke@chromium.org>
 
         webkitpy: clean up port factory methods
index 93e1456..989e16d 100644 (file)
@@ -220,17 +220,22 @@ bool PluginTest::NPN_ConvertPoint(double sourceX, double sourceY, NPCoordinateSp
 }
 #endif
 
-void PluginTest::executeScript(const char* script)
+bool PluginTest::executeScript(const NPString* script, NPVariant* result)
 {
     NPObject* windowScriptObject;
     browser->getvalue(m_npp, NPNVWindowNPObject, &windowScriptObject);
 
+    return browser->evaluate(m_npp, windowScriptObject, const_cast<NPString*>(script), result);
+}
+
+void PluginTest::executeScript(const char* script)
+{
     NPString npScript;
     npScript.UTF8Characters = script;
     npScript.UTF8Length = strlen(script);
 
     NPVariant browserResult;
-    browser->evaluate(m_npp, windowScriptObject, &npScript, &browserResult);
+    executeScript(&npScript, &browserResult);
     browser->releasevariantvalue(&browserResult);
 }
 
index 7f5cec2..e83e82c 100644 (file)
@@ -93,6 +93,7 @@ public:
     bool NPN_ConvertPoint(double sourceX, double sourceY, NPCoordinateSpace sourceSpace, double *destX, double *destY, NPCoordinateSpace destSpace);
 #endif
 
+    bool executeScript(const NPString*, NPVariant* result);
     void executeScript(const char*);
     void log(const char* format, ...);
 
index 38236e3..0e238e6 100644 (file)
@@ -38,12 +38,28 @@ private:
     // This is the test object.
     class TestObject : public Object<TestObject> { };
 
-    // This is the scriptable object. It has a single "testObject" property.
+    // This is the scriptable object. It has a single "testObject" property and an "evaluate" function.
     class ScriptableObject : public Object<ScriptableObject> { 
     public:
+        bool hasMethod(NPIdentifier methodName)
+        {
+            return identifierIs(methodName, "evaluate");
+        }
+
+        bool invoke(NPIdentifier methodName, const NPVariant* args, uint32_t argCount, NPVariant* result)
+        {
+            if (!identifierIs(methodName, "evaluate"))
+                return false;
+
+            if (argCount != 1 || !NPVARIANT_IS_STRING(args[0]))
+                return false;
+
+            return pluginTest()->executeScript(&NPVARIANT_TO_STRING(args[0]), result);
+        }
+
         bool hasProperty(NPIdentifier propertyName)
         {
-            return propertyName == pluginTest()->NPN_GetStringIdentifier("testObject");
+            return identifierIs(propertyName, "testObject");
         }
 
         bool getProperty(NPIdentifier propertyName, NPVariant* result)