Skip to content

fix(window): preserve native named-context identity #853

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
lanyue-llk wants to merge 1 commit into from
Open

fix(window): preserve native named-context identity #853

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
452 changes: 452 additions & 0 deletions moli-protocol-server/src/protocol_server/tests/classic/extracted/windows_and_popups.rs
View file
Open in desktop

Large diffs are not rendered by default.

11 changes: 10 additions & 1 deletion moli-protocol/src/domains/target/tests/test_support.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,6 +207,15 @@ pub(super) async fn load_bc_with_titled_page_async(
bc_id: &str,
target_id: &str,
html: &str,
) {
load_bc_with_page_url_async(ctx, bc_id, target_id, &format!("data:text/html,{html}")).await;
}

pub(super) async fn load_bc_with_page_url_async(
ctx: &mut TestContext,
bc_id: &str,
target_id: &str,
url: &str,
) {
// Most tests that use this helper assert Target-domain discovery output
// after manually seeding a loaded target. Real CDP clients only receive
Expand All @@ -219,7 +228,7 @@ pub(super) async fn load_bc_with_titled_page_async(
ctx.conn.insert_browser_context(bc);
let page = ctx
.conn
.load_page_via_runtime_async(&format!("data:text/html,{html}"))
.load_page_via_runtime_async(url)
.await
.expect("page should load");
let renderer_page = crate::conn::RendererPageResidenceIdentity::from_page(&page);
Expand Down
22 changes: 22 additions & 0 deletions moli-protocol/src/domains/target/tests/tests_background_staging/runtime.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,6 +575,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("reportWindow"),
Some(owner.target_id.as_str()),
"the popup proxy name and the live target engine must share one browsing-context owner"
);
})
.await;
}
Expand Down
149 changes: 138 additions & 11 deletions moli-protocol/src/domains/target/tests/tests_target_creation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1895,6 +1895,23 @@ async fn popup_initial_navigation_replaces_internal_blank_history_entry() {

#[tokio::test(flavor = "multi_thread")]
async fn window_open_named_target_reuses_existing_popup_target() {
// Rename access requires a same-origin WindowProxy. Separate HTTP paths
// retain full document navigations; data: documents have opaque origins.
let listener = TcpListener::bind("127.0.0.1:0").await.unwrap();
let address = listener.local_addr().unwrap();
let server = tokio::spawn(async move {
axum::serve(
listener,
Router::new()
.fallback(|| async { ([(CONTENT_TYPE, "text/html")], "<main>named popup</main>") }),
)
.await
.unwrap();
});
let opener_url = format!("http://{address}/opener");
let first_url = format!("http://{address}/first-popup");
let second_url = format!("http://{address}/second-popup");
let renamed_url = format!("http://{address}/renamed-popup");
let mut ctx = TestContext::new();
// Target creation/named-target selection is synchronous with
// `window.open()`, but fetching the selected target's URL is not. Mirror
Expand All @@ -1903,11 +1920,11 @@ async fn window_open_named_target_reuses_existing_popup_target() {
ctx.enable_background_navigation_scheduler_for_test();
tokio::task::LocalSet::new()
.run_until(async {
load_bc_with_titled_page_async(
load_bc_with_page_url_async(
&mut ctx,
"BID-popup-name",
"TID-opener-name",
"<main>popup opener</main>",
&opener_url,
)
.await;
ctx.process_async(json!({
Expand Down Expand Up @@ -1935,7 +1952,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": format!("window.__namedPopup = window.open({}, 'reportWindow'); window.__namedPopup !== null", json!(first_url))
}
}))
.await;
Expand All @@ -1951,21 +1968,20 @@ async fn window_open_named_target_reuses_existing_popup_target() {
.to_owned();
assert_eq!(
created["params"]["targetInfo"]["url"],
json!("data:text/html,first-popup")
json!(first_url)
);
assert!(
first_sent
.iter()
.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",
"sessionId": opener_session_id,
"params": {
"expression": "window.open('data:text/html,second-popup', 'reportWindow') !== null"
"expression": format!("window.open({}, 'reportWindow') !== null", json!(second_url))
}
}))
.await;
Expand Down Expand Up @@ -1994,7 +2010,7 @@ async fn window_open_named_target_reuses_existing_popup_target() {
&& message["params"]["targetInfo"]["targetId"]
== json!(target_id)
&& message["params"]["targetInfo"]["url"]
== json!("data:text/html,second-popup")
== json!(second_url)
},
)
.await;
Expand All @@ -2004,15 +2020,15 @@ async fn window_open_named_target_reuses_existing_popup_target() {
);
assert_eq!(
changed["params"]["targetInfo"]["url"],
json!("data:text/html,second-popup")
json!(second_url)
);
ctx.wait_until_scheduler_state("reused named popup remains selected", |conn| {
conn.browser_context_by_id("BID-popup-name")
.is_some_and(|browser_context| {
browser_context.active_target_id() == Some(target_id.as_str())
&& loaded_page_for_target(browser_context, &target_id).is_some_and(
|page| {
page.final_url().as_str() == "data:text/html,second-popup"
page.final_url().as_str() == second_url
},
)
})
Expand All @@ -2022,12 +2038,123 @@ async fn window_open_named_target_reuses_existing_popup_target() {
assert_eq!(browser_context.active_target_id(), Some(target_id.as_str()));
assert_eq!(
browser_context.target_url(),
"data:text/html,second-popup"
second_url
);
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.__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": format!("window.open({}, 'renamedWindow') !== null", json!(renamed_url))
}
}))
.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!(renamed_url)
})
.await;
let browser_context = ctx.conn.browser_context.as_ref().unwrap();
assert_eq!(
browser_context.target_id_for_window_name("renamedWindow"),
Some(target_id.as_str())
);
assert_eq!(
browser_context.target_id_for_window_name("reportWindow"),
None,
"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', 'renamedWindow') !== null"
}
}))
.await;
let old_name_sent = ctx.take_all();
assert!(
old_name_sent
.iter()
.any(|message| message["method"] == json!("Target.targetCreated")),
"the target's previous name must no longer resolve to it: {old_name_sent:?}"
);
})
.await;
server.abort();
}

