Bug 183702 – WebKit.WebsitePoliciesAutoplayQuirks API test times out with async policy delegates

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

[Chris Dumez]

WebKit.WebsitePoliciesAutoplayQuirks API test times out with async policy delegates.

[Chris Dumez]

Created attachment 335950 [details]
API test without fix

[Chris Dumez]

Comment on attachment 335950 [details]
API test without fix

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

> Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:723
> +    [webView loadHTMLString:@"" baseURL:nil];

It is somehow related to these loadHTMLString calls. If I comment them out, then the test passes.
Also note that the test does not wait for the load to be complete before continuing.

> Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:741
> +    [webView loadHTMLString:@"" baseURL:nil];

Same here.

[Chris Dumez]

While we're doing the navigation policy check for loading autoplay-inherits-gesture-from-document.html, another load gets committed and causes FrameLoader::continueAfterContentPolicy() to be called with shouldContinue=false.

The load that gets committed is triggered from:
1   0x1aa50d006 WebCore::FrameLoader::continueLoadAfterNavigationPolicy(WebCore::ResourceRequest const&, WebCore::FormState*, bool, WebCore::AllowNavigationToInvalidURL)
2   0x1aa52b12c WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WebCore::FormState*, WebCore::AllowNavigationToInvalidURL, WTF::CompletionHandler<void ()>&&)::$_9::operator()(WebCore::ResourceRequest const&, WebCore::FormState*, bool) const
3   0x1aa52b002 WTF::Function<void (WebCore::ResourceRequest&&, WebCore::FormState*, bool)>::CallableWrapper<WebCore::FrameLoader::loadWithDocumentLoader(WebCore::DocumentLoader*, WebCore::FrameLoadType, WebCore::FormState*, WebCore::AllowNavigationToInvalidURL, WTF::CompletionHandler<void ()>&&)::$_9>::call(WebCore::ResourceRequest&&, WebCore::FormState*, bool)
4   0x1aa55dbdd WTF::Function<void (WebCore::ResourceRequest&&, WebCore::FormState*, bool)>::operator()(WebCore::ResourceRequest&&, WebCore::FormState*, bool) const
5   0x1aa550a09 WTF::CompletionHandler<void (WebCore::ResourceRequest&&, WebCore::FormState*, bool)>::operator()(WebCore::ResourceRequest&&, WebCore::FormState*, bool) const
6   0x1aa55fead WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest&&, bool, WebCore::DocumentLoader*, WebCore::FormState*, WTF::CompletionHandler<void (WebCore::ResourceRequest&&, WebCore::FormState*, bool)>&&)::$_6::operator()(WebCore::PolicyAction)
7   0x1aa55fc7a WTF::Function<void (WebCore::PolicyAction)>::CallableWrapper<WebCore::PolicyChecker::checkNavigationPolicy(WebCore::ResourceRequest&&, bool, WebCore::DocumentLoader*, WebCore::FormState*, WTF::CompletionHandler<void (WebCore::ResourceRequest&&, WebCore::FormState*, bool)>&&)::$_6>::call(WebCore::PolicyAction)
8   0x10df6e841 WTF::Function<void (WebCore::PolicyAction)>::operator()(WebCore::PolicyAction) const
9   0x10e709885 WebKit::WebFrame::invalidatePolicyListener()
10  0x10e716179 WebKit::WebFrameLoaderClient::cancelPolicyCheck()
11  0x1aa5511d4 WebCore::PolicyChecker::stopCheck()
12  0x1aa5056d7 WebCore::FrameLoader::stopLoading(WebCore::UnloadEventPolicy)
13  0x1aa0def6c WebCore::firePageHideEventRecursively(WebCore::Frame&)
14  0x1aa0de9eb WebCore::PageCache::addIfCacheable(WebCore::HistoryItem&, WebCore::Page*)
15  0x1aa50f0a3 WebCore::FrameLoader::commitProvisionalLoad()
16  0x1aa4b90dc WebCore::DocumentLoader::commitIfReady()
17  0x1aa4b9402 WebCore::DocumentLoader::finishedLoading()
18  0x1aa4be5e5 WebCore::DocumentLoader::continueAfterContentPolicy(WebCore::PolicyAction)
19  0x1aa4bad57 WebCore::DocumentLoader::responseReceived(WebCore::ResourceResponse const&, WTF::CompletionHandler<void ()>&&)
20  0x1aa4b6b9d WebCore::DocumentLoader::handleSubstituteDataLoadNow()
21  0x1aa4dc3e6 WTF::RunLoopTimer<WebCore::DocumentLoader>::fired()

I believe this is the substituteData from the previous loadHTMLString. It should have probably been cancelled when we started loading loading autoplay-inherits-gesture-from-document.html.

[Chris Dumez]

Created attachment 335959 [details]
WIP patch

[EWS Watchlist]

Comment on attachment 335959 [details]
WIP patch

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

New failing tests:
imported/w3c/web-platform-tests/html/semantics/text-level-semantics/the-a-element/a-download-click.html
http/tests/contentfiltering/block-after-redirect.html
contentfiltering/block-after-response-then-deny-unblock.html
imported/w3c/web-platform-tests/html/semantics/embedded-content/the-area-element/area-download-click.html
contentfiltering/block-after-finished-adding-data-then-deny-unblock.html
contentfiltering/block-after-will-send-request-then-allow-unblock.html
contentfiltering/block-after-add-data.html
contentfiltering/block-after-add-data-then-deny-unblock.html
contentfiltering/block-after-add-data-then-allow-unblock.html
contentfiltering/block-after-response.html
contentfiltering/block-after-response-then-allow-unblock.html
contentfiltering/block-after-will-send-request.html
contentfiltering/block-after-finished-adding-data.html
contentfiltering/block-after-will-send-request-then-deny-unblock.html
contentfiltering/block-after-finished-adding-data-then-allow-unblock.html

