Changeset 20107 in webkit

Timestamp:

Mar 11, 2007 8:24:45 PM (17 years ago)

Author:

darin

Message:

Reviewed by Adele.

• fix ​http://bugs.webkit.org/show_bug.cgi?id=12964 <rdar://problem/5045717> REGRESSION: crash in -[WebBaseNetscapePluginStream _deliverData] at simpsonsmovie.com (12964)

• Plugins/WebBaseNetscapePluginStream.m: (-[WebBaseNetscapePluginStream startStreamResponseURL:expectedContentLength:lastModifiedDate:MIMEType:]): At every place we call the plug-in, since it could destroy the stream, get pluginView into a local variable; it will be set to nil if the stream is destroyed. (-[WebBaseNetscapePluginStream _destroyStream]): Added calls to retain/release to handle the case where one of the calls to the plug-in destroys the stream. Added a call to cancelPreviousPerformRequestsWithTarget in case _deliverData has been scheduled but not yet delivered. Also get pluginView into a local variable as mentioned above, and check at strategic points and exit if the stream was already destroyed to avoid multiple calls to NPP_DestroyStream or NPP_URLNotify. (-[WebBaseNetscapePluginStream _deliverData]): Ditto.

Location:

trunk/WebKit

Files:

• ChangeLog (modified) (1 diff)
• Plugins/WebBaseNetscapePluginStream.m (modified) (8 diffs)

trunk/WebKit/ChangeLog

@@ +1 @@
+2007-03-11  Darin Adler  <darin@apple.com>
+
+        Reviewed by Adele.
+
+        - fix http://bugs.webkit.org/show_bug.cgi?id=12964
+          <rdar://problem/5045717> REGRESSION: crash in -[WebBaseNetscapePluginStream _deliverData]
+          at simpsonsmovie.com (12964)
+
+        * Plugins/WebBaseNetscapePluginStream.m:
+        (-[WebBaseNetscapePluginStream startStreamResponseURL:expectedContentLength:lastModifiedDate:MIMEType:]):
+        At every place we call the plug-in, since it could destroy the stream, get pluginView into a local
+        variable; it will be set to nil if the stream is destroyed.
+        (-[WebBaseNetscapePluginStream _destroyStream]): Added calls to retain/release to handle the case where
+        one of the calls to the plug-in destroys the stream. Added a call to cancelPreviousPerformRequestsWithTarget
+        in case _deliverData has been scheduled but not yet delivered. Also get pluginView into a local variable
+        as mentioned above, and check at strategic points and exit if the stream was already destroyed to avoid
+        multiple calls to NPP_DestroyStream or NPP_URLNotify.
+        (-[WebBaseNetscapePluginStream _deliverData]): Ditto.
+
 2007-03-10  Geoffrey Garen  <ggaren@apple.com>
 

trunk/WebKit/Plugins/WebBaseNetscapePluginStream.m

