Bug 183297 – imported/w3c/web-platform-tests/html/semantics/text-level-semantics/the-a-element/a-download-click-404.html times out with async policy delegates

• Status: RESOLVED
• Resolution: FIXED
• Component: Page Loading

[Ali Juma]

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.

[Ali Juma]

Created attachment 334917 [details]
WIP patch

[EWS Watchlist]

Comment on attachment 334917 [details]
WIP patch

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

New failing tests:
http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404.html

[EWS Watchlist]

Created attachment 334923 [details]
Archive of layout-test-results from ews100 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews100  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 334917 [details]
WIP patch

Attachment 334917 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/6739839

New failing tests:
http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404.html

[EWS Watchlist]

Created attachment 334927 [details]
Archive of layout-test-results from ews124 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews124  Port: ios-simulator-wk2  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 334917 [details]
WIP patch

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

New failing tests:
http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404.html

[EWS Watchlist]

Created attachment 334930 [details]
Archive of layout-test-results from ews115 for mac-sierra

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115  Port: mac-sierra  Platform: Mac OS X 10.12.6

[EWS Watchlist]

Comment on attachment 334917 [details]
WIP patch

Attachment 334917 [details] did not pass win-ews (win):
Output: http://webkit-queues.webkit.org/results/6740107

New failing tests:
http/wpt/html/semantics/text-level-semantics/the-a-element/a-download-click-404.html

[EWS Watchlist]

Created attachment 334931 [details]
Archive of layout-test-results from ews204 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews204  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Ali Juma]

Created attachment 334934 [details]
WIP patch

[Ali Juma]

Created attachment 334936 [details]
WIP patch

[Chris Dumez]

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 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]

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]

Created attachment 335003 [details]
Patch

[Ali Juma]

(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]

(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 on attachment 335003 [details]
Patch

Clearing flags on attachment: 335003

Committed r229286: <https://trac.webkit.org/changeset/229286>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.

[Chris Dumez]

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]

<rdar://problem/38144334>

[Becky Spencer]

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]

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]

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>