dmaax/ladybird
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

LibWeb: Keep session history coherent when navigations jump the queue

Browse source 
Problem: A document could drop a load if it ran a stream of synchronous
same-document history navigations (say, a pushState flood) while it was
concurrently loaded again. The load never finished — so on sanitizer/
slow builds, this had been intermittently taking down unrelated tests in
CI — since test-web reuses one WebContent process, and the next test’s
load can arrive while the previous document’s history work is still
draining. A synchronous commit also claimed a session history step it
never retired — so claimed steps piled up without bound.

Cause: A sync same-document navigation committed immediately and could
jump the session-history-traversal queue while a queued apply-history-
step — such as a cross-document load — was still waiting behind it. The
queued run read the active session-history entry after the sync
navigation had installed it, but before its step number was assigned —
then judged itself stale against that still-pending step, and was
discarded. The shared step numbering was fragile under the same nesting:
A number computed from the current step alone could collide with an in-
flight one — and a stale run that completed later could write its own
step back over a newer one. 394312ab5a stopped the crash this used to
cause, but the races remained.

Fix: Treat a queued push whose displayed entry’s step is still pending
as live rather than stale — so the concurrent load isn’t dropped. Number
apply-history-step runs, and let a run commit its target step only if no
newer run has committed one — so a stale run can’t move the current step
backwards. Claim each new step past every claimed-but-uncommitted step —
rather than from the current step alone, and keep clearing the forward
session history from removing those entries. And retire the step a sync
commit claims, since it applies in the same task, and nothing else will.

See https://github.com/LadybirdBrowser/ladybird/issues/10028
This commit is contained in:
sideshowbarker 2026-06-17 08:06:28 +09:00 committed by Andreas Kling
parent 427a66d448
commit e61a91c017
7 changed files with 163 additions and 32 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
Libraries/LibWeb/HTML/Navigable.cpp
View file

