Title: [117012] trunk
Revision
117012
Author
[email protected]
Date
2012-05-14 16:37:39 -0700 (Mon, 14 May 2012)

Log Message

Source/WebCore: [V8] Crash in npObjectGetProperty() in V8NPObject.cpp
https://bugs.webkit.org/show_bug.cgi?id=86131

Reviewed by Adam Barth.

Tests: plugins/npruntime/delete-plugin-within-getProperty.html
       plugins/npruntime/delete-plugin-within-hasProperty-return-false.html
       plugins/npruntime/delete-plugin-within-hasProperty-return-true.html
       plugins/npruntime/delete-plugin-within-invoke.html
       plugins/npruntime/delete-plugin-within-setProperty.html

* bindings/v8/NPV8Object.cpp:
(_NPN_EvaluateHelper):
* bindings/v8/V8NPObject.cpp: Check NPN_IsAlive in a bunch of places we're not currently.
(WebCore::npObjectInvokeImpl):
(WebCore::npObjectGetProperty):
(WebCore::npObjectSetProperty):

Tools: Add end-of-life test cases for https://bugs.webkit.org/show_bug.cgi?id=86131.

Reviewed by Adam Barth.

* DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:
(callDeletePlugin):
(pluginHasProperty):
(pluginHasMethod):
(pluginGetProperty):
(pluginSetProperty):
(pluginInvoke):

LayoutTests: Test for https://bugs.webkit.org/show_bug.cgi?id=86131.

Reviewed by Adam Barth.

* plugins/npruntime/delete-plugin-within-getProperty-expected.txt: Added.
* plugins/npruntime/delete-plugin-within-getProperty.html: Added.
* plugins/npruntime/delete-plugin-within-hasProperty-return-false-expected.txt: Added.
* plugins/npruntime/delete-plugin-within-hasProperty-return-false.html: Added.
* plugins/npruntime/delete-plugin-within-hasProperty-return-true-expected.txt: Added.
* plugins/npruntime/delete-plugin-within-hasProperty-return-true.html: Added.
* plugins/npruntime/delete-plugin-within-invoke-expected.txt: Added.
* plugins/npruntime/delete-plugin-within-invoke.html: Added.
* plugins/npruntime/delete-plugin-within-setProperty-expected.txt: Added.
* plugins/npruntime/delete-plugin-within-setProperty.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (117011 => 117012)


--- trunk/LayoutTests/ChangeLog	2012-05-14 23:19:09 UTC (rev 117011)
+++ trunk/LayoutTests/ChangeLog	2012-05-14 23:37:39 UTC (rev 117012)
@@ -1,3 +1,20 @@
+2012-05-14  Nate Chapin  <[email protected]>
+
+        Test for https://bugs.webkit.org/show_bug.cgi?id=86131.
+
+        Reviewed by Adam Barth.
+
+        * plugins/npruntime/delete-plugin-within-getProperty-expected.txt: Added.
+        * plugins/npruntime/delete-plugin-within-getProperty.html: Added.
+        * plugins/npruntime/delete-plugin-within-hasProperty-return-false-expected.txt: Added.
+        * plugins/npruntime/delete-plugin-within-hasProperty-return-false.html: Added.
+        * plugins/npruntime/delete-plugin-within-hasProperty-return-true-expected.txt: Added.
+        * plugins/npruntime/delete-plugin-within-hasProperty-return-true.html: Added.
+        * plugins/npruntime/delete-plugin-within-invoke-expected.txt: Added.
+        * plugins/npruntime/delete-plugin-within-invoke.html: Added.
+        * plugins/npruntime/delete-plugin-within-setProperty-expected.txt: Added.
+        * plugins/npruntime/delete-plugin-within-setProperty.html: Added.
+
 2012-05-14  Dirk Pranke  <[email protected]>
 
         Rebaseline http/tests/misc/will-send-request-returns-null-on-redirect on chromium.

Added: trunk/LayoutTests/plugins/npruntime/delete-plugin-within-getProperty-expected.txt (0 => 117012)


