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") + ); }