[EWS Watchlist]

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

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

[EWS Watchlist]

Comment on attachment 335959 [details]
WIP patch

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

New failing tests:
http/tests/security/feed-urls-from-remote.html
http/tests/contentfiltering/block-after-redirect.html
contentfiltering/block-after-response-then-deny-unblock.html
contentfiltering/block-after-finished-adding-data-then-deny-unblock.html
contentfiltering/block-after-will-send-request-then-allow-unblock.html
contentfiltering/block-after-add-data.html
contentfiltering/block-after-add-data-then-deny-unblock.html
contentfiltering/block-after-add-data-then-allow-unblock.html
contentfiltering/block-after-response.html
contentfiltering/block-after-response-then-allow-unblock.html
contentfiltering/block-after-will-send-request.html
contentfiltering/block-after-finished-adding-data.html
contentfiltering/block-after-will-send-request-then-deny-unblock.html
contentfiltering/block-after-finished-adding-data-then-allow-unblock.html

[EWS Watchlist]

Created attachment 335963 [details]
Archive of layout-test-results from ews103 for mac-sierra

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

[EWS Watchlist]

Comment on attachment 335959 [details]
WIP patch

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

New failing tests:
http/tests/contentfiltering/block-after-redirect.html
contentfiltering/block-after-response-then-deny-unblock.html
contentfiltering/block-after-finished-adding-data-then-deny-unblock.html
contentfiltering/block-after-will-send-request-then-allow-unblock.html
contentfiltering/block-after-add-data.html
contentfiltering/block-after-add-data-then-deny-unblock.html
contentfiltering/block-after-add-data-then-allow-unblock.html
contentfiltering/block-after-response.html
contentfiltering/block-after-response-then-allow-unblock.html
contentfiltering/block-after-will-send-request.html
contentfiltering/block-after-finished-adding-data.html
contentfiltering/block-after-will-send-request-then-deny-unblock.html
contentfiltering/block-after-finished-adding-data-then-allow-unblock.html

[EWS Watchlist]

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

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

[EWS Watchlist]

Comment on attachment 335959 [details]
WIP patch

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

New failing tests:
http/tests/security/feed-urls-from-remote.html

[EWS Watchlist]

Created attachment 335966 [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

[EWS Watchlist]

Comment on attachment 335959 [details]
WIP patch

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

New failing tests:
http/tests/security/feed-urls-from-remote.html
http/tests/contentfiltering/block-after-redirect.html
contentfiltering/block-after-response-then-deny-unblock.html
contentfiltering/block-after-finished-adding-data-then-deny-unblock.html
contentfiltering/block-after-will-send-request-then-allow-unblock.html
contentfiltering/block-after-add-data.html
contentfiltering/block-after-add-data-then-deny-unblock.html
contentfiltering/block-after-add-data-then-allow-unblock.html
contentfiltering/block-after-response.html
contentfiltering/block-after-response-then-allow-unblock.html
contentfiltering/block-after-will-send-request.html
contentfiltering/block-after-finished-adding-data.html
contentfiltering/block-after-will-send-request-then-deny-unblock.html
contentfiltering/block-after-finished-adding-data-then-allow-unblock.html

[EWS Watchlist]

Created attachment 335967 [details]
Archive of layout-test-results from ews114 for mac-sierra

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

[Chris Dumez]

Created attachment 335977 [details]
WIP patch

[Chris Dumez]

Created attachment 335978 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 335978 [details]
Patch

Clearing flags on attachment: 335978

Committed r229689: <https://trac.webkit.org/changeset/229689>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/38566060>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 183735

[Chris Dumez]

Created attachment 336051 [details]
Patch

[Chris Dumez]

Comment on attachment 335978 [details]
Patch

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

> Source/WebCore/loader/FrameLoader.cpp:1546
> +    if (policyChecker().delegateIsDecidingNavigationPolicy()) {

This did not work well because this is too late. Calling DocumentLoader::stopLoading() may call FrameLoader::stopLoading() which would cancel the policy check we've just started. The latest patch iteration moves the stopLoading call *before* actually starting the policy check so that we do not cancel it.

[EWS Watchlist]

Comment on attachment 336051 [details]
Patch

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

New failing tests:
http/tests/security/feed-urls-from-remote.html

[EWS Watchlist]

Created attachment 336058 [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 336051 [details]
Patch

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

New failing tests:
imported/w3c/web-platform-tests/html/semantics/text-level-semantics/the-a-element/a-download-click.html
imported/w3c/web-platform-tests/html/semantics/embedded-content/the-area-element/area-download-click.html

[EWS Watchlist]

Created attachment 336059 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2

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

[EWS Watchlist]

Comment on attachment 336051 [details]
Patch

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

New failing tests:
http/tests/security/feed-urls-from-remote.html

[EWS Watchlist]

Created attachment 336063 [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 336051 [details]
Patch

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

New failing tests:
http/tests/security/feed-urls-from-remote.html

[EWS Watchlist]

Created attachment 336067 [details]
Archive of layout-test-results from ews206 for win-future

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

[Chris Dumez]

Created attachment 336068 [details]
Patch

[Chris Dumez]

Comment on attachment 336068 [details]
Patch

Clearing flags on attachment: 336068

Committed r229722: <https://trac.webkit.org/changeset/229722>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.