From c2449e9525b663ea11988a5d7f2fc43eb9bc94ee 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 | 4 + .../wpt-cross-current/passed-cases.txt | 5 - 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 | 185 ++++++++---------- moli-renderer-v8/src/context_bootstrap.rs | 1 - .../events/simple_event_target/callbacks.rs | 3 - .../context_bootstrap/microtask_checkpoint.rs | 64 ------ 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 +- .../rendering_update/update_lifecycle.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 +- .../runtime/tests/extracted/input_dispatch.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 | 25 +++ 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, 527 insertions(+), 351 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 ac015582ce..beb187df17 100644 --- a/moli-benchmark/wpt-cross-current/failed-cases.txt +++ b/moli-benchmark/wpt-cross-current/failed-cases.txt @@ -3183,7 +3183,11 @@ html/syntax/parsing/template/creating-an-element-for-the-token/template-owner-do html/syntax/serializing-xml-fragments/outerHTML.html html/webappapis/dynamic-markup-insertion/document-write/module-delayed.html html/webappapis/dynamic-markup-insertion/document-write/module-static-import-delayed.html +html/webappapis/dynamic-markup-insertion/document-write/module-static-import.html html/webappapis/dynamic-markup-insertion/document-write/module-tla-delayed.html +html/webappapis/dynamic-markup-insertion/document-write/module-tla-immediate-promise.html +html/webappapis/dynamic-markup-insertion/document-write/module-tla-promise.html +html/webappapis/dynamic-markup-insertion/document-write/module.html html/webappapis/dynamic-markup-insertion/document-write/mutation-observer.html html/webappapis/dynamic-markup-insertion/document-write/write-active-document.html html/webappapis/dynamic-markup-insertion/html-unsafe-methods/Document-parseHTMLUnsafe.html diff --git a/moli-benchmark/wpt-cross-current/passed-cases.txt b/moli-benchmark/wpt-cross-current/passed-cases.txt index 88f225496b..3360963ae6 100644 --- a/moli-benchmark/wpt-cross-current/passed-cases.txt +++ b/moli-benchmark/wpt-cross-current/passed-cases.txt @@ -7370,11 +7370,7 @@ html/webappapis/dynamic-markup-insertion/document-write/iframe_008.html html/webappapis/dynamic-markup-insertion/document-write/iframe_009.html html/webappapis/dynamic-markup-insertion/document-write/iframe_010.html html/webappapis/dynamic-markup-insertion/document-write/module-dynamic-import.html -html/webappapis/dynamic-markup-insertion/document-write/module-static-import.html -html/webappapis/dynamic-markup-insertion/document-write/module-tla-immediate-promise.html html/webappapis/dynamic-markup-insertion/document-write/module-tla-import.html -html/webappapis/dynamic-markup-insertion/document-write/module-tla-promise.html -html/webappapis/dynamic-markup-insertion/document-write/module.html html/webappapis/dynamic-markup-insertion/document-write/script_001.html html/webappapis/dynamic-markup-insertion/document-write/script_002.html html/webappapis/dynamic-markup-insertion/document-write/script_003.html @@ -7409,7 +7405,6 @@ html/webappapis/dynamic-markup-insertion/opening-the-input-stream/013.html html/webappapis/dynamic-markup-insertion/opening-the-input-stream/014.html html/webappapis/dynamic-markup-insertion/opening-the-input-stream/015.html html/webappapis/dynamic-markup-insertion/opening-the-input-stream/016.html -html/webappapis/dynamic-markup-insertion/opening-the-input-stream/bailout-side-effects-synchronous-script.window.js?moli-wpt-script=window html/webappapis/dynamic-markup-insertion/opening-the-input-stream/document-open-cancels-javascript-url-navigation.html html/webappapis/dynamic-markup-insertion/opening-the-input-stream/document.open-02.html html/webappapis/dynamic-markup-insertion/opening-the-input-stream/document.open-03.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 9144322ecf..bb93dd8215 100644 --- a/moli-core/tests/web_apis.rs +++ b/moli-core/tests/web_apis.rs @@ -30,6 +30,8 @@ use tokio::time::Duration; #[path = "web_apis/abort_signal.rs"] mod abort_signal; +#[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 e60b93a4d1..4b7826c56a 100644 --- a/moli-renderer-v8/src/callback_invocation.rs +++ b/moli-renderer-v8/src/callback_invocation.rs @@ -208,7 +208,6 @@ impl CallbackInvoker { log_level, callback_name, invocation, - false, |_scope, outcome| outcome, ) } @@ -222,26 +221,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)] @@ -252,7 +240,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 { @@ -289,90 +276,92 @@ impl CallbackInvoker { } } - 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 previous = get_private_value(scope, global, WINDOW_EVENT_SLOT) - .unwrap_or_else(|| v8::undefined(scope).into()); - set_private_value(scope, global, WINDOW_EVENT_SLOT, 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); - set_private_value(scope, global, WINDOW_EVENT_SLOT, 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); + set_private_value(scope, global, WINDOW_EVENT_SLOT, 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 0096f99840..36ec40ec66 100644 --- a/moli-renderer-v8/src/context_bootstrap.rs +++ b/moli-renderer-v8/src/context_bootstrap.rs @@ -365,7 +365,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 b8ebe83568..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,22 +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); - crate::util::set_private_value(scope, global, crate::host::WINDOW_EVENT_SLOT, previous); -} diff --git a/moli-renderer-v8/src/lib.rs b/moli-renderer-v8/src/lib.rs index 1f07d5e32a..2464e3ef56 100644 --- a/moli-renderer-v8/src/lib.rs +++ b/moli-renderer-v8/src/lib.rs @@ -96,6 +96,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 3368e26e84..f2b4d60fb0 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/core.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/core.rs @@ -518,8 +518,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, browsing_context_name: Default::default(), 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 a61f93ded3..b8ac30bed4 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 426e663731..e5771256b6 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/mod.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/mod.rs @@ -1005,8 +1005,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, browsing_context_name: crate::runtime::RendererBrowsingContextName, 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 978cc6a79d..7aa5d8e8fb 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(); @@ -74,8 +74,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>(()) }) @@ -120,7 +120,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 c3a926c3ba..0bce4e95b8 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 2c0a9c93a5..231b7813ca 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 8c6ee17351..5729424b74 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 5576968329..d5fb298a4d 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/lifecycle.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/lifecycle.rs @@ -1583,7 +1583,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 { @@ -1623,8 +1623,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/update_lifecycle.rs b/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update/update_lifecycle.rs index 78448b4fa6..3d1a8efdbe 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update/update_lifecycle.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update/update_lifecycle.rs @@ -189,7 +189,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"); @@ -225,8 +225,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 @@ -245,7 +245,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 38333c07b2..41d6fb4d8b 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/extracted/input_dispatch.rs b/moli-renderer-v8/src/runtime/tests/extracted/input_dispatch.rs index 1f1e1f6d66..5482eb1e2b 100644 --- a/moli-renderer-v8/src/runtime/tests/extracted/input_dispatch.rs +++ b/moli-renderer-v8/src/runtime/tests/extracted/input_dispatch.rs @@ -391,7 +391,7 @@ addEventListener("scroll", () => fetch("/action-window-scroll-applied"), { once: 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]}"# + r#"{"scrollY":100,"wheelLog":["event:100","microtask:100","event:-100","microtask:-100","event:100","microtask:100"],"ioLog":[false]}"# )) ); 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 04c86f822e..8c825c826b 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 51066794c5..6d83ca4d06 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, @@ -626,7 +623,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 423e1f3ea8..cecef93d29 100644 --- a/moli-renderer-v8/src/script_vm/runtime_bindings.rs +++ b/moli-renderer-v8/src/script_vm/runtime_bindings.rs @@ -187,6 +187,10 @@ fn flush_promise_rejection_state( 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 1bbcb92100..c8866f828d 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 26d2f95ce2..3ff18bec83 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 1dda6964ea..105f46ae52 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 db0a41dfd4..f321cecea7 100644 --- a/moli-renderer-v8/src/window_host.rs +++ b/moli-renderer-v8/src/window_host.rs @@ -388,8 +388,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 9faae8233a..1ecf13060a 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 9e87f22381..05a57c1ac7 100644 --- a/moli-renderer-v8/src/worker/thread/dispatch.rs +++ b/moli-renderer-v8/src/worker/thread/dispatch.rs @@ -425,11 +425,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) @@ -4181,6 +4198,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()); @@ -4206,6 +4227,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 d6be16c7ff..ba04214cb3 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::{ @@ -1831,6 +1833,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, @@ -1883,6 +1886,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); };",