Changeset 34988 in webkit

Timestamp:

Jul 3, 2008 1:52:56 PM (16 years ago)

Author:

jhoneycutt@apple.com

Message:

<rdar://5983747> Safari crashes trying to load the SilverLight plugin

If a plug-in returned an error code from NPP_NewStream, we would call
NPP_DestroyStream while cleaning up the request. We now only call
NPP_DestroyStream if NPP_NewStream was successful, matching Firefox.

Reviewed by Anders.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/plugins/return-error-from-new-stream-doesnt-invoke-destroy-stream-expected.txt (added)
• LayoutTests/plugins/return-error-from-new-stream-doesnt-invoke-destroy-stream.html (added)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/plugins/PluginStream.cpp (modified) (2 diffs)
• WebKitTools/ChangeLog (modified) (1 diff)
• WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp (modified) (5 diffs)
• WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.h (modified) (1 diff)
• WebKitTools/DumpRenderTree/win/TestNetscapePlugin/main.cpp (modified) (5 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2008-07-02  Jon Honeycutt  <jhoneycutt@apple.com>
+
+        Test for <rdar://5983747> Safari crashes trying to load the SilverLight plugin
+
+        Reviewed by Anders.
+
+        * plugins/return-error-from-new-stream-doesnt-invoke-destroy-stream-expected.txt: Added.
+        * plugins/return-error-from-new-stream-doesnt-invoke-destroy-stream.html: Added.
+
 2008-07-03  Alexey Proskuryakov  <ap@webkit.org>
 

trunk/WebCore/ChangeLog

@@ +1 @@
+2008-07-02  Jon Honeycutt  <jhoneycutt@apple.com>
+
+        <rdar://5983747> Safari crashes trying to load the SilverLight plugin
+
+        If a plug-in returned an error code from NPP_NewStream, we would call
+        NPP_DestroyStream while cleaning up the request. We now only call
+        NPP_DestroyStream if NPP_NewStream was successful, matching Firefox.
+
+        Reviewed by Anders.
+
+        * plugins/PluginStream.cpp:
+        (WebCore::PluginStream::startStream): If NPP_NewStream returns an error,
+        don't set m_streamState to StreamStarted, and return after calling
+        cancelAndDestroyStream.
+        (WebCore::PluginStream::destroyStream): Don't call NPP_DestroyStream if
+        the stream didn't start successfully.
+
 2008-07-03  David Hyatt  <hyatt@apple.com>
 

trunk/WebCore/plugins/PluginStream.cpp

@@ -188 +188 @@
         return;
         
+    if (npErr != NPERR_NO_ERROR) {
+        cancelAndDestroyStream(npErr);
+        return;
+    }
+
     m_streamState = StreamStarted;
-
-    if (npErr != NPERR_NO_ERROR)
-        cancelAndDestroyStream(npErr);
 
     if (m_transferMode == NP_NORMAL)
@@ -253 +255 @@
         }
 
-        if (m_loader)
-            m_loader->setDefersLoading(true);
-        NPError npErr = m_pluginFuncs->destroystream(m_instance, &m_stream, m_reason);
-        if (m_loader)
-            m_loader->setDefersLoading(false);
-        LOG_NPERROR(npErr);
+        if (m_streamState != StreamBeforeStarted) {
+            if (m_loader)
+                m_loader->setDefersLoading(true);
+
+            NPError npErr = m_pluginFuncs->destroystream(m_instance, &m_stream, m_reason);
+
+            if (m_loader)
+                m_loader->setDefersLoading(false);
+
+            LOG_NPERROR(npErr);
+        }
 
         m_stream.ndata = 0;

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2008-07-02  Jon Honeycutt  <jhoneycutt@apple.com>
+
+        Allow tests to define JavaScript to execute when NPP_DestroyStream or
+        NPP_URLNotify is called.
+
+        Reviewed by Anders.
+
+        * DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp: Add a new
+        property, "returnErrorFromNewStream." This is to support the test for
+        <rdar://5983747> Safari crashes trying to load the SilverLight plugin,
+        caused by WebKit calling NPP_DestroyStream after a plug-in returns an
+        error from NPP_NewStream.
+        (pluginGetProperty): 
+        (pluginSetProperty):
+        (pluginAllocate):
+        * DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.h: Added new
+        members, onStreamDestroy and onURLNotify.
+        * DumpRenderTree/win/TestNetscapePlugin/main.cpp:
+        (NPP_New): Remove initialization of onStreamLoad; this was moved to
+        pluginAllocate. Look for new arguments onStreamDestroy and
+        onURLNotify, and store their values.
+        (NPP_Destroy): Free new members.
+        (executeScript): Code moved from onStreamLoad
+        (NPP_NewStream): If returnErrorFromNewStream has been set to true,
+        return a generic error code. If onStreamLoad is set, execute it as
+        JavaScript.
+        (NPP_DestroyStream): If onStreamDestroy is set, execute it as JS.
+        (NPP_URLNotify): Same, for onURLNotify.
+
 2008-07-02  Brady Eidson  <beidson@apple.com>
 

trunk/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.cpp

@@ -62 +62 @@
 static bool identifiersInitialized = false;
 
-#define ID_PROPERTY_PROPERTY        0
-#define ID_PROPERTY_EVENT_LOGGING   1
-#define ID_PROPERTY_HAS_STREAM      2
-#define ID_PROPERTY_TEST_OBJECT     3
-#define ID_PROPERTY_LOG_DESTROY     4
-#define NUM_PROPERTY_IDENTIFIERS    5
+#define ID_PROPERTY_PROPERTY                    0
+#define ID_PROPERTY_EVENT_LOGGING               1
+#define ID_PROPERTY_HAS_STREAM                  2
+#define ID_PROPERTY_TEST_OBJECT                 3
+#define ID_PROPERTY_LOG_DESTROY                 4
+#define ID_PROPERTY_RETURN_ERROR_FROM_NEWSTREAM 5
+#define NUM_PROPERTY_IDENTIFIERS                6
 
 static NPIdentifier pluginPropertyIdentifiers[NUM_PROPERTY_IDENTIFIERS];
