fix(firefox): treat navigationCommitted events without navigationId as same-document URL updates#41224
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| await context1.pages()[0].goto('about:blank'); | ||
| await context1.close(); | ||
|
|
||
| // On the second launch with an existing profile, Firefox session restore would race with the user's goto and cause "interrupted by another navigation". |
There was a problem hiding this comment.
won't goto throw in case of interrupted navigation? I'd try to run this test with --repeat-each 100 or something to check
There was a problem hiding this comment.
hmm yeah this does fail if i run this test repeatedly in firefox 🤔
lemme dig a bit deeper
navigationCommitted events without navigationIdnavigationCommitted events without navigationId as a same-document URL update
navigationCommitted events without navigationId as a same-document URL updatenavigationCommitted events without navigationId as same-document URL updates
| await server.waitForRequest(route); | ||
|
|
||
| // Simulate a process swap where the new PageAgent is re-initialized using an existing frame and dispatches navigationCommitted without a navigationId. | ||
| delegate._onNavigationCommitted({ |
There was a problem hiding this comment.
This is too much, we should make the browser trigger this call if it may happen in practice.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
…d` as same-document URL updates Firefox juggler replays the current frame state by emitting `Page.navigationCommitted` with no `navigationId` after a process swap (e.g. while restoring a persistent profile) in Windows these replayed events can arrive after `Page.ready`, racing with a user `goto` and causing `Frame.gotoImpl` to throw `"interrupted by another navigation"` treat these as same-document URL updates so they don't interrupt any in-flight new-document navigation in `Frame.gotoImpl`
Test results for "MCP"7279 passed, 1119 skipped Merge workflow run. |
Test results for "tests 1"2 flaky39507 passed, 771 skipped Merge workflow run. |
|
Heads up: this PR regressed the
Bisection (
|
| Run (Jun 10) | macos-15-large (firefox) |
|---|---|
| 17:09 (before this PR) | ✅ success |
| 15:52 (before this PR) | ✅ success |
| 18:12 (this PR's merge) | ❌ failure |
| every run since | ❌ failure |
#41309 only added it.slow(), which doesn't help — should restore state still times out at the full 90s, so it's a genuine hang, not slowness.
Mechanism
From the blob report, the hung steps are launchPersistentContext (3 tests) and a goto-through-proxy (1 test) — all waiting on a navigation that never resolves.
gotoImpl waits for a new-document navigation event whose documentId matches the navigationId returned by navigate. On macOS-15, Firefox sometimes commits the navigation with no navigationId (the id is lost across the process swap during persistent-profile restore / proxied loads).
- Before this PR: an id-less commit became a new-document event with
documentId=''. It matched the predicate via!event.error, then mismatched the id and threw"interrupted by another navigation"([Bug]: Navigation interrupted by another navigation when launching from persistent context for second time #41216). Annoying, but fast-fail. - After this PR: id-less commits are routed to
frameCommittedSameDocumentNavigation, which fires an event withoutnewDocument. The new-document predicate never matches →gotoImplhangs forever → 30s/90s timeout.
So the change traded the rare "interrupted by another navigation" fast-fail for a hard hang that blankets the macOS-15 Firefox suite.
Note
The macOS-15 Firefox job isn't in the standard PR matrix (it runs in tests 2), which is why this slipped through.
The clean fix is juggler-side (preserve navigationId across the process swap). For an immediate unblock, reverting the ffPage.ts change here restores green and reverts #41216 to its rarer fast-fail form.
limit when `Page.navigationCommitted` should be treated as a `Page.sameDocumentNavigation` to only if the `url` and `frameId` match a known active navigation without these, we end up rerouting more than intended to handle the original process-swap scenario
…igationId` as same-document URL updates (microsoft#41224)" This is a partial revert for commits 81881e2 and c2ac911. Keep the test added as that's still a valuable scenario to check, though it's skipped for Firefox due to the known issue.
Firefox juggler replays the current frame state by emitting
Page.navigationCommittedwith nonavigationIdafter a process swap (e.g. while restoring a persistent profile)in Windows these replayed events can arrive after
Page.ready, racing with a usergotoand causingFrame.gotoImplto throw"interrupted by another navigation"treat these as same-document URL updates so they don't interrupt any in-flight new-document navigation in
Frame.gotoImplfixes #41216