Skip to content

Abort in-flight navigation API traversals to pruned entries - #12846

Open
noamr wants to merge 1 commit into
mainfrom
noamr/history-race
Open

Abort in-flight navigation API traversals to pruned entries#12846
noamr wants to merge 1 commit into
mainfrom
noamr/history-race

Conversation

@noamr

@noamr noamr commented Aug 26, 2026

Copy link
Copy Markdown
Contributor

When a traversal is in flight and an intervening same-document navigation (e.g., history.pushState()) prunes the target entry from navigation.entries(), the spec previously hit an assertion failure during same-document entry updates.

This change aligns with chromium implementation by:

  • Rejecting upcoming traverse method trackers with AbortError when their
    target entry is disposed or missing during traverse navigate event firing.
  • Removing the invalid assertion in same-document navigation API entry
    updates and failing silently, relying on the above rejection to notify the developer.

Closes #12574.

  • At least two implementers are interested (and none opposed):
  • Tests are written and can be reviewed and commented upon at:
  • Implementation bugs are filed:
    • Chromium: already works
    • Gecko: …
    • WebKit: …
    • Deno (only for timers, structured clone, base64 utils, channel messaging, module resolution, web workers, and web storage): …
    • Node.js (only for timers, structured clone, base64 utils, channel messaging, and module resolution): …
  • The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)


/browsing-the-web.html ( diff )
/nav-history-apis.html ( diff )

@noamr noamr changed the title Gracefully handle races in navigation API sync/async navigations Abort in-flight navigation API traversals to pruned entries Aug 26, 2026
@zcorpan

zcorpan commented Sep 1, 2026

Copy link
Copy Markdown
Member

I asked Claude if this matches Chromium:


I checked this against the Chromium implementation. The core of the change matches, but three of the four pieces don't, and one of them introduces a fresh assertion failure.

In Chromium the whole mechanism lives in one place, NavigationApi::DispatchNavigateEvent(), before PromoteUpcomingNavigationToOngoing() and before the NavigateEvent is created:

if (IsBackForwardOrRestore(params->frame_load_type) &&
    params->event_type == NavigateEventType::kFragment &&
    !keys_to_indices_.Contains(key)) {
  TraverseCancelled(key, mojom::blink::TraverseCancelledReason::kAbortedBeforeCommit);
  return DispatchResult::kAbort;
}

TraverseCancelled() rejects the finished promise with an AbortError and erases the tracker, and kAbort makes DocumentLoader::CommitSameDocumentNavigation() return CommitResult::Aborted, so the same-document traversal does not commit in that document at all. This is per-document and applies to child frames too.

The new bail in "fire a traverse navigate event" is a faithful translation of that check, including the property that no navigate event fires (and hence no navigateerror).

The dispose-time rejections have no counterpart

NavigationApi::UpdateForNavigation() and NavigationApi::SetEntriesForRestore() update keys_to_indices_ and fire dispose, but do not inspect upcoming_traverse_api_method_trackers_. Chrome Canary appears to settle the traversal promises several task turns after pushState() returns, whereas the new steps 7.1.2.1 of "update the navigation API entries for reactivation" and 13.2.1 of "update the navigation API entries for a same-document navigation" settle them synchronously inside pushState(), before the dispose event on the pruned entry.

They also make the observable error non-deterministic. If the entry is pruned before step 12.2 of "perform a navigation API traversal" runs, that step already rejects:

Let targetSHE be the session history entry in navigableSHEs whose navigation API key is key. If no such entry exists: [...] reject the finished promise for apiMethodTracker with an "InvalidStateError" DOMException

...and that matches Chromium's browser-side TraverseCancelledReason::kNotFound, which produces an InvalidStateError ("Invalid key"). With the dispose-time rejection added, whether authors see InvalidStateError or AbortError depends on the interleaving.

The dispose-time rejections can trigger an assertion

Rejecting the tracker does not cancel the steps already appended by step 12 of "perform a navigation API traversal". Once the dispose loop has rejected and cleaned up the tracker, those steps still run, and step 12.2 (quoted above) or 12.3 unconditionally rejects the same apiMethodTracker a second time. Step 3 of "reject the finished promise" then runs "clean up a navigation API method tracker", whose step 3.3 asserts:

