From 2d2149caff216b9ce8d65b2faa2095bae496f8b2 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Thu, 10 Sep 2026 08:42:40 +0800 Subject: [PATCH] fix: unload javascript URL documents without beforeunload A child javascript URL string result must unload the old document without checking whether unloading is canceled. Reuse the existing teardown that dispatches pagehide, visibilitychange, and unload, and cancels old timers. Apply it to the target and its descendants, capturing document identities before callbacks can remove or replace them. Add three Browser integration tests covering 12 scenarios, including nested frames, unrelated frames, non-string completion, ordinary navigation, and attempts to navigate or schedule timers during unload. Record the newly passing javascript-url-no-beforeunload WPT. Validation: cargo fmt --all; workspace all-targets all-features Clippy with -D warnings; cargo nextest run --no-fail-fast (17,904 passed, 13 skipped). Focused WPT: 110 cases, one new pass / two passing subtest gains, no regressions. --- .../wpt-cross-current/passed-cases.txt | 1 + moli-core/tests/javascript_url_lifecycle.rs | 164 ++++++++++++++++++ .../context_host/child_documents/lifecycle.rs | 43 ++++- .../child_frame_navigation/commit.rs | 2 +- .../context_host/child_frames/lookup.rs | 2 +- 5 files changed, 201 insertions(+), 11 deletions(-) create mode 100644 moli-core/tests/javascript_url_lifecycle.rs diff --git a/moli-benchmark/wpt-cross-current/passed-cases.txt b/moli-benchmark/wpt-cross-current/passed-cases.txt index 87bf149c05..267b73ad68 100644 --- a/moli-benchmark/wpt-cross-current/passed-cases.txt +++ b/moli-benchmark/wpt-cross-current/passed-cases.txt @@ -5614,6 +5614,7 @@ 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 diff --git a/moli-core/tests/javascript_url_lifecycle.rs b/moli-core/tests/javascript_url_lifecycle.rs new file mode 100644 index 0000000000..17824219a6 --- /dev/null +++ b/moli-core/tests/javascript_url_lifecycle.rs @@ -0,0 +1,164 @@ +use anyhow::Result; +use moli_core::runtime::{Browser, BrowserConfig}; +use moli_test_support::FixtureServer; +use serde_json::{Value, json}; +use tokio::time::Duration; +use url::Url; + +async fn child_navigation_lifecycle(via: &str, kind: &str, depth: usize) -> 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_eq!(result["unrelatedUnchanged"], true, "{via}/{kind}: {result}"); + assert_eq!(result["staleTimerRan"], false, "{via}/{kind}: {result}"); + assert_eq!( + result["unloadNavigationRan"], false, + "{via}/{kind}: {result}" + ); + Ok(result) +} + +#[tokio::test(flavor = "multi_thread")] +async fn child_javascript_url_string_unloads_without_beforeunload() -> Result<()> { + for via in ["location", "src", "anchor"] { + for depth in [0, 2] { + let result = child_navigation_lifecycle(via, "string", depth).await?; + assert_eq!(result["sameDocument"], false); + assert_eq!(result["loads"], 1); + assert_eq!(result["text"], "replacement"); + assert_eq!(result["children"], 0); + let events = result["events"].as_array().unwrap(); + let labels = std::iter::once("target".to_owned()) + .chain((0..depth).map(|index| format!("descendant-{index}"))); + for label in labels { + let actual: Vec<_> = events + .iter() + .filter(|event| event.as_str().unwrap().starts_with(&format!("{label}:"))) + .cloned() + .collect(); + assert_eq!( + actual, + vec![ + json!(format!("{label}:pagehide")), + json!(format!("{label}:visibilitychange")), + json!(format!("{label}:unload")), + ], + "{via}/{depth}: {result}" + ); + } + assert_eq!(events.len(), 3 * (depth + 1), "{via}/{depth}: {result}"); + } + } + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn child_javascript_url_non_string_preserves_document_and_descendants() -> Result<()> { + for via in ["location", "src", "anchor"] { + let result = child_navigation_lifecycle(via, "undefined", 2).await?; + assert_eq!(result["sameDocument"], true); + assert_eq!(result["loads"], 0); + assert_eq!(result["events"], json!([])); + assert_eq!(result["children"], 1); + } + Ok(()) +} + +#[tokio::test(flavor = "multi_thread")] +async fn ordinary_child_navigation_still_dispatches_beforeunload() -> Result<()> { + for via in ["location", "src", "anchor"] { + let result = child_navigation_lifecycle(via, "network", 0).await?; + assert_eq!(result["sameDocument"], false); + assert_eq!(result["loads"], 1); + assert_eq!(result["text"], "network"); + let events: Vec<_> = result["events"] + .as_array() + .unwrap() + .iter() + .filter(|event| *event != "target:visibilitychange") + .cloned() + .collect(); + assert_eq!( + events, + vec![ + json!("target:beforeunload"), + json!("target:pagehide"), + json!("target:unload") + ], + "{via}: {result}" + ); + } + Ok(()) +} 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 135c5f4317..c808a2211e 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 @@ -913,14 +913,39 @@ impl JsContextHost { true } - /// Dispatches the unload sequence used when `Document::open()` removes a - /// descendant frame. - /// - /// This is intentionally distinct from navigation teardown: Chromium's - /// document-open steps do not prompt the child with `beforeunload`, and - /// dispatch pagehide/visibilitychange before unload while the parent - /// document's listeners are still installed. - fn dispatch_child_browsing_context_document_open_unload_lifecycle_if_needed( + pub(in crate::native_bridge::context_host) fn dispatch_child_javascript_url_unload_lifecycle( + scope: &mut v8::PinScope<'_, '_>, + host_ptr: *mut Self, + handle: DomHandle, + ) { + let Some(document) = unsafe { &*host_ptr }.child_browsing_context_document_handle(handle) else { + return; + }; + let mut handles = vec![handle]; + unsafe { &*host_ptr }.collect_child_browsing_context_handles_in_document_order_from_document( + document, + &mut handles, + ); + // Snapshot the documents before any unload handler can remove or + // replace a descendant. A new document must not inherit this unload. + let documents: Vec<_> = handles + .into_iter() + .filter_map(|handle| { + unsafe { &*host_ptr }.child_browsing_context_document_handle(handle) + .map(|document| (handle, document)) + }) + .collect(); + for (handle, document) in documents { + if unsafe { &*host_ptr }.child_browsing_context_document_handle(handle) == Some(document) { + Self::dispatch_child_document_unload_without_beforeunload(scope, host_ptr, handle); + } + } + } + + /// JavaScript URL replacement and removal by `Document::open()` unload + /// documents without checking whether unloading is canceled. They still + /// dispatch the actual unload lifecycle and cancel the old window's timers. + fn dispatch_child_document_unload_without_beforeunload( scope: &mut v8::PinScope<'_, '_>, host_ptr: *mut Self, handle: DomHandle, @@ -970,7 +995,7 @@ impl JsContextHost { { continue; } - Self::dispatch_child_browsing_context_document_open_unload_lifecycle_if_needed( + Self::dispatch_child_document_unload_without_beforeunload( scope, host_ptr, handle, ); } diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_frame_navigation/commit.rs b/moli-renderer-v8/src/native_bridge/context_host/child_frame_navigation/commit.rs index 9176a7efee..09afaa4533 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_frame_navigation/commit.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_frame_navigation/commit.rs @@ -679,7 +679,7 @@ impl JsContextHost { self.child_document_credentialless_storage_nonce(document_credentialless); self.clear_pending_child_document_loads_for_handle(handle); - self.dispatch_child_browsing_context_unload_lifecycle_if_needed(scope, handle); + Self::dispatch_child_javascript_url_unload_lifecycle(scope, self, handle); if !self.child_document_window_commit_preflight_is_current(handle, &window_commit_preflight) { let _ = self.finish_child_frame_navigation_without_load_dispatch( diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_frames/lookup.rs b/moli-renderer-v8/src/native_bridge/context_host/child_frames/lookup.rs index 0e84827432..4572842d01 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_frames/lookup.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_frames/lookup.rs @@ -59,7 +59,7 @@ impl JsContextHost { self.lightweight_popup_id_for_document_handle(owner_document) } - fn collect_child_browsing_context_handles_in_document_order_from_document( + pub(in crate::native_bridge::context_host) fn collect_child_browsing_context_handles_in_document_order_from_document( &self, document: DomHandle, out: &mut Vec,