@@ -1 +1 @@
 /*
- * Copyright (C) 2005 Apple Computer, Inc.  All rights reserved.
+ * Copyright (C) 2005, 2006, 2007 Apple Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -232 +232 @@
     // FIXME: Need a way to check if stream is seekable
 
-    [pluginView willCallPlugInFunction];
+    WebBaseNetscapePluginView *pv = pluginView;
+    [pv willCallPlugInFunction];
     NPError npErr = NPP_NewStream(plugin, (char *)[MIMEType UTF8String], &stream, NO, &transferMode);
-    [pluginView didCallPlugInFunction];
+    [pv didCallPlugInFunction];
     LOG(Plugins, "NPP_NewStream URL=%@ MIME=%@ error=%d", responseURL, MIMEType, npErr);
 
@@ -275 +276 @@
     if (isTerminated)
         return;
-    
+
+    [self retain];
+
     ASSERT(reason != WEB_REASON_NONE);
     ASSERT([deliveryData length] == 0);
     
-    if (stream.ndata != NULL) {
+    [NSObject cancelPreviousPerformRequestsWithTarget:self selector:@selector(_deliverData) object:nil];
+
+    if (stream.ndata != nil) {
         if (reason == NPRES_DONE && (transferMode == NP_ASFILE || transferMode == NP_ASFILEONLY)) {
             ASSERT(path != NULL);
             char *carbonPath = CarbonPathFromPOSIXPath(path);
             ASSERT(carbonPath != NULL);
-            [pluginView willCallPlugInFunction];
+            WebBaseNetscapePluginView *pv = pluginView;
+            [pv willCallPlugInFunction];
             NPP_StreamAsFile(plugin, &stream, carbonPath);
-            [pluginView didCallPlugInFunction];
+            [pv didCallPlugInFunction];
 
             // Delete the file after calling NPP_StreamAsFile(), instead of in -dealloc/-finalize.  It should be OK
@@ -297 +303 @@
             LOG(Plugins, "NPP_StreamAsFile responseURL=%@ path=%s", responseURL, carbonPath);
             free(carbonPath);
-        }
-        
+
+            if (isTerminated)
+                goto exit;
+        }
+
         NPError npErr;
-        [pluginView willCallPlugInFunction];
+        WebBaseNetscapePluginView *pv = pluginView;
+        [pv willCallPlugInFunction];
         npErr = NPP_DestroyStream(plugin, &stream, reason);
-        [pluginView didCallPlugInFunction];
+        [pv didCallPlugInFunction];
         LOG(Plugins, "NPP_DestroyStream responseURL=%@ error=%d", responseURL, npErr);
-        
+
         stream.ndata = nil;
-    }
-    
+
+        if (isTerminated)
+            goto exit;
+    }
+
     if (sendNotification) {
         // NPP_URLNotify expects the request URL, not the response URL.
-        [pluginView willCallPlugInFunction];
+        WebBaseNetscapePluginView *pv = pluginView;
+        [pv willCallPlugInFunction];
         NPP_URLNotify(plugin, [requestURL _web_URLCString], reason, notifyData);
-        [pluginView didCallPlugInFunction];
+        [pv didCallPlugInFunction];
         LOG(Plugins, "NPP_URLNotify requestURL=%@ reason=%d", requestURL, reason);
     }
-    
+
     isTerminated = YES;
 
     [self setPlugin:NULL];
+
+exit:
+    [self release];
 }
 
@@ -396 +413 @@
     if (!stream.ndata || [deliveryData length] == 0)
         return;
-    
+
+    [self retain];
+
     int32 totalBytes = [deliveryData length];
     int32 totalBytesDelivered = 0;
-    
+
     while (totalBytesDelivered < totalBytes) {
-        [pluginView willCallPlugInFunction];
+        WebBaseNetscapePluginView *pv = pluginView;
+        [pv willCallPlugInFunction];
         int32 deliveryBytes = NPP_WriteReady(plugin, &stream);
-        [pluginView didCallPlugInFunction];
+        [pv didCallPlugInFunction];
         LOG(Plugins, "NPP_WriteReady responseURL=%@ bytes=%d", responseURL, deliveryBytes);
-        
+
+        if (isTerminated)
+            goto exit;
+
         if (deliveryBytes <= 0) {
             // Plug-in can't receive anymore data right now. Send it later.
@@ -413 +436 @@
             deliveryBytes = MIN(deliveryBytes, totalBytes - totalBytesDelivered);
             NSData *subdata = [deliveryData subdataWithRange:NSMakeRange(totalBytesDelivered, deliveryBytes)];
-            [pluginView willCallPlugInFunction];
+            pv = pluginView;
+            [pv willCallPlugInFunction];
             deliveryBytes = NPP_Write(plugin, &stream, offset, [subdata length], (void *)[subdata bytes]);
-            [pluginView didCallPlugInFunction];
+            [pv didCallPlugInFunction];
             if (deliveryBytes < 0) {
                 // Netscape documentation says that a negative result from NPP_Write means cancel the load.
@@ -427 +451 @@
         }
     }
-    
+
     if (totalBytesDelivered > 0) {
         if (totalBytesDelivered < totalBytes) {
@@ -441 +465 @@
         }
     }
+
+exit:
+    [self release];
 }