Changeset 229689 in webkit

Timestamp:

Mar 16, 2018 5:38:12 PM (6 years ago)

Author:

Chris Dumez

Message:

WebKit.WebsitePoliciesAutoplayQuirks API test times out with async policy delegates
​https://bugs.webkit.org/show_bug.cgi?id=183702

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, this patch updates loadWithDocumentLoader() to cancel any
provisional load when there is an asynchronous navigation policy decision pending.

Change covered by new API test.

• loader/FrameLoader.cpp:

(WebCore::FrameLoader::loadWithDocumentLoader):

Tools:

Add API test coverage.

• TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:

(-[AsyncAutoplayPoliciesDelegate webView:decidePolicyForNavigationAction:decisionHandler:]):
(-[AsyncAutoplayPoliciesDelegate _webView:decidePolicyForNavigationAction:decisionHandler:]):
(-[AsyncAutoplayPoliciesDelegate _webView:handleAutoplayEvent:withFlags:]):
(TEST):

Location:

trunk

Files:

• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/loader/FrameLoader.cpp (modified) (1 diff)
• Tools/ChangeLog (modified) (1 diff)
• Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-03-16  Chris Dumez  <cdumez@apple.com>
+
+        WebKit.WebsitePoliciesAutoplayQuirks API test times out with async policy delegates
+        https://bugs.webkit.org/show_bug.cgi?id=183702
+
+        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, this patch updates loadWithDocumentLoader() to cancel any
+        provisional load when there is an asynchronous navigation policy decision pending.
+
+        Change covered by new API test.
+
+        * loader/FrameLoader.cpp:
+        (WebCore::FrameLoader::loadWithDocumentLoader):
+
 2018-03-16  Brent Fulgham  <bfulgham@apple.com>
 

trunk/Source/WebCore/loader/FrameLoader.cpp

@@ -1543 +1543 @@
         completionHandler();
     });
+
+    if (policyChecker().delegateIsDecidingNavigationPolicy()) {
+        if (m_provisionalDocumentLoader) {
+            m_provisionalDocumentLoader->stopLoading();
+            setProvisionalDocumentLoader(nullptr);
+        }
+    }
 }
 

trunk/Tools/ChangeLog

@@ +1 @@
+2018-03-16  Chris Dumez  <cdumez@apple.com>
+
+        WebKit.WebsitePoliciesAutoplayQuirks API test times out with async policy delegates
+        https://bugs.webkit.org/show_bug.cgi?id=183702
+
+        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-16  Wenson Hsieh  <wenson_hsieh@apple.com>
 

trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm

@@ -189 +189 @@
         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
+
+@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);
+    });
 }
 
@@ -658 +694 @@
     [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)