@ -3018,7 +3018,9 @@ void finalize_a_cross_document_navigation(GC::Ref<Navigable> navigable, HistoryH
traversable->clear_the_forward_session_history();
// 2. Set targetStep to traversable's current session history step + 1.
target_step = traversable->current_session_history_step() + 1;
// AD-HOC: Claim the step instead — so a step claimed by an apply-history-step run still in flight can't be
// handed out twice. See https://github.com/whatwg/html/issues/12576.
target_step = traversable->claim_next_session_history_step();
// 3. Set historyEntry's step to targetStep.
history_entry->set_step(target_step);

108
Libraries/LibWeb/HTML/TraversableNavigable.cpp
View file

@ -804,7 +804,8 @@ public:
GC::Ptr<Navigable> expected_ongoing_navigation_navigable,
Optional<String> expected_ongoing_navigation_id,
GC::Ref<OnApplyHistoryStepComplete> on_complete)
: m_traversable(traversable)
: m_generation(++traversable->m_apply_history_step_generation_counter)
, m_traversable(traversable)
, m_step(step)
, m_target_step(target_step)
, m_source_snapshot_params(source_snapshot_params)
@ -919,6 +920,7 @@ private:
void clear_ongoing_traversals_for_changing_navigables();
Phase m_phase { Phase::WaitingForDocumentPopulation };
u64 m_generation { 0 };
GC::Ref<TraversableNavigable> m_traversable;
int m_step;
int m_target_step;
@ -1039,7 +1041,7 @@ void ApplyHistoryStepState::start()
auto displayed_step = displayed_entry ? displayed_entry->step_value() : Optional<int> {};
auto target_step = target_entry ? target_entry->step_value() : Optional<int> {};
if (!displayed_step.has_value() || !target_step.has_value()) {
if (!target_step.has_value()) {
// NB: Child navigables created during a busy top-level navigation can still have a pending initial
// session history entry. The spec's step-based history algorithms operate on used history steps,
// so a pending child entry must not block or crash the top-level apply-history step. The queued
@ -1090,12 +1092,13 @@ void ApplyHistoryStepState::start()
case Bindings::NavigationType::Replace:
// FIXME: Add ever populated check
// - "replace": Assert: targetEntry's step is displayedEntry's step and targetEntry's document state's ever populated is false.
VERIFY(target_entry->step() == displayed_entry->step());
if (displayed_step.has_value())
VERIFY(target_entry->step() == displayed_entry->step());
break;
case Bindings::NavigationType::Push:
// FIXME: Add ever populated check, and fix the bug where top level traversable's step is not updated when a child navigable navigates
// - "push": Assert: targetEntry's step is displayedEntry's step + 1 and targetEntry's document state's ever populated is false.
if (*target_step <= *displayed_step) {
if (displayed_step.has_value() && *target_step <= *displayed_step) {
// AD-HOC: A queued push can become stale if a later navigation commits before this task runs.
// Browser engines let the later navigation win; do the same and avoid moving the
// traversable's current step back to this push target during completion.
@ -1519,37 +1522,48 @@ void ApplyHistoryStepState::complete()
m_phase = Phase::Completed;
m_timeout->stop();
// https://html.spec.whatwg.org/multipage/browsing-the-web.html#getting-the-used-step
// NB: targetStep was computed before the asynchronous portions of applying the history step. If a child
// navigable was removed while those steps were running, that step can stop being used. Normalize again
// before storing it as the traversable's current session history step.
m_target_step = m_traversable->get_the_used_step(m_target_step);
// AD-HOC: Commit the target step only if no newer apply history step has committed one. A synchronous navigation
// jumping the queue while this step was paused has a newer target step; moving the current step back past
// it would let the next push assign a step number that an existing entry already holds.
// See https://github.com/whatwg/html/issues/12576.
if (m_generation > m_traversable->m_committed_apply_history_step_generation) {
m_traversable->m_committed_apply_history_step_generation = m_generation;
// 20. Set traversable's current session history step to targetStep.
m_traversable->m_current_session_history_step = m_target_step;
// https://html.spec.whatwg.org/multipage/browsing-the-web.html#getting-the-used-step
// NB: targetStep was computed before the asynchronous portions of applying the history step. If a child
// navigable was removed while those steps were running, that step can stop being used. Normalize again
// before storing it as the current session history step; keep it in a local so the claimed step retired
// below still matches what the navigation claimed, not the re-normalized value.
auto const used_target_step = m_traversable->get_the_used_step(m_target_step);
// AD-HOC: Report the updated session history descriptors to the UI-process mirror.
if (m_traversable->page().client().should_report_session_history_updates()) {
auto save_active_entry_persisted_state = TraversableNavigable::SaveActiveEntryPersistedState::Yes;
// NB: During history traversal, the active entry can point at the target
// entry before the active document's queued history-step update has
// restored the target entry's persisted state. Do not overwrite that
// target entry with the document's pre-restoration viewport offset.
if (m_navigation_type == Bindings::NavigationType::Traverse) {
auto document = m_traversable->active_document();
auto active_entry = m_traversable->active_session_history_entry();
if (document && active_entry && document->latest_entry() != active_entry)
save_active_entry_persisted_state = TraversableNavigable::SaveActiveEntryPersistedState::No;
// 20. Set traversable's current session history step to targetStep.
m_traversable->m_current_session_history_step = used_target_step;
// AD-HOC: Report the updated session history descriptors to the UI-process mirror.
if (m_traversable->page().client().should_report_session_history_updates()) {
auto save_active_entry_persisted_state = TraversableNavigable::SaveActiveEntryPersistedState::Yes;
// NB: During history traversal, the active entry can point at the target
// entry before the active document's queued history-step update has
// restored the target entry's persisted state. Do not overwrite that
// target entry with the document's pre-restoration viewport offset.
if (m_navigation_type == Bindings::NavigationType::Traverse) {
auto document = m_traversable->active_document();
auto active_entry = m_traversable->active_session_history_entry();
if (document && active_entry && document->latest_entry() != active_entry)
save_active_entry_persisted_state = TraversableNavigable::SaveActiveEntryPersistedState::No;
}
auto session_history_snapshot = m_traversable->create_session_history_snapshot(save_active_entry_persisted_state);
m_traversable->page().client().page_did_update_session_history(session_history_snapshot.top_level_session_history_entries, session_history_snapshot.used_session_history_steps, session_history_snapshot.current_used_step_index);
}
auto session_history_snapshot = m_traversable->create_session_history_snapshot(save_active_entry_persisted_state);
m_traversable->page().client().page_did_update_session_history(session_history_snapshot.top_level_session_history_entries, session_history_snapshot.used_session_history_steps, session_history_snapshot.current_used_step_index);
VERIFY(m_traversable->m_session_history_entries.size() > 0);
auto back_enabled = m_traversable->can_go_back();
auto forward_enabled = m_traversable->can_go_forward();
m_traversable->page().client().page_did_update_navigation_buttons_state(back_enabled, forward_enabled);
m_traversable->page().client().page_did_change_url(m_traversable->current_session_history_entry()->url());
}
VERIFY(m_traversable->m_session_history_entries.size() > 0);
auto back_enabled = m_traversable->can_go_back();
auto forward_enabled = m_traversable->can_go_forward();
m_traversable->page().client().page_did_update_navigation_buttons_state(back_enabled, forward_enabled);
m_traversable->page().client().page_did_change_url(m_traversable->current_session_history_entry()->url());
m_traversable->retire_claimed_session_history_step(m_target_step);
// Clear state BEFORE on_complete, because on_complete may resolve a promise
// that triggers the next session history traversal queue entry.
@ -1569,6 +1583,7 @@ void ApplyHistoryStepState::finish_without_applying()
m_phase = Phase::Completed;
m_timeout->stop();
clear_ongoing_traversals_for_changing_navigables();
m_traversable->retire_claimed_session_history_step(m_target_step);
m_traversable->m_apply_history_step_state = nullptr;
if (m_on_complete)
m_on_complete->function()(HistoryStepResult::Applied);
@ -1625,6 +1640,21 @@ void ApplyHistoryStepState::clear_ongoing_traversals_for_changing_navigables()
clear_ongoing_traversal_for_changing_navigable(navigable);
}
int TraversableNavigable::claim_next_session_history_step()
{
int step = m_current_session_history_step;
for (auto claimed : m_outstanding_claimed_session_history_steps)
step = max(step, claimed);
++step;
m_outstanding_claimed_session_history_steps.append(step);
return step;
}
void TraversableNavigable::retire_claimed_session_history_step(int step)
{
m_outstanding_claimed_session_history_steps.remove_first_matching([step](int claimed) { return claimed == step; });
}
// https://html.spec.whatwg.org/multipage/browsing-the-web.html#apply-the-history-step
void TraversableNavigable::apply_the_history_step(
int step,
@ -2075,6 +2105,13 @@ void TraversableNavigable::clear_the_forward_session_history()
// 2. Let step be the navigable's current session history step.
auto step = current_session_history_step();
// AD-HOC: An apply-history-step run can still be in flight with a claimed step above the current step; its entry is
// not forward history that traversing away abandoned — it's the entry the in-flight run is about to make
// current. Removing it would leave that run applying a step with no entry.
// See https://github.com/whatwg/html/issues/12576.
for (auto claimed : m_outstanding_claimed_session_history_steps)
step = max(step, claimed);
// 3. Let entryLists be the ordered set « navigable's session history entries ».
Vector<Vector<NonnullRefPtr<SessionHistoryEntry>>&> entry_lists;
entry_lists.append(session_history_entries());
@ -2344,7 +2381,9 @@ static Optional<int> update_session_history_entries_for_same_document_navigation
traversable.clear_the_forward_session_history();
// 2. Set targetStep to traversable's current session history step + 1.
target_step = traversable.current_session_history_step() + 1;
// AD-HOC: Claim the step instead — so a step claimed by an apply-history-step run still in flight can't be
// handed out twice. See https://github.com/whatwg/html/issues/12576.
target_step = traversable.claim_next_session_history_step();
// 3. Set targetEntry's step to targetStep.
target_entry->set_step(*target_step);
@ -2384,6 +2423,13 @@ bool TraversableNavigable::try_to_synchronously_commit_same_document_navigation(
target_navigable->set_current_session_history_entry(target_entry);
m_current_session_history_step = get_the_used_step(*target_step);
// AD-HOC: A synchronous commit applies its step in this same task, so it has no asynchronous window in which
// another run could observe the claim. Retire it immediately. Unlike the queued apply-history-step path,
// nothing else will. Leaving it outstanding would let claimed steps accumulate without bound — and push
// later step numbers ever higher. (A replacement reuses the current step and claimed nothing, so retiring
// it is a no-op.) See https://github.com/whatwg/html/issues/12576.
retire_claimed_session_history_step(*target_step);
// NB: The queued apply-history-step path clears the ongoing navigation when the history step finishes. The
// synchronous fast path has already committed the same-document navigation and the Navigation API entry update
// owns settling its promises/events, so do the same cleanup without reporting an abort to the Navigation API.

31
Libraries/LibWeb/HTML/TraversableNavigable.h
View file

@ -44,6 +44,13 @@ public:
virtual bool is_top_level_traversable() const override;
int current_session_history_step() const { return m_current_session_history_step; }
// Claims the step number for a new push-type session history entry. Claims are tracked separately from the current
// step: The current step only advances when an apply-history-step run commits — and several runs can have claimed
// steps in flight at once. So, computing a new step from the current step alone can hand out a step number that an
// existing entry already holds. A claim is retired when the run that applies it completes.
[[nodiscard]] int claim_next_session_history_step();
void retire_claimed_session_history_step(int step);
Vector<NonnullRefPtr<SessionHistoryEntry>>& session_history_entries() { return m_session_history_entries; }
Vector<NonnullRefPtr<SessionHistoryEntry>> const& session_history_entries() const { return m_session_history_entries; }
struct SessionHistorySnapshot {
@ -192,6 +199,30 @@ private:
// https://html.spec.whatwg.org/multipage/document-sequences.html#tn-current-session-history-step
int m_current_session_history_step { 0 };
// Concurrent apply-history-step runs share the step numbering below. Runs are serialized through the session
// history traversal queue — but a synchronous navigation can jump the queue while another run is paused (see the
// "sync navigations jump queue" in the spec). So, a nested run mutates this state while an outer run is mid-flight.
// The spec describes that concurrency — but not the necessary bookkeeping it ends up requiring in implementations.
// See https://github.com/whatwg/html/issues/12576.
//
// Four invariants keep the numbering coherent under that nesting:
//
// - uniqueness: a new step number is claimed past every claimed-but-uncommitted step — never just current step + 1
// (claim_next_session_history_step);
//
// - ordering: a run commits its target step only if no newer run has committed first — so the current step can't
// move backwards past a newer run's commit (ApplyHistoryStepState::complete);
//
// - integrity: clearing the forward session history spares entries whose steps are claimed by runs still in flight
// (clear_the_forward_session_history);
//
// - initialization: step numbers are only compared once assigned; a pushed entry is the active session history
// entry before its queued synchronous step assigns its step number (the Push assertion in
// ApplyHistoryStepState::start).
u64 m_apply_history_step_generation_counter { 0 };
u64 m_committed_apply_history_step_generation { 0 };
Vector<int> m_outstanding_claimed_session_history_steps;
// https://html.spec.whatwg.org/multipage/document-sequences.html#tn-session-history-entries
Vector<NonnullRefPtr<SessionHistoryEntry>> m_session_history_entries;

1
Tests/LibWeb/Text/expected/HTML/load-during-session-history-push-drain.txt Normal file
View file

@ -0,0 +1 @@
PASS (didn't crash)

1
Tests/LibWeb/Text/expected/HTML/load-during-session-history-push-flood.txt Normal file
View file

@ -0,0 +1 @@
PASS (didn't crash)

19
Tests/LibWeb/Text/input/HTML/load-during-session-history-push-drain.html Normal file
View file

@ -0,0 +1,19 @@
<!DOCTYPE html>
<script src="../include.js"></script>
<body>
<script>
asyncTest(done => {
if (location.search.includes("loaded")) {
println("PASS (didn't crash)");
done();
return;
}
document.body.appendChild(document.createElement("iframe"));
history.pushState({}, "", "#pushed");
const next = new URL(location.href);
next.search = "?loaded";
next.hash = "";
internals.loadURL(next.href);
});
</script>
</body>

31
Tests/LibWeb/Text/input/HTML/load-during-session-history-push-flood.html Normal file
View file

@ -0,0 +1,31 @@
<!DOCTYPE html>
<script src="../include.js"></script>
<!-- A stream of same-document pushStates keeps claimed-but-uncommitted steps in flight while the page is loaded again
in the same way a request from the UI process loads it. The load used to race the in-flight push machinery in three
ways: (1) it could read the pushed entry's still-pending step, (2) it could claim a step number an in-flight push
already held, (3) and clearing the forward session history could remove the in-flight push's entry out from under
its apply-history-step run. The reloaded page signals completion. The races need the load to land inside another
run's window — so this reproduces most readily on slow builds (e.g., Sanitizer builds). -->
<body>
<script>
asyncTest(done => {
if (location.search.includes("loaded")) {
println("PASS (didn't crash)");
done();
return;
}
let round = 0;
(function pump() {
if (round++ >= 60)
return;
for (let i = 0; i < 5; i++)
history.pushState({ i }, "", "#s" + round + "-" + i);
setTimeout(pump);
})();
const next = new URL(location.href);
next.search = "?loaded";
next.hash = "";
setTimeout(() => internals.loadURL(next.href), 8);
});
</script>
</body>