Title: [229689] trunk
- Revision
- 229689
- Author
- [email protected]
- Date
- 2018-03-16 17:38:12 -0700 (Fri, 16 Mar 2018)
Log 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):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (229688 => 229689)
--- trunk/Source/WebCore/ChangeLog 2018-03-16 23:56:53 UTC (rev 229688)
+++ trunk/Source/WebCore/ChangeLog 2018-03-17 00:38:12 UTC (rev 229689)
@@ -1,3 +1,30 @@
+2018-03-16 Chris Dumez <[email protected]>
+
+ 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 <[email protected]>
Set a trap to catch an infrequent form-related nullptr crash
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (229688 => 229689)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2018-03-16 23:56:53 UTC (rev 229688)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2018-03-17 00:38:12 UTC (rev 229689)
@@ -1542,6 +1542,13 @@
continueLoadAfterNavigationPolicy(request, formState, shouldContinue, allowNavigationToInvalidURL);
completionHandler();
});
+
+ if (policyChecker().delegateIsDecidingNavigationPolicy()) {
+ if (m_provisionalDocumentLoader) {
+ m_provisionalDocumentLoader->stopLoading();
+ setProvisionalDocumentLoader(nullptr);
+ }
+ }
}
void FrameLoader::reportLocalLoadFailed(Frame* frame, const String& url)
Modified: trunk/Tools/ChangeLog (229688 => 229689)
--- trunk/Tools/ChangeLog 2018-03-16 23:56:53 UTC (rev 229688)
+++ trunk/Tools/ChangeLog 2018-03-17 00:38:12 UTC (rev 229689)
@@ -1,3 +1,18 @@
+2018-03-16 Chris Dumez <[email protected]>
+
+ 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 <[email protected]>
Add SPI to expose width and height anchors for WKWebView's content view
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm (229688 => 229689)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm 2018-03-16 23:56:53 UTC (rev 229688)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm 2018-03-17 00:38:12 UTC (rev 229689)
@@ -200,6 +200,42 @@
@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 @@
[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)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
