From 3bb82b2fdbe046b3fa54e4071f0bc249ecdced06 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Wed, 9 Sep 2026 18:03:55 +0800 Subject: [PATCH] fix(script): checkpoint microtasks during event callback cleanup Perform HTML script cleanup between event callbacks when the execution-context stack becomes empty, retaining currentTarget, passive state, and window.event through the checkpoint. Preserve script execution scopes during exception reporting, use the same callback boundary for Worker.onerror, and guard the complete microtask checkpoint against reentrancy. Remove deferred window.event restoration. Update callback-order assertions and asynchronous fixtures to wait for the final event they inspect, while retaining scheduler, document-owner, and follow-up checks. Add Window, child Window, and Worker coverage for nested dispatch, listener removal in microtasks, and script versus callback exceptions. Callback exception ordering follows Web IDL cleanup before rethrow; the local Chromium probe reports the error before that checkpoint. WPT: 3 additional passes in the script-element microtask tests; the other 492 cases retain identical statuses and subtest results. Update the corresponding passed/failed case lists. Validation: cargo fmt --all; cargo clippy --workspace --all-targets --all-features -- -D warnings; cargo nextest run --no-fail-fast (17,843 passed, 13 skipped). Fresh CLI build SHA-256 matches the binary used for the 495-case WPT comparison. --- .../wpt-cross-current/failed-cases.txt | 3 - .../wpt-cross-current/passed-cases.txt | 3 + moli-core/tests/scripts/child_readiness.rs | 3 + moli-core/tests/web_apis.rs | 2 + moli-core/tests/web_apis/callback_cleanup.rs | 174 ++++++++++++++++ moli-renderer-v8/src/callback_invocation.rs | 187 ++++++++---------- moli-renderer-v8/src/context_bootstrap.rs | 1 - .../events/simple_event_target/callbacks.rs | 3 - .../context_bootstrap/microtask_checkpoint.rs | 68 ------- moli-renderer-v8/src/lib.rs | 1 + .../src/native_bridge/context_host/core.rs | 2 - .../context_host/event_callbacks.rs | 53 ----- .../src/native_bridge/context_host/mod.rs | 2 - .../tests/broadcast_channel_delivery.rs | 8 +- .../page_vm/tests/child_document_lifecycle.rs | 8 +- .../runtime/page_vm/tests/child_host_load.rs | 10 +- .../child_module_document_script_ready.rs | 6 +- .../tests/child_modulepreload_event_action.rs | 8 +- .../tests/dedicated_worker_client_event.rs | 9 +- .../page_vm/tests/element_toggle_event.rs | 8 +- .../page_vm/tests/hash_change_delivery.rs | 8 +- .../page_vm/tests/history_traversal.rs | 8 +- .../runtime/page_vm/tests/image_load_event.rs | 11 +- .../src/runtime/page_vm/tests/lifecycle.rs | 6 +- .../main_document_post_parse_completion.rs | 6 +- .../tests/main_runtime_module_completion.rs | 8 +- .../page_vm/tests/media_element_event.rs | 8 +- .../page_vm/tests/navigation_api_task.rs | 11 +- .../runtime/page_vm/tests/rendering_update.rs | 8 +- .../page_vm/tests/script_preparation_error.rs | 6 +- .../tests/service_worker_client_message.rs | 12 +- .../page_vm/tests/service_worker_internal.rs | 4 +- .../tests/shared_worker_client_event.rs | 8 +- .../page_vm/tests/storage_event_delivery.rs | 8 +- .../runtime/page_vm/tests/stylesheet_task.rs | 8 +- .../runtime/page_vm/tests/user_interaction.rs | 8 +- .../page_vm/tests/websocket/completion.rs | 6 +- moli-renderer-v8/src/runtime/tests.rs | 2 +- moli-renderer-v8/src/script_cleanup.rs | 99 ++++++++++ .../src/script_vm/classic_script_exception.rs | 2 + moli-renderer-v8/src/script_vm/eval_exec.rs | 11 +- .../src/script_vm/runtime_bindings.rs | 4 + .../src/script_vm/script_event_body.rs | 3 + .../src/script_vm/tests/post_parse.rs | 6 +- .../src/script_vm/tests/rendering_update.rs | 12 +- .../src/script_vm/window_message.rs | 6 +- moli-renderer-v8/src/window_host.rs | 2 - moli-renderer-v8/src/worker/mod.rs | 2 + .../src/worker/thread/dispatch.rs | 65 ++++-- moli-renderer-v8/src/worker/thread/mod.rs | 4 + .../fileapi/drag-operation-mask-basic.html | 6 +- .../messagechannel-close-event-basic.html | 2 +- .../worker-messageport-close-event-basic.html | 2 +- 53 files changed, 551 insertions(+), 370 deletions(-) create mode 100644 moli-core/tests/web_apis/callback_cleanup.rs create mode 100644 moli-renderer-v8/src/script_cleanup.rs diff --git a/moli-benchmark/wpt-cross-current/failed-cases.txt b/moli-benchmark/wpt-cross-current/failed-cases.txt index 1b5217acd8..a5512a7160 100644 --- a/moli-benchmark/wpt-cross-current/failed-cases.txt +++ b/moli-benchmark/wpt-cross-current/failed-cases.txt @@ -2949,9 +2949,6 @@ html/semantics/scripting-1/the-script-element/execution-timing/085.html html/semantics/scripting-1/the-script-element/execution-timing/128.html html/semantics/scripting-1/the-script-element/execution-timing/137.html html/semantics/scripting-1/the-script-element/json-module/json-module-service-worker-test.https.html -html/semantics/scripting-1/the-script-element/microtasks/checkpoint-after-window-onerror-module.html -html/semantics/scripting-1/the-script-element/microtasks/checkpoint-after-window-onerror.html -html/semantics/scripting-1/the-script-element/microtasks/checkpoint-after-workerglobalscope-onerror.html html/semantics/scripting-1/the-script-element/module/dynamic-import/code-cache-base-url.html html/semantics/scripting-1/the-script-element/module/inline-async-execorder.html html/semantics/scripting-1/the-script-element/moving-between-documents/ordering/delay-load-event-1.html diff --git a/moli-benchmark/wpt-cross-current/passed-cases.txt b/moli-benchmark/wpt-cross-current/passed-cases.txt index f0571e2388..787dc3246c 100644 --- a/moli-benchmark/wpt-cross-current/passed-cases.txt +++ b/moli-benchmark/wpt-cross-current/passed-cases.txt @@ -6975,6 +6975,9 @@ html/semantics/scripting-1/the-script-element/json-module/parse-error.html html/semantics/scripting-1/the-script-element/json-module/script-element-json-src.html html/semantics/scripting-1/the-script-element/json-module/valid-content-type.html html/semantics/scripting-1/the-script-element/load-error-events-1.html +html/semantics/scripting-1/the-script-element/microtasks/checkpoint-after-window-onerror-module.html +html/semantics/scripting-1/the-script-element/microtasks/checkpoint-after-window-onerror.html +html/semantics/scripting-1/the-script-element/microtasks/checkpoint-after-workerglobalscope-onerror.html html/semantics/scripting-1/the-script-element/microtasks/evaluation-order-1.html html/semantics/scripting-1/the-script-element/microtasks/evaluation-order-2.html html/semantics/scripting-1/the-script-element/microtasks/evaluation-order-3.html diff --git a/moli-core/tests/scripts/child_readiness.rs b/moli-core/tests/scripts/child_readiness.rs index 65b4528fe9..93e4dc2109 100644 --- a/moli-core/tests/scripts/child_readiness.rs +++ b/moli-core/tests/scripts/child_readiness.rs @@ -97,6 +97,9 @@ async fn child_stream_readiness_probe(operation: &str) -> Result setTimeout(resolve, 0)); const doc = frame.contentDocument; const win = frame.contentWindow; const oldBody = doc.body; diff --git a/moli-core/tests/web_apis.rs b/moli-core/tests/web_apis.rs index c1188cd605..ef10d537ca 100644 --- a/moli-core/tests/web_apis.rs +++ b/moli-core/tests/web_apis.rs @@ -13,6 +13,8 @@ use moli_fetch::FetchConfig; use support::FixtureServer; use tokio::time::Duration; +#[path = "web_apis/callback_cleanup.rs"] +mod callback_cleanup; #[path = "web_apis/event_dispatch.rs"] mod event_dispatch; diff --git a/moli-core/tests/web_apis/callback_cleanup.rs b/moli-core/tests/web_apis/callback_cleanup.rs new file mode 100644 index 0000000000..81b235a49a --- /dev/null +++ b/moli-core/tests/web_apis/callback_cleanup.rs @@ -0,0 +1,174 @@ +use super::event_dispatch::run_probe; +use super::*; + +#[tokio::test(flavor = "multi_thread")] +async fn posted_message_callbacks_checkpoint_before_the_next_listener() -> Result<()> { + let server = FixtureServer::spawn().await?; + let browser = Browser::new(AppConfig::default())?; + let source = r#" + const log = []; + let dispatched; + addEventListener('nested', () => { + log.push('nested 1'); + Promise.resolve().then(() => log.push('nested 1 microtask')); + }); + addEventListener('nested', () => { + log.push('nested 2'); + Promise.resolve().then(() => log.push('nested 2 microtask')); + }); + addEventListener('message', event => { + dispatched = event; + log.push('first'); + Promise.resolve().then(() => { + log.push(['first microtask', event.currentTarget === self, event.eventPhase === 2, + typeof document === 'undefined' || window.event === event]); + dispatchEvent(new Event('nested')); + log.push('after nested'); + }); + }); + addEventListener('message', event => { + log.push('second'); + Promise.resolve().then(() => { + event.stopImmediatePropagation(); + log.push('second microtask'); + setTimeout(() => { + log.push(['after dispatch', dispatched.currentTarget === null, dispatched.eventPhase === 0, + typeof document === 'undefined' || window.event === undefined]); + finish(log); + }, 0); + }); + }); + addEventListener('message', () => log.push('unexpected third')); + if (typeof document !== 'undefined') postMessage('go', '*'); + "#; + let mut results = Vec::new(); + for target in ["window", "child", "worker"] { + results.push((target, run_probe(&browser, &server, target, source).await?)); + } + server.shutdown().await; + for (target, observed) in results { + assert_eq!( + observed, + serde_json::json!([ + "first", + ["first microtask", true, true, true], + "nested 1", + "nested 2", + "after nested", + "nested 1 microtask", + "nested 2 microtask", + "second", + "second microtask", + ["after dispatch", true, true, true] + ]), + "target={target}" + ); + } + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn callback_microtasks_observe_once_and_listener_removal() -> Result<()> { + let server = FixtureServer::spawn().await?; + let browser = Browser::new(AppConfig::default())?; + let source = r#" + const log = []; + const removed = () => log.push('unexpected removed listener'); + addEventListener('message', () => { + log.push('once'); + Promise.resolve().then(() => { + log.push('once microtask'); + removeEventListener('message', removed); + dispatchEvent(new Event('message')); + }); + }, {once: true}); + addEventListener('message', removed); + addEventListener('message', () => { + log.push('retained'); + setTimeout(() => finish(log), 0); + }); + if (typeof document !== 'undefined') postMessage('go', '*'); + "#; + let mut results = Vec::new(); + for target in ["window", "child", "worker"] { + results.push((target, run_probe(&browser, &server, target, source).await?)); + } + server.shutdown().await; + for (target, observed) in results { + assert_eq!( + observed, + serde_json::json!(["once", "once microtask", "retained", "retained"]), + "target={target}" + ); + } + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn callback_and_script_exceptions_keep_their_distinct_cleanup_boundaries() -> Result<()> { + let server = FixtureServer::spawn().await?; + let browser = Browser::new(AppConfig::default())?; + let mut results = Vec::new(); + for target in ["window", "child", "worker"] { + for from_callback in [false, true] { + let source = format!( + r#" + const log = []; + onerror = () => {{ + log.push('error 1'); + Promise.resolve().then(() => log.push('error 1 microtask')); + return true; + }}; + addEventListener('error', () => {{ + log.push('error 2'); + Promise.resolve().then(() => log.push('error 2 microtask')); + setTimeout(() => finish(log), 0); + }}); + const fail = () => {{ + log.push('body'); + Promise.resolve().then(() => log.push('body microtask')); + throw new Error('cleanup boundary'); + }}; + if ({from_callback}) {{ + addEventListener('message', fail); + if (typeof document !== 'undefined') postMessage('go', '*'); + }} else {{ + fail(); + }} + "# + ); + results.push(( + target, + from_callback, + run_probe(&browser, &server, target, &source).await?, + )); + } + } + server.shutdown().await; + for (target, from_callback, observed) in results { + let expected = if from_callback { + serde_json::json!([ + "body", + "body microtask", + "error 1", + "error 1 microtask", + "error 2", + "error 2 microtask" + ]) + } else { + serde_json::json!([ + "body", + "error 1", + "error 2", + "body microtask", + "error 1 microtask", + "error 2 microtask" + ]) + }; + assert_eq!( + observed, expected, + "target={target}, from_callback={from_callback}" + ); + } + Ok(()) +} diff --git a/moli-renderer-v8/src/callback_invocation.rs b/moli-renderer-v8/src/callback_invocation.rs index 8ef53ac6d9..8c33e44953 100644 --- a/moli-renderer-v8/src/callback_invocation.rs +++ b/moli-renderer-v8/src/callback_invocation.rs @@ -199,7 +199,6 @@ impl CallbackInvoker { log_level, callback_name, invocation, - false, |_scope, outcome| outcome, ) } @@ -213,26 +212,15 @@ impl CallbackInvoker { invocation: CallbackInvocation<'s, '_>, complete: impl FnOnce(&mut v8::PinScope<'s, '_>, CallbackInvocationOutcome) -> R, ) -> R { - let host_ptr = invocation.host_ptr; - let event_callback_scope = host_ptr - .map(|host_ptr| unsafe { &mut *host_ptr }.enter_event_callback_invocation_scope()); - let defer_window_event_restore = event_callback_scope - .as_ref() - .is_some_and(|event_scope| event_scope.is_outermost()) - && host_ptr - .is_some_and(|host_ptr| !unsafe { &*host_ptr }.explicit_event_dispatch_is_active()); - let result = Self::invoke_with_completion( + Self::invoke_with_completion( scope, callback_kind, log_label, log_level, callback_name, invocation, - defer_window_event_restore, complete, - ); - drop(event_callback_scope); - result + ) } #[allow(clippy::too_many_arguments)] @@ -243,7 +231,6 @@ impl CallbackInvoker { log_level: CallbackExceptionLogLevel, callback_name: &str, invocation: CallbackInvocation<'s, '_>, - defer_window_event_restore: bool, complete: impl FnOnce(&mut v8::PinScope<'s, '_>, CallbackInvocationOutcome) -> R, ) -> R { if let Some(host_ptr) = invocation.host_ptr { @@ -256,92 +243,92 @@ impl CallbackInvoker { return complete(scope, CallbackInvocationOutcome::Retired); } - with_webidl_callback_contexts( - scope, - invocation.relevant_context, - invocation.incumbent_context, - |scope| { - let relevant_dispatch_scope = invocation - .relevant_identity - .map(WindowExecutionContextIdentity::dispatch_scope); - let previous_relevant_dispatch_scope = - relevant_dispatch_scope.map(|dispatch_scope| dispatch_scope.enter(scope)); - let relevant_context = invocation.relevant_context; - let previous_window_event = - invocation - .host_ptr - .and(invocation.current_event) - .map(|event| { - let global = relevant_context.global(scope); - let event_key = v8str(scope, WINDOW_EVENT_SLOT); - let previous = global - .get(scope, event_key.into()) - .unwrap_or_else(|| v8::undefined(scope).into()); - let _ = global.set(scope, event_key.into(), event.into()); - previous - }); - - let webidl_invocation = WebIdlCallbackInvocation::new( - invocation.callback, - invocation.callback_this, - invocation.is_callable, - invocation.operation_name, - invocation.arguments, - ); - let result = invoke_webidl_callback( - scope, - webidl_invocation, - |scope, callback, receiver, arguments| { - invoke_callback_with_report( - scope, - callback_kind, - log_label, - log_level, - callback_name, - callback, - receiver, - arguments, - ) - }, - |scope, failure| { - capture_callback_resolution_failure( - scope, - log_label, - log_level, - callback_name, - failure, - ) - }, - ); - - let outcome = match result { - Ok(value) => CallbackInvocationOutcome::Returned(value), - Err(report) => CallbackInvocationOutcome::Threw(report), - }; - let completed = complete(scope, outcome); - - if let Some(previous) = previous_window_event { - if defer_window_event_restore && let Some(host_ptr) = invocation.host_ptr { - crate::context_bootstrap::enqueue_window_event_restore_after_microtask_checkpoint( - scope, - host_ptr, - invocation.relevant_identity, - relevant_context, - previous, - ); - } else { + { + let scope = &mut v8::ContextScope::new(scope, invocation.relevant_context); + let relevant_dispatch_scope = invocation + .relevant_identity + .map(WindowExecutionContextIdentity::dispatch_scope); + let previous_relevant_dispatch_scope = + relevant_dispatch_scope.map(|dispatch_scope| dispatch_scope.enter(scope)); + let relevant_context = invocation.relevant_context; + let previous_window_event = + invocation + .host_ptr + .and(invocation.current_event) + .map(|event| { let global = relevant_context.global(scope); - let _ = global.set(scope, v8str(scope, WINDOW_EVENT_SLOT).into(), previous); - } - } - if let (Some(dispatch_scope), Some(previous_dispatch_scope)) = - (relevant_dispatch_scope, previous_relevant_dispatch_scope) - { - dispatch_scope.restore(scope, previous_dispatch_scope); - } - completed - }, - ) + let event_key = v8str(scope, WINDOW_EVENT_SLOT); + let previous = global + .get(scope, event_key.into()) + .unwrap_or_else(|| v8::undefined(scope).into()); + let _ = global.set(scope, event_key.into(), event.into()); + previous + }); + + let webidl_invocation = WebIdlCallbackInvocation::new( + invocation.callback, + invocation.callback_this, + invocation.is_callable, + invocation.operation_name, + invocation.arguments, + ); + let execution_scope = crate::script_cleanup::ScriptExecutionScope::enter(scope); + let result = with_webidl_callback_contexts( + scope, + invocation.relevant_context, + invocation.incumbent_context, + |scope| { + invoke_webidl_callback( + scope, + webidl_invocation, + |scope, callback, receiver, arguments| { + invoke_callback_with_report( + scope, + callback_kind, + log_label, + log_level, + callback_name, + callback, + receiver, + arguments, + ) + }, + |scope, failure| { + capture_callback_resolution_failure( + scope, + log_label, + log_level, + callback_name, + failure, + ) + }, + ) + }, + ); + drop(execution_scope); + // Web IDL cleans up the callback and script before returning + // an abrupt completion to DOM's exception-reporting steps. + // Keep currentTarget, passive state, and window.event alive + // throughout that checkpoint. + crate::script_cleanup::perform_callback_cleanup_checkpoint(scope); + + let outcome = match result { + Ok(value) => CallbackInvocationOutcome::Returned(value), + Err(report) => CallbackInvocationOutcome::Threw(report), + }; + let completed = complete(scope, outcome); + + if let Some(previous) = previous_window_event { + let global = relevant_context.global(scope); + let _ = global.set(scope, v8str(scope, WINDOW_EVENT_SLOT).into(), previous); + } + if let (Some(dispatch_scope), Some(previous_dispatch_scope)) = + (relevant_dispatch_scope, previous_relevant_dispatch_scope) + { + dispatch_scope.restore(scope, previous_dispatch_scope); + } + completed + } } } diff --git a/moli-renderer-v8/src/context_bootstrap.rs b/moli-renderer-v8/src/context_bootstrap.rs index 169e00d02f..a05c8d9b45 100644 --- a/moli-renderer-v8/src/context_bootstrap.rs +++ b/moli-renderer-v8/src/context_bootstrap.rs @@ -326,7 +326,6 @@ pub(crate) use self::message_ports::{ }; use self::message_ports::{schedule_host_callback, schedule_scope_callback}; pub(crate) use self::microtask_checkpoint::{ - enqueue_window_event_restore_after_microtask_checkpoint, install_agent_microtask_checkpoint_tasks, run_end_of_microtask_checkpoint_tasks, }; pub(crate) use self::navigation_bootstrap::{ diff --git a/moli-renderer-v8/src/context_bootstrap/media_queries/events/simple_event_target/callbacks.rs b/moli-renderer-v8/src/context_bootstrap/media_queries/events/simple_event_target/callbacks.rs index 68d23591ff..1facc61bc9 100644 --- a/moli-renderer-v8/src/context_bootstrap/media_queries/events/simple_event_target/callbacks.rs +++ b/moli-renderer-v8/src/context_bootstrap/media_queries/events/simple_event_target/callbacks.rs @@ -1,5 +1,4 @@ use super::*; -use crate::util::context_host_ptr_from_global_bridge; pub(crate) fn simple_event_target_add_event_listener_callback<'s>( scope: &mut v8::PinScope<'s, '_>, @@ -38,7 +37,5 @@ pub(crate) fn simple_event_target_dispatch_event_callback<'s>( rv.set(v8::Boolean::new(scope, true).into()); return; }; - let _explicit_dispatch_scope = context_host_ptr_from_global_bridge(scope) - .map(|host_ptr| unsafe { &mut *host_ptr }.enter_explicit_event_dispatch_scope()); simple_object_event_target_dispatch(scope, &args, slot_name, &mut rv); } diff --git a/moli-renderer-v8/src/context_bootstrap/microtask_checkpoint.rs b/moli-renderer-v8/src/context_bootstrap/microtask_checkpoint.rs index 6e6dfaf0bd..f520fbdf56 100644 --- a/moli-renderer-v8/src/context_bootstrap/microtask_checkpoint.rs +++ b/moli-renderer-v8/src/context_bootstrap/microtask_checkpoint.rs @@ -10,38 +10,6 @@ enum AgentMicrotaskCheckpointTask { context: v8::Global, transaction: v8::Global, }, - RestoreWindowEvent { - host_ptr: *mut crate::native_bridge::JsContextHost, - relevant_identity: Option, - context: v8::Global, - previous: v8::Global, - }, -} - -pub(crate) fn enqueue_window_event_restore_after_microtask_checkpoint( - scope: &mut v8::PinScope<'_, '_>, - host_ptr: *mut crate::native_bridge::JsContextHost, - relevant_identity: Option, - context: v8::Local<'_, v8::Context>, - previous: v8::Local<'_, v8::Value>, -) { - if scope.get_slot::().is_none() { - assert!( - scope.set_slot(AgentMicrotaskCheckpointTasks::default()), - "agent checkpoint state should be installed once on first use" - ); - } - let task = AgentMicrotaskCheckpointTask::RestoreWindowEvent { - host_ptr, - relevant_identity, - context: v8::Global::new(scope, context), - previous: v8::Global::new(scope, previous), - }; - scope - .get_slot_mut::() - .expect("agent checkpoint state should exist after installation") - .tasks - .push(task); } pub(crate) fn install_agent_microtask_checkpoint_tasks(isolate: &mut v8::Isolate) { @@ -82,27 +50,14 @@ pub(crate) fn run_end_of_microtask_checkpoint_tasks(scope: &mut v8::PinScope<'_, return; }; - let mut window_event_restores = Vec::new(); for task in tasks { match task { AgentMicrotaskCheckpointTask::DeactivateIndexedDbTransaction { context, transaction, } => run_indexed_db_transaction_deactivation(scope, context, transaction), - AgentMicrotaskCheckpointTask::RestoreWindowEvent { - host_ptr, - relevant_identity, - context, - previous, - } => { - window_event_restores.push((host_ptr, relevant_identity, context, previous)); - } } } - for (host_ptr, relevant_identity, context, previous) in window_event_restores.into_iter().rev() - { - restore_window_event(scope, host_ptr, relevant_identity, context, previous); - } } fn run_indexed_db_transaction_deactivation( @@ -118,26 +73,3 @@ fn run_indexed_db_transaction_deactivation( transaction, ); } - -fn restore_window_event( - scope: &mut v8::PinScope<'_, '_>, - host_ptr: *mut crate::native_bridge::JsContextHost, - relevant_identity: Option, - context: v8::Global, - previous: v8::Global, -) { - if relevant_identity.is_some_and(|identity| { - !unsafe { &*host_ptr }.window_execution_context_identity_is_current(identity) - }) { - return; - } - let context = v8::Local::new(scope, &context); - let scope = &mut v8::ContextScope::new(scope, context); - let global = context.global(scope); - let previous = v8::Local::new(scope, &previous); - let _ = global.set( - scope, - crate::util::v8str(scope, crate::host::WINDOW_EVENT_SLOT).into(), - previous, - ); -} diff --git a/moli-renderer-v8/src/lib.rs b/moli-renderer-v8/src/lib.rs index c7f7359912..f1b39d1d62 100644 --- a/moli-renderer-v8/src/lib.rs +++ b/moli-renderer-v8/src/lib.rs @@ -92,6 +92,7 @@ mod resource_owner; mod resource_ready; mod runtime; mod runtime_binding_data; +mod script_cleanup; mod script_execution; mod script_execution_control; mod script_provenance; 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 530f5e6f32..326974d28f 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/core.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/core.rs @@ -373,8 +373,6 @@ impl JsContextHost { child_window_event_listeners: HashMap::new(), next_child_window_event_registration_id: 0, event_callbacks: Default::default(), - explicit_event_dispatch_depth: 0, - event_callback_invocation_depth: 0, active_window_error_report_owners: HashSet::new(), browser_context_runtime, top_level_navigation_handoff_tx, diff --git a/moli-renderer-v8/src/native_bridge/context_host/event_callbacks.rs b/moli-renderer-v8/src/native_bridge/context_host/event_callbacks.rs index d7cba57beb..9062756c3b 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/event_callbacks.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/event_callbacks.rs @@ -22,41 +22,6 @@ pub(crate) struct WindowErrorReportingScope { owner: WindowExecutionContextOwner, } -pub(crate) struct ExplicitEventDispatchScope { - host_ptr: *mut JsContextHost, -} - -pub(crate) struct EventCallbackInvocationScope { - host_ptr: *mut JsContextHost, - outermost: bool, -} - -impl EventCallbackInvocationScope { - pub(crate) fn is_outermost(&self) -> bool { - self.outermost - } -} - -impl Drop for EventCallbackInvocationScope { - fn drop(&mut self) { - let host = unsafe { &mut *self.host_ptr }; - host.event_callback_invocation_depth = host - .event_callback_invocation_depth - .checked_sub(1) - .expect("event callback invocation scope must be active"); - } -} - -impl Drop for ExplicitEventDispatchScope { - fn drop(&mut self) { - let host = unsafe { &mut *self.host_ptr }; - host.explicit_event_dispatch_depth = host - .explicit_event_dispatch_depth - .checked_sub(1) - .expect("explicit EventTarget dispatch scope must be active"); - } -} - impl Drop for WindowErrorReportingScope { fn drop(&mut self) { let removed = unsafe { &mut *self.host_ptr } @@ -129,24 +94,6 @@ impl EventCallbackRegistry { } impl JsContextHost { - pub(crate) fn enter_event_callback_invocation_scope(&mut self) -> EventCallbackInvocationScope { - let outermost = self.event_callback_invocation_depth == 0; - self.event_callback_invocation_depth += 1; - EventCallbackInvocationScope { - host_ptr: self, - outermost, - } - } - - pub(crate) fn enter_explicit_event_dispatch_scope(&mut self) -> ExplicitEventDispatchScope { - self.explicit_event_dispatch_depth += 1; - ExplicitEventDispatchScope { host_ptr: self } - } - - pub(crate) fn explicit_event_dispatch_is_active(&self) -> bool { - self.explicit_event_dispatch_depth != 0 - } - pub(crate) fn enter_window_error_reporting_scope( &mut self, owner: WindowExecutionContextOwner, diff --git a/moli-renderer-v8/src/native_bridge/context_host/mod.rs b/moli-renderer-v8/src/native_bridge/context_host/mod.rs index 60187a58fa..256f3e179d 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/mod.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/mod.rs @@ -988,8 +988,6 @@ pub(crate) struct JsContextHost { HashMap>>, next_child_window_event_registration_id: u64, event_callbacks: event_callbacks::EventCallbackRegistry, - explicit_event_dispatch_depth: usize, - event_callback_invocation_depth: usize, active_window_error_report_owners: HashSet, browser_context_runtime: crate::runtime::RendererBrowserContextRuntime, top_level_navigation_handoff_tx: diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/broadcast_channel_delivery.rs b/moli-renderer-v8/src/runtime/page_vm/tests/broadcast_channel_delivery.rs index 84d5d5a442..df771f9c62 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/broadcast_channel_delivery.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/broadcast_channel_delivery.rs @@ -19,7 +19,7 @@ fn take_next_broadcast_channel_task_for_authorization_test( } #[tokio::test(flavor = "current_thread")] -async fn broadcast_channel_body_leaves_reactions_and_runtime_scripts_for_selected_completion() { +async fn broadcast_channel_body_cleans_up_callbacks_before_selected_completion() { run_page_vm_async_test(async move { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); @@ -54,15 +54,15 @@ __broadcastBodySender.postMessage("go"); ); assert_eq!( page_vm.vm_mut().eval("__broadcastBodyBoundary.join('|')")?, - "callback", - "the body-only executor must leave Promise reactions pending" + "callback|microtask|runtime-script", + "listener cleanup must drain reactions before the selected task completes" ); page_vm.finish_selected_page_callback_task(&loader).await?; assert_eq!( page_vm.vm_mut().eval("__broadcastBodyBoundary.join('|')")?, "callback|microtask|runtime-script", - "selected callback completion must own checkpoint and runtime-script follow-up" + "selected task completion must not repeat callback reactions or their inline scripts" ); Ok::<_, anyhow::Error>(()) }) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/child_document_lifecycle.rs b/moli-renderer-v8/src/runtime/page_vm/tests/child_document_lifecycle.rs index 7d2b240ab9..b62181bc2a 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/child_document_lifecycle.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/child_document_lifecycle.rs @@ -49,7 +49,7 @@ async fn install_child_document_lifecycle_fixture( } #[tokio::test(flavor = "current_thread")] -async fn child_document_lifecycle_body_leaves_reactions_for_selected_completion() { +async fn child_document_lifecycle_body_cleans_up_listener_reactions() { run_page_vm_async_test(async move { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); let document_url = Url::parse("https://example.com/child-document-lifecycle-body").unwrap(); @@ -70,8 +70,8 @@ async fn child_document_lifecycle_body_leaves_reactions_for_selected_completion( .eval_without_microtask_checkpoint_for_test( "__lmChildDocumentLifecycleBoundary.join('|')" )?, - "callback:interactive", - "the lifecycle body must leave listener reactions pending for selected completion" + "callback:interactive|microtask:interactive", + "listener cleanup must drain reactions before the selected task completes" ); Ok::<_, anyhow::Error>(()) }) @@ -116,7 +116,7 @@ async fn selected_child_document_lifecycle_completes_each_event_reaction_and_run "__lmChildDocumentLifecycleBoundary.join('|')" )?, expected, - "each selected lifecycle task must own its listener-reaction checkpoint" + "each lifecycle callback must finish its reactions before the next lifecycle task" ); if index == 0 { assert!( diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/child_host_load.rs b/moli-renderer-v8/src/runtime/page_vm/tests/child_host_load.rs index 86dae435d4..d7889b4ae8 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/child_host_load.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/child_host_load.rs @@ -47,7 +47,7 @@ async fn install_child_host_load_completion_fixture( } #[tokio::test(flavor = "current_thread")] -async fn child_host_load_body_leaves_reactions_for_selected_completion() { +async fn child_host_load_body_cleans_up_listener_reactions() { run_page_vm_async_test(async move { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); @@ -69,8 +69,8 @@ async fn child_host_load_body_leaves_reactions_for_selected_completion() { .eval_without_microtask_checkpoint_for_test( "__lmChildHostLoadTaskBoundary.join('|')" )?, - "callback", - "the HostLoad body must leave listener reactions pending for selected completion" + "callback|microtask", + "listener cleanup must drain reactions before the selected task completes" ); Ok::<_, anyhow::Error>(()) }) @@ -115,7 +115,7 @@ document.getElementById("host-load-body-replacement").onload = function () { .eval_without_microtask_checkpoint_for_test( "__lmChildHostLoadTaskBoundary.join('|')" )?, - "callback", + "callback|microtask", "replacement must not erase the fact that the callback body already ran" ); Ok::<_, anyhow::Error>(()) @@ -153,7 +153,7 @@ async fn selected_child_host_load_completes_reactions_and_runtime_followup() { "__lmChildHostLoadTaskBoundary.join('|')" )?, "callback|microtask", - "selected HostLoad completion must own the listener-reaction checkpoint" + "the selected HostLoad task must include listener cleanup" ); assert!( has_ready_runtime_script_continuation_for_test(&page_vm), diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/child_module_document_script_ready.rs b/moli-renderer-v8/src/runtime/page_vm/tests/child_module_document_script_ready.rs index b530338b46..62db957e62 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/child_module_document_script_ready.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/child_module_document_script_ready.rs @@ -197,7 +197,7 @@ Promise.resolve().then(() => { } #[tokio::test(flavor = "current_thread")] -async fn child_module_ready_body_keeps_load_reaction_for_selected_task_completion() { +async fn child_module_ready_body_cleans_up_script_before_load_callback() { run_page_vm_async_test(async move { let (base_url, server) = spawn_path_response_http_server(vec![( "/child-module-task-boundary.js", @@ -234,8 +234,8 @@ Promise.resolve().then(() => { .eval_without_microtask_checkpoint_for_test( "__lmChildModuleTaskBoundary.join('|')" )?, - "module-body|module-microtask|script-load", - "module error-handling must run its algorithmic checkpoint before script load, but the load listener reaction belongs to selected task completion" + "module-body|module-microtask|script-load|load-microtask", + "module cleanup precedes script load, whose callback has its own cleanup checkpoint" ); server diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/child_modulepreload_event_action.rs b/moli-renderer-v8/src/runtime/page_vm/tests/child_modulepreload_event_action.rs index 4dba4a2163..2a750336e1 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/child_modulepreload_event_action.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/child_modulepreload_event_action.rs @@ -89,7 +89,7 @@ fn take_child_modulepreload_event_action_body_task( } #[tokio::test(flavor = "current_thread")] -async fn child_modulepreload_event_body_leaves_reactions_for_selected_completion() { +async fn child_modulepreload_event_body_cleans_up_listener_reactions() { run_page_vm_async_test(async move { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); @@ -121,8 +121,8 @@ async fn child_modulepreload_event_body_leaves_reactions_for_selected_completion .eval_without_microtask_checkpoint_for_test( "__lmChildModulepreloadTaskBoundary.join('|')" )?, - "callback", - "the modulepreload event body must leave listener reactions pending" + "callback|microtask", + "listener cleanup must drain reactions before the selected task completes" ); Ok::<_, anyhow::Error>(()) }) @@ -164,7 +164,7 @@ async fn selected_child_modulepreload_event_completes_reactions_and_runtime_foll "__lmChildModulepreloadTaskBoundary.join('|')" )?, "callback|microtask", - "selected completion must own the listener-reaction checkpoint" + "the selected modulepreload task must include listener cleanup" ); assert!( has_ready_runtime_script_continuation_for_test(&page_vm), diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/dedicated_worker_client_event.rs b/moli-renderer-v8/src/runtime/page_vm/tests/dedicated_worker_client_event.rs index 57034ecb13..a98cf73e70 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/dedicated_worker_client_event.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/dedicated_worker_client_event.rs @@ -20,8 +20,7 @@ fn current_single_child_document_owner( } #[tokio::test(flavor = "current_thread")] -async fn dedicated_worker_message_body_leaves_reactions_and_runtime_scripts_for_selected_completion() - { +async fn dedicated_worker_message_body_cleans_up_callbacks_before_selected_completion() { run_page_vm_async_test(async move { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); @@ -81,8 +80,8 @@ __dedicatedWorkerTaskBoundaryWorker.onmessage = () => { page_vm .vm_mut() .eval("__dedicatedWorkerTaskBoundary.join('|')")?, - "callback", - "the DedicatedWorker message body must leave listener reactions pending" + "callback|microtask|runtime-script", + "listener cleanup must drain reactions before the selected task completes" ); assert_eq!( page_vm @@ -103,7 +102,7 @@ __dedicatedWorkerTaskBoundaryWorker.onmessage = () => { .vm_mut() .eval("__dedicatedWorkerTaskBoundary.join('|')")?, "callback|microtask|runtime-script", - "selected completion must own the checkpoint and runtime follow-up" + "selected task completion must not repeat callback reactions or their inline scripts" ); Ok::<_, anyhow::Error>(()) }) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/element_toggle_event.rs b/moli-renderer-v8/src/runtime/page_vm/tests/element_toggle_event.rs index 4ad12d8c16..c8571cf2e0 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/element_toggle_event.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/element_toggle_event.rs @@ -17,7 +17,7 @@ fn take_next_element_toggle_task_for_test( } #[tokio::test(flavor = "current_thread")] -async fn element_toggle_body_leaves_reactions_and_runtime_scripts_for_selected_completion() { +async fn element_toggle_body_cleans_up_callbacks_before_selected_completion() { run_page_vm_async_test(async move { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); @@ -52,15 +52,15 @@ details.open = true; ); assert_eq!( page_vm.vm_mut().eval("__elementToggleBoundary.join('|')")?, - "callback", - "the body-only executor must leave Promise reactions pending" + "callback|microtask|runtime-script", + "listener cleanup must drain reactions before the selected task completes" ); page_vm.finish_selected_page_callback_task(&loader).await?; assert_eq!( page_vm.vm_mut().eval("__elementToggleBoundary.join('|')")?, "callback|microtask|runtime-script", - "selected callback completion must own checkpoint and runtime-script follow-up" + "selected task completion must not repeat callback reactions or their inline scripts" ); Ok::<_, anyhow::Error>(()) }) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/hash_change_delivery.rs b/moli-renderer-v8/src/runtime/page_vm/tests/hash_change_delivery.rs index 6d0e2cfeb5..86cb528c97 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/hash_change_delivery.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/hash_change_delivery.rs @@ -62,7 +62,7 @@ location.hash = "#trusted"; } #[tokio::test(flavor = "current_thread")] -async fn hashchange_body_leaves_reactions_for_selected_callback_completion() { +async fn hashchange_body_cleans_up_callbacks_before_selected_completion() { run_page_vm_async_test(async move { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); @@ -95,8 +95,8 @@ location.hash = "#body"; page_vm .vm_mut() .eval("__hashChangeBodyBoundary.join('|')")?, - "callback:body", - "the body-only executor must leave Promise reactions pending" + "callback:body|microtask:body", + "listener cleanup must drain reactions before the selected task completes" ); page_vm.finish_selected_page_callback_task(&loader).await?; @@ -105,7 +105,7 @@ location.hash = "#body"; .vm_mut() .eval("__hashChangeBodyBoundary.join('|')")?, "callback:body|microtask:body", - "the selected callback completion must own the single task checkpoint" + "selected task completion must not repeat callback reactions or their inline scripts" ); Ok::<_, anyhow::Error>(()) }) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/history_traversal.rs b/moli-renderer-v8/src/runtime/page_vm/tests/history_traversal.rs index 425cc60643..bf22fdfa94 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/history_traversal.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/history_traversal.rs @@ -49,7 +49,7 @@ history.back(); } #[tokio::test(flavor = "current_thread")] -async fn history_traversal_body_leaves_reaction_for_selected_completion() { +async fn history_traversal_body_cleans_up_callbacks_before_selected_completion() { run_page_vm_async_test(async move { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); @@ -93,8 +93,8 @@ history.back(); .eval_without_microtask_checkpoint_for_test( "globalThis.__historyBodyBoundary.join('|')", )?, - "navigate", - "the traversal body must leave its Promise reaction for selected-task completion" + "navigate|microtask|runtime-script", + "listener cleanup must drain reactions before the selected task completes" ); let completion = outcome.action.into_page_task_completion(); assert!(matches!(completion, PageTaskCompletion::CallbackCompletion)); @@ -106,7 +106,7 @@ history.back(); .vm_mut() .eval("globalThis.__historyBodyBoundary.join('|')")?, "navigate|microtask|runtime-script", - "central callback completion must own the reaction and its runtime-script follow-up" + "selected task completion must not repeat callback reactions or their inline scripts" ); Ok::<_, anyhow::Error>(()) }) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/image_load_event.rs b/moli-renderer-v8/src/runtime/page_vm/tests/image_load_event.rs index 486259e2de..b39c05e926 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/image_load_event.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/image_load_event.rs @@ -17,9 +17,10 @@ fn take_next_image_load_event_task_for_test( } #[tokio::test(flavor = "current_thread")] -async fn image_event_body_leaves_reactions_and_runtime_scripts_for_selected_completion() { +async fn image_event_body_cleans_up_callbacks_and_decode_reactions() { run_page_vm_async_test(async move { - let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); + let loader = + crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); let document_url = Url::parse("https://example.com/image-body-boundary").unwrap(); let (mut page_vm, _resource_source, _owner_wake_rx) = page_vm_with_bound_task_sources_and_owner_wake(&loader, document_url); @@ -55,15 +56,15 @@ image.src = "data:image/gif;base64,R0lGODlhAQABAIAAAAAAAP///yH5BAEAAAAALAAAAAABA ); assert_eq!( page_vm.vm_mut().eval("__imageTaskBoundary.join('|')")?, - "callback", - "the image body must leave listener and image.decode() reactions pending" + "callback|microtask|runtime-script|decode", + "listener cleanup must drain reactions before the selected task completes" ); page_vm.finish_selected_page_callback_task(&loader).await?; assert_eq!( page_vm.vm_mut().eval("__imageTaskBoundary.join('|')")?, "callback|microtask|runtime-script|decode", - "selected image completion must own decode/listener reactions and runtime-script follow-up" + "selected task completion must not repeat callback reactions or their inline scripts" ); Ok::<_, anyhow::Error>(()) }) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/lifecycle.rs b/moli-renderer-v8/src/runtime/page_vm/tests/lifecycle.rs index b11cd6a86b..33edf6593c 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/lifecycle.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/lifecycle.rs @@ -1581,7 +1581,7 @@ window.addEventListener("pageshow", () => { } #[test] -fn ordinary_main_lifecycle_body_leaves_listener_reaction_for_its_typed_checkpoint() { +fn ordinary_main_lifecycle_body_cleans_up_callbacks_before_its_typed_checkpoint() { run_page_vm_local_runtime_async_test( "page-vm-main-lifecycle-body-only-checkpoint", || async move { @@ -1621,8 +1621,8 @@ document.addEventListener("readystatechange", () => { .eval_without_microtask_checkpoint_for_test( "__mainLifecycleBodyBoundary.join('|')" )?, - "callback", - "the lifecycle body must not perform its typed task-end checkpoint" + "callback|microtask", + "listener cleanup must run while the typed task-end checkpoint remains pending" ); Ok::<_, anyhow::Error>(()) }) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/main_document_post_parse_completion.rs b/moli-renderer-v8/src/runtime/page_vm/tests/main_document_post_parse_completion.rs index 7db6f624f5..b7d7b0d30f 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/main_document_post_parse_completion.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/main_document_post_parse_completion.rs @@ -56,7 +56,7 @@ fn csp_violation_task( } #[tokio::test(flavor = "current_thread")] -async fn post_parse_callback_body_retains_reactions_for_selected_completion() { +async fn post_parse_callback_body_cleans_up_without_a_pre_task_checkpoint() { run_page_vm_async_test(async move { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); let mut page_vm = test_page_vm(); @@ -98,8 +98,8 @@ queueMicrotask(() => __postParseBodyOrder.push("preexisting")); page_vm.vm_mut().eval_without_microtask_checkpoint_for_test( "__postParseBodyOrder.join('|')", )?, - "callback", - "body execution must neither run the old BeforeTask checkpoint nor end the selected task" + "callback|preexisting|callback:microtask", + "callback cleanup drains existing reactions after the callback, without a BeforeTask checkpoint" ); Ok::<_, anyhow::Error>(()) }) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/main_runtime_module_completion.rs b/moli-renderer-v8/src/runtime/page_vm/tests/main_runtime_module_completion.rs index 199fc40b9c..ef26b2586a 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/main_runtime_module_completion.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/main_runtime_module_completion.rs @@ -690,7 +690,7 @@ async fn runtime_blob_module_dependency_resolves_through_the_local_url_owner() { } #[tokio::test(flavor = "current_thread")] -async fn runtime_module_failure_body_leaves_checkpoint_and_lifecycle_prime_to_task_end() { +async fn runtime_module_failure_body_cleans_up_callbacks_but_leaves_lifecycle_prime_to_task_end() { run_page_vm_async_test(async move { let (base_url, server) = spawn_path_response_http_server(vec![( "/runtime-failure.mjs", @@ -750,8 +750,8 @@ document.getElementById("runtime-module-failure").onerror = () => { .eval_without_microtask_checkpoint_for_test( "__runtimeModuleEvents.join('|')", )?, - "error", - "the graph-terminal body may dispatch its error but must not run the outer task checkpoint" + "error|error-microtask", + "the graph-terminal body must clean up its error listener before returning lifecycle-unblock authority" ); assert_eq!( page_vm @@ -770,7 +770,7 @@ document.getElementById("runtime-module-failure").onerror = () => { .page_task_queue .post_parse_front() .is_some_and(PostParsePageOwnedWork::is_window_load_task), - "the body must return lifecycle-unblock authority instead of priming load before the checkpoint" + "the body must return lifecycle-unblock authority for selected task completion" ); server diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/media_element_event.rs b/moli-renderer-v8/src/runtime/page_vm/tests/media_element_event.rs index c7fc5a7e46..4dbd041b19 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/media_element_event.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/media_element_event.rs @@ -5,7 +5,7 @@ use crate::page_task_queue::{ }; #[tokio::test(flavor = "current_thread")] -async fn media_element_event_body_leaves_reactions_and_runtime_scripts_for_selected_completion() { +async fn media_element_event_body_cleans_up_callbacks_before_selected_completion() { run_page_vm_async_test(async move { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); @@ -45,8 +45,8 @@ media.currentTime = 2; page_vm .vm_mut() .eval("__mediaEventTaskBoundary.join('|')")?, - "callback", - "the media-event body must leave listener reactions pending" + "callback|microtask|runtime-script", + "listener cleanup must drain reactions before the selected task completes" ); assert_eq!( page_vm @@ -67,7 +67,7 @@ media.currentTime = 2; .vm_mut() .eval("__mediaEventTaskBoundary.join('|')")?, "callback|microtask|runtime-script", - "selected completion must own the checkpoint and runtime follow-up" + "selected task completion must not repeat callback reactions or their inline scripts" ); Ok::<_, anyhow::Error>(()) }) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/navigation_api_task.rs b/moli-renderer-v8/src/runtime/page_vm/tests/navigation_api_task.rs index b7452ab087..9eced3169c 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/navigation_api_task.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/navigation_api_task.rs @@ -6,9 +6,10 @@ use crate::{ }; #[tokio::test(flavor = "current_thread")] -async fn navigation_api_task_body_leaves_reaction_for_selected_completion() { +async fn navigation_api_task_body_cleans_up_callbacks_before_selected_completion() { run_page_vm_async_test(async move { - let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); + let loader = + crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); let document_url = Url::parse("https://example.com/navigation-api-body-completion-boundary").unwrap(); let (mut page_vm, _resource_source, _owner_wake_rx) = @@ -45,8 +46,8 @@ navigation.navigate("#replacement"); .eval_without_microtask_checkpoint_for_test( "globalThis.__navigationApiBodyBoundary.join('|')", )?, - "success", - "the Navigation API task body must leave its Promise reaction for selected-task completion" + "success|microtask|runtime-script", + "listener cleanup must drain reactions before the selected task completes" ); let completion = outcome.action.into_page_task_completion(); assert!(matches!(completion, PageTaskCompletion::CallbackCompletion)); @@ -58,7 +59,7 @@ navigation.navigate("#replacement"); .vm_mut() .eval("globalThis.__navigationApiBodyBoundary.join('|')")?, "success|microtask|runtime-script", - "selected callback completion must own the reaction and runtime-script follow-up" + "selected task completion must not repeat callback reactions or their inline scripts" ); Ok::<_, anyhow::Error>(()) }) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update.rs b/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update.rs index b26979a9de..61178a846b 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update.rs @@ -5465,7 +5465,7 @@ document.close(); } #[tokio::test(flavor = "current_thread")] -async fn rendering_update_body_leaves_reactions_and_runtime_scripts_for_selected_completion() { +async fn rendering_update_body_cleans_up_callbacks_before_selected_completion() { run_page_vm_async_test(async move { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); @@ -5501,8 +5501,8 @@ scrollTo(0, 10); ); assert_eq!( page_vm.vm_mut().eval("__renderingTaskBoundary.join('|')")?, - "callback", - "the rendering-update body must leave listener reactions pending" + "callback|microtask|runtime-script", + "listener cleanup must drain reactions before the selected task completes" ); assert_eq!( page_vm @@ -5521,7 +5521,7 @@ scrollTo(0, 10); assert_eq!( page_vm.vm_mut().eval("__renderingTaskBoundary.join('|')")?, "callback|microtask|runtime-script", - "selected completion must own the checkpoint and runtime follow-up" + "selected task completion must not repeat callback reactions or their inline scripts" ); Ok::<_, anyhow::Error>(()) }) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/script_preparation_error.rs b/moli-renderer-v8/src/runtime/page_vm/tests/script_preparation_error.rs index 262b619b5d..9ea08b7b0f 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/script_preparation_error.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/script_preparation_error.rs @@ -70,15 +70,15 @@ onerror = message => { events.push(message.includes('listener sentinel') ? 'exce let first = take_error_task(&mut page_vm); let outcome = page_vm.apply_selected_page_script_preparation_error_turn(first)?; assert_eq!(outcome.action.target_effect, PageScriptPreparationErrorTargetEffect::DispatchedToCurrentOwner); - assert_eq!(page_vm.vm_mut().eval("events.join('|')")?, "broadcast|microtask:broadcast|first|exception", - "the body retains detached elements but leaves listener microtasks for task completion"); + assert_eq!(page_vm.vm_mut().eval("events.join('|')")?, "broadcast|microtask:broadcast|first|microtask:first|exception", + "the body retains detached elements and cleans up the listener before reporting its exception"); page_vm.finish_selected_page_callback_task(&loader).await?; assert!(page_vm.run_exact_selected_page_task_for_test( PageSelectedTaskTestSelector::DomManipulation(PageDomManipulationTestFamily::ScriptPreparationError), &loader, ).await?); assert_eq!(page_vm.vm_mut().eval("events.join('|')")?, - "broadcast|microtask:broadcast|first|exception|microtask:first|second|microtask:second"); + "broadcast|microtask:broadcast|first|microtask:first|exception|second|microtask:second"); let duplicate = page_vm.apply_selected_page_script_preparation_error_turn(first)?; assert!(matches!(duplicate.action.target_effect, diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/service_worker_client_message.rs b/moli-renderer-v8/src/runtime/page_vm/tests/service_worker_client_message.rs index d5daaecfb8..7f48e18de6 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/service_worker_client_message.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/service_worker_client_message.rs @@ -25,7 +25,7 @@ fn service_worker_client_message( } #[tokio::test(flavor = "current_thread")] -async fn service_worker_client_message_body_leaves_reactions_for_selected_completion() { +async fn service_worker_client_message_body_cleans_up_callbacks_before_selected_completion() { run_page_vm_async_test(async move { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); @@ -80,8 +80,8 @@ navigator.serviceWorker.onmessage = event => { page_vm .vm_mut() .eval("__serviceWorkerClientMessageEvents.join('|')")?, - "message:payload", - "the ServiceWorker message body must leave its Promise reaction pending" + "message:payload|microtask|runtime-script", + "listener cleanup must drain reactions before the selected task completes" ); assert_eq!( page_vm @@ -101,7 +101,7 @@ navigator.serviceWorker.onmessage = event => { .vm_mut() .eval("__serviceWorkerClientMessageEvents.join('|')")?, "message:payload|microtask|runtime-script", - "selected completion must own the checkpoint and runtime follow-up" + "selected task completion must not repeat callback reactions or their inline scripts" ); Ok::<_, anyhow::Error>(()) }) @@ -160,8 +160,8 @@ navigator.serviceWorker.onmessageerror = event => { page_vm .vm_mut() .eval("__serviceWorkerClientMessageErrorEvents.join('|')")?, - "messageerror", - "the messageerror body must leave its reaction pending" + "messageerror|microtask", + "listener cleanup must drain reactions before the selected task completes" ); page_vm diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/service_worker_internal.rs b/moli-renderer-v8/src/runtime/page_vm/tests/service_worker_internal.rs index 3ee8e4f75d..dd4068bf23 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/service_worker_internal.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/service_worker_internal.rs @@ -226,8 +226,8 @@ __serviceWorkerInternalRegistration.addEventListener("updatefound", () => { page_vm .vm_mut() .eval("__serviceWorkerInternalLifecycleEvents.join('|')")?, - "callback", - "the event body must leave its Promise reaction for selected completion" + "callback|microtask|runtime-script", + "listener cleanup must drain reactions before the selected task completes" ); assert_eq!( page_vm diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/shared_worker_client_event.rs b/moli-renderer-v8/src/runtime/page_vm/tests/shared_worker_client_event.rs index b93b810e79..e3c5d43101 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/shared_worker_client_event.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/shared_worker_client_event.rs @@ -74,7 +74,7 @@ pub(super) fn install_shared_worker_service_wake( } #[tokio::test(flavor = "current_thread")] -async fn shared_worker_error_body_leaves_reactions_for_selected_completion() { +async fn shared_worker_error_body_cleans_up_callbacks_before_selected_completion() { run_page_vm_async_test(async move { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); @@ -136,8 +136,8 @@ async fn shared_worker_error_body_leaves_reactions_for_selected_completion() { page_vm .vm_mut() .eval("__typedSharedWorkerEvents.join('|')")?, - "error:error", - "the SharedWorker error body must leave listener reactions pending" + "error:error|microtask|runtime-script", + "listener cleanup must drain reactions before the selected task completes" ); assert_eq!( page_vm @@ -162,7 +162,7 @@ async fn shared_worker_error_body_leaves_reactions_for_selected_completion() { .vm_mut() .eval("__typedSharedWorkerEvents.join('|')")?, "error:error|microtask|runtime-script", - "selected completion must own the checkpoint and runtime follow-up" + "selected task completion must not repeat callback reactions or their inline scripts" ); Ok::<_, anyhow::Error>(()) }) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/storage_event_delivery.rs b/moli-renderer-v8/src/runtime/page_vm/tests/storage_event_delivery.rs index bf07254a71..4b88338ae2 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/storage_event_delivery.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/storage_event_delivery.rs @@ -16,7 +16,7 @@ fn take_next_storage_event_task_for_authorization_test( } #[tokio::test(flavor = "current_thread")] -async fn storage_event_body_leaves_reactions_for_selected_callback_completion() { +async fn storage_event_body_cleans_up_callbacks_before_selected_completion() { run_page_vm_async_test(async move { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); @@ -61,8 +61,8 @@ document.getElementById("storage-event-body-source").contentWindow.localStorage page_vm .vm_mut() .eval("__storageEventBodyBoundary.join('|')")?, - "callback:one", - "the body-only executor must leave Promise reactions pending" + "callback:one|microtask:one", + "listener cleanup must drain reactions before the selected task completes" ); page_vm.finish_selected_page_callback_task(&loader).await?; @@ -71,7 +71,7 @@ document.getElementById("storage-event-body-source").contentWindow.localStorage .vm_mut() .eval("__storageEventBodyBoundary.join('|')")?, "callback:one|microtask:one", - "the selected callback completion must own the single task checkpoint" + "selected task completion must not repeat callback reactions or their inline scripts" ); Ok::<_, anyhow::Error>(()) }) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/stylesheet_task.rs b/moli-renderer-v8/src/runtime/page_vm/tests/stylesheet_task.rs index b0441ba7db..ff22430a76 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/stylesheet_task.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/stylesheet_task.rs @@ -1236,7 +1236,7 @@ document.write('(()) }) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/user_interaction.rs b/moli-renderer-v8/src/runtime/page_vm/tests/user_interaction.rs index cb19a100f6..ed9798bfbc 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/user_interaction.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/user_interaction.rs @@ -22,7 +22,7 @@ fn take_next_user_interaction_task_for_authorization_test( } #[tokio::test(flavor = "current_thread")] -async fn user_interaction_body_leaves_reactions_for_selected_callback_completion() { +async fn user_interaction_body_cleans_up_callbacks_before_selected_completion() { run_page_vm_async_test(async move { let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); @@ -57,8 +57,8 @@ dialog.close(); page_vm .vm_mut() .eval("__userInteractionBodyBoundary.join('|')")?, - "callback", - "the body-only executor must leave Promise reactions pending" + "callback|microtask", + "listener cleanup must drain reactions before the selected task completes" ); page_vm.finish_selected_page_callback_task(&loader).await?; @@ -67,7 +67,7 @@ dialog.close(); .vm_mut() .eval("__userInteractionBodyBoundary.join('|')")?, "callback|microtask", - "the selected callback completion must own the single task checkpoint" + "selected task completion must not repeat callback reactions or their inline scripts" ); Ok::<_, anyhow::Error>(()) }) diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/websocket/completion.rs b/moli-renderer-v8/src/runtime/page_vm/tests/websocket/completion.rs index 0334198d4a..9c7035a846 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/websocket/completion.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/websocket/completion.rs @@ -125,7 +125,7 @@ __webSocketTaskBoundarySocket.onmessage = event => {{ } #[tokio::test(flavor = "current_thread")] -async fn websocket_message_body_leaves_reactions_for_selected_completion() { +async fn websocket_message_body_cleans_up_listener_reactions() { run_page_vm_async_test(async move { let (url, opened_rx, message_tx, server) = spawn_triggered_text_websocket_server().await; @@ -166,8 +166,8 @@ async fn websocket_message_body_leaves_reactions_for_selected_completion() { page_vm .vm_mut() .eval("__webSocketTaskBoundary.join('|')")?, - "callback:payload", - "the WebSocket body must leave Promise reactions for selected-task completion" + "callback:payload|microtask|runtime-script", + "listener cleanup must drain reactions before the selected task completes" ); Ok::<_, anyhow::Error>(()) }) diff --git a/moli-renderer-v8/src/runtime/tests.rs b/moli-renderer-v8/src/runtime/tests.rs index 7f08b8783f..2c17e76fba 100644 --- a/moli-renderer-v8/src/runtime/tests.rs +++ b/moli-renderer-v8/src/runtime/tests.rs @@ -12128,7 +12128,7 @@ __lmActionWindowObserver.observe(document.getElementById("target")); assert_eq!( renderer_json_value(state), Some(serde_json::json!( - r#"{"scrollY":100,"wheelLog":["event:100","event:-100","event:100","microtask:100","microtask:-100","microtask:100"],"ioLog":[false,true]}"# + r#"{"scrollY":100,"wheelLog":["event:100","microtask:100","event:-100","microtask:-100","event:100","microtask:100"],"ioLog":[false,true]}"# )) ); diff --git a/moli-renderer-v8/src/script_cleanup.rs b/moli-renderer-v8/src/script_cleanup.rs new file mode 100644 index 0000000000..bcf7a39bd9 --- /dev/null +++ b/moli-renderer-v8/src/script_cleanup.rs @@ -0,0 +1,99 @@ +use std::{cell::Cell, rc::Rc}; + +#[derive(Default)] +struct ScriptExecutionDepth(Rc>); + +/// HTML realm execution contexts outlive the JavaScript frames of a script or +/// callback, including callback resolution, return conversion, and script error +/// reporting. Keep that part of the execution stack on the isolate/Agent. +pub(crate) struct ScriptExecutionScope(Rc>); + +impl ScriptExecutionScope { + pub(crate) fn enter(isolate: &mut v8::Isolate) -> Self { + if isolate.get_slot::().is_none() { + isolate.set_slot(ScriptExecutionDepth::default()); + } + let depth = isolate + .get_slot::() + .expect("script execution depth was initialized") + .0 + .clone(); + depth.set(depth.get() + 1); + Self(depth) + } +} + +impl Drop for ScriptExecutionScope { + fn drop(&mut self) { + self.0.set( + self.0 + .get() + .checked_sub(1) + .expect("script execution scope must be active"), + ); + } +} + +#[derive(Default)] +struct MicrotaskCheckpointState(Rc>); + +/// HTML's performing-a-microtask-checkpoint flag also covers rejection +/// notification and checkpoint-end cleanup after V8 finishes draining jobs. +pub(crate) struct MicrotaskCheckpointScope(Rc>); + +impl MicrotaskCheckpointScope { + pub(crate) fn enter(scope: &mut v8::PinScope<'_, '_>) -> Option { + if scope + .get_current_context() + .get_microtask_queue() + .is_some_and(v8::MicrotaskQueue::is_running_microtasks) + { + return None; + } + if scope.get_slot::().is_none() { + scope.set_slot(MicrotaskCheckpointState::default()); + } + let active = scope + .get_slot::() + .expect("microtask checkpoint state was initialized") + .0 + .clone(); + if active.replace(true) { + return None; + } + Some(Self(active)) + } +} + +impl Drop for MicrotaskCheckpointScope { + fn drop(&mut self) { + self.0.set(false); + } +} + +pub(crate) fn can_perform_script_cleanup_checkpoint(scope: &mut v8::PinScope<'_, '_>) -> bool { + scope + .get_slot::() + .is_none_or(|depth| depth.0.get() == 0) + && scope + .get_slot::() + .is_none_or(|state| !state.0.get()) + && !scope + .get_current_context() + .get_microtask_queue() + .is_some_and(v8::MicrotaskQueue::is_running_microtasks) + && v8::StackTrace::current_stack_trace(scope, 1) + .is_some_and(|stack| stack.get_frame_count() == 0) +} + +pub(crate) fn perform_callback_cleanup_checkpoint(scope: &mut v8::PinScope<'_, '_>) { + if !can_perform_script_cleanup_checkpoint(scope) { + return; + } + if crate::worker::perform_callback_cleanup_checkpoint_if_worker(scope) { + return; + } + if let Err(error) = crate::script_vm::ScriptVm::perform_microtask_checkpoints(scope, None) { + tracing::warn!(%error, "callback cleanup microtask checkpoint failed"); + } +} diff --git a/moli-renderer-v8/src/script_vm/classic_script_exception.rs b/moli-renderer-v8/src/script_vm/classic_script_exception.rs index 285f6942a0..67d31f0f3d 100644 --- a/moli-renderer-v8/src/script_vm/classic_script_exception.rs +++ b/moli-renderer-v8/src/script_vm/classic_script_exception.rs @@ -62,6 +62,7 @@ impl ScriptVm { }; // SAFETY: as_ptr() — V8 callbacks are re-entrant; borrow_mut() panics. See util.rs. let host_ptr: *mut JsContextHost = (*context_host).as_ptr(); + let execution_scope = crate::script_cleanup::ScriptExecutionScope::enter(scope); let dispatch_result = dispatch_window_error_event_with_details( scope, host_ptr, @@ -72,6 +73,7 @@ impl ScriptVm { error_value, ) .map_err(anyhow::Error::msg); + drop(execution_scope); let checkpoint_result = Self::perform_microtask_checkpoints(scope, None); dispatch_result?; checkpoint_result diff --git a/moli-renderer-v8/src/script_vm/eval_exec.rs b/moli-renderer-v8/src/script_vm/eval_exec.rs index 605a7bda91..6ef7bca4b1 100644 --- a/moli-renderer-v8/src/script_vm/eval_exec.rs +++ b/moli-renderer-v8/src/script_vm/eval_exec.rs @@ -198,6 +198,7 @@ fn execute_source_text_on_current_stack_with_completion( report_target: UncaughtScriptReportTarget, completion_mode: SourceTextScriptCompletionMode, ) -> RawScriptExecutionResult { + let execution_scope = crate::script_cleanup::ScriptExecutionScope::enter(scope); let result = run_source_text_on_current_stack_with_completion( scope, source, @@ -207,6 +208,7 @@ fn execute_source_text_on_current_stack_with_completion( report_target, completion_mode, ); + drop(execution_scope); // HTML's script cleanup also runs after an exception has been reported. // LogOnly callers report errors themselves before completing that cleanup. // Keep this checkpoint inside the caller's currentScript/parser-nesting @@ -215,12 +217,7 @@ fn execute_source_text_on_current_stack_with_completion( && matches!(&result, Err(RawScriptExecutionError::Exception { .. })); if drain_microtasks && (result.is_ok() || exception_reported) - && !scope - .get_current_context() - .get_microtask_queue() - .is_some_and(v8::MicrotaskQueue::is_running_microtasks) - && v8::StackTrace::current_stack_trace(scope, 1) - .is_some_and(|stack| stack.get_frame_count() == 0) + && crate::script_cleanup::can_perform_script_cleanup_checkpoint(scope) { ScriptVm::perform_microtask_checkpoints( scope, @@ -596,7 +593,7 @@ impl ScriptVm { ) } - pub(super) fn perform_microtask_checkpoints( + pub(crate) fn perform_microtask_checkpoints( scope: &mut v8::PinScope<'_, '_>, script_url: Option<&Url>, ) -> Result<()> { diff --git a/moli-renderer-v8/src/script_vm/runtime_bindings.rs b/moli-renderer-v8/src/script_vm/runtime_bindings.rs index dc065d4c95..a21a982af1 100644 --- a/moli-renderer-v8/src/script_vm/runtime_bindings.rs +++ b/moli-renderer-v8/src/script_vm/runtime_bindings.rs @@ -153,6 +153,10 @@ pub(super) fn flush_pending_promise_rejections(scope: &mut v8::PinScope<'_, '_>) pub(crate) fn perform_microtask_checkpoint_and_report_pending_promise_rejections( scope: &mut v8::PinScope<'_, '_>, ) { + let Some(_checkpoint_scope) = crate::script_cleanup::MicrotaskCheckpointScope::enter(scope) + else { + return; + }; let trace_enabled = moli_trace::cdp_runtime_trace_enabled(); let dom_binding_trace_enabled = trace_enabled && moli_trace::dom_binding_timing_enabled(); let trace_started = trace_enabled.then(Instant::now); diff --git a/moli-renderer-v8/src/script_vm/script_event_body.rs b/moli-renderer-v8/src/script_vm/script_event_body.rs index fc33b7303c..9e3f024dfe 100644 --- a/moli-renderer-v8/src/script_vm/script_event_body.rs +++ b/moli-renderer-v8/src/script_vm/script_event_body.rs @@ -105,6 +105,9 @@ fn dispatch_script_failure_error_body( filename: Option<&str>, error_value: Option, ) -> Result<()> { + // Script-owned failures are reported inside the script or its rejection + // job, after V8 has already unwound the original JavaScript frames. + let _execution_scope = crate::script_cleanup::ScriptExecutionScope::enter(scope); let global = scope.get_current_context().global(scope); let message_value = v8_string(scope, message) .ok_or_else(|| anyhow!("failed to allocate reportError message"))?; diff --git a/moli-renderer-v8/src/script_vm/tests/post_parse.rs b/moli-renderer-v8/src/script_vm/tests/post_parse.rs index a142fefcb8..68196d4779 100644 --- a/moli-renderer-v8/src/script_vm/tests/post_parse.rs +++ b/moli-renderer-v8/src/script_vm/tests/post_parse.rs @@ -3508,7 +3508,7 @@ async fn reentrant_runtime_admission_survives_page_task_claim_in_stable_authorit } #[test] -fn script_terminal_event_body_defers_listener_reaction_to_task_completion() { +fn script_terminal_event_body_cleans_up_listener_reactions_before_task_completion() { let _js_runtime = crate::JsRuntime::initialize(); let document = HtmlParser::SCRIPTING_ENABLED.parse( Url::parse("https://example.com/").unwrap(), @@ -3569,8 +3569,8 @@ fn script_terminal_event_body_defers_listener_reaction_to_task_completion() { assert_eq!( vm.eval_without_microtask_checkpoint_for_test("__runtimeTerminalOrder.join('|')") .expect("terminal body order should be readable without a checkpoint"), - "load", - "the terminal body must not perform the enclosing task-end checkpoint" + "load|load-microtask", + "terminal event callback cleanup must drain listener reactions before task completion" ); vm.perform_script_task_checkpoint(None) .expect("selected task completion checkpoint should run"); diff --git a/moli-renderer-v8/src/script_vm/tests/rendering_update.rs b/moli-renderer-v8/src/script_vm/tests/rendering_update.rs index 6755f70c91..3a46659eea 100644 --- a/moli-renderer-v8/src/script_vm/tests/rendering_update.rs +++ b/moli-renderer-v8/src/script_vm/tests/rendering_update.rs @@ -110,7 +110,7 @@ scrollTo(0, 12); } #[tokio::test(flavor = "current_thread")] -async fn scroll_handler_reentrancy_queues_a_new_turn_and_checkpoints_after_scrollend() { +async fn scroll_handler_reentrancy_queues_a_new_turn_and_cleans_up_before_scrollend() { let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).expect("loader"); let mut vm = new_storage_page_task_executor_test_vm("https://scroll-reentrant-update.test/"); @@ -141,8 +141,8 @@ scrollTo(0, 10); assert_eq!( vm.eval("__scrollLog.join('|')") .expect("first-turn log should be readable"), - "scroll:10|scrollend:20|microtask:20", - "one rendering update dispatches its pending event list before the host-task checkpoint" + "scroll:10|microtask:20|scrollend:20", + "scroll callback cleanup precedes the next pending scrollend event" ); assert!( @@ -154,7 +154,7 @@ scrollTo(0, 10); assert_eq!( vm.eval("__scrollLog.join('|')") .expect("second-turn log should be readable"), - "scroll:10|scrollend:20|microtask:20|scroll:20|scrollend:20|microtask:20" + "scroll:10|microtask:20|scrollend:20|scroll:20|microtask:20|scrollend:20" ); assert!(!vm.has_ready_timeout()); } @@ -187,8 +187,8 @@ scrollTo(0, 10); assert_eq!( vm.eval("__scrollErrorLog.join('|')") .expect("listener-error log should remain readable"), - "scroll|scrollend|microtask", - "public listener errors must not suppress later pending events or the host-task checkpoint" + "scroll|microtask|scrollend", + "public listener errors must not suppress callback cleanup or later pending events" ); assert!(!vm.has_ready_timeout()); } diff --git a/moli-renderer-v8/src/script_vm/window_message.rs b/moli-renderer-v8/src/script_vm/window_message.rs index 4bfd406130..c462c0960c 100644 --- a/moli-renderer-v8/src/script_vm/window_message.rs +++ b/moli-renderer-v8/src/script_vm/window_message.rs @@ -29,10 +29,8 @@ impl ScriptVm { /// Apply one task body only after the Page arbiter has matched the root /// PageVm namespace and exact LocalWindow target. /// - /// The selected Page-task dispatcher owns the callback checkpoint and its - /// child/runtime follow-up. Keeping this method body-only prevents the V8 - /// context helper from creating an intermediate checkpoint before the - /// scheduler task has actually completed. + /// Each listener performs HTML callback cleanup. The selected Page-task + /// dispatcher still owns task-end checkpointing and child/runtime follow-up. pub(crate) fn apply_current_window_message_task_body( &mut self, authorization: crate::runtime::AuthorizedCurrentPageWindowMessage, diff --git a/moli-renderer-v8/src/window_host.rs b/moli-renderer-v8/src/window_host.rs index 1119b1e348..97a230c498 100644 --- a/moli-renderer-v8/src/window_host.rs +++ b/moli-renderer-v8/src/window_host.rs @@ -500,8 +500,6 @@ pub(super) fn event_target_dispatch_event_callback<'s>( return; } - let _explicit_dispatch_scope = host.enter_explicit_event_dispatch_scope(); - let event_type = event_type_string(scope, event); if let Some(handle) = child_window_target { let event_type = event_type.as_deref().unwrap_or_default(); diff --git a/moli-renderer-v8/src/worker/mod.rs b/moli-renderer-v8/src/worker/mod.rs index ec14b658bc..50806afe77 100644 --- a/moli-renderer-v8/src/worker/mod.rs +++ b/moli-renderer-v8/src/worker/mod.rs @@ -22,6 +22,8 @@ mod script_mime; mod thread; mod timer_callback; +pub(crate) use thread::perform_callback_cleanup_checkpoint_if_worker; + pub(crate) use data_url::decode_data_url_script_source; pub(crate) use global_scope::{ NestedWorkerContext, WORKER_STATE_SLOT, WorkerOpfsCompletion, WorkerWebCryptoCompletion, diff --git a/moli-renderer-v8/src/worker/thread/dispatch.rs b/moli-renderer-v8/src/worker/thread/dispatch.rs index 4cad04aade..f9d61eca76 100644 --- a/moli-renderer-v8/src/worker/thread/dispatch.rs +++ b/moli-renderer-v8/src/worker/thread/dispatch.rs @@ -9,7 +9,7 @@ use tokio::sync::mpsc; use moli_webapi_declare::WebApiObject; -use crate::callback_invocation::{CallbackInvocationOutcome, CallbackInvoker}; +use crate::callback_invocation::{CallbackInvocation, CallbackInvocationOutcome, CallbackInvoker}; use crate::context_bootstrap::{ EVENT_DISPATCHING_SLOT, EVENT_STOP_IMMEDIATE_PROPAGATION_SLOT, EVENT_STOP_PROPAGATION_SLOT, EventHandlerType, SimpleObjectEventListenerSnapshot, apply_event_handler_return_value, @@ -21,8 +21,7 @@ use crate::context_bootstrap::{ structured_deserialize_value_for_message_event, }; use crate::exception_reporting::{ - CallbackExceptionLogLevel, V8ExceptionReport, invoke_callback_with_report, - log_unhandled_promise_rejection, + CallbackExceptionLogLevel, V8ExceptionReport, log_unhandled_promise_rejection, }; use crate::network_host::{ MaterializedResponseBody, MaterializedResponseHead, @@ -470,11 +469,28 @@ fn pending_worker_promise_rejection_matches<'s>( pub(super) fn perform_worker_microtask_checkpoint_and_report_pending_promise_rejections( scope: &mut v8::PinScope<'_, '_>, ) { + let Some(_checkpoint_scope) = crate::script_cleanup::MicrotaskCheckpointScope::enter(scope) + else { + return; + }; scope.perform_microtask_checkpoint(); queue_pending_worker_promise_rejection_task(scope); crate::context_bootstrap::run_end_of_microtask_checkpoint_tasks(scope); } +pub(crate) fn perform_callback_cleanup_checkpoint_if_worker( + scope: &mut v8::PinScope<'_, '_>, +) -> bool { + if scope + .get_slot::() + .is_none() + { + return false; + } + perform_worker_microtask_checkpoint_and_report_pending_promise_rejections(scope); + true +} + fn queue_pending_worker_promise_rejection_task(scope: &mut v8::PinScope<'_, '_>) { let Some((worker_wake_tx, pending_unhandled_rejections, task_queued)) = worker_promise_reject_task_state(scope) @@ -1057,23 +1073,33 @@ pub(super) fn dispatch_worker_error_event<'s>( .get(scope, v8str(scope, "onerror").into()) .and_then(|value| v8::Local::::try_from(value).ok()) { - match invoke_callback_with_report( + let arguments = [ + message.into(), + filename.into(), + lineno.into(), + colno.into(), + error_value, + ]; + let context = scope.get_current_context(); + let invocation = CallbackInvocation::new( + handler.into(), + global.into(), + context, + context, + true, + "handleEvent", + &arguments, + Some(event), + ); + match CallbackInvoker::invoke( scope, "callback", "worker global onerror threw", crate::exception_reporting::CallbackExceptionLogLevel::Error, "WorkerGlobalScope.onerror", - handler, - global.into(), - &[ - message.into(), - filename.into(), - lineno.into(), - colno.into(), - error_value, - ], + invocation, ) { - Ok(returned) => { + CallbackInvocationOutcome::Returned(returned) => { apply_event_handler_return_value( scope, event, @@ -1081,7 +1107,7 @@ pub(super) fn dispatch_worker_error_event<'s>( EventHandlerType::OnErrorEventHandler, ); } - Err(nested_report) => { + CallbackInvocationOutcome::Threw(nested_report) => { report_exception_to_parent( &nested_report, script_url, @@ -1089,6 +1115,7 @@ pub(super) fn dispatch_worker_error_event<'s>( parent_tx, ); } + CallbackInvocationOutcome::Retired => {} } } @@ -4294,6 +4321,10 @@ pub(super) fn dispatch_worker_exception_with_phase_and_source<'s>( parent_tx: &mpsc::UnboundedSender, script_url: &str, ) -> bool { + // A module bootstrap failure may arrive on a later evaluation task, + // after V8 has unwound the script or rejection job that owns its report. + let execution_scope = matches!(parent_phase, WorkerErrorPhase::Bootstrap) + .then(|| crate::script_cleanup::ScriptExecutionScope::enter(scope)); let exception = if report.muted_errors { report.summary = "Script error.".to_owned(); report.source = Some(String::new()); @@ -4320,6 +4351,10 @@ pub(super) fn dispatch_worker_exception_with_phase_and_source<'s>( parent_tx, ); } + drop(execution_scope); + if matches!(parent_phase, WorkerErrorPhase::Bootstrap) { + crate::script_cleanup::perform_callback_cleanup_checkpoint(scope); + } handled } diff --git a/moli-renderer-v8/src/worker/thread/mod.rs b/moli-renderer-v8/src/worker/thread/mod.rs index 756f80e6f3..08bc735e53 100644 --- a/moli-renderer-v8/src/worker/thread/mod.rs +++ b/moli-renderer-v8/src/worker/thread/mod.rs @@ -11,6 +11,8 @@ use std::sync::{ }; use std::time::{Duration, Instant}; +pub(crate) use dispatch::perform_callback_cleanup_checkpoint_if_worker; + #[cfg(test)] use crate::broadcast_channel_runtime::new_broadcast_channel_registry; use crate::broadcast_channel_runtime::{ @@ -1858,6 +1860,7 @@ async fn worker_main( let referrer_policy = { state.borrow().referrer_policy.clone() }; // ── Evaluate the worker script ───────────────────────────────────── + let execution_scope = crate::script_cleanup::ScriptExecutionScope::enter(scope); match evaluate_worker_bootstrap_script( scope, &script_source, @@ -1910,6 +1913,7 @@ async fn worker_main( } // Run microtask checkpoint after initial script evaluation. + drop(execution_scope); perform_worker_microtask_checkpoint_and_report_pending_promise_rejections(scope); drain_worker_dynamic_module_imports(scope, &state, &module_graph_fetch_tx); } diff --git a/moli-wpt-compat/fixtures/wpt/ported/fileapi/drag-operation-mask-basic.html b/moli-wpt-compat/fixtures/wpt/ported/fileapi/drag-operation-mask-basic.html index 16767b6332..2e752a0411 100644 --- a/moli-wpt-compat/fixtures/wpt/ported/fileapi/drag-operation-mask-basic.html +++ b/moli-wpt-compat/fixtures/wpt/ported/fileapi/drag-operation-mask-basic.html @@ -86,9 +86,6 @@ const complete = new Promise(function (resolve) { target.value, ); expectedDrops += 1; - if (expectedDrops === 9) { - resolve(); - } }); target.addEventListener("input", function () { @@ -101,6 +98,9 @@ const complete = new Promise(function (resolve) { ":" + target.selectionEnd, ); + if (expectedDrops === 9) { + resolve(); + } }); }); diff --git a/moli-wpt-compat/fixtures/wpt/ported/webmessaging/messagechannel-close-event-basic.html b/moli-wpt-compat/fixtures/wpt/ported/webmessaging/messagechannel-close-event-basic.html index b2f27d0e3c..2ed72a945f 100644 --- a/moli-wpt-compat/fixtures/wpt/ported/webmessaging/messagechannel-close-event-basic.html +++ b/moli-wpt-compat/fixtures/wpt/ported/webmessaging/messagechannel-close-event-basic.html @@ -24,10 +24,10 @@ promise_test(async function () { const events = await close_peer_and_wait(function (port, events, resolve) { port.addEventListener("close", function (event) { events.push("listener:" + event.type + ":" + event.isTrusted); - resolve(); }); port.onclose = function (event) { events.push("onclose:" + event.type + ":" + event.isTrusted); + resolve(); }; }); diff --git a/moli-wpt-compat/fixtures/wpt/ported/worker/worker-messageport-close-event-basic.html b/moli-wpt-compat/fixtures/wpt/ported/worker/worker-messageport-close-event-basic.html index 98b83be26a..4bb54fedfd 100644 --- a/moli-wpt-compat/fixtures/wpt/ported/worker/worker-messageport-close-event-basic.html +++ b/moli-wpt-compat/fixtures/wpt/ported/worker/worker-messageport-close-event-basic.html @@ -14,10 +14,10 @@ function worker_url() { " if (mode === 'listener-first') {", " port.addEventListener('close', function (closeEvent) {", " events.push('listener:' + closeEvent.type + ':' + closeEvent.isTrusted);", - " Promise.resolve().then(done);", " });", " port.onclose = function (closeEvent) {", " events.push('onclose:' + closeEvent.type + ':' + closeEvent.isTrusted);", + " done();", " };", " } else {", " port.onclose = function (closeEvent) { events.push('onclose:' + closeEvent.isTrusted); };",