Bug 198469

Summary:

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

Product:

WebKit

Reporter:

Andy Estes <aestes>

Component:

New Bugs

Assignee:

Andy Estes <aestes>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, beidson, cdumez, dbates, ews-watchlist, ggaren, japhet, koivisto, rniwa, webkit-bug-importer, youennf

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Archive of layout-test-results from ews107 for mac-highsierra-wk2	none
Archive of layout-test-results from ews211 for win-future	none
Patch	ews-watchlist: commit-queue-
Archive of layout-test-results from ews107 for mac-highsierra-wk2	none

Andy Estes

Reported 2019-06-02 15:47:38 PDT

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

Attachments
Patch (12.23 KB, patch) 2019-06-02 15:59 PDT, Andy Estes	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews107 for mac-highsierra-wk2 (2.86 MB, application/zip) 2019-06-02 17:15 PDT, EWS Watchlist	no flags	Details
Archive of layout-test-results from ews211 for win-future (13.96 MB, application/zip) 2019-06-02 19:49 PDT, EWS Watchlist	no flags	Details
Patch (13.04 KB, patch) 2019-06-03 11:09 PDT, Andy Estes	ews-watchlist: commit-queue-	Details Formatted Diff Diff
Archive of layout-test-results from ews107 for mac-highsierra-wk2 (3.05 MB, application/zip) 2019-06-03 11:58 PDT, EWS Watchlist	no flags	Details
Show Obsolete (5) View All Add attachment proposed patch, testcase, etc.

Andy Estes

Comment 1 2019-06-02 15:47:59 PDT

rdar://problem/50512713

Andy Estes

Comment 2 2019-06-02 15:59:29 PDT

Created attachment 371167 [details] Patch

EWS Watchlist

Comment 3 2019-06-02 17:15:48 PDT Comment hidden (obsolete)

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

Comment 4 2019-06-02 17:15:49 PDT Comment hidden (obsolete)

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 5 2019-06-02 19:49:52 PDT Comment hidden (obsolete)

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

Comment 6 2019-06-02 19:49:54 PDT Comment hidden (obsolete)

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 7 2019-06-03 08:57:42 PDT

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

Comment 8 2019-06-03 09:01:20 PDT

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

Comment 9 2019-06-03 09:38:19 PDT

(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

Comment 10 2019-06-03 10:52:38 PDT

(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

Comment 11 2019-06-03 11:09:17 PDT Comment hidden (obsolete)

Created attachment 371197 [details] Patch

EWS Watchlist

Comment 12 2019-06-03 11:58:57 PDT Comment hidden (obsolete)

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

Comment 13 2019-06-03 11:58:59 PDT Comment hidden (obsolete)

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

Comment 14 2019-06-03 12:39:08 PDT

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

Note You need to log in before you can comment on or make changes to this bug.