--- trunk/LayoutTests/plugins/npruntime/delete-plugin-within-getProperty-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/plugins/npruntime/delete-plugin-within-getProperty-expected.txt	2012-05-14 23:37:39 UTC (rev 117012)
@@ -0,0 +1 @@
+

Added: trunk/LayoutTests/plugins/npruntime/delete-plugin-within-getProperty.html (0 => 117012)


--- trunk/LayoutTests/plugins/npruntime/delete-plugin-within-getProperty.html	                        (rev 0)
+++ trunk/LayoutTests/plugins/npruntime/delete-plugin-within-getProperty.html	2012-05-14 23:37:39 UTC (rev 117012)
@@ -0,0 +1,16 @@
+<html>
+<body>
+<embed name="plg" type="application/x-webkit-test-netscape"></embed>
+<script>
+
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+    
+    function deletePlugin() {
+        document.body.innerHTML = ""
+    }
+
+    var test = plg.deletePluginInGetProperty;
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-false-expected.txt (0 => 117012)


--- trunk/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-false-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-false-expected.txt	2012-05-14 23:37:39 UTC (rev 117012)
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: line 13: Uncaught ReferenceError: NPObject deleted
+

Added: trunk/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-false.html (0 => 117012)


--- trunk/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-false.html	                        (rev 0)
+++ trunk/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-false.html	2012-05-14 23:37:39 UTC (rev 117012)
@@ -0,0 +1,16 @@
+<html>
+<body>
+<embed name="plg" type="application/x-webkit-test-netscape"></embed>
+<script>
+
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+    
+    function deletePlugin() {
+        document.body.innerHTML = ""
+    }
+
+    plg.deletePluginReturnFalse;
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-true-expected.txt (0 => 117012)


--- trunk/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-true-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-true-expected.txt	2012-05-14 23:37:39 UTC (rev 117012)
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: line 13: Uncaught ReferenceError: NPObject deleted
+

Added: trunk/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-true.html (0 => 117012)


--- trunk/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-true.html	                        (rev 0)
+++ trunk/LayoutTests/plugins/npruntime/delete-plugin-within-hasProperty-return-true.html	2012-05-14 23:37:39 UTC (rev 117012)
@@ -0,0 +1,16 @@
+<html>
+<body>
+<embed name="plg" type="application/x-webkit-test-netscape"></embed>
+<script>
+
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+    
+    function deletePlugin() {
+        document.body.innerHTML = ""
+    }
+    
+    plg.deletePluginReturnTrue;
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/plugins/npruntime/delete-plugin-within-invoke-expected.txt (0 => 117012)


--- trunk/LayoutTests/plugins/npruntime/delete-plugin-within-invoke-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/plugins/npruntime/delete-plugin-within-invoke-expected.txt	2012-05-14 23:37:39 UTC (rev 117012)
@@ -0,0 +1 @@
+

Added: trunk/LayoutTests/plugins/npruntime/delete-plugin-within-invoke.html (0 => 117012)


--- trunk/LayoutTests/plugins/npruntime/delete-plugin-within-invoke.html	                        (rev 0)
+++ trunk/LayoutTests/plugins/npruntime/delete-plugin-within-invoke.html	2012-05-14 23:37:39 UTC (rev 117012)
@@ -0,0 +1,16 @@
+<html>
+<body>
+<embed name="plg" type="application/x-webkit-test-netscape"></embed>
+<script>
+
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+    
+    function deletePlugin() {
+        document.body.innerHTML = ""
+    }
+
+    plg.testDeleteWithinInvoke();
+</script>
+</body>
+</html>

Added: trunk/LayoutTests/plugins/npruntime/delete-plugin-within-setProperty-expected.txt (0 => 117012)


--- trunk/LayoutTests/plugins/npruntime/delete-plugin-within-setProperty-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/plugins/npruntime/delete-plugin-within-setProperty-expected.txt	2012-05-14 23:37:39 UTC (rev 117012)
@@ -0,0 +1,2 @@
+CONSOLE MESSAGE: line 13: Uncaught ReferenceError: NPObject deleted
+

