From 72db48cbdf30575680a4036e812372710f7dc1d0 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Fri, 4 Sep 2026 14:41:49 +0800 Subject: [PATCH] fix(history): count child pushes in joint session history --- moli-core/tests/history_child.rs | 6 +- .../history_runtime/length.rs | 48 +++++---------- .../navigation_cross_document.rs | 1 + .../navigation_mutation/local.rs | 5 +- .../navigation_projection.rs | 35 ++++------- .../navigation_traversal_execution.rs | 1 + .../context_host/child_documents/loads.rs | 2 + .../child_frame_navigation/commands.rs | 11 ++++ .../child_frame_navigation/commit.rs | 25 ++++++-- .../context_host/child_frames/lookup.rs | 6 +- .../tests/dom_elements/dom_surface.rs | 58 +++++++++++++++++++ 11 files changed, 128 insertions(+), 70 deletions(-) diff --git a/moli-core/tests/history_child.rs b/moli-core/tests/history_child.rs index 82b97d9d49..1269670ecf 100644 --- a/moli-core/tests/history_child.rs +++ b/moli-core/tests/history_child.rs @@ -306,7 +306,7 @@ async fn assert_child_location_navigation_stays_window_local( page.serialize_html_async() .await .unwrap() - .contains("data-top-history-unchanged=\"false\""), + .contains("data-top-history-unchanged=\"true\""), "{}", page.serialize_html_async().await.unwrap() ); @@ -314,13 +314,13 @@ async fn assert_child_location_navigation_stays_window_local( page.serialize_html_async() .await .unwrap() - .contains("data-child-history-advanced=\"true\"") + .contains("data-child-history-advanced=\"false\"") ); assert!( page.serialize_html_async() .await .unwrap() - .contains("data-child-current-entry-index=\"1\""), + .contains("data-child-current-entry-index=\"0\""), "{}", page.serialize_html_async().await.unwrap() ); diff --git a/moli-renderer-v8/src/context_bootstrap/history_runtime/length.rs b/moli-renderer-v8/src/context_bootstrap/history_runtime/length.rs index 707adab0de..1f97823588 100644 --- a/moli-renderer-v8/src/context_bootstrap/history_runtime/length.rs +++ b/moli-renderer-v8/src/context_bootstrap/history_runtime/length.rs @@ -8,38 +8,22 @@ pub(crate) fn increment_top_level_history_length_for_runtime_owner<'s>( scope: &mut v8::PinScope<'s, '_>, owner: v8::Local<'s, v8::Object>, ) { - if !runtime_window_uses_top_level_history_model(scope, owner) - && let Some(child_history) = window_history_for_holder(scope, owner) - && let Some(child_length) = history_length_number(scope, child_history) - { - set_top_level_history_length_at_least_for_runtime_owner( - scope, - owner, - child_length.max(0.0), - ); + let child_history = (!runtime_window_uses_top_level_history_model(scope, owner)) + .then(|| window_history_for_holder(scope, owner)) + .flatten(); + let top_window = runtime_top_window_owner(scope, owner); + let Some(history) = window_history_for_holder(scope, top_window) else { return; + }; + let current_length = history_length_number(scope, history) + .unwrap_or(0.0) + .max(0.0); + let next_length = current_length + 1.0; + set_history_length(scope, history, next_length); + if let Some(child_history) = child_history { + let child_length = history_length_number(scope, child_history) + .unwrap_or(0.0) + .max(0.0); + set_history_length(scope, child_history, child_length.max(next_length)); } - let top_window = runtime_top_window_owner(scope, owner); - let Some(history) = window_history_for_holder(scope, top_window) else { - return; - }; - let current_length = history_length_number(scope, history) - .unwrap_or(0.0) - .max(0.0); - set_history_length(scope, history, current_length + 1.0); -} - -pub(crate) fn set_top_level_history_length_at_least_for_runtime_owner<'s>( - scope: &mut v8::PinScope<'s, '_>, - owner: v8::Local<'s, v8::Object>, - length: f64, -) { - let top_window = runtime_top_window_owner(scope, owner); - let Some(history) = window_history_for_holder(scope, top_window) else { - return; - }; - let current_length = history_length_number(scope, history) - .unwrap_or(0.0) - .max(0.0); - set_history_length(scope, history, current_length.max(length.max(0.0))); } diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_cross_document.rs b/moli-renderer-v8/src/context_bootstrap/navigation_cross_document.rs index fa0787e0e1..61f09550c7 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_cross_document.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_cross_document.rs @@ -113,6 +113,7 @@ pub(super) fn handle_navigation_navigate_cross_document<'s>( child_handle, next_url.as_str(), entry_seed, + matches!(mutation, NavigationHistoryMutation::Push), ); host.sync_existing_child_browsing_context_window_state(scope, child_handle); navigation_signal diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_mutation/local.rs b/moli-renderer-v8/src/context_bootstrap/navigation_mutation/local.rs index 91c8ddd0c4..c8cfbf2af3 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_mutation/local.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_mutation/local.rs @@ -44,7 +44,10 @@ pub(crate) fn apply_local_window_location_navigation<'s>( let _ = next_entries.set_index(scope, next_index, next_entry.into()); set_history_entries(scope, history, next_entries); set_history_index(scope, history, next_index); - set_history_length_at_least_visible_entries(scope, history, next_entries); + // Cross-document pushes do not enter the joint session history + // until the new Document commits. Keep the pending child's local + // projection current without advancing the traversable yet. + set_history_length_from_visible_entries(scope, history, next_entries); set_history_state(scope, history, state); set_navigation_current_entry(scope, navigation, next_entry); dispatch_navigation_currententrychange(scope, navigation, previous_entry, Some("push")); diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_projection.rs b/moli-renderer-v8/src/context_bootstrap/navigation_projection.rs index 1598fe0684..592d934bca 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_projection.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_projection.rs @@ -227,7 +227,6 @@ pub(super) fn set_history_length_from_visible_entries<'s>( ) { let length = history_length_floor_from_visible_entries(scope, history, entries); set_history_length(scope, history, length); - set_top_history_length_at_least(scope, history, length); } pub(super) fn set_history_length_at_least_visible_entries<'s>( @@ -239,9 +238,18 @@ pub(super) fn set_history_length_at_least_visible_entries<'s>( let current_length = history_length_number(scope, history) .unwrap_or(0.0) .max(0.0); - let length = current_length.max(length); - set_history_length(scope, history, length); - set_top_history_length_at_least(scope, history, length); + let owner = runtime_window_owner(scope, history); + if runtime_window_uses_top_level_history_model(scope, owner) { + set_history_length(scope, history, current_length.max(length)); + return; + } + + // Same-document child pushes commit synchronously. They add one entry to + // the traversable's joint session history even when another child has + // already made the top-level length larger than this child's local list. + super::increment_top_level_history_length_for_runtime_owner(scope, owner); + let joint_length = history_length_floor_from_visible_entries(scope, history, entries); + set_history_length(scope, history, current_length.max(length).max(joint_length)); } fn history_length_floor_from_visible_entries<'s>( @@ -270,22 +278,3 @@ fn history_length_floor_from_visible_entries<'s>( .max(0.0); visible_length.max(top_length) } - -fn set_top_history_length_at_least<'s>( - scope: &mut v8::PinScope<'s, '_>, - history: v8::Local<'s, v8::Object>, - length: f64, -) { - let owner = runtime_window_owner(scope, history); - if runtime_window_uses_top_level_history_model(scope, owner) { - return; - } - let top_window = runtime_top_window_owner(scope, owner); - let Some(top_history) = window_history_for_holder(scope, top_window) else { - return; - }; - let current_length = history_length_number(scope, top_history) - .unwrap_or(0.0) - .max(0.0); - set_history_length(scope, top_history, current_length.max(length)); -} diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_traversal_execution.rs b/moli-renderer-v8/src/context_bootstrap/navigation_traversal_execution.rs index b9f2cb40aa..db17dd07f2 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_traversal_execution.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_traversal_execution.rs @@ -532,5 +532,6 @@ fn queue_child_cross_document_traversal( child_handle, target_url, seed, + false, ); } diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_documents/loads.rs b/moli-renderer-v8/src/native_bridge/context_host/child_documents/loads.rs index 4356f0ea4f..ee15ddfc13 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_documents/loads.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_documents/loads.rs @@ -640,6 +640,7 @@ impl JsContextHost { }; entry.clear_cached_snapshot(); entry.clear_completed_document_network(); + entry.clear_pending_top_level_history_length_increment(); self.reject_replaced_service_worker_child_client_navigation( handle, format!("Cannot navigate to URL: {error}"), @@ -694,6 +695,7 @@ impl JsContextHost { body_activity, }; }; + self.commit_pending_child_joint_history_push(scope, handle); initial_classic_ready_work = install.initial_classic_ready_work; parser_stop_action = install.parser_stop_action; owner_transition = Some(install.owner_transition); diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_frame_navigation/commands.rs b/moli-renderer-v8/src/native_bridge/context_host/child_frame_navigation/commands.rs index a643315503..0e022b78d9 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_frame_navigation/commands.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_frame_navigation/commands.rs @@ -29,6 +29,7 @@ impl JsContextHost { entry.replace_navigation_in_entry_seed(&url); } else { entry.apply_navigation_to_entry_seed(&url); + entry.mark_pending_top_level_history_length_increment(); } } self.sync_existing_child_browsing_context_runtime_surface_from_seed(scope, handle); @@ -51,6 +52,7 @@ impl JsContextHost { entry.replace_navigation_in_entry_seed(&request.url); } else { entry.apply_navigation_to_entry_seed(&request.url); + entry.mark_pending_top_level_history_length_increment(); } } self.sync_existing_child_browsing_context_runtime_surface_from_seed(scope, handle); @@ -75,6 +77,9 @@ impl JsContextHost { ); if let Some(entry) = self.child_browsing_contexts.get_mut(&handle) { entry.apply_queued_navigation_to_entry_seed(&url, replace_current); + if !replace_current { + entry.mark_pending_top_level_history_length_increment(); + } } self.queue_child_browsing_context_navigation_to_url(handle, &url) } @@ -98,6 +103,7 @@ impl JsContextHost { handle: DomHandle, resolved_url: &str, entry_seed: NavigationHistoryEntrySeed, + increments_joint_history: bool, ) -> bool { if !self.child_browsing_contexts.contains_key(&handle) { return false; @@ -107,6 +113,9 @@ impl JsContextHost { }; if let Some(entry) = self.child_browsing_contexts.get_mut(&handle) { entry.replace_navigation_entry_seed_and_clear_pending_history_increment(entry_seed); + if increments_joint_history { + entry.mark_pending_top_level_history_length_increment(); + } } if self .set_child_browsing_context_pending_navigation( @@ -134,6 +143,7 @@ impl JsContextHost { }; if let Some(entry) = self.child_browsing_contexts.get_mut(&handle) { entry.apply_deferred_navigation_to_entry_seed(&url); + entry.mark_pending_top_level_history_length_increment(); } if self .set_child_browsing_context_pending_navigation( @@ -162,6 +172,7 @@ impl JsContextHost { ); if let Some(entry) = self.child_browsing_contexts.get_mut(&handle) { entry.apply_deferred_navigation_to_entry_seed(&request.url); + entry.mark_pending_top_level_history_length_increment(); } if self .set_child_browsing_context_pending_navigation( diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_frame_navigation/commit.rs b/moli-renderer-v8/src/native_bridge/context_host/child_frame_navigation/commit.rs index 1703ad3995..62768740a8 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_frame_navigation/commit.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_frame_navigation/commit.rs @@ -416,10 +416,6 @@ impl JsContextHost { self.sync_existing_child_browsing_context_window_state(scope, handle); return None; } - let increment_top_level_history_length = self - .child_browsing_contexts - .get_mut(&handle) - .is_some_and(|entry| entry.take_pending_top_level_history_length_increment()); self.clear_child_browsing_context_pending_navigation(handle); self.clear_pending_form_submission_child_target(handle); let commit_result = self.commit_child_document_bootstrap_or_start_load( @@ -429,13 +425,30 @@ impl JsContextHost { navigation_load, ChildDocumentNavigationInitiator::BrowsingContext, ); + if commit_result + .as_ref() + .is_some_and(|result| result.state == ChildDocumentCommitState::Ready) + { + self.commit_pending_child_joint_history_push(scope, handle); + } self.sync_existing_child_browsing_context_window_state(scope, handle); - if increment_top_level_history_length + commit_result + } + + pub(in crate::native_bridge::context_host) fn commit_pending_child_joint_history_push( + &mut self, + scope: &mut v8::PinScope<'_, '_>, + handle: DomHandle, + ) { + let increments_joint_history = self + .child_browsing_contexts + .get_mut(&handle) + .is_some_and(|entry| entry.take_pending_top_level_history_length_increment()); + if increments_joint_history && let Some(window) = self.child_browsing_context_window_wrapper(scope, handle) { increment_top_level_history_length_for_runtime_owner(scope, window); } - commit_result } pub(crate) fn queue_child_browsing_context_javascript_url_execution( diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_frames/lookup.rs b/moli-renderer-v8/src/native_bridge/context_host/child_frames/lookup.rs index 9800a81d21..2458c263b7 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_frames/lookup.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_frames/lookup.rs @@ -22,11 +22,7 @@ impl JsContextHost { &self, handle: DomHandle, ) -> bool { - let seed_is_initial_about_blank = self - .child_browsing_contexts - .get(&handle) - .is_some_and(ChildBrowsingContextEntry::navigation_seed_is_initial_about_blank_commit); - seed_is_initial_about_blank + self.child_current_document_is_initial_empty(handle) && self .child_browsing_context_current_url(handle) .is_some_and(|url| moli_url::is_about_blank(&url)) diff --git a/moli-renderer-v8/src/script_vm/tests/dom_elements/dom_surface.rs b/moli-renderer-v8/src/script_vm/tests/dom_elements/dom_surface.rs index 418d8a7b1f..94aafd70bd 100644 --- a/moli-renderer-v8/src/script_vm/tests/dom_elements/dom_surface.rs +++ b/moli-renderer-v8/src/script_vm/tests/dom_elements/dom_surface.rs @@ -11830,6 +11830,64 @@ fn no_src_iframe_initial_about_blank_has_a_quirks_empty_document() { ); } +#[test] +fn child_joint_history_pushes_accumulate_across_distinct_frames() { + let mut vm = new_storage_test_vm("https://joint-child-length.test/page.html"); + + vm.exec( + r#" +const first = document.createElement('iframe'); +first.srcdoc = '

first

'; +(document.body || document.documentElement || document).appendChild(first); +globalThis.__firstJointLengthFrame = first; +"#, + None, + ) + .expect("first child setup should evaluate"); + vm.drain_pending_child_frame_work_for_test(); + assert_eq!( + vm.eval( + r#" +(() => { + const child = __firstJointLengthFrame.contentWindow; + const before = history.length; + child.history.pushState(null, '', '#first'); + return [before, history.length, child.history.length].join('|'); +})() +"#, + ) + .expect("first child history push should evaluate"), + "1|2|2" + ); + + vm.exec( + r#" +__firstJointLengthFrame.remove(); +const second = document.createElement('iframe'); +second.srcdoc = '

second

'; +(document.body || document.documentElement || document).appendChild(second); +globalThis.__secondJointLengthFrame = second; +"#, + None, + ) + .expect("second child setup should evaluate"); + vm.drain_pending_child_frame_work_for_test(); + assert_eq!( + vm.eval( + r#" +(() => { + const child = __secondJointLengthFrame.contentWindow; + const before = history.length; + child.history.pushState(null, '', '#second'); + return [before, history.length, child.history.length].join('|'); +})() +"#, + ) + .expect("second child history push should evaluate"), + "2|3|3" + ); +} + #[tokio::test] async fn top_history_back_routes_to_child_joint_history_entry() { let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).expect("loader");