html/browsers/browsing-the-web/navigating-across-documents/006.html fails with async...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 28 Feb 2018 23:58:22 +0000 (23:58 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 28 Feb 2018 23:58:22 +0000 (23:58 +0000)
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.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@229108 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006-expected.txt [new file with mode: 0644]
LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html [new file with mode: 0644]
LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html [new file with mode: 0644]
LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/loader/FrameLoader.cpp

index cdcf360..15d1ee4 100644 (file)
@@ -1,3 +1,19 @@
+2018-02-28  Chris Dumez  <cdumez@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  <aboya@igalia.com>
 
         Unreviewed GTK test gardening.
index 2a50f82..0d9d1b5 100644 (file)
@@ -289,6 +289,10 @@ imported/w3c/web-platform-tests/background-fetch [ Skip ]
 # 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 ]
 
diff --git a/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006-expected.txt b/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006-expected.txt
new file mode 100644 (file)
index 0000000..333d7e9
--- /dev/null
@@ -0,0 +1,4 @@
+
+PASS Link with onclick form submit and href navigation  
+
+Test
diff --git a/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html b/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/006.html
new file mode 100644 (file)
index 0000000..3eb173f
--- /dev/null
@@ -0,0 +1,21 @@
+<!doctype html>
+<title>Link with onclick form submit and href navigation </title>
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+if (window.testRunner && testRunner.setShouldDecideNavigationPolicyAfterDelay)
+    testRunner.setShouldDecideNavigationPolicyAfterDelay(true);
+</script>
+<div id="log"></div>
+<iframe name="test"></iframe>
+<form target="test" action="click.html"></form>
+<a target="test" onclick="document.forms[0].submit()" href="href.html">Test</a>
+<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>
diff --git a/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html b/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/click.html
new file mode 100644 (file)
index 0000000..8cb03b7
--- /dev/null
@@ -0,0 +1,4 @@
+<!doctype html>
+<script>
+parent.postMessage("click", "*");
+</script>
diff --git a/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html b/LayoutTests/http/wpt/html/browsers/browsing-the-web/navigating-across-documents/href.html
new file mode 100644 (file)
index 0000000..eccadad
--- /dev/null
@@ -0,0 +1,5 @@
+<!doctype html>
+<script>
+parent.postMessage("href", "*");
+</script>
+href
index 9b7158c..a255e22 100644 (file)
@@ -1,3 +1,34 @@
+2018-02-28  Chris Dumez  <cdumez@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  <wilander@apple.com>
 
         Add a second tier of prevalence to facilitate telemetry on very prevalent domains
index a9fcacc..ee36e5d 100644 (file)
@@ -1530,6 +1530,8 @@ void FrameLoader::loadWithDocumentLoader(DocumentLoader* loader, FrameLoadType t
         }
     }
 
+    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);
     });