Bug 115539 - [WK2][CustomProtocols] NSURLProtocolClient methods should be dispatched on NSURLConnection's resource loader run loop
Summary: [WK2][CustomProtocols] NSURLProtocolClient methods should be dispatched on NS...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: Mac All
: P2 Normal
Assignee: Andy Estes
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-05-02 17:26 PDT by Andy Estes
Modified: 2013-05-03 00:09 PDT (History)
1 user (show)

See Also:

Attachments
Patch (6.59 KB, patch)
2013-05-02 17:37 PDT, Andy Estes 		ap: review+
ap: commit-queue-
Details | Formatted Diff | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Andy Estes 2013-05-02 17:26:21 PDT 
[WK2][CustomProtocols] NSURLProtocolClient methods should be dispatched on NSURLConnection's resource loader run loop
Comment 1 Andy Estes 2013-05-02 17:28:26 PDT 
<rdar://problem/13695891>
Comment 2 Andy Estes 2013-05-02 17:37:03 PDT 
Created attachment 200378 [details]
Patch
Comment 3 Alexey Proskuryakov 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.
Comment 4 Andy Estes 2013-05-03 00:09:58 PDT 
Committed r149520: <http://trac.webkit.org/changeset/149520>