- Revision
- 229722
- Author
- cdu...@apple.com
- Date
- 2018-03-19 15:30:57 -0700 (Mon, 19 Mar 2018)
Log Message
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.
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (229721 => 229722)
--- trunk/LayoutTests/ChangeLog 2018-03-19 22:05:17 UTC (rev 229721)
+++ trunk/LayoutTests/ChangeLog 2018-03-19 22:30:57 UTC (rev 229722)
@@ -1,3 +1,17 @@
+2018-03-19 Chris Dumez <cdu...@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_...@apple.com>
[WebAuthN] Implement authenticatorMakeCredential
Added: trunk/LayoutTests/fast/loader/inner-iframe-loads-data-url-into-parent-on-unload-crash-async-delegate-expected.txt (0 => 229722)
--- trunk/LayoutTests/fast/loader/inner-iframe-loads-data-url-into-parent-on-unload-crash-async-delegate-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/loader/inner-iframe-loads-data-url-into-parent-on-unload-crash-async-delegate-expected.txt 2018-03-19 22:30:57 UTC (rev 229722)
@@ -0,0 +1,6 @@
+
+
+--------
+Frame: '<!--framePath //<!--frame0-->-->'
+--------
+PASS did not crash.
Added: trunk/LayoutTests/fast/loader/inner-iframe-loads-data-url-into-parent-on-unload-crash-async-delegate.html (0 => 229722)
--- trunk/LayoutTests/fast/loader/inner-iframe-loads-data-url-into-parent-on-unload-crash-async-delegate.html (rev 0)
+++ trunk/LayoutTests/fast/loader/inner-iframe-loads-data-url-into-parent-on-unload-crash-async-delegate.html 2018-03-19 22:30:57 UTC (rev 229722)
@@ -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 = ""
+
+var childFrame = parentFrame.contentDocument.body.appendChild(document.createElement("iframe"));
+childFrame.contentWindow._onunload_ = function () {
+ var link = parentFrame.contentDocument.createElement("a");
+ link.href = "" did not crash.<script>window.testRunner && window.testRunner.notifyDone()</" + "script>";
+ link.click(); // Navigates parentFrame
+}
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (229721 => 229722)
--- trunk/Source/WebCore/ChangeLog 2018-03-19 22:05:17 UTC (rev 229721)
+++ trunk/Source/WebCore/ChangeLog 2018-03-19 22:30:57 UTC (rev 229722)
@@ -1,3 +1,42 @@
+2018-03-19 Chris Dumez <cdu...@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.carl...@apple.com>
[Extra zoom mode] Require fullscreen for video playback
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (229721 => 229722)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2018-03-19 22:05:17 UTC (rev 229721)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2018-03-19 22:30:57 UTC (rev 229722)
@@ -1544,6 +1544,15 @@
});
}
+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());
Modified: trunk/Source/WebCore/loader/FrameLoader.h (229721 => 229722)
--- trunk/Source/WebCore/loader/FrameLoader.h 2018-03-19 22:05:17 UTC (rev 229721)
+++ trunk/Source/WebCore/loader/FrameLoader.h 2018-03-19 22:30:57 UTC (rev 229722)
@@ -141,6 +141,7 @@
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);
Modified: trunk/Source/WebCore/loader/PolicyChecker.cpp (229721 => 229722)
--- trunk/Source/WebCore/loader/PolicyChecker.cpp 2018-03-19 22:05:17 UTC (rev 229721)
+++ trunk/Source/WebCore/loader/PolicyChecker.cpp 2018-03-19 22:30:57 UTC (rev 229722)
@@ -144,6 +144,8 @@
m_contentFilterUnblockHandler = { };
#endif
+ m_frame.loader().clearProvisionalLoadForPolicyCheck();
+
m_delegateIsDecidingNavigationPolicy = true;
String suggestedFilename = action.downloadAttribute().isEmpty() ? nullAtom() : action.downloadAttribute();
ResourceRequest requestCopy = request;
Modified: trunk/Tools/ChangeLog (229721 => 229722)
--- trunk/Tools/ChangeLog 2018-03-19 22:05:17 UTC (rev 229721)
+++ trunk/Tools/ChangeLog 2018-03-19 22:30:57 UTC (rev 229722)
@@ -1,3 +1,19 @@
+2018-03-19 Chris Dumez <cdu...@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 <daba...@apple.com>
test-webkitpy no longer runs WebKit2 tests
Modified: trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm (229721 => 229722)
--- trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm 2018-03-19 22:05:17 UTC (rev 229721)
+++ trunk/Tools/TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm 2018-03-19 22:30:57 UTC (rev 229722)
@@ -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)
