Skip to content

Commit

Permalink
Make popstate always fire synchronously
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
natechapin authored and Chromium LUCI CQ committed Apr 27, 2022
1 parent 3a43e94 commit 425a4f7
Show file tree
Hide file tree
Showing 15 changed files with 348 additions and 15 deletions.
3 changes: 3 additions & 0 deletions third_party/blink/common/features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions third_party/blink/public/common/features.h
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
11 changes: 7 additions & 4 deletions third_party/blink/renderer/core/frame/local_dom_window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>
Original file line number Diff line number Diff line change
@@ -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>

This file was deleted.

0 comments on commit 425a4f7

Please sign in to comment.