Reviewed by Tim Hatcher.
authortomernic <tomernic@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Feb 2006 01:30:02 +0000 (01:30 +0000)
committertomernic <tomernic@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 10 Feb 2006 01:30:02 +0000 (01:30 +0000)
        <rdar://problem/4153419> CrashTracer: 576 crashes in Safari at com.apple.WebKit: NPN_DestroyStream + 56

        I never could reproduce this crasher, which seems to be caused by the Speed Download plugin.  However,
        I did find a way to make the affected code more bulletproof for those who are experiencing the crash.

        * Plugins/WebBaseNetscapePluginStream.h:
        Keep a WebBaseNetscapePluginView instead of the WebNetscapePluginPackage, since the plugin view could
        potentially be deallocated before the stream finishes loading.
        * Plugins/WebBaseNetscapePluginStream.m:
        (-[WebBaseNetscapePluginStream _pluginCancelledConnectionError]):
        Use pluginView instead of plugin.
        (-[WebBaseNetscapePluginStream dealloc]):
        Assert that the plugin instance has been nulled out, since that's now part of the stream's teardown
        phase.
        Release pluginView instead of plugin.
        (-[WebBaseNetscapePluginStream setPluginPointer:]):
        Retain the plugin view instead of the plugin package, since the plugin view could be deallocated while
        the stream is running.
        This method now accepts a NULL argument so that we can easily clear out the pluginView backpointer
        (and other ivars derived from it).
        (-[WebBaseNetscapePluginStream startStreamResponseURL:expectedContentLength:lastModifiedDate:MIMEType:]):
        Use pluginView instead of plugin.
        (-[WebBaseNetscapePluginStream _destroyStream]):
        ditto
        (-[WebBaseNetscapePluginStream finishedLoadingWithData:]):
        ditto
        (-[WebBaseNetscapePluginStream cancelLoadAndDestroyStreamWithError]):
        Set the plugin instance to NULL, so that the pluginView backpointer is released.  This method is called
        for every plugin view's stream when the plugin view is stopped/destroyed.
        (-[WebBaseNetscapePluginStream _deliverData]):
        Use pluginView instead of plugin.

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

WebKit/ChangeLog
WebKit/Plugins/WebBaseNetscapePluginStream.h
WebKit/Plugins/WebBaseNetscapePluginStream.m

index ce1e0ab351a589ec502315c69cb6eeb530b3310a..98318741172cb22a14cd1c10ee66f0dc834a8752 100644 (file)
@@ -1,2 +1,37 @@
+2006-02-09  Tim Omernick  <timo@apple.com>
+
+        Reviewed by Tim Hatcher.
+
+        <rdar://problem/4153419> CrashTracer: 576 crashes in Safari at com.apple.WebKit: NPN_DestroyStream + 56
+
+        I never could reproduce this crasher, which seems to be caused by the Speed Download plugin.  However,
+        I did find a way to make the affected code more bulletproof for those who are experiencing the crash.
+        
+        * Plugins/WebBaseNetscapePluginStream.h:
+        Keep a WebBaseNetscapePluginView instead of the WebNetscapePluginPackage, since the plugin view could
+        potentially be deallocated before the stream finishes loading.
+        * Plugins/WebBaseNetscapePluginStream.m:
+        (-[WebBaseNetscapePluginStream _pluginCancelledConnectionError]):
+        Use pluginView instead of plugin.
+        (-[WebBaseNetscapePluginStream dealloc]):
+        Assert that the plugin instance has been nulled out, since that's now part of the stream's teardown
+        phase.
+        Release pluginView instead of plugin.
+        (-[WebBaseNetscapePluginStream setPluginPointer:]):
+        Retain the plugin view instead of the plugin package, since the plugin view could be deallocated while
+        the stream is running.
+        This method now accepts a NULL argument so that we can easily clear out the pluginView backpointer
+        (and other ivars derived from it).
+        (-[WebBaseNetscapePluginStream startStreamResponseURL:expectedContentLength:lastModifiedDate:MIMEType:]):
+        Use pluginView instead of plugin.
+        (-[WebBaseNetscapePluginStream _destroyStream]):
+        ditto
+        (-[WebBaseNetscapePluginStream finishedLoadingWithData:]):
+        ditto
+        (-[WebBaseNetscapePluginStream cancelLoadAndDestroyStreamWithError]):
+        Set the plugin instance to NULL, so that the pluginView backpointer is released.  This method is called
+        for every plugin view's stream when the plugin view is stopped/destroyed.
+        (-[WebBaseNetscapePluginStream _deliverData]):
+        Use pluginView instead of plugin.
 
 == Rolled over to ChangeLog-2006-02-09 ==
index c784f7cca0494292d16bcef94a6ac0fd2ccfa8dc..3739eacb502bd56a65bd2b66ce2e9db685a10468 100644 (file)
@@ -30,7 +30,7 @@
 
 #import <WebKit/npfunctions.h>
 
-@class WebNetscapePluginPackage;
+@class WebBaseNetscapePluginView;
 @class NSURLResponse;
 
 @interface WebBaseNetscapePluginStream : NSObject
