From e5ad399b4fdd331f6f36b1ea2a07bd855e865954 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Thu, 24 Sep 2026 05:13:52 +0800 Subject: [PATCH] fix(history): settle retired traversals outside host borrows --- moli-renderer-v8/src/abort_signal_route.rs | 5 +- moli-renderer-v8/src/context_bootstrap.rs | 6 +- .../history_runtime/traversal.rs | 24 +-- .../context_bootstrap/location_navigation.rs | 6 +- .../navigation_callbacks/navigation.rs | 30 ++-- .../navigation_cancellation.rs | 12 +- .../context_bootstrap/navigation_events.rs | 6 +- .../context_bootstrap/navigation_result.rs | 25 +-- .../context_bootstrap/navigation_traversal.rs | 6 +- .../navigation_traversal_coordinator.rs | 31 ++-- .../window_runtime/dialogs.rs | 6 +- moli-renderer-v8/src/native_bridge/abort.rs | 95 +++++++---- .../src/native_bridge/abort/controller.rs | 2 +- .../src/native_bridge/abort/statics.rs | 4 +- .../child_frame_runtime/isolated_world.rs | 6 +- .../child_frame_runtime/window.rs | 14 +- .../src/native_bridge/context_host/popups.rs | 11 +- .../context_host/signal_bridge.rs | 15 -- .../src/native_bridge/history_traversal.rs | 23 ++- moli-renderer-v8/src/script_vm.rs | 12 +- .../src/script_vm/context_scope.rs | 29 +++- moli-renderer-v8/src/script_vm/eval_exec.rs | 1 + .../browser_api/traversal_coordinator.rs | 159 ++++++++++++++++++ .../webidl_callback_source_boundary_tests.rs | 4 +- 24 files changed, 373 insertions(+), 159 deletions(-) diff --git a/moli-renderer-v8/src/abort_signal_route.rs b/moli-renderer-v8/src/abort_signal_route.rs index 64976c071e..1e21937a54 100644 --- a/moli-renderer-v8/src/abort_signal_route.rs +++ b/moli-renderer-v8/src/abort_signal_route.rs @@ -67,10 +67,7 @@ impl<'s> ResolvedAbortSignal<'s> { pub(crate) fn abort(self, scope: &mut v8::PinScope<'s, '_>, reason: v8::Local<'s, v8::Value>) { match self.owner { AbortSignalOwner::Window => { - let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) else { - return; - }; - unsafe { &mut *host_ptr }.abort_signal(scope, self.signal, reason); + crate::native_bridge::abort::abort_signal(scope, self.signal, reason); } AbortSignalOwner::Worker => { let Some(signal_id) = diff --git a/moli-renderer-v8/src/context_bootstrap.rs b/moli-renderer-v8/src/context_bootstrap.rs index 05f1f4e53a..0873bbeefc 100644 --- a/moli-renderer-v8/src/context_bootstrap.rs +++ b/moli-renderer-v8/src/context_bootstrap.rs @@ -139,7 +139,9 @@ pub(crate) use location_navigation::{ navigate_location_object_with_source_element, navigate_top_level_meta_refresh, navigate_top_level_same_document_from_browser, }; -pub(crate) use navigation_cancellation::inform_about_canceled_navigation_for_window; +pub(crate) use navigation_cancellation::{ + NavigationCancellationReason, inform_about_canceled_navigation_for_window, +}; pub(crate) use navigation_events::dispatch_cross_document_navigation_navigate_event_for_window_with_form_data; pub(crate) use navigation_events::{ construct_original_hash_change_event, dispatch_beforeunload_for_runtime_owner, @@ -152,7 +154,7 @@ pub(crate) use navigation_history_pruning::{ pub(crate) use navigation_result::{ NavigationFinishedResultApplication, apply_pending_navigation_finished_result, }; -pub(crate) use navigation_traversal_coordinator::cancel_history_traversals_for_retiring_window; +pub(crate) use navigation_traversal_coordinator::abort_history_traversal_admissions; pub(crate) use navigation_traversal_execution::apply_authorized_history_traversal_task; pub(crate) use performance_runtime::PERFORMANCE_TIME_ORIGIN_SLOT; pub(crate) use performance_runtime::performance_slot_number; diff --git a/moli-renderer-v8/src/context_bootstrap/history_runtime/traversal.rs b/moli-renderer-v8/src/context_bootstrap/history_runtime/traversal.rs index df5e9cad53..1cc54b8fa9 100644 --- a/moli-renderer-v8/src/context_bootstrap/history_runtime/traversal.rs +++ b/moli-renderer-v8/src/context_bootstrap/history_runtime/traversal.rs @@ -243,10 +243,8 @@ pub(in crate::context_bootstrap) fn finish_history_participant<'s>( if let Some(error) = error { finish_navigation_error_events(scope, navigation, error, &applied.url); reject_resolver_array(scope, finished_resolvers, error, true); - if let Some(signal) = outcome.signal - && let Some(host) = context_host_ptr_from_global_bridge(scope) - { - unsafe { &mut *host }.abort_signal(scope, signal, error); + if let Some(signal) = outcome.signal { + crate::native_bridge::abort::abort_signal(scope, signal, error); } return; } @@ -467,10 +465,8 @@ pub(in crate::context_bootstrap) fn cancel_active_history_traversal_intercept_se }; set_traversal_intercept_inactive(scope, navigation, data.into()); let error = navigation_dom_exception(scope, "Navigation was canceled", "AbortError"); - if let Some(signal) = signal - && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) - { - unsafe { &mut *host_ptr }.abort_signal(scope, signal, error); + if let Some(signal) = signal { + crate::native_bridge::abort::abort_signal(scope, signal, error); } finish_navigation_error_events(scope, navigation, error, &url); reject_resolver_array(scope, finished_resolvers, error, true); @@ -491,10 +487,8 @@ fn traversal_intercept_fulfilled_callback<'s>( let owner = runtime_window_owner(scope, navigation); if !navigation_document_is_active(scope, owner) { let error = navigation_dom_exception(scope, "Navigation was canceled", "AbortError"); - if let Some(signal) = signal - && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) - { - unsafe { &mut *host_ptr }.abort_signal(scope, signal, error); + if let Some(signal) = signal { + crate::native_bridge::abort::abort_signal(scope, signal, error); } let top_owner = runtime_top_window_owner(scope, owner); let filename = window_location_for_holder(scope, top_owner) @@ -526,10 +520,8 @@ fn traversal_intercept_rejected_callback<'s>( .filter(|promise| promise.state() == v8::PromiseState::Rejected) .map(|promise| promise.result(scope)) .unwrap_or_else(|| args.get(0)); - if let Some(signal) = signal - && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) - { - unsafe { &mut *host_ptr }.abort_signal(scope, signal, error); + if let Some(signal) = signal { + crate::native_bridge::abort::abort_signal(scope, signal, error); } finish_navigation_error_events(scope, navigation, error, &url); reject_resolver_array(scope, finished_resolvers, error, true); diff --git a/moli-renderer-v8/src/context_bootstrap/location_navigation.rs b/moli-renderer-v8/src/context_bootstrap/location_navigation.rs index d53b2d1f17..c31f3b7574 100644 --- a/moli-renderer-v8/src/context_bootstrap/location_navigation.rs +++ b/moli-renderer-v8/src/context_bootstrap/location_navigation.rs @@ -798,10 +798,8 @@ fn finish_location_navigation_canceled<'s>( href: &str, ) { let error = navigation_dom_exception(scope, "Navigation was canceled", "AbortError"); - if let Some(signal) = outcome.signal - && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) - { - unsafe { &mut *host_ptr }.abort_signal(scope, signal, error); + if let Some(signal) = outcome.signal { + crate::native_bridge::abort::abort_signal(scope, signal, error); } finish_navigation_error_events(scope, navigation, error, href); } diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_callbacks/navigation.rs b/moli-renderer-v8/src/context_bootstrap/navigation_callbacks/navigation.rs index 0cfae2ee0e..bec1fa2fa9 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_callbacks/navigation.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_callbacks/navigation.rs @@ -685,10 +685,8 @@ fn navigation_canceled_after_dispatch_result<'s>( ) -> v8::Local<'s, v8::Object> { let error = navigation_dom_exception(scope, "Navigation was canceled", "AbortError"); super::super::navigation_events::mark_navigation_outcome_default_prevented(scope, outcome); - if let Some(signal) = outcome.signal - && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) - { - unsafe { &mut *host_ptr }.abort_signal(scope, signal, error); + if let Some(signal) = outcome.signal { + crate::native_bridge::abort::abort_signal(scope, signal, error); } finish_navigation_error_events(scope, navigation, error, href); navigation_rejected_value_result(scope, error) @@ -1010,10 +1008,8 @@ pub(in crate::context_bootstrap) fn cancel_pending_precommit_same_document_navig }; let error = navigation_dom_exception(scope, "Navigation was canceled before commit", "AbortError"); - if let Some(signal) = data.signal - && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) - { - unsafe { &mut *host_ptr }.abort_signal(scope, signal, error); + if let Some(signal) = data.signal { + crate::native_bridge::abort::abort_signal(scope, signal, error); } finish_navigation_error_events(scope, data.navigation, error, &data.current_href); let receiver = v8::undefined(scope).into(); @@ -1294,10 +1290,8 @@ fn precommit_commit_rejected_callback<'s>( .filter(|promise| promise.state() == v8::PromiseState::Rejected) .map(|promise| promise.result(scope)) .unwrap_or_else(|| args.get(0)); - if let Some(signal) = data.signal - && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) - { - unsafe { &mut *host_ptr }.abort_signal(scope, signal, error); + if let Some(signal) = data.signal { + crate::native_bridge::abort::abort_signal(scope, signal, error); } finish_navigation_error_events(scope, data.navigation, error, &data.current_href); let receiver = v8::undefined(scope).into(); @@ -1594,10 +1588,8 @@ pub(in crate::context_bootstrap) fn cancel_active_intercepted_same_document_navi return false; }; let error = navigation_dom_exception(scope, "Navigation was canceled", "AbortError"); - if let Some(signal) = signal - && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) - { - unsafe { &mut *host_ptr }.abort_signal(scope, signal, error); + if let Some(signal) = signal { + crate::native_bridge::abort::abort_signal(scope, signal, error); } finish_navigation_error_events(scope, navigation, error, &filename); let receiver = v8::undefined(scope).into(); @@ -1850,10 +1842,8 @@ fn finish_intercepted_navigation_rejected<'s>( error: v8::Local<'s, v8::Value>, filename: &str, ) { - if let Some(signal) = signal - && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) - { - unsafe { &mut *host_ptr }.abort_signal(scope, signal, error); + if let Some(signal) = signal { + crate::native_bridge::abort::abort_signal(scope, signal, error); } finish_navigation_error_events(scope, navigation, error, filename); if let Some(committed_resolve) = committed_resolve { diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_cancellation.rs b/moli-renderer-v8/src/context_bootstrap/navigation_cancellation.rs index 7726c393df..91bce88186 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_cancellation.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_cancellation.rs @@ -15,9 +15,17 @@ use super::{ }; use crate::util::context_host_ptr_from_global_bridge; +pub(crate) enum NavigationCancellationReason { + WindowStop, + /// Native retirement has already invalidated and extracted traversal + /// admissions; their JS settlement belongs to the owning ScriptVm. + LocalWindowRetirement, +} + pub(crate) fn inform_about_canceled_navigation_for_window<'s>( scope: &mut v8::PinScope<'s, '_>, window: v8::Local<'s, v8::Object>, + reason: NavigationCancellationReason, ) { let owner = runtime_window_owner(scope, window); let Some(navigation) = window_navigation_for_holder(scope, owner) else { @@ -26,7 +34,9 @@ pub(crate) fn inform_about_canceled_navigation_for_window<'s>( let _ = cancel_active_navigation_event(scope, navigation); cancel_active_intercepted_same_document_navigation(scope, navigation); cancel_active_cross_document_navigation(scope, navigation, None); - cancel_pending_precommit_history_traversal(scope, navigation); + if matches!(reason, NavigationCancellationReason::WindowStop) { + cancel_pending_precommit_history_traversal(scope, navigation); + } cancel_pending_precommit_same_document_navigation_for_window_stop(scope, navigation); cancel_pending_same_document_navigation_finishes_including_reentrant(scope, navigation); if runtime_window_is_global(scope, owner) diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_events.rs b/moli-renderer-v8/src/context_bootstrap/navigation_events.rs index 89cdbdab4b..251444c675 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_events.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_events.rs @@ -563,10 +563,8 @@ pub(super) fn cancel_active_navigation_event<'s>( } let error = navigation_dom_exception(scope, "Navigation was canceled", "AbortError"); set_private_value(scope, event, NAVIGATE_EVENT_ABORT_ERROR_SLOT, error); - if let Some(signal) = signal - && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) - { - unsafe { &mut *host_ptr }.abort_signal(scope, signal, error); + if let Some(signal) = signal { + crate::native_bridge::abort::abort_signal(scope, signal, error); } finish_navigation_error_events(scope, navigation, error, &href); Some(error) diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_result.rs b/moli-renderer-v8/src/context_bootstrap/navigation_result.rs index 261fe3580f..d6ca932377 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_result.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_result.rs @@ -292,18 +292,18 @@ pub(super) fn cancel_pending_same_document_navigation_finishes<'s>( let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) else { return canceled; }; - let host = unsafe { &mut *host_ptr }; - let pending = host.take_pending_navigation_finished_results_for_navigation(scope, navigation); + let pending = unsafe { &mut *host_ptr } + .take_pending_navigation_finished_results_for_navigation(scope, navigation); if pending.is_empty() { return canceled; } canceled = true; for result in pending { - host.cancel_navigation_lifecycle_attempt(result.attempt_id); + unsafe { &mut *host_ptr }.cancel_navigation_lifecycle_attempt(result.attempt_id); let error = navigation_dom_exception(scope, "Navigation was canceled", "AbortError"); if let Some(signal) = result.signal { let signal = v8::Local::new(scope, signal); - host.abort_signal(scope, signal, error); + crate::native_bridge::abort::abort_signal(scope, signal, error); } finish_navigation_error_events(scope, navigation, error, &result.href); if let (Some(resolve), Some(value)) = (result.committed_resolve, result.resolved_value) { @@ -354,13 +354,13 @@ pub(super) fn cancel_active_cross_document_navigation<'s>( clear_active_cross_document_navigation(scope, navigation); let error = navigation_dom_exception(scope, "Navigation was canceled", "AbortError"); if let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) { - let host = unsafe { &mut *host_ptr }; + // Remove the old load before abort listeners can start a successor. + unsafe { &mut *host_ptr }.clear_pending_location_navigation(); if let Some(signal) = get_private_value(scope, data, CROSS_DOCUMENT_PENDING_SIGNAL_SLOT) .and_then(|value| v8::Local::::try_from(value).ok()) { - host.abort_signal(scope, signal, error); + crate::native_bridge::abort::abort_signal(scope, signal, error); } - host.clear_pending_location_navigation(); } finish_navigation_error_events(scope, navigation, error, &href); let receiver = v8::undefined(scope).into(); @@ -621,13 +621,13 @@ fn navigation_result_with_task_finished<'s>( let task = host .take_pending_navigation_api_task(task_id) .expect("a rejected Navigation API route must retain its Host-local payload"); - reject_pending_navigation_api_task(scope, host, task.action); + reject_pending_navigation_api_task(scope, task.action); } } else { host.cancel_navigation_lifecycle_attempt(attempt_id); let error = navigation_dom_exception(scope, "Navigation was canceled", "AbortError"); if let Some(signal) = signal { - host.abort_signal(scope, signal, error); + crate::native_bridge::abort::abort_signal(scope, signal, error); } finish_navigation_error_events(scope, navigation, error, href); settle_navigation_finished_rejected(scope, finished_reject, error); @@ -688,17 +688,18 @@ pub(crate) fn apply_pending_navigation_finished_result<'s>( fn reject_pending_navigation_api_task<'s>( scope: &mut v8::PinScope<'s, '_>, - host: &mut JsContextHost, action: crate::native_bridge::PendingNavigationApiTaskAction, ) { match action { crate::native_bridge::PendingNavigationApiTaskAction::FinishResult(result) => { - host.cancel_navigation_lifecycle_attempt(result.attempt_id); + if let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) { + unsafe { &mut *host_ptr }.cancel_navigation_lifecycle_attempt(result.attempt_id); + } let navigation = v8::Local::new(scope, result.navigation); let error = navigation_dom_exception(scope, "Navigation was canceled", "AbortError"); if let Some(signal) = result.signal { let signal = v8::Local::new(scope, signal); - host.abort_signal(scope, signal, error); + crate::native_bridge::abort::abort_signal(scope, signal, error); } finish_navigation_error_events(scope, navigation, error, &result.href); if let Some(reject) = result.finished_reject { diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_traversal.rs b/moli-renderer-v8/src/context_bootstrap/navigation_traversal.rs index d20a4237d0..1a6ed38594 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_traversal.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_traversal.rs @@ -390,10 +390,8 @@ fn reload_canceled_after_dispatch_result<'s>( ) -> v8::Local<'s, v8::Object> { let error = navigation_dom_exception(scope, "Navigation was canceled", "AbortError"); super::navigation_events::mark_navigation_outcome_default_prevented(scope, outcome); - if let Some(signal) = outcome.signal - && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) - { - unsafe { &mut *host_ptr }.abort_signal(scope, signal, error); + if let Some(signal) = outcome.signal { + crate::native_bridge::abort::abort_signal(scope, signal, error); } finish_navigation_error_events(scope, navigation, error, href); navigation_rejected_value_result(scope, error) diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_traversal_coordinator.rs b/moli-renderer-v8/src/context_bootstrap/navigation_traversal_coordinator.rs index 8d15b04a0d..6804b582ef 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_traversal_coordinator.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_traversal_coordinator.rs @@ -310,16 +310,22 @@ fn abort<'s>( return; } deactivate(scope, admission); + settle_aborted_admission(scope, admission, error); +} + +fn settle_aborted_admission<'s>( + scope: &mut v8::PinScope<'s, '_>, + admission: &PendingHistoryTraversalAdmission, + error: Option>, +) { let error = error.unwrap_or_else(|| { navigation_dom_exception(scope, "History traversal was canceled", "AbortError") }); results::reject_pending_navigation_results(scope, &admission.results, error); for participant in &admission.participants { - if let Some(signal) = &participant.outcome.signal - && let Some(host) = context_host_ptr_from_global_bridge(scope) - { + if let Some(signal) = &participant.outcome.signal { let signal = v8::Local::new(scope, signal); - unsafe { &mut *host }.abort_signal(scope, signal, error); + crate::native_bridge::abort::abort_signal(scope, signal, error); } if let Some(navigation) = &participant.navigation { let navigation = v8::Local::new(scope, navigation); @@ -352,20 +358,15 @@ fn callback_id(value: v8::Local<'_, v8::Value>) -> Option { Some(HistoryTraversalId::from_raw(value.u64_value().0)) } -/// LocalWindow retirement must settle surviving callers before releasing handles. -/// Drain first: rejecting promises and dispatching abort/error events can reenter. -pub(crate) fn cancel_history_traversals_for_retiring_window( +/// The caller has removed and invalidated the entire batch under a short native +/// borrow. Neither extraction nor further native retirement happens around JS. +pub(crate) fn abort_history_traversal_admissions( scope: &mut v8::PinScope<'_, '_>, - owner: crate::native_bridge::WindowExecutionContextOwner, + admissions: Vec>, ) { - let Some(host) = context_host_ptr_from_global_bridge(scope) else { - return; - }; - let admissions = unsafe { &mut *host } - .pending_history_traversal_admissions - .take_for_owner(owner); for admission in admissions { - abort(scope, &admission, None); + debug_assert!(!admission.active.get()); + settle_aborted_admission(scope, &admission, None); } } diff --git a/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs b/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs index 1796538c16..28193f87f7 100644 --- a/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs +++ b/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs @@ -73,7 +73,11 @@ pub(crate) fn window_stop_callback<'s>( args: v8::FunctionCallbackArguments<'s>, _rv: v8::ReturnValue<'_, v8::Value>, ) { - inform_about_canceled_navigation_for_window(scope, args.this()); + inform_about_canceled_navigation_for_window( + scope, + args.this(), + super::super::NavigationCancellationReason::WindowStop, + ); } pub(in crate::context_bootstrap) fn window_confirm_callback<'s>( diff --git a/moli-renderer-v8/src/native_bridge/abort.rs b/moli-renderer-v8/src/native_bridge/abort.rs index 03bece35eb..d21b707869 100644 --- a/moli-renderer-v8/src/native_bridge/abort.rs +++ b/moli-renderer-v8/src/native_bridge/abort.rs @@ -51,6 +51,12 @@ struct AbortLinkedTargetListener { capture: bool, } +struct AbortSignalDispatch { + algorithms: Vec>, + linked_target_listeners: Vec, + dependent_signals: Vec, +} + impl AbortStore { fn alloc_signal_id(&mut self) -> u32 { self.next_signal_id = self @@ -257,48 +263,24 @@ impl AbortStore { } } - pub(super) fn abort_signal<'s>( + fn take_signal_abort<'s>( &mut self, scope: &mut v8::PinScope<'s, '_>, - host: &mut super::JsContextHost, signal: v8::Local<'s, v8::Object>, reason: v8::Local<'s, v8::Value>, - ) { - let Some(signal_id) = Self::signal_id_from_object(scope, signal) else { - return; - }; - let Some((abort_algorithms, linked_target_listeners, dependent_signals)) = ({ - let Some(state) = self.signal_state_mut(signal_id) else { - return; - }; - if state.aborted { - return; - } - state.aborted = true; - state.reason = Some(v8::Global::new(scope, reason)); - let abort_algorithms = std::mem::take(&mut state.abort_algorithms); - let linked_target_listeners = std::mem::take(&mut state.linked_target_listeners); - let dependent_signals = state.dependent_signals.clone(); - Some((abort_algorithms, linked_target_listeners, dependent_signals)) - }) else { - return; - }; - event::invoke_abort_algorithms(scope, signal, reason, abort_algorithms); - abort_signal_events::dispatch_abort(scope, signal); - for linked in linked_target_listeners { - host.remove_registered_event_listener_by_id( - linked.target, - &linked.event_type, - linked.callback_id, - linked.capture, - ); - } - for dependent_signal_id in dependent_signals { - let Some(dependent_signal) = self.signal_object(scope, dependent_signal_id) else { - continue; - }; - host.abort_signal(scope, dependent_signal, reason); + ) -> Option { + let signal_id = Self::signal_id_from_object(scope, signal)?; + let state = self.signal_state_mut(signal_id)?; + if state.aborted { + return None; } + state.aborted = true; + state.reason = Some(v8::Global::new(scope, reason)); + Some(AbortSignalDispatch { + algorithms: std::mem::take(&mut state.abort_algorithms), + linked_target_listeners: std::mem::take(&mut state.linked_target_listeners), + dependent_signals: state.dependent_signals.clone(), + }) } pub(super) fn link_dependent_signal( @@ -315,6 +297,45 @@ impl AbortStore { } } +/// Aborting a signal invokes author algorithms and listeners synchronously. +/// Only owned dispatch data may cross those calls; neither the host nor its +/// AbortStore can remain borrowed, including while aborting dependent signals. +pub(crate) fn abort_signal<'s>( + scope: &mut v8::PinScope<'s, '_>, + signal: v8::Local<'s, v8::Object>, + reason: v8::Local<'s, v8::Value>, +) { + let Some(host_ptr) = crate::util::context_host_ptr_from_global_bridge(scope) else { + return; + }; + let dispatch = unsafe { &mut *host_ptr } + .native_bridge_mut() + .abort + .take_signal_abort(scope, signal, reason); + let Some(dispatch) = dispatch else { + return; + }; + event::invoke_abort_algorithms(scope, signal, reason, dispatch.algorithms); + abort_signal_events::dispatch_abort(scope, signal); + for linked in dispatch.linked_target_listeners { + unsafe { &mut *host_ptr }.remove_registered_event_listener_by_id( + linked.target, + &linked.event_type, + linked.callback_id, + linked.capture, + ); + } + for dependent_signal_id in dispatch.dependent_signals { + let signal = unsafe { &mut *host_ptr } + .native_bridge_mut() + .abort + .signal_object(scope, dependent_signal_id); + if let Some(signal) = signal { + abort_signal(scope, signal, reason); + } + } +} + pub(crate) fn dom_exception_value<'s>( scope: &mut v8::PinScope<'s, '_>, message: &str, diff --git a/moli-renderer-v8/src/native_bridge/abort/controller.rs b/moli-renderer-v8/src/native_bridge/abort/controller.rs index 5cb6f4647c..ff0e78d888 100644 --- a/moli-renderer-v8/src/native_bridge/abort/controller.rs +++ b/moli-renderer-v8/src/native_bridge/abort/controller.rs @@ -87,6 +87,6 @@ pub(crate) fn abort_controller_abort_callback<'s>( rv.set_undefined(); return; } - host.abort_signal(scope, signal, reason); + crate::native_bridge::abort::abort_signal(scope, signal, reason); rv.set_undefined(); } diff --git a/moli-renderer-v8/src/native_bridge/abort/statics.rs b/moli-renderer-v8/src/native_bridge/abort/statics.rs index fd7fd0b111..b62d7ad9a9 100644 --- a/moli-renderer-v8/src/native_bridge/abort/statics.rs +++ b/moli-renderer-v8/src/native_bridge/abort/statics.rs @@ -105,7 +105,7 @@ pub(crate) fn abort_signal_any_callback<'s>( else { continue; }; - host.abort_signal(scope, signal, reason); + crate::native_bridge::abort::abort_signal(scope, signal, reason); rv.set(signal.into()); return; } @@ -150,6 +150,6 @@ fn abort_signal_timeout_fire_native_callback( return; }; let reason = timeout_error_value(scope); - host.abort_signal(scope, signal, reason); + crate::native_bridge::abort::abort_signal(scope, signal, reason); rv.set_undefined(); } diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/isolated_world.rs b/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/isolated_world.rs index c19977b42a..80c19866fc 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/isolated_world.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/isolated_world.rs @@ -135,10 +135,8 @@ impl JsContextHost { } let stale = pending_contexts.borrow_mut().remove(&handle); if let Some(stale) = stale { - crate::context_bootstrap::cancel_history_traversals_for_retiring_window( - scope, - super::super::WindowExecutionContextOwner::Frame(stale.local_window_id), - ); + self.pending_history_traversal_admissions + .retire_owner(WindowExecutionContextOwner::Frame(stale.local_window_id)); self.retire_window_execution_contexts_for_context_token( stale.runtime_observable_context_token, ); 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 47f3bc1c6c..ac231202c4 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 @@ -636,6 +636,14 @@ impl JsContextHost { 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; }; @@ -647,7 +655,11 @@ impl JsContextHost { 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::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( diff --git a/moli-renderer-v8/src/native_bridge/context_host/popups.rs b/moli-renderer-v8/src/native_bridge/context_host/popups.rs index 729374e5a5..e6a7adddac 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/popups.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/popups.rs @@ -1905,7 +1905,7 @@ impl JsContextHost { ); } if let Some(retired_local_window_id) = transition.retired_local_window_id { - self.retire_lightweight_popup_local_window(scope, popup_id, retired_local_window_id); + self.retire_lightweight_popup_local_window(popup_id, retired_local_window_id); } self.refresh_lightweight_popup_indexed_db_factory(scope, popup_id, window); tracing::debug!( @@ -1929,7 +1929,6 @@ impl JsContextHost { fn retire_lightweight_popup_local_window( &mut self, - scope: &mut v8::PinScope<'_, '_>, popup_id: u64, local_window_id: LightweightPopupLocalWindowId, ) { @@ -1937,10 +1936,8 @@ impl JsContextHost { popup_id, local_window_id, }; - crate::context_bootstrap::cancel_history_traversals_for_retiring_window( - scope, - execution_context_owner, - ); + self.pending_history_traversal_admissions + .retire_owner(execution_context_owner); let retired_timer_count = unsafe { &mut *self.runtime } .cancel_window_execution_context_timers(execution_context_owner); let retired_webcrypto_count = @@ -4321,7 +4318,7 @@ fn lightweight_popup_close_callback<'s>( host.clear_custom_element_registry_associations_for_document(document_handle); } host.retire_lightweight_popup_document_owner(transition.retired_owner); - host.retire_lightweight_popup_local_window(scope, popup_id, transition.retired_local_window_id); + host.retire_lightweight_popup_local_window(popup_id, transition.retired_local_window_id); host.lightweight_popup_window_names .retain(|_, named_popup_id| *named_popup_id != popup_id); } diff --git a/moli-renderer-v8/src/native_bridge/context_host/signal_bridge.rs b/moli-renderer-v8/src/native_bridge/context_host/signal_bridge.rs index c7fac77cad..a1fa662726 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/signal_bridge.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/signal_bridge.rs @@ -94,19 +94,4 @@ impl JsContextHost { self.release_event_callback(callback_id); } } - - pub(crate) fn abort_signal<'s>( - &mut self, - scope: &mut v8::PinScope<'s, '_>, - signal: v8::Local<'s, v8::Object>, - reason: v8::Local<'s, v8::Value>, - ) { - let host_ptr = self as *mut Self; - unsafe { - (*host_ptr) - .bridge - .abort - .abort_signal(scope, &mut *host_ptr, signal, reason); - } - } } diff --git a/moli-renderer-v8/src/native_bridge/history_traversal.rs b/moli-renderer-v8/src/native_bridge/history_traversal.rs index 2560c7f732..08232982f7 100644 --- a/moli-renderer-v8/src/native_bridge/history_traversal.rs +++ b/moli-renderer-v8/src/native_bridge/history_traversal.rs @@ -63,11 +63,13 @@ pub(crate) struct PendingHistoryTraversalAdmission { #[derive(Default)] pub(crate) struct PendingHistoryTraversalAdmissions { entries: HashMap>, + retired: Vec>, } impl PendingHistoryTraversalAdmissions { + #[cfg(test)] pub(crate) fn is_empty(&self) -> bool { - self.entries.is_empty() + self.entries.is_empty() && self.retired.is_empty() } pub(crate) fn insert( @@ -127,15 +129,32 @@ impl PendingHistoryTraversalAdmissions { .iter() .any(|participant| participant.execution_owner == owner) }) - .map(|(_, admission)| admission) + .map(|(_, admission)| { + // Invalidate even an Rc already held by an executing callback. + admission.active.set(false); + admission + }) .collect() } + /// Native retirement cannot run author script while borrowing JsContextHost. + /// Remove callbacks' tokens now; the VM settles the owned batch after the + /// native stack has returned. No cleanup of a successor runs after events. + pub(crate) fn retire_owner(&mut self, owner: WindowExecutionContextOwner) { + let admissions = self.take_for_owner(owner); + self.retired.extend(admissions); + } + + pub(crate) fn take_retired(&mut self) -> Vec> { + std::mem::take(&mut self.retired) + } + pub(crate) fn clear(&mut self) { for admission in self.entries.values() { admission.active.set(false); } self.entries.clear(); + self.retired.clear(); } #[cfg(test)] diff --git a/moli-renderer-v8/src/script_vm.rs b/moli-renderer-v8/src/script_vm.rs index 001c28b4e8..c9e84bf54b 100644 --- a/moli-renderer-v8/src/script_vm.rs +++ b/moli-renderer-v8/src/script_vm.rs @@ -3595,7 +3595,9 @@ impl ScriptVm { context.detach_global(); Ok(()) })?; - None + // Cancellation can synchronously install a successor realm or + // detach the frame. Resolve both owner and pending context anew. + return self.create_new_child_default_world(frame_id, child_handle); } None => None, }; @@ -3742,6 +3744,14 @@ impl ScriptVm { Ok(()) }); } + let refreshed_live; + let live = if stale_prebootstrapped_contexts.is_empty() { + live + } else { + // Retirement callbacks may have materialized successor realms. + refreshed_live = self.live_child_default_context_entries(); + &refreshed_live + }; let stale_context_ids = self .child_frame_realm_store .iter_by_execution_context_id() diff --git a/moli-renderer-v8/src/script_vm/context_scope.rs b/moli-renderer-v8/src/script_vm/context_scope.rs index 8c49fa1b57..dea6a0d944 100644 --- a/moli-renderer-v8/src/script_vm/context_scope.rs +++ b/moli-renderer-v8/src/script_vm/context_scope.rs @@ -8,20 +8,39 @@ use super::perform_microtask_checkpoint_and_report_pending_promise_rejections; use crate::{frame_owner_model::FrameRealmId, native_bridge::JsContextHost}; impl ScriptVm { + pub(super) fn settle_retired_history_traversals(&mut self) { + loop { + let admissions = self + ._context_host + .borrow_mut() + .pending_history_traversal_admissions + .take_retired(); + if admissions.is_empty() { + return; + } + let _ = self.with_default_context_scope(|scope, _| { + // Neither a host borrow nor native retirement stack crosses JS. + // Reentrant retirement is extracted by the next loop iteration. + crate::context_bootstrap::abort_history_traversal_admissions(scope, admissions); + Self::perform_microtask_checkpoints(scope, None) + }); + } + } + pub(super) fn cancel_history_traversals_for_retiring_window( &mut self, owner: crate::native_bridge::WindowExecutionContextOwner, ) { - if self + let admissions = self ._context_host - .borrow() + .borrow_mut() .pending_history_traversal_admissions - .is_empty() - { + .take_for_owner(owner); + if admissions.is_empty() { return; } let _ = self.with_default_context_scope(|scope, _| { - crate::context_bootstrap::cancel_history_traversals_for_retiring_window(scope, owner); + crate::context_bootstrap::abort_history_traversal_admissions(scope, admissions); Ok(()) }); } diff --git a/moli-renderer-v8/src/script_vm/eval_exec.rs b/moli-renderer-v8/src/script_vm/eval_exec.rs index 135c53b802..18f086bccb 100644 --- a/moli-renderer-v8/src/script_vm/eval_exec.rs +++ b/moli-renderer-v8/src/script_vm/eval_exec.rs @@ -1254,6 +1254,7 @@ impl ScriptVm { ) -> T { self.apply_pending_main_document_owner_transitions(); self.apply_pending_child_document_owner_retirements(); + self.settle_retired_history_traversals(); self.drain_pending_style_invalidations_for_turn_exit(boundary); let runtime_continuation_is_ready = self.runtime_script_work_should_signal_immediate_progress(); diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/traversal_coordinator.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/traversal_coordinator.rs index 25361159bd..c5988ee3d0 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/traversal_coordinator.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/traversal_coordinator.rs @@ -1,5 +1,164 @@ use super::*; +#[tokio::test] +async fn history_traversal_retirement_reentry_preserves_reattached_frame_and_sibling_state() { + let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).unwrap(); + let mut vm = + new_storage_page_task_executor_test_vm_with_loader("https://example.com/base", &loader); + vm.eval(r#" + globalThis.frame = document.createElement('iframe'); + globalThis.sibling = document.createElement('iframe'); + const container = document.body || document.documentElement || document; + container.appendChild(frame); container.appendChild(sibling); + globalThis.oldChild = frame.contentWindow; + history.replaceState(0, ''); oldChild.history.replaceState(0, ''); + history.pushState(1, ''); oldChild.history.pushState(1, ''); + globalThis.log = []; + navigation.onnavigate = event => { + event.signal.addEventListener('abort', () => { + sibling.contentWindow.history.pushState('from-abort', ''); + log.push('abort'); + }); + event.intercept({precommitHandler: () => new Promise(resolve => globalThis.release = resolve)}); + }; + navigation.addEventListener('navigateerror', () => { + log.push('error'); + sibling.contentWindow.history.pushState('from-error', ''); + container.appendChild(frame); + frame.contentWindow.history.replaceState('successor', ''); + release(); + }, {once:true}); + const result = navigation.back(); + result.committed.catch(error => log.push('committed:' + error.name)); + result.finished.catch(error => log.push('finished:' + error.name)); + "#).unwrap(); + assert!( + vm.run_one_history_traversal_executor_turn(&loader) + .await + .unwrap() + ); + assert_eq!( + vm._context_host + .borrow() + .pending_history_traversal_admissions + .len(), + 1 + ); + // Detach during a Promise reaction also exercises retirement discovered + // during a checkpoint, not only the outer script body's retirement batch. + vm.eval("Promise.resolve().then(() => frame.remove());") + .unwrap(); + let snapshot = r#"JSON.stringify([ + history.state, sibling.contentWindow.history.state, + frame.isConnected, frame.contentWindow.history.state, log + ])"#; + let expected = r#"[1,"from-error",true,"successor",["abort","error","committed:AbortError","finished:AbortError"]]"#; + assert_eq!(vm.eval(snapshot).unwrap(), expected); + assert!( + vm._context_host + .borrow() + .pending_history_traversal_admissions + .is_empty() + ); + vm.eval("release();").unwrap(); + assert_eq!(vm.eval(snapshot).unwrap(), expected); + assert!( + vm._context_host + .borrow() + .pending_history_traversal_admissions + .is_empty() + ); + assert_eq!( + vm.eval("frame.contentWindow.document.readyState").unwrap(), + "complete" + ); +} + +#[tokio::test] +async fn history_traversal_retirement_error_can_start_successor_admission() { + let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).unwrap(); + let mut vm = + new_storage_page_task_executor_test_vm_with_loader("https://example.com/base", &loader); + vm.eval( + r#" + globalThis.frame = document.createElement('iframe'); + (document.body || document.documentElement || document).appendChild(frame); + history.replaceState(0, ''); frame.contentWindow.history.replaceState(0, ''); + history.pushState(1, ''); frame.contentWindow.history.pushState(1, ''); + globalThis.gates = []; globalThis.log = []; + navigation.onnavigate = event => { + if (event.navigationType === 'traverse') event.intercept({ + precommitHandler: () => new Promise(resolve => gates.push(resolve)) + }); + }; + navigation.addEventListener('navigateerror', () => { + history.pushState(2, ''); + const next = navigation.back(); + next.committed.then(() => log.push('newCommitted')); + next.finished.then(() => log.push('newFinished')); + }, {once:true}); + const result = navigation.back(); + result.committed.catch(error => log.push('oldCommitted:' + error.name)); + result.finished.catch(error => log.push('oldFinished:' + error.name)); + "#, + ) + .unwrap(); + assert!( + vm.run_one_history_traversal_executor_turn(&loader) + .await + .unwrap() + ); + assert_eq!( + vm._context_host + .borrow() + .pending_history_traversal_admissions + .len(), + 1 + ); + vm.eval("frame.remove();").unwrap(); + assert!( + vm._context_host + .borrow() + .pending_history_traversal_admissions + .is_empty() + ); + assert!( + vm.run_one_history_traversal_executor_turn(&loader) + .await + .unwrap() + ); + assert_eq!( + vm._context_host + .borrow() + .pending_history_traversal_admissions + .len(), + 1 + ); + vm.eval("gates[0]();").unwrap(); + assert_eq!( + vm.eval("JSON.stringify([history.state, log])").unwrap(), + r#"[2,["oldCommitted:AbortError","oldFinished:AbortError"]]"# + ); + assert_eq!( + vm._context_host + .borrow() + .pending_history_traversal_admissions + .len(), + 1 + ); + vm.eval("gates[1]();").unwrap(); + assert_eq!( + vm.eval("JSON.stringify([history.state, log])").unwrap(), + r#"[1,["oldCommitted:AbortError","oldFinished:AbortError","newCommitted","newFinished"]]"# + ); + assert!( + vm._context_host + .borrow() + .pending_history_traversal_admissions + .is_empty() + ); +} + #[tokio::test] async fn history_traversal_canceled_precommit_callback_cannot_complete_successor() { let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).unwrap(); diff --git a/moli-renderer-v8/src/webidl_callback_source_boundary_tests.rs b/moli-renderer-v8/src/webidl_callback_source_boundary_tests.rs index 855816146f..73e27e2f61 100644 --- a/moli-renderer-v8/src/webidl_callback_source_boundary_tests.rs +++ b/moli-renderer-v8/src/webidl_callback_source_boundary_tests.rs @@ -38,7 +38,9 @@ const RAW_GLOBAL_FUNCTION_ALLOWLIST: &[(&str, usize)] = &[ ("host/timers.rs", 1), // AbortSignal listeners and onabort now use the shared typed EventTarget // registry. These remaining roots own browser-created abort algorithms. - ("native_bridge/abort.rs", 1), + // Native abort algorithms reside in the store or its extracted dispatch + // batch. Dispatch moves the roots; it does not add a Web IDL callback store. + ("native_bridge/abort.rs", 2), ("native_bridge/abort/event.rs", 1), ("native_bridge/history_queue.rs", 3), ("script_vm/frame_script_jobs.rs", 3),