Implement custom protocols when using NetworkSession

[Alex Christensen]

Implement custom protocols when using NetworkSession

[Alex Christensen]

Created attachment 268900 [details]
Patch

[Andy Estes]

Comment on attachment 268900 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=268900&action=review

> Source/WebKit2/NetworkProcess/CustomProtocols/Cocoa/CustomProtocolManagerCocoa.mm:194
> +    auto copy = Block_copy(block);
> +    callOnMainThread([copy] {
> +        copy();
> +        Block_release(copy);
> +    });

This isn't right. NSURLSession might deliver your delegate callbacks on the main thread, but it interacts with NSURLProtocol instances on a private background queue. You can see this by setting a breakpoint in -[WKCustomProtocol startLoading]. You'll see a backtrace like this:

* thread #8: tid = 0x2b100f, 0x000000010000107c NSURLSessionTest`-[CustomProtocol startLoading](self=0x0000658000000190, _cmd="startLoading") + 12 at ViewController.m:28, name = 'com.apple.NSURLConnectionLoader', stop reason = breakpoint 1.1
  * frame #0: 0x000000010000107c NSURLSessionTest`-[CustomProtocol startLoading](self=0x0000658000000190, _cmd="startLoading") + 12 at ViewController.m:28
    frame #1: 0x0000000100055fd4 CFNetwork`___ZN19URLConnectionLoader27_private_ScheduleOriginLoadEPK12NSURLRequestPK20_CFCachedURLResponse_block_invoke_2 + 61
    frame #2: 0x0000000100055f75 CFNetwork`___ZNK19URLConnectionLoader25withExistingProtocolAsyncEU13block_pointerFvP11URLProtocolE_block_invoke + 25
    frame #3: 0x00000001008abbf9 libdispatch.dylib`_dispatch_client_callout + 8
    frame #4: 0x00000001008bbe09 libdispatch.dylib`_dispatch_block_invoke + 920
    frame #5: 0x0000000100054e00 CFNetwork`RunloopBlockContext::_invoke_block(void const*, void*) + 24
    frame #6: 0x00007fff8f5e84d4 CoreFoundation`CFArrayApplyFunction + 68
    frame #7: 0x0000000100054cf9 CFNetwork`RunloopBlockContext::perform() + 137
    frame #8: 0x0000000100054b9a CFNetwork`MultiplexerSource::perform() + 282
    frame #9: 0x00000001000549bc CFNetwork`MultiplexerSource::_perform(void*) + 72
    frame #10: 0x00007fff8f645fe1 CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
    frame #11: 0x00007fff8f62571c CoreFoundation`__CFRunLoopDoSources0 + 556
    frame #12: 0x00007fff8f624c3f CoreFoundation`__CFRunLoopRun + 927
    frame #13: 0x00007fff8f624638 CoreFoundation`CFRunLoopRunSpecific + 296
    frame #14: 0x000000010002f6e9 CFNetwork`+[NSURLConnection(Loader) _resourceLoadLoop:] + 412
    frame #15: 0x00007fff919cfd64 Foundation`__NSThread__start__ + 1351
    frame #16: 0x0000000100915805 libsystem_pthread.dylib`_pthread_body + 131
    frame #17: 0x0000000100915782 libsystem_pthread.dylib`_pthread_start + 168
    frame #18: 0x0000000100912fa1 libsystem_pthread.dylib`thread_start + 13

So we need to be able to dispatch these blocks to com.apple.NSURLConnectionLoader in order to guarantee thread safety.

[Alex Christensen]

Created attachment 268924 [details]
Patch

[Alex Christensen]

(In reply to comment #2)
> Comment on attachment 268900 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=268900&action=review
> 
> > Source/WebKit2/NetworkProcess/CustomProtocols/Cocoa/CustomProtocolManagerCocoa.mm:194
> > +    auto copy = Block_copy(block);
> > +    callOnMainThread([copy] {
> > +        copy();
> > +        Block_release(copy);
> > +    });
> 
> So we need to be able to dispatch these blocks to
> com.apple.NSURLConnectionLoader in order to guarantee thread safety.
You are correct.  The tests pass without this change, and this change is incorrect.

[WebKit Commit Bot]

Comment on attachment 268924 [details]
Patch

Clearing flags on attachment: 268924

Committed r195005: <http://trac.webkit.org/changeset/195005>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.