Assert: navigation's upcoming traverse API method trackers[key] exists.

...which is now false, because the first rejection removed it. So this can replace one assertion failure with another. The new step in "fire a traverse navigate event" is guarded by a map exists check; steps 12.2 and 12.3 are not. TraverseCancelled() is idempotent by construction — it returns early when the key is absent from the map.

Descendant navigables aren't aborted

Step 12.7.2 of "apply the history step" ignores the return value of "fire a traverse navigate event", so the new return false has no effect there. Since the reported case is an iframe, the traversal is not aborted for it; instead the new step 6.1 of "update document for history step application" fires. By that point step 8.2 of "apply the history step" has changed the navigable's current session history entry and step 14.11.2 has activated the pruned target entry, and steps 3 and 4 of "update document for history step application" have already updated history.length and history.index. What is skipped is document's latest entry, the navigation API entry update, currententrychange, popstate, and hashchange. That leaves the navigable, the Document, the History object, and the navigation API mutually inconsistent.

Chromium returns CommitResult::Aborted before CommitSameDocumentNavigationInternal(), so none of that state changes. In Chrome Canary, running the issue's iframe repro, no traverse navigate event appears to fire and navigation.entries() ends up as [initial, #1, #pushed] with index 2, i.e. exactly as if only the pushState() had happened.

Removing the assertion is weaker than Chromium

NavigationApi::UpdateForNavigation() keeps CHECK(keys_to_indices_.Contains(item.GetNavigationApiKey())) for back/forward, relying on DispatchNavigateEvent() to have aborted first. Once step 6.1 of "update document for history step application" is in place, steps 4.1–4.3 of "update the navigation API entries for a same-document navigation" are unreachable, so the assertion could be kept as-is.

Incidentally, the dispose-loop step is dead in the "replace" branch: the disposed entry is the current one, and "perform a navigation API traversal" returns early for the current key, so no tracker can exist for it.

Suggested shape

Keep the bail in "fire a traverse navigate event", keep the assertion, drop the two dispose-time rejections and the early return in "update document for history step application", and instead honor the false return at step 12.7.2 of "apply the history step", before activation and document updating. That needs a signal distinct from "canceled-by-navigate", since the traverse navigate event is deliberately non-cancelable in child navigables.

Tests

web-platform-tests/wpt#62214 is the existing forward-to-pruned-entry.html with forward() swapped for traverseTo(). It would be worth also covering the iframe case from #12574, which is where the current PR and Chromium diverge, and the point at which the promises settle relative to pushState().

(Only Chrome Canary was tested here.)

@theIDinside

theIDinside commented Sep 1, 2026

Copy link
Copy Markdown

I think we can go with the suggestions of Claude here and potentially follow up with the dispose-time rejection, (as an additional hardening), that could come with its own test(s). By just having the fire navigate event abortion we would keep the same scope of change as with #12574 too.

So the suggestion:

Keep the bail in "fire a traverse navigate event", keep the assertion, drop the two dispose-time rejections and the early return in "update document for history step application", and instead honor the false return at step 12.7.2 of "apply the history step", before activation and document updating. That needs a signal distinct from "canceled-by-navigate", since the traverse navigate event is deliberately non-cancelable in child navigables.

is fine to me. The 12.7.2 steps it is referring to may need to be re-written somehow. Because, should we really be firing navigate events in other frames if we are to ultimately fail anyhow?

Wild speculation: Maybe we could somehow track that "this was the navigable that started the history traversal, and if that fails, bail"

@noamr
noamr force-pushed the noamr/history-race branch from b464905 to 57b0b62 Compare September 2, 2026 12:24
@noamr noamr closed this Sep 2, 2026
@noamr noamr reopened this Sep 2, 2026
@shannonbooth

Copy link
Copy Markdown
Member

since 12.7.2 is mentioned - it's actually unreachable in the current specification, see: #12859

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Navigation API: Perform navigation & Update URL and history (via replaceState/pushState) race?

4 participants