@@ -76 +77 @@
     "testObject",
     "logDestroy",
+    "returnErrorFromNewStream",
 };
 
@@ -165 +167 @@
         OBJECT_TO_NPVARIANT(testObject, *result);
         return true;
+    } else if (name == pluginPropertyIdentifiers[ID_PROPERTY_RETURN_ERROR_FROM_NEWSTREAM]) {
+        BOOLEAN_TO_NPVARIANT(plugin->returnErrorFromNewStream, *result);
+        return true;
     }
     return false;
@@ -177 +182 @@
     } else if (name == pluginPropertyIdentifiers[ID_PROPERTY_LOG_DESTROY]) {
         plugin->logDestroy = NPVARIANT_TO_BOOLEAN(*variant);
+        return true;
+    } else if (name == pluginPropertyIdentifiers[ID_PROPERTY_RETURN_ERROR_FROM_NEWSTREAM]) {
+        plugin->returnErrorFromNewStream = NPVARIANT_TO_BOOLEAN(*variant);
         return true;
     }
@@ -575 +583 @@
     newInstance->testObject = browser->createobject(npp, getTestClass());
     newInstance->eventLogging = FALSE;
+    newInstance->onStreamLoad = 0;
+    newInstance->onStreamDestroy = 0;
+    newInstance->onURLNotify = 0;
     newInstance->logDestroy = FALSE;
     newInstance->logSetWindow = FALSE;

trunk/WebKitTools/DumpRenderTree/TestNetscapePlugIn.subproj/PluginObject.h

@@ -38 +38 @@
     NPStream* stream;
     char* onStreamLoad;
+    char* onStreamDestroy;
+    char* onURLNotify;
     char* firstUrl;
     char* firstHeaders;

trunk/WebKitTools/DumpRenderTree/win/TestNetscapePlugin/main.cpp

@@ -77 +77 @@
     if (browser->version >= 14) {
         PluginObject* obj = (PluginObject*)browser->createobject(instance, getPluginClass());
-    
-        obj->onStreamLoad = NULL;
         
         for (int16 i = 0; i < argc; i++) {
             if (_stricmp(argn[i], "onstreamload") == 0 && !obj->onStreamLoad)
                 obj->onStreamLoad = _strdup(argv[i]);
+            else if (_stricmp(argn[i], "onStreamDestroy") == 0 && !obj->onStreamDestroy)
+                obj->onStreamDestroy = _strdup(argv[i]);
+            else if (_stricmp(argn[i], "onURLNotify") == 0 && !obj->onURLNotify)
+                obj->onURLNotify = _strdup(argv[i]);
         }
         
@@ -97 +99 @@
         if (obj->onStreamLoad)
             free(obj->onStreamLoad);
-        
+
+        if (obj->onURLNotify)
+            free(obj->onURLNotify);
+
+        if (obj->onStreamDestroy)
+            free(obj->onStreamDestroy);
+
         if (obj->logDestroy)
             printf("PLUGIN: NPP_Destroy\n");
@@ -111 +119 @@
 }
 
+static void executeScript(const PluginObject* obj, const char* script)
+{
+    NPObject *windowScriptObject;
+    browser->getvalue(obj->npp, NPNVWindowNPObject, &windowScriptObject);
+
+    NPString npScript;
+    npScript.UTF8Characters = script;
+    npScript.UTF8Length = strlen(script);
+
+    NPVariant browserResult;
+    browser->evaluate(obj->npp, windowScriptObject, &npScript, &browserResult);
+    browser->releasevariantvalue(&browserResult);
+}
+
 NPError NPP_NewStream(NPP instance, NPMIMEType type, NPStream *stream, NPBool seekable, uint16 *stype)
 {
     PluginObject* obj = (PluginObject*)instance->pdata;
+
+    if (obj->returnErrorFromNewStream)
+        return NPERR_GENERIC_ERROR;
+
     obj->stream = stream;
     *stype = NP_ASFILEONLY;
 
-    if (obj->onStreamLoad) {
-        NPObject *windowScriptObject;
-        browser->getvalue(obj->npp, NPNVWindowNPObject, &windowScriptObject);
-                
-        NPString script;
-        script.UTF8Characters = obj->onStreamLoad;
-        script.UTF8Length = strlen(obj->onStreamLoad);
-        
-        NPVariant browserResult;
-        browser->evaluate(obj->npp, windowScriptObject, &script, &browserResult);
-        browser->releasevariantvalue(&browserResult);
-    }
+    if (obj->onStreamLoad)
+        executeScript(obj, obj->onStreamLoad);
     
     return NPERR_NO_ERROR;
@@ -135 +151 @@
 NPError NPP_DestroyStream(NPP instance, NPStream *stream, NPReason reason)
 {
+    PluginObject* obj = (PluginObject*)instance->pdata;
+
+    if (obj->onStreamDestroy)
+        executeScript(obj, obj->onStreamDestroy);
+
     return NPERR_NO_ERROR;
 }
@@ -169 +190 @@
 {
     PluginObject *obj = (PluginObject*)instance->pdata;
+
+    if (obj->onURLNotify)
+        executeScript(obj, obj->onURLNotify);
         
     handleCallback(obj, url, reason, notifyData);