Repository navigation
fix(window): preserve native named-context identity - #853
lanyue-llk wants to merge 1 commit into
Conversation
Spider Bench A/B✅ All benchmark browser service runs completed; results are informational. Public HEAD: 184 / 240 rows; 38 / 48 sites produced rows; 10 unexpected empty sites. ✅ Deterministic fixture contract is clean. Common ancestor Public 48-site run · informational
Public-site content, timing, and memory are noisy. This single A/B run reports evidence only; site outcome counts explain missing rows without treating them as a deterministic regression. CPU and memory timelinesThese are bounded 40-point views of the same complete process-tree samples used by the HTML report. CPU uses 100% per occupied logical core; memory lines are RSS first and PSS second when complete PSS samples are available. Basexychart-beta
title "Base CPU"
x-axis "Elapsed seconds" [1.036, 1.537, 2.537, 3.537, 4.037, 5.539, 6.541, 8.043, 9.044, 10.545, 12.047, 13.049, 14.551, 16.054, 17.055, 18.556, 20.058, 21.06, 22.562, 24.064, 25.065, 26.568, 27.568, 29.07, 30.572, 31.574, 33.075, 34.578, 35.58, 37.082, 38.584, 39.585, 41.087, 41.588, 42.088, 43.59, 45.092, 46.093]
y-axis "CPU percent" 0 --> 200
line [21.99, 123.86, 49.97, 118.01, 117.9, 161.97, 153.89, 109.77, 187.89, 53.98, 77.94, 143.86, 11.99, 115.76, 111.93, 6, 1.99, 105.71, 91.86, 135.8, 107.81, 111.87, 141.88, 165.82, 127.79, 103.87, 89.9, 17.94, 1.99, 157.68, 81.88, 9.99, 15.98, 0, 91.91, 115.86, 77.92, 107.87]
xychart-beta
title "Base memory: RSS then PSS"
x-axis "Elapsed seconds" [0.036, 1.036, 1.537, 2.537, 3.537, 4.037, 5.539, 6.541, 8.043, 9.044, 10.545, 12.047, 13.049, 14.551, 16.054, 17.055, 18.556, 20.058, 21.06, 22.562, 24.064, 25.065, 26.568, 27.568, 29.07, 30.572, 31.574, 33.075, 34.578, 35.58, 37.082, 38.584, 39.585, 41.087, 41.588, 42.088, 43.59, 45.092, 46.093, 47.542]
y-axis "Memory MiB" 0 --> 500
line [12.3, 83.6, 114.33, 121.8, 119.92, 142.38, 146.48, 171.93, 165.08, 188.56, 150.48, 175.08, 188.74, 147.14, 155.08, 172.63, 144.88, 146.2, 165.77, 201.88, 180.24, 293.3, 233.98, 185.77, 167.84, 194.1, 171.94, 155.55, 159.03, 153.11, 203.31, 184.45, 174.85, 161.75, 161.7, 170.13, 191.82, 170.03, 188.13, 0]
line [9.21, 80.23, 110.92, 118.38, 116.56, 138.59, 143.07, 168.51, 161.6, 185.26, 147.01, 171.6, 185.26, 143.67, 151.63, 169.16, 141.49, 142.72, 162.33, 198.46, 176.79, 289.54, 230.54, 182.29, 164.37, 190.64, 168.28, 152.24, 156.46, 149.63, 200.16, 180.99, 171.38, 158.28, 158.22, 166.65, 188.47, 166.55, 184.65, 0]
HEADxychart-beta
title "HEAD CPU"
x-axis "Elapsed seconds" [1.036, 1.537, 2.537, 4.039, 5.041, 6.543, 8.044, 9.045, 10.547, 11.549, 13.051, 14.051, 15.554, 17.056, 18.058, 19.559, 20.561, 22.063, 23.565, 24.566, 25.567, 26.068, 27.069, 28.57, 29.571, 31.074, 32.576, 33.577, 35.079, 36.081, 37.582, 39.083, 40.084, 41.585, 42.586, 43.086, 44.088, 45.088]
y-axis "CPU percent" 0 --> 200
line [2, 113.77, 110.06, 111.86, 163.72, 175.91, 107.97, 171.86, 105.75, 143.83, 105.98, 2, 3.99, 45.92, 67.86, 2, 71.85, 85.82, 153.58, 133.88, 107.85, 121.84, 143.79, 159.87, 149.95, 91.8, 132.01, 101.91, 121.89, 7.99, 131.89, 71.93, 5.99, 125.89, 117.96, 187.74, 33.97, 119.92]
xychart-beta
title "HEAD memory: RSS then PSS"
x-axis "Elapsed seconds" [0.035, 1.036, 1.537, 2.537, 4.039, 5.041, 6.543, 8.044, 9.045, 10.547, 11.549, 13.051, 14.051, 15.554, 17.056, 18.058, 19.559, 20.561, 22.063, 23.565, 24.566, 25.567, 26.068, 27.069, 28.57, 29.571, 31.074, 32.576, 33.577, 35.079, 36.081, 37.582, 39.083, 40.084, 41.585, 42.586, 43.086, 44.088, 45.088, 46.358]
y-axis "Memory MiB" 0 --> 500
line [12.21, 62.1, 109.41, 127.5, 130.29, 159.51, 171.17, 185.39, 174.37, 185.85, 191.66, 165.71, 147.03, 147.02, 156.26, 157.4, 160.33, 154.24, 177.12, 170.65, 182.49, 284.26, 203.18, 204.45, 223.92, 181.93, 165.02, 166.66, 150.21, 160.42, 153.68, 215.63, 184.91, 171.63, 172.11, 177.7, 198.18, 155.63, 184.41, 0]
line [9.08, 58.73, 106.08, 124.11, 127.11, 155.78, 167.7, 181.82, 171.01, 182.34, 188.19, 162.57, 143.55, 143.54, 152.78, 153.92, 156.84, 150.74, 173.64, 166.69, 179.02, 280.8, 199.78, 200.98, 220.38, 178.44, 161.84, 163.29, 146.62, 156.97, 150.2, 211.37, 181.43, 168.15, 168.64, 174.22, 193.9, 151.91, 180.96, 0]
Deterministic fixture · required contractThe fixture exercises 15 routes: 8 are expected to emit rows and 7 intentionally exercise empty, loading, or timeout behavior.
Full HTML, JSON, CSV, logs, and page snapshots: workflow run and full The public-web A/B is informational. The exact HEAD fixture contract runs as a separate required CI check; timing and public-site content deltas remain non-blocking. |
CI Regression ReportSource CI run · source state at render:
Release regression — ✅ HEAD/base failures 0/0; raw binary +0.003021%Package and image size
Startup latency and PSS
Concurrency matrix
Frontend differential — ✅ 1,020/1,020 cases matched; 0 issues
Agent episodes · Moli vs Chromium — ✅ 8/8 Moli episodes passed; 0 failures
Runtime and CDP session contracts — ✅ 26 contract cases; 0 failures
CDP smoke — ✅ 53/53 groups passed; 622 scenariosWorkers: 4 · cumulative group time: 206.37 s · failed groups: 0
WebMainBench · 545 pages — ✅ 545/545 completed; 0 unexpected failures
Frozen HTML · Linux · Diagnostics: All artifact fields are parsed by the trusted default-branch renderer; missing or invalid inputs remain visible as unavailable. |
Sequential Navigation Soak A/B✅ HEAD completed the 200-navigation resilience and memory observation. One browser process, one target, and one CDP session navigate CSDN → SegmentFault → Huaban → example.com repeatedly. This run issued 200 Common ancestor Session resilience
A public-page failure is reported separately from an unrecoverable session. The soak requires all navigation attempts, zero failed recovery, zero lifecycle/network ordering violations, and complete resource evidence. Process-tree memory
HEAD memory by 50-navigation quarter
Raw reports and per-navigation boundary samples: workflow run and full Public-site timing and memory are observational. Use the A/B deltas and trend shape as evidence, not as a deterministic performance threshold. |
lanyue-llk
changed the title
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
审查结论：需要修改后再验收。本次使用 3 个子 agent 分别深入检查布局发布、窗口状态生命周期和目标路由，并以本地 Rust 测试及 Chrome 对照复核结论。
审查版本：HEAD 266b076af7e3fecb381e6d8913b0ac466595ce35；实际 PR 基线 66a74a67542140f0679a001bf22a5187d37bd850（#731），共 31 个变更文件。最新 main 已同步到 80660fb6d。未把堆叠 PR 的上游变更算成本 PR 引入的问题。
按需求评估
| 分类 | 原问题与方案判断 | 处理建议 |
|---|---|---|
| 无截图获取真实布局尺寸 | 该前提在当前基线上已解决。 普通 metrics 冷读会生成无 paint 的快照，暖读复用，DOM 变化后重建。本 PR 新测试却反向要求冷读报错，实际失败。 | 对顶层尺寸需求复用已有接口；如保留显式发布，需要明确其额外的强制刷新/递归 iframe 几何语义，并用对应场景证明必要性，不能以旧分支行为为依据。 |
| 同一顶层窗口跨文档保存 name | 将状态由 Document 移到 browsing context 的方向合理；原有跨源导航、父子窗口名称与新窗口初始名称隔离用例本次通过。 | PR 应区分同源、跨端口同站、跨站及 history 场景，说明兼容目标；当前描述仅写跨源，边界不清。 |
| 命名 popup 改名后复用、旧名失效 | 读取 live name 的目标合理；扫描整个 BrowserContext 的实现越过了独立窗口边界。真实回归已复现。 | 查询必须携带来源及可导航目标范围，不能只按字符串全局匹配。见 P1 行内意见。 |
| 无 paint 发布与资源约束 | 复用既有布局/嵌套 frame 组合链路是合理方案；但遗漏 Mock policy 检查，造成真实布局成本与模拟返回值混用。 | 在统一 renderer 入口遵守 layout policy，不要在各协议适配层分别打补丁。见 P2 行内意见。 |
代码与测试问题
已提交 4 条行内意见：独立标签页被错误命名导航、Mock 模式越界布局、非法 Window receiver 的错误测试预期、过时的普通 metrics 报错断言。
另外，既有 window_open_target_registry_preserves_named_target_bytes 也在此 HEAD 失败：fixture 登记了尚不存在的 TID-spaced / TID-exact，新增存活目标检查返回 None。这里应先创建真实 target 再检查名称空白/大小写的精确匹配；不应为让 fixture 通过而撤销存活目标检查。这属于必须修正的测试适配问题，不是额外的产品缺陷。
实测证据
- PR 原有两个 WebDriver Classic 正向测试：2/2 通过；补充独立 tab 场景后，该回归测试失败，窗口数实际 2、预期 3。Chrome 154.0.8037.57 对照为 3，原标签页未被导航。
- Renderer：原有 OnDemand publication 测试通过；新增 Mock 场景失败，真实 pass 0 → 1，2300×1500 内容返回 1920×1080 模拟尺寸。
- 新增普通 metrics 对照测试通过：冷读 1 次布局且 paint=0，暖读不新增 pass，DOM 变化后重新布局。这直接验证当前基线已支持原先宣称需要新增接口才能完成的顶层尺寸需求。
- Protocol：原始 HEAD 相关 8 项测试 6 通过、2 失败，失败就是上面的过时断言与不存在 target 的 fixture。仅临时修正这两处测试前提、不改生产逻辑后，8/8 通过，也到达并通过了显式 publication 的后续断言。临时编辑已还原。
- Chrome 的 Window name accessor 对普通对象调用会抛
TypeError: Illegal invocation；本 PR 新增的“成功存取 receiver-only-name”断言与此相反，不能作为健壮性的证据。
PR 当前 Verification 主要是通过数和概括性描述，没有按每个原问题提供 base 红 / HEAD 绿的对应证据；当前提交的两项仓库内测试失败也说明旧验证记录不能替代 retarget 后复测。本次是针对性审查，未将局部通过表述为全 workspace 通过。
描述建议按上面四类补齐“触发条件 → 原行为 → 预期行为 → 所属状态/生命周期 → 对应回归用例”。尤其需要先重写布局需求前提，再决定是否保留新增协议参数。Moli 面向智能体、多页面自动化和按需资源消费，本次关键验收点应是目标隔离、单一状态来源、默认模式不意外布局，以及可泛化的行为测试。
可复测补丁：独立窗口隔离、Mock 模式、普通 metrics 冷/暖/失效行为
在上述 HEAD 应用以下补丁后运行：
cargo nextest run -p moli-protocol-server -E 'test(review_853_)' --no-fail-fast
cargo nextest run -p moli-renderer-v8 -E 'test(review_853_)' --no-fail-fast预期：独立窗口隔离和 Mock 两项红测，普通 metrics 对照绿测。
--- a/moli-protocol-server/src/protocol_server/tests/classic.rs
+++ b/moli-protocol-server/src/protocol_server/tests/classic.rs
@@ -17944,3 +17944,92 @@
"non-Classic router errors must not be relabeled as JSON: {content_type}"
);
}
+
+#[tokio::test]
+async fn review_853_named_target_does_not_capture_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().unwrap();
+ let window_path = format!("/session/{session_id}/window");
+ let url_path = format!("/session/{session_id}/url");
+ let execute_path = format!("/session/{session_id}/execute/sync");
+ let victim_url = classic_data_url("<!doctype html><title>independent victim</title>");
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &url_path,
+ json!({"url": victim_url})
+ )
+ .await,
+ json!({"value": null})
+ );
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &execute_path,
+ json!({"script": "window.name = 'independent-report'; return window.name;", "args": []})
+ )
+ .await,
+ json!({"value": "independent-report"})
+ );
+ let new_window = classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{session_id}/window/new"),
+ json!({"type": "tab"}),
+ )
+ .await;
+ let new_handle = new_window["value"]["handle"].as_str().unwrap();
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &window_path,
+ json!({"handle": new_handle})
+ )
+ .await,
+ json!({"value": null})
+ );
+ let source_url = classic_data_url(
+ "<!doctype html><button id='open' onclick=\"window.open('about:blank', 'independent-report')\">open</button>",
+ );
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &url_path,
+ json!({"url": source_url})
+ )
+ .await,
+ json!({"value": null})
+ );
+ let button = classic_find_css_element_id(app.clone(), session_id, "#open").await;
+ assert_eq!(
+ classic_request_json(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{session_id}/element/{button}/click")
+ )
+ .await,
+ json!({"value": null})
+ );
+ let handles = classic_request_json(
+ app.clone(),
+ Method::GET,
+ &format!("/session/{session_id}/window/handles"),
+ )
+ .await;
+ assert_eq!(
+ handles["value"].as_array().unwrap().len(),
+ 3,
+ "a named popup must not navigate an unrelated tab with the same window.name: {handles}"
+ );
+ let _ = classic_request_json(
+ app.clone(),
+ Method::DELETE,
+ &format!("/session/{session_id}"),
+ )
+ .await;
+}
--- a/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update.rs
+++ b/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update.rs
@@ -6185,3 +6185,56 @@
.await
.expect("rendering-update post-checkpoint child synchronization test should run");
}
+
+#[tokio::test(flavor = "current_thread")]
+async fn review_853_mock_publication_does_not_run_real_layout() {
+ 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/mock-publication.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("document.body.innerHTML = '<div style=\"width:2300px;height:1500px\"></div>'")?;
+ let before = page_vm.vm().layout_pass_observability_for_test().1;
+ let result = page_vm.publish_layout_metrics();
+ let after = page_vm.vm().layout_pass_observability_for_test().1;
+ eprintln!("review853 Mock publication before={before} after={after} result={result:?}");
+ assert_eq!(after, before, "Mock policy must not run real layout");
+ Ok::<(), anyhow::Error>(())
+ })
+ .await
+ .expect("mock publication regression");
+}
+
+#[tokio::test(flavor = "current_thread")]
+async fn review_853_ordinary_metrics_already_publish_without_paint() {
+ run_page_vm_async_test(async move {
+ let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default()).expect("loader");
+ let mut page = test_page_vm_with_loader_and_document_url(&loader, Vec::new(), Url::parse("https://example.com/ordinary-publication.html")?);
+ page.vm_mut().eval("document.body.innerHTML = '<div id=target style=\"width:2300px;height:1500px\"></div>'")?;
+ let before = page.vm().layout_pass_observability_for_test().1;
+ let first = page.layout_metrics()?;
+ let after = page.vm().layout_pass_observability_for_test();
+ assert_eq!(after.1, before + 1);
+ assert!(first.content_width >= 2300.0 && first.content_height >= 1500.0);
+ assert_eq!(after.3.unwrap().paint_operation_count, 0);
+ assert_eq!(page.layout_metrics()?, first);
+ assert_eq!(page.vm().layout_pass_observability_for_test().1, after.1);
+ page.vm_mut().eval("document.getElementById('target').style.width = '2600px'")?;
+ let changed = page.layout_metrics()?;
+ assert!(changed.content_width >= 2600.0);
+ let after_mutation = page.vm().layout_pass_observability_for_test();
+ assert_eq!(after_mutation.1, after.1 + 1);
+ assert_eq!(after_mutation.3.unwrap().paint_operation_count, 0);
+ eprintln!("review853 ordinary metrics: cold={first:?}, changed={changed:?}, passes={before}/{} /{}; paint=0", after.1, after_mutation.1);
+ Ok::<(), anyhow::Error>(())
+ }).await.expect("ordinary metrics already publish");
+}仅修正两处测试前提的绿测对照补丁（未改生产逻辑）
diff --git a/moli-protocol/src/conn/page_state/page_targets.rs b/moli-protocol/src/conn/page_state/page_targets.rs
index 154a6b8fe..6cb045169 100644
--- a/moli-protocol/src/conn/page_state/page_targets.rs
+++ b/moli-protocol/src/conn/page_state/page_targets.rs
@@ -1373,6 +1373,11 @@ 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!(
diff --git a/moli-protocol/src/domains/page/tests/capture.rs b/moli-protocol/src/domains/page/tests/capture.rs
index 7642828e4..123d181d5 100644
--- a/moli-protocol/src/domains/page/tests/capture.rs
+++ b/moli-protocol/src/domains/page/tests/capture.rs
@@ -1135,11 +1135,9 @@ async fn get_layout_metrics_can_explicitly_publish_without_a_screenshot() {
}))
.await;
let missing = take_response_by_id(&mut ctx, 126);
- assert!(
- missing["error"]["message"]
- .as_str()
- .is_some_and(|message| message.contains("no published layout")),
- "ordinary Page.getLayoutMetrics must remain a side-effect-free read: {missing:?}"
+ assert_eq!(
+ missing["result"]["contentSize"],
+ json!({ "x": 0, "y": 0, "width": 2300.0, "height": 1500.0 })
);
ctx.process_async(json!({cargo nextest run -p moli-protocol -E 'test(get_layout_metrics_) | test(window_open_target_registry_preserves_named_target_bytes)' --no-fail-fast原 HEAD：6/8；仅应用此补丁：8/8。
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
复审结论：新版仍需修改；建议收敛为下面三项根因修复，不按每个失败场景继续添加特判。
审查 HEAD 4cb4eef7cc96ee4bda57fc10850a235f82138955，实际基线仍为 #731 的 66a74a67542140f0679a001bf22a5187d37bd850。3 个子 agent 分别覆盖窗口路由/状态生命周期、Window 绑定契约、布局及实际消费者。检查了完整 PR 差异及新版相对 266b076af 的 4 个文件变更。
先确认已修部分：上轮错误的“普通 metrics 必须报未发布布局”断言、名称注册表中不存在 target 的 fixture 已修正；新增 staged→engine 名称迁移和嵌套 frame 冻结测试通过。独立窗口查找范围、Mock 模式和非法 receiver 的生产行为/错误预期仍未修改。PR 描述及 Verification 也没有更新为新版的需求边界和证据。
| PR 要解决的问题 | 当前方案 | 本次判断 |
|---|---|---|
| 无截图获得真实 metrics、保留 iframe 几何 | 增加 publishLayout，跨协议/命令/renderer 触发强制整树布局 |
原有按需接口已经能完成指标读取、子 frame 几何和实际点击；新增能力是提前构建完整树，不能继续表述为修复“否则无法取得几何”。 |
| 顶层 name 跨 Document 保留，新窗口独立 | NavigationEngine 持有共享 name state，传给每个 Document | 状态跨文档共享的方向正确；独立 engine 初始化、导航延续及 child-context 区分路径未发现需另列的问题。 |
| 改名后命名窗口复用、旧名失效 | 全 BrowserContext 扫描 engine 的 live name | 缺少 source/group 约束，且只覆盖窗口自己写 name，未覆盖 opener 持有的 popup 代理。需求没有闭环。 |
| 尚未安装 engine 的目标也保留名称 | 先存 HashMap，bind 时查找、复制、删除 | 新测试能通过，但仍把一个窗口状态分成“engine 安装前/后”两套存储；维护成本来自状态归属问题，应在同一目标身份上消除这个分叉。 |
1. [P1] 窗口身份、名称来源和查找范围需要一起修正。
位置：目标查找、新增迁移、popup 代理名称维护。
实际复现的三种表现属于同一个模型缺口：
- 独立 A 设置 name，独立 B 按该名称打开：HEAD 错误导航 A，只有 2 个窗口；Chrome 创建第三个。此项是上轮已经确认、至今未修的 PR 回归。
- A/B 同名，B 按自己的名称打开：HEAD 导航更早创建的 A，B 留在原页面；Chrome 导航 B。不能靠调整遍历顺序修正，查询必须有来源身份。
- opener 保存
popup = window.open(..., 'old')，执行popup.name = 'new'再按 new 打开：HEAD 产生第三窗口，Chrome 复用原 popup。lightweight_popup_window_names/代理 slot 与 engine state 仍是不同的名称来源。此能力缺口原本就存在，但 PR 声称同步 live names 后仍未覆盖它；这里将其纳入同一修复的验收，不冒充另一个新增回归。
建议的最小完整结构：
- 现有
PageTargetHost/稳定目标身份从创建起持有同一个名称 state，engine 和后续 Document 接入同一 handle；删除target_window_names及 bind 时的字符串迁移。无需新增一套通用注册中心。 - popup 代理与实际目标共享同一名称来源。现有
RendererPendingPopupActivation已能携带共享 storage handle，可以沿既有 owner/source 受约束的通道接入名称 state；不能用全局裸 popup ID 转发。 - 复用既有目标时，代理必须绑定那个既有 owner/state，不能把新代理的状态覆盖给目标。 现有
remember_resolved_popup_target只记 ID，尚没有这种 state 回绑；只向前多传一个 Arc 不能算完整修复。 - 解析入口带 source owner，先当前目标，再同一 browsing-context group 中允许的目标。BrowserContext 是 profile/storage 域，不是这个选择范围。group identity 应是现有目标上的稳定字段，独立窗口分配独立身份、关联 popup 继承；不要临时沿 opener 边猜 group，因为关闭 opener 的清理会删除这些边。
这样消除的是多份状态和错误选择范围，不是为独立 tab、同名 tab、代理改名各加一个 if。
2. [P2] Window.name getter/setter 应满足统一的 receiver 与转换契约。
位置：runtime_state.rs、仍然错误的 receiver 断言。
新版继续使用 to_string(...).unwrap_or_default()，转换失败仍向 name owner 写空串。两项实测红测：
name='kept'后赋值一个toString抛错的对象，结果为[false, ""]，预期原异常按预期传播且名称保持kept。toString内先成功写inner再抛错，最终名称为""，预期inner。
非法 receiver 的原用例仍要求普通对象能成功存取 name；Chrome 会在转换参数之前抛 TypeError。这些应作为一个绑定契约修正，不要按 Symbol、抛错对象、重入对象分别特判。
最小正确顺序是：验证真实 Window 并解析对应 owner → DOMString 转换 → 失败直接返回 → 成功后写入该 owner。 不能在失败后“回滚到转换前的名字”，那会抹去重入转换中已经成功完成的内层写入。临时仅将当前转换改为失败立即返回，两项转换测试已从 2 红变为 2 绿；实验代码已还原。非法 receiver 校验仍须一并完成，不能把这个小补丁当完整修复。
3. [P2] 按当前需求，建议删除 publishLayout 的全栈扩展，复用已有按需布局入口。
位置：显式发布入口。新增嵌套测试证明它会构建完整私有树，但没有证明用户操作需要这条新协议路径。
本次补充了真实消费者测试：普通 metrics → 孙 frame 命中 → child JS 几何读取 → 子文档样式变化 → 真正 mousedown/mouseup 触发 onclick。不使用 publishLayout 也全部通过；现有 HitTest 会发现 embedded_frames_complete 不满足并按需升级，暖读复用，变更后自动重建，paint 为 0。显式路径也通过，并在该 fixture 中省去一次后续几何补齐：普通路径 pass 为 1→2→3，显式路径为 1→1→2。这是提前构建带来的差别，不是正确性缺失。
与之同时，Mock 缺陷仍复现：2300×1500 内容触发真实 pass 0→1，最后返回 1920×1080 的模拟尺寸。按当前写明的功能需求，移除新增参数及跨层命令分支，保留按需布局消费者回归测试，是更小且完整的方案，也避免继续补这条无必要路径的模式特判。若未来确实需要独立的整树预热能力，应有明确调用方、资源契约及收益证据；目前仓库内 publishLayout 的调用只有测试，PR 未提供这样的需求依据。
验证结果与收敛验收：
- 完整运行 renderer、protocol、protocol-server 三套测试：仓库原有用例 12,381 通过、4 跳过（renderer 8,332；protocol 3,559；server 490），当前 HEAD 的 GitHub CI 也全部通过。
- 最终按 Chrome 校准预期的额外场景矩阵：8 项，6 红、2 绿。三项窗口场景、两项转换场景和 Mock 场景为红；普通布局完成嵌套交互、已有命名 popup 的 noopener 行为为绿。
noopener的最初假设已被 Chrome 实测排除，不计为缺陷；不要为了迎合这一错误预期去改实现。- 名称转换的最小纠正实验：2/2 绿。没有把临时测试或实验生产修改提交到 PR。
后续验收应使用同一矩阵，覆盖创建前后同一 state、双方改名读回、已有目标复用、同名 source 优先、独立目标隔离、关闭 opener 后关联目标仍保持身份、转换异常/重入，以及普通按需布局的冷/暖/失效与嵌套输入。先完成这三项根因修复，再更新 PR 标题/描述与实际验证记录；不应只继续修正断言或增加同步分支。
最终 8 项复现场景补丁及运行命令
应用到本审查 HEAD：
cargo nextest run -p moli-protocol -p moli-protocol-server -p moli-renderer-v8 -E 'test(/review_?853/)' --no-fail-fast --success-output immediate当前结果：6 failed / 2 passed。
--- a/moli-protocol-server/src/protocol_server/tests/classic.rs
+++ b/moli-protocol-server/src/protocol_server/tests/classic.rs
@@ -17944,3 +17944,330 @@
"non-Classic router errors must not be relabeled as JSON: {content_type}"
);
}
+
+#[tokio::test]
+async fn review_853_named_target_does_not_capture_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().unwrap();
+ let window_path = format!("/session/{session_id}/window");
+ let url_path = format!("/session/{session_id}/url");
+ let execute_path = format!("/session/{session_id}/execute/sync");
+ let victim_url = classic_data_url("<!doctype html><title>independent victim</title>");
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &url_path,
+ json!({"url": victim_url})
+ )
+ .await,
+ json!({"value": null})
+ );
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &execute_path,
+ json!({"script": "window.name = 'independent-report'; return window.name;", "args": []})
+ )
+ .await,
+ json!({"value": "independent-report"})
+ );
+ let new_window = classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{session_id}/window/new"),
+ json!({"type": "tab"}),
+ )
+ .await;
+ let new_handle = new_window["value"]["handle"].as_str().unwrap();
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &window_path,
+ json!({"handle": new_handle})
+ )
+ .await,
+ json!({"value": null})
+ );
+ let source_url = classic_data_url(
+ "<!doctype html><button id='open' onclick=\"window.open('about:blank', 'independent-report')\">open</button>",
+ );
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &url_path,
+ json!({"url": source_url})
+ )
+ .await,
+ json!({"value": null})
+ );
+ let button = classic_find_css_element_id(app.clone(), session_id, "#open").await;
+ assert_eq!(
+ classic_request_json(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{session_id}/element/{button}/click")
+ )
+ .await,
+ json!({"value": null})
+ );
+ let handles = classic_request_json(
+ app.clone(),
+ Method::GET,
+ &format!("/session/{session_id}/window/handles"),
+ )
+ .await;
+ assert_eq!(
+ handles["value"].as_array().unwrap().len(),
+ 3,
+ "a named popup must not navigate an unrelated tab with the same window.name: {handles}"
+ );
+ let _ = classic_request_json(
+ app.clone(),
+ Method::DELETE,
+ &format!("/session/{session_id}"),
+ )
+ .await;
+}
+
+#[tokio::test]
+async fn review_853_noopener_matches_chrome_named_popup_reuse() {
+ let app = build_router(test_state());
+ let session = classic_request_json(app.clone(), Method::POST, "/session").await;
+ let sid = session["value"]["sessionId"].as_str().unwrap();
+ let original =
+ classic_request_json(app.clone(), Method::GET, &format!("/session/{sid}/window")).await;
+ let page = classic_data_url(
+ "<!doctype html><button id='first' onclick=\"window.open('about:blank', 'report')\">first</button><button id='open' onclick=\"window.open('about:blank', 'report', 'noopener')\">open</button>",
+ );
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/url"),
+ json!({"url":page})
+ )
+ .await,
+ json!({"value":null})
+ );
+ let first = classic_find_css_element_id(app.clone(), sid, "#first").await;
+ assert_eq!(
+ classic_request_json(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/element/{first}/click")
+ )
+ .await,
+ json!({"value":null})
+ );
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/window"),
+ json!({"handle":original["value"]})
+ )
+ .await,
+ json!({"value":null})
+ );
+ let button = classic_find_css_element_id(app.clone(), sid, "#open").await;
+ assert_eq!(
+ classic_request_json(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/element/{button}/click")
+ )
+ .await,
+ json!({"value":null})
+ );
+ let handles = classic_request_json(
+ app.clone(),
+ Method::GET,
+ &format!("/session/{sid}/window/handles"),
+ )
+ .await;
+ let _ = classic_request_json(app.clone(), Method::DELETE, &format!("/session/{sid}")).await;
+ assert_eq!(
+ handles["value"].as_array().unwrap().len(),
+ 2,
+ "Chrome retains the existing named auxiliary context for this noopener call: {handles}"
+ );
+}
+
+#[tokio::test]
+async fn review_853_same_name_prefers_source_tab() {
+ let app = build_router(test_state());
+ let session = classic_request_json(app.clone(), Method::POST, "/session").await;
+ let sid = session["value"]["sessionId"].as_str().unwrap();
+ let first =
+ classic_request_json(app.clone(), Method::GET, &format!("/session/{sid}/window")).await;
+ let first_url = classic_data_url("<!doctype html><title>first tab</title>");
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/url"),
+ json!({"url":first_url})
+ )
+ .await,
+ json!({"value":null})
+ );
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/execute/sync"),
+ json!({"script":"window.name='shared'; return window.name;", "args":[]})
+ )
+ .await,
+ json!({"value":"shared"})
+ );
+ let second = classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/window/new"),
+ json!({"type":"tab"}),
+ )
+ .await;
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/window"),
+ json!({"handle":second["value"]["handle"]})
+ )
+ .await,
+ json!({"value":null})
+ );
+ let source = classic_data_url(
+ "<!doctype html><button id='open' onclick=\"window.open('about:blank','shared')\">open</button>",
+ );
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/url"),
+ json!({"url":source})
+ )
+ .await,
+ json!({"value":null})
+ );
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/execute/sync"),
+ json!({"script":"window.name='shared'; return window.name;", "args":[]})
+ )
+ .await,
+ json!({"value":"shared"})
+ );
+ let button = classic_find_css_element_id(app.clone(), sid, "#open").await;
+ assert_eq!(
+ classic_request_json(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/element/{button}/click")
+ )
+ .await,
+ json!({"value":null})
+ );
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/window"),
+ json!({"handle":first["value"]})
+ )
+ .await,
+ json!({"value":null})
+ );
+ let first_after =
+ classic_request_json(app.clone(), Method::GET, &format!("/session/{sid}/url")).await;
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/window"),
+ json!({"handle":second["value"]["handle"]})
+ )
+ .await,
+ json!({"value":null})
+ );
+ let second_after =
+ classic_request_json(app.clone(), Method::GET, &format!("/session/{sid}/url")).await;
+ let _ = classic_request_json(app.clone(), Method::DELETE, &format!("/session/{sid}")).await;
+ assert_eq!(
+ json!([first_after["value"], second_after["value"]]),
+ json!([first_url, "about:blank"]),
+ "window.open using the source's own name must prefer the source over an earlier same-name tab"
+ );
+}
+
+#[tokio::test]
+async fn review_853_popup_proxy_rename_routes_same_target() {
+ let app = build_router(test_state());
+ let session = classic_request_json(app.clone(), Method::POST, "/session").await;
+ let sid = session["value"]["sessionId"].as_str().unwrap();
+ let original =
+ classic_request_json(app.clone(), Method::GET, &format!("/session/{sid}/window")).await;
+ let page = classic_data_url(
+ "<!doctype html><button id='first' onclick=\"window.popup=window.open('about:blank','oldName')\">first</button><button id='rename' onclick=\"window.popup.name='newName'; window.open('about:blank','newName')\">rename and reuse</button>",
+ );
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/url"),
+ json!({"url":page})
+ )
+ .await,
+ json!({"value":null})
+ );
+ let first = classic_find_css_element_id(app.clone(), sid, "#first").await;
+ assert_eq!(
+ classic_request_json(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/element/{first}/click")
+ )
+ .await,
+ json!({"value":null})
+ );
+ assert_eq!(
+ classic_request_json_with_body(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/window"),
+ json!({"handle":original["value"]})
+ )
+ .await,
+ json!({"value":null})
+ );
+ let rename = classic_find_css_element_id(app.clone(), sid, "#rename").await;
+ assert_eq!(
+ classic_request_json(
+ app.clone(),
+ Method::POST,
+ &format!("/session/{sid}/element/{rename}/click")
+ )
+ .await,
+ json!({"value":null})
+ );
+ let handles = classic_request_json(
+ app.clone(),
+ Method::GET,
+ &format!("/session/{sid}/window/handles"),
+ )
+ .await;
+ let _ = classic_request_json(app.clone(), Method::DELETE, &format!("/session/{sid}")).await;
+ assert_eq!(
+ handles["value"].as_array().unwrap().len(),
+ 2,
+ "renaming the popup WindowProxy must update the browser's target name: {handles}"
+ );
+}
--- a/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update.rs
+++ b/moli-renderer-v8/src/runtime/page_vm/tests/rendering_update.rs
@@ -6267,3 +6267,80 @@
.await
.expect("rendering-update post-checkpoint child synchronization test should run");
}
+
+#[tokio::test(flavor = "current_thread")]
+async fn review853_nested_input_upgrades_ordinary_metrics_without_publication() {
+ run_page_vm_async_test(async move {
+ let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default())?;
+ for explicit in [false, true] {
+ let mut page_vm = test_page_vm_with_loader_and_document_url(&loader, Vec::new(), Url::parse("https://example.com/nested-demand.html")?);
+ page_vm.vm_mut().eval(
+ r#"
+document.head.innerHTML = '<style>html,body{margin:0;padding:0}iframe{border:0}</style>';
+document.body.innerHTML = '<iframe id="outer" style="position:absolute;left:20px;top:30px;width:240px;height:140px"></iframe>';
+const outer = document.getElementById('outer');
+outer.contentDocument.head.innerHTML = '<style>html,body{margin:0;padding:0}iframe{border:0}</style>';
+outer.contentDocument.body.innerHTML = '<div id="child-target" style="position:absolute;left:11px;top:13px;width:70px;height:25px"></div><iframe id="inner" style="position:absolute;left:90px;top:40px;width:100px;height:60px"></iframe>';
+const inner = outer.contentDocument.getElementById('inner');
+inner.contentDocument.head.innerHTML = '<style>html,body{margin:0;padding:0}</style>';
+inner.contentDocument.body.innerHTML = '<div id="grandchild-target" style="position:absolute;left:7px;top:9px;width:30px;height:18px"></div>';
+'installed'
+"#,
+ )?;
+ page_vm.vm_mut().sync_live_document_style_sources();
+ let before = page_vm.vm().layout_pass_observability_for_test().1;
+ if explicit { page_vm.publish_layout_metrics()?; } else { page_vm.layout_metrics()?; }
+ let after_metrics = page_vm.vm().layout_pass_observability_for_test().1;
+ assert_eq!(after_metrics, before + 1);
+ let point = moli_layout::LayoutPoint::new(120.0, 82.0);
+ let hit = page_vm.vm_mut().observable_deep_hit_test_for_current_document(point, false)?.expect("nested point has a target");
+ let host = page_vm.vm().context_host_weak_for_test().upgrade().unwrap();
+ assert_eq!(host.borrow().dom_host().get_attribute(hit, "id").as_deref(), Some("grandchild-target"));
+ let after_hit = page_vm.vm().layout_pass_observability_for_test();
+ assert_eq!(after_hit.1, after_metrics + if explicit { 0 } else { 1 });
+ assert_eq!(after_hit.3.unwrap().paint_operation_count, 0);
+ assert_eq!(page_vm.vm_mut().observable_deep_hit_test_for_current_document(point, false)?, Some(hit));
+ assert_eq!(page_vm.vm().layout_pass_observability_for_test().1, after_hit.1);
+ let rect = page_vm.vm_mut().eval("outer.contentDocument.getElementById('inner').contentDocument.getElementById('grandchild-target').getBoundingClientRect().width")?;
+ assert_eq!(rect, "30");
+ assert_eq!(page_vm.vm().layout_pass_observability_for_test().1, after_hit.1, "child JS geometry reuses the complete tree");
+ page_vm.vm_mut().eval("inner.contentDocument.getElementById('grandchild-target').style.width = '35px'")?;
+ assert_eq!(page_vm.vm_mut().observable_deep_hit_test_for_current_document(point, false)?, Some(hit));
+ assert_eq!(page_vm.vm().layout_pass_observability_for_test().1, after_hit.1 + 1, "dirty child input updates geometry automatically");
+ eprintln!("review853 explicit={explicit}: metrics then input correct; cold metrics pass={} input pass={} dirty pass={}", after_metrics, after_hit.1, after_hit.1+1);
+ page_vm.vm_mut().eval("globalThis.__review853Clicks = 0; inner.contentDocument.getElementById('grandchild-target').onclick = () => globalThis.__review853Clicks++")?;
+ page_vm.vm_mut().dispatch_mouse_event_at_point(120.0, 82.0, "mousedown", 0, None, 0.0, 0.0)?;
+ page_vm.vm_mut().dispatch_mouse_event_at_point(120.0, 82.0, "mouseup", 0, Some(0), 0.0, 0.0)?;
+ assert_eq!(page_vm.vm_mut().eval("String(globalThis.__review853Clicks)")?, "1", "actual coordinate input reaches the nested frame");
+ }
+ Ok::<(), anyhow::Error>(())
+ }).await.expect("nested demand upgrades automatically");
+}
+
+#[tokio::test(flavor = "current_thread")]
+async fn review_853_mock_publication_does_not_run_real_layout() {
+ 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/mock-publication.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("document.body.innerHTML = '<div style=\"width:2300px;height:1500px\"></div>'")?;
+ let before = page_vm.vm().layout_pass_observability_for_test().1;
+ let result = page_vm.publish_layout_metrics();
+ let after = page_vm.vm().layout_pass_observability_for_test().1;
+ eprintln!("review853 Mock publication before={before} after={after} result={result:?}");
+ assert_eq!(after, before, "Mock policy must not run real layout");
+ Ok::<(), anyhow::Error>(())
+ })
+ .await
+ .expect("mock publication regression");
+}
--- a/moli-renderer-v8/src/runtime/page_vm/tests/mod.rs
+++ b/moli-renderer-v8/src/runtime/page_vm/tests/mod.rs
@@ -157,6 +157,7 @@
mod popup_document_completion;
mod preferred_aspect_ratio;
mod rendering_update;
+mod review_window_name_contract;
mod service_worker;
mod service_worker_client_message;
mod service_worker_internal;
--- /dev/null
+++ b/moli-renderer-v8/src/runtime/page_vm/tests/review_window_name_contract.rs
@@ -0,0 +1,57 @@
+use super::*;
+
+#[tokio::test(flavor = "current_thread")]
+async fn review_853_window_name_failed_conversion_preserves_owner_state() {
+ run_page_vm_async_test(async move {
+ let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default())?;
+ let mut page_vm = test_page_vm_with_loader_and_document_url(
+ &loader,
+ Vec::new(),
+ Url::parse("https://example.com/window-name.html")?,
+ );
+ let result = page_vm.vm_mut().eval(
+ r#"
+window.name = 'kept';
+let caught = false;
+try { window.name = {toString() { throw new Error('conversion failed'); }}; }
+catch (e) { caught = e.message === 'conversion failed'; }
+JSON.stringify([caught, window.name]);
+"#,
+ )?;
+ assert_eq!(result, r#"[true,"kept"]"#);
+ assert_eq!(
+ page_vm
+ .vm()
+ .top_level_browsing_context_state()
+ .window_name(),
+ "kept"
+ );
+ Ok::<(), anyhow::Error>(())
+ })
+ .await
+ .expect("window name conversion contract");
+}
+
+#[tokio::test(flavor = "current_thread")]
+async fn review_853_window_name_reentrant_conversion_preserves_completed_inner_write() {
+ run_page_vm_async_test(async move {
+ let loader = crate::network::ResourceRequestClient::new(&FetchConfig::default())?;
+ let mut page_vm = test_page_vm_with_loader_and_document_url(
+ &loader,
+ Vec::new(),
+ Url::parse("https://example.com/window-name.html")?,
+ );
+ let result = page_vm.vm_mut().eval(
+ r#"
+window.name = 'before';
+try { window.name = {toString() { window.name = 'inner'; throw new Error('conversion failed'); }}; }
+catch (e) {}
+window.name;
+"#,
+ )?;
+ assert_eq!(result, "inner");
+ Ok::<(), anyhow::Error>(())
+ })
+ .await
+ .expect("reentrant window name conversion contract");
+}两项转换用例的最小绿测实验（仅验证转换根因，不代表完整修复）
diff --git a/moli-renderer-v8/src/context_bootstrap/runtime_state.rs b/moli-renderer-v8/src/context_bootstrap/runtime_state.rs
index d942d938d..c94b4c8e3 100644
--- a/moli-renderer-v8/src/context_bootstrap/runtime_state.rs
+++ b/moli-renderer-v8/src/context_bootstrap/runtime_state.rs
@@ -900,11 +900,10 @@ 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();
+ let Some(next) = args.get(0).to_string(scope) else {
+ return;
+ };
+ let next = next.to_rust_string_lossy(scope);
if let Some(handle) = child_context_handle_from_owner(scope, receiver)
&& let Some(host_ptr) = context_host_ptr_from_global_bridge(scope)
{cargo nextest run -p moli-protocol -p moli-protocol-server -p moli-renderer-v8 -E 'test(review_853_window_name_)' --no-fail-fast结果：2 passed；所有临时编辑已还原。
93a9bdb to
7bc4360
Compare
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
本轮复审结论：仍需修改；剩余业务问题收敛为“窗口名称状态没有贯通”和“Window.name 绑定契约不完整”两类，不按每个失败表现继续拆分意见。 以下只引用原始 HEAD 的已完成复测，不把临时修复实验当成当前 PR 的通过证据。
审查 HEAD：7bc43604fa85d79be03410a51361a3e1d32b2849；实际堆叠基线：b1fe8592246c6f859a25c293232159db694dd48f（#731）。已同步 main，并由 3 个子 agent 分别检查路由与身份、Window 绑定、布局与实际消费者。发布前已再次确认 HEAD/base 未变。
| 需求 | 新版实现判断 | 验收结果 |
|---|---|---|
| 同一窗口跨 Document 保留 name、新窗口独立 | engine 持有共享 state 的方向正确，但新版创建请求丢失该 state | data→data、HTTP 页面替换均丢名；命名空 popup 初始 name 也为空 |
| live name 路由、关联窗口复用、独立窗口隔离 | source 优先及稳定 group 过滤合理；真实页面与代理仍没有统一名称来源 | 独立窗口隔离通过；自身同名导航、代理改名复用失败 |
| 无截图更新布局、保留嵌套 frame 几何 | 当前基线下显式刷新已有快照有实际作用；复用现有 publication 链路合理 | Mock 限制、无 paint、嵌套重发布通过；实际孙 frame 命中、尺寸更新和点击也通过 |
| Web IDL 的 Window 接收者及参数转换 | 新 guard 只认当前 realm 的 global，仍用空字符串兜底转换失败 | 合法跨 realm Window 被拒绝；异常转换会破坏已有名称 |
1. [P1] 把同一个名称状态贯通 engine → 页面 → popup 代理，先修新引入的创建请求断链。
新版在 HTML 创建路径第 928 行和 streaming 创建路径第 1229 行填入 top_level_browsing_context: Default::default()。NavigationEngine 虽然已向 RendererDocumentOptions 传入共享 state，但两个 request 类型和 builder 没有携带它，实际 Document 因此拿到另一个 state。编译通过并不代表 ownership 传递正确。
原始 HEAD 实测：
- data 文档设置
inline-context，替换文档后读到空字符串。 - HTTP 同源页面设置
streaming-context，导航到另一个 query 的文档后读到空字符串。 window.open('', 'html-context-name')，切换到 popup 读取其真实window.name，得到空字符串。- 两个窗口都自行设置
shared，当前窗口按自己的名称打开时，当前窗口没有被导航。source 优先的查找结构已经正确，但 engine 读不到页面实际写入的 name。
这些场景的 Chrome 154.0.8037.58 对照符合测试期望。另一个仍未闭环的路径是 轻量 popup 代理 getter/setter：它仍更新代理 slot/局部名称表，路由则读 engine state。opener 保存 popup，执行 popup.name='new' 后按 new 打开，Moli 产生 3 个窗口，Chrome 为 2 个。这个代理缺口上轮已经指出，不算本轮新发明的问题。
最小完整修法：恢复两条 options → request → PageVmEnv 的 state 传递，不能在新 Document 创建处重新分配；再沿现有 popup activation 通道把代理关联到真实目标的同一名称状态。复用已有 target 时，需要把代理绑定回已有 state，不能用新代理的状态覆盖旧目标。保留已修好的 source/group 约束。暂存表可以继续承担 engine 安装前的暂存职责，不因为仍有 HashMap 就强制重构或新建全局注册中心。
测试覆盖还发生了回退：上一版 4cb4eef7c 在跨源窗口引用测试和命名 popup 测试中加入的 name 持久化、初始名、改名及旧名失效断言，没有随本次文件拆分保留下来。当前同名测试主要只验证窗口句柄或页面文本，不能证明 name 契约；应恢复这些行为断言。
2. [P2] 统一按真实 Window receiver 解析 owner，再进行可失败的字符串转换。
第 921–949 行把“等于当前 realm 的 global”当作顶层 Window 品牌。这修复了普通对象调用，却误拒绝合法的同源跨 realm 接收者：在 iframe 内取得自己的 name descriptor，调用 descriptor.get.call(parent)、descriptor.set.call(parent, 'parent-after')，Moli 两次抛 TypeError，父窗口保持原值；Chrome 两次成功，父窗口变为 parent-after。这是新 guard 引入的回归，不能通过只覆盖 {} 接收者来验收。
第 951–963 行的旧问题也仍在：to_string() 失败后 unwrap_or_default() 继续写 owner。本轮实际结果为：
- 原名
kept，toString()抛错：异常被捕获，但 name 变成''，应仍为kept。 toString()先成功写入inner再抛错：最终变成''，应保留已完成的inner写入。
修法应是一个一致的绑定流程：通过真实 Window 身份解析 receiver 所属 owner（包括合法跨 realm Window）→ 拒绝非法 receiver → 转换一次 → 转换失败立即返回 → 成功后只更新该 owner。不要按调用 realm 猜 owner，不要将失败转换当作空字符串，也不要用回滚原值破坏重入期间已经完成的写入。
3. CI 的 12 项失败也已归因，需收敛基线整合，不能靠删除场景或放宽断言过关。
| 失败组 | 根因与归属 | 最小处理方向 |
|---|---|---|
| 8 项 dirty geometry/grid/offsetParent/CSSOM + 2 项 frame click | 已有快照后 ensure_initial_layout() 直接复用，DOM/style 变化未刷新。相关生产路径及测试文件与本 PR 当前基线相同，属于继承的契约冲突 |
README 仍承诺 dirty demand 自动刷新，应在统一 geometry/输入 owner 边界保证新鲜布局，保留正确尺寸、遮挡和点击次数断言，不给每个用例加私有 CDP 调用掩盖问题 |
stop_loading_before_response_preserves_document_and_allows_next_navigation |
基线 typed cancellation 早退成 Err，materializer 保留了旧 Document，却把结果映射成顶层 protocol error；测试要求的是带 net::ERR_ABORTED 的导航结果 |
在统一结果映射边界保留 typed cause，同时输出 canonical cancellation 和正确 CDP result 形状；同步修正要求完整 anyhow 文本/top-level error 的冲突测试 |
same_context_named_popup_reuse_navigates_and_activates_loaded_owner |
fixture 用 Target.createTarget 创建独立窗口，仅赋名后就要求另一个窗口复用它；与本 PR 正确的新 group 约束冲突 |
用真正关联的 popup 构造复用场景，保留独立窗口隔离对照，不回退 group 过滤 |
两个点击失败是相反方向的同一问题：新增遮挡 iframe 后错误返回成功；隐藏遮挡后仍错误返回拦截。它们不能因“手动发布后可点击”就视为已修好，标准 WebDriver 调用者不应额外知道私有 publication 参数。
同时纠正上一轮布局建议：当前 base 已变化。普通 geometry 首次仍会构建快照，但已有快照不会因 dirty 自动更新，显式 publishLayout 因而确实提供无 paint 刷新的作用。本轮不再建议仅以“首次普通 metrics 已足够”为由删除该接口。 应把这个真实需求、与普通读取的区别、基线契约冲突写清楚。当前 PR body/Verification 仍沿用旧描述和通过数，未反映这些边界。
删除代码的核查结果：5 个 navigation 测试以及 fixed_inline_font 原来各有两份，逐个函数全文相同，本次各保留一份；这一部分是去重，没有削减不同场景的覆盖。
已完成的验证：
- 原始 HEAD 的
moli-protocol、moli-protocol-server、moli-renderer-v8全套加 6 项审查探针：12,650 run / 12,634 pass / 16 fail / 4 skip。其中仓库原有测试失败正是上述 CI 的 12 项，另 4 项为审查补充场景。 - 扩展后的独立场景矩阵：11 项，3 通过、8 失败。8 个失败按上面两类业务问题处理；3 个通过为独立窗口隔离、Chrome 对照校准的既有 named popup/noopener 复用、嵌套 publication 实际消费者。
- 嵌套消费者实测：孙 frame 命中正确，重新 publication 后宽度 30→35，两次 paint=0，真实 mousedown/mouseup 触发 onclick=1。
- 本轮新增的导航/初始名及跨 realm receiver 期望均用 Chrome 154.0.8037.58 核验。Mock 拒绝真实布局、无 paint 发布、嵌套 frame 重发布三项仓库测试通过。
修复验收应让这些真实红测在不降低期望的情况下转绿，并消除 CI 的契约冲突。本轮只交付审查意见；所有本地临时修改已还原，没有提交或推送代码。
7bc4360 to
67e920b
Compare
67e920b to
4353c96
Compare
4353c96 to
1be6b44
Compare
Review closeout on current headCurrent head
This keeps popup lookup, proxy mutation, Document replacement, and the live target on one state owner. It does not add a global registry or browser/task-specific exceptions. Validated on the current head: new cross-realm and throwing-conversion scenario; named-popup state after Document replacement and live-target lookup; existing Window.name 5/5; protocol layout/group/staged 3/3; layout publication 3/3; Classic named-popup 2/2; full workspace all-targets/all-features Clippy; fmt and diff checks. The full nextest run was started as the repository gate, but this closeout does not wait on its long tail or treat remote CI as review evidence. |
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
复审版本：ad381b33c42824c69a83d3f640f5b706f7d1a590，实际 PR base：1cb3f28df4a32496a41ee928da0254a6b75a8dee。已同步 main，并按实际 base 区分本 PR 改动与上游改动。三路并行只读审查；本轮没有修改仓库代码或测试。
结论：新版确实修复了上一轮多项问题；仍有两个需要收敛的名称状态问题。不要按下述复现分别增加对象特判，应先明确“合法 Window 接收者 → 具体 browsing context → 唯一 name 状态”的关系。
需求与方案复核
| 需求 | 当前结果与方案评价 |
|---|---|
| 同一顶层 browsing context 换 Document 后保留名称 | data→data、HTTP 同源、跨端口导航均通过真实 WebDriver 验证。共享状态已贯穿普通/流式页面创建；这是正确的生命周期归属修复。 |
| 名称转换失败不破坏旧状态；合法跨 realm 调用 | 普通转换抛错、重入后抛错、子 frame 借用 descriptor 读写 parent 均通过。保留成功转换后再写入的顺序。 |
| 命名目标初始名称、同名源窗口优先、group 隔离、相关目标复用 | 当前协议场景均通过。不能再沿用前轮已经失效的结论；也不能把 noopener 一概解释成必须新建目标。 |
| 修改 opener 持有的 popup.name 后按新名复用 | 仍失败，见问题 2。新增测试读取的是目标文档和 engine，尚未证明 opener 持有的代理同步。 |
| Window.name 借用访问的接收者正确性 | 合法 parent 场景修好了，但宿主指针不等于 Window 身份/owner，见问题 1。 |
| 不截图也能获得真实 metrics、嵌套 frame 几何并点击 | 当前实际 base 已恢复普通读取的 dirty refresh；真实 CDP 客户端用普通请求和 publishLayout=true 均通过。扩展参数的独立需求仍需讲清楚，详见后文。 |
1. [P2] 宿主指针不能代替 Window 品牌和具体接收者归属
位置：moli-renderer-v8/src/context_bootstrap/runtime_state.rs:919–960，尤其 child_handle.is_none() 后直接访问 host 的顶层 name 状态。
is_window_receiver 依赖的 context_host_ptr_from_window_object 会读取 internal field 0；NodeList、NodeIterator 等非 Window 原生对象也存有同一个 host 指针。同时 lightweight popup 与 opener 共用运行时，没有 child handle 不意味着接收者就是 opener 的顶层 Window。
两个独立实际复现（均已对照 Chrome 154）：
const d = Object.getOwnPropertyDescriptor(window, 'name');
window.name = 'kept';
const r = document.createNodeIterator(document); // NodeList 也相同
let converted = false;
d.get.call(r);
d.set.call(r, { toString() { converted = true; return 'forged'; } });Chrome：getter/setter 都抛 TypeError，setter 不执行转换，window.name 仍为 kept。Moli：getter 返回 kept，setter 接受并执行转换，真实 window.name 变成 forged。原有宽松品牌判断在这次接到共享状态后变成了真实窗口状态污染。
window.name = 'opener';
const p = window.open('about:blank', 'popup');
const d = Object.getOwnPropertyDescriptor(window, 'name');
const before = d.get.call(p);
d.set.call(p, 'renamed');
// 观察 before、window.name、p.nameChrome：popup / opener / renamed；Moli：opener / renamed / renamed。也就是把对 popup 的合法借用调用错误应用到了 opener。它不是单纯的错误消息差异，会破坏名称查找和后续导航目标。
最小完整建议：先确认真正的 Window 身份，再按接收者绑定的具体 owner 解析名称；利用现有 frame/popup 身份与 WindowExecutionContextOwner 等绑定信息，不新建另一套全局注册体系。不要通过排除 NodeList/NodeIterator、判断字段数量、或“没有 child handle 就是 root”的分支修复这两个 case。测试应覆盖非 Window 品牌先于 ToString、合法 parent 借用、popup 借用且 opener 不变，并保留异常转换与重入用例。
2. [P2] opener 侧 popup 代理与实际目标仍有两份名称状态
位置：moli-renderer-v8/src/native_bridge/context_host/popups.rs:4293–4326 的代理 slot/local map，与 moli-protocol/src/conn/page_state/page_targets.rs 的 live engine 名称查找之间。
真实 WebDriver 分三次执行，确保目标已经创建：
window.__popup = window.open('about:blank', 'old');
// 下一次 execute
__popup.name = 'new';
// 再下一次 execute
window.open('about:blank#new', 'new');预期复用原 popup，总窗口数 2；Moli 实际为 3。代理 setter 只更新 opener 内的 slot/name map，实际目标 engine 的共享 name 没有同步；查找新名时于是创建了额外目标。这是上一轮尚未关闭的问题，不作为新发现重复扩散。
新增 runtime 测试里“popup proxy name and live target engine share one owner”的描述超出了实际覆盖：它验证目标文档的 window.name 与 engine，没有修改 opener 持有的 proxy。
最小完整建议：沿现有 popup 激活/目标绑定路径，让代理和实际目标引用同一份 name 状态；新目标只初始化一次，复用目标绑定已有状态，避免激活时覆盖已经发生的 rename。保留当前 source 优先和 group 隔离规则。用真实 opener 代理覆盖双向名称更新、按新名复用、旧名失效、已有目标复用及 group 隔离；不再用仅改目标文档的测试代替代理同步。
布局需求和基线测试合同需要一次整理
当前 base 与上一轮不同：普通 Page.getLayoutMetrics/geometry/input 已支持 dirty refresh。本轮真实 CDP 测试中，无扩展参数和 publishLayout=true 都得到 content height 2300→2600、嵌套子 frame 宽度 30→35，并成功通过坐标点击产生一次事件。共享无绘制 publication 路径及 Mock guard 本身没有发现错误；不能把显式强制刷新笼统判为 bug。
但若需求只是“无截图真实 metrics + 子 frame 几何”，标准路径已满足，继续增加 force-publication 协议分支的必要性不足。建议优先保留标准路径与端到端回归；若保留扩展，应在 PR 描述中明确其独立的强制刷新需求、调用者及额外工作边界，而不是继续以普通读取不会刷新为前提。
实测结果与基线归属
- 实际 CLI 编译成功；外部 WebDriver 矩阵 13 项：10 通过、3 失败，失败收敛为上面两个根因。Chrome 对照验证了非 Window 接收者和 popup 借用行为。
- 外部 CDP 普通/显式 publication 两种模式均通过 metrics、嵌套 frame 更新和实际坐标点击验证。
- 现有重点回归 48 项：47 通过；1 条旧 metrics 断言失败，属于下述基线合同遗漏。
- 扩大到三个模块的测试已主动停止，未作为 review 必须跑完的门槛：停止前 6692 通过、6 条真实失败；另 14 条被 SIGINT 中断、5946 未运行、4 跳过。不能将中断项算作产品缺陷，也不宣称全套通过。
6 条已完成失败均逐项核对了实际 base 的代码/依赖，分三类，不计入本 PR 新增 findings：
- 4 条布局旧契约断言：
get_layout_metrics_initializes_layout_and_reuses_content_until_capture、cdp_geometry_initializes_layout_and_mouse_reuses_it_until_capture、print_to_pdf_returns_base64_pdf_without_publishing_interactive_geometry、print_capture_preserves_published_geometry_frame_viewports_and_mouse_targets。普通读取已正确返回新高度1600/宽400、移位后的按钮旧位置不再命中、打印后的 geometry read 刷新得到 screen 新尺寸100或60。应统一更新场景合同：冷读初始化、无改动可复用、dirty 读/输入立即刷新；打印“不主动发布”的计数应放在首次 geometry read 之前检查，之后再验证读取的是新 screen 几何而非 print 几何。相关测试/实现与 base 相同。 - 1 条取消导航载体契约：
cancelled_background_transport_does_not_prepare_a_network_error_document仍期待 typed Err，base runtime 已返回 NetworkFailure(ERR_ABORTED)，真实消费者仍走 PreserveCommittedDocument。应对实际 outcome 的 materialization 验证原文档保留，不能把 Ok 载体直接等同于创建错误页。相关文件与 base 相同。 - 1 条 base Stylo 依赖回归：
content_alt_counters_use_stylo_declaration_blocks的content: "" / counter(cnt)被 base 新 pinlanyue-llk/stylo@402a73d的alt_start.is_none()限制拒绝。不是本 PR 改动，也不是 prefs 未初始化。应在 Stylo 侧恢复此前 alt-counter 支持并保留新 pin 所需能力，不删测试或增加 renderer 字符串绕过。
当前 HEAD 的 Format、Clippy 已在 CI 成功，Nextest 仍排队；以上是本轮实测口径，不替代全工作区 CI。
上述测试失败按共同原因处理，不应为了让旧断言通过而回退正常 dirty refresh，也不应把基线遗漏包装成本 PR 新增缺陷。PR 描述需要分开列出名称持久化、接收者归属、命名路由/代理同步、布局 publication 四类需求、对应方案和当前实测结果；现有旧验证数字不足以代表此 HEAD。
本轮仅审查和外部复现，没有代写修复、修改仓库测试、提交、推送或清理既有缓存。
lanyue-llk
changed the title
|
Resolved the actionable findings from review
Verification:
The six failures identified as existing on the base stack were not copied into this PR. |
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
复审 5919dde5492141ecb77354784c107228f8662fad，实际 base 为 1cb3f28df4a32496a41ee928da0254a6b75a8dee。已同步 main，三路并行检查增量及相关调用链。本轮是只读代码审查，没有重新编译、运行全量测试或修改仓库；以下证据来自当前代码路径和测试内容，不冒称新版黑盒实测。
新版的 PR 描述已按需求分类，较前版清楚；Window 品牌判定不再接受任意宿主指针，普通 popup 的借用 accessor 也已显式识别 owner。未使用的 publishLayout 扩展删除完整，保留了基线标准布局能力。这些调整方向正确。
仍有两个需要解决的问题，均集中在 popup 身份与名称归属，不建议继续围绕单个用例补路由分支。
[P1] 用局部 popup_id 在整个 BrowserContext 查找，会复用其他来源的窗口
定位：moli-protocol/src/domains/target/popup.rs:98–103。
新分支先执行 target_id_for_popup_id(popup_id)，命中后不再执行带 source/group 检查的名称查找。但这个 ID 不是 BrowserContext 全局身份：JsContextHost 在 core.rs:412 将计数器初始化为 1，popups.rs:732–736 只在该 host 内自增；domains/page/popup.rs:107 将这个裸 ID 直接传入目标创建，未加来源限定。反查方法则扫描整个 BrowserContext 的 target→popup_id 表。
因此同一 BrowserContext 的两个独立标签页，各自第一次打开 popup 时都得到 ID 1，第二次创建会命中第一个标签页的 popup，并进入其导航/名称更新路径。不同名称、甚至 _blank 都不能避免，因为 ID 匹配发生在名称规则之前。同一 opener 更换文档、重建 host 后，计数器重置也有同类冲突。
最小修复方向：不能把裸 u64 当作已绑定目标的权威身份。复用必须使用带来源及 document/host 世代的绑定，沿用现有 prepared activation 的 page_owner、source.root_document 等身份信息；仅补一个 group 判断仍挡不住同组的不同 host 或文档重建。不要让 target→单个局部 ID 的反查承担跨来源身份判断。
验收场景：同一浏览器上下文内 A/B 两个独立标签页分别打开第一个 popup，应共四个窗口且互不导航；分别覆盖不同名、同名、_blank，以及 opener 换文档后再打开。应以当前代码失败、修复后通过的回归证明边界，而不是只测试单个 opener 多次打开。
[P2] 名称仍是两份状态；再次 open 时补写会让失效旧名重新生效
定位：moli-protocol/src/domains/target/popup.rs:130–132；代理 getter/setter 仍在 moli-renderer-v8/src/native_bridge/context_host/popups.rs:4293–4326。
代理仍读取自己的 WINDOW_NAME_SLOT，setter 只更新 opener 内的 lightweight 名称表；真实 target 的 window.name 使用另一份 top-level state。新逻辑只是下一次 window.open 按 popup_id 找到 target 后，把此次请求的 target_name 写回 engine，并没有实现 PR 描述声称的同一名称状态。
后果有两种，同属一个根因：
- 单独执行
proxy.name='new'后，真实 target 不会立即读到新名；反向在 target 中改名，已持有的 proxy 仍读旧值。 - target 自己从
old改成new后，opener 的局部名称表仍把old指向旧 popup ID。此时直接window.open(url,'old')，新增 ID 优先分支会复用该旧 target，随后又把它的真实名称覆盖回old。正确行为应创建新的 old 窗口，原 target 保持 new。
新增 window_open_named_target_reuses_existing_popup_target 测试把“proxy 改名”和“按新名再次 open”放在同一表达式，恰好经过补写点之后才检查 target，所以在两份状态仍然分离的实现上也能通过。它证明了这个组合操作能复用，不能证明共享名称身份，更不能证明反向改名。
最小完整方案：在合法 proxy 与实际 target 建立绑定时，使双方名称访问指向同一个 browsing-context 状态，或者通过该绑定即时读写同一权威状态；复用已有 target 时采用其已有状态。不要把一次 window.open 的查找名称当作覆盖现有 target 名称的同步来源，也不要引入第三份缓存。
测试必须拆开观察：① proxy 赋名后、不再 open，立即从 target 读取；② target 赋名后立即从已有 proxy 读取；③ target 改名后，先按旧名打开，再按新名打开，验证目标数量、身份、原 target 名称均正确。与上面的多来源/文档重建场景一起覆盖，才能一次收敛。
验证与交付结论
当前 HEAD 的 CI Format、Clippy、Windows storage、Python/Node harness 已成功，Nextest 仍在运行。这些成功不能替代上述缺失场景。上轮六条基线失败不在本轮重复报告。
建议保留本轮品牌与布局收敛修复，撤掉“裸 popup_id 优先匹配 + 下一次 open 补写名称”作为完成共享状态的依据，集中修正绑定身份和名称权威。PR 描述中“one name identity”的完成声明应在独立双向观察和来源隔离测试通过后成立。
|
已在
同时补齐普通 独立复核未发现新的功能阻塞、死锁或生命周期泄漏。未采纳 本地验证：
|
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
复审 c8c819b75dd5ec60c72ae8ae7a2d6bb9383b745a，actual base 1cb3f28df4a32496a41ee928da0254a6b75a8dee。已同步最新 main，三路并行只读核对代码与测试。此次没有修改代码、重新编译或跑全量测试；以下是代码路径审查结论，不冒称新版黑盒实测。
上轮两项根因已在实现上解决：
- popup ID 提升到 BrowserContextRuntime 的共享原子分配器，与协议注册表作用域一致，不再按每个 host 从 1 重置。
- popup record、激活事件、实际 target 现在传递/绑定同一名称状态，getter 和名称查找均读取当前共享值。新测试已将双向赋值与读取拆成独立请求，并验证目标改名后的旧名失效；还覆盖了保留 proxy 跨一次目标 Document 替换。这不再是上轮“再次 open 时补写”的方案。
布局扩展删除保持完整。不要恢复旧名称 map，也不需要另建一套名称注册表。
[P1] 本地 popup 预选结果仍绕过目标选择规则，导致隔离目标误复用
主定位：moli-renderer-v8/src/native_bridge/context_host/popups.rs:666–683；对应信任路径：moli-protocol/src/domains/target/popup.rs:102–107。
新版扫描所有打开的 popup record，只按当前名称匹配。opener.is_some() 判断的是本次调用，不是候选是否属于源窗口可复用的组。随后协议优先按 popup ID 复用，命中后完全跳过带 source/group 判断的 target_id_for_window_name。ID 唯一只能证明“是哪一个对象”，不能证明“这次应当选它”。
同一根因有两个明确场景，建议一并收敛：
A. 新版引入的 noopener 候选泄漏
在同一源窗口分两次请求执行，等第一次目标创建后再执行第二次：
window.open('about:blank#isolated', 'report', 'noopener');
// 下一次请求
window.open('about:blank#second', 'report');创建路径保留 report 作为 record 名称，但第一个 popup 没有可访问 opener，协议将其留在独立组。旧实现只有创建时存在 opener 才登记可复用名称；新扫描丢掉了这一资格限制，第二次会找到这个隔离 record，随后通过其 ID 复用并导航原目标。应当新建第二个 popup，原隔离目标不被导航，总计三个窗口。这里不是声称“任何 noopener 调用都不能复用已有相关目标”，而是不能把已经创建出的独立组目标重新纳入源窗口的命名选择。
B. 源窗口同名时仍被已有 popup 抢先选中
window.open('about:blank#popup', 'shared');
// 等 popup 创建；仍在原窗口执行
window.name = 'shared';
window.open('about:blank#self', 'shared');renderer 先在 popup records 中选出原 popup，协议优先信任这个 ID，绕过 target_id_for_window_name 中已经写好的 source 优先规则。应导航原窗口自身，保留 popup 的原 URL。已有“源窗口同名优先”测试没有同时放入源窗口持有的同名 popup，因此不能覆盖此路径。
最小完整修复方向
在现有目标解析路径中，先确定源 navigable 的优先级及候选的关联/组资格，再复用合法 popup；协议按 ID 解析时也不能跳过这些约束。不要只调换协议分支：renderer 此前已经 reopen_lightweight_popup_window 并选择了同步返回的 Window，晚到协议层纠正目标仍可能造成返回对象与实际导航不一致。沿用现有身份、分组和 owner 信息完成选择，不增加针对 report、noopener 字符串或某个测试顺序的补丁。
验收应同时检查目标 ID、源/popup URL、返回对象、窗口数量，而非只检查“没有 Target.targetCreated”。建议将两个场景与正常命名复用、双向改名、关闭后重开、不同 host 首次 popup、opener 换文档后再打开组成有限场景矩阵，避免继续逐个 case 修补。
测试与说明
新增 allocator 单测证明两个 runtime handle 共用计数器，代码接线也正确；但它不是两个真实 renderer host 各自打开 popup 的协议验收。激活绑定中的 observed_name != target_name 有保留 open 后立即改名的合理用途，本轮没有证据把它判为 hack；可补“同一 JS turn open 后立即改名、激活消费后各端一致”的场景测试，而不是为内部比较分支定制断言。
发布前 GitHub 对这个 HEAD 返回的 statusCheckRollup 为空，因此不沿用上一 HEAD 的 CI 成功结论；PR 描述中的本地验证记录与本轮只读审查分开看。上轮基线六条失败不重复计入本 PR。
结论：共享身份与共享名称本轮可以关闭；剩余修复应集中在一个边界——合法目标选择规则不能被本地 popup ID 快捷路径绕过。
|
已处理最新 review，并对建议做了独立复核。 正确且已修复的问题：
未采用的扩展设计：没有恢复旧的 name map，没有新增第二套目标注册表，没有引入 browser/case 特判，也没有修改布局或 WebArena 评分语义。 验证：相关 9 个目标选择场景全部通过（首轮 7 个通过，两个测试环境/fixture 问题修正后单独通过）；新增 alias-aware 回归矩阵 8/8 通过； 修复提交： |
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
复审 51316b77be62b8732b9b3b0613417f19155eb87f；actual base 1cb3f28df4a32496a41ee928da0254a6b75a8dee。本轮按 Window.open / hyperlink / form 入口、source/child/popup 归属、组隔离、调用与异步消费顺序、Document/close 生命周期、多 alias 到 WebDriver 消费者完整分工检查后，一次性汇总。
没有修改仓库、重新编译或跑全量测试。Moli 侧以下为当前代码因果链；预期行为另用独立 Chrome 154.0.8037.58 最小场景及 HTML 标准核对，不将 Chrome 实测冒称 Moli 新版实测。当前 HEAD 的 GitHub statusCheckRollup 为空。
已修复项：BrowserContext 级 ID、共享 name 双向读写、普通非空 URL 的 source 优先、隔离 popup 候选过滤、opener 关闭后组身份保持、popup 内 child 的真实顶层身份传播，方向正确。不能退回裸 ID 无条件复用或分离名称副本。
仍有 4 项确定问题，应共同解决以下边界，而非再逐个场景增加分支。
1. [P2] noopener 被过度解释成禁止选择任何已有命名目标
定位：moli-protocol/src/domains/target/popup.rs:102 的 can_access_opener 总开关；moli-renderer-v8/src/native_bridge/element/activation/targets.rs:517–525 的提前 popup 分支，以及 Window.open 中同名 source/iframe 的 suppress_opener gate。
本轮 Chrome 对照：先正常 open 到名为 report 的 popup，再 open('about:blank#second','report','noopener=yes')，返回 null，但仍只有两个窗口，原 popup 导航到 second，保留原 opener。另 <a rel=noopener target=dest> 指向已有 iframe dest 时仍导航该 iframe，只有一个顶层窗口。新版却直接跳过已有目标并新建。
这与“先前已创建的独立组 popup 不应被外部复用”不同。当前 HTML 规则先按名称选择已有合法 navigable，找不到才进入新建分支并依据 noopener 决定 opener；noopener 还影响返回值。见 HTML 目标选择规则。
最小修复：分开“是否允许选择已有目标”“新建时是否带 opener”“JS 返回值”三件事，保留组隔离，不能用 can_access_opener 一票否决已有 source/iframe/相关 popup。新增 noopener_named_popup_does_not_reuse_a_related_target 和 noopener_named_hyperlink_does_not_reuse_an_existing_iframe 正在固化错误预期，应按上述场景纠正；不要把 form 当前正确的已有 iframe 复用行为也改错。
2. [P2] 异步消费时按最新名称重选，会把一次合法复用变成新建
定位：moli-protocol/src/conn/page_state/page_targets.rs:299–307，及 target/popup.rs 的 fallback 新建路径。
先已有 P，name=old。下一次 JS 执行中：
const p = window.open('about:blank#next', 'old');
p.name = 'new';renderer 调用时已选中 P，激活携带其 popup_id；共享 name setter 随后立即把 P 的 engine 名称变为 new。协议消费时却要求“现在再查 old 仍等于 P”才接受 ID，条件失败，按 old 的 fallback 也失败，最终创建额外 target，并可能把同一 incoming name state 安装给两个 target。
Chrome 对照为同一个 Window、两个窗口总数、P 的 URL=next/name=new。这里不是名称共享失败，而是把调用时已接受的目标选择，错误推迟到名称可能改变之后重做。
最小修复：在选择时完成 source/组资格判断，携带已选择的稳定目标身份；消费时校验其存活及来源/生命周期，不用可变名称重新决定导航对象。不能为了修这里恢复上轮“裸 ID 绕过所有规则”。回归覆盖同一 turn 中复用后改名、两次排队打开后改名，以及目标已关闭/来源失效时安全拒绝。
3. [P2] 空 URL 获取同名 source 引用会清空当前页面
定位：moli-renderer-v8/src/context_bootstrap/window_runtime/dialogs.rs:198–207 新增 source 分支及 navigate_window_open_self。
window.name = 'self';
window.open('', 'self');前面已将原始空 URL 转为 about:blank，新增分支无条件调用 Location Assign，导致当前 Document 被空白页替换。Chrome 对照返回当前 window，同时保留原 URL、DOM 和 JS marker。child/popup source 也走同一 helper。
应保留原始“未提供导航 URL”语义：选中已有上下文时只返回引用，不导航；创建新上下文时才使用初始空白页。标准也明确区分已有目标的 null URL 与新建目标的 about:blank，见 window open steps。测试应区分空串与显式 about:blank，不能只测带 fragment 的非空 URL。
4. [P2] 多 alias 只修了内部反查，Classic WebDriver 仍使旧代理失效
定位：page_targets.rs:261–265 同时维护全 alias 和 latest alias；moli-protocol/src/conn.rs:3270、moli-protocol-server/src/protocol_server/webdriver_classic.rs:2536 / 1372。
场景：A 保存 p=window.open(...,'report')；另一个相关窗口 Q 再按 report 打开，获得同一 target 的第二个 renderer alias；回 A 执行 return p。
内部 popup_target_ids 的确保留两个 ID，但 GetTargets 仍通过 target_popup_id 只输出最新 moli_popup_id。Classic 的窗口引用映射只收集这一个 ID，旧 p 序列化时查不到，返回 NoSuchWindow / "popup window handle not found"，尽管窗口仍打开。这是“保留所有 live alias”要求尚未贯穿消费者，不应以内部 map 单测宣布关闭。
最小修复：让 Classic 窗口引用解析通过所有 live alias→稳定 target 的映射完成，或直接解析请求中的具体 alias；无需为一个 target 输出重复窗口。验收两个来源的旧/新 proxy 均返回同一个 WebDriver window handle，关闭后两者均失效，并核对 dialog/close 使用同一身份边界。
同类完整性缺口，明确与新回归分开
form 普通命名提交在 native_bridge/element/forms/submission.rs:616–692 直接走 iframe 分支。顶层或 popup 自身 name=same 且有 descendant iframe same 时，仍未应用这次给 Window.open/anchor 增加的 source 优先；没有同名 iframe 时也不能据新增 anchor 测试断言表单会复用协议 target。相关路径在 actual base 已存在，不列为本 PR 新引入的第五个缺陷。PR 若继续声称“HTML named-target selection consistently”，应补齐该入口或明确收窄完成声明。不要复制 anchor 的 noopener 特判到 form；应解析实际目标后保留 GET/POST 各自导航语义。
一次性收敛的验收边界
| 维度 | 必须区分的场景 |
|---|---|
| 入口 | Window.open、anchor、form；先明确哪些已支持、哪些是基线缺口 |
| 目标 | source / iframe / 同组 popup / 已隔离 popup / 未存在目标 |
| 参数 | 空 URL vs 显式 about:blank；普通 vs noopener/noreferrer |
| 时序 | 调用时选择后立即改名、同 turn 多次打开、异步消费前关闭或文档更换 |
| 身份 | 多 host、多个 live proxy alias、关闭后全部失效 |
已有正确的共享状态、组身份和 child 传播应保留。修复核心应是明确区分“选目标、提交导航、决定 opener/返回值、投影协议身份”，并让同一决策贯穿消费者；不是为以上四段复现各加一个局部分支。本轮不重复上游六项基线失败，也不把尚无确定证据的边界猜测包装为新增 finding。
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
复审 54406cd7630f1bbd3c11a16578e183da9d676bd0，actual base 1cb3f28df4a32496a41ee928da0254a6b75a8dee。本轮三路并行，完整核对入口、共享状态、选择/消费时序、关闭重开、多 alias 的协议输出及消费者，再一次性汇总。没有修改仓库、编译或跑全量测试。Moli 侧结论来自当前代码路径；Chrome 侧仅做最小预期行为对照，两种证据不混称。
上轮关闭情况
| 上轮问题 | 本轮判断 |
|---|---|
| 同 turn 选中后改名导致重新选目标 | 已修：携带 selected_existing_target，消费时不再按 mutable name 重选，并拒绝真正失效的已选目标 |
| 普通空 URL 获取引用误导航 | 主路径已修：renderer/protocol 两端保留 navigation_requested，已有 source/popup 不再导航，新上下文仍初始 about:blank；特殊目标解析另有下面第 3 项 |
| Classic 旧 alias 丢失 | 已修：全部 aliases 经 GetTargets 到 Classic 映射，端到端测试跨两个真实窗口返回旧/新代理到同一 handle，且不重复枚举 |
| noopener 过度排除已有合法目标 | 未修，见第 1 项 |
剩余 3 项，其中 1 项原问题未关闭、2 项是新增分支暴露的完整性问题。
1. [P2] noopener 的选择资格、opener 设置、返回值仍被混用
定位：context_bootstrap/window_runtime/dialogs.rs:217/235 及传入 lightweight lookup 的 initiator；domains/target/popup.rs:114/130；native_bridge/element/activation/targets.rs 中 suppress_opener 提前新建分支。
现有 !suppress_opener / can_access_opener gate 仍直接跳过已有 source、iframe、同组 popup。新加的内层 if suppress_opener { return null } 在外层要求 !suppress_opener 的分支里不可达，没有改变行为。
上轮已用 Chrome 154 实测并核对标准：
- 先正常创建 report，再
open('about:blank#second','report','noopener=yes')：返回 null，但复用原 report，不新增 target。 <a rel=noopener target=dest>指向已有 iframe dest：导航该 iframe，不新建顶层窗口。
本轮上述错误路径与固化错误预期的 tests 仍在，不能作为已关闭项。标准先找已有合法 navigable，没找到才依据 noopener 创建隔离上下文，见 HTML 目标选择规则。应区分选择资格、新建 opener 和返回值，继续保留“已隔离组不可被外部复用”的正确边界；不要反复在这两个相反方向之间切换。
2. [P2] 协议关闭之后再打开，被误判为过期的已选请求并丢弃
定位：domains/target/popup.rs:114–128，与 native_bridge/context_host/popups.rs:708–737 的本地存活筛选。
场景：A 打开 p = open('about:blank#one','report')；通过 Classic WebDriver 关闭 P（或 Target.closeTarget）；切回 A，再 open('about:blank#two','report')。这次应创建新的 report。
代码链：协议 take_page_target_for_close 删除 target 和全部 alias，conn/browser_context/target_session_owner.rs 关闭该 target 的 page；但是 opener 内的 lightweight record 没有同步转为 Closed，is_open() 仍只读取该本地生命周期。新调用因此仍选旧 popup_id、标记 selected_existing_target=true；消费端发现 ID 不存在后直接 return None，导致一次正常的关闭后重开被吞掉。
新增 stale receipt 测试验证的是“已选择之后目标关闭，不应改投同名替身”；它没有覆盖这里“调用之前目标已经关闭，新调用应新建”。两种时序必须同时成立。
最小完整修复：让真实 target 的关闭同步到所有 live proxy 的选择资格/生命周期，调用时不再选已关闭目标；保留真正 stale receipt 的拒绝规则。不能简单恢复按旧名字 fallback，否则会重新引入把已接受导航发给替身的错误。回归覆盖 CDP/Classic 关闭后重开、两个 alias 同时失效、选择后关闭，以及原始 JS close 路径。
3. [P2] 空 URL 的 _parent 分支把脚本可覆写属性当成真实目标身份
定位：context_bootstrap/window_runtime/dialogs.rs:199–207。
const fake = { fake: true };
window.parent = fake;
const result = window.open('', '_parent');window.parent 在项目里是 replaceable 属性。新空 URL 分支直接 entered_window.get('parent') 并返回任意 Object，因此会把 fake 当作选中的父窗口；getter 被覆写时还会执行不应参与目标选择的脚本。它没有导航，但返回了错误身份。
本轮独立 Chrome 154.0.8037.58 实测：parent 属性确实可替换；open 返回真实 window，result===fake 为 false，原 DOM 保留。一般 frame 中也应返回真实父上下文。
最小修复：把“按内部关系选择特殊目标”与“是否提交导航”拆开，共用既有 owner/parent/top 身份解析；空 URL 只跳过导航，不能改为读取公开 JS 别名。测试覆盖可替换 parent、嵌套 frame 的 _parent/_top、空 URL 不导航及显式 about:blank 仍导航，不要只给 fake 对象增加特判。
收敛建议和验证边界
共享名称、BrowserContext ID、组生命周期、child→popup opener 传播以及全 alias 映射本轮均未发现需要推翻的确定问题。新 receipt 的方向也正确，需补齐其依赖的真实关闭状态。
已有 form source-first 缺口仍属于实际 base 的旧路径，本轮不复制成新 finding；PR 对入口一致性的声明应保持准确。当前 HEAD 的 GitHub statusCheckRollup 为空；PR 描述中的本地 check/clippy/测试记录是作者报告，不替代本轮独立审查，也不沿用旧 HEAD 的 CI 结果。
建议在现有实现上统一三个边界：合法目标选择与 noopener 返回值分离、真实 target 生命周期与全部 proxy 一致、内部身份解析与导航动作分离。按上述场景完成红绿回归后关闭问题，无需再堆名称缓存、全局注册表或针对某条复现的分支。
|
已在
没有采用跨 target WindowProxy 扩建：当前独立页面 runtime 的 验证结果：
renderer lib-test 可执行文件仍被堆叠基线中既有 API/test consumer 不同步阻断；本提交未修改这些基线 API，生产与 all-target checks 正常，跨层行为由上述 protocol 与 Classic 测试覆盖。 |
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
复审 a9858cc311b13a03931bf973f020d5cb0da2d5dc，actual base 1cb3f28df4a32496a41ee928da0254a6b75a8dee。三路并行完整核对入口、目标选择/消费、名称共享、协议关闭、异步回执、多 alias 和 Classic 消费者后，一次性汇总。
本轮没有修改仓库、编译或跑全量测试。Moli 结论来自当前代码路径和测试审阅；Chrome 154.0.8037.58 仅用于独立验证预期行为，不冒称新版 Moli 黑盒实测。
上轮三项可以关闭：
- noopener 的目标选择与新建 opener/返回值已分开，已有合法命名目标可复用，独立组仍不可跨组复用；新 Classic 场景验证了实际目标数量、URL 和原 opener。
- 协议关闭会退休全部 alias 的共享 live ID，旧代理 closed 能反映关闭，关闭后的首次同名重开可创建替代窗口。
_parent/_top已改用内部 owner/child 关系，空/非空 URL 路径均不再依赖可覆写的公开别名。
旧的名称共享、调用时选定身份、Classic 全 alias 映射也未回退。没有找到可确定的额外注册表泄漏或死锁，不把猜测列为 finding。
[P2] 关闭的旧记录仍可挡住已经重开的同名活窗口
定位：moli-renderer-v8/src/native_bridge/context_host/popups.rs:717–723。
名称查找的 find_map 只检查本地 record.is_open()、组和名称，选中第一条记录之后才调用 lightweight_popup_is_open(popup_id) 检查共享存活状态。协议关闭只退休共享 ID，opener 原有 record 的本地 lifecycle 仍可为 Open。因此发生以下顺序时，旧记录和替代记录会同时参与名称匹配：
- A 创建名为 report 的 P。
- 经 Classic WebDriver / Target.closeTarget 关闭 P。
- A 重开 report，保存新的
replacement。 - A 再次
open('', 'report')。
如果 HashMap 先枚举旧记录，find_map 就选中旧 ID；外面的共享 live 检查失败后，整个复用分支结束，代码直接创建新的 lightweight WindowProxy/Document，而不会继续查找真正存活的 replacement。结果是再次获取的代理不等于 replacement，且本地文档/expando 可丢失。
这里不应断言一定增加 protocol window handle：后续协议名称 fallback 可能把新 alias 合并回现有 target，窗口数量仍正确，掩盖错误身份。应断言 open('', 'report') === replacement，并检查文档和自定义状态保持。
本轮 Chrome 对照：协议关闭旧窗口后重开，再连续 20 次按空 URL/同名获取，全部严格等于 replacement；marker=19、DOM 文本 kept 保留，窗口总数为 2。当前 Moli 路径的问题取决于 HashMap 枚举顺序，不能以一次恰好选到新记录来排除。
最小修复：把共享 live 检查纳入 find_map 的候选过滤，让失效记录被跳过后继续扫描；不要再增加名称表或为旧 ID 创建特别 fallback。当前新测试只覆盖关闭后的第一次重开，需追加“替代窗口再次复用”的代理相等和 Document 保留断言，并覆盖旧死记录先被扫描的顺序。
收敛结论
本轮只保留以上 1 项确定遗漏，其根因是存活筛选发生在选中候选之后。现有共享身份、组关系、稳定选择回执和全 alias 生命周期方案均可保留，不需要继续扩展架构。异步 Document/classic script 回执已经过共享 live 校验，不能据这个查找问题泛化出额外的已证实生命周期漏洞。
表单 source-first 的基线旧缺口此前已独立说明，本轮不重复计为新缺陷。发布前 GitHub 对当前 HEAD 返回空 statusCheckRollup；PR 描述的本地验证是作者报告，与本轮独立代码审查分开。
a9858cc to
2a717a8
Compare
lanyue-llk
changed the title
2a717a8 to
960918c
Compare
960918c to
6c703ee
Compare
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
复审 6c703eed2c2814976a5872084d7bfdee2c830b65，本轮实际 base 已变为 1bb1325a55a9c19fbb71f1a9f6a83c3644f57831。已按重 base 后的两条 exclusive commits 分清 PR 与上游变化，三路并行复核后一次性汇总。
没有修改代码、编译或重跑全量测试。证据包括当前代码、这个 HEAD 的 GitHub CI 原始日志，以及独立 Chrome 最小对照。
上轮的关闭旧记录遮挡问题已修复：共享 live 判断现在处于 find_map 候选条件中，失效记录会被跳过，继续寻找活的同名替代对象。名称共享、调用时选定身份、组隔离、noopener、关闭退休和 Classic 多 alias 链路没有发现需要推翻的新问题。
本轮仍有 2 项确定问题：
1. [P1] 新增 alias 字段没有补全重 base 后的消费者，CI 无法编译测试
定位：moli-protocol/src/devtools_runtime.rs:1584 新增的必填字段 moli_popup_alias_ids，以及以下 11 个遗漏的结构体构造：
moli-protocol-webdriver-classic/src/tests.rs：3240、3252、3264、3276。moli-protocol-webdriver-bidi/src/tests.rs：59、74。moli-protocol-webdriver-bidi/src/tests/log_and_runtime_events.rs：902、922、934、1073、1178。
本 HEAD 的 Clippy 日志 和 Nextest 日志 均明确报 E0063: missing field moli_popup_alias_ids。Nextest 停在编译阶段，不是执行后出现几条基线失败。实际 base 的 Clippy/Nextest 均成功，不能沿用此前“base APIs 不同步”的归因。
最小修复：为没有 popup alias 的这些 fixture 补空向量，按语义保留其它字段；检索整个 workspace 的 DevToolsTargetInfo 构造，避免只修当前三个包。随后恢复仓库规定的 workspace 检查。无需修改公共类型设计，也不应跳过或删除这些测试。PR 中 33/29 个 focused tests 的成功不能代替 workspace 消费者已集成。
2. [P2] 新的 child 特殊目标快速路径仍从网页可写属性解析身份
定位：moli-renderer-v8/src/native_bridge/element/activation/targets.rs:351–364，尤其调用 child_window_object_slot 的分支。
该路径看似读取内部 slot，实际上 child_frame_runtime/window.rs:1283 的 helper 是普通 window.get(string)；helpers/object_slots.rs 仅以 DONT_ENUM 定义这些属性，未使其只读或私有。网页可以修改 __moliWindowParent 和 __moliWindowTop：
const w = frame.contentWindow; // 同源 iframe
const fake = { tag: 1 };
w.__moliWindowParent = fake;
w.__moliWindowTop = fake;
[w.open('', '_parent') === fake, w.open('', '_top') === fake];当前新快速路径会读到 fake 并提前返回，尚未进入稳定 owner/child 关系解析。这是之前“公开 parent 属性不能充当 browsing-context 身份”的同类回归，换了属性名仍未建立正确边界；用 getter 替换还会执行网页脚本。
本轮 Chrome 154.0.8037.58 最小对照：两个自定义属性均可写，但 _parent、_top 仍返回真实父/顶层窗口，忽略 fake。Moli 侧结论来自确定代码路径，不冒称本地运行过新版 Moli。
最小修复：直接从稳定的 child_window_proxy_records/owner 图，或真正的 V8 private identity 读取父/顶层关系，保留本次正确的 receiver 归属修复；不要再增加一个可写的 __moli... 名称替代。回归应覆盖顶层/child/popup 的借用调用、空与非空 URL，以及这两个属性被替换为对象/有副作用 getter 时仍选中真实上下文。
CI 还有一条独立失败，不能混作已定位代码缺陷
本 HEAD 的 CDP smoke 为 49/50：classic-scrollbar 在首次 geometry evaluate 处超时 120 秒；实际 base 的 同项 CI 成功。相关 smoke/layout/geometry 实现与 base 无差异，当前日志不足以判定是偶发调度还是 PR 间接回归。因此不能把它称为已确认的“base 失败”，也不把未定位的超时捏造成第三个代码 finding；修复编译后应单独复查该 smoke，而不是忽略红灯。
收敛结论
已完成窗口身份、名称、目标选择时序、live registry、关闭重开、alias 消费者和入口测试的本轮交叉检查。保持现有正确架构，集中补齐 workspace 类型消费者，并让 child 特殊目标解析真正使用不可由网页改写的身份数据。其它上游重 base 改动与历史表单入口缺口不重复计为本 PR 新回归。
|
已处理 review
验证：workspace 全量 Clippy 通过；Classic/BiDi all-targets 编译通过；child、popup、top-level 三个特殊目标回归通过。CI 中独立的 |
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
针对提交 3bf68f4f6cf63cf741d26a1e49e7c2f666a4a67d，以实际 PR base 1bb1325a55a9c19fbb71f1a9f6a83c3644f57831 为基线，完成三路并行只读复审。本轮收敛为 3 个代码根因，不以 CI 状态作为结论门槛。
-
[P1] 跨源 iframe 创建 about:blank 弹窗时误用顶层存储身份。
dialogs.rs:222的普通 popup 路径仍通过entered_owner_dispatch_scope获取 source；它依赖的 entered child marker 在当前仓库只有读取，没有设置，普通 child realm 会落为 Top。与此同时，opener_child_handle已正确识别 child，但popups.rs:792–793新增的initiator.or(opener_endpoint)让错误的 Top 覆盖正确的 Child，最终选择top_web_storage_scope()。现有跨源场景已观测到 popup 的 localStorage 读出父页面的 "top"，而应为隔离区域的 null。这是存储隔离错误；实际基线根据正确的 opener_child_handle 选择 child 存储区域。建议统一从真实 execution-context 身份解析 creator，并贯穿 target selection、storage 与 activation；不要仅交换 or 的顺序，因为 noopener 没有 opener 仍需要正确 creator。 回归应覆盖跨源 child 创建普通/无 opener 弹窗、初始读取及写入后父/子存储均不受影响。 -
[P2] 内部 _parent/_top 选择返回裸 Top global，丢失合法跨域接口。 对直接隶属于顶层的跨源 iframe，新
targets.rs:371–372经 root helper 返回top_context.global()。安全检查仍拒绝读取 document，但该对象的跨域 handler（child_frame_runtime/window.rs:2133–2140）只为 Child 身份提供受限 surface，Top 返回 None，因而const w = open('', '_top'); w.postMessage('probe', '*')的合法访问也被拒绝；该场景的 _parent 同样受影响。非空 URL 设置 location 也经过这个对象，有相同的接口缺失。实际基线使用 child realm 的跨域 projection。独立 Chrome 154 对照已验证 parent/top 身份正确、两次 postMessage 均送达，同时 document 仍抛 SecurityError。建议内部关系只负责选定目标，再按访问方 realm 投影成适当的 WindowProxy；不要恢复读取页面可写属性。 测试应同时断言合法通信/允许条件下导航成功和禁止读取 document，并区分一级与嵌套 frame。 -
[P2] 严格 Window 品牌检查漏掉正式原生 Window wrapper。
window_receiver.rs:37–43收紧识别后，__moliNativeBridge.window返回的 reflected Window 不属于 popup/child/navigator/global 四类，导致合法 accessor 被bound_callback_data_item拒绝，抛出 "Window getter called on incompatible receiver"。该对象由正式 Window 模板创建并携带 host pointer、reflector ID（native_bridge/window.rs:6–19、bindings.rs），不是任意伪造对象；现有 native bridge 的 window/location 等调用场景也因此中断。建议用确切模板/reflector 类型或原生私有身份识别合法 wrapper，同时保留 NodeList、NodeIterator 等非法借用的拒绝。 不应恢复“有 host pointer 就算 Window”的宽松判断，也不应删除合法 bridge 场景来迁就实现。
需求与方案复核：PR 现在已清楚区分名称持久化、同组复用、调用时目标身份、空 URL、noopener、alias 映射与关闭回收。共享上下文状态、稳定 ID、关闭时统一回收的方向合理；剩余问题集中在 creator 身份、目标投影和合法 receiver 的边界，宜修复这三处统一抽象，不再增加针对名称或单个失败案例的分支。上一轮 11 处构造缺字段和可写 parent/top alias 选择问题已修复，不重复计数。
本轮未修改仓库代码，未本地编译或运行全量测试。依据实际基线差异、调用链、已有场景运行证据及独立 Chrome 对照作出以上判断；单纯 CI 状态和新增测试的任务推进时序问题未列为代码 finding。
|
已处理对
另修正 修复提交： 验证：完整 workspace Nextest 19,139 passed / 14 skipped / 0 failed；完整 all-targets/all-features Clippy ( 正常推送保留 #640 → #731 → #853 的依赖链；本 PR 在 #731 之上有五个线性提交，无 merge commit。没有重跑 WebArena case，也没有放宽超时或评分标准。 |
lanyue-llk
left a comment
lanyue-llk
left a comment
There was a problem hiding this comment.
复审提交 e8f6894aae7163246aaaf409a6a7bcb3ad2d7a7e，比较实际 PR base 1bb1325a55a9c19fbb71f1a9f6a83c3644f57831，并完整核对相对上一轮的修改。三路并行复查完成，本轮未发现新增的确定代码缺陷，上轮三个 finding 均可关闭：
- Creator/storage 身份：
entered_owner_dispatch_scope在动态 marker 缺失时改用已注册的执行上下文身份。普通和嵌套 child 不再被误判为 Top；存储、资源 authority、activation source 沿用同一 owner，Document URL/CSP 也同步修正。noopener 保留独立 creator，未用 opener 替代 creator。新增回归既观察普通 popup 的存储隔离，也检查无 opener 的初始存储身份，能检出上一版根因。 - 跨域 parent/top 投影：内部目标解析复用既有 realm projection，移除了页面可写 alias 的 fallback，同时保留允许的 postMessage 接口及 document 访问限制。新增直接/嵌套 frame 场景验证 parent 与 top 的身份区别、合法通信以及跨域读取拒绝，覆盖了上一版裸 Top global 的问题。
- 原生 Window 品牌：私有 marker 只在 genuine Window wrapper 创建路径写入，跨 realm 可识别，页面普通属性不能伪造。合法 bridge Window 不再被误拒绝；NodeList/NodeIterator 等其它 host 对象没有重新获得 Window 品牌，detached global 的 fallback 也保留。
方案评价：PR 已清楚说明名称持久化、同组复用、调用时选择、空 URL、noopener、alias 和关闭回收等需求。当前修复复用了执行上下文身份、WindowProxy 投影和 wrapper 创建边界，没有用交换 fallback 顺序、放宽所有 host 对象或针对特定窗口名增加分支来绕过问题。共享名称/存活状态、异步 selection receipt、alias 到协议 handle 的映射与关闭退休链也已复核，此前修复仍保留。
另核查了 popup 内嵌套跨源 frame 的 TopWindow 投影局限：完整对照基线的 slot 同步、realm 初始化、公开 top getter 和 Window.open 选择路径，确认该行为在实际 base 已存在，未作为本 PR 新 finding，也不据中间版本的变化归责当前提交。
本轮只读审查，已同步最新 main，未修改项目代码、未本地编译或运行全量测试，未跟进 CI。结论依据当前代码、实际基线差异、完整调用链和回归场景设计；不等同于证明所有可能输入均无缺陷。
|
测试现在按 target 比较，仍保留消息数量、每条消息的窗口身份、合法通信和跨域拒绝读取的完整断言。没有修改生产实现，也没有放宽超时、权限或评测条件。 完整本地门禁：Nextest 19,139 passed / 14 skipped / 0 failed；workspace all-targets/all-features Clippy、format 和 diff check 均通过。正常推送新增一个线性测试提交，未改写其它 PR 分支。 |
80e49d7 to
6602367
Compare
6602367 to
9ae23bd
Compare
lanyue-llk
changed the title
1 participant
Purpose
Preserve named-window behavior on the current browser-context architecture. Window operations use main's context names, window groups, auxiliary Page lifecycle and protocol aliases.
Overall design
Full-size overview · All current PR diagrams
Implementation flow (Mermaid)
flowchart TD A["Window operation or named hyperlink"] --> B["Native receiver and source context"] B --> S["Select target in the source window group"] S --> X["Reuse eligible source, frame or popup"] S --> Y["Create through existing auxiliary Page owner"] X --> N["Navigate only when URL is nonempty"] Y --> N N --> C["Keep creator origin, resources and storage authority"] C --> E{"Expose opener and WindowProxy?"} E -->|"noopener"| Z["Return null"] E -->|"ordinary open"| W["Project existing realm and protocol aliases"] W --> L["Close and retire aliases through existing lifecycle"]Scope and ownership
_parent/_topidentity.Target selection and opener exposure
flowchart LR S["Selected eligible named context"] --> U{"URL empty?"} U -->|"yes"| K["Keep current document"] U -->|"no"| N["Navigate selected context"] K --> O{"noopener?"} N --> O O -->|"yes"| Z["Return null"] O -->|"no"| W["Return selected WindowProxy"]An empty URL leaves the selected document intact. Opener suppression controls exposure after eligible target selection; creator authority follows its own existing security and storage rules.
Dependencies
main → #640 → #731 → #853. This PR targets #731; head9ae23bd61. Its remaining changes form one commit on #731, with all 19 original named-window regression scenarios retained.Reproduce affected cases
Local fixtures and expected behavior
Each case below is an existing regression test at this PR's pinned head. The fixtures create their own pages and local services; Rainbond, WebArena credentials and model calls are unnecessary. Follow the repository's Rust toolchain and install cargo-nextest before running. Initial compilation is still required; the filter runs only the listed cases.
window_name_accessors_reject_non_window_receivers_without_mutating_the_windowwindow_name_accessors_follow_the_receiver_realm_and_preserve_state_on_conversion_errorswindow_open_empty_parent_uses_internal_identity_without_running_public_lookupborrowed_popup_open_parent_and_top_use_the_popup_receiver_identitywindow_open_resolves_relative_urls_against_the_entry_function_realmwindow_open_empty_url_selects_named_source_without_navigationwindow_open_noopener_reuses_a_related_named_popup_but_returns_nullwindow_open_does_not_reuse_named_noopener_popup_from_an_isolated_groupnoopener_named_hyperlink_reuses_an_existing_iframewebdriver_classic_named_popup_reuse_navigates_existing_windowwebdriver_classic_close_retires_popup_aliases_before_named_reopenExpected result: all selected tests pass. To run one case, replace the expression after
-Ewithtest(the_case_name). Test failures print the expected/actual assertion. These are regression case identifiers, with inputs and assertions linked to the exact source version.The receiver, alias and window-group cases include issues found during semantic review, beyond the original trajectory failures. They have no assigned WebArena task number. Named-popup navigation uses same-origin HTTP pages, so rename and reuse exercise permitted access rather than failing on opaque-origin data URLs.
Evaluation reports
Current-head validation
CI report for #853: all 19 applicable checks passed, including Nextest, CDP/WebDriver, the 1,020-case frontend differential, 545-page WebMainBench, navigation soak and release comparisons.
Local cumulative validation: formatting and all-feature Clippy passed; 19,809 tests passed, 14 ignored. Published source tree
ae5c31ad7da9bc6ca8c24413d0669893350ad6a3matches the tested tree.Independent Chromium controls cover same-origin popup rename/reuse, cross-origin access rejection, and native parent/top selection when page getters are replaced.