ResourceHandle always asks the client whether it uses async callbacks, and it has special cases for when there is no client. But we start with a non-null client and zero it out, after which it's not possible to guess what the behavior was. The client behavior determines how we set up an NSURLConnection. In particular, this means that we could respond to NSURLConnection delegates incorrectly at the time when a load is being shut down. At the worst (theoretical) case, we'd even leak delegate threads.
Created attachment 257648 [details] proposed patch
Attachment 257648 [details] did not pass style-queue: ERROR: Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:422: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] ERROR: Source/WebCore/platform/network/mac/ResourceHandleMac.mm:633: An else statement can be removed when the prior "if" concludes with a return, break, continue or goto statement. [readability/control_flow] [4] Total errors found: 2 in 12 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 257648 [details] proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=257648&action=review > Source/WebCore/platform/network/ResourceHandle.h:255 > +#if PLATFORM(COCOA) > + bool usesAsyncCallbacks() const; > +#endif Since the BlobResourceHandle code is not inside PLATFORM(COCOA), this needs to be available cross-platform. I think that means that we need to put considerably more of this code outside conditionals. > Source/WebCore/platform/network/ResourceHandleInternal.h:101 > + , m_usesAsyncCallbacks(client && client->usesAsyncCallbacks()) It’s a little peculiar to use a client callback to supply a flag that is read one time when initializing. We might want to change the way this works to use an argument passed when constructing a resource handle instead of a callback to the client, if we can do that in a clean enough style. > Source/WebCore/platform/network/cf/ResourceHandleCFNet.cpp:429 > + return false; // Ignored by caller. > + } else > + return client && client->canAuthenticateAgainstProtectionSpace(this, protectionSpace); Normally we don’t use an else after return. > Source/WebCore/platform/network/mac/ResourceHandleMac.mm:640 > } else > - return client() && client()->canAuthenticateAgainstProtectionSpace(this, protectionSpace); > + return client && client->canAuthenticateAgainstProtectionSpace(this, protectionSpace); Normally we don’t do an else after return.
Created attachment 257666 [details] patch for landing
Comment on attachment 257666 [details] patch for landing Clearing flags on attachment: 257666 Committed r187533: <http://trac.webkit.org/changeset/187533>
All reviewed patches have been landed. Closing bug.