Bug 198469 – Memory-cached main resources continue to load after the client decides a content policy of PolicyAction::Download

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Andy Estes]

Memory-cached main resources continue to load after the client decides a content policy of PollicyAction::Download

[Andy Estes]

rdar://problem/50512713

[Andy Estes]

Created attachment 371167 [details]
Patch

[EWS Watchlist]

Comment on attachment 371167 [details]
Patch

Attachment 371167 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12358696

New failing tests:
inspector/canvas/recording.html

[EWS Watchlist]

Created attachment 371171 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[EWS Watchlist]

Comment on attachment 371167 [details]
Patch

Attachment 371167 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/12359257

New failing tests:
svg/text/textpath-reference-update.html

[EWS Watchlist]

Created attachment 371174 [details]
Archive of layout-test-results from ews211 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews211  Port: win-future  Platform: CYGWIN_NT-10.0-17763-3.0.5-338.x86_64-x86_64-64bit

[youenn fablet]

Comment on attachment 371167 [details]
Patch

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

> Source/WebCore/loader/DocumentLoader.cpp:953
> +        ASSERT(m_mainResource);

Is this assert needed given we check for m_mainResource at the beginning of the Download case block?

[youenn fablet]

As a side note, there is the case of substitute resource loads where we call mainReceivedError() in case of PolicyAction::Download. I wonder whether we should have consistent calls there.

[Andy Estes]

(In reply to youenn fablet from comment #8)
> As a side note, there is the case of substitute resource loads where we call
> mainReceivedError() in case of PolicyAction::Download. I wonder whether we
> should have consistent calls there.

stopLoadingForPolicyChange() calls mainReceivedError(), so I think we're consistent in that sense. It uses a different error code than in the call you cited ("interrupted for policy change" vs. "cannot show URL"), but I wanted to match the error code used in the call to ResourceLoader::didFail.

[Andy Estes]

(In reply to youenn fablet from comment #7)
> Comment on attachment 371167 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=371167&action=review
> 
> > Source/WebCore/loader/DocumentLoader.cpp:953
> > +        ASSERT(m_mainResource);
> 
> Is this assert needed given we check for m_mainResource at the beginning of
> the Download case block?

I was curious to see if it could ever go missing after that check at the beginning, but I don't think it's needed. Will remove.

[Andy Estes]

Created attachment 371197 [details]
Patch

[EWS Watchlist]

Comment on attachment 371197 [details]
Patch

Attachment 371197 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/12364606

New failing tests:
http/wpt/service-workers/service-worker-networkprocess-crash.html

[EWS Watchlist]

Created attachment 371202 [details]
Archive of layout-test-results from ews107 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews107  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Andy Estes]

Committed r246043: <https://trac.webkit.org/changeset/246043>