Skip to content
Permalink
Browse files
WebKit.WebsitePoliciesAutoplayQuirks API test times out with async po…
…licy delegates

https://bugs.webkit.org/show_bug.cgi?id=183702
<rdar://problem/38566060>

Reviewed by Alex Christensen.

Source/WebCore:

The issue is that the test calls loadHTMLString then loadRequest right after, without
waiting for the first load to complete first. loadHTMLString is special as it relies
on substitute data and which schedules a timer to commit the data. When doing the
navigation policy check for the following loadRequest(), the substitute data timer
would fire and commit its data and load. This would in turn cancel the pending
navigation policy check for the loadRequest().

With sync policy delegates, this is not an issue because we take care of stopping
all loaders when receiving the policy decision, which happens synchronously. However,
when the policy decision happens asynchronously, the pending substitute data load
does not get cancelled in time and it gets committed.

To address the issue, we now cancel any pending provisional load before doing the
navigation policy check.

Test: fast/loader/inner-iframe-loads-data-url-into-parent-on-unload-crash-async-delegate.html

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::clearProvisionalLoadForPolicyCheck):
* loader/FrameLoader.h:
* loader/PolicyChecker.cpp:
(WebCore::PolicyChecker::checkNavigationPolicy):
Cancel any pending provisional load before starting the navigation policy check. This call
needs to be here rather than in the call site of policyChecker().checkNavigationPolicy()
because there is code in PolicyChecker::checkNavigationPolicy() which relies on
FrameLoader::activeDocumentLoader().
Also, we only cancel the provisional load if there is a policy document loader. In some
rare cases (when we receive a redirect after navigation policy has been decided for the
initial request), the provisional document loader needs to receive navigation policy
decisions so we cannot clear the provisional document loader in such case.

Tools:

Add API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:
(-[AsyncAutoplayPoliciesDelegate webView:decidePolicyForNavigationAction:decisionHandler:]):
(-[AsyncAutoplayPoliciesDelegate _webView:decidePolicyForNavigationAction:decisionHandler:]):
(-[AsyncAutoplayPoliciesDelegate _webView:handleAutoplayEvent:withFlags:]):
(TEST):

LayoutTests:

Add variant of fast/loader/inner-iframe-loads-data-url-into-parent-on-unload-crash.html with async navigation
delegate since the previous iteration of this patch broke this test case.

* fast/loader/inner-iframe-loads-data-url-into-parent-on-unload-crash-async-delegate-expected.txt: Added.
* fast/loader/inner-iframe-loads-data-url-into-parent-on-unload-crash-async-delegate.html: Added.

Canonical link: https://commits.webkit.org/199383@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229722 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
cdumez committed Mar 19, 2018
1 parent 7fc48da commit 9e7fee6401297c5256a640760f089b1012c5a168
@@ -1,3 +1,17 @@
2018-03-19 Chris Dumez <cdumez@apple.com>

WebKit.WebsitePoliciesAutoplayQuirks API test times out with async policy delegates
https://bugs.webkit.org/show_bug.cgi?id=183702
<rdar://problem/38566060>

Reviewed by Alex Christensen.

Add variant of fast/loader/inner-iframe-loads-data-url-into-parent-on-unload-crash.html with async navigation
delegate since the previous iteration of this patch broke this test case.

* fast/loader/inner-iframe-loads-data-url-into-parent-on-unload-crash-async-delegate-expected.txt: Added.
* fast/loader/inner-iframe-loads-data-url-into-parent-on-unload-crash-async-delegate.html: Added.

2018-03-17 Jiewen Tan <jiewen_tan@apple.com>

[WebAuthN] Implement authenticatorMakeCredential
@@ -0,0 +1,6 @@


--------
Frame: '<!--framePath //<!--frame0-->-->'
--------
PASS did not crash.
@@ -0,0 +1,23 @@
<!DOCTYPE html>
<html>
<body>
<script>
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.dumpChildFramesAsText();
testRunner.waitUntilDone();
if (testRunner.setShouldDecideNavigationPolicyAfterDelay)
testRunner.setShouldDecideNavigationPolicyAfterDelay(true);
}
var parentFrame = document.body.appendChild(document.createElement("iframe"));
parentFrame.src = "data:text/html,";