Added: trunk/LayoutTests/plugins/npruntime/delete-plugin-within-setProperty.html (0 => 117012)


--- trunk/LayoutTests/plugins/npruntime/delete-plugin-within-setProperty.html	                        (rev 0)
+++ trunk/LayoutTests/plugins/npruntime/delete-plugin-within-setProperty.html	2012-05-14 23:37:39 UTC (rev 117012)
@@ -0,0 +1,16 @@
+<html>
+<body>
+<embed name="plg" type="application/x-webkit-test-netscape"></embed>
+<script>
+
+    if (window.layoutTestController)
+        layoutTestController.dumpAsText();
+    
+    function deletePlugin() {
+        document.body.innerHTML = ""
+    }
+
+    plg.deletePluginReturnTrue = true;
+</script>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (117011 => 117012)


--- trunk/Source/WebCore/ChangeLog	2012-05-14 23:19:09 UTC (rev 117011)
+++ trunk/Source/WebCore/ChangeLog	2012-05-14 23:37:39 UTC (rev 117012)
@@ -1,3 +1,23 @@
+2012-05-14  Nate Chapin  <[email protected]>
+
+        [V8] Crash in npObjectGetProperty() in V8NPObject.cpp
+        https://bugs.webkit.org/show_bug.cgi?id=86131
+
+        Reviewed by Adam Barth.
+
+        Tests: plugins/npruntime/delete-plugin-within-getProperty.html
+               plugins/npruntime/delete-plugin-within-hasProperty-return-false.html
+               plugins/npruntime/delete-plugin-within-hasProperty-return-true.html
+               plugins/npruntime/delete-plugin-within-invoke.html
+               plugins/npruntime/delete-plugin-within-setProperty.html
+
+        * bindings/v8/NPV8Object.cpp:
+        (_NPN_EvaluateHelper):
+        * bindings/v8/V8NPObject.cpp: Check NPN_IsAlive in a bunch of places we're not currently.
+        (WebCore::npObjectInvokeImpl):
+        (WebCore::npObjectGetProperty):
+        (WebCore::npObjectSetProperty):
+
 2012-05-14  Brent Fulgham  <[email protected]>
 
         [WinCairo] Unreviewed build correction.

Modified: trunk/Source/WebCore/bindings/v8/NPV8Object.cpp (117011 => 117012)


--- trunk/Source/WebCore/bindings/v8/NPV8Object.cpp	2012-05-14 23:19:09 UTC (rev 117011)
+++ trunk/Source/WebCore/bindings/v8/NPV8Object.cpp	2012-05-14 23:37:39 UTC (rev 117012)
@@ -336,7 +336,8 @@
     if (v8result.IsEmpty())
         return false;
 
-    convertV8ObjectToNPVariant(v8result, npObject, result);
+    if (_NPN_IsAlive(npObject))
+        convertV8ObjectToNPVariant(v8result, npObject, result);
     return true;
 }
 

Modified: trunk/Source/WebCore/bindings/v8/V8NPObject.cpp (117011 => 117012)


--- trunk/Source/WebCore/bindings/v8/V8NPObject.cpp	2012-05-14 23:19:09 UTC (rev 117011)
+++ trunk/Source/WebCore/bindings/v8/V8NPObject.cpp	2012-05-14 23:37:39 UTC (rev 117012)
@@ -139,7 +139,9 @@
         _NPN_ReleaseVariantValue(&npArgs[i]);
 
     // Unwrap return values.
-    v8::Handle<v8::Value> returnValue = convertNPVariantToV8Object(&result, npObject);
+    v8::Handle<v8::Value> returnValue;
+    if (_NPN_IsAlive(npObject))
+        returnValue = convertNPVariantToV8Object(&result, npObject);
     _NPN_ReleaseVariantValue(&result);
 
     return returnValue;
@@ -191,21 +193,30 @@
         return throwError("NPObject deleted", V8Proxy::ReferenceError);
 
 
