Bug 115539

Summary: [WK2][CustomProtocols] NSURLProtocolClient methods should be dispatched on NSURLConnection's resource loader run loop
Product: WebKit Reporter: Andy Estes <aestes>
Component: WebKit2Assignee: Andy Estes <aestes>
Status: RESOLVED FIXED    
Severity: Normal CC: webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: All   
Attachments:
Description Flags
Patch ap: review+, ap: commit-queue-

Andy Estes
Reported 2013-05-02 17:26:21 PDT
[WK2][CustomProtocols] NSURLProtocolClient methods should be dispatched on NSURLConnection's resource loader run loop
Attachments
Patch (6.59 KB, patch)
2013-05-02 17:37 PDT, Andy Estes
ap: review+
ap: commit-queue-
Andy Estes
Comment 1 2013-05-02 17:28:26 PDT
Andy Estes
Comment 2 2013-05-02 17:37:03 PDT
Alexey Proskuryakov
Comment 3 2013-05-02 23:01:41 PDT
Comment on attachment 200378 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=200378&action=review > Source/WebKit2/Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm:205 > + RetainPtr<NSError> nsError(error.nsError()); I think that we prefer assignment syntax to initialization. > Source/WebKit2/Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm:220 > + RetainPtr<NSData> nsData([NSData dataWithBytes:(void*)data.data() length:data.size()]); This should be RetainPtr<NSData> nsData = adoptNS([[NSData alloc] initWithBytes:...]). No need to create an autoreleased object and thrash its refcount. > Source/WebKit2/Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm:233 > + RetainPtr<NSURLResponse> nsResponse(response.nsURLResponse()); I think that we prefer assignment syntax to initialization. > Source/WebKit2/Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm:269 > +void CustomProtocolManager::dispatchOnResourceLoaderRunLoop(void (^block)()) > +{ > + CFRunLoopPerformBlock([NSURLConnection resourceLoaderRunLoop], kCFRunLoopDefaultMode, block); > + CFRunLoopWakeUp([NSURLConnection resourceLoaderRunLoop]); > +} This looks like it should be a static function in .mm file.
Andy Estes
Comment 4 2013-05-03 00:09:58 PDT
Note You need to log in before you can comment on or make changes to this bug.