From a6dbce6ee6c318275d6b97e3c91f57fd56fb8627 Mon Sep 17 00:00:00 2001 From: lanyue-llk <270302213+lanyue-llk@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:46:15 +0800 Subject: [PATCH 1/8] feat(cdp): publish layout metrics without paint --- .../src/page/renderer_command_support.rs | 4 + .../src/protocol_server/webdriver_bidi.rs | 1 + .../src/commands/window.rs | 1 + moli-protocol/src/conn/dispatch_tests/page.rs | 1 + moli-protocol/src/devtools_runtime.rs | 1 + moli-protocol/src/domains/page/capture.rs | 42 +++- .../domains/page/protocol_neutral_tests.rs | 7 +- .../src/domains/page/tests/capture.rs | 64 ++++++ moli-renderer-v8/src/runtime/page_commands.rs | 3 + moli-renderer-v8/src/runtime/page_geometry.rs | 11 + moli-renderer-v8/src/runtime/page_surface.rs | 2 + .../tests/extracted/page_task_dispatch.rs | 1 + .../src/runtime/page_vm/tests/mod.rs | 1 + .../tests/rendering_update/layout_geometry.rs | 201 ++++++++++++++++++ 14 files changed, 331 insertions(+), 9 deletions(-) diff --git a/moli-core/src/page/renderer_command_support.rs b/moli-core/src/page/renderer_command_support.rs index 24770e6170..78d626ca8f 100644 --- a/moli-core/src/page/renderer_command_support.rs +++ b/moli-core/src/page/renderer_command_support.rs @@ -1466,6 +1466,10 @@ impl Page { self.start_page_command(RendererPageCommand::LayoutMetrics) } + pub fn start_published_layout_metrics(&self) -> Result { + self.start_page_command(RendererPageCommand::PublishLayoutMetrics) + } + pub fn finish_layout_metrics( &mut self, completion: CompletedPageCommand, diff --git a/moli-protocol-server/src/protocol_server/webdriver_bidi.rs b/moli-protocol-server/src/protocol_server/webdriver_bidi.rs index eb7179e52a..13c0b524d9 100644 --- a/moli-protocol-server/src/protocol_server/webdriver_bidi.rs +++ b/moli-protocol-server/src/protocol_server/webdriver_bidi.rs @@ -2949,6 +2949,7 @@ async fn bidi_input_viewport_bounds( target_id: Some(DevToolsTargetId::from(context_id)), browser_context_id: None, }, + publish_layout: false, }); match scheduler .execute_devtools_command_with_protocol_messages(command) diff --git a/moli-protocol-webdriver-classic/src/commands/window.rs b/moli-protocol-webdriver-classic/src/commands/window.rs index 92d6d7b66b..0a4529d549 100644 --- a/moli-protocol-webdriver-classic/src/commands/window.rs +++ b/moli-protocol-webdriver-classic/src/commands/window.rs @@ -31,6 +31,7 @@ pub fn create_initial_target_command(context: &ClassicDevToolsCommandContext) -> pub fn layout_metrics_command(context: &ClassicDevToolsCommandContext) -> DevToolsCommand { DevToolsCommand::GetLayoutMetrics(DevToolsGetLayoutMetricsCommand { context: context.command_context(), + publish_layout: false, }) } diff --git a/moli-protocol/src/conn/dispatch_tests/page.rs b/moli-protocol/src/conn/dispatch_tests/page.rs index 7ecc0ba766..adf4943ca7 100644 --- a/moli-protocol/src/conn/dispatch_tests/page.rs +++ b/moli-protocol/src/conn/dispatch_tests/page.rs @@ -913,6 +913,7 @@ async fn devtools_command_executes_context_viewport_override() { target_id: Some(target_id), ..context }, + publish_layout: false, }, )) .await diff --git a/moli-protocol/src/devtools_runtime.rs b/moli-protocol/src/devtools_runtime.rs index 037d771542..69ebc42691 100644 --- a/moli-protocol/src/devtools_runtime.rs +++ b/moli-protocol/src/devtools_runtime.rs @@ -605,6 +605,7 @@ pub struct DevToolsGetFrameTreesCommand { #[derive(Debug, Clone, PartialEq, Eq)] pub struct DevToolsGetLayoutMetricsCommand { pub context: DevToolsCommandContext, + pub publish_layout: bool, } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/moli-protocol/src/domains/page/capture.rs b/moli-protocol/src/domains/page/capture.rs index 23545e1cb6..8ff839e6d6 100644 --- a/moli-protocol/src/domains/page/capture.rs +++ b/moli-protocol/src/domains/page/capture.rs @@ -1,5 +1,12 @@ use super::*; +#[derive(Default, Deserialize)] +#[serde(rename_all = "camelCase")] +struct GetLayoutMetricsParams { + #[serde(default)] + publish_layout: bool, +} + #[derive(Deserialize)] #[serde(rename_all = "camelCase")] pub(super) struct ScreenshotClip { @@ -357,13 +364,16 @@ pub(super) async fn execute_devtools_get_layout_metrics_command( command: DevToolsGetLayoutMetricsCommand, ) -> Result { let owner = page_command_owner(conn, &command.context)?; - let result = execute_devtools_get_layout_metrics_for_current_owner(conn, &owner).await; + let result = + execute_devtools_get_layout_metrics_for_current_owner(conn, &owner, command.publish_layout) + .await; result.map(DevToolsCommandResult::LayoutMetrics) } pub(super) async fn execute_devtools_get_layout_metrics_for_current_owner( conn: &mut CdpConnection, owner: &CommandOwnerScope, + publish_layout: bool, ) -> Result { let Some(page) = conn .runtime_session_owner_slot_mut_for_owner(owner) @@ -372,7 +382,12 @@ pub(super) async fn execute_devtools_get_layout_metrics_for_current_owner( else { return Err(devtools_layout_metrics_error("NoDocumentLoaded")); }; - let pending = page.start_layout_metrics().map_err(|error| { + let pending = if publish_layout { + page.start_published_layout_metrics() + } else { + page.start_layout_metrics() + } + .map_err(|error| { devtools_layout_metrics_error(format!("Failed to start layout metrics: {error}")) })?; let completed = pending.wait().await.map_err(|error| { @@ -672,21 +687,29 @@ pub(super) fn start_devtools_print_to_pdf_command( pub(super) fn build_cdp_get_layout_metrics_command( conn: &CdpConnection, cmd: &Cmd<'_>, -) -> crate::devtools_runtime::DevToolsGetLayoutMetricsCommand { +) -> Result { + let params = cmd + .get_params::() + .map_err(|message| CommandOutputPlan::error(-32602, message))? + .unwrap_or_default(); let (browser_context_id, target_id) = conn .target_owner_identity_for_session(cmd.session_id) .map(|(browser_context_id, target_id)| (Some(browser_context_id), target_id)) .unwrap_or((None, None)); - crate::devtools_runtime::DevToolsGetLayoutMetricsCommand { + Ok(crate::devtools_runtime::DevToolsGetLayoutMetricsCommand { context: cmd.devtools_command_context(target_id.as_deref(), browser_context_id.as_deref()), - } + publish_layout: params.publish_layout, + }) } pub(super) fn try_start_page_get_layout_metrics_command( conn: &mut CdpConnection, cmd: &Cmd<'_>, ) -> PageCommandTaskStep { - let command = build_cdp_get_layout_metrics_command(conn, cmd); + let command = match build_cdp_get_layout_metrics_command(conn, cmd) { + Ok(command) => command, + Err(plan) => return PageCommandTaskStep::Complete(plan), + }; start_devtools_page_command(conn, cmd.id, DevToolsCommand::GetLayoutMetrics(command)) } @@ -706,7 +729,12 @@ pub(super) fn start_devtools_get_layout_metrics_command( devtools_layout_metrics_error("NoDocumentLoaded"), )); }; - match page.start_layout_metrics() { + let pending = if command.publish_layout { + page.start_published_layout_metrics() + } else { + page.start_layout_metrics() + }; + match pending { Ok(pending) => PageCommandTaskStep::Pending(PendingPageCommandDispatch { command_id, owner_scope, diff --git a/moli-protocol/src/domains/page/protocol_neutral_tests.rs b/moli-protocol/src/domains/page/protocol_neutral_tests.rs index 0c4e66e027..d4cd60be3a 100644 --- a/moli-protocol/src/domains/page/protocol_neutral_tests.rs +++ b/moli-protocol/src/domains/page/protocol_neutral_tests.rs @@ -74,7 +74,8 @@ fn cdp_get_layout_metrics_builds_protocol_neutral_command() { r#"{"id":122,"method":"Page.getLayoutMetrics"}"#, ); - let command = build_cdp_get_layout_metrics_command(&conn, &cmd); + let command = + build_cdp_get_layout_metrics_command(&conn, &cmd).expect("valid layout metrics command"); assert_eq!(command.context.protocol, DevToolsProtocol::Cdp); assert_eq!( @@ -83,6 +84,7 @@ fn cdp_get_layout_metrics_builds_protocol_neutral_command() { ); assert_eq!(command.context.target_id, None); assert_eq!(command.context.browser_context_id, None); + assert!(!command.publish_layout); } #[test] @@ -96,7 +98,8 @@ fn devtools_page_entry_routes_get_layout_metrics_command_to_page_owner() { None, r#"{"id":123,"method":"Page.getLayoutMetrics"}"#, ); - let command = build_cdp_get_layout_metrics_command(&conn, &cmd); + let command = + build_cdp_get_layout_metrics_command(&conn, &cmd).expect("valid layout metrics command"); let step = start_devtools_page_command( &mut conn, diff --git a/moli-protocol/src/domains/page/tests/capture.rs b/moli-protocol/src/domains/page/tests/capture.rs index b914934121..6d6af8ca4c 100644 --- a/moli-protocol/src/domains/page/tests/capture.rs +++ b/moli-protocol/src/domains/page/tests/capture.rs @@ -1345,6 +1345,70 @@ async fn get_layout_metrics_queries_live_renderer_for_loaded_pages() { "content size should come from a one-shot live layout: {metrics:?}" ); } + +#[tokio::test(flavor = "multi_thread")] +async fn get_layout_metrics_can_explicitly_publish_without_a_screenshot() { + let mut ctx = TestContext::new(); + load_bc_with_session( + &mut ctx, + "BID-PUBLISH-LAYOUT-METRICS", + "TID-PUBLISH-LAYOUT-METRICS", + "SID-PUBLISH-LAYOUT-METRICS", + "about:blank", + ); + let page_url = "data:text/html,
"; + let page = ctx + .conn + .load_page_via_runtime_async(page_url) + .await + .expect("page should load"); + ctx.conn + .browser_context + .as_mut() + .expect("browser context") + .active_page_target_mut() + .runtime_slot + .replace_loaded_page(Some(page)); + + ctx.process_async(json!({ + "id": 126, + "method": "Page.getLayoutMetrics", + "sessionId": "SID-PUBLISH-LAYOUT-METRICS" + })) + .await; + let sampled = take_response_by_id(&mut ctx, 126); + assert_eq!( + sampled["result"]["contentSize"], + json!({ "x": 0, "y": 0, "width": 2300.0, "height": 1500.0 }), + "ordinary Page.getLayoutMetrics must keep its one-shot live-layout contract" + ); + + ctx.process_async(json!({ + "id": 127, + "method": "Page.getLayoutMetrics", + "sessionId": "SID-PUBLISH-LAYOUT-METRICS", + "params": {"publishLayout": true} + })) + .await; + let published = take_response_by_id(&mut ctx, 127); + assert_eq!( + published["result"]["contentSize"], + json!({ "x": 0, "y": 0, "width": 2300.0, "height": 1500.0 }) + ); + + ctx.process_async(json!({ + "id": 128, + "method": "Page.getLayoutMetrics", + "sessionId": "SID-PUBLISH-LAYOUT-METRICS" + })) + .await; + let cached = take_response_by_id(&mut ctx, 128); + assert_eq!( + cached["result"]["contentSize"], + published["result"]["contentSize"] + ); +} + #[tokio::test(flavor = "multi_thread")] async fn get_layout_metrics_targets_loaded_background_owner_without_activation() { let mut ctx = TestContext::new(); diff --git a/moli-renderer-v8/src/runtime/page_commands.rs b/moli-renderer-v8/src/runtime/page_commands.rs index 79a35ccb93..f36443ef58 100644 --- a/moli-renderer-v8/src/runtime/page_commands.rs +++ b/moli-renderer-v8/src/runtime/page_commands.rs @@ -856,6 +856,9 @@ impl PageVm { RendererPageCommand::LayoutMetrics => { Ok(RendererPageReply::LayoutMetrics(self.layout_metrics()?)) } + RendererPageCommand::PublishLayoutMetrics => self + .publish_layout_metrics() + .map(RendererPageReply::LayoutMetrics), RendererPageCommand::PublishLayout => { self.vm_mut().publish_layout()?; Ok(RendererPageReply::Unit) diff --git a/moli-renderer-v8/src/runtime/page_geometry.rs b/moli-renderer-v8/src/runtime/page_geometry.rs index 847da643f8..a0ea77b1e1 100644 --- a/moli-renderer-v8/src/runtime/page_geometry.rs +++ b/moli-renderer-v8/src/runtime/page_geometry.rs @@ -13,4 +13,15 @@ impl PageVm { device_pixel_ratio: f64::from(metrics.viewport.device_pixel_ratio), }) } + + pub(crate) fn publish_layout_metrics(&mut self) -> anyhow::Result { + if !self.layout_policy.uses_real_layout() { + return Err(anyhow::anyhow!( + "layout publication is disabled by the current layout policy" + )); + } + self.flush_page_action_window(moli_action_window::ActionBarrier::Explicit)?; + self.vm_mut().publish_layout()?; + self.layout_metrics() + } } diff --git a/moli-renderer-v8/src/runtime/page_surface.rs b/moli-renderer-v8/src/runtime/page_surface.rs index 532f72931d..57d23ce4e2 100644 --- a/moli-renderer-v8/src/runtime/page_surface.rs +++ b/moli-renderer-v8/src/runtime/page_surface.rs @@ -5195,6 +5195,7 @@ pub enum RendererPageCommand { }, SerializeHtml, LayoutMetrics, + PublishLayoutMetrics, PublishLayout, CaptureScreenshot(RendererCaptureScreenshotRequest), CaptureScreencastFrame(RendererCaptureScreencastFrameRequest), @@ -5799,6 +5800,7 @@ impl RendererPageCommand { Self::RenderPageDump { .. } => Some("RenderPageDump"), Self::SerializeHtml => Some("SerializeHtml"), Self::LayoutMetrics => Some("LayoutMetrics"), + Self::PublishLayoutMetrics => Some("PublishLayoutMetrics"), Self::PublishLayout => Some("PublishLayout"), Self::CaptureScreenshot(_) => Some("CaptureScreenshot"), Self::CaptureScreencastFrame(_) => Some("CaptureScreencastFrame"), diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/extracted/page_task_dispatch.rs b/moli-renderer-v8/src/runtime/page_vm/tests/extracted/page_task_dispatch.rs index 435ae18128..3127d4b9f9 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/extracted/page_task_dispatch.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/extracted/page_task_dispatch.rs @@ -11,6 +11,7 @@ fn default_runtime_hooks_reject_direct_no_owner_page_vm_construction() { local_executor, &loader, &PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), web_storage: crate::RendererWebStorageHandles::ephemeral(), root_frame_id: None, main_document_commit: None, diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/mod.rs b/moli-renderer-v8/src/runtime/page_vm/tests/mod.rs index 7d4f8ac18e..dbd0dd2965 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/mod.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/mod.rs @@ -658,6 +658,7 @@ fn test_page_vm_with_loader_dom_host_hooks_and_response_referrer_policy( local_executor, loader, &PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), web_storage: crate::RendererWebStorageHandles::ephemeral(), root_frame_id, main_document_commit: None, diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update/layout_geometry.rs b/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update/layout_geometry.rs index c3806fa271..f314dcc1f4 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update/layout_geometry.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update/layout_geometry.rs @@ -1133,3 +1133,204 @@ document.body.innerHTML = ` .await .expect("intrinsic width fixture should run"); } +#[tokio::test(flavor = "current_thread")] +async fn explicit_layout_metrics_publication_rejects_mock_policy_without_a_layout_pass() { + run_page_vm_async_test(async move { + let loader = + crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); + let mut page_vm = test_page_vm_with_loader_and_document_url( + &loader, + Vec::new(), + Url::parse("https://example.com/layout-metrics-policy.html")?, + ); + page_vm.layout_policy = moli_page_types::LayoutPolicy::Mock; + page_vm + .vm_mut() + .set_layout_policy(moli_page_types::LayoutPolicy::Mock); + page_vm.vm_mut().eval( + r#" +document.body.innerHTML = '
'; +'installed' +"#, + )?; + let before = page_vm.vm().layout_pass_observability_for_test(); + + let error = page_vm + .publish_layout_metrics() + .expect_err("Mock policy must reject explicit real-layout publication"); + assert!( + error.to_string().contains("layout publication is disabled"), + "unexpected publication error: {error:#}" + ); + assert_eq!( + page_vm.vm().layout_pass_observability_for_test().1, + before.1, + "rejecting publication must not build a hidden real-layout pass" + ); + Ok::<(), anyhow::Error>(()) + }) + .await + .expect("layout policy publication boundary should run"); +} + +#[tokio::test(flavor = "current_thread")] +async fn explicit_layout_metrics_publication_skips_paint_and_reuses_the_published_snapshot() { + run_page_vm_async_test(async move { + let loader = + crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); + let mut page_vm = test_page_vm_with_loader_and_document_url( + &loader, + Vec::new(), + Url::parse("https://example.com/layout-metrics-publication.html")?, + ); + page_vm.vm_mut().eval( + r#" +document.head.innerHTML = ''; +document.body.innerHTML = '
'; +'installed' +"#, + )?; + page_vm.vm_mut().sync_live_document_style_sources(); + let before = page_vm.vm().layout_pass_observability_for_test(); + + let metrics = page_vm.publish_layout_metrics()?; + assert_eq!(metrics.content_width, 1920.0); + assert_eq!(metrics.content_height, 1080.0); + let after = page_vm.vm().layout_pass_observability_for_test(); + assert_eq!(after.1, before.1 + 1); + let pass = after.3.expect("publication records one layout pass"); + assert_eq!(pass.reason, moli_layout::LayoutFlushReason::Explicit); + assert_eq!(pass.paint_operation_count, 0); + + let cached = page_vm.layout_metrics()?; + assert_eq!(cached, metrics); + assert_eq!(page_vm.vm().layout_pass_observability_for_test().1, after.1); + Ok::<(), anyhow::Error>(()) + }) + .await + .expect("explicit layout metrics publication should run"); +} + +#[tokio::test(flavor = "current_thread")] +async fn explicit_layout_metrics_publication_freezes_nested_frame_geometry_without_paint() { + run_page_vm_async_test(async move { + let loader = + crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); + let mut page_vm = test_page_vm_with_loader_and_document_url( + &loader, + Vec::new(), + Url::parse("https://example.com/nested-frame-layout-publication.html")?, + ); + page_vm.vm_mut().eval( + r#" +document.head.innerHTML = ''; +document.body.innerHTML = ''; +const outer = document.getElementById('outer'); +outer.contentDocument.head.innerHTML = ''; +outer.contentDocument.body.innerHTML = '
'; +const inner = outer.contentDocument.getElementById('inner'); +inner.contentDocument.head.innerHTML = ''; +inner.contentDocument.body.innerHTML = '
'; +'installed' +"#, + )?; + page_vm.vm_mut().sync_live_document_style_sources(); + + let outer = page_vm + .vm() + .element_handle_by_id_for_test("outer") + .expect("outer iframe handle"); + let root_document = page_vm.vm().document_handle_for_test(); + let before = page_vm.vm().layout_pass_observability_for_test(); + + page_vm.publish_layout_metrics()?; + + let after = page_vm.vm().layout_pass_observability_for_test(); + assert_eq!(after.1, before.1 + 1); + let pass = after.3.expect("publication records one layout pass"); + assert_eq!(pass.paint_operation_count, 0); + + let host = page_vm + .vm() + .context_host_weak_for_test() + .upgrade() + .expect("page context host"); + host.borrow() + .with_latest_layout_tree_for_document(root_document, |root_tree| { + let child_tree = root_tree + .embedded_frame_tree(outer) + .expect("the publication must freeze the child frame tree"); + assert_eq!( + child_tree.viewport, + moli_layout::LayoutViewport::new(240, 140, 1.0), + "the child tree must retain the iframe's embedded viewport" + ); + let mut nested_frames = child_tree.embedded_frames(); + let grandchild_tree = &nested_frames + .next() + .expect("the publication must recursively freeze the nested frame tree") + .tree; + assert_eq!(nested_frames.len(), 0, "the fixture has one nested frame"); + assert_eq!( + grandchild_tree.viewport, + moli_layout::LayoutViewport::new(100, 60, 1.0), + "the grandchild tree must retain the nested iframe's embedded viewport" + ); + }) + .expect("the publication must retain the root layout tree"); + + drop(host); + page_vm.vm_mut().eval( + r#" +document.getElementById('outer').style.width = '300px'; +document.getElementById('outer').contentDocument.getElementById('inner').style.height = '80px'; +'mutated' +"#, + )?; + page_vm.vm_mut().sync_live_document_style_sources(); + page_vm.publish_layout_metrics()?; + let republished = page_vm.vm().layout_pass_observability_for_test(); + assert_eq!(republished.1, after.1 + 1); + assert_eq!( + republished + .3 + .expect("republication records the rebuilt layout pass") + .paint_operation_count, + 0 + ); + let host = page_vm + .vm() + .context_host_weak_for_test() + .upgrade() + .expect("page context host"); + host.borrow() + .with_latest_layout_tree_for_document(root_document, |root_tree| { + let child_tree = root_tree + .embedded_frame_tree(outer) + .expect("republication must retain the rebuilt child frame tree"); + assert_eq!( + child_tree.viewport, + moli_layout::LayoutViewport::new(300, 140, 1.0) + ); + let grandchild_tree = &child_tree + .embedded_frames() + .next() + .expect("republication must retain the rebuilt nested frame tree") + .tree; + assert_eq!( + grandchild_tree.viewport, + moli_layout::LayoutViewport::new(100, 80, 1.0) + ); + }) + .expect("republication must replace the retained root layout tree"); + + assert_eq!( + page_vm.vm().layout_pass_observability_for_test().1, + republished.1, + "inspecting nested geometry must not hide a missing frame projection with another pass" + ); + Ok::<(), anyhow::Error>(()) + }) + .await + .expect("nested frame layout publication should run"); +} From 1be6b4470659cf986d1af6dcc7f71dd8e320bc9f Mon Sep 17 00:00:00 2001 From: lanyue-llk <270302213+lanyue-llk@users.noreply.github.com> Date: Tue, 29 Sep 2026 14:46:24 +0800 Subject: [PATCH 2/8] fix(browser): preserve top-level window name by browsing context --- moli-core/src/runtime/navigation_engine.rs | 18 +- .../classic/extracted/windows_and_popups.rs | 71 +++++++ .../src/conn/page_state/fetch_state.rs | 8 +- .../src/conn/page_state/page_targets.rs | 197 +++++++++++++++++- .../src/conn/state/browser_context.rs | 50 ++++- .../src/domains/page/tests/lifecycle.rs | 2 +- moli-protocol/src/domains/target.rs | 12 +- moli-protocol/src/domains/target/popup.rs | 7 +- .../tests/tests_background_staging/runtime.rs | 13 +- .../src/browsing_context_state.rs | 29 +++ .../src/context_bootstrap/runtime_state.rs | 24 ++- moli-renderer-v8/src/lib.rs | 2 + .../src/native_bridge/context_host/core.rs | 15 ++ .../context_host/host_environment.rs | 7 + .../src/native_bridge/context_host/mod.rs | 1 + .../src/runtime/owner/page_creation.rs | 2 + moli-renderer-v8/src/runtime/page.rs | 1 + .../runtime/page_vm/followed_navigation.rs | 1 + moli-renderer-v8/src/runtime/page_vm/mod.rs | 4 + .../src/runtime/page_vm/test_support.rs | 1 + moli-renderer-v8/src/runtime/phase_one/mod.rs | 1 + .../src/runtime/phase_one/streaming.rs | 1 + .../extracted/parser_blocking_scripts.rs | 4 + .../tests/extracted/parser_script_handoffs.rs | 6 + .../tree_mutations_and_document_write.rs | 1 + .../src/script_vm/browsing_contexts.rs | 17 ++ .../misc/extracted/window_surface.rs | 30 +++ 27 files changed, 502 insertions(+), 23 deletions(-) create mode 100644 moli-renderer-v8/src/browsing_context_state.rs diff --git a/moli-core/src/runtime/navigation_engine.rs b/moli-core/src/runtime/navigation_engine.rs index c96b3c891e..f3f0cc6840 100644 --- a/moli-core/src/runtime/navigation_engine.rs +++ b/moli-core/src/runtime/navigation_engine.rs @@ -24,7 +24,8 @@ use moli_renderer_v8::{ RendererBrowserContextRuntime, RendererBrowserContextRuntimeOwner, RendererBrowserContextRuntimeOwnerAccess, RendererDocumentReplacement, RendererReservedServiceWorkerClient, RendererServiceWorkerMainResourceFetch, - RendererWebStorageHandles, SharedStorageBucketStore, WeakIndexedDbManager, + RendererTopLevelBrowsingContextState, RendererWebStorageHandles, SharedStorageBucketStore, + WeakIndexedDbManager, network::{ BrowserResourceRuntime, BrowserResourceRuntimeOwner, PageNetworkPolicy, navigation::{DocumentFetchContextSeed, NavigationResourceLoader}, @@ -424,6 +425,7 @@ pub struct NavigationEngine { js_runtime: JsRuntime, resource_runtime: Option, browser_context_access: RendererBrowserContextRuntimeOwnerAccess, + top_level_browsing_context: RendererTopLevelBrowsingContextState, document_activity: moli_page_types::DocumentActivity, // Standalone engines share this last-drop owner. BrowserContext engines // leave it empty and borrow only the context's weak, bound access. @@ -453,6 +455,15 @@ impl Default for NavigationEngine { } impl NavigationEngine { + pub fn top_level_window_name(&self) -> String { + self.top_level_browsing_context.window_name() + } + + pub fn set_top_level_window_name(&self, value: impl Into) { + self.top_level_browsing_context + .set_window_name(value.into()); + } + /// Reserves a renderer Page identity before the corresponding creation /// command is enqueued. /// @@ -583,6 +594,7 @@ impl NavigationEngine { js_runtime, resource_runtime: Some(resource_runtime), browser_context_access, + top_level_browsing_context: RendererTopLevelBrowsingContextState::default(), document_activity: Default::default(), standalone_lifetime_owner, }) @@ -634,6 +646,7 @@ impl NavigationEngine { js_runtime: renderer_owner_source.js_runtime.clone(), resource_runtime: Some(resource_runtime), browser_context_access: renderer_owner_source.browser_context_access.clone(), + top_level_browsing_context: RendererTopLevelBrowsingContextState::default(), document_activity: Default::default(), standalone_lifetime_owner: renderer_owner_source.standalone_lifetime_owner.clone(), }) @@ -1535,6 +1548,7 @@ impl NavigationEngine { top_level_storage_key, moli_renderer_v8::RendererTopLevelNavigationDispatch::DelegateToBrowser, moli_renderer_v8::RendererDocumentOptions { + top_level_browsing_context: self.top_level_browsing_context.clone(), indexed_db_manager, storage_bucket_store, document_start_scripts, @@ -1815,6 +1829,7 @@ impl NavigationEngine { reserved_service_worker_client, None, moli_renderer_v8::RendererDocumentOptions { + top_level_browsing_context: self.top_level_browsing_context.clone(), indexed_db_manager, storage_bucket_store, document_start_scripts, @@ -2068,6 +2083,7 @@ impl NavigationEngine { web_storage, options.response_body, moli_renderer_v8::RendererDocumentOptions { + top_level_browsing_context: self.top_level_browsing_context.clone(), indexed_db_manager, storage_bucket_store, document_start_scripts: options.document_start_scripts, diff --git a/moli-protocol-server/src/protocol_server/tests/classic/extracted/windows_and_popups.rs b/moli-protocol-server/src/protocol_server/tests/classic/extracted/windows_and_popups.rs index c4dc0fe97a..358a1102a7 100644 --- a/moli-protocol-server/src/protocol_server/tests/classic/extracted/windows_and_popups.rs +++ b/moli-protocol-server/src/protocol_server/tests/classic/extracted/windows_and_popups.rs @@ -2581,3 +2581,74 @@ async fn webdriver_classic_new_window_user_prompt_behavior_matches_chromium_wpt( .await; } } +#[tokio::test] +async fn webdriver_classic_named_popup_does_not_reuse_an_independent_tab() { + let app = build_router(test_state()); + let session = classic_request_json(app.clone(), Method::POST, "/session").await; + let session_id = session["value"]["sessionId"] + .as_str() + .expect("classic session id"); + let window_path = format!("/session/{session_id}/window"); + let handles_path = format!("/session/{session_id}/window/handles"); + let execute_path = format!("/session/{session_id}/execute/sync"); + + let original = classic_request_json(app.clone(), Method::GET, &window_path).await; + let original = original["value"] + .as_str() + .expect("original window handle") + .to_owned(); + let named = classic_request_json_with_body( + app.clone(), + Method::POST, + &execute_path, + json!({ + "script": "window.name = 'independent-report'; return window.name;", + "args": [] + }), + ) + .await; + assert_eq!(named, json!({ "value": "independent-report" })); + + let independent = classic_request_json_with_body( + app.clone(), + Method::POST, + &format!("/session/{session_id}/window/new"), + json!({ "type": "tab" }), + ) + .await; + let independent = independent["value"]["handle"] + .as_str() + .expect("independent tab handle") + .to_owned(); + let switched = classic_request_json_with_body( + app.clone(), + Method::POST, + &window_path, + json!({ "handle": independent }), + ) + .await; + assert_eq!(switched, json!({ "value": null })); + + let opened = classic_request_json_with_body( + app.clone(), + Method::POST, + &execute_path, + json!({ + "script": "const popup = window.open('about:blank#independent-popup', 'independent-report'); return popup !== null;", + "args": [] + }), + ) + .await; + assert_eq!(opened, json!({ "value": true })); + let handles = classic_request_json(app.clone(), Method::GET, &handles_path).await; + let handles = handles["value"].as_array().expect("window handles"); + assert_eq!( + handles.len(), + 3, + "an unrelated same-name tab must not be selected as the popup target: {handles:?}" + ); + assert!(handles.contains(&json!(original))); + assert!(handles.contains(&json!(independent))); + + let _ = classic_request_json(app, Method::DELETE, &format!("/session/{session_id}")).await; +} diff --git a/moli-protocol/src/conn/page_state/fetch_state.rs b/moli-protocol/src/conn/page_state/fetch_state.rs index e34aeb519b..1fe734aee5 100644 --- a/moli-protocol/src/conn/page_state/fetch_state.rs +++ b/moli-protocol/src/conn/page_state/fetch_state.rs @@ -60,6 +60,7 @@ impl BrowserContext { } pub(crate) fn insert_page_target_host(&mut self, mut host: PageTargetHost) -> bool { + let target_id = host.target_id().to_owned(); if self.page_targets.is_empty() { host.document_cookie_manager_surface = self.default_document_cookie_manager_surface.clone(); @@ -69,7 +70,12 @@ impl BrowserContext { let engine = self.new_page_navigation_engine(config); host.install_navigation_engine(engine); } - self.page_targets.insert(host) + let inserted = self.page_targets.insert(host); + if inserted { + self.target_browsing_context_group_ids + .insert(target_id.clone(), target_id); + } + inserted } #[cfg(test)] diff --git a/moli-protocol/src/conn/page_state/page_targets.rs b/moli-protocol/src/conn/page_state/page_targets.rs index facc195fff..eecb72c48e 100644 --- a/moli-protocol/src/conn/page_state/page_targets.rs +++ b/moli-protocol/src/conn/page_state/page_targets.rs @@ -17,6 +17,7 @@ impl BrowserContext { pub(crate) fn take_page_target_for_close(&mut self, target_id: &str) -> Option { let target = self.page_targets.remove(target_id)?; self.forget_target_opener_references_for_target(target_id); + self.target_browsing_context_group_ids.remove(target_id); self.forget_target_window_names_for_target(target_id); self.forget_target_popup_id_for_target(target_id); Some(target) @@ -134,9 +135,60 @@ impl BrowserContext { Some(target_name.to_owned()) } - pub(crate) fn target_id_for_window_name(&self, target_name: &str) -> Option<&str> { + pub(crate) fn target_id_for_window_name( + &self, + source_target_id: &str, + target_name: &str, + ) -> Option<&str> { let name = Self::reusable_window_open_target_name(target_name)?; - self.target_window_names.get(&name).map(String::as_str) + let source_group_id = self + .target_browsing_context_group_ids + .get(source_target_id)?; + let target_matches = |target: &PageTargetHost| { + self.target_browsing_context_group_ids + .get(target.target_id()) + .is_some_and(|group_id| group_id == source_group_id) + && target + .navigation_engine() + .is_some_and(|engine| engine.top_level_window_name() == name) + }; + if let Some(source) = self.page_target(source_target_id) + && target_matches(source) + { + return Some(source.target_id()); + } + if let Some(target) = self + .page_targets + .iter() + .find(|target| target.target_id() != source_target_id && target_matches(target)) + { + return Some(target.target_id()); + } + let staged_target_matches = |target_id: &str, staged_name: &str| { + staged_name == name + && self.page_target(target_id).is_some_and(|target| { + target.navigation_engine().is_none() + && self + .target_browsing_context_group_ids + .get(target_id) + .is_some_and(|group_id| group_id == source_group_id) + }) + }; + if self + .target_window_names + .get(source_target_id) + .is_some_and(|staged_name| staged_target_matches(source_target_id, staged_name)) + { + return self + .page_target(source_target_id) + .map(PageTargetHost::target_id); + } + self.target_window_names + .iter() + .find_map(|(target_id, staged_name)| { + (target_id != source_target_id && staged_target_matches(target_id, staged_name)) + .then_some(target_id.as_str()) + }) } pub(crate) fn has_attached_child_frame_id(&self, frame_id: &str) -> bool { @@ -147,7 +199,14 @@ impl BrowserContext { pub(crate) fn remember_target_window_name(&mut self, target_name: &str, target_id: &str) { if let Some(name) = Self::reusable_window_open_target_name(target_name) { - self.target_window_names.insert(name, target_id.to_owned()); + if let Some(engine) = self + .page_target(target_id) + .and_then(PageTargetHost::navigation_engine) + { + engine.set_top_level_window_name(name); + return; + } + self.target_window_names.insert(target_id.to_owned(), name); } } @@ -162,8 +221,7 @@ impl BrowserContext { } pub(crate) fn forget_target_window_names_for_target(&mut self, target_id: &str) { - self.target_window_names - .retain(|_, mapped_target_id| mapped_target_id != target_id); + self.target_window_names.remove(target_id); } pub(crate) fn forget_target_popup_id_for_target(&mut self, target_id: &str) { @@ -192,12 +250,23 @@ impl BrowserContext { opener_frame_id: String, can_access_opener: bool, ) { + let inherited_group_id = can_access_opener + .then(|| { + self.target_browsing_context_group_ids + .get(&opener_target_id) + .cloned() + }) + .flatten(); self.target_opener_ids .insert(target_id.to_owned(), opener_target_id); self.target_opener_frame_ids .insert(target_id.to_owned(), opener_frame_id); if can_access_opener { self.target_can_access_opener.insert(target_id.to_owned()); + if let Some(group_id) = inherited_group_id { + self.target_browsing_context_group_ids + .insert(target_id.to_owned(), group_id); + } } else { self.target_can_access_opener.remove(target_id); } @@ -1357,17 +1426,129 @@ mod tests { ); let mut context = BrowserContext::new("BC-window-name".to_owned()); + for target_id in ["TID-spaced", "TID-exact"] { + context.stage_background_target( + target_id.to_owned(), + None, + "about:blank".to_owned(), + None, + None, + ); + } context.remember_target_window_name(" ReportWindow ", "TID-spaced"); context.remember_target_window_name("ReportWindow", "TID-exact"); assert_eq!( - context.target_id_for_window_name(" ReportWindow "), + context.target_id_for_window_name("TID-spaced", " ReportWindow "), Some("TID-spaced") ); assert_eq!( - context.target_id_for_window_name("ReportWindow"), + context.target_id_for_window_name("TID-exact", "ReportWindow"), Some("TID-exact") ); - assert_eq!(context.target_id_for_window_name("reportwindow"), None); + assert_eq!( + context.target_id_for_window_name("TID-exact", "reportwindow"), + None + ); + } + + #[test] + fn staged_window_name_seeds_the_target_engine_and_live_renames_replace_it() { + let mut context = BrowserContext::new("BC-window-name-owner".to_owned()); + context.stage_background_target( + "TID-named".to_owned(), + None, + "about:blank".to_owned(), + None, + None, + ); + context.remember_target_window_name("reportWindow", "TID-named"); + assert_eq!( + context.target_id_for_window_name("TID-named", "reportWindow"), + Some("TID-named"), + "the protocol registry must route a named target while it is staged" + ); + + context.bind_page_navigation_engines( + moli_core::runtime::NavigationRuntimeConfig::default(), + None, + ); + let engine = context + .page_navigation_engine("TID-named") + .expect("binding the context installs the target engine"); + assert_eq!(engine.top_level_window_name(), "reportWindow"); + assert!( + context.target_window_names.is_empty(), + "the protocol registry is only staging storage once the renderer owner exists" + ); + assert_eq!( + context.target_id_for_window_name("TID-named", "reportWindow"), + Some("TID-named"), + "installing the renderer owner must preserve the staged browsing-context name" + ); + + engine.set_top_level_window_name("renamedWindow"); + assert_eq!( + context.target_id_for_window_name("TID-named", "reportWindow"), + None + ); + assert_eq!( + context.target_id_for_window_name("TID-named", "renamedWindow"), + Some("TID-named"), + "after installation, named-target lookup must follow the live browsing context" + ); + } + + #[test] + fn named_target_lookup_stays_within_the_source_browsing_context_group() { + let mut context = BrowserContext::new("BC-window-name-groups".to_owned()); + for target_id in ["TID-a", "TID-b", "TID-popup"] { + context.stage_background_target( + target_id.to_owned(), + None, + "about:blank".to_owned(), + None, + None, + ); + } + context.remember_target_window_name("shared", "TID-a"); + assert_eq!( + context.target_id_for_window_name("TID-b", "shared"), + None, + "an independently created tab must not target a same-name tab" + ); + + context.remember_target_opener("TID-popup", "TID-b".to_owned(), "FRAME-b".to_owned(), true); + context.remember_target_window_name("shared", "TID-popup"); + assert_eq!( + context.target_id_for_window_name("TID-b", "shared"), + Some("TID-popup"), + "a popup that can access its opener belongs to the same target-name group" + ); + + context.remember_target_window_name("renamed", "TID-popup"); + assert_eq!( + context.target_id_for_window_name("TID-b", "shared"), + None, + "the popup's old name must stop routing after a live rename" + ); + assert_eq!( + context.target_id_for_window_name("TID-b", "renamed"), + Some("TID-popup"), + "a related popup is reusable by its current name" + ); + assert_eq!( + context.target_id_for_window_name("TID-a", "renamed"), + None, + "renaming a popup must not expose it to an independent group" + ); + + context.remember_target_window_name("current", "TID-popup"); + context.remember_target_window_name("current", "TID-b"); + assert_eq!( + context.target_id_for_window_name("TID-b", "current"), + Some("TID-b"), + "the source navigable takes priority over another related target" + ); } #[test] diff --git a/moli-protocol/src/conn/state/browser_context.rs b/moli-protocol/src/conn/state/browser_context.rs index 18adbdd4af..441f79353a 100644 --- a/moli-protocol/src/conn/state/browser_context.rs +++ b/moli-protocol/src/conn/state/browser_context.rs @@ -54,6 +54,12 @@ pub struct BrowserContext { /// an implicit-noopener `_blank` target still has an `openerId`, but is /// intentionally absent from this set. pub(crate) target_can_access_opener: HashSet, + /// Stable browsing-context group membership for top-level page targets. + /// + /// A popup that retains script access to its opener inherits the opener's + /// group. Independent targets and noopener popups keep their own group so + /// a matching `window.name` cannot route navigation across that boundary. + pub(crate) target_browsing_context_group_ids: HashMap, pub target_window_names: HashMap, pub target_popup_ids: HashMap, pending_popup_javascript_dialogs: HashMap>, @@ -505,6 +511,7 @@ impl BrowserContext { target_opener_ids: HashMap::new(), target_opener_frame_ids: HashMap::new(), target_can_access_opener: HashSet::new(), + target_browsing_context_group_ids: HashMap::new(), target_window_names: HashMap::new(), target_popup_ids: HashMap::new(), pending_popup_javascript_dialogs: HashMap::new(), @@ -563,6 +570,7 @@ impl BrowserContext { let renderer_runtime = self.renderer_runtime_owner_access(); let sender = self.renderer_output_transport_sender.clone(); + let target_window_names = &self.target_window_names; for host in self.page_targets.iter_mut() { if host.navigation_engine().is_some() { continue; @@ -575,8 +583,17 @@ impl BrowserContext { if let Some(sender) = sender.clone() { engine.set_renderer_output_transport_sender(sender); } + if let Some(window_name) = target_window_names.get(host.target_id()) { + engine.set_top_level_window_name(window_name.clone()); + } host.install_navigation_engine(engine); } + let page_targets = &self.page_targets; + self.target_window_names.retain(|target_id, _| { + page_targets + .get(target_id) + .is_none_or(|target| target.navigation_engine().is_none()) + }); } pub(crate) fn set_renderer_output_transport_sender( @@ -912,6 +929,11 @@ impl BrowserContext { "targetOpenerCount": self.target_opener_ids.len(), "targetOpenerFrameCount": self.target_opener_frame_ids.len(), "targetCanAccessOpenerCount": self.target_can_access_opener.len(), + "targetBrowsingContextGroupCount": self + .target_browsing_context_group_ids + .values() + .collect::>() + .len(), "targetWindowNameCount": self.target_window_names.len(), "defaultDocumentStartScriptCount": self.default_document_start_scripts.len(), "domRemoteObjectNodeCacheCount": active_target @@ -1631,7 +1653,33 @@ impl BrowserContext { } pub(crate) fn rekey_active_target(&mut self, target_id: impl Into) -> bool { - self.page_targets.rekey_active(target_id.into()) + let Some(previous_target_id) = self.active_target_id().map(str::to_owned) else { + return false; + }; + let target_id = target_id.into(); + if !self.page_targets.rekey_active(target_id.clone()) { + return false; + } + let previous_group_id = self + .target_browsing_context_group_ids + .remove(&previous_target_id) + .unwrap_or_else(|| previous_target_id.clone()); + if previous_group_id == previous_target_id { + for group_id in self.target_browsing_context_group_ids.values_mut() { + if *group_id == previous_target_id { + *group_id = target_id.clone(); + } + } + } + self.target_browsing_context_group_ids.insert( + target_id.clone(), + if previous_group_id == previous_target_id { + target_id + } else { + previous_group_id + }, + ); + true } pub(crate) fn active_session_id(&self) -> Option<&str> { diff --git a/moli-protocol/src/domains/page/tests/lifecycle.rs b/moli-protocol/src/domains/page/tests/lifecycle.rs index afa29aece9..254850469d 100644 --- a/moli-protocol/src/domains/page/tests/lifecycle.rs +++ b/moli-protocol/src/domains/page/tests/lifecycle.rs @@ -3043,7 +3043,7 @@ async fn close_clears_loaded_page_state_and_emits_detached_events() { assert!(!bc.has_active_target()); assert!(!bc.has_active_session()); assert!(bc.attached_target_id_for_session("SID-attached").is_none()); - assert!(bc.target_id_for_window_name("close-me").is_none()); + assert!(bc.target_id_for_window_name("TID-1", "close-me").is_none()); assert!(!bc.target_opener_ids.contains_key("TID-popup-after-close")); assert!( !bc.target_opener_frame_ids diff --git a/moli-protocol/src/domains/target.rs b/moli-protocol/src/domains/target.rs index 61cd163172..7e47836548 100644 --- a/moli-protocol/src/domains/target.rs +++ b/moli-protocol/src/domains/target.rs @@ -48,20 +48,18 @@ pub(crate) fn popup_activation_creates_new_target_for_owner( owner: &CommandOwnerScope, target_name: &str, ) -> bool { - if let Some((browser_context_id, _)) = conn.target_owner_identity_for_owner(owner) { + if let Some((browser_context_id, Some(source_target_id))) = + conn.target_owner_identity_for_owner(owner) + { return conn .browser_context_by_id(&browser_context_id) .is_none_or(|browser_context| { browser_context - .target_id_for_window_name(target_name) + .target_id_for_window_name(&source_target_id, target_name) .is_none() }); } - conn.browser_context.as_ref().is_none_or(|browser_context| { - browser_context - .target_id_for_window_name(target_name) - .is_none() - }) + true } pub(in crate::domains) use worker_target::{ TargetPreparedOutputSlot, dedicated_worker_main_script_network_replay_for_session, diff --git a/moli-protocol/src/domains/target/popup.rs b/moli-protocol/src/domains/target/popup.rs index d130829614..21c8b04e72 100644 --- a/moli-protocol/src/domains/target/popup.rs +++ b/moli-protocol/src/domains/target/popup.rs @@ -95,8 +95,11 @@ pub(crate) async fn create_popup_target_from_renderer_output_background_events_a return None; }; - if let Some(existing_target_id) = browser_context - .target_id_for_window_name(&target_name) + if let Some(existing_target_id) = opener + .as_ref() + .and_then(|opener| { + browser_context.target_id_for_window_name(&opener.target_id, &target_name) + }) .map(str::to_owned) { let navigation = diff --git a/moli-protocol/src/domains/target/tests/tests_background_staging/runtime.rs b/moli-protocol/src/domains/target/tests/tests_background_staging/runtime.rs index 2c0ed32d09..6f3a21728f 100644 --- a/moli-protocol/src/domains/target/tests/tests_background_staging/runtime.rs +++ b/moli-protocol/src/domains/target/tests/tests_background_staging/runtime.rs @@ -449,16 +449,27 @@ async fn same_context_named_popup_reuse_navigates_and_activates_loaded_owner() { let mut ctx = TestContext::new(); tokio::task::LocalSet::new() .run_until(async { + let active_target_id = "TID-000000000NPA"; let owner = load_same_context_loaded_background_runtime_owner_async( &mut ctx, "BID-9-NAMED-POPUP", - "TID-000000000NPA", + active_target_id, "active
active target
", "data:text/html,background
background target
", 1041949440, ) .await; ctx.enable_background_navigation_scheduler_for_test(); + ctx.conn + .browser_context + .as_mut() + .expect("browser context") + .remember_target_opener( + &owner.target_id, + active_target_id.to_owned(), + active_target_id.to_owned(), + true, + ); ctx.conn .browser_context .as_mut() diff --git a/moli-renderer-v8/src/browsing_context_state.rs b/moli-renderer-v8/src/browsing_context_state.rs new file mode 100644 index 0000000000..76469ed2be --- /dev/null +++ b/moli-renderer-v8/src/browsing_context_state.rs @@ -0,0 +1,29 @@ +use std::{fmt, sync::Arc}; + +use parking_lot::Mutex; + +/// Mutable state owned by one top-level browsing context and shared by each +/// Document committed into that context. +#[derive(Clone, Default)] +pub struct RendererTopLevelBrowsingContextState { + window_name: Arc>, +} + +impl RendererTopLevelBrowsingContextState { + pub fn window_name(&self) -> String { + self.window_name.lock().clone() + } + + pub fn set_window_name(&self, value: String) { + *self.window_name.lock() = value; + } +} + +impl fmt::Debug for RendererTopLevelBrowsingContextState { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter + .debug_struct("RendererTopLevelBrowsingContextState") + .field("strong_count", &Arc::strong_count(&self.window_name)) + .finish_non_exhaustive() + } +} diff --git a/moli-renderer-v8/src/context_bootstrap/runtime_state.rs b/moli-renderer-v8/src/context_bootstrap/runtime_state.rs index 9cd9305503..71c5594c8b 100644 --- a/moli-renderer-v8/src/context_bootstrap/runtime_state.rs +++ b/moli-renderer-v8/src/context_bootstrap/runtime_state.rs @@ -917,6 +917,20 @@ fn window_name_runtime_getter<'s>( mut rv: v8::ReturnValue<'s, v8::Value>, ) { let receiver = callback_this_object(scope, &args); + let child_handle = child_context_handle_from_owner(scope, receiver); + let is_top_level = receiver.strict_equals(scope.get_current_context().global(scope).into()); + if child_handle.is_none() && !is_top_level { + throw_type_error(scope, "Illegal invocation"); + return; + } + if child_handle.is_none() + && is_top_level + && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) + && let Some(value) = v8_string(scope, &unsafe { &*host_ptr }.top_level_window_name()) + { + rv.set(value.into()); + return; + } let value = object_hidden_value(scope, receiver, WINDOW_NAME_SLOT) .unwrap_or_else(|| v8::String::empty(scope).into()); rv.set(value); @@ -928,15 +942,23 @@ fn window_name_runtime_setter<'s>( _rv: v8::ReturnValue<'s, v8::Value>, ) { let receiver = callback_this_object(scope, &args); + let child_handle = child_context_handle_from_owner(scope, receiver); + let is_top_level = receiver.strict_equals(scope.get_current_context().global(scope).into()); + if child_handle.is_none() && !is_top_level { + throw_type_error(scope, "Illegal invocation"); + return; + } let next = args .get(0) .to_string(scope) .map(|value| value.to_rust_string_lossy(scope)) .unwrap_or_default(); - if let Some(handle) = child_context_handle_from_owner(scope, receiver) + if let Some(handle) = child_handle && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) { unsafe { &mut *host_ptr }.set_child_browsing_context_name(handle, next.clone()); + } else if is_top_level && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) { + unsafe { &*host_ptr }.set_top_level_window_name(next.clone()); } define_non_enumerable_string_property(scope, receiver, WINDOW_NAME_SLOT, &next); } diff --git a/moli-renderer-v8/src/lib.rs b/moli-renderer-v8/src/lib.rs index e53c3b98e8..92c95faf6d 100644 --- a/moli-renderer-v8/src/lib.rs +++ b/moli-renderer-v8/src/lib.rs @@ -30,6 +30,7 @@ mod abort_signal_route; mod app_manifest; mod blob; mod broadcast_channel_runtime; +mod browsing_context_state; mod callback_invocation; #[cfg(test)] mod chromium_property_surface; @@ -183,6 +184,7 @@ pub(crate) use crate::stylesheet_blocking::{ collect_document_owned_blocking_stylesheets_before_in_view, }; +pub use browsing_context_state::RendererTopLevelBrowsingContextState; pub use context_bootstrap::{ DEFAULT_ORIGIN_STORAGE_QUOTA_BYTES, IndexedDbKey, IndexedDbObjectStoreOptions, IndexedDbOpenOptions, IndexedDbTransactionMode, SharedIndexedDbManager, WeakIndexedDbManager, 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 4cf33aed5f..3662bf6c05 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/core.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/core.rs @@ -212,6 +212,7 @@ impl JsContextHost { output_journal: None, page_context_resources_closed: false, page_default_context: None, + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), v8_finalizers: crate::v8_finalizer::V8FinalizerRegistry::default(), bridge: NativeDomBridge::new(bindings), dom_agent_state: crate::runtime::RendererDomAgentState::new( @@ -478,6 +479,20 @@ impl JsContextHost { host } + pub(crate) fn top_level_window_name(&self) -> String { + self.top_level_browsing_context.window_name() + } + + pub(crate) fn top_level_browsing_context_state( + &self, + ) -> crate::RendererTopLevelBrowsingContextState { + self.top_level_browsing_context.clone() + } + + pub(crate) fn set_top_level_window_name(&self, value: String) { + self.top_level_browsing_context.set_window_name(value); + } + pub(crate) fn set_root_document_lifecycle( &mut self, lifecycle: RendererDocumentLifecycleJournalHandle, diff --git a/moli-renderer-v8/src/native_bridge/context_host/host_environment.rs b/moli-renderer-v8/src/native_bridge/context_host/host_environment.rs index 160cd8eddd..292727fc29 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/host_environment.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/host_environment.rs @@ -444,6 +444,13 @@ impl JsContextHost { self.session_storage_store = handles.session_storage(); } + pub(crate) fn set_top_level_browsing_context_state( + &mut self, + state: crate::RendererTopLevelBrowsingContextState, + ) { + self.top_level_browsing_context = state; + } + pub(crate) fn set_stored_document_start_scripts( &mut self, scripts: &[crate::DocumentStartScript], 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 2895814c45..caf14d9ef8 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/mod.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/mod.rs @@ -817,6 +817,7 @@ pub(crate) struct JsContextHost { output_journal: Option, page_context_resources_closed: bool, page_default_context: Option>, + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState, pub(crate) v8_finalizers: crate::v8_finalizer::V8FinalizerRegistry, pub(super) bridge: NativeDomBridge, backend_node_registry: SharedRendererBackendNodeRegistry, diff --git a/moli-renderer-v8/src/runtime/owner/page_creation.rs b/moli-renderer-v8/src/runtime/owner/page_creation.rs index 3125726221..c755cde5d3 100644 --- a/moli-renderer-v8/src/runtime/owner/page_creation.rs +++ b/moli-renderer-v8/src/runtime/owner/page_creation.rs @@ -925,6 +925,7 @@ impl RendererOwnerHandle { root_frame_id, main_document_commit, top_level_storage_key, + top_level_browsing_context: Default::default(), navigation_bootstrap_entry: None, reserved_service_worker_client_id: reserved_service_worker_client .map(RendererReservedServiceWorkerClient::release), @@ -1225,6 +1226,7 @@ impl RendererOwnerHandle { root_frame_id, main_document_commit, top_level_storage_key: None, + top_level_browsing_context: Default::default(), navigation_bootstrap_entry: None, reserved_service_worker_client_id: reserved_service_worker_client .map(RendererReservedServiceWorkerClient::release), diff --git a/moli-renderer-v8/src/runtime/page.rs b/moli-renderer-v8/src/runtime/page.rs index bcb6933071..c86287968a 100644 --- a/moli-renderer-v8/src/runtime/page.rs +++ b/moli-renderer-v8/src/runtime/page.rs @@ -28,6 +28,7 @@ use super::{ /// Applied before document-start scripts; live updates use Page commands. #[derive(Default)] pub struct RendererDocumentOptions { + pub top_level_browsing_context: crate::RendererTopLevelBrowsingContextState, pub indexed_db_manager: Option, pub storage_bucket_store: Option, pub document_start_scripts: Vec, diff --git a/moli-renderer-v8/src/runtime/page_vm/followed_navigation.rs b/moli-renderer-v8/src/runtime/page_vm/followed_navigation.rs index 89dfacb4cb..0d0dd4b615 100644 --- a/moli-renderer-v8/src/runtime/page_vm/followed_navigation.rs +++ b/moli-renderer-v8/src/runtime/page_vm/followed_navigation.rs @@ -1176,6 +1176,7 @@ impl PageVm { fn followed_location_navigation_env(&self) -> PageVmEnvConfig { PageVmEnvConfig { + top_level_browsing_context: self.vm().top_level_browsing_context_state(), main_document_commit: None, web_storage: self.vm().web_storage_handles(), document_start_scripts: self.document_start_scripts.clone(), diff --git a/moli-renderer-v8/src/runtime/page_vm/mod.rs b/moli-renderer-v8/src/runtime/page_vm/mod.rs index 572baad675..645abd3f13 100644 --- a/moli-renderer-v8/src/runtime/page_vm/mod.rs +++ b/moli-renderer-v8/src/runtime/page_vm/mod.rs @@ -1066,6 +1066,7 @@ async fn execute_page_owned_work_on_script_execution_lane( /// environment setup does not grow owner-loop wiring fields. #[derive(Clone)] pub(crate) struct PageVmEnvConfig { + pub(crate) top_level_browsing_context: crate::RendererTopLevelBrowsingContextState, pub(crate) root_frame_id: Option, pub(crate) main_document_commit: Option, pub(crate) top_level_storage_key: Option, @@ -4357,6 +4358,9 @@ impl PageVm { .set_storage_bucket_store(storage_bucket_store); } page_vm.vm_mut().set_web_storage_handles(&env.web_storage); + page_vm + .vm_mut() + .set_top_level_browsing_context_state(env.top_level_browsing_context.clone()); page_vm .vm_mut() .set_script_execution_disabled(env.script_execution_disabled); diff --git a/moli-renderer-v8/src/runtime/page_vm/test_support.rs b/moli-renderer-v8/src/runtime/page_vm/test_support.rs index 359d7d853d..e89ebf5716 100644 --- a/moli-renderer-v8/src/runtime/page_vm/test_support.rs +++ b/moli-renderer-v8/src/runtime/page_vm/test_support.rs @@ -669,6 +669,7 @@ impl DerefMut for PageVmTaskExecutorTestHarness { fn minimal_test_page_vm_env_config() -> PageVmEnvConfig { PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), web_storage: crate::RendererWebStorageHandles::ephemeral(), root_frame_id: None, main_document_commit: None, diff --git a/moli-renderer-v8/src/runtime/phase_one/mod.rs b/moli-renderer-v8/src/runtime/phase_one/mod.rs index e23da710ee..b7c3ceab4d 100644 --- a/moli-renderer-v8/src/runtime/phase_one/mod.rs +++ b/moli-renderer-v8/src/runtime/phase_one/mod.rs @@ -652,6 +652,7 @@ mod tests { pub(super) fn default_test_page_vm_env_config() -> PageVmEnvConfig { PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), web_storage: crate::RendererWebStorageHandles::ephemeral(), root_frame_id: None, main_document_commit: None, diff --git a/moli-renderer-v8/src/runtime/phase_one/streaming.rs b/moli-renderer-v8/src/runtime/phase_one/streaming.rs index 742ddb222b..143d6762c8 100644 --- a/moli-renderer-v8/src/runtime/phase_one/streaming.rs +++ b/moli-renderer-v8/src/runtime/phase_one/streaming.rs @@ -1079,6 +1079,7 @@ mod tests { fn default_test_page_vm_env_config() -> PageVmEnvConfig { PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), web_storage: crate::RendererWebStorageHandles::ephemeral(), root_frame_id: None, main_document_commit: None, diff --git a/moli-renderer-v8/src/runtime/phase_one/tests/extracted/parser_blocking_scripts.rs b/moli-renderer-v8/src/runtime/phase_one/tests/extracted/parser_blocking_scripts.rs index 3eafa3a496..95b20bb744 100644 --- a/moli-renderer-v8/src/runtime/phase_one/tests/extracted/parser_blocking_scripts.rs +++ b/moli-renderer-v8/src/runtime/phase_one/tests/extracted/parser_blocking_scripts.rs @@ -725,6 +725,7 @@ fn parser_owner_boundary_with_live_backend_queues_document_turn_before_runtime_w local_executor, &loader, &PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), web_storage: crate::RendererWebStorageHandles::ephemeral(), root_frame_id: None, main_document_commit: None, @@ -1016,6 +1017,7 @@ fn parser_connected_head_script_does_not_push_later_head_tokens_into_body() { local_executor, loader, &PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), web_storage: crate::RendererWebStorageHandles::ephemeral(), root_frame_id: None, main_document_commit: None, @@ -1195,6 +1197,7 @@ fn parser_connected_external_head_script_with_live_head_and_body_mutation_keeps_ local_executor, loader, &PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), web_storage: crate::RendererWebStorageHandles::ephemeral(), root_frame_id: None, main_document_commit: None, @@ -1796,6 +1799,7 @@ fn parser_owner_body_stylesheet_pause_preserves_unconsumed_tail_on_live_page_vm( local_executor, &loader, &PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), root_frame_id: None, main_document_commit: None, top_level_storage_key: None, diff --git a/moli-renderer-v8/src/runtime/phase_one/tests/extracted/parser_script_handoffs.rs b/moli-renderer-v8/src/runtime/phase_one/tests/extracted/parser_script_handoffs.rs index 8f5431be17..b7949dfef4 100644 --- a/moli-renderer-v8/src/runtime/phase_one/tests/extracted/parser_script_handoffs.rs +++ b/moli-renderer-v8/src/runtime/phase_one/tests/extracted/parser_script_handoffs.rs @@ -332,6 +332,7 @@ fn external_async_handoff_marks_parser_stream_already_started_on_live_backend() local_executor, &loader, &PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), web_storage: crate::RendererWebStorageHandles::ephemeral(), root_frame_id: None, main_document_commit: None, @@ -458,6 +459,7 @@ fn blocking_classic_handoff_registers_parser_owned_handle_on_live_backend() { local_executor, &loader, &PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), web_storage: crate::RendererWebStorageHandles::ephemeral(), root_frame_id: None, main_document_commit: None, @@ -594,6 +596,7 @@ fn non_async_post_parse_handoff_registers_pending_before_source_and_seals_withou local_executor, &loader, &PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), web_storage: crate::RendererWebStorageHandles::ephemeral(), root_frame_id: None, main_document_commit: None, @@ -765,6 +768,7 @@ fn parser_owned_external_module_handoff_starts_pending_script_tree_root_fetch() local_executor, &loader, &PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), web_storage: crate::RendererWebStorageHandles::ephemeral(), root_frame_id: None, main_document_commit: None, @@ -1082,6 +1086,7 @@ fn parser_owned_inline_importmap_handoff_registers_parser_owned_handle_on_live_b local_executor, loader, &PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), web_storage: crate::RendererWebStorageHandles::ephemeral(), root_frame_id: None, main_document_commit: None, @@ -1249,6 +1254,7 @@ fn parser_owner_style_import_handoff_is_stylesheet_gated_on_live_page_vm() { local_executor, &loader, &PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), web_storage: crate::RendererWebStorageHandles::ephemeral(), root_frame_id: None, main_document_commit: None, diff --git a/moli-renderer-v8/src/runtime/phase_one/tests/extracted/tree_mutations_and_document_write.rs b/moli-renderer-v8/src/runtime/phase_one/tests/extracted/tree_mutations_and_document_write.rs index 9b013e68da..22380d896e 100644 --- a/moli-renderer-v8/src/runtime/phase_one/tests/extracted/tree_mutations_and_document_write.rs +++ b/moli-renderer-v8/src/runtime/phase_one/tests/extracted/tree_mutations_and_document_write.rs @@ -1126,6 +1126,7 @@ fn parser_connected_head_document_write_keeps_later_head_tokens_in_head() { local_executor, loader, &PageVmEnvConfig { + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState::default(), web_storage: crate::RendererWebStorageHandles::ephemeral(), root_frame_id: None, main_document_commit: None, diff --git a/moli-renderer-v8/src/script_vm/browsing_contexts.rs b/moli-renderer-v8/src/script_vm/browsing_contexts.rs index d7e342c699..f2c138c58c 100644 --- a/moli-renderer-v8/src/script_vm/browsing_contexts.rs +++ b/moli-renderer-v8/src/script_vm/browsing_contexts.rs @@ -201,6 +201,23 @@ impl ScriptVm { .set_web_storage_handles(handles); } + pub(crate) fn set_top_level_browsing_context_state( + &mut self, + state: crate::RendererTopLevelBrowsingContextState, + ) { + self._context_host + .borrow_mut() + .set_top_level_browsing_context_state(state); + } + + pub(crate) fn top_level_browsing_context_state( + &self, + ) -> crate::RendererTopLevelBrowsingContextState { + self._context_host + .borrow() + .top_level_browsing_context_state() + } + pub(crate) fn web_storage_handles(&self) -> crate::RendererWebStorageHandles { let host = self._context_host.borrow(); crate::RendererWebStorageHandles::new( diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/window_surface.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/window_surface.rs index 5acb350809..ee6e6f6118 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/window_surface.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/window_surface.rs @@ -833,3 +833,33 @@ fn window_dialog_and_open_arguments_use_webidl_conversion() { "undefined|TypeError|false|RangeError|null|TypeError|[object Window]|TypeError|TypeError|RangeError" ); } +#[test] +fn window_name_accessors_reject_non_window_receivers_without_mutating_the_window() { + let mut vm = new_storage_test_vm("https://example.com/"); + + let result = vm + .eval( + r#" + (() => { + const descriptor = Object.getOwnPropertyDescriptor(window, "name"); + const outcome = callback => { + try { callback(); return "ok"; } + catch (error) { return error.name; } + }; + const before = window.name; + return JSON.stringify({ + setter: outcome(() => descriptor.set.call({}, "forged")), + getter: outcome(() => descriptor.get.call({})), + before, + after: window.name + }); + })() + "#, + ) + .expect("window.name illegal receiver probe should evaluate"); + + assert_eq!( + result, + r#"{"setter":"TypeError","getter":"TypeError","before":"","after":""}"# + ); +} From ad381b33c42824c69a83d3f640f5b706f7d1a590 Mon Sep 17 00:00:00 2001 From: lanyue-llk <270302213+lanyue-llk@users.noreply.github.com> Date: Tue, 29 Sep 2026 16:43:52 +0800 Subject: [PATCH 3/8] fix(browser): preserve window name ownership across documents --- .../tests/tests_background_staging/runtime.rs | 22 +++++++++ .../src/context_bootstrap/runtime_state.rs | 41 +++++++++++------ moli-renderer-v8/src/runtime/owner.rs | 2 + .../src/runtime/owner/page_creation.rs | 8 +++- .../misc/extracted/window_surface.rs | 46 +++++++++++++++++++ 5 files changed, 102 insertions(+), 17 deletions(-) diff --git a/moli-protocol/src/domains/target/tests/tests_background_staging/runtime.rs b/moli-protocol/src/domains/target/tests/tests_background_staging/runtime.rs index 6f3a21728f..5eb18c288c 100644 --- a/moli-protocol/src/domains/target/tests/tests_background_staging/runtime.rs +++ b/moli-protocol/src/domains/target/tests/tests_background_staging/runtime.rs @@ -564,6 +564,28 @@ async fn same_context_named_popup_reuse_navigates_and_activates_loaded_owner() { .await; let response = take_response_by_id(&mut ctx, 1041949447); assert_eq!(response["result"]["result"]["value"], json!("named target")); + + ctx.process_async(json!({ + "id": 1041949448, + "method": "Runtime.evaluate", + "sessionId": owner.session_id, + "params": { + "expression": "window.name" + } + })) + .await; + let response = take_response_by_id(&mut ctx, 1041949448); + assert_eq!( + response["result"]["result"]["value"], + json!("reportWindow"), + "a reused named target must retain its browsing-context name across Document replacement" + ); + let browser_context = ctx.conn.browser_context.as_ref().expect("browser context"); + assert_eq!( + browser_context.target_id_for_window_name(active_target_id, "reportWindow"), + Some(owner.target_id.as_str()), + "the popup proxy name and the live target engine must share one browsing-context owner" + ); }) .await; } diff --git a/moli-renderer-v8/src/context_bootstrap/runtime_state.rs b/moli-renderer-v8/src/context_bootstrap/runtime_state.rs index 71c5594c8b..f8cebb3f7e 100644 --- a/moli-renderer-v8/src/context_bootstrap/runtime_state.rs +++ b/moli-renderer-v8/src/context_bootstrap/runtime_state.rs @@ -917,15 +917,13 @@ fn window_name_runtime_getter<'s>( mut rv: v8::ReturnValue<'s, v8::Value>, ) { let receiver = callback_this_object(scope, &args); - let child_handle = child_context_handle_from_owner(scope, receiver); - let is_top_level = receiver.strict_equals(scope.get_current_context().global(scope).into()); - if child_handle.is_none() && !is_top_level { + if !super::window_receiver::is_window_receiver(scope, receiver) { throw_type_error(scope, "Illegal invocation"); return; } + let child_handle = child_context_handle_from_owner(scope, receiver); if child_handle.is_none() - && is_top_level - && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) + && let Some(host_ptr) = context_host_ptr_from_window_name_receiver(scope, receiver) && let Some(value) = v8_string(scope, &unsafe { &*host_ptr }.top_level_window_name()) { rv.set(value.into()); @@ -942,27 +940,40 @@ fn window_name_runtime_setter<'s>( _rv: v8::ReturnValue<'s, v8::Value>, ) { let receiver = callback_this_object(scope, &args); - let child_handle = child_context_handle_from_owner(scope, receiver); - let is_top_level = receiver.strict_equals(scope.get_current_context().global(scope).into()); - if child_handle.is_none() && !is_top_level { + if !super::window_receiver::is_window_receiver(scope, receiver) { throw_type_error(scope, "Illegal invocation"); return; } - let next = args - .get(0) - .to_string(scope) - .map(|value| value.to_rust_string_lossy(scope)) - .unwrap_or_default(); + let child_handle = child_context_handle_from_owner(scope, receiver); + let Some(next) = args.get(0).to_string(scope) else { + return; + }; + let next = next.to_rust_string_lossy(scope); + let host_ptr = context_host_ptr_from_window_name_receiver(scope, receiver); if let Some(handle) = child_handle - && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) + && let Some(host_ptr) = host_ptr { unsafe { &mut *host_ptr }.set_child_browsing_context_name(handle, next.clone()); - } else if is_top_level && let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) { + } else if child_handle.is_none() + && let Some(host_ptr) = host_ptr + { unsafe { &*host_ptr }.set_top_level_window_name(next.clone()); } define_non_enumerable_string_property(scope, receiver, WINDOW_NAME_SLOT, &next); } +fn context_host_ptr_from_window_name_receiver<'s>( + scope: &mut v8::PinScope<'s, '_>, + receiver: v8::Local<'s, v8::Object>, +) -> Option<*mut JsContextHost> { + context_host_ptr_from_window_object(scope, receiver).or_else(|| { + receiver + .strict_equals(scope.get_current_context().global(scope).into()) + .then(|| context_host_ptr_from_global_bridge(scope)) + .flatten() + }) +} + fn install_public_window_surface_accessors<'s>( scope: &mut v8::PinScope<'s, '_>, global: v8::Local<'s, v8::Object>, diff --git a/moli-renderer-v8/src/runtime/owner.rs b/moli-renderer-v8/src/runtime/owner.rs index 62ceeddec0..f09d667296 100644 --- a/moli-renderer-v8/src/runtime/owner.rs +++ b/moli-renderer-v8/src/runtime/owner.rs @@ -140,6 +140,7 @@ pub struct RendererCreateHtmlPageRequest { pub root_frame_id: Option, pub main_document_commit: Option, pub top_level_storage_key: Option, + pub top_level_browsing_context: crate::RendererTopLevelBrowsingContextState, pub requested_url: Url, pub navigation_initiator_url: Option, pub navigation_redirected: bool, @@ -189,6 +190,7 @@ pub struct RendererCreateStreamingRawPageRequest { Option>, pub root_frame_id: Option, pub main_document_commit: Option, + pub top_level_browsing_context: crate::RendererTopLevelBrowsingContextState, pub requested_url: Url, pub final_url: Url, pub navigation_initiator_url: Option, diff --git a/moli-renderer-v8/src/runtime/owner/page_creation.rs b/moli-renderer-v8/src/runtime/owner/page_creation.rs index c755cde5d3..f46dfcb9b1 100644 --- a/moli-renderer-v8/src/runtime/owner/page_creation.rs +++ b/moli-renderer-v8/src/runtime/owner/page_creation.rs @@ -662,6 +662,7 @@ impl RendererOwnerHandle { root_frame_id: options.root_frame_id, main_document_commit: options.main_document_commit, top_level_storage_key: None, + top_level_browsing_context: options.top_level_browsing_context, requested_url, navigation_initiator_url, navigation_redirected, @@ -725,6 +726,7 @@ impl RendererOwnerHandle { document_replacement: None, root_frame_id: options.root_frame_id, main_document_commit: options.main_document_commit, + top_level_browsing_context: options.top_level_browsing_context, requested_url, final_url, navigation_initiator_url, @@ -801,6 +803,7 @@ impl RendererOwnerHandle { root_frame_id, main_document_commit, top_level_storage_key, + top_level_browsing_context, requested_url, navigation_initiator_url, navigation_redirected, @@ -925,7 +928,7 @@ impl RendererOwnerHandle { root_frame_id, main_document_commit, top_level_storage_key, - top_level_browsing_context: Default::default(), + top_level_browsing_context, navigation_bootstrap_entry: None, reserved_service_worker_client_id: reserved_service_worker_client .map(RendererReservedServiceWorkerClient::release), @@ -1113,6 +1116,7 @@ impl RendererOwnerHandle { document_replacement: _document_replacement, root_frame_id, main_document_commit, + top_level_browsing_context, requested_url, final_url, navigation_initiator_url, @@ -1226,7 +1230,7 @@ impl RendererOwnerHandle { root_frame_id, main_document_commit, top_level_storage_key: None, - top_level_browsing_context: Default::default(), + top_level_browsing_context, navigation_bootstrap_entry: None, reserved_service_worker_client_id: reserved_service_worker_client .map(RendererReservedServiceWorkerClient::release), diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/window_surface.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/window_surface.rs index ee6e6f6118..be4348deb9 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/window_surface.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/window_surface.rs @@ -863,3 +863,49 @@ fn window_name_accessors_reject_non_window_receivers_without_mutating_the_window r#"{"setter":"TypeError","getter":"TypeError","before":"","after":""}"# ); } + +#[test] +fn window_name_accessors_follow_the_receiver_realm_and_preserve_state_on_conversion_errors() { + let mut vm = new_storage_test_vm("https://example.com/"); + + let result = vm + .eval( + r#" + (() => { + const frame = document.createElement("iframe"); + (document.body || document.documentElement || document).appendChild(frame); + const child = frame.contentWindow; + const parentDescriptor = Object.getOwnPropertyDescriptor(window, "name"); + const childDescriptor = Object.getOwnPropertyDescriptor(child, "name"); + + parentDescriptor.set.call(child, "child-from-parent-realm"); + childDescriptor.set.call(window, "parent-from-child-realm"); + const borrowedParent = childDescriptor.get.call(window); + const borrowedChild = parentDescriptor.get.call(child); + + let conversionError; + try { + childDescriptor.set.call(window, { + toString() { throw new RangeError("window-name-conversion"); } + }); + } catch (error) { + conversionError = `${error.name}:${error.message}`; + } + + return JSON.stringify({ + borrowedParent, + borrowedChild, + parentAfterError: window.name, + childAfterError: child.name, + conversionError + }); + })() + "#, + ) + .expect("borrowed Window.name accessors should preserve their receiver realm"); + + assert_eq!( + result, + r#"{"borrowedParent":"parent-from-child-realm","borrowedChild":"child-from-parent-realm","parentAfterError":"parent-from-child-realm","childAfterError":"child-from-parent-realm","conversionError":"RangeError:window-name-conversion"}"# + ); +} From 5919dde5492141ecb77354784c107228f8662fad Mon Sep 17 00:00:00 2001 From: lanyue-llk <270302213+lanyue-llk@users.noreply.github.com> Date: Tue, 29 Sep 2026 17:42:39 +0800 Subject: [PATCH 4/8] fix(browser): unify named popup ownership --- .../src/page/renderer_command_support.rs | 4 - .../src/protocol_server/webdriver_bidi.rs | 1 - .../src/commands/window.rs | 1 - moli-protocol/src/conn/dispatch_tests/page.rs | 1 - moli-protocol/src/devtools_runtime.rs | 1 - moli-protocol/src/domains/page/capture.rs | 42 +--- .../domains/page/protocol_neutral_tests.rs | 7 +- .../src/domains/page/tests/capture.rs | 64 ------ moli-protocol/src/domains/target/popup.rs | 13 +- .../target/tests/tests_target_creation.rs | 53 ++++- .../src/context_bootstrap/runtime_state.rs | 97 ++++++--- .../src/context_bootstrap/window_receiver.rs | 11 +- .../src/native_bridge/context_host/popups.rs | 2 +- moli-renderer-v8/src/runtime/page_commands.rs | 3 - moli-renderer-v8/src/runtime/page_geometry.rs | 11 - moli-renderer-v8/src/runtime/page_surface.rs | 2 - .../tests/rendering_update/layout_geometry.rs | 201 ------------------ .../misc/extracted/window_surface.rs | 34 ++- 18 files changed, 175 insertions(+), 373 deletions(-) diff --git a/moli-core/src/page/renderer_command_support.rs b/moli-core/src/page/renderer_command_support.rs index 78d626ca8f..24770e6170 100644 --- a/moli-core/src/page/renderer_command_support.rs +++ b/moli-core/src/page/renderer_command_support.rs @@ -1466,10 +1466,6 @@ impl Page { self.start_page_command(RendererPageCommand::LayoutMetrics) } - pub fn start_published_layout_metrics(&self) -> Result { - self.start_page_command(RendererPageCommand::PublishLayoutMetrics) - } - pub fn finish_layout_metrics( &mut self, completion: CompletedPageCommand, diff --git a/moli-protocol-server/src/protocol_server/webdriver_bidi.rs b/moli-protocol-server/src/protocol_server/webdriver_bidi.rs index 13c0b524d9..eb7179e52a 100644 --- a/moli-protocol-server/src/protocol_server/webdriver_bidi.rs +++ b/moli-protocol-server/src/protocol_server/webdriver_bidi.rs @@ -2949,7 +2949,6 @@ async fn bidi_input_viewport_bounds( target_id: Some(DevToolsTargetId::from(context_id)), browser_context_id: None, }, - publish_layout: false, }); match scheduler .execute_devtools_command_with_protocol_messages(command) diff --git a/moli-protocol-webdriver-classic/src/commands/window.rs b/moli-protocol-webdriver-classic/src/commands/window.rs index 0a4529d549..92d6d7b66b 100644 --- a/moli-protocol-webdriver-classic/src/commands/window.rs +++ b/moli-protocol-webdriver-classic/src/commands/window.rs @@ -31,7 +31,6 @@ pub fn create_initial_target_command(context: &ClassicDevToolsCommandContext) -> pub fn layout_metrics_command(context: &ClassicDevToolsCommandContext) -> DevToolsCommand { DevToolsCommand::GetLayoutMetrics(DevToolsGetLayoutMetricsCommand { context: context.command_context(), - publish_layout: false, }) } diff --git a/moli-protocol/src/conn/dispatch_tests/page.rs b/moli-protocol/src/conn/dispatch_tests/page.rs index adf4943ca7..7ecc0ba766 100644 --- a/moli-protocol/src/conn/dispatch_tests/page.rs +++ b/moli-protocol/src/conn/dispatch_tests/page.rs @@ -913,7 +913,6 @@ async fn devtools_command_executes_context_viewport_override() { target_id: Some(target_id), ..context }, - publish_layout: false, }, )) .await diff --git a/moli-protocol/src/devtools_runtime.rs b/moli-protocol/src/devtools_runtime.rs index 69ebc42691..037d771542 100644 --- a/moli-protocol/src/devtools_runtime.rs +++ b/moli-protocol/src/devtools_runtime.rs @@ -605,7 +605,6 @@ pub struct DevToolsGetFrameTreesCommand { #[derive(Debug, Clone, PartialEq, Eq)] pub struct DevToolsGetLayoutMetricsCommand { pub context: DevToolsCommandContext, - pub publish_layout: bool, } #[derive(Debug, Clone, PartialEq, Eq)] diff --git a/moli-protocol/src/domains/page/capture.rs b/moli-protocol/src/domains/page/capture.rs index 8ff839e6d6..23545e1cb6 100644 --- a/moli-protocol/src/domains/page/capture.rs +++ b/moli-protocol/src/domains/page/capture.rs @@ -1,12 +1,5 @@ use super::*; -#[derive(Default, Deserialize)] -#[serde(rename_all = "camelCase")] -struct GetLayoutMetricsParams { - #[serde(default)] - publish_layout: bool, -} - #[derive(Deserialize)] #[serde(rename_all = "camelCase")] pub(super) struct ScreenshotClip { @@ -364,16 +357,13 @@ pub(super) async fn execute_devtools_get_layout_metrics_command( command: DevToolsGetLayoutMetricsCommand, ) -> Result { let owner = page_command_owner(conn, &command.context)?; - let result = - execute_devtools_get_layout_metrics_for_current_owner(conn, &owner, command.publish_layout) - .await; + let result = execute_devtools_get_layout_metrics_for_current_owner(conn, &owner).await; result.map(DevToolsCommandResult::LayoutMetrics) } pub(super) async fn execute_devtools_get_layout_metrics_for_current_owner( conn: &mut CdpConnection, owner: &CommandOwnerScope, - publish_layout: bool, ) -> Result { let Some(page) = conn .runtime_session_owner_slot_mut_for_owner(owner) @@ -382,12 +372,7 @@ pub(super) async fn execute_devtools_get_layout_metrics_for_current_owner( else { return Err(devtools_layout_metrics_error("NoDocumentLoaded")); }; - let pending = if publish_layout { - page.start_published_layout_metrics() - } else { - page.start_layout_metrics() - } - .map_err(|error| { + let pending = page.start_layout_metrics().map_err(|error| { devtools_layout_metrics_error(format!("Failed to start layout metrics: {error}")) })?; let completed = pending.wait().await.map_err(|error| { @@ -687,29 +672,21 @@ pub(super) fn start_devtools_print_to_pdf_command( pub(super) fn build_cdp_get_layout_metrics_command( conn: &CdpConnection, cmd: &Cmd<'_>, -) -> Result { - let params = cmd - .get_params::() - .map_err(|message| CommandOutputPlan::error(-32602, message))? - .unwrap_or_default(); +) -> crate::devtools_runtime::DevToolsGetLayoutMetricsCommand { let (browser_context_id, target_id) = conn .target_owner_identity_for_session(cmd.session_id) .map(|(browser_context_id, target_id)| (Some(browser_context_id), target_id)) .unwrap_or((None, None)); - Ok(crate::devtools_runtime::DevToolsGetLayoutMetricsCommand { + crate::devtools_runtime::DevToolsGetLayoutMetricsCommand { context: cmd.devtools_command_context(target_id.as_deref(), browser_context_id.as_deref()), - publish_layout: params.publish_layout, - }) + } } pub(super) fn try_start_page_get_layout_metrics_command( conn: &mut CdpConnection, cmd: &Cmd<'_>, ) -> PageCommandTaskStep { - let command = match build_cdp_get_layout_metrics_command(conn, cmd) { - Ok(command) => command, - Err(plan) => return PageCommandTaskStep::Complete(plan), - }; + let command = build_cdp_get_layout_metrics_command(conn, cmd); start_devtools_page_command(conn, cmd.id, DevToolsCommand::GetLayoutMetrics(command)) } @@ -729,12 +706,7 @@ pub(super) fn start_devtools_get_layout_metrics_command( devtools_layout_metrics_error("NoDocumentLoaded"), )); }; - let pending = if command.publish_layout { - page.start_published_layout_metrics() - } else { - page.start_layout_metrics() - }; - match pending { + match page.start_layout_metrics() { Ok(pending) => PageCommandTaskStep::Pending(PendingPageCommandDispatch { command_id, owner_scope, diff --git a/moli-protocol/src/domains/page/protocol_neutral_tests.rs b/moli-protocol/src/domains/page/protocol_neutral_tests.rs index d4cd60be3a..0c4e66e027 100644 --- a/moli-protocol/src/domains/page/protocol_neutral_tests.rs +++ b/moli-protocol/src/domains/page/protocol_neutral_tests.rs @@ -74,8 +74,7 @@ fn cdp_get_layout_metrics_builds_protocol_neutral_command() { r#"{"id":122,"method":"Page.getLayoutMetrics"}"#, ); - let command = - build_cdp_get_layout_metrics_command(&conn, &cmd).expect("valid layout metrics command"); + let command = build_cdp_get_layout_metrics_command(&conn, &cmd); assert_eq!(command.context.protocol, DevToolsProtocol::Cdp); assert_eq!( @@ -84,7 +83,6 @@ fn cdp_get_layout_metrics_builds_protocol_neutral_command() { ); assert_eq!(command.context.target_id, None); assert_eq!(command.context.browser_context_id, None); - assert!(!command.publish_layout); } #[test] @@ -98,8 +96,7 @@ fn devtools_page_entry_routes_get_layout_metrics_command_to_page_owner() { None, r#"{"id":123,"method":"Page.getLayoutMetrics"}"#, ); - let command = - build_cdp_get_layout_metrics_command(&conn, &cmd).expect("valid layout metrics command"); + let command = build_cdp_get_layout_metrics_command(&conn, &cmd); let step = start_devtools_page_command( &mut conn, diff --git a/moli-protocol/src/domains/page/tests/capture.rs b/moli-protocol/src/domains/page/tests/capture.rs index 6d6af8ca4c..b914934121 100644 --- a/moli-protocol/src/domains/page/tests/capture.rs +++ b/moli-protocol/src/domains/page/tests/capture.rs @@ -1345,70 +1345,6 @@ async fn get_layout_metrics_queries_live_renderer_for_loaded_pages() { "content size should come from a one-shot live layout: {metrics:?}" ); } - -#[tokio::test(flavor = "multi_thread")] -async fn get_layout_metrics_can_explicitly_publish_without_a_screenshot() { - let mut ctx = TestContext::new(); - load_bc_with_session( - &mut ctx, - "BID-PUBLISH-LAYOUT-METRICS", - "TID-PUBLISH-LAYOUT-METRICS", - "SID-PUBLISH-LAYOUT-METRICS", - "about:blank", - ); - let page_url = "data:text/html,
"; - let page = ctx - .conn - .load_page_via_runtime_async(page_url) - .await - .expect("page should load"); - ctx.conn - .browser_context - .as_mut() - .expect("browser context") - .active_page_target_mut() - .runtime_slot - .replace_loaded_page(Some(page)); - - ctx.process_async(json!({ - "id": 126, - "method": "Page.getLayoutMetrics", - "sessionId": "SID-PUBLISH-LAYOUT-METRICS" - })) - .await; - let sampled = take_response_by_id(&mut ctx, 126); - assert_eq!( - sampled["result"]["contentSize"], - json!({ "x": 0, "y": 0, "width": 2300.0, "height": 1500.0 }), - "ordinary Page.getLayoutMetrics must keep its one-shot live-layout contract" - ); - - ctx.process_async(json!({ - "id": 127, - "method": "Page.getLayoutMetrics", - "sessionId": "SID-PUBLISH-LAYOUT-METRICS", - "params": {"publishLayout": true} - })) - .await; - let published = take_response_by_id(&mut ctx, 127); - assert_eq!( - published["result"]["contentSize"], - json!({ "x": 0, "y": 0, "width": 2300.0, "height": 1500.0 }) - ); - - ctx.process_async(json!({ - "id": 128, - "method": "Page.getLayoutMetrics", - "sessionId": "SID-PUBLISH-LAYOUT-METRICS" - })) - .await; - let cached = take_response_by_id(&mut ctx, 128); - assert_eq!( - cached["result"]["contentSize"], - published["result"]["contentSize"] - ); -} - #[tokio::test(flavor = "multi_thread")] async fn get_layout_metrics_targets_loaded_background_owner_without_activation() { let mut ctx = TestContext::new(); diff --git a/moli-protocol/src/domains/target/popup.rs b/moli-protocol/src/domains/target/popup.rs index 21c8b04e72..17ea83d6e3 100644 --- a/moli-protocol/src/domains/target/popup.rs +++ b/moli-protocol/src/domains/target/popup.rs @@ -95,10 +95,12 @@ pub(crate) async fn create_popup_target_from_renderer_output_background_events_a return None; }; - if let Some(existing_target_id) = opener - .as_ref() - .and_then(|opener| { - browser_context.target_id_for_window_name(&opener.target_id, &target_name) + if let Some(existing_target_id) = popup_id + .and_then(|popup_id| browser_context.target_id_for_popup_id(popup_id)) + .or_else(|| { + opener.as_ref().and_then(|opener| { + browser_context.target_id_for_window_name(&opener.target_id, &target_name) + }) }) .map(str::to_owned) { @@ -126,6 +128,9 @@ pub(crate) async fn create_popup_target_from_renderer_output_background_events_a browser_context.update_target_url(&existing_target_id, url.clone()) }); if target_url_updated { + if let Some(browser_context) = conn.browser_context_by_id_mut(&browser_context_id) { + browser_context.remember_target_window_name(&target_name, &existing_target_id); + } emit_target_info_changed_for_target_background_event( conn, out, diff --git a/moli-protocol/src/domains/target/tests/tests_target_creation.rs b/moli-protocol/src/domains/target/tests/tests_target_creation.rs index 24fbe712ab..17aa814671 100644 --- a/moli-protocol/src/domains/target/tests/tests_target_creation.rs +++ b/moli-protocol/src/domains/target/tests/tests_target_creation.rs @@ -1790,7 +1790,7 @@ async fn window_open_named_target_reuses_existing_popup_target() { "method": "Runtime.evaluate", "sessionId": opener_session_id, "params": { - "expression": "window.open('data:text/html,first-popup', 'reportWindow') !== null" + "expression": "window.__namedPopup = window.open('data:text/html,first-popup', 'reportWindow'); window.__namedPopup !== null" } })) .await; @@ -1879,6 +1879,57 @@ async fn window_open_named_target_reuses_existing_popup_target() { browser_context.target_url(), "data:text/html,second-popup" ); + + ctx.process_async(json!({ + "id": 16, + "method": "Runtime.evaluate", + "sessionId": opener_session_id, + "params": { + "expression": "window.__namedPopup.name = 'renamedWindow'; window.open('data:text/html,renamed-popup', 'renamedWindow') !== null" + } + })) + .await; + let renamed_sent = ctx.take_all(); + assert!( + !renamed_sent + .iter() + .any(|message| message["method"] == json!("Target.targetCreated")), + "opening a popup by its live renamed Window.name must reuse its bound target: {renamed_sent:?}" + ); + ctx.wait_for_scheduler_message("renamed popup target navigation", |message| { + message["method"] == json!("Target.targetInfoChanged") + && message["params"]["targetInfo"]["targetId"] == json!(target_id) + && message["params"]["targetInfo"]["url"] + == json!("data:text/html,renamed-popup") + }) + .await; + let browser_context = ctx.conn.browser_context.as_ref().unwrap(); + assert_eq!( + browser_context.target_id_for_window_name("TID-opener-name", "renamedWindow"), + Some(target_id.as_str()) + ); + assert_eq!( + browser_context.target_id_for_window_name("TID-opener-name", "reportWindow"), + None, + "renaming a live popup must invalidate its previous target name" + ); + + ctx.process_async(json!({ + "id": 17, + "method": "Runtime.evaluate", + "sessionId": opener_session_id, + "params": { + "expression": "window.open('about:blank', 'reportWindow') !== null" + } + })) + .await; + let old_name_sent = ctx.take_all(); + assert!( + old_name_sent + .iter() + .any(|message| message["method"] == json!("Target.targetCreated")), + "the popup's previous name must no longer resolve to its target: {old_name_sent:?}" + ); }) .await; } diff --git a/moli-renderer-v8/src/context_bootstrap/runtime_state.rs b/moli-renderer-v8/src/context_bootstrap/runtime_state.rs index f8cebb3f7e..f2cb18f009 100644 --- a/moli-renderer-v8/src/context_bootstrap/runtime_state.rs +++ b/moli-renderer-v8/src/context_bootstrap/runtime_state.rs @@ -31,9 +31,10 @@ use crate::{ }, network_host, util::{ - callback_data_index_value, callback_data_item, context_host_ptr_from_global_bridge, - context_host_ptr_from_window_object, create_script_origin_with_base_url, get_private_value, - script_base_url_continuation_data, set_private_value, throw_type_error, v8_string, v8str, + callback_data_index_value, callback_data_item, context_host_ptr_from_context_slot, + context_host_ptr_from_global_bridge, context_host_ptr_from_window_object, + create_script_origin_with_base_url, get_private_value, script_base_url_continuation_data, + set_private_value, throw_type_error, v8_string, v8str, }, webidl, webidl_iterator::install_webidl_collection_iterator_intrinsics, @@ -917,20 +918,20 @@ fn window_name_runtime_getter<'s>( mut rv: v8::ReturnValue<'s, v8::Value>, ) { let receiver = callback_this_object(scope, &args); - if !super::window_receiver::is_window_receiver(scope, receiver) { + let Some(owner) = window_name_owner(scope, receiver) else { throw_type_error(scope, "Illegal invocation"); return; + }; + let value = match owner { + WindowNameOwner::TopLevel(host_ptr) => { + v8_string(scope, &unsafe { &*host_ptr }.top_level_window_name()) + .map(v8::Local::::from) + } + WindowNameOwner::Child { .. } | WindowNameOwner::LightweightPopup { .. } => { + object_hidden_value(scope, receiver, WINDOW_NAME_SLOT) + } } - let child_handle = child_context_handle_from_owner(scope, receiver); - if child_handle.is_none() - && let Some(host_ptr) = context_host_ptr_from_window_name_receiver(scope, receiver) - && let Some(value) = v8_string(scope, &unsafe { &*host_ptr }.top_level_window_name()) - { - rv.set(value.into()); - return; - } - let value = object_hidden_value(scope, receiver, WINDOW_NAME_SLOT) - .unwrap_or_else(|| v8::String::empty(scope).into()); + .unwrap_or_else(|| v8::String::empty(scope).into()); rv.set(value); } @@ -940,38 +941,68 @@ fn window_name_runtime_setter<'s>( _rv: v8::ReturnValue<'s, v8::Value>, ) { let receiver = callback_this_object(scope, &args); - if !super::window_receiver::is_window_receiver(scope, receiver) { + let Some(owner) = window_name_owner(scope, receiver) else { throw_type_error(scope, "Illegal invocation"); return; - } - let child_handle = child_context_handle_from_owner(scope, receiver); + }; let Some(next) = args.get(0).to_string(scope) else { return; }; let next = next.to_rust_string_lossy(scope); - let host_ptr = context_host_ptr_from_window_name_receiver(scope, receiver); - if let Some(handle) = child_handle - && let Some(host_ptr) = host_ptr - { - unsafe { &mut *host_ptr }.set_child_browsing_context_name(handle, next.clone()); - } else if child_handle.is_none() - && let Some(host_ptr) = host_ptr - { - unsafe { &*host_ptr }.set_top_level_window_name(next.clone()); + match owner { + WindowNameOwner::TopLevel(host_ptr) => { + unsafe { &*host_ptr }.set_top_level_window_name(next.clone()); + } + WindowNameOwner::Child { host_ptr, handle } => { + unsafe { &mut *host_ptr }.set_child_browsing_context_name(handle, next.clone()); + } + WindowNameOwner::LightweightPopup { host_ptr, popup_id } => { + unsafe { &mut *host_ptr }.set_lightweight_popup_window_name(popup_id, &next); + } } define_non_enumerable_string_property(scope, receiver, WINDOW_NAME_SLOT, &next); } -fn context_host_ptr_from_window_name_receiver<'s>( +enum WindowNameOwner { + TopLevel(*mut JsContextHost), + Child { + host_ptr: *mut JsContextHost, + handle: DomHandle, + }, + LightweightPopup { + host_ptr: *mut JsContextHost, + popup_id: u64, + }, +} + +fn window_name_owner<'s>( scope: &mut v8::PinScope<'s, '_>, receiver: v8::Local<'s, v8::Object>, -) -> Option<*mut JsContextHost> { - context_host_ptr_from_window_object(scope, receiver).or_else(|| { +) -> Option { + if let Some(popup_id) = crate::native_bridge::lightweight_popup_id_from_window(scope, receiver) + { + return context_host_ptr_from_global_bridge(scope) + .map(|host_ptr| WindowNameOwner::LightweightPopup { host_ptr, popup_id }); + } + if !super::window_receiver::is_window_receiver(scope, receiver) { + return None; + } + let host_ptr = context_host_ptr_from_window_object(scope, receiver).or_else(|| { receiver - .strict_equals(scope.get_current_context().global(scope).into()) - .then(|| context_host_ptr_from_global_bridge(scope)) - .flatten() - }) + .get_creation_context(scope) + .and_then(context_host_ptr_from_context_slot) + .or_else(|| { + receiver + .strict_equals(scope.get_current_context().global(scope).into()) + .then(|| context_host_ptr_from_global_bridge(scope)) + .flatten() + }) + })?; + if let Some(handle) = super::window_accessors::window_child_context_handle(scope, receiver) { + Some(WindowNameOwner::Child { host_ptr, handle }) + } else { + Some(WindowNameOwner::TopLevel(host_ptr)) + } } fn install_public_window_surface_accessors<'s>( diff --git a/moli-renderer-v8/src/context_bootstrap/window_receiver.rs b/moli-renderer-v8/src/context_bootstrap/window_receiver.rs index b619477b1e..7f5a46839b 100644 --- a/moli-renderer-v8/src/context_bootstrap/window_receiver.rs +++ b/moli-renderer-v8/src/context_bootstrap/window_receiver.rs @@ -1,7 +1,7 @@ -use super::shared::WINDOW_NAVIGATOR_SLOT; +use super::shared::{CHILD_BROWSING_CONTEXT_HANDLE_SLOT, WINDOW_NAVIGATOR_SLOT}; use crate::util::{ - callback_data_index_value, context_host_ptr_from_context_slot, - context_host_ptr_from_window_object, get_private_value, throw_type_error, + callback_data_index_value, context_host_ptr_from_context_slot, get_private_value, + throw_type_error, }; /// Recognizes a native Window receiver for WebIDL brand checks. @@ -34,7 +34,10 @@ fn is_live_window_receiver<'s>( scope: &mut v8::PinScope<'s, '_>, receiver: v8::Local<'s, v8::Object>, ) -> bool { - if context_host_ptr_from_window_object(scope, receiver).is_some() { + if crate::native_bridge::lightweight_popup_id_from_window(scope, receiver).is_some() + || crate::native_bridge::cross_origin_lightweight_popup_id(scope, receiver).is_some() + || get_private_value(scope, receiver, CHILD_BROWSING_CONTEXT_HANDLE_SLOT).is_some() + { return true; } // The shared helper maps an `undefined` rollback marker to `None`. diff --git a/moli-renderer-v8/src/native_bridge/context_host/popups.rs b/moli-renderer-v8/src/native_bridge/context_host/popups.rs index 5204a4d921..2f92a63136 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/popups.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/popups.rs @@ -631,7 +631,7 @@ impl JsContextHost { true } - fn set_lightweight_popup_window_name(&mut self, popup_id: u64, next: &str) { + pub(crate) fn set_lightweight_popup_window_name(&mut self, popup_id: u64, next: &str) { self.lightweight_popup_window_names .retain(|_, candidate| *candidate != popup_id); if !self.lightweight_popup_is_open(popup_id) { diff --git a/moli-renderer-v8/src/runtime/page_commands.rs b/moli-renderer-v8/src/runtime/page_commands.rs index f36443ef58..79a35ccb93 100644 --- a/moli-renderer-v8/src/runtime/page_commands.rs +++ b/moli-renderer-v8/src/runtime/page_commands.rs @@ -856,9 +856,6 @@ impl PageVm { RendererPageCommand::LayoutMetrics => { Ok(RendererPageReply::LayoutMetrics(self.layout_metrics()?)) } - RendererPageCommand::PublishLayoutMetrics => self - .publish_layout_metrics() - .map(RendererPageReply::LayoutMetrics), RendererPageCommand::PublishLayout => { self.vm_mut().publish_layout()?; Ok(RendererPageReply::Unit) diff --git a/moli-renderer-v8/src/runtime/page_geometry.rs b/moli-renderer-v8/src/runtime/page_geometry.rs index a0ea77b1e1..847da643f8 100644 --- a/moli-renderer-v8/src/runtime/page_geometry.rs +++ b/moli-renderer-v8/src/runtime/page_geometry.rs @@ -13,15 +13,4 @@ impl PageVm { device_pixel_ratio: f64::from(metrics.viewport.device_pixel_ratio), }) } - - pub(crate) fn publish_layout_metrics(&mut self) -> anyhow::Result { - if !self.layout_policy.uses_real_layout() { - return Err(anyhow::anyhow!( - "layout publication is disabled by the current layout policy" - )); - } - self.flush_page_action_window(moli_action_window::ActionBarrier::Explicit)?; - self.vm_mut().publish_layout()?; - self.layout_metrics() - } } diff --git a/moli-renderer-v8/src/runtime/page_surface.rs b/moli-renderer-v8/src/runtime/page_surface.rs index 57d23ce4e2..532f72931d 100644 --- a/moli-renderer-v8/src/runtime/page_surface.rs +++ b/moli-renderer-v8/src/runtime/page_surface.rs @@ -5195,7 +5195,6 @@ pub enum RendererPageCommand { }, SerializeHtml, LayoutMetrics, - PublishLayoutMetrics, PublishLayout, CaptureScreenshot(RendererCaptureScreenshotRequest), CaptureScreencastFrame(RendererCaptureScreencastFrameRequest), @@ -5800,7 +5799,6 @@ impl RendererPageCommand { Self::RenderPageDump { .. } => Some("RenderPageDump"), Self::SerializeHtml => Some("SerializeHtml"), Self::LayoutMetrics => Some("LayoutMetrics"), - Self::PublishLayoutMetrics => Some("PublishLayoutMetrics"), Self::PublishLayout => Some("PublishLayout"), Self::CaptureScreenshot(_) => Some("CaptureScreenshot"), Self::CaptureScreencastFrame(_) => Some("CaptureScreencastFrame"), diff --git a/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update/layout_geometry.rs b/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update/layout_geometry.rs index f314dcc1f4..c3806fa271 100644 --- a/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update/layout_geometry.rs +++ b/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update/layout_geometry.rs @@ -1133,204 +1133,3 @@ document.body.innerHTML = ` .await .expect("intrinsic width fixture should run"); } -#[tokio::test(flavor = "current_thread")] -async fn explicit_layout_metrics_publication_rejects_mock_policy_without_a_layout_pass() { - run_page_vm_async_test(async move { - let loader = - crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); - let mut page_vm = test_page_vm_with_loader_and_document_url( - &loader, - Vec::new(), - Url::parse("https://example.com/layout-metrics-policy.html")?, - ); - page_vm.layout_policy = moli_page_types::LayoutPolicy::Mock; - page_vm - .vm_mut() - .set_layout_policy(moli_page_types::LayoutPolicy::Mock); - page_vm.vm_mut().eval( - r#" -document.body.innerHTML = '
'; -'installed' -"#, - )?; - let before = page_vm.vm().layout_pass_observability_for_test(); - - let error = page_vm - .publish_layout_metrics() - .expect_err("Mock policy must reject explicit real-layout publication"); - assert!( - error.to_string().contains("layout publication is disabled"), - "unexpected publication error: {error:#}" - ); - assert_eq!( - page_vm.vm().layout_pass_observability_for_test().1, - before.1, - "rejecting publication must not build a hidden real-layout pass" - ); - Ok::<(), anyhow::Error>(()) - }) - .await - .expect("layout policy publication boundary should run"); -} - -#[tokio::test(flavor = "current_thread")] -async fn explicit_layout_metrics_publication_skips_paint_and_reuses_the_published_snapshot() { - run_page_vm_async_test(async move { - let loader = - crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); - let mut page_vm = test_page_vm_with_loader_and_document_url( - &loader, - Vec::new(), - Url::parse("https://example.com/layout-metrics-publication.html")?, - ); - page_vm.vm_mut().eval( - r#" -document.head.innerHTML = ''; -document.body.innerHTML = '
'; -'installed' -"#, - )?; - page_vm.vm_mut().sync_live_document_style_sources(); - let before = page_vm.vm().layout_pass_observability_for_test(); - - let metrics = page_vm.publish_layout_metrics()?; - assert_eq!(metrics.content_width, 1920.0); - assert_eq!(metrics.content_height, 1080.0); - let after = page_vm.vm().layout_pass_observability_for_test(); - assert_eq!(after.1, before.1 + 1); - let pass = after.3.expect("publication records one layout pass"); - assert_eq!(pass.reason, moli_layout::LayoutFlushReason::Explicit); - assert_eq!(pass.paint_operation_count, 0); - - let cached = page_vm.layout_metrics()?; - assert_eq!(cached, metrics); - assert_eq!(page_vm.vm().layout_pass_observability_for_test().1, after.1); - Ok::<(), anyhow::Error>(()) - }) - .await - .expect("explicit layout metrics publication should run"); -} - -#[tokio::test(flavor = "current_thread")] -async fn explicit_layout_metrics_publication_freezes_nested_frame_geometry_without_paint() { - run_page_vm_async_test(async move { - let loader = - crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader"); - let mut page_vm = test_page_vm_with_loader_and_document_url( - &loader, - Vec::new(), - Url::parse("https://example.com/nested-frame-layout-publication.html")?, - ); - page_vm.vm_mut().eval( - r#" -document.head.innerHTML = ''; -document.body.innerHTML = ''; -const outer = document.getElementById('outer'); -outer.contentDocument.head.innerHTML = ''; -outer.contentDocument.body.innerHTML = '
'; -const inner = outer.contentDocument.getElementById('inner'); -inner.contentDocument.head.innerHTML = ''; -inner.contentDocument.body.innerHTML = '
'; -'installed' -"#, - )?; - page_vm.vm_mut().sync_live_document_style_sources(); - - let outer = page_vm - .vm() - .element_handle_by_id_for_test("outer") - .expect("outer iframe handle"); - let root_document = page_vm.vm().document_handle_for_test(); - let before = page_vm.vm().layout_pass_observability_for_test(); - - page_vm.publish_layout_metrics()?; - - let after = page_vm.vm().layout_pass_observability_for_test(); - assert_eq!(after.1, before.1 + 1); - let pass = after.3.expect("publication records one layout pass"); - assert_eq!(pass.paint_operation_count, 0); - - let host = page_vm - .vm() - .context_host_weak_for_test() - .upgrade() - .expect("page context host"); - host.borrow() - .with_latest_layout_tree_for_document(root_document, |root_tree| { - let child_tree = root_tree - .embedded_frame_tree(outer) - .expect("the publication must freeze the child frame tree"); - assert_eq!( - child_tree.viewport, - moli_layout::LayoutViewport::new(240, 140, 1.0), - "the child tree must retain the iframe's embedded viewport" - ); - let mut nested_frames = child_tree.embedded_frames(); - let grandchild_tree = &nested_frames - .next() - .expect("the publication must recursively freeze the nested frame tree") - .tree; - assert_eq!(nested_frames.len(), 0, "the fixture has one nested frame"); - assert_eq!( - grandchild_tree.viewport, - moli_layout::LayoutViewport::new(100, 60, 1.0), - "the grandchild tree must retain the nested iframe's embedded viewport" - ); - }) - .expect("the publication must retain the root layout tree"); - - drop(host); - page_vm.vm_mut().eval( - r#" -document.getElementById('outer').style.width = '300px'; -document.getElementById('outer').contentDocument.getElementById('inner').style.height = '80px'; -'mutated' -"#, - )?; - page_vm.vm_mut().sync_live_document_style_sources(); - page_vm.publish_layout_metrics()?; - let republished = page_vm.vm().layout_pass_observability_for_test(); - assert_eq!(republished.1, after.1 + 1); - assert_eq!( - republished - .3 - .expect("republication records the rebuilt layout pass") - .paint_operation_count, - 0 - ); - let host = page_vm - .vm() - .context_host_weak_for_test() - .upgrade() - .expect("page context host"); - host.borrow() - .with_latest_layout_tree_for_document(root_document, |root_tree| { - let child_tree = root_tree - .embedded_frame_tree(outer) - .expect("republication must retain the rebuilt child frame tree"); - assert_eq!( - child_tree.viewport, - moli_layout::LayoutViewport::new(300, 140, 1.0) - ); - let grandchild_tree = &child_tree - .embedded_frames() - .next() - .expect("republication must retain the rebuilt nested frame tree") - .tree; - assert_eq!( - grandchild_tree.viewport, - moli_layout::LayoutViewport::new(100, 80, 1.0) - ); - }) - .expect("republication must replace the retained root layout tree"); - - assert_eq!( - page_vm.vm().layout_pass_observability_for_test().1, - republished.1, - "inspecting nested geometry must not hide a missing frame projection with another pass" - ); - Ok::<(), anyhow::Error>(()) - }) - .await - .expect("nested frame layout publication should run"); -} diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/window_surface.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/window_surface.rs index be4348deb9..10931ee775 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/window_surface.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/window_surface.rs @@ -883,6 +883,32 @@ fn window_name_accessors_follow_the_receiver_realm_and_preserve_state_on_convers const borrowedParent = childDescriptor.get.call(window); const borrowedChild = parentDescriptor.get.call(child); + const popup = window.open("about:blank", "popup-owner"); + const borrowedPopupBefore = parentDescriptor.get.call(popup); + parentDescriptor.set.call(popup, "renamed-popup-owner"); + const borrowedPopupAfter = parentDescriptor.get.call(popup); + const openerAfterPopupRename = window.name; + + const iterator = document.createNodeIterator(document); + let invalidReceiverGetter; + let invalidReceiverSetter; + let invalidReceiverConverted = false; + try { + parentDescriptor.get.call(iterator); + } catch (error) { + invalidReceiverGetter = error.name; + } + try { + parentDescriptor.set.call(iterator, { + toString() { + invalidReceiverConverted = true; + return "forged"; + } + }); + } catch (error) { + invalidReceiverSetter = error.name; + } + let conversionError; try { childDescriptor.set.call(window, { @@ -895,6 +921,12 @@ fn window_name_accessors_follow_the_receiver_realm_and_preserve_state_on_convers return JSON.stringify({ borrowedParent, borrowedChild, + borrowedPopupBefore, + borrowedPopupAfter, + openerAfterPopupRename, + invalidReceiverGetter, + invalidReceiverSetter, + invalidReceiverConverted, parentAfterError: window.name, childAfterError: child.name, conversionError @@ -906,6 +938,6 @@ fn window_name_accessors_follow_the_receiver_realm_and_preserve_state_on_convers assert_eq!( result, - r#"{"borrowedParent":"parent-from-child-realm","borrowedChild":"child-from-parent-realm","parentAfterError":"parent-from-child-realm","childAfterError":"child-from-parent-realm","conversionError":"RangeError:window-name-conversion"}"# + r#"{"borrowedParent":"parent-from-child-realm","borrowedChild":"child-from-parent-realm","borrowedPopupBefore":"popup-owner","borrowedPopupAfter":"renamed-popup-owner","openerAfterPopupRename":"parent-from-child-realm","invalidReceiverGetter":"TypeError","invalidReceiverSetter":"TypeError","invalidReceiverConverted":false,"parentAfterError":"parent-from-child-realm","childAfterError":"child-from-parent-realm","conversionError":"RangeError:window-name-conversion"}"# ); } From c8c819b75dd5ec60c72ae8ae7a2d6bb9383b745a Mon Sep 17 00:00:00 2001 From: lanyue-llk <270302213+lanyue-llk@users.noreply.github.com> Date: Tue, 29 Sep 2026 18:50:02 +0800 Subject: [PATCH 5/8] Share popup identity across renderer hosts --- moli-core/src/lib.rs | 1 + moli-core/src/runtime/navigation_engine.rs | 15 +++- .../src/conn/page_state/page_targets.rs | 48 ++++++++++++ .../src/conn/state/browser_context.rs | 17 +++- moli-protocol/src/domains/page/popup.rs | 2 + moli-protocol/src/domains/target/popup.rs | 23 +++++- .../target/tests/tests_target_creation.rs | 67 +++++++++++++++- .../src/browsing_context_state.rs | 50 +++++++++++- .../src/context_bootstrap/runtime_state.rs | 8 +- .../window_runtime/dialogs.rs | 8 +- .../src/native_bridge/context_host/core.rs | 2 - .../src/native_bridge/context_host/mod.rs | 2 - .../src/native_bridge/context_host/popups.rs | 77 +++++++++++-------- .../element/activation/targets.rs | 5 +- .../src/runtime/browser_context_runtime.rs | 27 +++++++ .../runtime/page_surface/popup_activation.rs | 21 +++++ ...ice_worker_internal_client_request_body.rs | 12 ++- 17 files changed, 321 insertions(+), 64 deletions(-) diff --git a/moli-core/src/lib.rs b/moli-core/src/lib.rs index 675a2518c1..d770be1c76 100644 --- a/moli-core/src/lib.rs +++ b/moli-core/src/lib.rs @@ -33,4 +33,5 @@ pub use moli_renderer_v8::{ RendererOwnerRuntimeActivitySource, RendererProtocolObservation, RendererRuntimeCommandCausalIdentity, RendererRuntimeInspectorAsyncCompletion, RendererRuntimeInspectorResponseChannel, RendererRuntimeInspectorResponseSender, + RendererTopLevelBrowsingContextState, }; diff --git a/moli-core/src/runtime/navigation_engine.rs b/moli-core/src/runtime/navigation_engine.rs index f3f0cc6840..bbd26794f7 100644 --- a/moli-core/src/runtime/navigation_engine.rs +++ b/moli-core/src/runtime/navigation_engine.rs @@ -1,4 +1,5 @@ use crate::{ + RendererTopLevelBrowsingContextState, network::{ResourceRequestClient, SharedWebStorageStore}, page::{ CompletedPageCommand, DocumentStartScript, EmulatedMediaOverrides, NavigationResponse, @@ -24,8 +25,7 @@ use moli_renderer_v8::{ RendererBrowserContextRuntime, RendererBrowserContextRuntimeOwner, RendererBrowserContextRuntimeOwnerAccess, RendererDocumentReplacement, RendererReservedServiceWorkerClient, RendererServiceWorkerMainResourceFetch, - RendererTopLevelBrowsingContextState, RendererWebStorageHandles, SharedStorageBucketStore, - WeakIndexedDbManager, + RendererWebStorageHandles, SharedStorageBucketStore, WeakIndexedDbManager, network::{ BrowserResourceRuntime, BrowserResourceRuntimeOwner, PageNetworkPolicy, navigation::{DocumentFetchContextSeed, NavigationResourceLoader}, @@ -459,11 +459,22 @@ impl NavigationEngine { self.top_level_browsing_context.window_name() } + pub fn top_level_browsing_context_state(&self) -> RendererTopLevelBrowsingContextState { + self.top_level_browsing_context.clone() + } + pub fn set_top_level_window_name(&self, value: impl Into) { self.top_level_browsing_context .set_window_name(value.into()); } + pub fn set_top_level_browsing_context_state( + &mut self, + state: RendererTopLevelBrowsingContextState, + ) { + self.top_level_browsing_context = state; + } + /// Reserves a renderer Page identity before the corresponding creation /// command is enqueued. /// diff --git a/moli-protocol/src/conn/page_state/page_targets.rs b/moli-protocol/src/conn/page_state/page_targets.rs index eecb72c48e..a4c0d62f71 100644 --- a/moli-protocol/src/conn/page_state/page_targets.rs +++ b/moli-protocol/src/conn/page_state/page_targets.rs @@ -19,6 +19,8 @@ impl BrowserContext { self.forget_target_opener_references_for_target(target_id); self.target_browsing_context_group_ids.remove(target_id); self.forget_target_window_names_for_target(target_id); + self.target_top_level_browsing_context_states + .remove(target_id); self.forget_target_popup_id_for_target(target_id); Some(target) } @@ -174,6 +176,26 @@ impl BrowserContext { .is_some_and(|group_id| group_id == source_group_id) }) }; + if self + .target_top_level_browsing_context_states + .get(source_target_id) + .is_some_and(|state| staged_target_matches(source_target_id, &state.window_name())) + { + return self + .page_target(source_target_id) + .map(PageTargetHost::target_id); + } + if let Some(target_id) = self + .target_top_level_browsing_context_states + .iter() + .find_map(|(target_id, state)| { + (target_id != source_target_id + && staged_target_matches(target_id, &state.window_name())) + .then_some(target_id) + }) + { + return Some(target_id); + } if self .target_window_names .get(source_target_id) @@ -210,6 +232,32 @@ impl BrowserContext { } } + pub(crate) fn remember_target_top_level_browsing_context_state( + &mut self, + target_id: &str, + state: moli_core::RendererTopLevelBrowsingContextState, + ) { + if let Some(engine) = self.page_navigation_engine_mut(target_id) { + engine.set_top_level_browsing_context_state(state); + return; + } + self.target_top_level_browsing_context_states + .insert(target_id.to_owned(), state); + } + + pub(crate) fn target_top_level_browsing_context_state( + &self, + target_id: &str, + ) -> Option { + self.page_navigation_engine(target_id) + .map(moli_core::runtime::NavigationEngine::top_level_browsing_context_state) + .or_else(|| { + self.target_top_level_browsing_context_states + .get(target_id) + .cloned() + }) + } + pub(crate) fn remember_target_popup_id(&mut self, popup_id: Option, target_id: &str) { if let Some(popup_id) = popup_id && let Some(replaced_popup_id) = diff --git a/moli-protocol/src/conn/state/browser_context.rs b/moli-protocol/src/conn/state/browser_context.rs index 441f79353a..4519a7950c 100644 --- a/moli-protocol/src/conn/state/browser_context.rs +++ b/moli-protocol/src/conn/state/browser_context.rs @@ -61,6 +61,8 @@ pub struct BrowserContext { /// a matching `window.name` cannot route navigation across that boundary. pub(crate) target_browsing_context_group_ids: HashMap, pub target_window_names: HashMap, + pub(crate) target_top_level_browsing_context_states: + HashMap, pub target_popup_ids: HashMap, pending_popup_javascript_dialogs: HashMap>, pub(crate) shared_worker_targets: BTreeMap, @@ -513,6 +515,7 @@ impl BrowserContext { target_can_access_opener: HashSet::new(), target_browsing_context_group_ids: HashMap::new(), target_window_names: HashMap::new(), + target_top_level_browsing_context_states: HashMap::new(), target_popup_ids: HashMap::new(), pending_popup_javascript_dialogs: HashMap::new(), shared_worker_targets: BTreeMap::new(), @@ -571,11 +574,13 @@ impl BrowserContext { let renderer_runtime = self.renderer_runtime_owner_access(); let sender = self.renderer_output_transport_sender.clone(); let target_window_names = &self.target_window_names; + let target_top_level_browsing_context_states = + &self.target_top_level_browsing_context_states; for host in self.page_targets.iter_mut() { if host.navigation_engine().is_some() { continue; } - let engine = NavigationEngine::new_with_runtime_config_and_browser_context_access( + let mut engine = NavigationEngine::new_with_runtime_config_and_browser_context_access( config.clone(), renderer_runtime.clone(), ) @@ -583,7 +588,9 @@ impl BrowserContext { if let Some(sender) = sender.clone() { engine.set_renderer_output_transport_sender(sender); } - if let Some(window_name) = target_window_names.get(host.target_id()) { + if let Some(state) = target_top_level_browsing_context_states.get(host.target_id()) { + engine.set_top_level_browsing_context_state(state.clone()); + } else if let Some(window_name) = target_window_names.get(host.target_id()) { engine.set_top_level_window_name(window_name.clone()); } host.install_navigation_engine(engine); @@ -594,6 +601,12 @@ impl BrowserContext { .get(target_id) .is_none_or(|target| target.navigation_engine().is_none()) }); + self.target_top_level_browsing_context_states + .retain(|target_id, _| { + page_targets + .get(target_id) + .is_none_or(|target| target.navigation_engine().is_none()) + }); } pub(crate) fn set_renderer_output_transport_sender( diff --git a/moli-protocol/src/domains/page/popup.rs b/moli-protocol/src/domains/page/popup.rs index 8d123dc07a..4566292661 100644 --- a/moli-protocol/src/domains/page/popup.rs +++ b/moli-protocol/src/domains/page/popup.rs @@ -95,6 +95,7 @@ pub(super) async fn emit_prepared( target_name, session_storage_store, initial_empty_document_storage_key, + top_level_browsing_context, ) = activation.into_parts(); let can_access_opener = matches!( &source, @@ -114,6 +115,7 @@ pub(super) async fn emit_prepared( disposition, session_storage_store, initial_empty_document_storage_key, + top_level_browsing_context, ); let browser_context_id = page_owner.browser_context_id().to_owned(); let target_id = diff --git a/moli-protocol/src/domains/target/popup.rs b/moli-protocol/src/domains/target/popup.rs index 17ea83d6e3..7b200733a0 100644 --- a/moli-protocol/src/domains/target/popup.rs +++ b/moli-protocol/src/domains/target/popup.rs @@ -41,6 +41,7 @@ pub(crate) struct PopupTargetCreation { disposition: moli_core::page::RendererPopupDisposition, session_storage_store: Option, initial_empty_document_storage_key: Option, + top_level_browsing_context: Option, } impl PopupTargetCreation { @@ -54,6 +55,7 @@ impl PopupTargetCreation { disposition: moli_core::page::RendererPopupDisposition, session_storage_store: Option, initial_empty_document_storage_key: Option, + top_level_browsing_context: Option, ) -> Self { Self { browser_context_id, @@ -65,6 +67,7 @@ impl PopupTargetCreation { disposition, session_storage_store, initial_empty_document_storage_key, + top_level_browsing_context, } } } @@ -84,6 +87,7 @@ pub(crate) async fn create_popup_target_from_renderer_output_background_events_a disposition, session_storage_store, initial_empty_document_storage_key, + top_level_browsing_context, } = creation; let Some(browser_context) = conn.browser_context_by_id(&browser_context_id) else { tracing::debug!( @@ -104,6 +108,16 @@ pub(crate) async fn create_popup_target_from_renderer_output_background_events_a }) .map(str::to_owned) { + if let Some(incoming_state) = top_level_browsing_context.as_ref() + && let Some(owner_state) = + browser_context.target_top_level_browsing_context_state(&existing_target_id) + { + let observed_name = incoming_state.window_name(); + incoming_state.bind_to(&owner_state); + if observed_name != target_name { + owner_state.set_window_name(observed_name); + } + } let navigation = popup_target_has_loaded_page(conn, &browser_context_id, &existing_target_id) .then(|| { @@ -128,9 +142,6 @@ pub(crate) async fn create_popup_target_from_renderer_output_background_events_a browser_context.update_target_url(&existing_target_id, url.clone()) }); if target_url_updated { - if let Some(browser_context) = conn.browser_context_by_id_mut(&browser_context_id) { - browser_context.remember_target_window_name(&target_name, &existing_target_id); - } emit_target_info_changed_for_target_background_event( conn, out, @@ -208,7 +219,11 @@ pub(crate) async fn create_popup_target_from_renderer_output_background_events_a can_access_opener, ); } - browser_context.remember_target_window_name(&target_name, &target_id); + if let Some(state) = top_level_browsing_context { + browser_context.remember_target_top_level_browsing_context_state(&target_id, state); + } else { + browser_context.remember_target_window_name(&target_name, &target_id); + } browser_context.remember_target_popup_id(popup_id, &target_id); } diff --git a/moli-protocol/src/domains/target/tests/tests_target_creation.rs b/moli-protocol/src/domains/target/tests/tests_target_creation.rs index 17aa814671..c31525c567 100644 --- a/moli-protocol/src/domains/target/tests/tests_target_creation.rs +++ b/moli-protocol/src/domains/target/tests/tests_target_creation.rs @@ -1814,7 +1814,6 @@ async fn window_open_named_target_reuses_existing_popup_target() { .any(|message| message["method"] == json!("Page.windowOpen")), "first named window.open should emit Page.windowOpen: {first_sent:?}" ); - ctx.process_async(json!({ "id": 15, "method": "Runtime.evaluate", @@ -1879,13 +1878,49 @@ async fn window_open_named_target_reuses_existing_popup_target() { browser_context.target_url(), "data:text/html,second-popup" ); + ctx.process_async(json!({ + "id": 141, + "method": "Target.attachToTarget", + "params": { "targetId": target_id } + })) + .await; + let popup_session_id = take_response_by_id(&mut ctx, 141)["result"]["sessionId"] + .as_str() + .expect("popup session id") + .to_owned(); + ctx.sent.clear(); ctx.process_async(json!({ "id": 16, "method": "Runtime.evaluate", "sessionId": opener_session_id, "params": { - "expression": "window.__namedPopup.name = 'renamedWindow'; window.open('data:text/html,renamed-popup', 'renamedWindow') !== null" + "expression": "window.__namedPopup.name = 'renamedWindow'; window.__namedPopup.name" + } + })) + .await; + assert_eq!( + take_response_by_id(&mut ctx, 16)["result"]["result"]["value"], + json!("renamedWindow") + ); + ctx.process_async(json!({ + "id": 161, + "method": "Runtime.evaluate", + "sessionId": popup_session_id, + "params": { "expression": "window.name" } + })) + .await; + assert_eq!( + take_response_by_id(&mut ctx, 161)["result"]["result"]["value"], + json!("renamedWindow"), + "a proxy rename must be immediately visible in the target" + ); + ctx.process_async(json!({ + "id": 162, + "method": "Runtime.evaluate", + "sessionId": opener_session_id, + "params": { + "expression": "window.open('data:text/html,renamed-popup', 'renamedWindow') !== null" } })) .await; @@ -1914,12 +1949,36 @@ async fn window_open_named_target_reuses_existing_popup_target() { "renaming a live popup must invalidate its previous target name" ); + ctx.process_async(json!({ + "id": 163, + "method": "Runtime.evaluate", + "sessionId": popup_session_id, + "params": { "expression": "window.name = 'targetRenamed'; window.name" } + })) + .await; + assert_eq!( + take_response_by_id(&mut ctx, 163)["result"]["result"]["value"], + json!("targetRenamed") + ); + ctx.process_async(json!({ + "id": 164, + "method": "Runtime.evaluate", + "sessionId": opener_session_id, + "params": { "expression": "window.__namedPopup.name" } + })) + .await; + assert_eq!( + take_response_by_id(&mut ctx, 164)["result"]["result"]["value"], + json!("targetRenamed"), + "a target rename must be immediately visible through its retained proxy" + ); + ctx.process_async(json!({ "id": 17, "method": "Runtime.evaluate", "sessionId": opener_session_id, "params": { - "expression": "window.open('about:blank', 'reportWindow') !== null" + "expression": "window.open('about:blank', 'renamedWindow') !== null" } })) .await; @@ -1928,7 +1987,7 @@ async fn window_open_named_target_reuses_existing_popup_target() { old_name_sent .iter() .any(|message| message["method"] == json!("Target.targetCreated")), - "the popup's previous name must no longer resolve to its target: {old_name_sent:?}" + "the target's previous name must no longer resolve to it: {old_name_sent:?}" ); }) .await; diff --git a/moli-renderer-v8/src/browsing_context_state.rs b/moli-renderer-v8/src/browsing_context_state.rs index 76469ed2be..ffa8d3b49d 100644 --- a/moli-renderer-v8/src/browsing_context_state.rs +++ b/moli-renderer-v8/src/browsing_context_state.rs @@ -6,16 +6,33 @@ use parking_lot::Mutex; /// Document committed into that context. #[derive(Clone, Default)] pub struct RendererTopLevelBrowsingContextState { - window_name: Arc>, + window_name_owner: Arc>>>, } impl RendererTopLevelBrowsingContextState { pub fn window_name(&self) -> String { - self.window_name.lock().clone() + self.window_name_owner.lock().clone().lock().clone() } pub fn set_window_name(&self, value: String) { - *self.window_name.lock() = value; + *self.window_name_owner.lock().clone().lock() = value; + } + + pub fn shares_identity_with(&self, other: &Self) -> bool { + if Arc::ptr_eq(&self.window_name_owner, &other.window_name_owner) { + return true; + } + let left = self.window_name_owner.lock().clone(); + let right = other.window_name_owner.lock().clone(); + Arc::ptr_eq(&left, &right) + } + + pub fn bind_to(&self, owner: &Self) { + if Arc::ptr_eq(&self.window_name_owner, &owner.window_name_owner) { + return; + } + let owner = owner.window_name_owner.lock().clone(); + *self.window_name_owner.lock() = owner; } } @@ -23,7 +40,32 @@ impl fmt::Debug for RendererTopLevelBrowsingContextState { fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { formatter .debug_struct("RendererTopLevelBrowsingContextState") - .field("strong_count", &Arc::strong_count(&self.window_name)) + .field( + "owner_strong_count", + &Arc::strong_count(&self.window_name_owner), + ) .finish_non_exhaustive() } } + +#[cfg(test)] +mod tests { + use super::RendererTopLevelBrowsingContextState; + + #[test] + fn binding_redirects_both_handles_to_one_window_name_owner() { + let owner = RendererTopLevelBrowsingContextState::default(); + owner.set_window_name("target".to_owned()); + let proxy = RendererTopLevelBrowsingContextState::default(); + proxy.set_window_name("proxy-before-bind".to_owned()); + + proxy.bind_to(&owner); + assert!(proxy.shares_identity_with(&owner)); + assert_eq!(proxy.window_name(), "target"); + + proxy.set_window_name("from-proxy".to_owned()); + assert_eq!(owner.window_name(), "from-proxy"); + owner.set_window_name("from-target".to_owned()); + assert_eq!(proxy.window_name(), "from-target"); + } +} diff --git a/moli-renderer-v8/src/context_bootstrap/runtime_state.rs b/moli-renderer-v8/src/context_bootstrap/runtime_state.rs index f2cb18f009..3e7506be89 100644 --- a/moli-renderer-v8/src/context_bootstrap/runtime_state.rs +++ b/moli-renderer-v8/src/context_bootstrap/runtime_state.rs @@ -927,9 +927,11 @@ fn window_name_runtime_getter<'s>( v8_string(scope, &unsafe { &*host_ptr }.top_level_window_name()) .map(v8::Local::::from) } - WindowNameOwner::Child { .. } | WindowNameOwner::LightweightPopup { .. } => { - object_hidden_value(scope, receiver, WINDOW_NAME_SLOT) - } + WindowNameOwner::Child { .. } => object_hidden_value(scope, receiver, WINDOW_NAME_SLOT), + WindowNameOwner::LightweightPopup { host_ptr, popup_id } => unsafe { &*host_ptr } + .lightweight_popup_top_level_browsing_context_state(popup_id) + .and_then(|state| v8_string(scope, &state.window_name())) + .map(v8::Local::::from), } .unwrap_or_else(|| v8::String::empty(scope).into()); rv.set(value); diff --git a/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs b/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs index 28193f87f7..4fa0704056 100644 --- a/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs +++ b/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs @@ -255,6 +255,8 @@ pub(crate) fn window_open_callback<'s>( let session_storage_store = host.lightweight_popup_session_storage_store(popup_id); let initial_empty_document_storage_key = host.lightweight_popup_initial_empty_document_storage_key(popup_id); + let top_level_browsing_context = + host.lightweight_popup_top_level_browsing_context_state(popup_id); let window_open_event = opened_popup .created_new_browsing_context .then_some(window_open_event); @@ -268,10 +270,8 @@ pub(crate) fn window_open_callback<'s>( parsed.target_name, popup_disposition, ) - .with_initial_auxiliary_state( - session_storage_store, - initial_empty_document_storage_key, - ), + .with_initial_auxiliary_state(session_storage_store, initial_empty_document_storage_key) + .with_top_level_browsing_context_state(top_level_browsing_context), window_open_event, ); if suppress_opener { 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 3662bf6c05..e44597dc9d 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/core.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/core.rs @@ -409,13 +409,11 @@ impl JsContextHost { pending_download_activations: Vec::new(), #[cfg(test)] pending_popup_activations: Vec::new(), - next_lightweight_popup_id: 1, next_lightweight_popup_local_window_id: 1, next_lightweight_popup_document_id: 1, next_lightweight_popup_document_load_id: 0, next_lightweight_popup_classic_script_load_id: 0, lightweight_popup_browsing_contexts: HashMap::new(), - lightweight_popup_window_names: HashMap::new(), lightweight_popup_document_handles: HashMap::new(), pending_lightweight_popup_document_loads: HashMap::new(), pending_lightweight_popup_classic_script_loads: HashMap::new(), 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 caf14d9ef8..0e03b351cc 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/mod.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/mod.rs @@ -1024,14 +1024,12 @@ pub(crate) struct JsContextHost { pending_download_activations: Vec, #[cfg(test)] pending_popup_activations: Vec, - next_lightweight_popup_id: u64, next_lightweight_popup_local_window_id: u64, next_lightweight_popup_document_id: u64, next_lightweight_popup_document_load_id: u64, next_lightweight_popup_classic_script_load_id: u64, lightweight_popup_browsing_contexts: HashMap, - lightweight_popup_window_names: HashMap, lightweight_popup_document_handles: HashMap, pending_lightweight_popup_document_loads: HashMap, diff --git a/moli-renderer-v8/src/native_bridge/context_host/popups.rs b/moli-renderer-v8/src/native_bridge/context_host/popups.rs index 2f92a63136..e17e2061ac 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/popups.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/popups.rs @@ -5,7 +5,7 @@ use crate::web_api_interfaces; use crate::{ content_security_policy::content_security_policy_forces_opaque_origin, context_bootstrap::{ - SharedWebStorageStore, WINDOW_NAME_SLOT, apply_local_window_location_navigation, + SharedWebStorageStore, apply_local_window_location_navigation, deep_clone_shared_web_storage_store, dispatch_simple_event_target_event, install_navigation_bootstrap_entry_for_holder, install_simple_event_target_methods, install_storage_aliases_for_window, @@ -430,6 +430,7 @@ pub(super) struct LightweightPopupBrowsingContextRecord { opener_sandbox_policy: Option, lifecycle: LightweightPopupLifecycle, navigation_id: LightweightPopupNavigationId, + top_level_browsing_context: crate::RendererTopLevelBrowsingContextState, } impl LightweightPopupBrowsingContextRecord { @@ -632,16 +633,25 @@ impl JsContextHost { } pub(crate) fn set_lightweight_popup_window_name(&mut self, popup_id: u64, next: &str) { - self.lightweight_popup_window_names - .retain(|_, candidate| *candidate != popup_id); - if !self.lightweight_popup_is_open(popup_id) { - return; - } - if let Some(name) = trackable_lightweight_popup_window_name(next) { - self.lightweight_popup_window_names.insert(name, popup_id); + if let Some(record) = self.lightweight_popup_record(popup_id) + && record.is_open() + { + record + .top_level_browsing_context + .set_window_name(next.to_owned()); } } + pub(crate) fn lightweight_popup_top_level_browsing_context_state( + &self, + popup_id: u64, + ) -> Option { + let record = self.lightweight_popup_record(popup_id)?; + record + .is_open() + .then(|| record.top_level_browsing_context.clone()) + } + pub(crate) fn open_lightweight_popup_window<'s>( &mut self, scope: &mut v8::PinScope<'s, '_>, @@ -655,7 +665,14 @@ impl JsContextHost { ) -> Option> { if opener.is_some() && let Some(name) = trackable_lightweight_popup_window_name(target_name) - && let Some(popup_id) = self.lightweight_popup_window_names.get(&name).copied() + && let Some(popup_id) = + self.lightweight_popup_browsing_contexts + .iter() + .find_map(|(popup_id, record)| { + (record.is_open() + && record.top_level_browsing_context.window_name() == name) + .then_some(*popup_id) + }) && self.lightweight_popup_is_open(popup_id) && let Some(window) = self.reopen_lightweight_popup_window( scope, @@ -725,15 +742,7 @@ impl JsContextHost { &initial_url, opener_sandbox_policy.is_some_and(|policy| policy.forces_opaque_origin), ); - let tracked_name = opener - .is_some() - .then(|| trackable_lightweight_popup_window_name(target_name)) - .flatten(); - let popup_id = self.next_lightweight_popup_id; - self.next_lightweight_popup_id = self - .next_lightweight_popup_id - .checked_add(1) - .expect("lightweight popup id space exhausted"); + let popup_id = self.browser_context_runtime.next_lightweight_popup_id(); let window = self .bridge .bindings @@ -756,10 +765,10 @@ impl JsContextHost { LIGHTWEIGHT_POPUP_ID_SLOT, popup_id_private_value.into(), ); - let initial_window_name = - trackable_lightweight_popup_window_name(target_name).unwrap_or_default(); - let initial_window_name = v8_string(scope, &initial_window_name)?; - set_object_slot(scope, window, WINDOW_NAME_SLOT, initial_window_name.into()); + let top_level_browsing_context = crate::RendererTopLevelBrowsingContextState::default(); + top_level_browsing_context.set_window_name( + trackable_lightweight_popup_window_name(target_name).unwrap_or_default(), + ); LightweightPopupWindowNameDeclaration::default() .initialize(scope, window) .ok()?; @@ -884,6 +893,7 @@ impl JsContextHost { session_storage_store, })), navigation_id: LightweightPopupNavigationId::new(1), + top_level_browsing_context, }, ); self.register_committed_document_resource_loader( @@ -904,9 +914,6 @@ impl JsContextHost { initial_url.clone(), ); } - if let Some(name) = tracked_name { - self.lightweight_popup_window_names.insert(name, popup_id); - } if moli_url::is_about_blank(&initial_url) && let Some(document) = crate::dom_parser::parse_browsing_context_document_projection_from_source( @@ -4296,15 +4303,20 @@ fn lightweight_popup_window_name_getter<'s>( mut rv: v8::ReturnValue<'_, v8::Value>, ) { let window = args.this(); - if lightweight_popup_id_from_window(scope, window).is_none() { + let Some(popup_id) = lightweight_popup_id_from_window(scope, window) else { throw_type_error(scope, "Window.name getter called on incompatible receiver."); return; + }; + let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) else { + return; + }; + let value = unsafe { &*host_ptr } + .lightweight_popup_top_level_browsing_context_state(popup_id) + .map(|state| state.window_name()) + .unwrap_or_default(); + if let Some(value) = v8_string(scope, &value) { + rv.set(value.into()); } - let value = window - .get(scope, v8str(scope, WINDOW_NAME_SLOT).into()) - .filter(|value| value.is_string()) - .unwrap_or_else(|| v8::String::empty(scope).into()); - rv.set(value); } fn lightweight_popup_window_name_setter<'s>( @@ -4321,7 +4333,6 @@ fn lightweight_popup_window_name_setter<'s>( return; }; let next_string = next.to_rust_string_lossy(scope); - set_object_slot(scope, window, WINDOW_NAME_SLOT, next.into()); if let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) { unsafe { &mut *host_ptr }.set_lightweight_popup_window_name(popup_id, &next_string); } @@ -4359,8 +4370,6 @@ fn lightweight_popup_close_callback<'s>( } host.retire_lightweight_popup_document_owner(transition.retired_owner); host.retire_lightweight_popup_local_window(popup_id, transition.retired_local_window_id); - host.lightweight_popup_window_names - .retain(|_, named_popup_id| *named_popup_id != popup_id); } fn lightweight_popup_initiator_endpoint<'s>( diff --git a/moli-renderer-v8/src/native_bridge/element/activation/targets.rs b/moli-renderer-v8/src/native_bridge/element/activation/targets.rs index f9af6e32d5..04656b750d 100644 --- a/moli-renderer-v8/src/native_bridge/element/activation/targets.rs +++ b/moli-renderer-v8/src/native_bridge/element/activation/targets.rs @@ -253,6 +253,8 @@ fn navigate_hyperlink_popup_target( let session_storage_store = runtime.lightweight_popup_session_storage_store(popup_id); let initial_empty_document_storage_key = runtime.lightweight_popup_initial_empty_document_storage_key(popup_id); + let top_level_browsing_context = + runtime.lightweight_popup_top_level_browsing_context_state(popup_id); let user_gesture = runtime.protocol_user_gesture_activation(); let window_open_event = opened_popup.created_new_browsing_context.then(|| { RendererPendingWindowOpenEvent::browser_window(resolved_url, target_name, user_gesture) @@ -267,7 +269,8 @@ fn navigate_hyperlink_popup_target( target_name.to_owned(), disposition, ) - .with_initial_auxiliary_state(session_storage_store, initial_empty_document_storage_key), + .with_initial_auxiliary_state(session_storage_store, initial_empty_document_storage_key) + .with_top_level_browsing_context_state(top_level_browsing_context), window_open_event, ); true diff --git a/moli-renderer-v8/src/runtime/browser_context_runtime.rs b/moli-renderer-v8/src/runtime/browser_context_runtime.rs index 343ff229b4..5936044794 100644 --- a/moli-renderer-v8/src/runtime/browser_context_runtime.rs +++ b/moli-renderer-v8/src/runtime/browser_context_runtime.rs @@ -206,6 +206,7 @@ struct RendererBrowserContextRuntimeInner { service_worker_runtime: service_worker_runtime::LazyServiceWorkerRuntime, storage_partition_identity: RendererStoragePartitionIdentity, next_child_document_loader_id: AtomicU64, + next_lightweight_popup_id: AtomicU64, next_detached_parser_script_fetch_id: AtomicU64, next_dedicated_worker_instance_id: AtomicU64, dedicated_worker_devtools_targets: Mutex>, @@ -368,6 +369,15 @@ impl Default for RendererBrowserContextRuntimeOwner { } impl RendererBrowserContextRuntime { + pub(crate) fn next_lightweight_popup_id(&self) -> u64 { + let id = self + .inner + .next_lightweight_popup_id + .fetch_add(1, Ordering::Relaxed); + assert_ne!(id, u64::MAX, "lightweight popup id space exhausted"); + id + } + pub(crate) fn clipboard_snapshot(&self) -> ClipboardSnapshot { self.inner.clipboard_snapshot.lock().clone() } @@ -582,6 +592,7 @@ impl RendererBrowserContextRuntime { service_worker_runtime, storage_partition_identity, next_child_document_loader_id: AtomicU64::default(), + next_lightweight_popup_id: AtomicU64::new(1), next_detached_parser_script_fetch_id: AtomicU64::default(), next_dedicated_worker_instance_id: AtomicU64::default(), dedicated_worker_devtools_targets: Mutex::new(HashMap::new()), @@ -1011,6 +1022,22 @@ mod tests { }, }; + #[test] + fn lightweight_popup_ids_are_unique_across_hosts_in_one_browser_context() { + let browser_context = RendererBrowserContextRuntime::new(); + let first_host = browser_context.handle(); + let second_host = browser_context.handle(); + + assert_eq!(first_host.next_lightweight_popup_id(), 1); + assert_eq!(second_host.next_lightweight_popup_id(), 2); + + let other_browser_context = RendererBrowserContextRuntime::new(); + assert_eq!( + other_browser_context.handle().next_lightweight_popup_id(), + 1 + ); + } + async fn assert_single_page_reservation_release( output_rx: &mut crate::runtime::RendererOutputTransportReceiver, token: RendererPageReservationToken, diff --git a/moli-renderer-v8/src/runtime/page_surface/popup_activation.rs b/moli-renderer-v8/src/runtime/page_surface/popup_activation.rs index c0babd6c61..d139ae2147 100644 --- a/moli-renderer-v8/src/runtime/page_surface/popup_activation.rs +++ b/moli-renderer-v8/src/runtime/page_surface/popup_activation.rs @@ -51,6 +51,7 @@ pub struct RendererPendingPopupActivation { target_name: String, session_storage_store: Option, initial_empty_document_storage_key: Option, + top_level_browsing_context: Option, } impl RendererPendingPopupActivation { @@ -79,6 +80,7 @@ impl RendererPendingPopupActivation { target_name, session_storage_store: None, initial_empty_document_storage_key: None, + top_level_browsing_context: None, } } @@ -100,6 +102,7 @@ impl RendererPendingPopupActivation { target_name, session_storage_store: None, initial_empty_document_storage_key: None, + top_level_browsing_context: None, } } @@ -122,6 +125,14 @@ impl RendererPendingPopupActivation { self } + pub fn with_top_level_browsing_context_state( + mut self, + state: Option, + ) -> Self { + self.top_level_browsing_context = state; + self + } + pub fn source(&self) -> &RendererPopupActivationSource { &self.source } @@ -153,6 +164,7 @@ impl RendererPendingPopupActivation { String, Option, Option, + Option, ) { ( self.source, @@ -162,6 +174,7 @@ impl RendererPendingPopupActivation { self.target_name, self.session_storage_store, self.initial_empty_document_storage_key, + self.top_level_browsing_context, ) } } @@ -179,6 +192,14 @@ impl PartialEq for RendererPendingPopupActivation { _ => false, } && self.initial_empty_document_storage_key == other.initial_empty_document_storage_key + && match ( + &self.top_level_browsing_context, + &other.top_level_browsing_context, + ) { + (None, None) => true, + (Some(left), Some(right)) => left.shares_identity_with(right), + _ => false, + } } } diff --git a/moli-renderer-v8/src/script_vm/service_worker_internal_client_request_body.rs b/moli-renderer-v8/src/script_vm/service_worker_internal_client_request_body.rs index d261a40ddc..298f7851c9 100644 --- a/moli-renderer-v8/src/script_vm/service_worker_internal_client_request_body.rs +++ b/moli-renderer-v8/src/script_vm/service_worker_internal_client_request_body.rs @@ -262,6 +262,9 @@ impl ScriptVm { let initial_empty_document_storage_key = popup_id.and_then(|popup_id| { host.lightweight_popup_initial_empty_document_storage_key(popup_id) }); + let top_level_browsing_context = popup_id.and_then(|popup_id| { + host.lightweight_popup_top_level_browsing_context_state(popup_id) + }); host.record_pending_popup_activation( crate::RendererPendingPopupActivation::browser_context( popup_id, @@ -272,7 +275,8 @@ impl ScriptVm { .with_initial_auxiliary_state( session_storage_store, initial_empty_document_storage_key, - ), + ) + .with_top_level_browsing_context_state(top_level_browsing_context), None, ); if let Some(popup_id) = popup_id { @@ -344,6 +348,9 @@ impl ScriptVm { let initial_empty_document_storage_key = popup_id.and_then(|popup_id| { host.lightweight_popup_initial_empty_document_storage_key(popup_id) }); + let top_level_browsing_context = popup_id.and_then(|popup_id| { + host.lightweight_popup_top_level_browsing_context_state(popup_id) + }); host.record_pending_popup_activation( crate::RendererPendingPopupActivation::browser_context( popup_id, @@ -354,7 +361,8 @@ impl ScriptVm { .with_initial_auxiliary_state( session_storage_store, initial_empty_document_storage_key, - ), + ) + .with_top_level_browsing_context_state(top_level_browsing_context), None, ); host_scope.restore(scope, previous_owner_context); From 51316b77be62b8732b9b3b0613417f19155eb87f Mon Sep 17 00:00:00 2001 From: lanyue-llk <270302213+lanyue-llk@users.noreply.github.com> Date: Tue, 29 Sep 2026 20:27:41 +0800 Subject: [PATCH 6/8] Honor named browsing context selection --- .../src/conn/page_state/page_targets.rs | 68 ++++++++--- .../src/conn/state/browser_context.rs | 2 + moli-protocol/src/domains/page/popup.rs | 110 +++++++++++++++++- .../src/domains/page/producer_tests.rs | 2 + moli-protocol/src/domains/page/tests/mod.rs | 1 + moli-protocol/src/domains/target/popup.rs | 28 +++-- .../window_runtime/dialogs.rs | 12 +- .../context_host/child_frames.rs | 6 + .../context_host/child_frames/lookup.rs | 18 +++ .../src/native_bridge/context_host/core.rs | 15 +++ .../src/native_bridge/context_host/popups.rs | 48 +++++++- .../context_host/window_document_tasks.rs | 1 + .../element/activation/targets.rs | 35 ++++++ .../page_surface/window_document_source.rs | 1 + .../misc/extracted/popup_window.rs | 98 ++++++++++++++++ .../navigation/element_navigation.rs | 37 ++++++ 16 files changed, 457 insertions(+), 25 deletions(-) diff --git a/moli-protocol/src/conn/page_state/page_targets.rs b/moli-protocol/src/conn/page_state/page_targets.rs index a4c0d62f71..0507e12549 100644 --- a/moli-protocol/src/conn/page_state/page_targets.rs +++ b/moli-protocol/src/conn/page_state/page_targets.rs @@ -259,12 +259,9 @@ impl BrowserContext { } pub(crate) fn remember_target_popup_id(&mut self, popup_id: Option, target_id: &str) { - if let Some(popup_id) = popup_id - && let Some(replaced_popup_id) = - self.target_popup_ids.insert(target_id.to_owned(), popup_id) - && replaced_popup_id != popup_id - { - self.dismiss_pending_popup_javascript_dialogs(replaced_popup_id); + if let Some(popup_id) = popup_id { + self.target_popup_ids.insert(target_id.to_owned(), popup_id); + self.popup_target_ids.insert(popup_id, target_id.to_owned()); } } @@ -273,7 +270,16 @@ impl BrowserContext { } pub(crate) fn forget_target_popup_id_for_target(&mut self, target_id: &str) { - if let Some(popup_id) = self.target_popup_ids.remove(target_id) { + self.target_popup_ids.remove(target_id); + let popup_ids = self + .popup_target_ids + .iter() + .filter_map(|(popup_id, candidate_target_id)| { + (candidate_target_id == target_id).then_some(*popup_id) + }) + .collect::>(); + for popup_id in popup_ids { + self.popup_target_ids.remove(&popup_id); self.dismiss_pending_popup_javascript_dialogs(popup_id); } } @@ -283,12 +289,22 @@ impl BrowserContext { } pub(crate) fn target_id_for_popup_id(&self, popup_id: u64) -> Option<&str> { - self.target_popup_ids - .iter() - .find_map(|(target_id, candidate)| { - (*candidate == popup_id && self.devtools_target_info(target_id).is_some()) - .then_some(target_id.as_str()) - }) + self.popup_target_ids.get(&popup_id).and_then(|target_id| { + self.devtools_target_info(target_id) + .is_some() + .then_some(target_id.as_str()) + }) + } + + pub(crate) fn reusable_target_id_for_popup_id( + &self, + source_target_id: &str, + popup_id: u64, + target_name: &str, + ) -> Option<&str> { + let candidate_target_id = self.target_id_for_popup_id(popup_id)?; + (self.target_id_for_window_name(source_target_id, target_name) == Some(candidate_target_id)) + .then_some(candidate_target_id) } pub(crate) fn remember_target_opener( @@ -1592,11 +1608,37 @@ mod tests { context.remember_target_window_name("current", "TID-popup"); context.remember_target_window_name("current", "TID-b"); + context.remember_target_popup_id(Some(41), "TID-popup"); assert_eq!( context.target_id_for_window_name("TID-b", "current"), Some("TID-b"), "the source navigable takes priority over another related target" ); + assert_eq!( + context.reusable_target_id_for_popup_id("TID-b", 41, "current"), + None, + "a popup identity hint must not bypass source-first name selection" + ); + + context.remember_target_window_name("source", "TID-b"); + assert_eq!( + context.reusable_target_id_for_popup_id("TID-b", 41, "current"), + Some("TID-popup"), + "the popup identity is accepted when the canonical name resolver selects it" + ); + assert_eq!( + context.reusable_target_id_for_popup_id("TID-a", 41, "current"), + None, + "a popup identity hint must not cross browsing-context groups" + ); + + context.remember_target_popup_id(Some(42), "TID-popup"); + assert_eq!(context.target_id_for_popup_id(41), Some("TID-popup")); + assert_eq!(context.target_id_for_popup_id(42), Some("TID-popup")); + assert_eq!(context.target_popup_id("TID-popup"), Some(42)); + context.forget_target_popup_id_for_target("TID-popup"); + assert_eq!(context.target_id_for_popup_id(41), None); + assert_eq!(context.target_id_for_popup_id(42), None); } #[test] diff --git a/moli-protocol/src/conn/state/browser_context.rs b/moli-protocol/src/conn/state/browser_context.rs index 4519a7950c..6b494da510 100644 --- a/moli-protocol/src/conn/state/browser_context.rs +++ b/moli-protocol/src/conn/state/browser_context.rs @@ -64,6 +64,7 @@ pub struct BrowserContext { pub(crate) target_top_level_browsing_context_states: HashMap, pub target_popup_ids: HashMap, + pub(crate) popup_target_ids: HashMap, pending_popup_javascript_dialogs: HashMap>, pub(crate) shared_worker_targets: BTreeMap, pub(crate) dedicated_worker_targets: BTreeMap, @@ -517,6 +518,7 @@ impl BrowserContext { target_window_names: HashMap::new(), target_top_level_browsing_context_states: HashMap::new(), target_popup_ids: HashMap::new(), + popup_target_ids: HashMap::new(), pending_popup_javascript_dialogs: HashMap::new(), shared_worker_targets: BTreeMap::new(), dedicated_worker_targets: BTreeMap::new(), diff --git a/moli-protocol/src/domains/page/popup.rs b/moli-protocol/src/domains/page/popup.rs index 4566292661..06bda70b2d 100644 --- a/moli-protocol/src/domains/page/popup.rs +++ b/moli-protocol/src/domains/page/popup.rs @@ -161,8 +161,15 @@ fn resolve_devtools_opener( .is_some() .then(|| PopupTargetOpenerIdentity::new(target_id, target_id)) } - RendererWindowDocumentSource::ChildFrame { frame_id, .. } => { - let target_id = page_owner.target_id()?; + RendererWindowDocumentSource::ChildFrame { + frame_id, + top_level_popup_id, + .. + } => { + let target_id = match top_level_popup_id { + Some(popup_id) => browser_context.target_id_for_popup_id(*popup_id)?, + None => page_owner.target_id()?, + }; browser_context .devtools_target_info(target_id) .is_some() @@ -251,6 +258,23 @@ mod tests { ) } + fn named_window_activation( + exposes_opener: bool, + popup_id: u64, + url: &str, + target_name: &str, + ) -> RendererPendingPopupActivation { + RendererPendingPopupActivation::window( + source_document(), + RendererWindowDocumentSource::RootFrame, + exposes_opener, + Some(popup_id), + url.to_owned(), + target_name.to_owned(), + RendererPopupDisposition::Background, + ) + } + async fn emit( conn: &mut CdpConnection, owner: TargetPageResidenceIdentity, @@ -345,6 +369,42 @@ mod tests { assert!(!info.can_access_opener); } + #[tokio::test(flavor = "multi_thread")] + async fn noopener_named_popup_does_not_reuse_a_related_target() { + let mut conn = CdpConnection::default(); + conn.browser_context = Some(context("BID-1", "TID-opener", "SID-1")); + let owner = page_owner("BID-1", "TID-opener"); + + emit( + &mut conn, + owner.clone(), + vec![named_window_activation( + true, + 80, + "about:blank#related", + "report", + )], + ) + .await; + emit( + &mut conn, + owner, + vec![named_window_activation( + false, + 81, + "about:blank#isolated", + "report", + )], + ) + .await; + + let context = conn.browser_context_by_id("BID-1").unwrap(); + assert_ne!( + context.target_id_for_popup_id(80), + context.target_id_for_popup_id(81) + ); + } + #[tokio::test(flavor = "multi_thread")] async fn removed_opener_downgrades_access_without_rebinding_to_current_target() { let mut conn = CdpConnection::default(); @@ -384,6 +444,7 @@ mod tests { frame_id: "FRAME-child".to_owned(), local_window_id: 9, document_id: 11, + top_level_popup_id: None, }, true, Some(43), @@ -406,6 +467,51 @@ mod tests { assert!(info.can_access_opener); } + #[tokio::test(flavor = "multi_thread")] + async fn child_inside_lightweight_popup_uses_popup_as_its_top_level_opener() { + let mut conn = CdpConnection::default(); + conn.browser_context = Some(context("BID-1", "TID-root", "SID-1")); + let owner = page_owner("BID-1", "TID-root"); + + emit( + &mut conn, + owner, + vec![ + window_activation( + RendererWindowDocumentSource::RootFrame, + true, + Some(70), + "about:blank#parent-popup", + ), + window_activation( + RendererWindowDocumentSource::ChildFrame { + frame_id: "FRAME-popup-child".to_owned(), + local_window_id: 9, + document_id: 11, + top_level_popup_id: Some(70), + }, + true, + Some(71), + "about:blank#child-popup", + ), + ], + ) + .await; + + let context = conn.browser_context_by_id("BID-1").unwrap(); + let parent_target_id = context.target_id_for_popup_id(70).unwrap(); + let child_target_id = context.target_id_for_popup_id(71).unwrap(); + let child = context.devtools_target_info(child_target_id).unwrap(); + assert_eq!( + child.opener_id.as_ref().map(|id| id.as_str()), + Some(parent_target_id) + ); + assert_eq!( + child.opener_frame_id.as_ref().map(|id| id.as_str()), + Some("FRAME-popup-child") + ); + } + #[tokio::test(flavor = "multi_thread")] async fn fifo_popup_batch_resolves_a_lightweight_popup_as_the_next_opener() { let mut conn = CdpConnection::default(); diff --git a/moli-protocol/src/domains/page/producer_tests.rs b/moli-protocol/src/domains/page/producer_tests.rs index f8a08231b8..99c6d7afd7 100644 --- a/moli-protocol/src/domains/page/producer_tests.rs +++ b/moli-protocol/src/domains/page/producer_tests.rs @@ -174,6 +174,7 @@ fn renderer_javascript_dialog_for_test( frame_id: frame_id.to_owned(), local_window_id: 1, document_id: 1, + top_level_popup_id: None, }, "https://example.test/dialog-source".to_owned(), "alert".to_owned(), @@ -926,6 +927,7 @@ async fn javascript_dialog_projection_uses_captured_url_and_frame() { frame_id: "FRAME-source".to_owned(), local_window_id: 4, document_id: 5, + top_level_popup_id: None, }, "https://source.example/dialog".to_owned(), "alert".to_owned(), diff --git a/moli-protocol/src/domains/page/tests/mod.rs b/moli-protocol/src/domains/page/tests/mod.rs index 79f7fe6c11..5f48e6de73 100644 --- a/moli-protocol/src/domains/page/tests/mod.rs +++ b/moli-protocol/src/domains/page/tests/mod.rs @@ -54,6 +54,7 @@ fn renderer_dialog_for_test( frame_id: frame_id.to_owned(), local_window_id: 1, document_id: 1, + top_level_popup_id: None, } }); RendererPendingJavaScriptDialog::new( diff --git a/moli-protocol/src/domains/target/popup.rs b/moli-protocol/src/domains/target/popup.rs index 7b200733a0..493b576720 100644 --- a/moli-protocol/src/domains/target/popup.rs +++ b/moli-protocol/src/domains/target/popup.rs @@ -99,15 +99,27 @@ pub(crate) async fn create_popup_target_from_renderer_output_background_events_a return None; }; - if let Some(existing_target_id) = popup_id - .and_then(|popup_id| browser_context.target_id_for_popup_id(popup_id)) - .or_else(|| { - opener.as_ref().and_then(|opener| { - browser_context.target_id_for_window_name(&opener.target_id, &target_name) + let existing_target_id = if can_access_opener { + opener + .as_ref() + .and_then(|opener| { + popup_id.and_then(|popup_id| { + browser_context.reusable_target_id_for_popup_id( + &opener.target_id, + popup_id, + &target_name, + ) + }) }) - }) - .map(str::to_owned) - { + .or_else(|| { + opener.as_ref().and_then(|opener| { + browser_context.target_id_for_window_name(&opener.target_id, &target_name) + }) + }) + } else { + None + }; + if let Some(existing_target_id) = existing_target_id.map(str::to_owned) { if let Some(incoming_state) = top_level_browsing_context.as_ref() && let Some(owner_state) = browser_context.target_top_level_browsing_context_state(&existing_target_id) diff --git a/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs b/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs index 4fa0704056..106a866673 100644 --- a/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs +++ b/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs @@ -195,6 +195,17 @@ pub(crate) fn window_open_callback<'s>( } return; } + let source_scope = unsafe { &*host_ptr }.entered_owner_dispatch_scope(scope); + if trackable_named_popup_target_name(&parsed.target_name).is_some() + && !suppress_opener + && unsafe { &*host_ptr } + .window_name_for_dispatch_scope(source_scope) + .as_deref() + == Some(parsed.target_name.as_str()) + { + navigate_window_open_self(scope, entered_window, &url, &mut rv); + return; + } if let Some(target_window) = existing_named_child_window_for_window_open(scope, host_ptr, &parsed.target_name) && !suppress_opener @@ -214,7 +225,6 @@ pub(crate) fn window_open_callback<'s>( window_features: parsed_features.enabled_feature_strings(), user_gesture: host.protocol_user_gesture_activation(), }; - let source_scope = host.entered_owner_dispatch_scope(scope); let Some((_, root_document, source)) = host.renderer_window_document_source_for_dispatch_scope(source_scope) else { diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_frames.rs b/moli-renderer-v8/src/native_bridge/context_host/child_frames.rs index 4555cdb0e3..d4173323a8 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_frames.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_frames.rs @@ -1270,6 +1270,12 @@ impl JsContextHost { }; entry.set_window_name(name); } + + pub(crate) fn child_browsing_context_window_name(&self, handle: DomHandle) -> Option<&str> { + self.child_browsing_contexts + .get(&handle) + .map(ChildBrowsingContextEntry::window_name) + } } fn child_navigation_current_url(seed: &NavigationHistoryEntrySeed) -> Option<&str> { 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 11f24ae3a9..91838668fe 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,6 +54,24 @@ impl JsContextHost { self.lightweight_popup_id_for_document_handle(owner_document) } + pub(crate) fn child_browsing_context_top_level_popup_id( + &self, + handle: DomHandle, + ) -> Option { + let mut current = Some(handle); + let mut visited = HashSet::new(); + while let Some(handle) = current { + if !visited.insert(handle) { + return None; + } + if let Some(popup_id) = self.child_browsing_context_popup_owner_id(handle) { + return Some(popup_id); + } + current = self.child_browsing_context_parent_handle(handle); + } + None + } + fn collect_child_browsing_context_handles_in_document_order_from_document( &self, document: DomHandle, 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 e44597dc9d..a3194c0f62 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/core.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/core.rs @@ -491,6 +491,21 @@ impl JsContextHost { self.top_level_browsing_context.set_window_name(value); } + pub(crate) fn window_name_for_dispatch_scope( + &self, + scope: super::OwnerDispatchScope, + ) -> Option { + match scope { + super::OwnerDispatchScope::Top => Some(self.top_level_window_name()), + super::OwnerDispatchScope::Child(handle) => self + .child_browsing_context_window_name(handle) + .map(str::to_owned), + super::OwnerDispatchScope::LightweightPopup(popup_id) => self + .lightweight_popup_top_level_browsing_context_state(popup_id) + .map(|state| state.window_name()), + } + } + pub(crate) fn set_root_document_lifecycle( &mut self, lifecycle: RendererDocumentLifecycleJournalHandle, diff --git a/moli-renderer-v8/src/native_bridge/context_host/popups.rs b/moli-renderer-v8/src/native_bridge/context_host/popups.rs index e17e2061ac..55b701de4b 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/popups.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/popups.rs @@ -426,6 +426,7 @@ enum LightweightPopupLifecycle { pub(super) struct LightweightPopupBrowsingContextRecord { window_proxy: v8::Global, opener: Option, + group: LightweightPopupBrowsingContextGroup, location_url: Url, opener_sandbox_policy: Option, lifecycle: LightweightPopupLifecycle, @@ -433,6 +434,12 @@ pub(super) struct LightweightPopupBrowsingContextRecord { top_level_browsing_context: crate::RendererTopLevelBrowsingContextState, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +enum LightweightPopupBrowsingContextGroup { + Top, + Isolated(u64), +} + impl LightweightPopupBrowsingContextRecord { fn is_open(&self) -> bool { matches!(self.lifecycle, LightweightPopupLifecycle::Open(_)) @@ -652,6 +659,39 @@ impl JsContextHost { .then(|| record.top_level_browsing_context.clone()) } + fn lightweight_popup_group( + &self, + endpoint: super::PendingWindowMessageEndpoint, + ) -> LightweightPopupBrowsingContextGroup { + match endpoint { + super::PendingWindowMessageEndpoint::TopWindow => { + LightweightPopupBrowsingContextGroup::Top + } + super::PendingWindowMessageEndpoint::ChildWindow(handle) => { + let Some(popup_id) = self.child_browsing_context_top_level_popup_id(handle) else { + return LightweightPopupBrowsingContextGroup::Top; + }; + self.lightweight_popup_record(popup_id) + .map(|record| record.group) + .unwrap_or(LightweightPopupBrowsingContextGroup::Isolated(popup_id)) + } + super::PendingWindowMessageEndpoint::LightweightPopup(popup_id) => self + .lightweight_popup_record(popup_id) + .map(|record| record.group) + .unwrap_or(LightweightPopupBrowsingContextGroup::Isolated(popup_id)), + } + } + + fn lightweight_popup_is_related_to_initiator( + &self, + popup_id: u64, + initiator: super::PendingWindowMessageEndpoint, + ) -> bool { + self.lightweight_popup_group(super::PendingWindowMessageEndpoint::LightweightPopup( + popup_id, + )) == self.lightweight_popup_group(initiator) + } + pub(crate) fn open_lightweight_popup_window<'s>( &mut self, scope: &mut v8::PinScope<'s, '_>, @@ -663,13 +703,15 @@ impl JsContextHost { creator_base_url: Url, creator_policy_container: DocumentPolicyContainer, ) -> Option> { - if opener.is_some() + let initiator = lightweight_popup_initiator_endpoint(scope, opener, opener_child_handle); + if let Some(initiator) = initiator && let Some(name) = trackable_lightweight_popup_window_name(target_name) && let Some(popup_id) = self.lightweight_popup_browsing_contexts .iter() .find_map(|(popup_id, record)| { (record.is_open() + && self.lightweight_popup_is_related_to_initiator(*popup_id, initiator) && record.top_level_browsing_context.window_name() == name) .then_some(*popup_id) }) @@ -743,6 +785,9 @@ impl JsContextHost { opener_sandbox_policy.is_some_and(|policy| policy.forces_opaque_origin), ); let popup_id = self.browser_context_runtime.next_lightweight_popup_id(); + let group = opener_endpoint + .map(|endpoint| self.lightweight_popup_group(endpoint)) + .unwrap_or(LightweightPopupBrowsingContextGroup::Isolated(popup_id)); let window = self .bridge .bindings @@ -872,6 +917,7 @@ impl JsContextHost { LightweightPopupBrowsingContextRecord { window_proxy: v8::Global::new(scope, window), opener: opener_endpoint, + group, location_url: initial_url.clone(), opener_sandbox_policy, lifecycle: LightweightPopupLifecycle::Open(Box::new(LightweightPopupOpenState { diff --git a/moli-renderer-v8/src/native_bridge/context_host/window_document_tasks.rs b/moli-renderer-v8/src/native_bridge/context_host/window_document_tasks.rs index 61e49362f0..d44c6bb5c5 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/window_document_tasks.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/window_document_tasks.rs @@ -188,6 +188,7 @@ impl JsContextHost { frame_id, local_window_id: owner.local_window_id.0, document_id: owner.document_id.0, + top_level_popup_id: self.child_browsing_context_top_level_popup_id(handle), } } ( diff --git a/moli-renderer-v8/src/native_bridge/element/activation/targets.rs b/moli-renderer-v8/src/native_bridge/element/activation/targets.rs index 04656b750d..67f5d8dc98 100644 --- a/moli-renderer-v8/src/native_bridge/element/activation/targets.rs +++ b/moli-renderer-v8/src/native_bridge/element/activation/targets.rs @@ -512,6 +512,41 @@ pub(in crate::native_bridge) fn navigate_hyperlink_target_browsing_context<'s>( if let Some(target_name) = target_name && special_target.is_none() { + let relations = + hyperlink_popup_relations(unsafe { &*runtime_ptr }, source_handle, target_name); + if relations.suppress_opener { + return navigate_hyperlink_popup_target( + scope, + runtime_ptr, + source_handle, + target_name, + resolved_url, + popup_disposition, + ); + } + if !target_name.is_empty() + && let Some(dispatch_scope) = + browsing_context_dispatch_scope_for_node(scope, runtime_ptr, source_handle) + && unsafe { &*runtime_ptr } + .window_name_for_dispatch_scope(dispatch_scope) + .as_deref() + == Some(target_name) + { + return match dispatch_scope { + crate::native_bridge::OwnerDispatchScope::Top => { + queue_top_level_location_navigation(scope, runtime_ptr, resolved_url) + } + crate::native_bridge::OwnerDispatchScope::Child(_) + | crate::native_bridge::OwnerDispatchScope::LightweightPopup(_) => { + navigate_hyperlink_source_browsing_context( + scope, + runtime_ptr, + source_handle, + resolved_url, + ) + } + }; + } let source_document = unsafe { &*runtime_ptr } .dom_host() .node(source_handle) diff --git a/moli-renderer-v8/src/runtime/page_surface/window_document_source.rs b/moli-renderer-v8/src/runtime/page_surface/window_document_source.rs index 89ceffb876..9291453f0e 100644 --- a/moli-renderer-v8/src/runtime/page_surface/window_document_source.rs +++ b/moli-renderer-v8/src/runtime/page_surface/window_document_source.rs @@ -12,6 +12,7 @@ pub enum RendererWindowDocumentSource { frame_id: String, local_window_id: u64, document_id: u64, + top_level_popup_id: Option, }, LightweightPopup { popup_id: u64, diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/popup_window.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/popup_window.rs index a28b2e9060..abbfe0f617 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/popup_window.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/popup_window.rs @@ -589,6 +589,104 @@ fn window_open_named_lightweight_popup_reuses_without_recloning_session_storage( r#"{"sameWindow":true,"popupName":"sessionStorageTestWindow","initial":null,"openerFoo":"BAR","popupFoo":null}"# ); } + +#[test] +fn window_open_does_not_reuse_named_noopener_popup_from_an_isolated_group() { + let mut vm = new_storage_test_vm("https://example.com/"); + + assert_eq!( + vm.eval("open('about:blank#isolated', 'report', 'noopener') === null") + .expect("isolated named popup should evaluate"), + "true" + ); + let first = vm.take_pending_popup_activations(); + assert_eq!(first.len(), 1); + let first_popup_id = first[0].popup_id().expect("lightweight popup id"); + + assert_eq!( + vm.eval( + "(() => { const popup = open('about:blank#related', 'report'); return [popup.name, popup.location.href].join('|'); })()" + ) + .expect("related named popup should evaluate"), + "report|about:blank#related" + ); + let second = vm.take_pending_popup_activations(); + assert_eq!(second.len(), 1); + assert_ne!( + second[0].popup_id(), + Some(first_popup_id), + "an isolated noopener popup is not a reusable named target for its creator" + ); +} + +#[test] +fn window_open_noopener_does_not_reuse_a_related_named_popup() { + let mut vm = new_storage_test_vm("https://example.com/"); + + let first = vm + .eval("open('about:blank#related', 'report').location.href") + .expect("related named popup should evaluate"); + assert_eq!(first, "about:blank#related"); + let first_activation = vm.take_pending_popup_activations(); + let first_popup_id = first_activation[0].popup_id().expect("first popup id"); + + assert_eq!( + vm.eval("open('about:blank#isolated', 'report', 'noopener') === null") + .expect("isolated named popup should evaluate"), + "true" + ); + let second_activation = vm.take_pending_popup_activations(); + assert_eq!(second_activation.len(), 1); + assert_ne!(second_activation[0].popup_id(), Some(first_popup_id)); +} + +#[test] +fn named_popup_group_survives_intermediate_opener_close() { + let mut vm = new_storage_test_vm("https://example.com/"); + + assert_eq!( + vm.eval( + "(() => { const parent = open('', 'parent'); const child = parent.open('', 'child'); parent.close(); return open('', 'child') === child; })()" + ) + .expect("popup group should survive opener close"), + "true" + ); +} + +#[test] +fn window_open_prefers_the_same_named_source_over_a_related_popup() { + let mut vm = new_storage_test_vm("https://example.com/source"); + + assert_eq!( + vm.eval( + r#" +(() => { + const popup = open("about:blank#popup", "shared"); + window.name = "shared"; + const selected = open("about:blank#self", "shared"); + return JSON.stringify({ + selectedSelf: selected === window, + popupHref: popup.location.href, + popupName: popup.name + }); +})() +"#, + ) + .expect("same-named source selection should evaluate"), + r#"{"selectedSelf":true,"popupHref":"about:blank#popup","popupName":"shared"}"# + ); + let activations = vm.take_pending_popup_activations(); + assert_eq!( + activations.len(), + 1, + "the second open must navigate the source instead of reopening the popup" + ); + let navigation = vm + .take_pending_location_navigation_with_seed() + .expect("same-named source navigation"); + assert_eq!(navigation.url.as_str(), "about:blank#self"); +} + #[tokio::test] async fn window_open_named_lightweight_popup_reuse_pushes_history_and_back_traverses() { let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).expect("loader"); diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/navigation/element_navigation.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/navigation/element_navigation.rs index b307bf70bc..652c64430f 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/navigation/element_navigation.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/navigation/element_navigation.rs @@ -1,5 +1,42 @@ use super::*; +#[test] +fn noopener_named_hyperlink_does_not_reuse_an_existing_iframe() { + let mut vm = new_storage_test_vm("https://example.com/page.html"); + vm.eval( + r#" +const html = document.createElement('html'); +const body = document.createElement('body'); +html.appendChild(body); +document.appendChild(html); +const frame = document.createElement('iframe'); +frame.name = 'report'; +body.append(frame); +const link = document.createElement('a'); +link.target = 'report'; +link.rel = 'noopener'; +link.href = 'about:blank#isolated'; +body.append(link); +link.click(); +"#, + ) + .expect("noopener named hyperlink should evaluate"); + + assert_eq!( + vm.eval("frame.contentWindow.location.href") + .expect("iframe URL should evaluate"), + "about:blank" + ); + let activations = vm.take_pending_popup_activations(); + assert_eq!(activations.len(), 1); + let crate::RendererPopupActivationSource::Window { exposes_opener, .. } = + activations[0].source() + else { + panic!("hyperlink must retain its exact Window source"); + }; + assert!(!exposes_opener); +} + #[tokio::test] async fn named_element_navigation_prefers_its_source_frame_over_duplicate_names() { for action in ["anchor", "submit", "requestSubmit"] { From 54406cd7630f1bbd3c11a16578e183da9d676bd0 Mon Sep 17 00:00:00 2001 From: lanyue-llk <270302213+lanyue-llk@users.noreply.github.com> Date: Tue, 29 Sep 2026 21:35:41 +0800 Subject: [PATCH 7/8] fix(browser): preserve accepted popup selections --- .../classic/extracted/windows_and_popups.rs | 97 ++++++++++ .../src/protocol_server/webdriver_classic.rs | 30 +-- moli-protocol/src/conn.rs | 2 + moli-protocol/src/conn/output.rs | 4 + .../src/conn/page_state/page_targets.rs | 38 +++- moli-protocol/src/conn/target/control.rs | 1 + .../src/conn/target/default_target.rs | 1 + moli-protocol/src/conn/target/observer.rs | 1 + moli-protocol/src/conn/target/projection.rs | 2 + moli-protocol/src/conn/target/transaction.rs | 2 + moli-protocol/src/devtools_runtime.rs | 1 + moli-protocol/src/domains/page/popup.rs | 183 ++++++++++++++++++ moli-protocol/src/domains/target.rs | 1 + .../src/domains/target/browser_context.rs | 1 + moli-protocol/src/domains/target/events.rs | 1 + moli-protocol/src/domains/target/popup.rs | 112 +++++++---- .../window_runtime/dialogs.rs | 48 ++++- .../src/native_bridge/context_host/popups.rs | 25 ++- .../element/activation/targets.rs | 5 + .../runtime/page_surface/popup_activation.rs | 31 +++ ...ice_worker_internal_client_request_body.rs | 4 + .../misc/extracted/popup_window.rs | 28 +++ 22 files changed, 542 insertions(+), 76 deletions(-) diff --git a/moli-protocol-server/src/protocol_server/tests/classic/extracted/windows_and_popups.rs b/moli-protocol-server/src/protocol_server/tests/classic/extracted/windows_and_popups.rs index 358a1102a7..2808ce4445 100644 --- a/moli-protocol-server/src/protocol_server/tests/classic/extracted/windows_and_popups.rs +++ b/moli-protocol-server/src/protocol_server/tests/classic/extracted/windows_and_popups.rs @@ -730,6 +730,103 @@ async fn webdriver_classic_window_open_self_click_waits_for_current_url() { ) .await; } + +#[tokio::test] +async fn webdriver_classic_keeps_all_live_popup_proxy_aliases() { + let app = build_router(test_state()); + let session = classic_request_json(app.clone(), Method::POST, "/session").await; + let session_id = session["value"]["sessionId"] + .as_str() + .expect("classic session id"); + let window_path = format!("/session/{session_id}/window"); + let execute_path = format!("/session/{session_id}/execute/sync"); + let original = classic_request_json(app.clone(), Method::GET, &window_path).await; + let original = original["value"].as_str().unwrap().to_owned(); + + let created = classic_request_json_with_body( + app.clone(), + Method::POST, + &execute_path, + json!({ + "script": "window.__oldReport = open('about:blank#report', 'report'); const helper = open('about:blank#helper', 'helper'); return [window.__oldReport, helper];", + "args": [] + }), + ) + .await; + let report = created["value"][0][CLASSIC_WINDOW_REFERENCE_KEY] + .as_str() + .expect("report handle") + .to_owned(); + let helper = created["value"][1][CLASSIC_WINDOW_REFERENCE_KEY] + .as_str() + .expect("helper handle") + .to_owned(); + + assert_eq!( + classic_request_json_with_body( + app.clone(), + Method::POST, + &window_path, + json!({ "handle": helper }), + ) + .await, + json!({ "value": null }) + ); + let second_alias = classic_request_json_with_body( + app.clone(), + Method::POST, + &execute_path, + json!({ + "script": "window.__newReport = open('about:blank#next', 'report'); return window.__newReport;", + "args": [] + }), + ) + .await; + assert_eq!( + second_alias["value"][CLASSIC_WINDOW_REFERENCE_KEY], + json!(report) + ); + + assert_eq!( + classic_request_json_with_body( + app.clone(), + Method::POST, + &window_path, + json!({ "handle": original }), + ) + .await, + json!({ "value": null }) + ); + let old_alias = classic_request_json_with_body( + app.clone(), + Method::POST, + &execute_path, + json!({ "script": "return window.__oldReport;", "args": [] }), + ) + .await; + assert_eq!( + old_alias["value"][CLASSIC_WINDOW_REFERENCE_KEY], + json!(report) + ); + + let handles = classic_request_json( + app.clone(), + Method::GET, + &format!("/session/{session_id}/window/handles"), + ) + .await; + assert_eq!( + handles["value"] + .as_array() + .unwrap() + .iter() + .filter(|handle| **handle == json!(report)) + .count(), + 1 + ); + + let _ = classic_request_json(app, Method::DELETE, &format!("/session/{session_id}")).await; +} #[tokio::test] async fn webdriver_classic_named_popup_reuse_navigates_existing_window() { let app = build_router(test_state()); diff --git a/moli-protocol-server/src/protocol_server/webdriver_classic.rs b/moli-protocol-server/src/protocol_server/webdriver_classic.rs index f282a78868..8213623f4b 100644 --- a/moli-protocol-server/src/protocol_server/webdriver_classic.rs +++ b/moli-protocol-server/src/protocol_server/webdriver_classic.rs @@ -2526,18 +2526,26 @@ async fn classic_popup_window_handles_by_id_for_script_result( .execute(window_handles_command(&context)) .await { - Ok(DevToolsCommandResult::GetTargets(result)) => Ok(result - .targets - .into_iter() - .filter(|target| { + Ok(DevToolsCommandResult::GetTargets(result)) => { + let mut handles = BTreeMap::new(); + for target in result.targets.into_iter().filter(|target| { target.kind == moli_protocol::devtools_runtime::DevToolsTargetKind::Page - }) - .filter_map(|target| { - let popup_id = target.moli_popup_id?; - let target_id = target.target_id?.into_string(); - Some((popup_id, target_id)) - }) - .collect()), + }) { + let Some(target_id) = target.target_id.map(|id| id.into_string()) else { + continue; + }; + let mut aliases = target.moli_popup_alias_ids; + if aliases.is_empty() + && let Some(popup_id) = target.moli_popup_id + { + aliases.push(popup_id); + } + for popup_id in aliases { + handles.insert(popup_id, target_id.clone()); + } + } + Ok(handles) + } Ok(_) => Err(ClassicError::new( ClassicErrorCode::UnknownError, "window handles returned an unexpected result", diff --git a/moli-protocol/src/conn.rs b/moli-protocol/src/conn.rs index cf96c84f0f..17f4afe571 100644 --- a/moli-protocol/src/conn.rs +++ b/moli-protocol/src/conn.rs @@ -3269,6 +3269,8 @@ impl CdpConnection { if let Some(target_id) = page_or_worker_target_info.target_id.as_ref() { page_or_worker_target_info.moli_popup_id = browser_context.target_popup_id(target_id.as_str()); + page_or_worker_target_info.moli_popup_alias_ids = + browser_context.target_popup_alias_ids(target_id.as_str()); } if let Some(tab_target_info) = self .target_control diff --git a/moli-protocol/src/conn/output.rs b/moli-protocol/src/conn/output.rs index db3feb769a..969651885a 100644 --- a/moli-protocol/src/conn/output.rs +++ b/moli-protocol/src/conn/output.rs @@ -3984,6 +3984,7 @@ impl BackgroundTargetCreatedEvent { can_access_opener: false, browser_context_id: event.browser_context_id, moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), }); build_event( "Target.targetCreated", @@ -5011,6 +5012,7 @@ mod tests { can_access_opener: true, browser_context_id: Some(DevToolsBrowserContextId::from("BID-target-info")), moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), }, ); @@ -5051,6 +5053,7 @@ mod tests { can_access_opener: false, browser_context_id: Some(DevToolsBrowserContextId::from("BID-created")), moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), }; let mut attached = BackgroundProtocolEvent::target_attached(TargetAttachmentEvent { target_id: DevToolsTargetId::from("TID-attached"), @@ -5067,6 +5070,7 @@ mod tests { can_access_opener: false, browser_context_id: None, moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), }, waiting_for_debugger: true, }); diff --git a/moli-protocol/src/conn/page_state/page_targets.rs b/moli-protocol/src/conn/page_state/page_targets.rs index 0507e12549..f0adedded4 100644 --- a/moli-protocol/src/conn/page_state/page_targets.rs +++ b/moli-protocol/src/conn/page_state/page_targets.rs @@ -288,6 +288,18 @@ impl BrowserContext { self.target_popup_ids.get(target_id).copied() } + pub(crate) fn target_popup_alias_ids(&self, target_id: &str) -> Vec { + let mut aliases = self + .popup_target_ids + .iter() + .filter_map(|(popup_id, candidate_target_id)| { + (candidate_target_id == target_id).then_some(*popup_id) + }) + .collect::>(); + aliases.sort_unstable(); + aliases + } + pub(crate) fn target_id_for_popup_id(&self, popup_id: u64) -> Option<&str> { self.popup_target_ids.get(&popup_id).and_then(|target_id| { self.devtools_target_info(target_id) @@ -300,11 +312,16 @@ impl BrowserContext { &self, source_target_id: &str, popup_id: u64, - target_name: &str, ) -> Option<&str> { let candidate_target_id = self.target_id_for_popup_id(popup_id)?; - (self.target_id_for_window_name(source_target_id, target_name) == Some(candidate_target_id)) - .then_some(candidate_target_id) + let source_group_id = self + .target_browsing_context_group_ids + .get(source_target_id)?; + (self + .target_browsing_context_group_ids + .get(candidate_target_id) + == Some(source_group_id)) + .then_some(candidate_target_id) } pub(crate) fn remember_target_opener( @@ -865,6 +882,7 @@ impl BrowserContext { can_access_opener: self.target_can_access_opener.contains(target_id), browser_context_id: Some(DevToolsBrowserContextId::from(self.id.as_str())), moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), }); } @@ -1256,6 +1274,7 @@ impl BrowserContext { can_access_opener: false, browser_context_id: Some(DevToolsBrowserContextId::from(self.id.as_str())), moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), } } @@ -1281,6 +1300,7 @@ impl BrowserContext { can_access_opener: false, browser_context_id: Some(DevToolsBrowserContextId::from(self.id.as_str())), moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), } } @@ -1299,6 +1319,7 @@ impl BrowserContext { can_access_opener: false, browser_context_id: Some(DevToolsBrowserContextId::from(self.id.as_str())), moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), } } } @@ -1615,19 +1636,19 @@ mod tests { "the source navigable takes priority over another related target" ); assert_eq!( - context.reusable_target_id_for_popup_id("TID-b", 41, "current"), - None, - "a popup identity hint must not bypass source-first name selection" + context.reusable_target_id_for_popup_id("TID-b", 41), + Some("TID-popup"), + "a call-time selected popup remains valid after its mutable name changes" ); context.remember_target_window_name("source", "TID-b"); assert_eq!( - context.reusable_target_id_for_popup_id("TID-b", 41, "current"), + context.reusable_target_id_for_popup_id("TID-b", 41), Some("TID-popup"), "the popup identity is accepted when the canonical name resolver selects it" ); assert_eq!( - context.reusable_target_id_for_popup_id("TID-a", 41, "current"), + context.reusable_target_id_for_popup_id("TID-a", 41), None, "a popup identity hint must not cross browsing-context groups" ); @@ -1635,6 +1656,7 @@ mod tests { context.remember_target_popup_id(Some(42), "TID-popup"); assert_eq!(context.target_id_for_popup_id(41), Some("TID-popup")); assert_eq!(context.target_id_for_popup_id(42), Some("TID-popup")); + assert_eq!(context.target_popup_alias_ids("TID-popup"), vec![41, 42]); assert_eq!(context.target_popup_id("TID-popup"), Some(42)); context.forget_target_popup_id_for_target("TID-popup"); assert_eq!(context.target_id_for_popup_id(41), None); diff --git a/moli-protocol/src/conn/target/control.rs b/moli-protocol/src/conn/target/control.rs index 18a2ac44bc..a23e97766f 100644 --- a/moli-protocol/src/conn/target/control.rs +++ b/moli-protocol/src/conn/target/control.rs @@ -582,6 +582,7 @@ mod tests { can_access_opener: false, browser_context_id: None, moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), } } diff --git a/moli-protocol/src/conn/target/default_target.rs b/moli-protocol/src/conn/target/default_target.rs index 4c9d6f7adc..35a39b35de 100644 --- a/moli-protocol/src/conn/target/default_target.rs +++ b/moli-protocol/src/conn/target/default_target.rs @@ -63,6 +63,7 @@ impl DefaultTargetLifecycle { can_access_opener: false, browser_context_id: Some(DevToolsBrowserContextId::from(DEFAULT_BROWSER_CONTEXT_ID)), moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), }) } diff --git a/moli-protocol/src/conn/target/observer.rs b/moli-protocol/src/conn/target/observer.rs index 0f65429314..ee9d9a7dce 100644 --- a/moli-protocol/src/conn/target/observer.rs +++ b/moli-protocol/src/conn/target/observer.rs @@ -308,6 +308,7 @@ mod tests { can_access_opener: false, browser_context_id: None, moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), } } diff --git a/moli-protocol/src/conn/target/projection.rs b/moli-protocol/src/conn/target/projection.rs index ceeea064c8..876037f12f 100644 --- a/moli-protocol/src/conn/target/projection.rs +++ b/moli-protocol/src/conn/target/projection.rs @@ -19,6 +19,7 @@ pub(crate) fn tab_target_info_from_page_target_info( can_access_opener: page_target_info.can_access_opener, browser_context_id: page_target_info.browser_context_id, moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), } } @@ -65,6 +66,7 @@ mod tests { can_access_opener: false, browser_context_id: Some(DevToolsBrowserContextId::from("BID-1")), moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), }, ); diff --git a/moli-protocol/src/conn/target/transaction.rs b/moli-protocol/src/conn/target/transaction.rs index 8af7348cf0..d2886b9e5b 100644 --- a/moli-protocol/src/conn/target/transaction.rs +++ b/moli-protocol/src/conn/target/transaction.rs @@ -749,6 +749,7 @@ mod tests { can_access_opener: false, browser_context_id: None, moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), }; let prepared = PreparedTargetAttach::new( "TID-page", @@ -858,6 +859,7 @@ mod tests { can_access_opener: false, browser_context_id: None, moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), }; let prepared = PreparedTargetHostDelta::created("TID-worker", Some(target_info.clone())); diff --git a/moli-protocol/src/devtools_runtime.rs b/moli-protocol/src/devtools_runtime.rs index 037d771542..16a4af7f1c 100644 --- a/moli-protocol/src/devtools_runtime.rs +++ b/moli-protocol/src/devtools_runtime.rs @@ -1581,6 +1581,7 @@ pub struct DevToolsTargetInfo { pub can_access_opener: bool, pub browser_context_id: Option, pub moli_popup_id: Option, + pub moli_popup_alias_ids: Vec, } impl DevToolsTargetInfo { diff --git a/moli-protocol/src/domains/page/popup.rs b/moli-protocol/src/domains/page/popup.rs index 06bda70b2d..ecee46923f 100644 --- a/moli-protocol/src/domains/page/popup.rs +++ b/moli-protocol/src/domains/page/popup.rs @@ -91,6 +91,9 @@ pub(super) async fn emit_prepared( source, disposition, popup_id, + selected_existing_target, + allows_named_target_selection, + navigation_requested, url, target_name, session_storage_store, @@ -108,6 +111,9 @@ pub(super) async fn emit_prepared( let creation = PopupTargetCreation::new( page_owner.browser_context_id().to_owned(), popup_id, + selected_existing_target, + allows_named_target_selection, + navigation_requested, url, target_name, opener, @@ -405,6 +411,183 @@ mod tests { ); } + #[tokio::test(flavor = "multi_thread")] + async fn selected_popup_identity_survives_same_turn_window_name_change() { + let mut conn = CdpConnection::default(); + conn.browser_context = Some(context("BID-1", "TID-opener", "SID-1")); + let owner = page_owner("BID-1", "TID-opener"); + + emit( + &mut conn, + owner.clone(), + vec![named_window_activation( + true, + 82, + "about:blank#first", + "old", + )], + ) + .await; + let target_id = conn + .browser_context_by_id("BID-1") + .and_then(|context| context.target_id_for_popup_id(82)) + .expect("first popup target") + .to_owned(); + let target_count = conn + .browser_context_by_id("BID-1") + .unwrap() + .devtools_target_infos() + .len(); + conn.browser_context_by_id_mut("BID-1") + .unwrap() + .remember_target_window_name("new", &target_id); + + emit( + &mut conn, + owner, + vec![ + named_window_activation(true, 82, "about:blank#next", "old") + .with_selected_existing_target(true), + ], + ) + .await; + + let context = conn.browser_context_by_id("BID-1").unwrap(); + assert_eq!(context.devtools_target_infos().len(), target_count); + assert_eq!(context.target_id_for_popup_id(82), Some(target_id.as_str())); + assert_eq!( + context + .devtools_target_info(&target_id) + .map(|info| info.url), + Some("about:blank#next".to_owned()) + ); + assert_eq!( + context.target_id_for_window_name("TID-opener", "new"), + Some(target_id.as_str()) + ); + assert_eq!(context.target_id_for_window_name("TID-opener", "old"), None); + } + + #[tokio::test(flavor = "multi_thread")] + async fn empty_url_keeps_the_selected_popup_document() { + let mut conn = CdpConnection::default(); + conn.browser_context = Some(context("BID-1", "TID-opener", "SID-1")); + let owner = page_owner("BID-1", "TID-opener"); + + emit( + &mut conn, + owner.clone(), + vec![named_window_activation( + true, + 83, + "about:blank#existing-document", + "report", + )], + ) + .await; + let target_id = conn + .browser_context_by_id("BID-1") + .and_then(|context| context.target_id_for_popup_id(83)) + .expect("named popup target") + .to_owned(); + let target_count = conn + .browser_context_by_id("BID-1") + .unwrap() + .devtools_target_infos() + .len(); + + emit( + &mut conn, + owner, + vec![ + named_window_activation(true, 83, "about:blank", "report") + .with_selected_existing_target(true) + .with_navigation_requested(false), + ], + ) + .await; + + let context = conn.browser_context_by_id("BID-1").unwrap(); + assert_eq!(context.devtools_target_infos().len(), target_count); + assert_eq!(context.target_id_for_popup_id(83), Some(target_id.as_str())); + assert_eq!( + context + .devtools_target_info(&target_id) + .map(|info| info.url), + Some("about:blank#existing-document".to_owned()) + ); + } + + #[tokio::test(flavor = "multi_thread")] + async fn stale_selected_popup_identity_does_not_fall_back_to_its_old_name() { + let mut conn = CdpConnection::default(); + conn.browser_context = Some(context("BID-1", "TID-opener", "SID-1")); + let owner = page_owner("BID-1", "TID-opener"); + + emit( + &mut conn, + owner.clone(), + vec![named_window_activation( + true, + 84, + "about:blank#selected", + "report", + )], + ) + .await; + let selected_target_id = conn + .browser_context_by_id("BID-1") + .and_then(|context| context.target_id_for_popup_id(84)) + .expect("selected popup target") + .to_owned(); + conn.browser_context_by_id_mut("BID-1") + .unwrap() + .take_page_target_for_close(&selected_target_id) + .expect("close selected popup target"); + + emit( + &mut conn, + owner.clone(), + vec![named_window_activation( + true, + 85, + "about:blank#replacement", + "report", + )], + ) + .await; + let replacement_target_id = conn + .browser_context_by_id("BID-1") + .and_then(|context| context.target_id_for_popup_id(85)) + .expect("replacement popup target") + .to_owned(); + let target_count = conn + .browser_context_by_id("BID-1") + .unwrap() + .devtools_target_infos() + .len(); + + emit( + &mut conn, + owner, + vec![ + named_window_activation(true, 84, "about:blank#must-not-land", "report") + .with_selected_existing_target(true), + ], + ) + .await; + + let context = conn.browser_context_by_id("BID-1").unwrap(); + assert_eq!(context.devtools_target_infos().len(), target_count); + assert_eq!(context.target_id_for_popup_id(84), None); + assert_eq!( + context + .devtools_target_info(&replacement_target_id) + .map(|info| info.url), + Some("about:blank#replacement".to_owned()) + ); + } + #[tokio::test(flavor = "multi_thread")] async fn removed_opener_downgrades_access_without_rebinding_to_current_target() { let mut conn = CdpConnection::default(); diff --git a/moli-protocol/src/domains/target.rs b/moli-protocol/src/domains/target.rs index 7e47836548..efee7543a6 100644 --- a/moli-protocol/src/domains/target.rs +++ b/moli-protocol/src/domains/target.rs @@ -1140,6 +1140,7 @@ mod devtools_runtime_entry_tests { can_access_opener: false, browser_context_id: None, moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), }, None, ) diff --git a/moli-protocol/src/domains/target/browser_context.rs b/moli-protocol/src/domains/target/browser_context.rs index ad25f102f4..a3b5ed9b47 100644 --- a/moli-protocol/src/domains/target/browser_context.rs +++ b/moli-protocol/src/domains/target/browser_context.rs @@ -389,6 +389,7 @@ pub(super) fn devtools_browser_target_info() -> DevToolsTargetInfo { can_access_opener: false, browser_context_id: None, moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), } } diff --git a/moli-protocol/src/domains/target/events.rs b/moli-protocol/src/domains/target/events.rs index 647ddbe841..7942286a72 100644 --- a/moli-protocol/src/domains/target/events.rs +++ b/moli-protocol/src/domains/target/events.rs @@ -202,6 +202,7 @@ fn devtools_target_info_from_cdp_value_lossy(value: Value) -> DevToolsTargetInfo .and_then(Value::as_str) .map(crate::devtools_runtime::DevToolsBrowserContextId::from), moli_popup_id: None, + moli_popup_alias_ids: Vec::new(), } } diff --git a/moli-protocol/src/domains/target/popup.rs b/moli-protocol/src/domains/target/popup.rs index 493b576720..4e31d4fd71 100644 --- a/moli-protocol/src/domains/target/popup.rs +++ b/moli-protocol/src/domains/target/popup.rs @@ -34,6 +34,9 @@ impl PopupTargetOpenerIdentity { pub(crate) struct PopupTargetCreation { browser_context_id: String, popup_id: Option, + selected_existing_target: bool, + allows_named_target_selection: bool, + navigation_requested: bool, url: String, target_name: String, opener: Option, @@ -48,6 +51,9 @@ impl PopupTargetCreation { pub(crate) fn new( browser_context_id: String, popup_id: Option, + selected_existing_target: bool, + allows_named_target_selection: bool, + navigation_requested: bool, url: String, target_name: String, opener: Option, @@ -60,6 +66,9 @@ impl PopupTargetCreation { Self { browser_context_id, popup_id, + selected_existing_target, + allows_named_target_selection, + navigation_requested, url, target_name, opener, @@ -80,6 +89,9 @@ pub(crate) async fn create_popup_target_from_renderer_output_background_events_a let PopupTargetCreation { browser_context_id, popup_id, + selected_existing_target, + allows_named_target_selection, + navigation_requested, url, target_name, opener, @@ -99,23 +111,26 @@ pub(crate) async fn create_popup_target_from_renderer_output_background_events_a return None; }; - let existing_target_id = if can_access_opener { - opener - .as_ref() - .and_then(|opener| { - popup_id.and_then(|popup_id| { - browser_context.reusable_target_id_for_popup_id( - &opener.target_id, - popup_id, - &target_name, - ) - }) - }) - .or_else(|| { - opener.as_ref().and_then(|opener| { - browser_context.target_id_for_window_name(&opener.target_id, &target_name) - }) + let existing_target_id = if can_access_opener && selected_existing_target { + let selected = opener.as_ref().and_then(|opener| { + popup_id.and_then(|popup_id| { + browser_context.reusable_target_id_for_popup_id(&opener.target_id, popup_id) }) + }); + let Some(selected) = selected else { + tracing::debug!( + browser_context_id, + ?popup_id, + ?target_name, + "dropping accepted popup action after its selected target became unavailable" + ); + return None; + }; + Some(selected) + } else if can_access_opener && allows_named_target_selection { + opener.as_ref().and_then(|opener| { + browser_context.target_id_for_window_name(&opener.target_id, &target_name) + }) } else { None }; @@ -130,44 +145,57 @@ pub(crate) async fn create_popup_target_from_renderer_output_background_events_a owner_state.set_window_name(observed_name); } } - let navigation = + let should_navigate = if navigation_requested { popup_target_has_loaded_page(conn, &browser_context_id, &existing_target_id) - .then(|| { - PopupTargetNavigationOwnerAction::capture( - conn, - &browser_context_id, - &existing_target_id, - url.clone(), - PopupTargetNavigationKind::NamedTargetReuse, - ) - }) - .flatten(); + } else { + false + }; + let navigation = should_navigate + .then(|| { + PopupTargetNavigationOwnerAction::capture( + conn, + &browser_context_id, + &existing_target_id, + url.clone(), + PopupTargetNavigationKind::NamedTargetReuse, + ) + }) + .flatten(); let activation = (disposition == moli_core::page::RendererPopupDisposition::Foreground) .then(|| { PopupTargetActivationAction::capture(conn, &browser_context_id, &existing_target_id) }) .flatten(); - let target_url_updated = conn - .browser_context_by_id_mut(&browser_context_id) - .is_some_and(|browser_context| { - browser_context.update_target_url(&existing_target_id, url.clone()) - }); - if target_url_updated { - emit_target_info_changed_for_target_background_event( - conn, - out, - &browser_context_id, - &existing_target_id, - ); - if let Some(navigation) = navigation { - conn.publish_popup_target_navigation_owner_action(navigation); + let target_updated = if navigation_requested { + conn.browser_context_by_id_mut(&browser_context_id) + .is_some_and(|browser_context| { + browser_context.update_target_url(&existing_target_id, url.clone()) + }) + } else { + conn.browser_context_by_id(&browser_context_id) + .and_then(|browser_context| { + browser_context.devtools_target_info(&existing_target_id) + }) + .is_some() + }; + if target_updated { + if navigation_requested { + emit_target_info_changed_for_target_background_event( + conn, + out, + &browser_context_id, + &existing_target_id, + ); + if let Some(navigation) = navigation { + conn.publish_popup_target_navigation_owner_action(navigation); + } } if let Some(activation) = activation { conn.publish_popup_target_activation_action(activation); } } - return (target_url_updated + return (target_updated && remember_resolved_popup_target( conn, &browser_context_id, diff --git a/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs b/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs index 106a866673..02fadb4069 100644 --- a/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs +++ b/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs @@ -128,6 +128,7 @@ pub(crate) fn window_open_callback<'s>( let Some(parsed) = webidl::parse_args::(scope, &args) else { return; }; + let navigation_requested = !parsed.raw_url.is_empty(); let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) else { rv.set(v8::null(scope).into()); return; @@ -160,7 +161,11 @@ pub(crate) fn window_open_callback<'s>( } }; if special_target == Some(SpecialBrowsingContextTarget::Current) { - navigate_window_open_self(scope, entered_window, url.as_str(), &mut rv); + if navigation_requested { + navigate_window_open_self(scope, entered_window, url.as_str(), &mut rv); + } else { + rv.set(entered_window.into()); + } return; } let parsed_features = WindowOpenFeatures::parse(&parsed.features); @@ -189,7 +194,19 @@ pub(crate) fn window_open_callback<'s>( target @ (SpecialBrowsingContextTarget::Parent | SpecialBrowsingContextTarget::Top), ) = special_target { - match navigate_existing_browsing_context_target(scope, host_ptr, target, &url) { + let selected = if navigation_requested { + navigate_existing_browsing_context_target(scope, host_ptr, target, &url) + } else { + let property = match target { + SpecialBrowsingContextTarget::Parent => "parent", + SpecialBrowsingContextTarget::Top => "top", + _ => unreachable!(), + }; + entered_window + .get(scope, crate::util::v8str(scope, property).into()) + .and_then(|value| v8::Local::::try_from(value).ok()) + }; + match selected { Some(window) => rv.set(window.into()), None => rv.set(v8::null(scope).into()), } @@ -203,15 +220,27 @@ pub(crate) fn window_open_callback<'s>( .as_deref() == Some(parsed.target_name.as_str()) { - navigate_window_open_self(scope, entered_window, &url, &mut rv); + if navigation_requested { + navigate_window_open_self(scope, entered_window, &url, &mut rv); + } else { + rv.set(entered_window.into()); + } + if suppress_opener { + rv.set(v8::null(scope).into()); + } return; } if let Some(target_window) = existing_named_child_window_for_window_open(scope, host_ptr, &parsed.target_name) && !suppress_opener - && navigate_named_iframe_target(scope, host_ptr, &parsed.target_name, &url, None) + && (!navigation_requested + || navigate_named_iframe_target(scope, host_ptr, &parsed.target_name, &url, None)) { - rv.set(target_window.into()); + if suppress_opener { + rv.set(v8::null(scope).into()); + } else { + rv.set(target_window.into()); + } return; } let host = unsafe { &mut *host_ptr }; @@ -253,8 +282,14 @@ pub(crate) fn window_open_callback<'s>( && let Some(opened_popup) = host.open_lightweight_popup_window( scope, host_ptr, + (!suppress_opener).then_some( + crate::native_bridge::PendingWindowMessageEndpoint::from_dispatch_scope( + source_scope, + ), + ), opener, opener_child_handle, + navigation_requested, &parsed.target_name, &url, entered_base_url, @@ -280,6 +315,8 @@ pub(crate) fn window_open_callback<'s>( parsed.target_name, popup_disposition, ) + .with_selected_existing_target(!opened_popup.created_new_browsing_context) + .with_navigation_requested(navigation_requested) .with_initial_auxiliary_state(session_storage_store, initial_empty_document_storage_key) .with_top_level_browsing_context_state(top_level_browsing_context), window_open_event, @@ -301,6 +338,7 @@ pub(crate) fn window_open_callback<'s>( parsed.target_name, popup_disposition, ) + .with_navigation_requested(navigation_requested) .with_initial_auxiliary_state(None, None), Some(window_open_event), ); diff --git a/moli-renderer-v8/src/native_bridge/context_host/popups.rs b/moli-renderer-v8/src/native_bridge/context_host/popups.rs index 55b701de4b..e045eafc22 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/popups.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/popups.rs @@ -696,14 +696,15 @@ impl JsContextHost { &mut self, scope: &mut v8::PinScope<'s, '_>, host_ptr: *mut JsContextHost, + initiator: Option, opener: Option>, opener_child_handle: Option, + navigate_existing: bool, target_name: &str, href: &str, creator_base_url: Url, creator_policy_container: DocumentPolicyContainer, ) -> Option> { - let initiator = lightweight_popup_initiator_endpoint(scope, opener, opener_child_handle); if let Some(initiator) = initiator && let Some(name) = trackable_lightweight_popup_window_name(target_name) && let Some(popup_id) = @@ -716,15 +717,19 @@ impl JsContextHost { .then_some(*popup_id) }) && self.lightweight_popup_is_open(popup_id) - && let Some(window) = self.reopen_lightweight_popup_window( - scope, - popup_id, - opener, - opener_child_handle, - href, - creator_base_url.clone(), - creator_policy_container.clone(), - ) + && let Some(window) = if navigate_existing { + self.reopen_lightweight_popup_window( + scope, + popup_id, + opener, + opener_child_handle, + href, + creator_base_url.clone(), + creator_policy_container.clone(), + ) + } else { + self.lightweight_popup_window(scope, popup_id) + } { return Some(OpenedLightweightPopup { window, diff --git a/moli-renderer-v8/src/native_bridge/element/activation/targets.rs b/moli-renderer-v8/src/native_bridge/element/activation/targets.rs index 67f5d8dc98..7115d94d7d 100644 --- a/moli-renderer-v8/src/native_bridge/element/activation/targets.rs +++ b/moli-renderer-v8/src/native_bridge/element/activation/targets.rs @@ -222,8 +222,12 @@ fn navigate_hyperlink_popup_target( let Some(opened_popup) = runtime.open_lightweight_popup_window( scope, runtime_ptr, + (!relations.suppress_opener).then_some( + crate::native_bridge::PendingWindowMessageEndpoint::from_dispatch_scope(dispatch_scope), + ), opener, None, + true, target_name, resolved_url, creator.base_url, @@ -269,6 +273,7 @@ fn navigate_hyperlink_popup_target( target_name.to_owned(), disposition, ) + .with_selected_existing_target(!opened_popup.created_new_browsing_context) .with_initial_auxiliary_state(session_storage_store, initial_empty_document_storage_key) .with_top_level_browsing_context_state(top_level_browsing_context), window_open_event, diff --git a/moli-renderer-v8/src/runtime/page_surface/popup_activation.rs b/moli-renderer-v8/src/runtime/page_surface/popup_activation.rs index d139ae2147..13cefb91a8 100644 --- a/moli-renderer-v8/src/runtime/page_surface/popup_activation.rs +++ b/moli-renderer-v8/src/runtime/page_surface/popup_activation.rs @@ -47,6 +47,9 @@ pub struct RendererPendingPopupActivation { source: RendererPopupActivationSource, disposition: RendererPopupDisposition, popup_id: Option, + selected_existing_target: bool, + allows_named_target_selection: bool, + navigation_requested: bool, url: String, target_name: String, session_storage_store: Option, @@ -68,6 +71,9 @@ impl RendererPendingPopupActivation { !is_special_browsing_context_target(&target_name), "popup activation must not carry an existing-context special target" ); + let allows_named_target_selection = !target_name.is_empty() + && !target_name.eq_ignore_ascii_case("_blank") + && !is_special_browsing_context_target(&target_name); Self { source: RendererPopupActivationSource::Window { root_document, @@ -76,6 +82,9 @@ impl RendererPendingPopupActivation { }, disposition, popup_id, + selected_existing_target: false, + allows_named_target_selection, + navigation_requested: true, url, target_name, session_storage_store: None, @@ -98,6 +107,9 @@ impl RendererPendingPopupActivation { source: RendererPopupActivationSource::BrowserContext, disposition, popup_id, + selected_existing_target: false, + allows_named_target_selection: false, + navigation_requested: true, url, target_name, session_storage_store: None, @@ -133,6 +145,16 @@ impl RendererPendingPopupActivation { self } + pub fn with_selected_existing_target(mut self, selected: bool) -> Self { + self.selected_existing_target = selected; + self + } + + pub fn with_navigation_requested(mut self, requested: bool) -> Self { + self.navigation_requested = requested; + self + } + pub fn source(&self) -> &RendererPopupActivationSource { &self.source } @@ -160,6 +182,9 @@ impl RendererPendingPopupActivation { RendererPopupActivationSource, RendererPopupDisposition, Option, + bool, + bool, + bool, String, String, Option, @@ -170,6 +195,9 @@ impl RendererPendingPopupActivation { self.source, self.disposition, self.popup_id, + self.selected_existing_target, + self.allows_named_target_selection, + self.navigation_requested, self.url, self.target_name, self.session_storage_store, @@ -184,6 +212,9 @@ impl PartialEq for RendererPendingPopupActivation { self.source == other.source && self.disposition == other.disposition && self.popup_id == other.popup_id + && self.selected_existing_target == other.selected_existing_target + && self.allows_named_target_selection == other.allows_named_target_selection + && self.navigation_requested == other.navigation_requested && self.url == other.url && self.target_name == other.target_name && match (&self.session_storage_store, &other.session_storage_store) { diff --git a/moli-renderer-v8/src/script_vm/service_worker_internal_client_request_body.rs b/moli-renderer-v8/src/script_vm/service_worker_internal_client_request_body.rs index 298f7851c9..5e54dc70e0 100644 --- a/moli-renderer-v8/src/script_vm/service_worker_internal_client_request_body.rs +++ b/moli-renderer-v8/src/script_vm/service_worker_internal_client_request_body.rs @@ -251,6 +251,8 @@ impl ScriptVm { host_ptr, None, None, + None, + true, "_blank", &url, creator_base_url.clone(), @@ -337,6 +339,8 @@ impl ScriptVm { host_ptr, None, None, + None, + true, "_blank", &url, creator_base_url.clone(), diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/popup_window.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/popup_window.rs index abbfe0f617..8caa577a67 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/popup_window.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/popup_window.rs @@ -653,6 +653,34 @@ fn named_popup_group_survives_intermediate_opener_close() { ); } +#[test] +fn window_open_empty_url_selects_named_source_without_navigation() { + let mut vm = new_storage_test_vm("https://example.com/source"); + assert_eq!( + vm.eval( + "window.name = 'self'; globalThis.marker = 7; const selected = open('', 'self'); JSON.stringify([selected === window, location.href, marker]);" + ) + .expect("empty URL named-source selection should evaluate"), + r#"[true,"https://example.com/source",7]"# + ); + assert!(vm.take_pending_location_navigation_with_seed().is_none()); +} + +#[test] +fn window_open_empty_url_reuses_named_popup_without_navigation() { + let mut vm = new_storage_test_vm("https://example.com/source"); + assert_eq!( + vm.eval( + "globalThis.popup = open('about:blank#kept', 'report'); const selected = open('', 'report'); JSON.stringify([selected === popup, popup.location.href]);" + ) + .expect("empty URL named-popup selection should evaluate"), + r#"[true,"about:blank#kept"]"# + ); + let activations = vm.take_pending_popup_activations(); + assert_eq!(activations.len(), 2); + assert_eq!(activations[0].popup_id(), activations[1].popup_id()); +} + #[test] fn window_open_prefers_the_same_named_source_over_a_related_popup() { let mut vm = new_storage_test_vm("https://example.com/source"); From a9858cc311b13a03931bf973f020d5cb0da2d5dc Mon Sep 17 00:00:00 2001 From: lanyue-llk <270302213+lanyue-llk@users.noreply.github.com> Date: Tue, 29 Sep 2026 22:57:42 +0800 Subject: [PATCH 8/8] fix(browser): align named target lifecycle semantics --- .../classic/extracted/windows_and_popups.rs | 264 ++++++++++++++++++ .../src/conn/page_state/page_targets.rs | 14 +- moli-protocol/src/domains/page/popup.rs | 12 +- moli-protocol/src/domains/target/popup.rs | 4 +- .../window_runtime/dialogs.rs | 17 +- .../child_frame_runtime/window.rs | 4 +- .../src/native_bridge/context_host/popups.rs | 117 +++++--- moli-renderer-v8/src/native_bridge/element.rs | 4 +- .../native_bridge/element/activation/mod.rs | 4 +- .../element/activation/targets.rs | 127 +++++---- .../src/runtime/browser_context_runtime.rs | 39 ++- .../misc/extracted/popup_window.rs | 120 +++++++- .../navigation/element_navigation.rs | 13 +- 13 files changed, 617 insertions(+), 122 deletions(-) diff --git a/moli-protocol-server/src/protocol_server/tests/classic/extracted/windows_and_popups.rs b/moli-protocol-server/src/protocol_server/tests/classic/extracted/windows_and_popups.rs index 2808ce4445..1c6fcf4798 100644 --- a/moli-protocol-server/src/protocol_server/tests/classic/extracted/windows_and_popups.rs +++ b/moli-protocol-server/src/protocol_server/tests/classic/extracted/windows_and_popups.rs @@ -825,8 +825,272 @@ async fn webdriver_classic_keeps_all_live_popup_proxy_aliases() { 1 ); + assert_eq!( + classic_request_json_with_body( + app.clone(), + Method::POST, + &window_path, + json!({ "handle": report }), + ) + .await, + json!({ "value": null }) + ); + let remaining = classic_request_json(app.clone(), Method::DELETE, &window_path).await; + assert_eq!(remaining["value"].as_array().map(Vec::len), Some(2)); + + for (owner, proxy) in [(&original, "__oldReport"), (&helper, "__newReport")] { + assert_eq!( + classic_request_json_with_body( + app.clone(), + Method::POST, + &window_path, + json!({ "handle": owner }), + ) + .await, + json!({ "value": null }) + ); + assert_eq!( + classic_request_json_with_body( + app.clone(), + Method::POST, + &execute_path, + json!({ + "script": format!("return window.{proxy}.closed;"), + "args": [] + }), + ) + .await, + json!({ "value": true }), + "every live alias must observe the shared target close" + ); + } + let _ = classic_request_json(app, Method::DELETE, &format!("/session/{session_id}")).await; } + +#[tokio::test] +async fn webdriver_classic_noopener_reuses_a_related_named_popup() { + let app = build_router(test_state()); + let session = classic_request_json(app.clone(), Method::POST, "/session").await; + let session_id = session["value"]["sessionId"] + .as_str() + .expect("classic session id"); + let window_path = format!("/session/{session_id}/window"); + let handles_path = format!("/session/{session_id}/window/handles"); + let execute_path = format!("/session/{session_id}/execute/sync"); + + let created = classic_request_json_with_body( + app.clone(), + Method::POST, + &execute_path, + json!({ + "script": "window.__report = open('about:blank#first', 'report'); return window.__report;", + "args": [] + }), + ) + .await; + let report = created["value"][CLASSIC_WINDOW_REFERENCE_KEY] + .as_str() + .expect("report handle") + .to_owned(); + assert_eq!( + classic_request_json_with_body( + app.clone(), + Method::POST, + &execute_path, + json!({ + "script": "return window.__report.opener === window;", + "args": [] + }), + ) + .await, + json!({ "value": true }), + "the original popup proxy must retain its opener" + ); + let before = classic_request_json(app.clone(), Method::GET, &handles_path).await; + + let reused = classic_request_json_with_body( + app.clone(), + Method::POST, + &execute_path, + json!({ + "script": "return open('about:blank#second', 'report', 'noopener') === null;", + "args": [] + }), + ) + .await; + assert_eq!(reused, json!({ "value": true })); + assert_eq!( + classic_request_json(app.clone(), Method::GET, &handles_path).await, + before, + "noopener must not create a new target when a related named target exists" + ); + + assert_eq!( + classic_request_json_with_body( + app.clone(), + Method::POST, + &window_path, + json!({ "handle": report }), + ) + .await, + json!({ "value": null }) + ); + assert_eq!( + classic_request_json_with_body( + app.clone(), + Method::POST, + &execute_path, + json!({ + "script": "return location.href;", + "args": [] + }), + ) + .await, + json!({ "value": "about:blank#second" }), + "the selected target must receive the noopener navigation" + ); + + let original = before["value"] + .as_array() + .and_then(|handles| { + handles.iter().find_map(|handle| { + let handle = handle.as_str()?; + (handle != report).then(|| handle.to_owned()) + }) + }) + .expect("original window handle"); + assert_eq!( + classic_request_json_with_body( + app.clone(), + Method::POST, + &window_path, + json!({ "handle": original }), + ) + .await, + json!({ "value": null }) + ); + assert_eq!( + classic_request_json_with_body( + app.clone(), + Method::POST, + &execute_path, + json!({ + "script": "return window.__report.opener === window;", + "args": [] + }), + ) + .await, + json!({ "value": true }), + "reusing with noopener must not clear the existing proxy's opener" + ); + + let _ = classic_request_json(app, Method::DELETE, &format!("/session/{session_id}")).await; +} + +#[tokio::test] +async fn webdriver_classic_close_retires_popup_aliases_before_named_reopen() { + let app = build_router(test_state()); + let session = classic_request_json(app.clone(), Method::POST, "/session").await; + let session_id = session["value"]["sessionId"] + .as_str() + .expect("classic session id"); + let window_path = format!("/session/{session_id}/window"); + let execute_path = format!("/session/{session_id}/execute/sync"); + let original = classic_request_json(app.clone(), Method::GET, &window_path).await; + let original = original["value"].as_str().unwrap().to_owned(); + + let created = classic_request_json_with_body( + app.clone(), + Method::POST, + &execute_path, + json!({ + "script": "window.__oldReport = open('about:blank#one', 'report'); return window.__oldReport;", + "args": [] + }), + ) + .await; + let old_report = created["value"][CLASSIC_WINDOW_REFERENCE_KEY] + .as_str() + .expect("old report handle") + .to_owned(); + assert_eq!( + classic_request_json_with_body( + app.clone(), + Method::POST, + &window_path, + json!({ "handle": old_report }), + ) + .await, + json!({ "value": null }) + ); + let remaining = classic_request_json(app.clone(), Method::DELETE, &window_path).await; + assert_eq!(remaining, json!({ "value": [original.clone()] })); + + assert_eq!( + classic_request_json_with_body( + app.clone(), + Method::POST, + &window_path, + json!({ "handle": original }), + ) + .await, + json!({ "value": null }) + ); + assert_eq!( + classic_request_json_with_body( + app.clone(), + Method::POST, + &execute_path, + json!({ + "script": "return window.__oldReport.closed;", + "args": [] + }), + ) + .await, + json!({ "value": true }), + "a proxy must observe that its protocol target was closed" + ); + let replacement = classic_request_json_with_body( + app.clone(), + Method::POST, + &execute_path, + json!({ + "script": "window.__newReport = open('about:blank#two', 'report'); return window.__newReport;", + "args": [] + }), + ) + .await; + let new_report = replacement["value"][CLASSIC_WINDOW_REFERENCE_KEY] + .as_str() + .expect("replacement report handle"); + assert_ne!(new_report, old_report); + + let _ = classic_request_json(app, Method::DELETE, &format!("/session/{session_id}")).await; +} + +#[tokio::test] +async fn webdriver_classic_empty_parent_ignores_replaced_public_parent_property() { + let app = build_router(test_state()); + let session = classic_request_json(app.clone(), Method::POST, "/session").await; + let session_id = session["value"]["sessionId"] + .as_str() + .expect("classic session id"); + let result = classic_request_json_with_body( + app.clone(), + Method::POST, + &format!("/session/{session_id}/execute/sync"), + json!({ + "script": "const fake = {}; let reads = 0; Object.defineProperty(window, 'parent', { configurable: true, get() { ++reads; return fake; } }); const selected = open('', '_parent'); return [selected === window, selected === fake, reads];", + "args": [] + }), + ) + .await; + assert_eq!(result, json!({ "value": [true, false, 0] })); + + let _ = classic_request_json(app, Method::DELETE, &format!("/session/{session_id}")).await; +} + #[tokio::test] async fn webdriver_classic_named_popup_reuse_navigates_existing_window() { let app = build_router(test_state()); diff --git a/moli-protocol/src/conn/page_state/page_targets.rs b/moli-protocol/src/conn/page_state/page_targets.rs index f0adedded4..0af7203d13 100644 --- a/moli-protocol/src/conn/page_state/page_targets.rs +++ b/moli-protocol/src/conn/page_state/page_targets.rs @@ -16,6 +16,9 @@ use moli_core::network::SharedWebStorageStore; impl BrowserContext { pub(crate) fn take_page_target_for_close(&mut self, target_id: &str) -> Option { let target = self.page_targets.remove(target_id)?; + let popup_alias_ids = self.target_popup_alias_ids(target_id); + self.renderer_runtime() + .retire_lightweight_popup_ids(&popup_alias_ids); self.forget_target_opener_references_for_target(target_id); self.target_browsing_context_group_ids.remove(target_id); self.forget_target_window_names_for_target(target_id); @@ -1629,6 +1632,8 @@ mod tests { context.remember_target_window_name("current", "TID-popup"); context.remember_target_window_name("current", "TID-b"); + let renderer_runtime = context.renderer_runtime(); + renderer_runtime.register_lightweight_popup_id(41); context.remember_target_popup_id(Some(41), "TID-popup"); assert_eq!( context.target_id_for_window_name("TID-b", "current"), @@ -1653,14 +1658,21 @@ mod tests { "a popup identity hint must not cross browsing-context groups" ); + renderer_runtime.register_lightweight_popup_id(42); context.remember_target_popup_id(Some(42), "TID-popup"); assert_eq!(context.target_id_for_popup_id(41), Some("TID-popup")); assert_eq!(context.target_id_for_popup_id(42), Some("TID-popup")); assert_eq!(context.target_popup_alias_ids("TID-popup"), vec![41, 42]); assert_eq!(context.target_popup_id("TID-popup"), Some(42)); - context.forget_target_popup_id_for_target("TID-popup"); + assert!(renderer_runtime.lightweight_popup_id_is_live(41)); + assert!(renderer_runtime.lightweight_popup_id_is_live(42)); + context + .take_page_target_for_close("TID-popup") + .expect("popup target should close"); assert_eq!(context.target_id_for_popup_id(41), None); assert_eq!(context.target_id_for_popup_id(42), None); + assert!(!renderer_runtime.lightweight_popup_id_is_live(41)); + assert!(!renderer_runtime.lightweight_popup_id_is_live(42)); } #[test] diff --git a/moli-protocol/src/domains/page/popup.rs b/moli-protocol/src/domains/page/popup.rs index ecee46923f..aa9e99aa24 100644 --- a/moli-protocol/src/domains/page/popup.rs +++ b/moli-protocol/src/domains/page/popup.rs @@ -376,7 +376,7 @@ mod tests { } #[tokio::test(flavor = "multi_thread")] - async fn noopener_named_popup_does_not_reuse_a_related_target() { + async fn noopener_named_popup_reuses_a_related_target_without_changing_its_opener() { let mut conn = CdpConnection::default(); conn.browser_context = Some(context("BID-1", "TID-opener", "SID-1")); let owner = page_owner("BID-1", "TID-opener"); @@ -405,10 +405,18 @@ mod tests { .await; let context = conn.browser_context_by_id("BID-1").unwrap(); - assert_ne!( + assert_eq!( context.target_id_for_popup_id(80), context.target_id_for_popup_id(81) ); + let target_id = context.target_id_for_popup_id(80).unwrap(); + let info = context.devtools_target_info(target_id).unwrap(); + assert_eq!(info.url, "about:blank#isolated"); + assert_eq!( + info.opener_id.as_ref().map(|id| id.as_str()), + Some("TID-opener") + ); + assert!(info.can_access_opener); } #[tokio::test(flavor = "multi_thread")] diff --git a/moli-protocol/src/domains/target/popup.rs b/moli-protocol/src/domains/target/popup.rs index 4e31d4fd71..9404f37484 100644 --- a/moli-protocol/src/domains/target/popup.rs +++ b/moli-protocol/src/domains/target/popup.rs @@ -111,7 +111,7 @@ pub(crate) async fn create_popup_target_from_renderer_output_background_events_a return None; }; - let existing_target_id = if can_access_opener && selected_existing_target { + let existing_target_id = if selected_existing_target { let selected = opener.as_ref().and_then(|opener| { popup_id.and_then(|popup_id| { browser_context.reusable_target_id_for_popup_id(&opener.target_id, popup_id) @@ -127,7 +127,7 @@ pub(crate) async fn create_popup_target_from_renderer_output_background_events_a return None; }; Some(selected) - } else if can_access_opener && allows_named_target_selection { + } else if allows_named_target_selection { opener.as_ref().and_then(|opener| { browser_context.target_id_for_window_name(&opener.target_id, &target_name) }) diff --git a/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs b/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs index 02fadb4069..999ab81a08 100644 --- a/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs +++ b/moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs @@ -8,8 +8,8 @@ use crate::{ native_bridge::{ InputNavigationPolicy, OwnerDispatchScope, child_window_handle_from_marker_data, element::{ - SpecialBrowsingContextTarget, navigate_existing_browsing_context_target, - navigate_named_iframe_target, + SpecialBrowsingContextTarget, existing_browsing_context_target, + navigate_existing_browsing_context_target, navigate_named_iframe_target, }, entered_child_window_handle, }, @@ -197,14 +197,7 @@ pub(crate) fn window_open_callback<'s>( let selected = if navigation_requested { navigate_existing_browsing_context_target(scope, host_ptr, target, &url) } else { - let property = match target { - SpecialBrowsingContextTarget::Parent => "parent", - SpecialBrowsingContextTarget::Top => "top", - _ => unreachable!(), - }; - entered_window - .get(scope, crate::util::v8str(scope, property).into()) - .and_then(|value| v8::Local::::try_from(value).ok()) + existing_browsing_context_target(scope, host_ptr, target) }; match selected { Some(window) => rv.set(window.into()), @@ -214,7 +207,6 @@ pub(crate) fn window_open_callback<'s>( } let source_scope = unsafe { &*host_ptr }.entered_owner_dispatch_scope(scope); if trackable_named_popup_target_name(&parsed.target_name).is_some() - && !suppress_opener && unsafe { &*host_ptr } .window_name_for_dispatch_scope(source_scope) .as_deref() @@ -232,7 +224,6 @@ pub(crate) fn window_open_callback<'s>( } if let Some(target_window) = existing_named_child_window_for_window_open(scope, host_ptr, &parsed.target_name) - && !suppress_opener && (!navigation_requested || navigate_named_iframe_target(scope, host_ptr, &parsed.target_name, &url, None)) { @@ -282,7 +273,7 @@ pub(crate) fn window_open_callback<'s>( && let Some(opened_popup) = host.open_lightweight_popup_window( scope, host_ptr, - (!suppress_opener).then_some( + Some( crate::native_bridge::PendingWindowMessageEndpoint::from_dispatch_scope( source_scope, ), diff --git a/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/window.rs b/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/window.rs index 2976259f5f..13e2488174 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/window.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/child_frame_runtime/window.rs @@ -1214,7 +1214,7 @@ impl JsContextHost { self.child_window_proxy_records.realm_top(scope, handle) } - pub(in crate::native_bridge::context_host) fn child_browsing_context_parent_window<'s>( + pub(crate) fn child_browsing_context_parent_window<'s>( &mut self, scope: &mut v8::PinScope<'s, '_>, handle: DomHandle, @@ -1368,7 +1368,7 @@ impl JsContextHost { (parent, top) } - pub(in crate::native_bridge::context_host) fn child_browsing_context_root_window<'s>( + pub(crate) fn child_browsing_context_root_window<'s>( &mut self, scope: &mut v8::PinScope<'s, '_>, handle: DomHandle, diff --git a/moli-renderer-v8/src/native_bridge/context_host/popups.rs b/moli-renderer-v8/src/native_bridge/context_host/popups.rs index e045eafc22..ceb46cf014 100644 --- a/moli-renderer-v8/src/native_bridge/context_host/popups.rs +++ b/moli-renderer-v8/src/native_bridge/context_host/popups.rs @@ -39,9 +39,9 @@ use crate::{ PopupDocumentLoadCompletion, PopupDocumentLoadOutcome, }, util::{ - context_host_ptr_from_global_bridge, define_non_enumerable_static_property, - get_private_value, global_constructor_object, set_private_value, throw_type_error, - v8_string, v8str, + context_host_ptr_from_global_bridge, context_host_ptr_from_window_object, + define_non_enumerable_static_property, get_private_value, global_constructor_object, + set_private_value, throw_type_error, v8_string, v8str, }, window_document_identity::{ LightweightPopupDocumentId, LightweightPopupDocumentOwner, LightweightPopupLocalWindowId, @@ -160,6 +160,8 @@ struct LightweightPopupPopStateEventDeclaration<'scope> { struct LightweightPopupWindowMethodsDeclaration { #[webapi(method, length = 0, callback = lightweight_popup_close_callback)] close: (), + #[webapi(accessor_property, getter = lightweight_popup_closed_getter)] + closed: (), } #[derive(Default, WebApiObject)] @@ -607,6 +609,8 @@ impl JsContextHost { &mut self, popup_id: u64, ) -> Option { + self.browser_context_runtime + .retire_lightweight_popup_ids(&[popup_id]); let record = self.lightweight_popup_record_mut(popup_id)?; let lifecycle = std::mem::replace(&mut record.lifecycle, LightweightPopupLifecycle::Closed); let LightweightPopupLifecycle::Open(open) = lifecycle else { @@ -721,6 +725,7 @@ impl JsContextHost { self.reopen_lightweight_popup_window( scope, popup_id, + Some(initiator), opener, opener_child_handle, href, @@ -740,6 +745,7 @@ impl JsContextHost { let (window, popup_id) = self.create_lightweight_popup_window( scope, host_ptr, + initiator, opener, opener_child_handle, target_name, @@ -760,6 +766,7 @@ impl JsContextHost { &mut self, scope: &mut v8::PinScope<'s, '_>, host_ptr: *mut JsContextHost, + initiator: Option, opener: Option>, opener_child_handle: Option, target_name: &str, @@ -783,9 +790,7 @@ impl JsContextHost { .expect("new popup Document requires its creator's exact resource authority") .clone(); let storage_scope = self.lightweight_popup_storage_scope_for_initiated_navigation( - scope, - opener, - opener_child_handle, + initiator.or(opener_endpoint), &initial_url, opener_sandbox_policy.is_some_and(|policy| policy.forces_opaque_origin), ); @@ -833,12 +838,6 @@ impl JsContextHost { set_object_slot(scope, window, "__moliWindowParent", window.into()); set_object_slot(scope, window, "__moliWindowTop", window.into()); set_object_slot(scope, window, "__moliWindowFrames", window.into()); - set_object_slot( - scope, - window, - "closed", - v8::Boolean::new(scope, false).into(), - ); set_object_slot(scope, window, "self", window.into()); set_object_slot(scope, window, "window", window.into()); set_object_slot(scope, window, "globalThis", window.into()); @@ -885,7 +884,7 @@ impl JsContextHost { initial_execution_context_owner, ) } else if moli_url::is_about_blank(&initial_url) - && let Some(inherited) = opener_endpoint.and_then(|endpoint| { + && let Some(inherited) = initiator.or(opener_endpoint).and_then(|endpoint| { self.window_access_origin_for_dispatch_scope(endpoint.dispatch_scope()) }) { @@ -947,6 +946,8 @@ impl JsContextHost { top_level_browsing_context, }, ); + self.browser_context_runtime + .register_lightweight_popup_id(popup_id); self.register_committed_document_resource_loader( crate::network::context::DocumentFetchContext::new( super::WindowDocumentOwner::LightweightPopup(initial_document_owner), @@ -1029,6 +1030,7 @@ impl JsContextHost { &mut self, scope: &mut v8::PinScope<'s, '_>, popup_id: u64, + initiator: Option, opener: Option>, opener_child_handle: Option, href: &str, @@ -1047,8 +1049,8 @@ impl JsContextHost { &mut navigation_state.policy_container.sandbox, opener_sandbox_policy, ); - let initiator_endpoint = - lightweight_popup_initiator_endpoint(scope, opener, opener_child_handle); + let initiator_endpoint = initiator + .or_else(|| lightweight_popup_initiator_endpoint(scope, opener, opener_child_handle)); let previous_url = lightweight_popup_location_href(scope, window).unwrap_or_else(about_blank_url); let target_url = parsed_url; @@ -1078,9 +1080,7 @@ impl JsContextHost { ); let queue_synthetic_load = if moli_url::is_about_blank(&target_url) { let storage_scope = self.lightweight_popup_storage_scope_for_initiated_navigation( - scope, - opener, - opener_child_handle, + initiator_endpoint, &target_url, opener_sandbox_policy.is_some_and(|policy| policy.forces_opaque_origin), ); @@ -1262,11 +1262,9 @@ impl JsContextHost { target_store } - fn lightweight_popup_storage_scope_for_initiated_navigation<'s>( + fn lightweight_popup_storage_scope_for_initiated_navigation( &mut self, - scope: &mut v8::PinScope<'s, '_>, - opener: Option>, - opener_child_handle: Option, + initiator: Option, target_url: &Url, sandbox_forces_opaque_origin: bool, ) -> LightweightPopupStorageScope { @@ -1274,13 +1272,16 @@ impl JsContextHost { return self.lightweight_popup_opaque_storage_scope(target_url); } if moli_url::is_about_blank(target_url) - && let Some(opener_scope) = - self.lightweight_popup_opener_storage_scope(scope, opener, opener_child_handle) + && let Some(opener_scope) = initiator + .and_then(|initiator| self.lightweight_popup_initiator_storage_scope(initiator)) { if opener_scope.origin() == "null" { return opener_scope; } - if opener_child_handle.is_none() { + if !matches!( + initiator, + Some(super::PendingWindowMessageEndpoint::ChildWindow(_)) + ) { return opener_scope; } let storage_key = web_storage_key_for_child_about_blank_popup(&opener_scope); @@ -1296,6 +1297,25 @@ impl JsContextHost { ) } + fn lightweight_popup_initiator_storage_scope( + &mut self, + initiator: super::PendingWindowMessageEndpoint, + ) -> Option { + match initiator { + super::PendingWindowMessageEndpoint::TopWindow => Some( + LightweightPopupStorageScope::from_web_storage_scope(self.top_web_storage_scope()), + ), + super::PendingWindowMessageEndpoint::ChildWindow(handle) => { + let top_origin = origin_ascii_serialization(self.document_url()); + self.child_browsing_context_web_storage_scope(handle, &top_origin) + .map(LightweightPopupStorageScope::from_web_storage_scope) + } + super::PendingWindowMessageEndpoint::LightweightPopup(popup_id) => self + .lightweight_popup_bound_web_storage_scope(popup_id) + .map(LightweightPopupStorageScope::from_web_storage_scope), + } + } + fn lightweight_popup_response_storage_scope( &mut self, final_url: &Url, @@ -1509,15 +1529,24 @@ impl JsContextHost { } pub(crate) fn lightweight_popup_is_open(&self, popup_id: u64) -> bool { - self.lightweight_popup_record(popup_id) - .is_some_and(LightweightPopupBrowsingContextRecord::is_open) + self.browser_context_runtime + .lightweight_popup_id_is_live(popup_id) + && self + .lightweight_popup_record(popup_id) + .is_some_and(LightweightPopupBrowsingContextRecord::is_open) } pub(crate) fn open_lightweight_popup_ids(&self) -> Vec { let mut popup_ids = self .lightweight_popup_browsing_contexts .iter() - .filter_map(|(popup_id, record)| record.is_open().then_some(*popup_id)) + .filter_map(|(popup_id, record)| { + (record.is_open() + && self + .browser_context_runtime + .lightweight_popup_id_is_live(*popup_id)) + .then_some(*popup_id) + }) .collect::>(); popup_ids.sort_unstable(); popup_ids @@ -4405,12 +4434,6 @@ fn lightweight_popup_close_callback<'s>( let Some(transition) = host.close_lightweight_popup_browsing_context(popup_id) else { return; }; - set_object_slot( - scope, - window, - "closed", - v8::Boolean::new(scope, true).into(), - ); host.unregister_service_worker_popup_client(popup_id); host.cancel_lightweight_popup_document_loads(popup_id); host.cancel_lightweight_popup_classic_script_loads(popup_id); @@ -4423,6 +4446,32 @@ fn lightweight_popup_close_callback<'s>( host.retire_lightweight_popup_local_window(popup_id, transition.retired_local_window_id); } +fn lightweight_popup_closed_getter<'s>( + scope: &mut v8::PinScope<'s, '_>, + args: v8::FunctionCallbackArguments<'s>, + mut rv: v8::ReturnValue<'_, v8::Value>, +) { + let window = args.this(); + let Some(popup_id) = lightweight_popup_id_from_window(scope, window) else { + throw_type_error( + scope, + "Window.closed getter called on incompatible receiver.", + ); + return; + }; + let Some(host_ptr) = context_host_ptr_from_window_object(scope, window) else { + throw_type_error( + scope, + "Window.closed getter called on incompatible receiver.", + ); + return; + }; + let is_closed = !unsafe { &*host_ptr } + .browser_context_runtime + .lightweight_popup_id_is_live(popup_id); + rv.set(v8::Boolean::new(scope, is_closed).into()); +} + fn lightweight_popup_initiator_endpoint<'s>( scope: &mut v8::PinScope<'s, '_>, opener: Option>, diff --git a/moli-renderer-v8/src/native_bridge/element.rs b/moli-renderer-v8/src/native_bridge/element.rs index 2121bd02a5..9c6761d440 100644 --- a/moli-renderer-v8/src/native_bridge/element.rs +++ b/moli-renderer-v8/src/native_bridge/element.rs @@ -190,8 +190,8 @@ use super::document::{ }; use activation::navigate_form_target_browsing_context; pub(crate) use activation::{ - SpecialBrowsingContextTarget, navigate_existing_browsing_context_target, - navigate_named_iframe_target, + SpecialBrowsingContextTarget, existing_browsing_context_target, + navigate_existing_browsing_context_target, navigate_named_iframe_target, }; pub(crate) use activation::{ activate_default_submit_button_via_keyboard, activate_handle_after_pointer_release, diff --git a/moli-renderer-v8/src/native_bridge/element/activation/mod.rs b/moli-renderer-v8/src/native_bridge/element/activation/mod.rs index 0219bdd595..41a34ce01b 100644 --- a/moli-renderer-v8/src/native_bridge/element/activation/mod.rs +++ b/moli-renderer-v8/src/native_bridge/element/activation/mod.rs @@ -15,8 +15,8 @@ pub(crate) use default_action::{ }; pub(in crate::native_bridge) use targets::named_iframe_target_handle_for_navigation; pub(crate) use targets::{ - SpecialBrowsingContextTarget, navigate_existing_browsing_context_target, - navigate_named_iframe_target, + SpecialBrowsingContextTarget, existing_browsing_context_target, + navigate_existing_browsing_context_target, navigate_named_iframe_target, }; pub(in crate::native_bridge::element) use targets::{ queue_deferred_named_iframe_target_navigation_from_document, diff --git a/moli-renderer-v8/src/native_bridge/element/activation/targets.rs b/moli-renderer-v8/src/native_bridge/element/activation/targets.rs index 7115d94d7d..fa6494db29 100644 --- a/moli-renderer-v8/src/native_bridge/element/activation/targets.rs +++ b/moli-renderer-v8/src/native_bridge/element/activation/targets.rs @@ -222,7 +222,7 @@ fn navigate_hyperlink_popup_target( let Some(opened_popup) = runtime.open_lightweight_popup_window( scope, runtime_ptr, - (!relations.suppress_opener).then_some( + Some( crate::native_bridge::PendingWindowMessageEndpoint::from_dispatch_scope(dispatch_scope), ), opener, @@ -340,24 +340,61 @@ fn browsing_context_dispatch_scope_for_node( .map(crate::native_bridge::OwnerDispatchScope::Child) } -fn navigate_special_target_from_window<'s>( +fn browsing_context_target_window_for_dispatch_scope<'s>( scope: &mut v8::PinScope<'s, '_>, runtime_ptr: *mut JsContextHost, - source_window: v8::Local<'s, v8::Object>, + dispatch_scope: crate::native_bridge::OwnerDispatchScope, + target: Option, +) -> Option> { + let global = scope.get_current_context().global(scope); + match (dispatch_scope, target) { + (_, Some(SpecialBrowsingContextTarget::Blank)) => None, + (crate::native_bridge::OwnerDispatchScope::Top, _) + | (crate::native_bridge::OwnerDispatchScope::LightweightPopup(_), None) + | ( + crate::native_bridge::OwnerDispatchScope::LightweightPopup(_), + Some( + SpecialBrowsingContextTarget::Current + | SpecialBrowsingContextTarget::Parent + | SpecialBrowsingContextTarget::Top, + ), + ) => browsing_context_window_for_dispatch_scope(scope, runtime_ptr, dispatch_scope), + (crate::native_bridge::OwnerDispatchScope::Child(handle), None) + | ( + crate::native_bridge::OwnerDispatchScope::Child(handle), + Some(SpecialBrowsingContextTarget::Current), + ) => unsafe { &*runtime_ptr }.existing_child_browsing_context_window_wrapper(scope, handle), + ( + crate::native_bridge::OwnerDispatchScope::Child(handle), + Some(SpecialBrowsingContextTarget::Top), + ) => Some( + unsafe { &mut *runtime_ptr }.child_browsing_context_root_window(scope, handle, global), + ), + ( + crate::native_bridge::OwnerDispatchScope::Child(handle), + Some(SpecialBrowsingContextTarget::Parent), + ) => { + let runtime = unsafe { &mut *runtime_ptr }; + let top = runtime.child_browsing_context_root_window(scope, handle, global); + Some(runtime.child_browsing_context_parent_window(scope, handle, top)) + } + } +} + +fn navigate_special_target_for_dispatch_scope<'s>( + scope: &mut v8::PinScope<'s, '_>, + runtime_ptr: *mut JsContextHost, + dispatch_scope: crate::native_bridge::OwnerDispatchScope, target: Option, resolved_url: &str, ) -> Option> { let global = scope.get_current_context().global(scope); - let target_window = match target { - None | Some(SpecialBrowsingContextTarget::Current) => source_window, - Some(SpecialBrowsingContextTarget::Top) => source_window - .get(scope, v8str(scope, "top").into()) - .and_then(|value| v8::Local::::try_from(value).ok())?, - Some(SpecialBrowsingContextTarget::Parent) => source_window - .get(scope, v8str(scope, "parent").into()) - .and_then(|value| v8::Local::::try_from(value).ok())?, - Some(SpecialBrowsingContextTarget::Blank) => return None, - }; + let target_window = browsing_context_target_window_for_dispatch_scope( + scope, + runtime_ptr, + dispatch_scope, + target, + )?; let navigated = if target_window.strict_equals(global.into()) { queue_top_level_location_navigation(scope, runtime_ptr, resolved_url) } else { @@ -378,17 +415,34 @@ pub(crate) fn navigate_existing_browsing_context_target<'s>( "a new-context target cannot use existing-context navigation" ); let dispatch_scope = unsafe { &*runtime_ptr }.entered_owner_dispatch_scope(scope); - let source_window = - browsing_context_window_for_dispatch_scope(scope, runtime_ptr, dispatch_scope)?; - navigate_special_target_from_window( + navigate_special_target_for_dispatch_scope( scope, runtime_ptr, - source_window, + dispatch_scope, Some(target), resolved_url, ) } +pub(crate) fn existing_browsing_context_target<'s>( + scope: &mut v8::PinScope<'s, '_>, + runtime_ptr: *mut JsContextHost, + target: SpecialBrowsingContextTarget, +) -> Option> { + assert_ne!( + target, + SpecialBrowsingContextTarget::Blank, + "a new-context target cannot select an existing browsing context" + ); + let dispatch_scope = unsafe { &*runtime_ptr }.entered_owner_dispatch_scope(scope); + browsing_context_target_window_for_dispatch_scope( + scope, + runtime_ptr, + dispatch_scope, + Some(target), + ) +} + pub(super) fn navigate_hyperlink_source_browsing_context( scope: &mut v8::PinScope<'_, '_>, runtime_ptr: *mut JsContextHost, @@ -405,15 +459,10 @@ pub(super) fn navigate_hyperlink_source_browsing_context( crate::native_bridge::OwnerDispatchScope::Child(handle) => unsafe { &mut *runtime_ptr } .navigate_child_browsing_context_to_url(scope, handle, resolved_url), crate::native_bridge::OwnerDispatchScope::LightweightPopup(popup_id) => { - let Some(source_window) = - unsafe { &*runtime_ptr }.lightweight_popup_window(scope, popup_id) - else { - return false; - }; - navigate_special_target_from_window( + navigate_special_target_for_dispatch_scope( scope, runtime_ptr, - source_window, + crate::native_bridge::OwnerDispatchScope::LightweightPopup(popup_id), Some(SpecialBrowsingContextTarget::Current), resolved_url, ) @@ -448,15 +497,10 @@ pub(crate) fn navigate_target_browsing_context<'s>( } None => { let dispatch_scope = unsafe { &*runtime_ptr }.entered_owner_dispatch_scope(scope); - let Some(source_window) = - browsing_context_window_for_dispatch_scope(scope, runtime_ptr, dispatch_scope) - else { - return false; - }; - navigate_special_target_from_window( + navigate_special_target_for_dispatch_scope( scope, runtime_ptr, - source_window, + dispatch_scope, None, resolved_url, ) @@ -517,18 +561,6 @@ pub(in crate::native_bridge) fn navigate_hyperlink_target_browsing_context<'s>( if let Some(target_name) = target_name && special_target.is_none() { - let relations = - hyperlink_popup_relations(unsafe { &*runtime_ptr }, source_handle, target_name); - if relations.suppress_opener { - return navigate_hyperlink_popup_target( - scope, - runtime_ptr, - source_handle, - target_name, - resolved_url, - popup_disposition, - ); - } if !target_name.is_empty() && let Some(dispatch_scope) = browsing_context_dispatch_scope_for_node(scope, runtime_ptr, source_handle) @@ -577,15 +609,10 @@ pub(in crate::native_bridge) fn navigate_hyperlink_target_browsing_context<'s>( else { return false; }; - let Some(source_window) = - browsing_context_window_for_dispatch_scope(scope, runtime_ptr, dispatch_scope) - else { - return false; - }; - navigate_special_target_from_window( + navigate_special_target_for_dispatch_scope( scope, runtime_ptr, - source_window, + dispatch_scope, special_target, resolved_url, ) diff --git a/moli-renderer-v8/src/runtime/browser_context_runtime.rs b/moli-renderer-v8/src/runtime/browser_context_runtime.rs index 5936044794..7d8f750f13 100644 --- a/moli-renderer-v8/src/runtime/browser_context_runtime.rs +++ b/moli-renderer-v8/src/runtime/browser_context_runtime.rs @@ -4,7 +4,7 @@ use std::sync::{ }; use std::{ cell::RefCell, - collections::HashMap, + collections::{HashMap, HashSet}, rc::{Rc, Weak}, }; @@ -207,6 +207,7 @@ struct RendererBrowserContextRuntimeInner { storage_partition_identity: RendererStoragePartitionIdentity, next_child_document_loader_id: AtomicU64, next_lightweight_popup_id: AtomicU64, + live_lightweight_popup_ids: Mutex>, next_detached_parser_script_fetch_id: AtomicU64, next_dedicated_worker_instance_id: AtomicU64, dedicated_worker_devtools_targets: Mutex>, @@ -378,6 +379,27 @@ impl RendererBrowserContextRuntime { id } + pub fn register_lightweight_popup_id(&self, popup_id: u64) { + self.inner + .live_lightweight_popup_ids + .lock() + .insert(popup_id); + } + + pub fn retire_lightweight_popup_ids(&self, popup_ids: &[u64]) { + let mut live_popup_ids = self.inner.live_lightweight_popup_ids.lock(); + for popup_id in popup_ids { + live_popup_ids.remove(popup_id); + } + } + + pub fn lightweight_popup_id_is_live(&self, popup_id: u64) -> bool { + self.inner + .live_lightweight_popup_ids + .lock() + .contains(&popup_id) + } + pub(crate) fn clipboard_snapshot(&self) -> ClipboardSnapshot { self.inner.clipboard_snapshot.lock().clone() } @@ -593,6 +615,7 @@ impl RendererBrowserContextRuntime { storage_partition_identity, next_child_document_loader_id: AtomicU64::default(), next_lightweight_popup_id: AtomicU64::new(1), + live_lightweight_popup_ids: Mutex::new(HashSet::new()), next_detached_parser_script_fetch_id: AtomicU64::default(), next_dedicated_worker_instance_id: AtomicU64::default(), dedicated_worker_devtools_targets: Mutex::new(HashMap::new()), @@ -1038,6 +1061,20 @@ mod tests { ); } + #[test] + fn lightweight_popup_retirement_is_shared_across_hosts() { + let browser_context = RendererBrowserContextRuntime::new(); + let first_host = browser_context.handle(); + let second_host = browser_context.handle(); + let popup_id = first_host.next_lightweight_popup_id(); + + assert!(!second_host.lightweight_popup_id_is_live(popup_id)); + first_host.register_lightweight_popup_id(popup_id); + assert!(second_host.lightweight_popup_id_is_live(popup_id)); + first_host.retire_lightweight_popup_ids(&[popup_id]); + assert!(!second_host.lightweight_popup_id_is_live(popup_id)); + } + async fn assert_single_page_reservation_release( output_rx: &mut crate::runtime::RendererOutputTransportReceiver, token: RendererPageReservationToken, diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/popup_window.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/popup_window.rs index 8caa577a67..8cd560bfc8 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/popup_window.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/misc/extracted/popup_window.rs @@ -620,11 +620,11 @@ fn window_open_does_not_reuse_named_noopener_popup_from_an_isolated_group() { } #[test] -fn window_open_noopener_does_not_reuse_a_related_named_popup() { +fn window_open_noopener_reuses_a_related_named_popup_but_returns_null() { let mut vm = new_storage_test_vm("https://example.com/"); let first = vm - .eval("open('about:blank#related', 'report').location.href") + .eval("globalThis.popup = open('about:blank#related', 'report'); popup.location.href") .expect("related named popup should evaluate"); assert_eq!(first, "about:blank#related"); let first_activation = vm.take_pending_popup_activations(); @@ -637,7 +637,12 @@ fn window_open_noopener_does_not_reuse_a_related_named_popup() { ); let second_activation = vm.take_pending_popup_activations(); assert_eq!(second_activation.len(), 1); - assert_ne!(second_activation[0].popup_id(), Some(first_popup_id)); + assert_eq!(second_activation[0].popup_id(), Some(first_popup_id)); + assert_eq!( + vm.eval("JSON.stringify([popup.location.href, popup.opener === window])") + .expect("reused popup state should evaluate"), + r#"["about:blank#isolated",true]"# + ); } #[test] @@ -666,6 +671,83 @@ fn window_open_empty_url_selects_named_source_without_navigation() { assert!(vm.take_pending_location_navigation_with_seed().is_none()); } +#[test] +fn window_open_empty_parent_uses_internal_identity_without_running_public_lookup() { + let mut vm = new_storage_test_vm("https://example.com/source"); + assert_eq!( + vm.eval( + r#" +const fake = { fake: true }; +let parentReads = 0; +Object.defineProperty(window, "parent", { + configurable: true, + get() { ++parentReads; return fake; } +}); +globalThis.marker = 9; +const selected = open("", "_parent"); +JSON.stringify([selected === window, selected === fake, location.href, marker, parentReads]); +"#, + ) + .expect("empty URL parent selection should evaluate"), + r#"[true,false,"https://example.com/source",9,0]"# + ); + assert!(vm.take_pending_location_navigation_with_seed().is_none()); +} + +#[tokio::test] +async fn child_window_open_empty_parent_and_top_use_internal_browsing_context_identity() { + let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).expect("loader"); + let mut vm = + new_storage_page_task_executor_test_vm_with_loader("https://example.com/source", &loader); + vm.eval( + r#" +globalThis.frame = document.createElement("iframe"); +frame.srcdoc = "

child

"; +(document.body || document.documentElement || document).appendChild(frame); +"#, + ) + .expect("child frame setup should evaluate"); + vm.drain_ready_page_task_executor_turns_for_setup(&loader, 128) + .await + .expect("child frame should commit"); + + assert_eq!( + vm.eval( + r#" +frame.contentWindow.eval(` + const realParent = window.parent; + const realTop = window.top; + const fake = {}; + let parentReads = 0; + let topReads = 0; + Object.defineProperty(window, "parent", { + configurable: true, + get() { ++parentReads; return fake; } + }); + Object.defineProperty(window, "top", { + configurable: true, + get() { ++topReads; return fake; } + }); + const selectedParent = open("", "_parent"); + const selectedTop = open("", "_top"); + JSON.stringify([ + selectedParent === realParent, + selectedTop === realTop, + selectedParent === fake, + selectedTop === fake, + parentReads, + topReads, + location.href + ]); +`) +"#, + ) + .expect("child special-target selection should evaluate"), + r#"[true,true,false,false,0,0,"about:srcdoc"]"# + ); + assert!(vm.take_pending_location_navigation_with_seed().is_none()); +} + #[test] fn window_open_empty_url_reuses_named_popup_without_navigation() { let mut vm = new_storage_test_vm("https://example.com/source"); @@ -715,6 +797,38 @@ fn window_open_prefers_the_same_named_source_over_a_related_popup() { assert_eq!(navigation.url.as_str(), "about:blank#self"); } +#[test] +fn window_open_noopener_prefers_the_same_named_source_and_returns_null() { + let mut vm = new_storage_test_vm("https://example.com/source"); + + assert_eq!( + vm.eval( + r#" +const popup = open("about:blank#popup", "shared"); +window.name = "shared"; +open("about:blank#self", "shared", "noopener") === null; +"#, + ) + .expect("noopener same-named source selection should evaluate"), + "true" + ); + let activations = vm.take_pending_popup_activations(); + assert_eq!( + activations.len(), + 1, + "the noopener call must navigate the source instead of reopening the popup" + ); + assert_eq!( + vm.eval("popup.location.href") + .expect("existing popup URL should evaluate"), + "about:blank#popup" + ); + let navigation = vm + .take_pending_location_navigation_with_seed() + .expect("noopener same-named source navigation"); + assert_eq!(navigation.url.as_str(), "about:blank#self"); +} + #[tokio::test] async fn window_open_named_lightweight_popup_reuse_pushes_history_and_back_traverses() { let loader = ResourceRequestClient::new(&moli_fetch::FetchConfig::default()).expect("loader"); diff --git a/moli-renderer-v8/src/script_vm/tests/browser_api/navigation/element_navigation.rs b/moli-renderer-v8/src/script_vm/tests/browser_api/navigation/element_navigation.rs index 652c64430f..7384f3b5e5 100644 --- a/moli-renderer-v8/src/script_vm/tests/browser_api/navigation/element_navigation.rs +++ b/moli-renderer-v8/src/script_vm/tests/browser_api/navigation/element_navigation.rs @@ -1,7 +1,7 @@ use super::*; #[test] -fn noopener_named_hyperlink_does_not_reuse_an_existing_iframe() { +fn noopener_named_hyperlink_reuses_an_existing_iframe() { let mut vm = new_storage_test_vm("https://example.com/page.html"); vm.eval( r#" @@ -25,16 +25,9 @@ link.click(); assert_eq!( vm.eval("frame.contentWindow.location.href") .expect("iframe URL should evaluate"), - "about:blank" + "about:blank#isolated" ); - let activations = vm.take_pending_popup_activations(); - assert_eq!(activations.len(), 1); - let crate::RendererPopupActivationSource::Window { exposes_opener, .. } = - activations[0].source() - else { - panic!("hyperlink must retain its exact Window source"); - }; - assert!(!exposes_opener); + assert!(vm.take_pending_popup_activations().is_empty()); } #[tokio::test]