Skip to content

Commit 00f54b8

Browse files
cdumezcdumez
committed
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): Canonical link: https://commits.webkit.org/199356@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229689 268f45cc-cd09-0410-ab3c-d52691b4dbfc
1 parent d59aa4e commit 00f54b8

File tree

4 files changed

+128
-0
lines changed

4 files changed

+128
-0
lines changed

‎Source/WebCore/ChangeLog

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,30 @@
1+
2018-03-16 Chris Dumez <cdumez@apple.com>
2+
3+
WebKit.WebsitePoliciesAutoplayQuirks API test times out with async policy delegates
4+
https://bugs.webkit.org/show_bug.cgi?id=183702
5+
6+
Reviewed by Alex Christensen.
7+
8+
The issue is that the test calls loadHTMLString then loadRequest right after, without
9+
waiting for the first load to complete first. loadHTMLString is special as it relies
10+
on substitute data and which schedules a timer to commit the data. When doing the
11+
navigation policy check for the following loadRequest(), the substitute data timer
12+
would fire and commit its data and load. This would in turn cancel the pending
13+
navigation policy check for the loadRequest().
14+
15+
With sync policy delegates, this is not an issue because we take care of stopping
16+
all loaders when receiving the policy decision, which happens synchronously. However,
17+
when the policy decision happens asynchronously, the pending substitute data load
18+
does not get cancelled in time and it gets committed.
19+
20+
To address the issue, this patch updates loadWithDocumentLoader() to cancel any
21+
provisional load when there is an asynchronous navigation policy decision pending.
22+
23+
Change covered by new API test.
24+
25+
* loader/FrameLoader.cpp:
26+
(WebCore::FrameLoader::loadWithDocumentLoader):
27+
128
2018-03-16 Brent Fulgham <bfulgham@apple.com>
229

330
Set a trap to catch an infrequent form-related nullptr crash

‎Source/WebCore/loader/FrameLoader.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1542,6 +1542,13 @@ void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType t
15421542
continueLoadAfterNavigationPolicy(request, formState, shouldContinue, allowNavigationToInvalidURL);
15431543
completionHandler();
15441544
});
1545+
1546+
if (policyChecker().delegateIsDecidingNavigationPolicy()) {
1547+
if (m_provisionalDocumentLoader) {
1548+
m_provisionalDocumentLoader->stopLoading();
1549+
setProvisionalDocumentLoader(nullptr);
1550+
}
1551+
}
15451552
}
15461553

15471554
void FrameLoader::reportLocalLoadFailed(Frame* frame, const String& url)

‎Tools/ChangeLog

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,18 @@
1+
2018-03-16 Chris Dumez <cdumez@apple.com>
2+
3+
WebKit.WebsitePoliciesAutoplayQuirks API test times out with async policy delegates
4+
https://bugs.webkit.org/show_bug.cgi?id=183702
5+
6+
Reviewed by Alex Christensen.
7+
8+
Add API test coverage.
9+
10+
* TestWebKitAPI/Tests/WebKitCocoa/WebsitePolicies.mm:
11+
(-[AsyncAutoplayPoliciesDelegate webView:decidePolicyForNavigationAction:decisionHandler:]):
12+
(-[AsyncAutoplayPoliciesDelegate _webView:decidePolicyForNavigationAction:decisionHandler:]):
13+
(-[AsyncAutoplayPoliciesDelegate _webView:handleAutoplayEvent:withFlags:]):
14+
(TEST):
15+
116
2018-03-16 Wenson Hsieh <wenson_hsieh@apple.com>
217

318
Add SPI to expose width and height anchors for WKWebView's content view

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

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,42 @@ - (void)_webView:(WKWebView *)webView handleAutoplayEvent:(_WKAutoplayEvent)even
200200

201201
@end
202202

