[webkit-reviews] review granted: [Bug 115539] [WK2][CustomProtocols] NSURLProtocolClient methods should be dispatched on NSURLConnection's resource loader run loop : [Attachment 200378] Patch

[bugzilla-daemon at webkit.org]

Alexey Proskuryakov <ap at webkit.org> has granted Andy Estes <aestes at apple.com>'s
request for review:
Bug 115539: [WK2][CustomProtocols] NSURLProtocolClient methods should be
dispatched on NSURLConnection's resource loader run loop
https://bugs.webkit.org/show_bug.cgi?id=115539

Attachment 200378: Patch
https://bugs.webkit.org/attachment.cgi?id=200378&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=200378&action=review


>
Source/WebKit2/Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm:2
05
> +    RetainPtr<NSError> nsError(error.nsError());

I think that we prefer assignment syntax to initialization.

>
Source/WebKit2/Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm:2
20
> +    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:2
33
> +    RetainPtr<NSURLResponse> nsResponse(response.nsURLResponse());

I think that we prefer assignment syntax to initialization.

>
Source/WebKit2/Shared/Network/CustomProtocols/mac/CustomProtocolManagerMac.mm:2
69
> +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.