-    if (npObject->_class->hasProperty && npObject->_class->hasProperty(npObject, identifier)
-        && npObject->_class->getProperty) {
+    if (npObject->_class->hasProperty && npObject->_class->getProperty && npObject->_class->hasProperty(npObject, identifier)) {
+        if (!_NPN_IsAlive(npObject))
+            return throwError("NPObject deleted", V8Proxy::ReferenceError);
 
         NPVariant result;
         VOID_TO_NPVARIANT(result);
         if (!npObject->_class->getProperty(npObject, identifier, &result))
             return v8::Handle<v8::Value>();
 
-        v8::Handle<v8::Value> returnValue = convertNPVariantToV8Object(&result, npObject);
+        v8::Handle<v8::Value> returnValue;
+        if (_NPN_IsAlive(npObject))
+            returnValue = convertNPVariantToV8Object(&result, npObject);
         _NPN_ReleaseVariantValue(&result);
         return returnValue;
 
     }
 
+    if (!_NPN_IsAlive(npObject))
+        return throwError("NPObject deleted", V8Proxy::ReferenceError);
+
     if (key->IsString() && npObject->_class->hasMethod && npObject->_class->hasMethod(npObject, identifier)) {
+        if (!_NPN_IsAlive(npObject))
+            return throwError("NPObject deleted", V8Proxy::ReferenceError);
+
         PrivateIdentifier* id = static_cast<PrivateIdentifier*>(identifier);
         v8::Persistent<v8::FunctionTemplate> functionTemplate = staticTemplateMap().get(id);
         // Cache templates using identifier as the key.
@@ -266,8 +277,9 @@
         return value;  // Intercepted, but an exception was thrown.
     }
 
-    if (npObject->_class->hasProperty && npObject->_class->hasProperty(npObject, identifier)
-        && npObject->_class->setProperty) {
+    if (npObject->_class->hasProperty && npObject->_class->setProperty && npObject->_class->hasProperty(npObject, identifier)) {
+        if (!_NPN_IsAlive(npObject))
+            return throwError("NPObject deleted", V8Proxy::ReferenceError);
 
         NPVariant npValue;
         VOID_TO_NPVARIANT(npValue);

Modified: trunk/Tools/ChangeLog (117011 => 117012)


--- trunk/Tools/ChangeLog	2012-05-14 23:19:09 UTC (rev 117011)
+++ trunk/Tools/ChangeLog	2012-05-14 23:37:39 UTC (rev 117012)
@@ -1,3 +1,17 @@
+2012-05-14  Nate Chapin  <[email protected]>
+
+        Add end-of-life test cases for https://bugs.webkit.org/show_bug.cgi?id=86131.
+
+        Reviewed by Adam Barth.
+
+        * DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp:
+        (callDeletePlugin):
+        (pluginHasProperty):
+        (pluginHasMethod):
+        (pluginGetProperty):
+        (pluginSetProperty):
+        (pluginInvoke):
+
 2012-05-14  Dirk Pranke  <[email protected]>
 
         Re-enable "drt mode" on chromium-mac-leopard

Modified: trunk/Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp (117011 => 117012)


--- trunk/Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp	2012-05-14 23:19:09 UTC (rev 117011)
+++ trunk/Tools/DumpRenderTree/TestNetscapePlugIn/PluginObject.cpp	2012-05-14 23:37:39 UTC (rev 117012)
@@ -145,6 +145,7 @@
     ID_LAST_SET_WINDOW_ARGUMENTS,
     ID_PROPERTY_WINDOWED_PLUGIN,
     ID_PROPERTY_TEST_OBJECT_COUNT,
+    ID_PROPERTY_DELETE_IN_GET_PROPERTY,
     NUM_PROPERTY_IDENTIFIERS
 };
 
@@ -161,6 +162,7 @@
     "lastSetWindowArguments",
     "windowedPlugin",
     "testObjectCount",
+    "deletePluginInGetProperty"
 };
 
 enum {
@@ -202,6 +204,7 @@
     ID_NORMALIZE,
     ID_INVALIDATE_RECT,
     ID_OBJECTS_ARE_SAME,
+    ID_TEST_DELETE_WITHIN_INVOKE,
     NUM_METHOD_IDENTIFIERS
 };
 
@@ -244,7 +247,8 @@
     "resizeTo",
     "normalize",
     "invalidateRect",
-    "objectsAreSame"
+    "objectsAreSame",
+    "testDeleteWithinInvoke"
 };
 
 static NPUTF8* createCStringFromNPVariant(const NPVariant* variant)
