From b7bcb21292df4e02084f351ab01f5c3ef94b103a Mon Sep 17 00:00:00 2001 From: ldm0 Date: Thu, 24 Sep 2026 19:18:25 +0800 Subject: [PATCH] fix(history): reuse navigation traversals only for the same key A later traverseTo() with a different destination could overwrite an older Navigation task ahead of an intervening History task, leaving the page at the wrong final entry. Match both the Window target and destination key in both reuse paths. Keep the original traversal destination, info, and promises when sharing a pending request. Add a local mixed-API regression checking event order, promise targets, and the final URL. Cover repeated requests after a History request and preservation of the first request's info and single traversal task. Fixes: 51c9e1df6 (fix(history): preserve each queued History API traversal) --- .../src/native_bridge/history_queue.rs | 45 +++---- .../script_vm/tests/browser_api/navigation.rs | 125 +++++++++++++++++- 2 files changed, 143 insertions(+), 27 deletions(-) diff --git a/moli-renderer-v8/src/native_bridge/history_queue.rs b/moli-renderer-v8/src/native_bridge/history_queue.rs index 131a31a94e..9f62f45133 100644 --- a/moli-renderer-v8/src/native_bridge/history_queue.rs +++ b/moli-renderer-v8/src/native_bridge/history_queue.rs @@ -141,14 +141,18 @@ impl HistoryQueueState { ) -> Option { // History API requests are ordered steps, including a same-document // traversal followed by a cross-document traversal. Only Navigation - // API requests with result promises can share a pending task. + // API requests for the same destination key can share a pending task; + // a different destination must retain its own position in the queue. if result.is_some() + && target_key.is_some() && let Some(pending) = self.pending_history_traversal_tasks .iter_mut() .find_map(|queued| match &mut queued.action { PendingHistoryTraversalAction::SameDocument(pending) - if pending.target == target && !pending.results.is_empty() => + if pending.target == target + && pending.target_key == target_key + && !pending.results.is_empty() => { Some(pending) } @@ -156,10 +160,6 @@ impl HistoryQueueState { | PendingHistoryTraversalAction::ChildCrossDocument(_) => None, }) { - pending.target_index = target_index; - pending.joint_step = joint_step; - pending.target_key = target_key; - pending.info = info; if let Some(result) = result { pending.results.push(result); } @@ -536,26 +536,23 @@ impl JsContextHost { Option, )> { let execution_context = self.current_runtime_window_execution_context_identity(scope)?; - if let Some(existing_index) = self - .history_queue - .pending_history_traversal_tasks - .iter() - .position(|queued| { - matches!( - &queued.action, + if target_key.is_some() + && let Some(result) = self + .history_queue + .pending_history_traversal_tasks + .iter() + .find_map(|queued| match &queued.action { PendingHistoryTraversalAction::SameDocument(pending) - if pending.target == target - && pending.target_index == target_index - ) - }) - && let PendingHistoryTraversalAction::SameDocument(pending) = - &mut self.history_queue.pending_history_traversal_tasks[existing_index].action - && !pending.results.is_empty() + if pending.target == target && pending.target_key == target_key => + { + pending.results.first() + } + PendingHistoryTraversalAction::SameDocument(_) + | PendingHistoryTraversalAction::ChildCrossDocument(_) => None, + }) { - pending.joint_step = joint_step; - pending.target_key = target_key; - pending.info = info.map(|info| v8::Global::new(scope, info)); - let result = &pending.results[0]; + // Repeated requests share the original tracker, including its + // destination and info, even if a History task also targets it. let committed_resolver = v8::Local::new(scope, &result.committed_resolver); let finished_resolver = v8::Local::new(scope, &result.finished_resolver); return Some(( diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/navigation.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/navigation.rs index 260e539c0f..1b08b017e1 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/navigation.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/navigation.rs @@ -4689,6 +4689,117 @@ async fn traversal_navigate_destination_index_tracks_entry_identity() { r#"{"committed":"InvalidStateError","finished":"InvalidStateError","all":"done"}"# ); } +#[tokio::test] +async fn traverse_to_preserves_intervening_history_back() { + let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).expect("loader"); + let mut vm = + new_storage_page_task_executor_test_vm_with_loader("https://example.com/base", &loader); + + let setup = vm + .eval( + r##" + (() => { + history.replaceState(null, "", "#0"); + const keys = [navigation.currentEntry.key]; + for (let i = 1; i <= 3; i++) { + history.pushState(null, "", `#${i}`); + keys.push(navigation.currentEntry.key); + } + const probe = globalThis.__lmMixedTraversals = { + popstate: [], committed: [], finished: [] + }; + onpopstate = () => probe.popstate.push(location.hash); + const observe = (label, result) => { + result.committed.then( + entry => probe.committed.push(`${label}:${new URL(entry.url).hash}:${location.hash}`), + error => probe.committed.push(`${label}:rejected:${error.name}`) + ); + result.finished.then( + entry => probe.finished.push(`${label}:${new URL(entry.url).hash}`), + error => probe.finished.push(`${label}:rejected:${error.name}`) + ); + }; + const first = navigation.traverseTo(keys[2]); + observe("first", first); + history.back(); + const last = navigation.traverseTo(keys[0]); + observe("last", last); + return [location.hash, first.committed !== last.committed, + first.finished !== last.finished].join("|"); + })() + "##, + ) + .expect("mixed traversal requests should queue in one script turn"); + assert_eq!(setup, "#3|true|true"); + + let mut executed = Vec::new(); + for _ in 0..4 { + executed.push( + vm.run_one_history_traversal_executor_turn(&loader) + .await + .expect("mixed history traversal should execute"), + ); + } + let settled = vm + .eval("JSON.stringify({hash: location.hash, ...__lmMixedTraversals})") + .expect("mixed traversal results should be inspectable"); + assert_eq!( + settled, + r##"{"hash":"#0","popstate":["#2","#1","#0"],"committed":["first:#2:#2","last:#0:#0"],"finished":["first:#2","last:#0"]}"## + ); + assert_eq!(executed, [true, true, true, false]); +} + +#[tokio::test] +async fn repeated_traverse_to_reuses_promises_after_history_request() { + let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).expect("loader"); + let mut vm = + new_storage_page_task_executor_test_vm_with_loader("https://example.com/base", &loader); + + let setup = vm + .eval( + r##" + (() => { + history.pushState(null, "", "#1"); + const key = navigation.currentEntry.key; + history.pushState(null, "", "#2"); + history.back(); + const first = navigation.traverseTo(key); + const second = navigation.traverseTo(key); + globalThis.__lmRepeatedAfterHistory = []; + for (const [label, result] of [["first", first], ["second", second]]) { + result.finished.then( + entry => __lmRepeatedAfterHistory.push(`${label}:${new URL(entry.url).hash}`), + error => __lmRepeatedAfterHistory.push(`${label}:rejected:${error.name}`) + ); + } + return [first !== second, first.committed === second.committed, + first.finished === second.finished].join("|"); + })() + "##, + ) + .expect("a History request should not hide a matching Navigation request"); + assert_eq!(setup, "true|true|true"); + + for _ in 0..2 { + assert!( + vm.run_one_history_traversal_executor_turn(&loader) + .await + .expect("History and Navigation requests should execute separately") + ); + } + assert_eq!( + vm.eval("[location.hash, ...__lmRepeatedAfterHistory].join('|')") + .expect("both callers should finish at their shared destination"), + "#1|first:#1|second:#1" + ); + assert!( + !vm.run_one_history_traversal_executor_turn(&loader) + .await + .expect("the repeated Navigation request should not add another task") + ); +} + #[tokio::test] async fn repeated_traverse_to_reuses_pending_navigation_promises() { let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).expect("loader"); @@ -4701,9 +4812,12 @@ async fn repeated_traverse_to_reuses_pending_navigation_promises() { (() => { const key = navigation.currentEntry.key; navigation.navigate("#one"); - const first = navigation.traverseTo(key); - const second = navigation.traverseTo(key); + const first = navigation.traverseTo(key, { info: "first" }); + const second = navigation.traverseTo(key, { info: "second" }); globalThis.__lmRepeatedTraverseTo = { first, second, log: [] }; + navigation.addEventListener("navigate", event => { + __lmRepeatedTraverseTo.log.push(`info:${event.info}`); + }, { once: true }); first.finished.then( entry => globalThis.__lmRepeatedTraverseTo.log.push(`finished:${entry.url}:${location.hash}`), error => globalThis.__lmRepeatedTraverseTo.log.push(`rejected:${error.name}`) @@ -4727,5 +4841,10 @@ async fn repeated_traverse_to_reuses_pending_navigation_promises() { let settled = vm .eval("globalThis.__lmRepeatedTraverseTo.log.join('|')") .expect("repeated traverseTo settlement should evaluate"); - assert_eq!(settled, "finished:https://example.com/base:"); + assert_eq!(settled, "info:first|finished:https://example.com/base:"); + assert!( + !vm.run_one_history_traversal_executor_turn(&loader) + .await + .expect("repeated traverseTo should share one traversal task") + ); }