var childFrame = parentFrame.contentDocument.body.appendChild(document.createElement("iframe"));
childFrame.contentWindow.onunload = function () {
var link = parentFrame.contentDocument.createElement("a");
link.href = "data:text/html,PASS did not crash.<script>window.testRunner && window.testRunner.notifyDone()</" + "script>";
link.click(); // Navigates parentFrame
}
</script>
</body>
</html>
@@ -1,3 +1,42 @@
2018-03-19 Chris Dumez <cdumez@apple.com>

WebKit.WebsitePoliciesAutoplayQuirks API test times out with async policy delegates
https://bugs.webkit.org/show_bug.cgi?id=183702
<rdar://problem/38566060>

Reviewed by Alex Christensen.

The issue is that the test calls loadHTMLString then loadRequest right after, without
waiting for the first load to complete first. loadHTMLString is special as it relies
on substitute data and which schedules a timer to commit the data. When doing the
navigation policy check for the following loadRequest(), the substitute data timer
would fire and commit its data and load. This would in turn cancel the pending
navigation policy check for the loadRequest().

With sync policy delegates, this is not an issue because we take care of stopping
all loaders when receiving the policy decision, which happens synchronously. However,
when the policy decision happens asynchronously, the pending substitute data load
does not get cancelled in time and it gets committed.

To address the issue, we now cancel any pending provisional load before doing the
navigation policy check.

Test: fast/loader/inner-iframe-loads-data-url-into-parent-on-unload-crash-async-delegate.html

* loader/FrameLoader.cpp:
(WebCore::FrameLoader::clearProvisionalLoadForPolicyCheck):
* loader/FrameLoader.h:
* loader/PolicyChecker.cpp:
(WebCore::PolicyChecker::checkNavigationPolicy):
Cancel any pending provisional load before starting the navigation policy check. This call
needs to be here rather than in the call site of policyChecker().checkNavigationPolicy()
because there is code in PolicyChecker::checkNavigationPolicy() which relies on
FrameLoader::activeDocumentLoader().
Also, we only cancel the provisional load if there is a policy document loader. In some
rare cases (when we receive a redirect after navigation policy has been decided for the
initial request), the provisional document loader needs to receive navigation policy
decisions so we cannot clear the provisional document loader in such case.

2018-03-19 Eric Carlson <eric.carlson@apple.com>

[Extra zoom mode] Require fullscreen for video playback
@@ -1544,6 +1544,15 @@ void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType t
});
}

void FrameLoader::clearProvisionalLoadForPolicyCheck()
{
if (!m_policyDocumentLoader || !m_provisionalDocumentLoader)
return;

m_provisionalDocumentLoader->stopLoading();
setProvisionalDocumentLoader(nullptr);
}

