Bug 183297

Summary: imported/w3c/web-platform-tests/html/semantics/text-level-semantics/the-a-element/a-download-click-404.html times out with async policy delegates
Product: WebKit Reporter: Ali Juma <ajuma>
Component: Page LoadingAssignee: Ali Juma <ajuma>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beckyspencer121, beidson, brandphicofficial, cdumez, dbates, ews-watchlist, japhet, oliviajohnsonswork, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=183337
Bug Depends on:    
Bug Blocks: 180568    
Attachments:
Description Flags
WIP patch
none
Archive of layout-test-results from ews100 for mac-sierra
none
Archive of layout-test-results from ews124 for ios-simulator-wk2
none
Archive of layout-test-results from ews115 for mac-sierra
none
Archive of layout-test-results from ews204 for win-future
none
WIP patch
none
WIP patch
none
Archive of layout-test-results from ews202 for win-future
none
Patch none

Ali Juma
Reported 2018-03-02 11:57:53 PST
This test times out because WebFrameLoaderClient::dispatchDidFinishLoad() never fires for the main frame. The problem is that the last time that checkLoadCompleteForThisFrame is called for the main frame, the subframe is still waiting for a policy decision so still has a policy loader. So isLoadingInAPISense is still true for the main frame. When the subframe policy decision is received, it's "Download", which means that continueLoadAfterNavigationPolicy gets called with shouldContinue==false. So then the policy loader gets cleared, but nothing triggers another checkLoadCompleteForThisFrame on the main frame. A possible fix is to call checkLoadComplete after clearing the policy loader in the !canContinue case in continueLoadAfterNavigationPolicy.
Attachments
WIP patch (5.79 KB, patch)
2018-03-02 12:48 PST, Ali Juma 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews100 for mac-sierra (2.25 MB, application/zip)
2018-03-02 13:50 PST, EWS Watchlist 		no flags
Details
Archive of layout-test-results from ews124 for ios-simulator-wk2 (2.16 MB, application/zip)
2018-03-02 14:17 PST, EWS Watchlist 		no flags
Details
Archive of layout-test-results from ews115 for mac-sierra (2.92 MB, application/zip)
2018-03-02 14:23 PST, EWS Watchlist 		no flags
Details
Archive of layout-test-results from ews204 for win-future (12.06 MB, application/zip)
2018-03-02 14:24 PST, EWS Watchlist 		no flags
Details
WIP patch (8.93 KB, patch)
2018-03-02 15:14 PST, Ali Juma 		no flags
DetailsFormatted DiffDiff
WIP patch (8.93 KB, patch)
2018-03-02 15:38 PST, Ali Juma 		no flags
DetailsFormatted DiffDiff
Archive of layout-test-results from ews202 for win-future (12.09 MB, application/zip)
2018-03-02 21:16 PST, EWS Watchlist 		no flags
Details
Patch (9.88 KB, patch)
2018-03-05 09:05 PST, Ali Juma 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Ali Juma
Comment 1 2018-03-02 12:48:03 PST
Created attachment 334917 [details] WIP patch
EWS Watchlist
Comment 2 2018-03-02 13:50:22 PST Comment hidden (obsolete)
EWS Watchlist
Comment 3 2018-03-02 13:50:23 PST Comment hidden (obsolete)
EWS Watchlist
Comment 4 2018-03-02 14:17:36 PST Comment hidden (obsolete)
EWS Watchlist
Comment 5 2018-03-02 14:17:38 PST Comment hidden (obsolete)
EWS Watchlist
Comment 6 2018-03-02 14:23:15 PST Comment hidden (obsolete)
EWS Watchlist
Comment 7 2018-03-02 14:23:17 PST Comment hidden (obsolete)
EWS Watchlist
Comment 8 2018-03-02 14:24:05 PST Comment hidden (obsolete)
EWS Watchlist
Comment 9 2018-03-02 14:24:16 PST Comment hidden (obsolete)
Ali Juma
Comment 10 2018-03-02 15:14:12 PST
Created attachment 334934 [details] WIP patch
Ali Juma
Comment 11 2018-03-02 15:38:22 PST
Created attachment 334936 [details] WIP patch
Chris Dumez
Comment 12 2018-03-02 17:10:28 PST
Comment on attachment 334936 [details] WIP patch View in context: https://bugs.webkit.org/attachment.cgi?id=334936&action=review > LayoutTests/platform/ios-wk2/http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt:1 > +CONSOLE MESSAGE: line 21: TypeError: null is not an object (evaluating 'errorFrame.contentDocument.querySelector("#error-url").click') This does not look OK. > LayoutTests/platform/mac-wk1/http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt:1 > +CONSOLE MESSAGE: line 21: TypeError: null is not an object (evaluating 'errorFrame.contentDocument.querySelector("#error-url").click') ditto.
EWS Watchlist
Comment 13 2018-03-02 21:16:38 PST
Comment on attachment 334936 [details] WIP patch Attachment 334936 [details] did not pass win-ews (win): Output: http://webkit-queues.webkit.org/results/6747062 New failing tests: http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404.html http/wpt/html/browsers/history/the-location-interface/location-protocol-setter-non-broken-async-delegate.html
EWS Watchlist
Comment 14 2018-03-02 21:16:50 PST
Created attachment 334949 [details] Archive of layout-test-results from ews202 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews202 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Ali Juma
Comment 15 2018-03-05 09:05:42 PST
Created attachment 335003 [details] Patch
Ali Juma
Comment 16 2018-03-05 09:06:03 PST
(In reply to Chris Dumez from comment #12) > Comment on attachment 334936 [details] > WIP patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=334936&action=review > > > LayoutTests/platform/ios-wk2/http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt:1 > > +CONSOLE MESSAGE: line 21: TypeError: null is not an object (evaluating 'errorFrame.contentDocument.querySelector("#error-url").click') > > This does not look OK. > > > LayoutTests/platform/mac-wk1/http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt:1 > > +CONSOLE MESSAGE: line 21: TypeError: null is not an object (evaluating 'errorFrame.contentDocument.querySelector("#error-url").click') > > ditto. The existing test has these errors in its expected output for mac-wk1 and ios-wk2 as well. These errors are because the 'download' attribute isn't supported on WK1 or on iOS (see bug 156069 for WK1 and bug 167341 for iOS), so the click ends up navigating. I verified that I get the same console error on WK2 without my change if I remove the 'download' attribute from the test iframe (and also get the same error in Chrome and Firefox if I remove the 'download' attribute). Would it be better to skip these tests instead?
Chris Dumez
Comment 17 2018-03-05 09:14:31 PST
(In reply to Ali Juma from comment #16) > (In reply to Chris Dumez from comment #12) > > Comment on attachment 334936 [details] > > WIP patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=334936&action=review > > > > > LayoutTests/platform/ios-wk2/http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt:1 > > > +CONSOLE MESSAGE: line 21: TypeError: null is not an object (evaluating 'errorFrame.contentDocument.querySelector("#error-url").click') > > > > This does not look OK. > > > > > LayoutTests/platform/mac-wk1/http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404-expected.txt:1 > > > +CONSOLE MESSAGE: line 21: TypeError: null is not an object (evaluating 'errorFrame.contentDocument.querySelector("#error-url").click') > > > > ditto. > > The existing test has these errors in its expected output for mac-wk1 and > ios-wk2 as well. These errors are because the 'download' attribute isn't > supported on WK1 or on iOS (see bug 156069 for WK1 and bug 167341 for iOS), > so the click ends up navigating. I verified that I get the same console > error on WK2 without my change if I remove the 'download' attribute from the > test iframe (and also get the same error in Chrome and Firefox if I remove > the 'download' attribute). > > Would it be better to skip these tests instead? Oh, I see that now. Please disregard my comment then.
Chris Dumez
Comment 18 2018-03-05 10:15:53 PST
Comment on attachment 335003 [details] Patch Clearing flags on attachment: 335003 Committed r229286: <https://trac.webkit.org/changeset/229286>
Chris Dumez
Comment 19 2018-03-05 10:15:55 PST
All reviewed patches have been landed. Closing bug.
Chris Dumez
Comment 20 2018-03-05 10:16:30 PST
Comment on attachment 335003 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=335003&action=review > Source/WebCore/loader/FrameLoader.cpp:3162 > + checkLoadComplete(); I think we really want checkCompleted() here instead. I'll follow up in https://bugs.webkit.org/show_bug.cgi?id=183337.
Radar WebKit Bug Importer
Comment 21 2018-03-05 10:21:05 PST
<rdar://problem/38144334>
Becky Spencer
Comment 22 2018-07-09 04:48:04 PDT
Good news It applies to everyone to use with me. Please do what you are doing. More and more communication is a new way to achieve development and development. Facilitate community communication. Hundreds of machines and images do not easily understand. We have submitted papers, letters and emails for all important information about cell phones. The university university provides communication facilities and all information. try [Bulk SMS](https://www.experttexting.com/) services for your future!
Brandphic
Comment 23 2019-10-02 06:11:37 PDT
Great offer. Get your custom logo design & website design by worlds best designers, we don't make logo, we design to make your business a brand. 100% refund guarantee if not satisfied. https://www.brandphic.com/
Olivia Johnson
Comment 24 2021-04-02 02:28:05 PDT
Outstanding words, you have rendered the depth of mental health. We should consider the significance of it as we do with regards to other organs of our body, just head for outpatient counseling nearby if symptoms are evident. <a href="https://hamiltonbehavioralrtms.com/">outpatient counseling near me</a>
Note You need to log in before you can comment on or make changes to this bug.