[webkit-changes] [149520] trunk/Source/WebKit2

Fri, 03 May 2013 00:08:36 -0700

Title: [149520] trunk/Source/WebKit2
Revision
149520
Author
[email protected]
Date
2013-05-03 00:10:20 -0700 (Fri, 03 May 2013)

Log 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.

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (149519 => 149520)


--- trunk/Source/WebKit2/ChangeLog	2013-05-03 06:47:37 UTC (rev 149519)
+++ trunk/Source/WebKit2/ChangeLog	2013-05-03 07:10:20 UTC (rev 149520)
@@ -1,3 +1,36 @@
+2013-05-02  Andy Estes  <[email protected]>
+
+        [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  <[email protected]>
 
         REGRESSION (149389) Main resources converted to downloads are immediately cancelled.

Modified: trunk/Source/WebKit2/Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm (149519 => 149520)


--- trunk/Source/WebKit2/Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm	2013-05-03 06:47:37 UTC (rev 149519)
+++ trunk/Source/WebKit2/Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm	2013-05-03 07:10:20 UTC (rev 149520)
@@ -43,6 +43,16 @@
 #import "NetworkProcessCreationParameters.h"
 #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;
 
 namespace WebKit {
@@ -186,13 +196,24 @@
     return m_registeredSchemes.contains(scheme);
 }
 
+static inline void dispatchOnResourceLoaderRunLoop(void (^block)())
+{
+    CFRunLoopPerformBlock([NSURLConnection resourceLoaderRunLoop], kCFRunLoopDefaultMode, block);
+    CFRunLoopWakeUp([NSURLConnection resourceLoaderRunLoop]);
+}
+
 void CustomProtocolManager::didFailWithError(uint64_t customProtocolID, const WebCore::ResourceError& error)
 {
     RetainPtr<WKCustomProtocol> protocol = protocolForID(customProtocolID);
     if (!protocol)
         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());
 }
 
@@ -201,8 +222,12 @@
     RetainPtr<WKCustomProtocol> protocol = protocolForID(customProtocolID);
     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()];
+    });
 }
 
 void CustomProtocolManager::didReceiveResponse(uint64_t customProtocolID, const WebCore::ResourceResponse& response, uint32_t cacheStoragePolicy)
@@ -210,8 +235,12 @@
     RetainPtr<WKCustomProtocol> protocol = protocolForID(customProtocolID);
     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)];
+    });
 }
 
 void CustomProtocolManager::didFinishLoading(uint64_t customProtocolID)
@@ -219,8 +248,11 @@
     RetainPtr<WKCustomProtocol> protocol = protocolForID(customProtocolID);
     if (!protocol)
         return;
-    
-    [[protocol.get() client] URLProtocolDidFinishLoading:protocol.get()];
+
+    dispatchOnResourceLoaderRunLoop(^ {
+        [[protocol.get() client] URLProtocolDidFinishLoading:protocol.get()];
+    });
+
     removeCustomProtocol(protocol.get());
 }
 

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to