@@ -262,8 +266,30 @@
     browser->getstringidentifiers(pluginMethodIdentifierNames, NUM_METHOD_IDENTIFIERS, pluginMethodIdentifiers);
 }
 
+static bool callDeletePlugin(NPObject* obj, NPIdentifier name, NPIdentifier identifierToMatch)
+{
+    if (name != identifierToMatch)
+        return false;
+
+    PluginObject* plugin = reinterpret_cast<PluginObject*>(obj);
+    NPObject* windowScriptObject;
+    browser->getvalue(plugin->npp, NPNVWindowNPObject, &windowScriptObject);
+
+    NPIdentifier callbackIdentifier = browser->getstringidentifier("deletePlugin");
+    NPVariant browserResult;
+    if (browser->invoke(plugin->npp, windowScriptObject, callbackIdentifier, 0, 0, &browserResult))
+        browser->releasevariantvalue(&browserResult);
+    return true;
+}
+
 static bool pluginHasProperty(NPObject *obj, NPIdentifier name)
 {
+    if (callDeletePlugin(obj, name, browser->getstringidentifier("deletePluginReturnTrue")))
+        return true;
+
+    if (callDeletePlugin(obj, name, browser->getstringidentifier("deletePluginReturnFalse")))
+        return false;
+
     for (int i = 0; i < NUM_PROPERTY_IDENTIFIERS; i++)
         if (name == pluginPropertyIdentifiers[i])
             return true;
@@ -272,6 +298,9 @@
 
 static bool pluginHasMethod(NPObject *obj, NPIdentifier name)
 {
+    if (callDeletePlugin(obj, name, browser->getstringidentifier("deletePluginInHasMethod")))
+        return true;
+
     for (int i = 0; i < NUM_METHOD_IDENTIFIERS; i++)
         if (name == pluginMethodIdentifiers[i])
             return true;
@@ -331,12 +360,25 @@
         return true;
     }
 
+    if (name == pluginPropertyIdentifiers[ID_PROPERTY_DELETE_IN_GET_PROPERTY]) {
+        browser->retainobject(obj);
+        callDeletePlugin(obj, name, pluginPropertyIdentifiers[ID_PROPERTY_DELETE_IN_GET_PROPERTY]);
+        NPObject* testObject = plugin->testObject;
+        browser->retainobject(testObject);
+        OBJECT_TO_NPVARIANT(testObject, *result);
+        browser->releaseobject(obj);
+        return true;
+    }
+
     return false;
 }
 
 static bool pluginSetProperty(NPObject* obj, NPIdentifier name, const NPVariant* variant)
 {
     PluginObject* plugin = reinterpret_cast<PluginObject*>(obj);
+    if (callDeletePlugin(obj, name, browser->getstringidentifier("deletePluginReturnTrue")))
+        return true;
+
     if (name == pluginPropertyIdentifiers[ID_PROPERTY_EVENT_LOGGING]) {
         plugin->eventLogging = NPVARIANT_TO_BOOLEAN(*variant);
         return true;
@@ -1123,7 +1165,12 @@
         return invalidateRect(plugin, args, argCount, result);
     if (name == pluginMethodIdentifiers[ID_OBJECTS_ARE_SAME])
         return objectsAreSame(plugin, args, argCount, result);
-
+    if (name == pluginMethodIdentifiers[ID_TEST_DELETE_WITHIN_INVOKE]) {
+        NPObject* newObject = browser->createobject(plugin->npp, &pluginClass);
+        OBJECT_TO_NPVARIANT(newObject, *result);
+        callDeletePlugin(header, name, pluginMethodIdentifiers[ID_TEST_DELETE_WITHIN_INVOKE]);
+        return true;
+    }
     return false;
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to