Make popstate always fire synchronously

Interop discussion: whatwg/html#1792 Intent to ship: https://groups.google.com/a/chromium.org/g/blink-dev/c/HXRHWirKarU Bug: 1254926 Change-Id: I7e41ab603a15a14bf9df5000edca2724766a20e9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3580022 Reviewed-by: Domenic Denicola <domenic@chromium.org> Commit-Queue: Nate Chapin <japhet@chromium.org> Cr-Commit-Position: refs/heads/main@{#996748} NOKEYCHECK=True GitOrigin-RevId: 425a4f723fdc2b4118713b0aed8203621129f01a
mjfroman · Apr 27, 2022 · 19cffee · 19cffee
1 parent ac13a24
commit 19cffee
Show file tree

Hide file tree

Showing 15 changed files with 348 additions and 15 deletions.
diff --git a/blink/common/features.cc b/blink/common/features.cc
@@ -1457,5 +1457,8 @@ const base::Feature kAllowSourceSwitchOnPausedVideoMediaStream{
     "AllowSourceSwitchOnPausedVideoMediaStream",
     base::FEATURE_ENABLED_BY_DEFAULT};
 
+const base::Feature kDispatchPopstateSync{"DispatchPopstateSync",
+                                          base::FEATURE_ENABLED_BY_DEFAULT};
+
 }  // namespace features
 }  // namespace blink
diff --git a/blink/public/common/features.h b/blink/public/common/features.h
@@ -747,6 +747,10 @@ BLINK_COMMON_EXPORT extern const base::Feature kDecodeScriptSourceOffThread;
 BLINK_COMMON_EXPORT extern const base::Feature
     kAllowSourceSwitchOnPausedVideoMediaStream;
 
+// Kill switch for firing popstate immediately, instaed of deferring it until
+// after onload.
+BLINK_COMMON_EXPORT extern const base::Feature kDispatchPopstateSync;
+
 }  // namespace features
 }  // namespace blink
 

diff --git a/blink/renderer/core/frame/local_dom_window.cc b/blink/renderer/core/frame/local_dom_window.cc
@@ -840,12 +840,15 @@ void LocalDOMWindow::StatePopped(
   if (!GetFrame())
     return;
 
-  // Per step 11 of section 6.5.9 (history traversal) of the HTML5 spec, we
-  // defer firing of popstate until we're in the complete state.
-  if (document()->IsLoadCompleted())
+  // TODO(crbug.com/1254926): Remove pending_state_object_ and the capacity to
+  // delay popstate until after the load event once the behavior is proven
+  // compatible in M103.
+  if (document()->IsLoadCompleted() ||
+      base::FeatureList::IsEnabled(features::kDispatchPopstateSync)) {
     EnqueuePopstateEvent(std::move(state_object));
-  else
+  } else {
     pending_state_object_ = std::move(state_object);
+  }
 }
 
 LocalDOMWindow::~LocalDOMWindow() = default;

