Bug 183254 - Converting a load to a download does not work with async policy delegates
Summary: Converting a load to a download does not work with async policy delegates
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Page Loading (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks: 180568
  Show dependency treegraph
 
Reported: 2018-03-01 11:52 PST by Chris Dumez
Modified: 2018-03-02 09:41 PST (History)
11 users (show)

See Also:


Attachments
WIP Patch (11.74 KB, patch)
2018-03-01 12:31 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Patch (16.05 KB, patch)
2018-03-01 12:50 PST, Chris Dumez
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-sierra (2.19 MB, application/zip)
2018-03-01 13:51 PST, EWS Watchlist
no flags Details
Patch (19.39 KB, patch)
2018-03-01 13:55 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Dumez 2018-03-01 11:52:17 PST
Converting a load to a download does not work with async policy delegates.

This causes fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download.html and a few other tests to fail when enabling asyncpolicy delegates.
Comment 1 Radar WebKit Bug Importer 2018-03-01 11:53:08 PST
<rdar://problem/38035334>
Comment 2 Chris Dumez 2018-03-01 12:31:23 PST
Created attachment 334839 [details]
WIP Patch
Comment 3 Chris Dumez 2018-03-01 12:50:43 PST
Created attachment 334842 [details]
Patch
Comment 4 EWS Watchlist 2018-03-01 13:51:30 PST
Comment on attachment 334842 [details]
Patch

Attachment 334842 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/6724935

New failing tests:
fast/dom/HTMLAnchorElement/anchor-file-blob-convert-to-download-async-delegate.html
Comment 5 EWS Watchlist 2018-03-01 13:51:32 PST
Created attachment 334849 [details]
Archive of layout-test-results from ews101 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-sierra  Platform: Mac OS X 10.12.6
Comment 6 Chris Dumez 2018-03-01 13:55:33 PST
Created attachment 334850 [details]
Patch
Comment 7 youenn fablet 2018-03-02 09:11:06 PST
Comment on attachment 334850 [details]
Patch

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

> Source/WebCore/loader/DocumentLoader.cpp:789
> +    frameLoader()->checkContentPolicy(m_response, [this, protectedThis = makeRef(*this), mainResourceLoader = WTFMove(mainResourceLoader)](PolicyAction policy) {

I guess capturing mainResourceLoader in the lambda is to be extra safe that we call didReceiveResponsePolicy on the exact same loader from which we called markInAsyncResponsePolicyCheck.
Or is there some more subtlety there?
Comment 8 Chris Dumez 2018-03-02 09:12:08 PST
Comment on attachment 334850 [details]
Patch

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

>> Source/WebCore/loader/DocumentLoader.cpp:789
>> +    frameLoader()->checkContentPolicy(m_response, [this, protectedThis = makeRef(*this), mainResourceLoader = WTFMove(mainResourceLoader)](PolicyAction policy) {
> 
> I guess capturing mainResourceLoader in the lambda is to be extra safe that we call didReceiveResponsePolicy on the exact same loader from which we called markInAsyncResponsePolicyCheck.
> Or is there some more subtlety there?

No subtlety, I thought it looked better :)
Comment 9 WebKit Commit Bot 2018-03-02 09:41:12 PST
Comment on attachment 334850 [details]
Patch

Clearing flags on attachment: 334850

Committed r229177: <https://trac.webkit.org/changeset/229177>
Comment 10 WebKit Commit Bot 2018-03-02 09:41:14 PST
All reviewed patches have been landed.  Closing bug.