From 1922971bb8b7e1ab30d8555776ea6bc84dfa2394 Mon Sep 17 00:00:00 2001 From: ldm0 Date: Sat, 26 Sep 2026 09:44:36 +0800 Subject: [PATCH] fix(navigation): share non-cancelable child unload lifecycle --- moli-core/tests/javascript_url_lifecycle.rs | 164 ++++++++++++++++++ .../context_host/child_documents/lifecycle.rs | 50 ++++-- .../child_frame_navigation/commit.rs | 2 +- .../context_host/child_frames/lookup.rs | 2 +- 4 files changed, 205 insertions(+), 13 deletions(-) create mode 100644 moli-core/tests/javascript_url_lifecycle.rs 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 f036c38a68..7cb1c9d3ad 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 @@ -867,14 +867,44 @@ 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, @@ -924,9 +954,7 @@ impl JsContextHost { { continue; } - Self::dispatch_child_browsing_context_document_open_unload_lifecycle_if_needed( - scope, host_ptr, handle, - ); + 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 926236274b..3aad21883a 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 @@ -669,7 +669,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 c6dd50eb64..d7ba1e1c1d 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 @@ -54,7 +54,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,