203+
@interface AsyncAutoplayPoliciesDelegate : NSObject <WKNavigationDelegate, WKUIDelegatePrivate>
204+
@property (nonatomic, copy) _WKWebsiteAutoplayPolicy(^autoplayPolicyForURL)(NSURL *);
205+
@property (nonatomic, copy) _WKWebsiteAutoplayQuirk(^allowedAutoplayQuirksForURL)(NSURL *);
206+
@end
207+
208+
@implementation AsyncAutoplayPoliciesDelegate
209+
210+
- (void)webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction decisionHandler:(void (^)(WKNavigationActionPolicy))decisionHandler
211+
{
212+
// _webView:decidePolicyForNavigationAction:decisionHandler: should be called instead if it is implemented.
213+
EXPECT_TRUE(false);
214+
decisionHandler(WKNavigationActionPolicyAllow);
215+
}
216+
217+
- (void)_webView:(WKWebView *)webView decidePolicyForNavigationAction:(WKNavigationAction *)navigationAction decisionHandler:(void (^)(WKNavigationActionPolicy, _WKWebsitePolicies *))decisionHandler
218+
{
219+
dispatch_async(dispatch_get_main_queue(), ^{
220+
_WKWebsitePolicies *websitePolicies = [[[_WKWebsitePolicies alloc] init] autorelease];
221+
if (_allowedAutoplayQuirksForURL)
222+
websitePolicies.allowedAutoplayQuirks = _allowedAutoplayQuirksForURL(navigationAction.request.URL);
223+
if (_autoplayPolicyForURL)
224+
websitePolicies.autoplayPolicy = _autoplayPolicyForURL(navigationAction.request.URL);
225+
decisionHandler(WKNavigationActionPolicyAllow, websitePolicies);
226+
});
227+
}
228+
229+
#if PLATFORM(MAC)
230+
- (void)_webView:(WKWebView *)webView handleAutoplayEvent:(_WKAutoplayEvent)event withFlags:(_WKAutoplayEventFlags)flags
231+
{
232+
receivedAutoplayEventFlags = flags;
233+
receivedAutoplayEvent = event;
234+
}
235+
#endif
236+
237+
@end
238+
203239
TEST(WebKit, WebsitePoliciesAutoplayEnabled)
204240
{
205241
auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
@@ -658,6 +694,49 @@ - (void)webView:(WKWebView *)webView stopURLSchemeTask:(id <WKURLSchemeTask>)tas
658694
[webView stringByEvaluatingJavaScript:@"play()"];
659695
[webView waitForMessage:@"playing"];
660696
}
697+
698+
TEST(WebKit, WebsitePoliciesAutoplayQuirksAsyncPolicyDelegate)
699+
{
700+
auto configuration = adoptNS([[WKWebViewConfiguration alloc] init]);
701+
auto webView = adoptNS([[TestWKWebView alloc] initWithFrame:NSMakeRect(0, 0, 800, 600) configuration:configuration.get()]);
702+
703+
auto delegate = adoptNS([[AsyncAutoplayPoliciesDelegate alloc] init]);
704+
[webView setNavigationDelegate:delegate.get()];
705+
706+
WKRetainPtr<WKPreferencesRef> preferences(AdoptWK, WKPreferencesCreate());
707+
WKPreferencesSetNeedsSiteSpecificQuirks(preferences.get(), true);
708+
WKPageGroupSetPreferences(WKPageGetPageGroup([webView _pageForTesting]), preferences.get());
709+
710+
NSURLRequest *requestWithAudio = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"autoplay-check" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
711+
712+
[delegate setAllowedAutoplayQuirksForURL:^_WKWebsiteAutoplayQuirk(NSURL *url) {
713+
return _WKWebsiteAutoplayQuirkSynthesizedPauseEvents;
714+
}];
715+
[delegate setAutoplayPolicyForURL:^(NSURL *) {
716+
return _WKWebsiteAutoplayPolicyDeny;
717+
}];
718+
[webView loadRequest:requestWithAudio];
719+
[webView waitForMessage:@"did-not-play"];
720+
[webView waitForMessage:@"on-pause"];
721+
722+
receivedAutoplayEvent = std::nullopt;
723+
[webView loadHTMLString:@"" baseURL:nil];
724+
725+
NSURLRequest *requestWithAudioInFrame = [NSURLRequest requestWithURL:[[NSBundle mainBundle] URLForResource:@"autoplay-check-in-iframe" withExtension:@"html" subdirectory:@"TestWebKitAPI.resources"]];
726+
727+
[delegate setAllowedAutoplayQuirksForURL:^_WKWebsiteAutoplayQuirk(NSURL *url) {
728+
if ([url.lastPathComponent isEqualToString:@"autoplay-check-frame.html"])
729+
return _WKWebsiteAutoplayQuirkInheritedUserGestures;
730+
731+
return _WKWebsiteAutoplayQuirkSynthesizedPauseEvents | _WKWebsiteAutoplayQuirkInheritedUserGestures;
732+
}];
733+
[delegate setAutoplayPolicyForURL:^(NSURL *) {
734+
return _WKWebsiteAutoplayPolicyDeny;
735+
}];
736+
[webView loadRequest:requestWithAudioInFrame];
737+
[webView waitForMessage:@"did-not-play"];
738+
[webView waitForMessage:@"on-pause"];
739+
}
661740
#endif // PLATFORM(MAC)
662741

663742
TEST(WebKit, InvalidCustomHeaders)

0 commit comments

Comments
 (0)