diff --git a/...t/html/browsers/browsing-the-web/history-traversal/event-order/after-load-hash-twice.html b/...t/html/browsers/browsing-the-web/history-traversal/event-order/after-load-hash-twice.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Popstate/hashchange/load event ordering</title>
+
+<script>
+// Set these up super-early before we hit the network for the test harness, just in case.
+window.eventOrder = [];
+window.onhashchange = () => window.eventOrder.push("hashchange");
+window.onpopstate = () => window.eventOrder.push("popstate");
+window.onload = () => window.eventOrder.push("load");
+</script>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<script>
+async_test(t => {
+  assert_array_equals(window.eventOrder, []);
+
+  // 0 timeout is necessary because if we do location.hash assignment before load is finished firing it counts as a replacement.
+  window.addEventListener("load", () => t.step_timeout(() => {
+    assert_array_equals(window.eventOrder, ["load"]);
+
+    window.addEventListener("hashchange", t.step_func(() => {
+      assert_array_equals(window.eventOrder, ["load", "popstate", "popstate", "hashchange"]);
+
+      window.addEventListener("hashchange", t.step_func_done(() => {
+        assert_array_equals(window.eventOrder, ["load", "popstate", "popstate", "hashchange", "hashchange"]);
+      }));
+    }), { once: true });
+
+    location.hash = "#1";
+    assert_array_equals(window.eventOrder, ["load", "popstate"]);
+    location.hash = "#2";
+    assert_array_equals(window.eventOrder, ["load", "popstate", "popstate"]);
+  }, 0));
+}, "when changing hash, after the load event");
+</script>
diff --git a/...nal/wpt/html/browsers/browsing-the-web/history-traversal/event-order/after-load-hash.html b/...nal/wpt/html/browsers/browsing-the-web/history-traversal/event-order/after-load-hash.html
@@ -0,0 +1,32 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Popstate/hashchange/load event ordering</title>
+
+<script>
+// Set these up super-early before we hit the network for the test harness, just in case.
+window.eventOrder = [];
+window.onhashchange = () => window.eventOrder.push("hashchange");
+window.onpopstate = () => window.eventOrder.push("popstate");
+window.onload = () => window.eventOrder.push("load");
+</script>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<script>
+async_test(t => {
+  assert_array_equals(window.eventOrder, []);
+
+  // 0 timeout is necessary because if we do location.hash assignment before load is finished firing it counts as a replacement.
+  window.addEventListener("load", () => t.step_timeout(() => {
+    assert_array_equals(window.eventOrder, ["load"]);
+
+    window.addEventListener("hashchange", t.step_func_done(() => {
+      assert_array_equals(window.eventOrder, ["load", "popstate", "hashchange"]);
+    }));
+
+    location.hash = "#1";
+    assert_array_equals(window.eventOrder, ["load", "popstate"]);
+  }, 0));
+}, "when changing hash, after the load event");
+</script>
diff --git a/...pt/html/browsers/browsing-the-web/history-traversal/event-order/after-load-pushState.html b/...pt/html/browsers/browsing-the-web/history-traversal/event-order/after-load-pushState.html
@@ -0,0 +1,31 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Popstate/hashchange/load event ordering</title>
+
+<script>
+// Set these up super-early before we hit the network for the test harness, just in case.
+window.eventOrder = [];
+window.onhashchange = () => window.eventOrder.push("hashchange");
+window.onpopstate = () => window.eventOrder.push("popstate");
+window.onload = () => window.eventOrder.push("load");
+</script>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<script>
+async_test(t => {
+  assert_array_equals(window.eventOrder, []);
+
+  // 0 timeout is necessary because if we do pushState before load is finished firing it counts as a replacement.
+  window.addEventListener("load", () => t.step_timeout(() => {
+    assert_array_equals(window.eventOrder, ["load"]);
+
+    t.step_timeout(t.step_func_done(() => {
+      assert_array_equals(window.eventOrder, ["load"]);
+    }), 100);
+
+    history.pushState({ state: "new state" }, "");
+  }, 0));
+}, "when pushing state, after the load event");
+</script>
diff --git a/...html/browsers/browsing-the-web/history-traversal/event-order/after-load-replaceState.html b/...html/browsers/browsing-the-web/history-traversal/event-order/after-load-replaceState.html
@@ -0,0 +1,30 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Popstate/hashchange/load event ordering</title>
+
+<script>
+// Set these up super-early before we hit the network for the test harness, just in case.
+window.eventOrder = [];
+window.onhashchange = () => window.eventOrder.push("hashchange");
+window.onpopstate = () => window.eventOrder.push("popstate");
+window.onload = () => window.eventOrder.push("load");
+</script>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<script>
+async_test(t => {
+  assert_array_equals(window.eventOrder, []);
+
+  window.addEventListener("load", t.step_func(() => {
+    assert_array_equals(window.eventOrder, ["load"]);
+
+    t.step_timeout(t.step_func_done(() => {
+      assert_array_equals(window.eventOrder, ["load"]);
+    }), 100);
+
+    history.replaceState({ state: "new state" }, "");
+  }));
+}, "when replacing state, after the load event");
+</script>
diff --git a/.../html/browsers/browsing-the-web/history-traversal/event-order/before-load-hash-twice.html b/.../html/browsers/browsing-the-web/history-traversal/event-order/before-load-hash-twice.html
@@ -0,0 +1,29 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Popstate/hashchange/load event ordering</title>
+
+<script>
+// Set these up super-early before we hit the network for the test harness, just in case.
+window.eventOrder = [];
+window.onhashchange = () => window.eventOrder.push("hashchange");
+window.onpopstate = () => window.eventOrder.push("popstate");
+window.onload = () => window.eventOrder.push("load");
+</script>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<script>
+async_test(t => {
+  assert_array_equals(window.eventOrder, []);
+
+  window.addEventListener("load", t.step_func_done(() => {
+    assert_array_equals(window.eventOrder, ["popstate", "popstate", "hashchange", "hashchange", "load"]);
+  }));
+
+  location.hash = "#1";
+  assert_array_equals(window.eventOrder, ["popstate"]);
+  location.hash = "#2";
+  assert_array_equals(window.eventOrder, ["popstate", "popstate"]);
+}, "when changing hash twice, before load");
+</script>
diff --git a/...al/wpt/html/browsers/browsing-the-web/history-traversal/event-order/before-load-hash.html b/...al/wpt/html/browsers/browsing-the-web/history-traversal/event-order/before-load-hash.html
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Popstate/hashchange/load event ordering</title>
+
+<script>
+// Set these up super-early before we hit the network for the test harness, just in case.
+window.eventOrder = [];
+window.onhashchange = () => window.eventOrder.push("hashchange");
+window.onpopstate = () => window.eventOrder.push("popstate");
+window.onload = () => window.eventOrder.push("load");
+</script>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<script>
+async_test(t => {
+  assert_array_equals(window.eventOrder, []);
+
+  window.addEventListener("load", t.step_func_done(() => {
+    assert_array_equals(window.eventOrder, ["popstate", "hashchange", "load"]);
+  }));
+
+  location.hash = "#1";
+  assert_array_equals(window.eventOrder, ["popstate"]);
+}, "when changing hash, before load");
+</script>
diff --git a/...t/html/browsers/browsing-the-web/history-traversal/event-order/before-load-pushState.html b/...t/html/browsers/browsing-the-web/history-traversal/event-order/before-load-pushState.html
@@ -0,0 +1,28 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Popstate/hashchange/load event ordering</title>
+
+<script>
+// Set these up super-early before we hit the network for the test harness, just in case.
+window.eventOrder = [];
+window.onhashchange = () => window.eventOrder.push("hashchange");
+window.onpopstate = () => window.eventOrder.push("popstate");
+window.onload = () => window.eventOrder.push("load");
+</script>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<script>
+async_test(t => {
+  assert_array_equals(window.eventOrder, []);
+
+  window.addEventListener("load", t.step_func(() => {
+    t.step_timeout(t.step_func_done(() => {
+      assert_array_equals(window.eventOrder, ["load"]);
+    }), 100);
+  }));
+
+  history.pushState({ state: "new state" }, "");
+}, "when pushing state, before load");
+</script>
diff --git a/...tml/browsers/browsing-the-web/history-traversal/event-order/before-load-replaceState.html b/...tml/browsers/browsing-the-web/history-traversal/event-order/before-load-replaceState.html
@@ -0,0 +1,26 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Popstate/hashchange/load event ordering</title>
+
+<script>
+// Set these up super-early before we hit the network for the test harness, just in case.
+window.eventOrder = [];
+window.onhashchange = () => window.eventOrder.push("hashchange");
+window.onpopstate = () => window.eventOrder.push("popstate");
+window.onload = () => window.eventOrder.push("load");
+</script>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<script>
+async_test(t => {
+  assert_array_equals(window.eventOrder, []);
+
+  t.step_timeout(t.step_func_done(() => {
+    assert_array_equals(window.eventOrder, ["load"]);
+  }), 100);
+
+  history.replaceState({ state: "new state" }, "");
+}, "when replacing state, before load");
+</script>
diff --git a/...ml/browsers/browsing-the-web/history-traversal/event-order/pushState-inside-popstate.html b/...ml/browsers/browsing-the-web/history-traversal/event-order/pushState-inside-popstate.html
@@ -0,0 +1,16 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<script>
+test(t => {
+  let popstate_called = false;
+  window.onpopstate = t.step_func(e => {
+    popstate_called = true;
+    history.pushState(2, null, "#2");
+    assert_not_equals(history.state, e.state);
+  });
+  location.hash = "#1";
+  assert_true(popstate_called);
+}, "pushState inside popstate")
+</script>
diff --git a/...sers/browsing-the-web/history-traversal/event-order/same-document-traverse-immediate.html b/...sers/browsing-the-web/history-traversal/event-order/same-document-traverse-immediate.html
@@ -0,0 +1,38 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Popstate/hashchange/load event ordering</title>
+
+<script>
+// Set these up super-early before we hit the network for the test harness, just in case.
+window.eventOrder = [];
+window.onhashchange = () => window.eventOrder.push("hashchange");
+window.onpopstate = () => window.eventOrder.push("popstate");
+window.onload = () => window.eventOrder.push("load");
+</script>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<script>
+async_test(t => {
+  assert_array_equals(window.eventOrder, []);
+
+  // 0 timeout is necessary because if we do location.hash assignment before load is finished firing it counts as a replacement.
+  window.addEventListener("load", () => t.step_timeout(() => {
+    assert_array_equals(window.eventOrder, ["load"]);
+
+    window.addEventListener("hashchange", t.step_func(() => {
+      assert_array_equals(window.eventOrder, ["load", "popstate", "hashchange"]);
+
+      window.addEventListener("hashchange", t.step_func_done(() => {
+        assert_array_equals(window.eventOrder, ["load", "popstate", "hashchange", "popstate", "hashchange"]);
+      }));
+    }), { once: true });
+
+    location.hash = "#1";
+    assert_array_equals(window.eventOrder, ["load", "popstate"]);
+    history.back();
+    assert_array_equals(window.eventOrder, ["load", "popstate"]);
+  }, 0));
+}, "when traversing back, before hashchange");
+</script>
diff --git a/.../browsers/browsing-the-web/history-traversal/event-order/same-document-traverse-wait.html b/.../browsers/browsing-the-web/history-traversal/event-order/same-document-traverse-wait.html
@@ -0,0 +1,39 @@
+<!DOCTYPE html>
+<meta charset="utf-8">
+<title>Popstate/hashchange/load event ordering</title>
+
+<script>
+// Set these up super-early before we hit the network for the test harness, just in case.
+window.eventOrder = [];
+window.onhashchange = () => window.eventOrder.push("hashchange");
+window.onpopstate = () => window.eventOrder.push("popstate");
+window.onload = () => window.eventOrder.push("load");
+</script>
+
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+
+<script>
+async_test(t => {
+  assert_array_equals(window.eventOrder, []);
+
+  // 0 timeout is necessary because if we do location.hash assignment before load is finished firing it counts as a replacement.
+  window.addEventListener("load", () => t.step_timeout(() => {
+    assert_array_equals(window.eventOrder, ["load"]);
+
+    window.addEventListener("hashchange", t.step_func(() => {
+      assert_array_equals(window.eventOrder, ["load", "popstate", "hashchange"]);
+
+      window.addEventListener("hashchange", t.step_func_done(() => {
+        assert_array_equals(window.eventOrder, ["load", "popstate", "hashchange", "popstate", "hashchange"]);
+      }));
+
+      history.back();
+      assert_array_equals(window.eventOrder, ["load", "popstate", "hashchange"]);
+    }), { once: true });
+
+    location.hash = "#1";
+    assert_array_equals(window.eventOrder, ["load", "popstate"]);
+  }, 0));
+}, "when traversing back, after hashchange");
+</script>
diff --git a/blink/web_tests/external/wpt/html/browsers/history/the-history-interface/007-expected.txt b/blink/web_tests/external/wpt/html/browsers/history/the-history-interface/007-expected.txt