#[tokio::test(flavor = "multi_thread")]
async fn window_open_named_target_reused_in_same_command_emits_one_page_event() {
let mut ctx = TestContext::new();
Expand Down
2 changes: 1 addition & 1 deletion moli-renderer-v8/src/context_bootstrap.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ pub(crate) use window_accessors::{
mod window_events;
mod window_lazy_surface;
mod window_receiver;
pub(crate) use window_receiver::is_window_receiver;
pub(crate) use window_receiver::{NATIVE_WINDOW_BRAND_SLOT, is_window_receiver};
mod window_runtime;

pub(crate) use form_data_runtime::{
Expand Down
84 changes: 62 additions & 22 deletions moli-renderer-v8/src/context_bootstrap/runtime_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -871,26 +871,59 @@ fn window_surface_replaceable_setter<'s>(
define_replaceable_window_property(scope, receiver, name, args.get(0));
}

fn window_name_receiver_owner<'s>(
scope: &mut v8::PinScope<'s, '_>,
receiver: v8::Local<'s, v8::Object>,
) -> Option<(*mut JsContextHost, native_bridge::OwnerDispatchScope)> {
if !super::is_window_receiver(scope, receiver) {
return None;
}
let host_ptr = crate::util::context_host_ptr_from_window_object(scope, receiver)
.or_else(|| {
receiver
.get_creation_context(scope)
.and_then(crate::util::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()
})?;
let owner = super::navigation_window::runtime_window_dispatch_scope(scope, receiver)
.unwrap_or(native_bridge::OwnerDispatchScope::Top);
Some((host_ptr, owner))
}

fn window_name_runtime_getter<'s>(
scope: &mut v8::PinScope<'s, '_>,
args: v8::FunctionCallbackArguments<'s>,
mut rv: v8::ReturnValue<'s, v8::Value>,
) {
let receiver = callback_this_object(scope, &args);
if child_context_handle_from_owner(scope, receiver).is_none()
&& let Some(host_ptr) = context_host_ptr_from_global_bridge(scope)
{
let host = unsafe { &*host_ptr };
let name = if host.browsing_context_is_closed() {
String::new()
} else {
host.browsing_context_name().get()
};
rv.set(v8::String::new(scope, &name).unwrap().into());
let Some((host_ptr, owner)) = window_name_receiver_owner(scope, receiver) else {
throw_type_error(scope, "Window.name getter called on incompatible receiver.");
return;
};
let host = unsafe { &*host_ptr };
let value = match owner {
native_bridge::OwnerDispatchScope::Top => {
let name = if host.browsing_context_is_closed() {
String::new()
} else {
host.browsing_context_name().get()
};
v8_string(scope, &name).map(v8::Local::<v8::Value>::from)
}
native_bridge::OwnerDispatchScope::Child(_) => {
object_hidden_value(scope, receiver, WINDOW_NAME_SLOT)
}
native_bridge::OwnerDispatchScope::LightweightPopup(popup_id) => host
.lightweight_popup_window_name(popup_id)
.and_then(|name| v8_string(scope, &name))
.map(v8::Local::<v8::Value>::from),
}
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);
}

Expand All @@ -900,21 +933,28 @@ fn window_name_runtime_setter<'s>(
_rv: v8::ReturnValue<'s, v8::Value>,
) {
let receiver = callback_this_object(scope, &args);
let next = args
.get(0)
.to_string(scope)
.map(|value| value.to_rust_string_lossy(scope))
.unwrap_or_default();
if let Some(host_ptr) = context_host_ptr_from_global_bridge(scope) {
let host = unsafe { &mut *host_ptr };
if let Some(handle) = child_context_handle_from_owner(scope, receiver) {
host.set_child_browsing_context_name(handle, next.clone());
} else {
let Some((host_ptr, owner)) = window_name_receiver_owner(scope, receiver) else {
throw_type_error(scope, "Window.name setter called on incompatible receiver.");
return;
};
let Some(next) = args.get(0).to_string(scope) else {
return;
};
let next = next.to_rust_string_lossy(scope);
let host = unsafe { &mut *host_ptr };
match owner {
native_bridge::OwnerDispatchScope::Top => {
if host.browsing_context_is_closed() {
return;
}
host.browsing_context_name().set(next.clone());
}
native_bridge::OwnerDispatchScope::Child(handle) => {
host.set_child_browsing_context_name(handle, next.clone())
}
native_bridge::OwnerDispatchScope::LightweightPopup(popup_id) => {
host.set_lightweight_popup_window_name(popup_id, &next)
}
}
define_non_enumerable_string_property(scope, receiver, WINDOW_NAME_SLOT, &next);
}
Expand Down
Loading
Loading