Changeset 149520 in webkit

Timestamp:

May 3, 2013 12:10:20 AM (11 years ago)

Author:

aestes@apple.com

Message:

[WK2][CustomProtocols] NSURLProtocolClient methods should be dispatched on NSURLConnection's resource loader run loop
​https://bugs.webkit.org/show_bug.cgi?id=115539

Reviewed by Alexey Proskuryakov.

It turns out that calling NSURLProtocolClient methods from a different
thread than the one running the NSURLConnection run loop is unsafe.
Although I can't capture it reliably in a test case, doing so can
sometimes result in a load timing out because the call to
-[NSURLProtocolClient URLProtocolDidFinishLoading:] was ignored by
NSURLConnection.

Fix this by dispatching these methods on the NSURLConnection resource
load run loop. This matches where NSURLProtocolClient methods are
dispatched by typical NSURLProtocol implementations, and in my testing
this solves the timeout issue.

• Shared/Network/CustomProtocols/CustomProtocolManager.h: Declare

dispatchOnResourceLoaderRunLoop().

• Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm:

Declare +[NSURLConnection resourceLoaderRunLoop] on a category of
NSURLConnection. Also include the header that declares it if it's present.
(WebKit::CustomProtocolManager::didFailWithError): Call the
NSURLProtocolClient method via dispatchOnResourceLoaderRunLoop().
(WebKit::CustomProtocolManager::didLoadData): Ditto.
(WebKit::CustomProtocolManager::didReceiveResponse): Ditto.
(WebKit::CustomProtocolManager::didFinishLoading): Ditto.
(WebKit::CustomProtocolManager::dispatchOnResourceLoaderRunLoop):
Dispatch a block on the NSURLConnection resource loader run loop and
then wake up the run loop.

Location:

trunk/Source/WebKit2

Files:

• ChangeLog (modified) (1 diff)
• Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm (modified) (6 diffs)

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2013-05-02  Andy Estes  <aestes@apple.com>
+
+        [WK2][CustomProtocols] NSURLProtocolClient methods should be dispatched on NSURLConnection's resource loader run loop
+        https://bugs.webkit.org/show_bug.cgi?id=115539
+
+        Reviewed by Alexey Proskuryakov.
+
+        It turns out that calling NSURLProtocolClient methods from a different
+        thread than the one running the NSURLConnection run loop is unsafe.
+        Although I can't capture it reliably in a test case, doing so can
+        sometimes result in a load timing out because the call to
+        -[NSURLProtocolClient URLProtocolDidFinishLoading:] was ignored by
+        NSURLConnection.
+
+        Fix this by dispatching these methods on the NSURLConnection resource
+        load run loop. This matches where NSURLProtocolClient methods are
+        dispatched by typical NSURLProtocol implementations, and in my testing
+        this solves the timeout issue.
+
+        * Shared/Network/CustomProtocols/CustomProtocolManager.h: Declare
+        dispatchOnResourceLoaderRunLoop().
+        * Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm:
+        Declare +[NSURLConnection resourceLoaderRunLoop] on a category of
+        NSURLConnection. Also include the header that declares it if it's present.
+        (WebKit::CustomProtocolManager::didFailWithError): Call the
+        NSURLProtocolClient method via dispatchOnResourceLoaderRunLoop().
+        (WebKit::CustomProtocolManager::didLoadData): Ditto.
+        (WebKit::CustomProtocolManager::didReceiveResponse): Ditto.
+        (WebKit::CustomProtocolManager::didFinishLoading): Ditto.
+        (WebKit::CustomProtocolManager::dispatchOnResourceLoaderRunLoop):
+        Dispatch a block on the NSURLConnection resource loader run loop and
+        then wake up the run loop.
+
 2013-05-02  Brady Eidson  <beidson@apple.com>
 

trunk/Source/WebKit2/Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm

@@ -44 +44 @@
 #endif
 
+#ifdef __has_include
+#if __has_include(<Foundation/NSURLConnectionPrivate.h>)
+#import <Foundation/NSURLConnectionPrivate.h>
+#endif
+#endif
+
+@interface NSURLConnection (Details)
++ (CFRunLoopRef)resourceLoaderRunLoop;
+@end
+
 using namespace WebKit;
 
@@ -187 +197 @@
 }
 
+static inline void dispatchOnResourceLoaderRunLoop(void (^block)())
+{
+    CFRunLoopPerformBlock([NSURLConnection resourceLoaderRunLoop], kCFRunLoopDefaultMode, block);
+    CFRunLoopWakeUp([NSURLConnection resourceLoaderRunLoop]);
+}
+
 void CustomProtocolManager::didFailWithError(uint64_t customProtocolID, const WebCore::ResourceError& error)
 {
@@ -193 +209 @@
         return;
 
-    [[protocol.get() client] URLProtocol:protocol.get() didFailWithError:error.nsError()];
+    RetainPtr<NSError> nsError = error.nsError();
+
+    dispatchOnResourceLoaderRunLoop(^ {
+        [[protocol.get() client] URLProtocol:protocol.get() didFailWithError:nsError.get()];
+    });
+
     removeCustomProtocol(protocol.get());
 }
@@ -202 +223 @@
     if (!protocol)
         return;
-    
-    [[protocol.get() client] URLProtocol:protocol.get() didLoadData:[NSData dataWithBytes:(void*)data.data() length:data.size()]];
+
+    RetainPtr<NSData> nsData = adoptNS([[NSData alloc] initWithBytes:data.data() length:data.size()]);
+
+    dispatchOnResourceLoaderRunLoop(^ {
+        [[protocol.get() client] URLProtocol:protocol.get() didLoadData:nsData.get()];
+    });
 }
 
@@ -211 +236 @@
     if (!protocol)
         return;
-    
-    [[protocol.get() client] URLProtocol:protocol.get() didReceiveResponse:response.nsURLResponse() cacheStoragePolicy:static_cast<NSURLCacheStoragePolicy>(cacheStoragePolicy)];
+
+    RetainPtr<NSURLResponse> nsResponse = response.nsURLResponse();
+
+    dispatchOnResourceLoaderRunLoop(^ {
+        [[protocol.get() client] URLProtocol:protocol.get() didReceiveResponse:nsResponse.get() cacheStoragePolicy:static_cast<NSURLCacheStoragePolicy>(cacheStoragePolicy)];
+    });
 }
 
@@ -220 +249 @@
     if (!protocol)
         return;
-    
-    [[protocol.get() client] URLProtocolDidFinishLoading:protocol.get()];
+
+    dispatchOnResourceLoaderRunLoop(^ {
+        [[protocol.get() client] URLProtocolDidFinishLoading:protocol.get()];
+    });
+
     removeCustomProtocol(protocol.get());
 }