void FrameLoader::reportLocalLoadFailed(Frame* frame, const String& url)
{
ASSERT(!url.isEmpty());
@@ -141,6 +141,7 @@ class FrameLoader {
void stopLoading(UnloadEventPolicy);
bool closeURL();
void cancelAndClear();
void clearProvisionalLoadForPolicyCheck();
// FIXME: clear() is trying to do too many things. We should break it down into smaller functions (ideally with fewer raw Boolean parameters).
void clear(Document* newDocument, bool clearWindowProperties = true, bool clearScriptObjects = true, bool clearFrameView = true);

@@ -144,6 +144,8 @@ void PolicyChecker::checkNavigationPolicy(ResourceRequest&& request, bool didRec
m_contentFilterUnblockHandler = { };
#endif

m_frame.loader().clearProvisionalLoadForPolicyCheck();

m_delegateIsDecidingNavigationPolicy = true;
String suggestedFilename = action.downloadAttribute().isEmpty() ? nullAtom() : action.downloadAttribute();
ResourceRequest requestCopy = request;
@@ -1,3 +1,19 @@
2018-03-19 Chris Dumez <cdumez@apple.com>

WebKit.WebsitePoliciesAutoplayQuirks API test times out with async policy delegates
https://bugs.webkit.org/show_bug.cgi?id=183702
<rdar://problem/38566060>

Reviewed by Alex Christensen.

Add API test coverage.

* TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:
(-[AsyncAutoplayPoliciesDelegate webView:decidePolicyForNavigationAction:decisionHandler:]):
(-[AsyncAutoplayPoliciesDelegate _webView:decidePolicyForNavigationAction:decisionHandler:]):
(-[AsyncAutoplayPoliciesDelegate _webView:handleAutoplayEvent:withFlags:]):
(TEST):

2018-03-19 Daniel Bates <dabates@apple.com>

test-webkitpy no longer runs WebKit2 tests
@@ -200,6 +200,42 @@ - (void)_webView:(WKWebView *)webView handleAutoplayEvent:(_WKAutoplayEvent)even

@end

@interface AsyncAutoplayPoliciesDelegate : NSObject <WKNavigationDelegate, WKUIDelegatePrivate>
@property (nonatomic, copy) _WKWebsiteAutoplayPolicy(^autoplayPolicyForURL)(NSURL *);
@property (nonatomic, copy) _WKWebsiteAutoplayQuirk(^allowedAutoplayQuirksForURL)(NSURL *);
@end

@implementation AsyncAutoplayPoliciesDelegate

- (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction decisionHandler:(void (^)(WKNavigationActionPolicy))decisionHandler
{
// _webView:decidePolicyForNavigationAction:decisionHandler: should be called instead if it is implemented.
EXPECT_TRUE(false);
decisionHandler(WKNavigationActionPolicyAllow);
}

- (void)_webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction decisionHandler:(void (^)(WKNavigationActionPolicy, _WKWebsitePolicies *))decisionHandler
{
dispatch_async(dispatch_get_main_queue(), ^{
_WKWebsitePolicies *websitePolicies = [[[_WKWebsitePolicies alloc] init] autorelease];
if (_allowedAutoplayQuirksForURL)
websitePolicies.allowedAutoplayQuirks = _allowedAutoplayQuirksForURL(navigationAction.request.URL);
if (_autoplayPolicyForURL)
websitePolicies.autoplayPolicy = _autoplayPolicyForURL(navigationAction.request.URL);
decisionHandler(WKNavigationActionPolicyAllow, websitePolicies);
});
}

#if PLATFORM(MAC)
- (void)_webView:(WKWebView *)webView handleAutoplayEvent:(_WKAutoplayEvent)event withFlags:(_WKAutoplayEventFlags)flags
{
receivedAutoplayEventFlags = flags;
receivedAutoplayEvent = event;
}
#endif

@end

TEST(WebKit, WebsitePoliciesAutoplayEnabled)
{
auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
@@ -658,6 +694,49 @@ - (void)webView:(WKWebView *)webView stopURLSchemeTask:(id <WKURLSchemeTask>)tas
[webView stringByEvaluatingJavaScript:@"play()"];
[webView waitForMessage:@"playing"];
}

TEST(WebKit, WebsitePoliciesAutoplayQuirksAsyncPolicyDelegate)
{
auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);

auto delegate = adoptNS([[AsyncAutoplayPoliciesDelegate alloc] init]);
[webView setNavigationDelegate:delegate.get()];

WKRetainPtr<WKPreferencesRef> preferences(AdoptWK, WKPreferencesCreate());
WKPreferencesSetNeedsSiteSpecificQuirks(preferences.get(), true);
WKPageGroupSetPreferences(WKPageGetPageGroup([webView _pageForTesting]), preferences.get());

NSURLRequest *requestWithAudio = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"autoplay-check" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];

[delegate setAllowedAutoplayQuirksForURL:^_WKWebsiteAutoplayQuirk(NSURL *url) {
return _WKWebsiteAutoplayQuirkSynthesizedPauseEvents;
}];
[delegate setAutoplayPolicyForURL:^(NSURL *) {
return _WKWebsiteAutoplayPolicyDeny;
}];
[webView loadRequest:requestWithAudio];
[webView waitForMessage:@"did-not-play"];
[webView waitForMessage:@"on-pause"];

receivedAutoplayEvent = std::nullopt;
[webView loadHTMLString:@"" baseURL:nil];

NSURLRequest *requestWithAudioInFrame = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"autoplay-check-in-iframe" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];

[delegate setAllowedAutoplayQuirksForURL:^_WKWebsiteAutoplayQuirk(NSURL *url) {
if ([url.lastPathComponent isEqualToString:@"autoplay-check-frame.html"])
return _WKWebsiteAutoplayQuirkInheritedUserGestures;

return _WKWebsiteAutoplayQuirkSynthesizedPauseEvents | _WKWebsiteAutoplayQuirkInheritedUserGestures;
}];
[delegate setAutoplayPolicyForURL:^(NSURL *) {
return _WKWebsiteAutoplayPolicyDeny;
}];
[webView loadRequest:requestWithAudioInFrame];
[webView waitForMessage:@"did-not-play"];
[webView waitForMessage:@"on-pause"];
}
#endif // PLATFORM(MAC)

TEST(WebKit, InvalidCustomHeaders)

0 comments on commit 9e7fee6

Please sign in to comment.