- Revision
- 229108
- Author
- cdu...@apple.com
- Date
- 2018-02-28 15:58:22 -0800 (Wed, 28 Feb 2018)
Log Message
html/browsers/browsing-the-web/navigating-across-documents/006.html fails with async policy delegates
https://bugs.webkit.org/show_bug.cgi?id=183168
<rdar://problem/37951341>
Reviewed by Alex Christensen.
Source/WebCore:
The test has an anchor element with both a 'click' event handler which submits a form
and an href attribute. When clicking the link, as per specification, things happen in
this order:
1. We fire the click event at the anchor, which will execute the event handler and submit the form.
Submitting the form *schedules* a navigation to 'click.html'.
2. We execute the anchor activation code which *navigates* to 'href.html'. The navigation to
'href' is supposed to cancel the pending navigation to 'click.html' and we should navigate
to 'href.html', which is what the test asserts.
The issue for us is that we do not cancel pending navigations until after the navigation
policy decision is made, when the provisional loads actually starts, in FrameLoader::provisionalLoadStarted().
Because the policy decision for the navigation can now be made asynchronously, the NavigationScheduler
timer can now fire while the decision is made and we'll submit the form, thus navigating to
'click.html'.
To address the issue, we now cancel any pending navigations in FrameLoader::loadWithDocumentLoader(),
*before* doing the policy check for the navigation.
Test: http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html
* loader/FrameLoader.cpp:
(WebCore::FrameLoader::loadWithDocumentLoader):
LayoutTests:
Add layout test coverage.
* TestExpectations:
* http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006-expected.txt: Added.
* http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html: Added.
* http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html: Added.
* http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (229107 => 229108)
--- trunk/LayoutTests/ChangeLog 2018-02-28 23:23:48 UTC (rev 229107)
+++ trunk/LayoutTests/ChangeLog 2018-02-28 23:58:22 UTC (rev 229108)
@@ -1,3 +1,19 @@
+2018-02-28 Chris Dumez <cdu...@apple.com>
+
+ html/browsers/browsing-the-web/navigating-across-documents/006.html fails with async policy delegates
+ https://bugs.webkit.org/show_bug.cgi?id=183168
+ <rdar://problem/37951341>
+
+ Reviewed by Alex Christensen.
+
+ Add layout test coverage.
+
+ * TestExpectations:
+ * http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006-expected.txt: Added.
+ * http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html: Added.
+ * http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html: Added.
+ * http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html: Added.
+
2018-02-28 Alicia Boya García <ab...@igalia.com>
Unreviewed GTK test gardening.
Modified: trunk/LayoutTests/TestExpectations (229107 => 229108)
--- trunk/LayoutTests/TestExpectations 2018-02-28 23:23:48 UTC (rev 229107)
+++ trunk/LayoutTests/TestExpectations 2018-02-28 23:58:22 UTC (rev 229108)
@@ -289,6 +289,10 @@
# This is a resources folder.
http/tests/workers/service/other_resources [ Skip ]
+# These are resource files.
+http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html
+http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html
+
# We fail this reftest
webkit.org/b/179881 imported/w3c/web-platform-tests/encoding/eof-shift_jis.html [ ImageOnlyFailure ]
Added: trunk/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006-expected.txt (0 => 229108)
--- trunk/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006-expected.txt (rev 0)
+++ trunk/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006-expected.txt 2018-02-28 23:58:22 UTC (rev 229108)
@@ -0,0 +1,4 @@
+
+PASS Link with onclick form submit and href navigation
+
+Test
Added: trunk/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html (0 => 229108)
--- trunk/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html (rev 0)
+++ trunk/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html 2018-02-28 23:58:22 UTC (rev 229108)
@@ -0,0 +1,21 @@
+<!doctype html>
+<title>Link with onclick form submit and href navigation </title>
+<script src=""
+<script src=""
+<script>
+if (window.testRunner && testRunner.setShouldDecideNavigationPolicyAfterDelay)
+ testRunner.setShouldDecideNavigationPolicyAfterDelay(true);
+</script>
+<div id="log"></div>
+<iframe name="test"></iframe>
+<form target="test" action=""
+<a target="test" _onclick_="document.forms[0].submit()" href=""
+<script>
+var t = async_test();
+t.step(function() {document.links[0].click()});
+_onmessage_ = t.step_func(
+ function(e) {
+ assert_equals(e.data, "href");
+ t.done();
+ });
+</script>
Added: trunk/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html (0 => 229108)
--- trunk/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html (rev 0)
+++ trunk/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html 2018-02-28 23:58:22 UTC (rev 229108)
@@ -0,0 +1,4 @@
+<!doctype html>
+<script>
+parent.postMessage("click", "*");
+</script>
Added: trunk/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html (0 => 229108)
--- trunk/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html (rev 0)
+++ trunk/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html 2018-02-28 23:58:22 UTC (rev 229108)
@@ -0,0 +1,5 @@
+<!doctype html>
+<script>
+parent.postMessage("href", "*");
+</script>
+href
Modified: trunk/Source/WebCore/ChangeLog (229107 => 229108)
--- trunk/Source/WebCore/ChangeLog 2018-02-28 23:23:48 UTC (rev 229107)
+++ trunk/Source/WebCore/ChangeLog 2018-02-28 23:58:22 UTC (rev 229108)
@@ -1,3 +1,34 @@
+2018-02-28 Chris Dumez <cdu...@apple.com>
+
+ html/browsers/browsing-the-web/navigating-across-documents/006.html fails with async policy delegates
+ https://bugs.webkit.org/show_bug.cgi?id=183168
+ <rdar://problem/37951341>
+
+ Reviewed by Alex Christensen.
+
+ The test has an anchor element with both a 'click' event handler which submits a form
+ and an href attribute. When clicking the link, as per specification, things happen in
+ this order:
+ 1. We fire the click event at the anchor, which will execute the event handler and submit the form.
+ Submitting the form *schedules* a navigation to 'click.html'.
+ 2. We execute the anchor activation code which *navigates* to 'href.html'. The navigation to
+ 'href' is supposed to cancel the pending navigation to 'click.html' and we should navigate
+ to 'href.html', which is what the test asserts.
+
+ The issue for us is that we do not cancel pending navigations until after the navigation
+ policy decision is made, when the provisional loads actually starts, in FrameLoader::provisionalLoadStarted().
+ Because the policy decision for the navigation can now be made asynchronously, the NavigationScheduler
+ timer can now fire while the decision is made and we'll submit the form, thus navigating to
+ 'click.html'.
+
+ To address the issue, we now cancel any pending navigations in FrameLoader::loadWithDocumentLoader(),
+ *before* doing the policy check for the navigation.
+
+ Test: http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html
+
+ * loader/FrameLoader.cpp:
+ (WebCore::FrameLoader::loadWithDocumentLoader):
+
2018-02-28 John Wilander <wilan...@apple.com>
Add a second tier of prevalence to facilitate telemetry on very prevalent domains
Modified: trunk/Source/WebCore/loader/FrameLoader.cpp (229107 => 229108)
--- trunk/Source/WebCore/loader/FrameLoader.cpp 2018-02-28 23:23:48 UTC (rev 229107)
+++ trunk/Source/WebCore/loader/FrameLoader.cpp 2018-02-28 23:58:22 UTC (rev 229108)
@@ -1530,6 +1530,8 @@
}
}
+ m_frame.navigationScheduler().cancel(true);
+
policyChecker().checkNavigationPolicy(ResourceRequest(loader->request()), false /* didReceiveRedirectResponse */, loader, formState, [this, allowNavigationToInvalidURL] (const ResourceRequest& request, FormState* formState, bool shouldContinue) {
continueLoadAfterNavigationPolicy(request, formState, shouldContinue, allowNavigationToInvalidURL);
});
