Bug 147985

Summary: [Cocoa] Downloads do not start if policy decision is made asynchronously
Product: WebKit Reporter: Andy Estes <aestes>
Component: Page LoadingAssignee: Andy Estes <aestes>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, beidson, commit-queue, ddkilzer, esprehn+autocc, japhet, kangil.han, sam, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: All   
Attachments:
Description Flags
Patch
none
Patch beidson: review+

Description Andy Estes 2015-08-13 12:47:57 PDT
[Cocoa] Downloads do not start if policy decision is made asynchronously
Comment 1 Andy Estes 2015-08-13 12:49:43 PDT
rdar://problem/22222453
Comment 2 Andy Estes 2015-08-13 13:04:00 PDT
Created attachment 258919 [details]
Patch
Comment 3 Alexey Proskuryakov 2015-08-13 15:38:21 PDT
Comment on attachment 258919 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        asynchronously. If a client chooses to download a response asynchronously, we can no longer convert the
> +        connection to a download, so we should start a new download instead.

This is surely not a permanent solution to this, is it? In the general case, it is not possible to start a new download.

We need to delay responding to willSendRequest until the decision is made.
Comment 4 Andy Estes 2015-08-13 15:42:14 PDT
(In reply to comment #3)
> Comment on attachment 258919 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=258919&action=review
> 
> > Source/WebCore/ChangeLog:11
> > +        asynchronously. If a client chooses to download a response asynchronously, we can no longer convert the
> > +        connection to a download, so we should start a new download instead.
> 
> This is surely not a permanent solution to this, is it? In the general case,
> it is not possible to start a new download.
> 
> We need to delay responding to willSendRequest until the decision is made.

didReceiveResponse in this case, but you're right, that's the correct solution. This is a stopgap measure until that solution can be implemented. I included more information about this in the linked radar.
Comment 5 Andy Estes 2015-08-13 15:44:36 PDT
Created attachment 258942 [details]
Patch
Comment 6 Brady Eidson 2015-08-14 12:33:11 PDT
Comment on attachment 258942 [details]
Patch

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

> Source/WebCore/loader/SubresourceLoader.cpp:80
> +    , m_callingDidReceiveResponse(false)

Default value can be in the class declaration in the header.

> Source/WebCore/loader/SubresourceLoader.h:129
> +    bool m_callingDidReceiveResponse;

...right here.

> Source/WebKit/mac/WebCoreSupport/WebFrameLoaderClient.mm:292
>          // The resource has already been cached, start a new download.

Inaccurate comment
Comment 7 Andy Estes 2015-08-14 14:08:37 PDT
Committed r188486: <http://trac.webkit.org/changeset/188486>