diff --git a/moli-benchmark/wpt-cross-current/passed-cases.txt b/moli-benchmark/wpt-cross-current/passed-cases.txt index 267b73ad68..3976a2be71 100644 --- a/moli-benchmark/wpt-cross-current/passed-cases.txt +++ b/moli-benchmark/wpt-cross-current/passed-cases.txt @@ -5614,7 +5614,6 @@ html/browsers/browsing-the-web/navigating-across-documents/initial-empty-documen html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/window-open-history-length.html html/browsers/browsing-the-web/navigating-across-documents/initial-empty-document/window-open-nourl.html html/browsers/browsing-the-web/navigating-across-documents/javascript-url-global-scope.html -html/browsers/browsing-the-web/navigating-across-documents/javascript-url-no-beforeunload.window.js?moli-wpt-script=window html/browsers/browsing-the-web/navigating-across-documents/javascript-url-query-fragment-components.html html/browsers/browsing-the-web/navigating-across-documents/javascript-url-return-value-handling-dynamic.html html/browsers/browsing-the-web/navigating-across-documents/javascript-url-return-value-handling.html @@ -6264,12 +6263,10 @@ html/semantics/forms/form-submission-0/form-double-submit-to-different-origin-fr html/semantics/forms/form-submission-0/form-double-submit.html html/semantics/forms/form-submission-0/form-submit-iframe-then-location-navigate.html html/semantics/forms/form-submission-0/getactionurl.html -html/semantics/forms/form-submission-0/jsurl-form-submit.tentative.html html/semantics/forms/form-submission-0/jsurl-navigation-then-form-submit.html html/semantics/forms/form-submission-0/newline-normalization.html html/semantics/forms/form-submission-0/reparent-form-during-planned-navigation-task.html html/semantics/forms/form-submission-0/request-submit-activation.html -html/semantics/forms/form-submission-0/submit-entity-body.html html/semantics/forms/form-submission-0/url-encoded.html html/semantics/forms/historical.html html/semantics/forms/resetting-a-form/reset-event.html diff --git a/moli-core/tests/document_open_unload.rs b/moli-core/tests/document_open_unload.rs new file mode 100644 index 0000000000..66259a9e9a --- /dev/null +++ b/moli-core/tests/document_open_unload.rs @@ -0,0 +1,155 @@ +use anyhow::Result; +use moli_core::runtime::{Browser, BrowserConfig}; +use moli_test_support::FixtureServer; +use serde_json::Value; +use tokio::time::Duration; +use url::Url; + +async fn stream_operation_during_unload( + event: &str, + operation: &str, + target: &str, +) -> Result { + let server = FixtureServer::spawn().await?; + let browser = Browser::new(BrowserConfig::default())?; + let markup = format!( + r#""# + ); + let mut url = Url::parse(&server.url("/compat/child-dynamic-markup-document"))?; + url.query_pairs_mut().append_pair("markup", &markup); + let result = tokio::time::timeout(Duration::from_secs(10), async { + let mut page = browser.fetch(url.as_str()).await?; + page.evaluate_runtime_expression_with_await_async( + "finished.then(value => JSON.stringify(value))", + true, + ) + .await + }) + .await??; + let result: Value = serde_json::from_str(result["value"].as_str().unwrap())?; + server.shutdown().await; + assert!( + result["error"].is_null(), + "{event}/{operation}/{target}: {result}" + ); + Ok(result) +} + +#[tokio::test(flavor = "multi_thread")] +async fn document_stream_operations_have_no_side_effects_during_unload() -> Result<()> { + for event in ["beforeunload", "pagehide", "unload"] { + for operation in [ + "open", + "prototype-open", + "write", + "writeln", + "prototype-write", + ] { + let result = stream_operation_during_unload(event, operation, "self").await?; + assert_eq!(result["sameRoot"], true, "{event}/{operation}: {result}"); + assert_eq!(result["sameLength"], true); + assert_eq!(result["sameURL"], true); + assert_eq!(result["listenerCount"], 1); + let is_open = operation.ends_with("open"); + assert_eq!(result["returnedDocument"], is_open); + assert_eq!(result["converted"], usize::from(!is_open)); + } + } + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn ancestor_unload_counter_covers_descendant_callbacks() -> Result<()> { + for event in ["pagehide", "visibilitychange", "unload"] { + let result = stream_operation_during_unload(event, "open", "ancestor").await?; + assert_eq!(result["sameRoot"], true, "{event}: {result}"); + assert_eq!(result["sameLength"], true); + assert_eq!(result["listenerCount"], 1); + assert_eq!(result["returnedDocument"], true); + } + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn unload_does_not_block_opening_another_document() -> Result<()> { + for event in ["beforeunload", "pagehide", "unload"] { + let result = stream_operation_during_unload(event, "open", "other").await?; + assert_eq!(result["sameRoot"], false, "{event}: {result}"); + assert_eq!(result["listenerCount"], 0); + assert_eq!(result["returnedDocument"], true); + } + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn synthetic_unload_events_do_not_block_document_open() -> Result<()> { + for event in ["beforeunload", "pagehide", "visibilitychange", "unload"] { + let result = stream_operation_during_unload(event, "open", "synthetic").await?; + assert_eq!(result["sameRoot"], false, "{event}: {result}"); + assert_eq!(result["listenerCount"], 0); + assert_eq!(result["returnedDocument"], true); + } + Ok(()) +} diff --git a/moli-renderer-v8/src/context_bootstrap/navigation_events.rs b/moli-renderer-v8/src/context_bootstrap/navigation_events.rs index 8ec8de4830..64a613878a 100644 --- a/moli-renderer-v8/src/context_bootstrap/navigation_events.rs +++ b/moli-renderer-v8/src/context_bootstrap/navigation_events.rs @@ -669,6 +669,11 @@ fn dispatch_unload_lifecycle_event_for_runtime_owner<'s>( event_type: &str, event: v8::Local<'s, v8::Object>, ) { + let _document_unload = context_host_ptr_from_global_bridge(scope).and_then(|host_ptr| { + let host = unsafe { &*host_ptr }; + super::window_accessors::window_document_handle(scope, owner, host) + .map(|document| host.enter_document_unload(document)) + }); set_navigation_unload_event_active(scope, owner, true); if runtime_window_is_global(scope, owner) { if let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) { diff --git a/moli-renderer-v8/src/context_bootstrap/window_accessors.rs b/moli-renderer-v8/src/context_bootstrap/window_accessors.rs index f12b852fe4..757a93d422 100644 --- a/moli-renderer-v8/src/context_bootstrap/window_accessors.rs +++ b/moli-renderer-v8/src/context_bootstrap/window_accessors.rs @@ -22,7 +22,8 @@ pub(super) use child_context::{ }; pub(crate) use helpers::{current_window_style_viewport, window_host_ptr}; pub(super) use helpers::{ - window_child_context_handle, window_has_discarded_child_browsing_context, + window_child_context_handle, window_document_handle, + window_has_discarded_child_browsing_context, }; pub(super) use interceptors::{ window_indexed_property_definer, window_indexed_property_deleter, diff --git a/moli-renderer-v8/src/context_bootstrap/window_accessors/helpers.rs b/moli-renderer-v8/src/context_bootstrap/window_accessors/helpers.rs index b7c94c5f42..c83c5691f0 100644 --- a/moli-renderer-v8/src/context_bootstrap/window_accessors/helpers.rs +++ b/moli-renderer-v8/src/context_bootstrap/window_accessors/helpers.rs @@ -103,7 +103,7 @@ pub(super) fn window_owner_dispatch_scope<'s>( }) } -pub(super) fn window_document_handle<'s>( +pub(in crate::context_bootstrap) fn window_document_handle<'s>( scope: &mut v8::PinScope<'s, '_>, receiver: v8::Local<'s, v8::Object>, host: &JsContextHost, diff --git a/moli-renderer-v8/src/document_runtime.rs b/moli-renderer-v8/src/document_runtime.rs index c483ad939a..e454377110 100644 --- a/moli-renderer-v8/src/document_runtime.rs +++ b/moli-renderer-v8/src/document_runtime.rs @@ -739,6 +739,7 @@ pub(super) struct DocumentRuntime { resource_loader_binding: Option, script_context_stack: Vec, destructive_write_counters: destructive_writes::DocumentWriteCounters, + document_unload_counters: destructive_writes::DocumentWriteCounters, root_document_parser: Option, post_parse_schedule_invalidated: bool, stylesheet_lifecycle: StylesheetLifecycleState, diff --git a/moli-renderer-v8/src/document_runtime/destructive_writes.rs b/moli-renderer-v8/src/document_runtime/destructive_writes.rs index f1f657c3bf..3d9d2614d0 100644 --- a/moli-renderer-v8/src/document_runtime/destructive_writes.rs +++ b/moli-renderer-v8/src/document_runtime/destructive_writes.rs @@ -5,22 +5,21 @@ use super::{DocumentRuntime, DomHandle}; #[derive(Debug, Default)] pub(super) struct DocumentWriteCounters(Rc>>); -/// Owns an execute-script-element counter through script cleanup, including -/// its microtask checkpoint, but not subsequent tasks or pending TLA work. -/// The shared state avoids borrowing the runtime across JavaScript reentry. -pub(crate) struct IgnoreDestructiveWritesGuard { +/// Holds a document-local writing restriction across JavaScript reentry +/// without borrowing the runtime. Nested scopes release only their own count. +pub(crate) struct DocumentWriteCounterGuard { counters: Rc>>, document: DomHandle, } impl DocumentWriteCounters { - fn enter(&self, document: DomHandle) -> IgnoreDestructiveWritesGuard { + fn enter(&self, document: DomHandle) -> DocumentWriteCounterGuard { let mut counters = self.0.borrow_mut(); let counter = counters.entry(document).or_default(); *counter = counter .checked_add(1) .expect("document write counter overflow"); - IgnoreDestructiveWritesGuard { + DocumentWriteCounterGuard { counters: Rc::clone(&self.0), document, } @@ -31,7 +30,7 @@ impl DocumentWriteCounters { } } -impl Drop for IgnoreDestructiveWritesGuard { +impl Drop for DocumentWriteCounterGuard { fn drop(&mut self) { let mut counters = self.counters.borrow_mut(); let counter = counters @@ -45,16 +44,26 @@ impl Drop for IgnoreDestructiveWritesGuard { } impl DocumentRuntime { + /// Keep the script-element counter through script cleanup and its + /// microtask checkpoint, but not subsequent tasks or pending TLA work. pub(crate) fn enter_ignore_destructive_writes( &self, document: DomHandle, - ) -> IgnoreDestructiveWritesGuard { + ) -> DocumentWriteCounterGuard { self.destructive_write_counters.enter(document) } pub(crate) fn has_ignore_destructive_writes_counter(&self, document: DomHandle) -> bool { self.destructive_write_counters.is_active(document) } + + pub(crate) fn enter_document_unload(&self, document: DomHandle) -> DocumentWriteCounterGuard { + self.document_unload_counters.enter(document) + } + + pub(crate) fn has_document_unload_counter(&self, document: DomHandle) -> bool { + self.document_unload_counters.is_active(document) + } } #[cfg(test)] diff --git a/moli-renderer-v8/src/document_runtime/runtime_core.rs b/moli-renderer-v8/src/document_runtime/runtime_core.rs index 2e76167010..e386b979a5 100644 --- a/moli-renderer-v8/src/document_runtime/runtime_core.rs +++ b/moli-renderer-v8/src/document_runtime/runtime_core.rs @@ -83,6 +83,7 @@ impl DocumentRuntime { resource_loader_binding: None, script_context_stack: Vec::new(), destructive_write_counters: Default::default(), + document_unload_counters: Default::default(), root_document_parser: None, post_parse_schedule_invalidated: false, stylesheet_lifecycle, @@ -215,6 +216,7 @@ impl DocumentRuntime { resource_loader_binding: _, script_context_stack: _, destructive_write_counters: _, + document_unload_counters: _, root_document_parser: _, post_parse_schedule_invalidated: _, stylesheet_lifecycle: _, diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_documents/lifecycle.rs b/moli-renderer-v8/src/native_bridge/context_host/child_documents/lifecycle.rs index c808a2211e..dfde73368b 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_documents/lifecycle.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_documents/lifecycle.rs @@ -896,6 +896,9 @@ impl JsContextHost { let execution_context_owner = crate::native_bridge::WindowExecutionContextOwner::Frame( action.owner().local_window_id, ); + let _document_unload = self + .child_browsing_context_document_handle(handle) + .map(|document| self.enter_document_unload(document)); dispatch_beforeunload_for_runtime_owner(scope, window); dispatch_pagehide_for_runtime_owner(scope, window); dispatch_unload_for_runtime_owner(scope, window); @@ -932,11 +935,27 @@ impl JsContextHost { .into_iter() .filter_map(|handle| { unsafe { &*host_ptr }.child_browsing_context_document_handle(handle) - .map(|document| (handle, document)) + .map(|document| { + ( + handle, + document, + unsafe { &*host_ptr }.dom_host().owner_document_handle(handle), + ) + }) }) .collect(); - for (handle, document) in documents { + // Keep an ancestor's counter active while its descendants unload, + // and release a completed sibling before entering the next subtree. + let mut unload_guards = Vec::new(); + for (handle, document, parent_document) in documents { + while unload_guards + .last() + .is_some_and(|(document, _)| Some(*document) != parent_document) + { + unload_guards.pop(); + } if unsafe { &*host_ptr }.child_browsing_context_document_handle(handle) == Some(document) { + unload_guards.push((document, unsafe { &*host_ptr }.enter_document_unload(document))); Self::dispatch_child_document_unload_without_beforeunload(scope, host_ptr, handle); } } @@ -970,6 +989,9 @@ impl JsContextHost { action.owner().local_window_id, ); + let _document_unload = unsafe { &*host_ptr } + .child_browsing_context_document_handle(handle) + .map(|document| unsafe { &*host_ptr }.enter_document_unload(document)); dispatch_pagehide_for_runtime_owner(scope, window); if let Some(event) = construct_original_event(scope, "visibilitychange") { let _ = call_object_method(scope, document, "dispatchEvent", &[event.into()]); diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/document.rs b/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/document.rs index 0d109a2cab..40b386cef3 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/document.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/document.rs @@ -592,6 +592,9 @@ impl JsContextHost { { return None; } + if unsafe { &*host_ptr }.has_document_unload_counter(document_handle) { + return None; + } if unsafe { &*host_ptr }.child_document_stream_is_blocked_by_navigation(child_handle) { tracing::debug!( ?child_handle, diff --git a/moli-renderer-v8/src/native_bridge/document/lifecycle.rs b/moli-renderer-v8/src/native_bridge/document/lifecycle.rs index b73e2a26c5..afa3fb232f 100644 --- a/moli-renderer-v8/src/native_bridge/document/lifecycle.rs +++ b/moli-renderer-v8/src/native_bridge/document/lifecycle.rs @@ -123,6 +123,10 @@ fn node_document_write_or_writeln_callback<'s>( if detached_native_handle_for_runtime(scope, runtime_ptr, args.this()).is_some() { let document = args.this(); let stream_was_open = detached_document_write_stream_is_open(scope, document); + if !stream_was_open && unsafe { &*runtime_ptr }.has_document_unload_counter(handle) { + rv.set_undefined(); + return; + } if !stream_was_open { set_detached_document_write_stream_open(scope, document, true); } @@ -141,7 +145,8 @@ fn node_document_write_or_writeln_callback<'s>( let implicit_replacement_session = !runtime.has_active_parser_write_insertion_point() && !runtime.host_document().replace_on_close(); if implicit_replacement_session - && (runtime.has_ignore_destructive_writes_counter(handle) + && (runtime.has_document_unload_counter(handle) + || runtime.has_ignore_destructive_writes_counter(handle) || current_script_ignores_document_write_without_parser_insertion_point(runtime)) { rv.set_undefined(); @@ -243,6 +248,10 @@ pub(in crate::native_bridge) fn node_document_open_callback<'s>( ); return; } + if unsafe { &*runtime_ptr }.has_document_unload_counter(handle) { + rv.set(args.this().into()); + return; + } if detached_native_handle_for_runtime(scope, runtime_ptr, args.this()).is_some() { let document = args.this(); set_detached_document_write_stream_open(scope, document, true); diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/navigation.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/navigation.rs index 544c2a8fdc..38a053b979 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/navigation.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/navigation.rs @@ -2440,6 +2440,50 @@ fn cross_document_unload_lifecycle_orders_pagehide_before_unload_without_timer() ); } +#[test] +fn main_document_stream_operations_are_suppressed_only_during_unload() { + for event in ["beforeunload", "pagehide", "unload"] { + for operation in ["open", "write", "writeln"] { + let mut vm = new_storage_test_vm("https://document-open-unload.test/source"); + let result = vm + .eval(&format!( + r#"(() => {{ + const doc = document; + if (!doc.documentElement) doc.appendChild(doc.createElement('html')); + const root = doc.documentElement; + root.appendChild(doc.createElement('p')); + let retained = false; + let nestedRetained = false; + let listenerCount = 0; + let converted = 0; + doc.addEventListener('retained-listener', () => listenerCount++); + addEventListener('nested-open', () => {{ + doc.open(); + nestedRetained = doc.documentElement === root; + }}); + addEventListener({event:?}, () => {{ + if ({operation:?} === 'open') doc.open(); + else doc[{operation:?}]({{toString() {{ converted++; return 'changed'; }}}}); + retained = doc.documentElement === root; + dispatchEvent(new Event('nested-open')); + doc.dispatchEvent(new Event('retained-listener')); + }}); + navigation.navigate('/destination'); + doc.open(); + return [retained, nestedRetained, listenerCount, converted, + doc.documentElement !== root].join('|'); + }})()"# + )) + .expect("main document unload stream operations should evaluate"); + assert_eq!( + result, + format!("true|true|1|{}|true", usize::from(operation != "open")), + "{event}/{operation}" + ); + } + } +} + #[test] fn before_unload_handler_coerces_its_result_while_window_event_is_current() { let mut vm = new_storage_test_vm("https://example.com/base");