From bf236b174ebc9713eb2cb78cd425122f97245f02 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Thu, 24 Sep 2026 06:03:31 +0800 Subject: [PATCH] fix(navigation): preserve successor windows during retirement --- .../src/document_runtime/mutation_commands.rs | 5 +- .../tree/context_followups.rs | 16 +- .../tree/custom_element_lifecycle.rs | 5 +- .../context_host/child_documents/lifecycle.rs | 30 ++- .../child_frame_runtime/document.rs | 213 ++++++++++-------- .../child_frame_runtime/window.rs | 31 --- .../context_host/child_frames.rs | 1 + .../context_host/child_frames/lookup.rs | 1 + .../context_host/child_frames/registry.rs | 138 ++++++++++-- .../src/native_bridge/context_host/core.rs | 23 +- .../src/native_bridge/document/lifecycle.rs | 41 ++-- .../src/script_vm/document_content.rs | 6 +- .../script_vm/tests/browser_api/navigation.rs | 110 +++++++++ 13 files changed, 428 insertions(+), 192 deletions(-) diff --git a/moli-renderer-v8/src/document_runtime/mutation_commands.rs b/moli-renderer-v8/src/document_runtime/mutation_commands.rs index 828f1c1d7d..fb07b7ff08 100644 --- a/moli-renderer-v8/src/document_runtime/mutation_commands.rs +++ b/moli-renderer-v8/src/document_runtime/mutation_commands.rs @@ -325,8 +325,9 @@ impl DocumentRuntime { ); } } - unsafe { &mut *host_ptr } - .drop_child_browsing_context_subtree_with_window_realm(scope, root); + JsContextHost::drop_child_browsing_context_subtree_with_window_realm( + scope, host_ptr, root, + ); } } record_dom_binding_timing("dom.setTextContent", started); diff --git a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/context_followups.rs b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/context_followups.rs index 2e653ac11b..0d7cf3941a 100644 --- a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/context_followups.rs +++ b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/context_followups.rs @@ -11,16 +11,17 @@ impl DocumentRuntime { host_ptr: *mut JsContextHost, insertion_plan: &TreeInsertionPlan<'_>, ) { - let runtime = unsafe { &mut *host_ptr }; if insertion_plan.adoption.crosses_documents() { for &root in insertion_plan.insertion_roots { - runtime.migrate_inline_style_metadata_in_subtree(root); + unsafe { &mut *host_ptr }.migrate_inline_style_metadata_in_subtree(root); } } for &root in insertion_plan.insertion_roots { - runtime.clear_disconnected_shadow_roots_in_subtree(root); - runtime.drop_child_browsing_contexts_moved_into_own_document_subtree(scope, root); - runtime.sync_child_browsing_context_subtree(scope, root); + unsafe { &mut *host_ptr }.clear_disconnected_shadow_roots_in_subtree(root); + JsContextHost::drop_child_browsing_contexts_moved_into_own_document_subtree( + scope, host_ptr, root, + ); + unsafe { &mut *host_ptr }.sync_child_browsing_context_subtree(scope, root); } } @@ -30,9 +31,10 @@ impl DocumentRuntime { host_ptr: *mut JsContextHost, roots: &[DomHandle], ) { - let runtime = unsafe { &mut *host_ptr }; for &root in roots { - runtime.drop_child_browsing_context_subtree_with_window_realm(scope, root); + JsContextHost::drop_child_browsing_context_subtree_with_window_realm( + scope, host_ptr, root, + ); } } } diff --git a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/custom_element_lifecycle.rs b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/custom_element_lifecycle.rs index 234424128f..026ae15d82 100644 --- a/moli-renderer-v8/src/document_runtime/mutation_commands/tree/custom_element_lifecycle.rs +++ b/moli-renderer-v8/src/document_runtime/mutation_commands/tree/custom_element_lifecycle.rs @@ -81,8 +81,9 @@ impl DocumentRuntime { scope, host_ptr, handle, ); } - unsafe { &mut *host_ptr } - .drop_child_browsing_context_subtree_with_window_realm(scope, root); + JsContextHost::drop_child_browsing_context_subtree_with_window_realm( + scope, host_ptr, root, + ); } } diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_documents/lifecycle.rs b/moli-renderer-v8/src/native_bridge/context_host/child_documents/lifecycle.rs index 07f85fedcf..f036c38a68 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_documents/lifecycle.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_documents/lifecycle.rs @@ -875,18 +875,21 @@ impl JsContextHost { /// dispatch pagehide/visibilitychange before unload while the parent /// document's listeners are still installed. fn dispatch_child_browsing_context_document_open_unload_lifecycle_if_needed( - &mut self, scope: &mut v8::PinScope<'_, '_>, + host_ptr: *mut Self, handle: DomHandle, ) { - let Some(window) = self.existing_child_browsing_context_window_wrapper(scope, handle) + let Some(window) = + unsafe { &mut *host_ptr }.existing_child_browsing_context_window_wrapper(scope, handle) else { return; }; - let Some(document) = self.child_browsing_context_document_wrapper(scope, handle) else { + let Some(document) = + unsafe { &mut *host_ptr }.child_browsing_context_document_wrapper(scope, handle) + else { return; }; - let Some(action) = self + let Some(action) = unsafe { &mut *host_ptr } .frame_owner_store .begin_current_child_document_unload(handle) else { @@ -901,25 +904,28 @@ impl JsContextHost { let _ = call_object_method(scope, document, "dispatchEvent", &[event.into()]); } dispatch_unload_for_runtime_owner(scope, window); - let _ = self + let _ = unsafe { &mut *host_ptr } .frame_owner_store .finish_current_child_document_unload(action); - unsafe { &mut *self.runtime } - .cancel_window_execution_context_timers(execution_context_owner); + unsafe { &mut *host_ptr }.cancel_window_execution_context_timers(execution_context_owner); } pub(crate) fn dispatch_document_open_descendant_frame_unload_lifecycle( - &mut self, scope: &mut v8::PinScope<'_, '_>, + host_ptr: *mut Self, document_handle: DomHandle, ) { - let handles = self.child_browsing_context_handles_in_document_order(); + let handles = unsafe { &*host_ptr }.child_browsing_context_handles_in_document_order(); for handle in handles { - if self.dom_host().owner_document_handle(handle) != Some(document_handle) { + if unsafe { &*host_ptr } + .dom_host() + .owner_document_handle(handle) + != Some(document_handle) + { continue; } - self.dispatch_child_browsing_context_document_open_unload_lifecycle_if_needed( - scope, handle, + Self::dispatch_child_browsing_context_document_open_unload_lifecycle_if_needed( + scope, host_ptr, handle, ); } } diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/document.rs b/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/document.rs index 00ba4db298..b8d3174877 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/document.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/document.rs @@ -539,7 +539,7 @@ fn begin_child_document_stream_replacement<'s>( ) -> Option> { let host_ptr = context_host_ptr_from_global_bridge(scope)?; let document_handle = child_document_native_handle_for_runtime(scope, host_ptr, document)?; - unsafe { &mut *host_ptr }.begin_child_document_stream_replacement( + JsContextHost::begin_child_document_stream_replacement( scope, host_ptr, child_handle, @@ -549,17 +549,17 @@ fn begin_child_document_stream_replacement<'s>( impl JsContextHost { fn begin_child_document_stream_replacement<'s>( - &mut self, scope: &mut v8::PinScope<'s, '_>, host_ptr: *mut JsContextHost, child_handle: DomHandle, document_handle: DomHandle, ) -> Option> { - debug_assert!(std::ptr::eq(host_ptr, self)); - if self.child_browsing_context_document_handle(child_handle) != Some(document_handle) { + if unsafe { &*host_ptr }.child_browsing_context_document_handle(child_handle) + != Some(document_handle) + { return None; } - if self.child_document_stream_is_blocked_by_navigation(child_handle) { + if unsafe { &*host_ptr }.child_document_stream_is_blocked_by_navigation(child_handle) { tracing::debug!( ?child_handle, ?document_handle, @@ -567,7 +567,7 @@ impl JsContextHost { ); return None; } - let script_context = match self + let script_context = match unsafe { &mut *host_ptr } .ensure_prebootstrapped_child_default_context(scope, child_handle) { Ok(context) => context, @@ -581,16 +581,17 @@ impl JsContextHost { return None; } }; - if self.child_document_is_executing_parser_script(document_handle) { + if unsafe { &*host_ptr }.child_document_is_executing_parser_script(document_handle) { return Some(script_context); } - let document_url = self.document_url_for_handle(document_handle); - let document_base_url = self.document_base_url_for_handle(document_handle); + let document_url = unsafe { &*host_ptr }.document_url_for_handle(document_handle); + let document_base_url = unsafe { &*host_ptr }.document_base_url_for_handle(document_handle); // Validate the target owner before any unload callback is observable. // Descendant unload handlers can run arbitrary script, including a // reentrant document.open(), so this admission snapshot must not be // committed after callbacks without being refreshed. - self.frame_owner_store + unsafe { &mut *host_ptr } + .frame_owner_store .plan_child_document_open_replacement( child_handle, document_handle, @@ -598,23 +599,29 @@ impl JsContextHost { document_base_url, )?; - self.dispatch_document_open_descendant_frame_unload_lifecycle(scope, document_handle); - if self + Self::dispatch_document_open_descendant_frame_unload_lifecycle( + scope, + host_ptr, + document_handle, + ); + if unsafe { &*host_ptr } .child_browsing_contexts .get(&child_handle) .is_some_and(|entry| entry.pending_attribute_bootstrap_commit()) { - self.cancel_child_browsing_context_attribute_navigation(child_handle); + unsafe { &mut *host_ptr } + .cancel_child_browsing_context_attribute_navigation(child_handle); } - if self.child_browsing_context_document_handle(child_handle) != Some(document_handle) - || self.child_document_stream_is_blocked_by_navigation(child_handle) + if unsafe { &*host_ptr }.child_browsing_context_document_handle(child_handle) + != Some(document_handle) + || unsafe { &*host_ptr }.child_document_stream_is_blocked_by_navigation(child_handle) { return None; } - let document_url = self.document_url_for_handle(document_handle); - let document_base_url = self.document_base_url_for_handle(document_handle); - let replacement_plan = self + let document_url = unsafe { &*host_ptr }.document_url_for_handle(document_handle); + let document_base_url = unsafe { &*host_ptr }.document_base_url_for_handle(document_handle); + let replacement_plan = unsafe { &mut *host_ptr } .frame_owner_store .plan_child_document_open_replacement( child_handle, @@ -623,101 +630,125 @@ impl JsContextHost { document_base_url.clone(), )?; let retired_owner = replacement_plan.retired_owner(); - let resource_authority = self + let resource_authority = unsafe { &*host_ptr } .document_resource_loader_for_owner(retired_owner) .expect("child document.open() requires its exact committed resource authority") .clone(); - let document_origin = self + let document_origin = unsafe { &*host_ptr } .child_browsing_context_window_origin(child_handle) .expect("child document.open() requires its committed Window origin"); - let children = self + let children = unsafe { &*host_ptr } .dom_host() .child_handles(document_handle) .collect::>(); - crate::custom_elements::with_custom_element_reaction_scope(scope, host_ptr, |scope| { - let host = unsafe { &mut *host_ptr }; - let transition = host - .frame_owner_store - .commit_child_document_open_replacement(replacement_plan); - let current_owner = transition - .current_owner() - .expect("committed child document-open replacement must install an owner"); - host.replace_document_resource_loader_for_document_open( - crate::native_bridge::WindowDocumentOwner::Frame(retired_owner), - crate::network::context::DocumentFetchContext::new( - crate::native_bridge::WindowDocumentOwner::Frame(current_owner), - document_url.clone(), - document_base_url, - document_origin, - ), - crate::network::context::DocumentResourceAuthoritySource::Inherited( - resource_authority, - ), - ); + let current_owner = + crate::custom_elements::with_custom_element_reaction_scope(scope, host_ptr, |scope| { + let host = unsafe { &mut *host_ptr }; + let transition = host + .frame_owner_store + .commit_child_document_open_replacement(replacement_plan); + let current_owner = transition + .current_owner() + .expect("committed child document-open replacement must install an owner"); + host.replace_document_resource_loader_for_document_open( + crate::native_bridge::WindowDocumentOwner::Frame(retired_owner), + crate::network::context::DocumentFetchContext::new( + crate::native_bridge::WindowDocumentOwner::Frame(current_owner), + document_url.clone(), + document_base_url, + document_origin, + ), + crate::network::context::DocumentResourceAuthoritySource::Inherited( + resource_authority, + ), + ); - host.clear_child_window_document_event_state(scope, child_handle); - host.clear_event_callbacks_for_document_replacement(document_handle, false); - for child in children { - let _ = - remove_child_to_current_reaction_queue(scope, host_ptr, document_handle, child); - } + host.clear_child_window_document_event_state(scope, child_handle); + host.clear_event_callbacks_for_document_replacement(document_handle, false); + for child in children { + let _ = remove_child_to_current_reaction_queue( + scope, + host_ptr, + document_handle, + child, + ); + } - host.cancel_child_meta_refresh_navigation(child_handle); - host.cancel_stylesheet_subresource_fetches_for_document_owner(retired_owner); - host.retire_image_state_for_document(document_handle); - host.cancel_pending_media_loads_for_document(document_handle); - host.cancel_pending_text_track_loads_for_document(document_handle); - host.cancel_child_document_script_work_for_owner(child_handle, retired_owner); - host.child_document_parsers - .clear(retired_owner.document_owner()); - host.drop_child_browsing_context_subtree_with_window_realm(scope, document_handle); - if let Some(entry) = host.child_browsing_contexts.get_mut(&child_handle) { - entry.clear_document_runtime_state(); - } - host.request_child_frame_realm_materialization(child_handle); - host.install_empty_child_classic_script_runner_for_current_document( - child_handle, - current_owner.local_window_id, - current_owner.document_id, - ); - host.dom_host_mut() - .mark_subtree_connected_preserving_owner_document(document_handle); - let security_token_refreshed = - host.refresh_child_default_world_security_token(scope, child_handle); - host.install_child_document_write_parser( - child_handle, - current_owner.document_owner(), - document_handle, - document_url, - ); - host.note_child_frame_load_started_for_parent(child_handle); - host.queue_child_frame_document_opened_event(child_handle); - tracing::debug!( - ?child_handle, - ?retired_owner, - ?current_owner, - ?document_handle, - security_token_refreshed, - "opened child document stream through same-LocalWindow owner transaction" - ); - }); - Some(script_context) + if unsafe { &*host_ptr }.current_child_document_task_owner(child_handle) + != Some(current_owner) + { + return None; + } + let host = unsafe { &mut *host_ptr }; + host.cancel_child_meta_refresh_navigation(child_handle); + host.cancel_stylesheet_subresource_fetches_for_document_owner(retired_owner); + host.retire_image_state_for_document(document_handle); + host.cancel_pending_media_loads_for_document(document_handle); + host.cancel_pending_text_track_loads_for_document(document_handle); + host.cancel_child_document_script_work_for_owner(child_handle, retired_owner); + host.child_document_parsers + .clear(retired_owner.document_owner()); + Self::drop_child_browsing_context_subtree_with_window_realm( + scope, + host_ptr, + document_handle, + ); + if unsafe { &*host_ptr }.current_child_document_task_owner(child_handle) + != Some(current_owner) + { + return None; + } + let host = unsafe { &mut *host_ptr }; + if let Some(entry) = host.child_browsing_contexts.get_mut(&child_handle) { + entry.clear_document_runtime_state(); + } + host.request_child_frame_realm_materialization(child_handle); + host.install_empty_child_classic_script_runner_for_current_document( + child_handle, + current_owner.local_window_id, + current_owner.document_id, + ); + host.dom_host_mut() + .mark_subtree_connected_preserving_owner_document(document_handle); + let security_token_refreshed = + host.refresh_child_default_world_security_token(scope, child_handle); + host.install_child_document_write_parser( + child_handle, + current_owner.document_owner(), + document_handle, + document_url, + ); + host.note_child_frame_load_started_for_parent(child_handle); + host.queue_child_frame_document_opened_event(child_handle); + tracing::debug!( + ?child_handle, + ?retired_owner, + ?current_owner, + ?document_handle, + security_token_refreshed, + "opened child document stream through same-LocalWindow owner transaction" + ); + Some(current_owner) + })?; + (unsafe { &*host_ptr }.current_child_document_task_owner(child_handle) + == Some(current_owner)) + .then_some(script_context) } /// Replaces a child frame's current document without invoking page-visible /// `Document.open`, `write`, or `close` properties. pub(crate) fn set_child_browsing_context_document_content( - &mut self, scope: &mut v8::PinScope<'_, '_>, host_ptr: *mut JsContextHost, child_handle: DomHandle, html: &str, ) -> bool { - let Some(document_handle) = self.child_browsing_context_document_handle(child_handle) + let Some(document_handle) = + unsafe { &*host_ptr }.child_browsing_context_document_handle(child_handle) else { return false; }; - let Some(script_context) = self.begin_child_document_stream_replacement( + let Some(script_context) = Self::begin_child_document_stream_replacement( scope, host_ptr, child_handle, @@ -729,7 +760,7 @@ impl JsContextHost { // successfully handed the markup to this child Document. A `false` // pump result can mean that the parser is intentionally parked on a // parser-blocking stylesheet; it is not a missing-Document failure. - let _ = self.pump_child_document_write_parser( + let _ = unsafe { &mut *host_ptr }.pump_child_document_write_parser( scope, script_context, child_handle, diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/window.rs b/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/window.rs index ac231202c4..2976259f5f 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/window.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/window.rs @@ -631,37 +631,6 @@ unsafe extern "C" fn window_access_check_callback( } impl JsContextHost { - pub(in crate::native_bridge::context_host) fn inform_about_canceled_child_navigation_before_detach( - &mut self, - scope: &mut v8::PinScope<'_, '_>, - handle: DomHandle, - ) { - // Pending traversal admission is settled by ScriptVm after this native - // detach finishes. Other navigation cancellation keeps its synchronous - // event/committed-entry semantics. - if let Some(owner) = self.current_child_document_task_owner(handle) { - self.pending_history_traversal_admissions.retire_owner( - super::super::WindowExecutionContextOwner::Frame(owner.local_window_id), - ); - } - let Some(window) = self.child_window_proxy_records.live_window(scope, handle) else { - return; - }; - let Some(context) = window.get_creation_context(scope) else { - return; - }; - let window = v8::Global::new(scope, window); - let context = v8::Global::new(scope, context); - let context = v8::Local::new(scope, &context); - let child_scope = &mut v8::ContextScope::new(scope, context); - let window = v8::Local::new(child_scope, &window); - crate::context_bootstrap::inform_about_canceled_navigation_for_window( - child_scope, - window, - crate::context_bootstrap::NavigationCancellationReason::LocalWindowRetirement, - ); - } - pub(in crate::native_bridge::context_host) fn refresh_child_window_access_surfaces_after_origin_mutation( &mut self, scope: &mut v8::PinScope<'_, '_>, diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_frames.rs b/moli-renderer-v8/src/native_bridge/context_host/child_frames.rs index c76b31b8c2..41495e6b76 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_frames.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_frames.rs @@ -40,6 +40,7 @@ pub(in crate::native_bridge::context_host) use request_scope::{ #[derive(Debug, Clone)] pub(super) struct ChildBrowsingContextEntry { frame_id: String, + retiring: bool, current_document_loader_id: Option, name: Option, id: Option, 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 f0db7e5a2a..c6dd50eb64 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 @@ -98,6 +98,7 @@ impl JsContextHost { } } + #[cfg(test)] pub(crate) fn top_level_child_browsing_context_handles_in_document_order( &self, ) -> Vec { diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_frames/registry.rs b/moli-renderer-v8/src/native_bridge/context_host/child_frames/registry.rs index c7cfde2912..93df2f3f70 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_frames/registry.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_frames/registry.rs @@ -3,6 +3,14 @@ use super::*; use crate::custom_elements::{CustomElementRegistryAssociation, CustomElementRegistryKey}; use crate::document_script_scheduler::FrameDocumentClassicScriptSchedulerWork; +/// Identity captured before author callbacks. The DOM handle can be reused by +/// a newly attached browsing context while the old Window is being canceled. +struct ChildWindowRetirement { + handle: DomHandle, + frame_id: String, + local_window: Option, +} + impl JsContextHost { fn remove_child_browsing_context_entry( &mut self, @@ -100,6 +108,15 @@ impl JsContextHost { scope: &mut v8::PinScope<'_, '_>, handle: DomHandle, ) -> Option { + // Reinsertion from an abort/navigateerror handler creates a new Window. + // Finish only the retiring context before installing its successor. + if self + .child_browsing_contexts + .get(&handle) + .is_some_and(|entry| entry.retiring) + { + self.drop_child_browsing_context_handles(vec![handle]); + } match self.child_browsing_context_bootstrap_for_handle(handle) { Some(attribute_bootstrap) => { let existing = self.child_browsing_contexts.get(&handle).cloned(); @@ -366,6 +383,7 @@ impl JsContextHost { handle, ChildBrowsingContextEntry { frame_id, + retiring: false, current_document_loader_id: existing.as_ref().and_then(|entry| { entry.current_document_loader_id().map(ToOwned::to_owned) }), @@ -596,24 +614,91 @@ impl JsContextHost { pub(crate) fn drop_child_browsing_context_subtree(&mut self, root: DomHandle) { let mut handles = Vec::new(); self.collect_child_browsing_context_host_handles(root, &mut handles); - self.drop_child_browsing_context_handles(handles, None); + self.drop_child_browsing_context_handles(handles); } + /// Orchestrate synchronous cancellation without borrowing the host across + /// author JS. Callers must also release their host borrow before entering. pub(crate) fn drop_child_browsing_context_subtree_with_window_realm( - &mut self, scope: &mut v8::PinScope<'_, '_>, + host_ptr: *mut Self, root: DomHandle, ) { - let mut handles = Vec::new(); - self.collect_child_browsing_context_host_handles(root, &mut handles); - self.drop_child_browsing_context_handles(handles, Some(scope)); + let retirements = { + let host = unsafe { &mut *host_ptr }; + let mut handles = Vec::new(); + host.collect_child_browsing_context_host_handles(root, &mut handles); + let mut retirements = Vec::new(); + // Mark the whole batch before the first callback: a handler for A + // can reattach B before B reaches its own cancellation boundary. + for handle in handles { + let Some(entry) = host.child_browsing_contexts.get_mut(&handle) else { + continue; + }; + if entry.retiring { + // Nested removal must not dispatch the same cancellation twice. + host.drop_child_browsing_context_handles(vec![handle]); + continue; + } + entry.retiring = true; + let frame_id = entry.frame_id.clone(); + let retirement = ChildWindowRetirement { + handle, + frame_id, + local_window: host + .current_child_document_task_owner(handle) + .map(|owner| owner.local_window_id), + }; + host.prepare_child_window_retirement(handle); + let window = host.child_window_proxy_records.live_window(scope, handle); + retirements.push((retirement, window)); + } + retirements + }; + for (retirement, window) in retirements { + // Settle the captured old Window even if an earlier callback has + // already replaced it. Looking up by handle here would cancel its + // successor, or leave the old Window's promises pending forever. + if let Some(window) = window + && let Some(context) = window.get_creation_context(scope) + { + let scope = &mut v8::ContextScope::new(scope, context); + crate::context_bootstrap::inform_about_canceled_navigation_for_window( + scope, + window, + crate::context_bootstrap::NavigationCancellationReason::LocalWindowRetirement, + ); + } + let host = unsafe { &mut *host_ptr }; + if host.child_window_retirement_is_current(&retirement) { + host.drop_child_browsing_context_handles(vec![retirement.handle]); + } + } } - fn drop_child_browsing_context_handles( - &mut self, - handles: Vec, - mut scope: Option<&mut v8::PinScope<'_, '_>>, - ) { + fn child_window_retirement_is_current(&self, retirement: &ChildWindowRetirement) -> bool { + self.child_browsing_contexts + .get(&retirement.handle) + .is_some_and(|entry| entry.frame_id == retirement.frame_id) + && self + .current_child_document_task_owner(retirement.handle) + .map(|owner| owner.local_window_id) + == retirement.local_window + } + + fn prepare_child_window_retirement(&mut self, handle: DomHandle) { + if let Some(owner) = self.current_child_document_task_owner(handle) { + self.pending_history_traversal_admissions.retire_owner( + super::super::WindowExecutionContextOwner::Frame(owner.local_window_id), + ); + } + self.cancel_child_meta_refresh_navigation(handle); + self.clear_pending_child_document_loads_for_handle(handle); + self.retire_current_child_navigation_commit_task(handle); + self.unregister_service_worker_child_client(handle); + } + + fn drop_child_browsing_context_handles(&mut self, handles: Vec) { for handle in handles { let document_handle_before_drop = self.child_browsing_context_document_handle(handle); let frame_id = self @@ -625,13 +710,7 @@ impl JsContextHost { self.completed_child_browsing_context_loads .retain(|load| load.frame_id != frame_id); } - self.cancel_child_meta_refresh_navigation(handle); - self.clear_pending_child_document_loads_for_handle(handle); - self.retire_current_child_navigation_commit_task(handle); - self.unregister_service_worker_child_client(handle); - if let Some(scope) = scope.as_deref_mut() { - self.inform_about_canceled_child_navigation_before_detach(scope, handle); - } + self.prepare_child_window_retirement(handle); self.clear_child_parser_classic_runner_for_current_document(handle); let removed = self.remove_child_browsing_context_entry(handle).is_some(); self.clear_child_browsing_context_current_document(handle); @@ -658,18 +737,29 @@ impl JsContextHost { } pub(crate) fn drop_child_browsing_contexts_moved_into_own_document_subtree( - &mut self, scope: &mut v8::PinScope<'_, '_>, + host_ptr: *mut Self, root: DomHandle, ) { - let mut handles = Vec::new(); - self.collect_child_browsing_context_host_handles(root, &mut handles); + let handles = { + let host = unsafe { &*host_ptr }; + let mut handles = Vec::new(); + host.collect_child_browsing_context_host_handles(root, &mut handles); + handles + }; for handle in handles { - let Some(owner_document) = self.dom_host().owner_document_handle(handle) else { - continue; + let should_drop = { + let host = unsafe { &*host_ptr }; + host.dom_host() + .owner_document_handle(handle) + .is_some_and(|document| { + host.child_browsing_context_host_is_ancestor_of_document(handle, document) + }) }; - if self.child_browsing_context_host_is_ancestor_of_document(handle, owner_document) { - self.drop_child_browsing_context_subtree_with_window_realm(scope, handle); + if should_drop { + Self::drop_child_browsing_context_subtree_with_window_realm( + scope, host_ptr, handle, + ); } } } diff --git a/moli-renderer-v8/src/native_bridge/context_host/core.rs b/moli-renderer-v8/src/native_bridge/context_host/core.rs index 4fad6cea80..f9d2322f5e 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/core.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/core.rs @@ -501,11 +501,26 @@ impl JsContextHost { .map(RendererDocumentLifecycleJournalHandle::identity) } - pub(crate) fn open_root_document(&mut self, scope: &mut v8::PinScope<'_, '_>) { - let descendant_count_before = self.child_browsing_contexts.len(); - for child_handle in self.top_level_child_browsing_context_handles_in_document_order() { - self.drop_child_browsing_context_subtree_with_window_realm(scope, child_handle); + pub(crate) fn open_root_document(scope: &mut v8::PinScope<'_, '_>, host_ptr: *mut Self) { + let (owner, descendant_count_before, document) = { + let host = unsafe { &*host_ptr }; + ( + host.current_main_document_task_owner(), + host.child_browsing_contexts.len(), + host.document_handle(), + ) + }; + Self::drop_child_browsing_context_subtree_with_window_realm(scope, host_ptr, document); + if unsafe { &*host_ptr }.current_main_document_task_owner() == owner { + unsafe { &mut *host_ptr }.commit_root_document_open(scope, descendant_count_before); } + } + + fn commit_root_document_open( + &mut self, + scope: &mut v8::PinScope<'_, '_>, + descendant_count_before: usize, + ) { let retired_document_handle = self.document_handle(); let retired_image_event_count = self.retire_image_state_for_document(retired_document_handle); diff --git a/moli-renderer-v8/src/native_bridge/document/lifecycle.rs b/moli-renderer-v8/src/native_bridge/document/lifecycle.rs index 7e4f77c323..37b215e950 100644 --- a/moli-renderer-v8/src/native_bridge/document/lifecycle.rs +++ b/moli-renderer-v8/src/native_bridge/document/lifecycle.rs @@ -102,13 +102,13 @@ fn node_document_write_or_writeln_callback<'s>( } if implicit_replacement_session { clear_window_event_handlers(scope); - runtime.prepare_root_document_replacement(scope, runtime_ptr, handle); + JsContextHost::prepare_root_document_replacement(scope, runtime_ptr, handle); } for chunk in parsed.text { - let _ = runtime.write_html(scope, runtime_ptr, handle, &chunk); + let _ = unsafe { &mut *runtime_ptr }.write_html(scope, runtime_ptr, handle, &chunk); } if append_newline { - let _ = runtime.write_html(scope, runtime_ptr, handle, "\n"); + let _ = unsafe { &mut *runtime_ptr }.write_html(scope, runtime_ptr, handle, "\n"); } rv.set_undefined(); } @@ -185,7 +185,7 @@ pub(in crate::native_bridge) fn node_document_open_callback<'s>( let runtime = unsafe { &mut *runtime_ptr }; if !runtime.has_active_parser_write_insertion_point() { clear_window_event_handlers(scope); - runtime.prepare_root_document_replacement(scope, runtime_ptr, handle); + JsContextHost::prepare_root_document_replacement(scope, runtime_ptr, handle); } } rv.set(args.this().into()); @@ -201,13 +201,21 @@ fn clear_window_event_handlers(scope: &mut v8::PinScope<'_, '_>) { impl JsContextHost { fn prepare_root_document_replacement( - &mut self, scope: &mut v8::PinScope<'_, '_>, host_ptr: *mut JsContextHost, document_handle: DomHandle, ) { - self.dispatch_document_open_descendant_frame_unload_lifecycle(scope, document_handle); - self.clear_event_callbacks_for_document_replacement(document_handle, true); + let owner = unsafe { &*host_ptr }.current_main_document_task_owner(); + Self::dispatch_document_open_descendant_frame_unload_lifecycle( + scope, + host_ptr, + document_handle, + ); + if unsafe { &*host_ptr }.current_main_document_task_owner() != owner { + return; + } + unsafe { &mut *host_ptr } + .clear_event_callbacks_for_document_replacement(document_handle, true); custom_elements::with_custom_element_reaction_scope(scope, host_ptr, |scope| { let _ = unsafe { &mut *host_ptr }.remove_all_children_for_document_replacement( scope, @@ -215,25 +223,24 @@ impl JsContextHost { document_handle, ); }); - self.open_root_document(scope); + if unsafe { &*host_ptr }.current_main_document_task_owner() == owner { + Self::open_root_document(scope, host_ptr); + } } /// Replaces the active root document through the native document stream. - /// - /// This is the internal equivalent of Blink's `Document::SetContent`: it - /// deliberately bypasses the page-visible `document.open/write/close` - /// properties, which may have been replaced by page script. + /// Bypasses page-visible document methods without retaining a host borrow + /// across descendant retirement callbacks. pub(crate) fn set_root_document_content( - &mut self, scope: &mut v8::PinScope<'_, '_>, host_ptr: *mut JsContextHost, html: &str, ) { - let document_handle = self.document_handle(); + let document_handle = unsafe { &*host_ptr }.document_handle(); clear_window_event_handlers(scope); - self.prepare_root_document_replacement(scope, host_ptr, document_handle); - let _ = self.write_html(scope, host_ptr, document_handle, html); - self.close_document(scope, host_ptr); + Self::prepare_root_document_replacement(scope, host_ptr, document_handle); + let _ = unsafe { &mut *host_ptr }.write_html(scope, host_ptr, document_handle, html); + unsafe { &mut *host_ptr }.close_document(scope, host_ptr); } } diff --git a/moli-renderer-v8/src/script_vm/document_content.rs b/moli-renderer-v8/src/script_vm/document_content.rs index 15a78c1d6b..1319421c22 100644 --- a/moli-renderer-v8/src/script_vm/document_content.rs +++ b/moli-renderer-v8/src/script_vm/document_content.rs @@ -32,7 +32,9 @@ impl ScriptVm { ) -> Result { let result = if self.root_frame_id() == Some(frame_id) { self.with_default_context_scope(|scope, host_ptr| { - unsafe { &mut *host_ptr }.set_root_document_content(scope, host_ptr, html); + crate::native_bridge::JsContextHost::set_root_document_content( + scope, host_ptr, html, + ); Ok(()) })?; RendererSetDocumentContentResult::Updated @@ -46,7 +48,7 @@ impl ScriptVm { }; let updated = self.with_default_context_scope(|scope, host_ptr| { Ok( - unsafe { &mut *host_ptr }.set_child_browsing_context_document_content( + crate::native_bridge::JsContextHost::set_child_browsing_context_document_content( scope, host_ptr, child_handle, 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 df68fbfec2..260e539c0f 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 @@ -2995,6 +2995,116 @@ async fn pending_precommit_navigation_slot_is_not_script_writable() { "#two:precommitSpoof:false:reload|navigate:|precommit:|exposed:false|abort:AbortError:|navigate:|precommit:|handler:#two|firstCommittedRejected:AbortError|firstFinishedRejected:AbortError|secondCommitted:#two|secondFinished:#two" ); } +#[tokio::test] +async fn navigation_retirement_reentry_preserves_successor_window() { + for pending_sibling in [false, true] { + let server = StaticHttpServer::spawn(if pending_sibling { 2 } else { 1 }).await; + let parent_url = server.base_url().join("parent").unwrap(); + let loader = static_http_loader([]); + let mut vm = + new_storage_page_task_executor_test_vm_with_loader(parent_url.as_str(), &loader); + vm.eval(&format!("globalThis.pendingSibling = {pending_sibling};")) + .unwrap(); + vm.eval( + r##" + globalThis.frame = document.createElement('iframe'); + frame.src = new URL('/child', location.href).href; + globalThis.loaded = 0; + frame.onload = () => loaded++; + const container = document.body || document.documentElement || document; + globalThis.group = document.createElement('div'); + globalThis.survivor = document.createElement('iframe'); + if (globalThis.pendingSibling) { + survivor.src = new URL('/survivor', location.href).href; + survivor.onload = () => loaded++; + } + group.appendChild(frame); group.appendChild(survivor); + container.appendChild(group); + "##, + ) + .unwrap(); + advance_page_task_executor_until_eval_equals( + &mut vm, + &loader, + "String(loaded === (pendingSibling ? 2 : 1))", + "true", + "retirement child load", + ) + .await; + vm.eval( + r##" + const oldChild = frame.contentWindow; + const oldNavigation = oldChild.navigation; + globalThis.log = []; + globalThis.settled = []; + const oldSurvivor = survivor.contentWindow; + globalThis.siblingSettled = []; + globalThis.siblingErrors = 0; + if (globalThis.pendingSibling) { + oldSurvivor.navigation.addEventListener('navigateerror', () => siblingErrors++); + oldSurvivor.navigation.addEventListener('navigate', event => { + event.intercept({handler: () => new Promise(resolve => globalThis.releaseSibling = resolve)}); + }, {once: true}); + const pending = oldSurvivor.navigation.navigate(oldSurvivor.location.href + '#waiting'); + pending.committed.then(() => siblingSettled.push('committed'), e => siblingSettled.push(e.name)); + pending.finished.then(() => siblingSettled.push('finished'), e => siblingSettled.push(e.name)); + } + oldNavigation.addEventListener('navigateerror', event => { + log.push('error:' + event.error.name); + survivor.remove(); + survivor.removeAttribute('src'); + container.appendChild(survivor); + survivor.contentWindow.history.replaceState('successor', ''); + log.push('new-window:' + (survivor.contentWindow !== oldSurvivor)); + }, {once: true}); + oldNavigation.addEventListener('navigate', event => { + event.signal.addEventListener('abort', () => log.push('abort')); + event.intercept({handler() { + log.push('handler'); + group.remove(); + log.push('after-remove'); + }}); + }, {once: true}); + const result = oldNavigation.navigate(oldChild.location.href + '#pending'); + result.committed.then(() => settled.push('committed'), e => settled.push(e.name)); + result.finished.then(() => settled.push('finished'), e => settled.push(e.name)); + log.push('after-navigate'); + "##, + ) + .unwrap(); + assert_eq!( + vm.eval("JSON.stringify(log)").unwrap(), + r#"["handler","abort","error:AbortError","new-window:true","after-remove","after-navigate"]"# + ); + assert_eq!( + vm.eval( + "JSON.stringify([survivor.isConnected, survivor.contentWindow.history.state, settled])" + ) + .unwrap(), + r#"[true,"successor",["committed","AbortError"]]"# + ); + assert!( + vm._context_host + .borrow() + .pending_history_traversal_admissions + .is_empty() + ); + if pending_sibling { + assert_eq!( + vm.eval("JSON.stringify([siblingSettled, siblingErrors])") + .unwrap(), + r#"[["committed","AbortError"],1]"# + ); + vm.eval("releaseSibling();").unwrap(); + assert_eq!( + vm.eval("survivor.contentWindow.history.state").unwrap(), + "successor" + ); + assert_eq!(vm.eval("String(siblingErrors)").unwrap(), "1"); + } + } +} + #[tokio::test] async fn navigation_intercept_handlers_preserve_cancellation_and_committed_entry() { for api in [