@@ -47,7 +47,7 @@
     char *path;
     BOOL sendNotification;
     void *notifyData;
-    WebNetscapePluginPackage *plugin;
+    WebBaseNetscapePluginView *pluginView;
     NPReason reason;
     BOOL isTerminated;
         
index d565476b98447c4b47e117fa13f845d588cfd8c8..9e7ad416ce1d0cea90f94c08be7f0c04f11a61ca 100644 (file)
@@ -60,7 +60,7 @@ static const char *CarbonPathFromPOSIXPath(const char *posixPath);
     return [[[NSError alloc] _initWithPluginErrorCode:WebKitErrorPlugInCancelledConnection
                                            contentURL:responseURL != nil ? responseURL : requestURL
                                         pluginPageURL:nil
-                                           pluginName:[plugin name]
+                                           pluginName:[[pluginView plugin] name]
                                              MIMEType:MIMEType] autorelease];
 }
 
@@ -104,6 +104,7 @@ static const char *CarbonPathFromPOSIXPath(const char *posixPath);
 
 - (void)dealloc
 {
+    ASSERT(!instance);
     ASSERT(isTerminated);
     ASSERT(stream.ndata == nil);
 
@@ -117,7 +118,7 @@ static const char *CarbonPathFromPOSIXPath(const char *posixPath);
     [requestURL release];
     [responseURL release];
     [MIMEType release];
-    [plugin release];
+    [pluginView release];
     [deliveryData release];
     
     free((void *)stream.url);
@@ -163,16 +164,27 @@ static const char *CarbonPathFromPOSIXPath(const char *posixPath);
 
 - (void)setPluginPointer:(NPP)pluginPointer
 {
-    instance = pluginPointer;
-    
-    plugin = [[(WebBaseNetscapePluginView *)instance->ndata plugin] retain];
-
-    NPP_NewStream =     [plugin NPP_NewStream];
-    NPP_WriteReady =    [plugin NPP_WriteReady];
-    NPP_Write =         [plugin NPP_Write];
-    NPP_StreamAsFile =  [plugin NPP_StreamAsFile];
-    NPP_DestroyStream = [plugin NPP_DestroyStream];
-    NPP_URLNotify =     [plugin NPP_URLNotify];
+    if (pluginPointer) {
+        instance = pluginPointer;
+        pluginView = [(WebBaseNetscapePluginView *)instance->ndata retain];
+        WebNetscapePluginPackage *plugin = [pluginView plugin];
+        NPP_NewStream = [plugin NPP_NewStream];
+        NPP_WriteReady = [plugin NPP_WriteReady];
+        NPP_Write = [plugin NPP_Write];
+        NPP_StreamAsFile = [plugin NPP_StreamAsFile];
+        NPP_DestroyStream = [plugin NPP_DestroyStream];
+        NPP_URLNotify = [plugin NPP_URLNotify];
+    } else {
+        instance = NULL;
+        [pluginView release];
+        pluginView = nil;
+        NPP_NewStream = NULL;
+        NPP_WriteReady = NULL;
+        NPP_Write = NULL;
+        NPP_StreamAsFile = NULL;
+        NPP_DestroyStream = NULL;
+        NPP_URLNotify = NULL;
+    }
 }
 
 - (void)setMIMEType:(NSString *)theMIMEType
@@ -189,7 +201,7 @@ static const char *CarbonPathFromPOSIXPath(const char *posixPath);
 {
     ASSERT(!isTerminated);
     
-    if (![plugin isLoaded]) {
+    if (![[pluginView plugin] isLoaded]) {
         return;
     }
     
@@ -249,7 +261,7 @@ static const char *CarbonPathFromPOSIXPath(const char *posixPath);
 
 - (void)_destroyStream
 {
-    if (isTerminated || ![plugin isLoaded]) {
+    if (isTerminated || ![[pluginView plugin] isLoaded]) {
         return;
     }
     
@@ -279,6 +291,8 @@ static const char *CarbonPathFromPOSIXPath(const char *posixPath);
     }
     
     isTerminated = YES;
+
+    [self setPluginPointer:NULL];
 }
 
 - (void)_destroyStreamWithReason:(NPReason)theReason
@@ -310,11 +324,12 @@ static const char *CarbonPathFromPOSIXPath(const char *posixPath);
 {
     [self cancelLoadWithError:error];
     [self destroyStreamWithError:error];
+    [self setPluginPointer:NULL];
 }
 
 - (void)finishedLoadingWithData:(NSData *)data
 {
-    if (![plugin isLoaded] || !stream.ndata) {
+    if (![[pluginView plugin] isLoaded] || !stream.ndata) {
         return;
     }
     
@@ -352,7 +367,7 @@ static const char *CarbonPathFromPOSIXPath(const char *posixPath);
 
 - (void)_deliverData
 {
-    if (![plugin isLoaded] || !stream.ndata || [deliveryData length] == 0) {
+    if (![[pluginView plugin] isLoaded] || !stream.ndata || [deliveryData length] == 0) {
         return;
     }