fix(cdp): prune valid_context_ids on executionContextsCleared (#413)
Runtime.executionContextsCleared on navigation told clients every prior context was gone, but valid_context_ids was insert-only, so a Runtime.callFunctionOn targeting a pre-navigation context id still ran instead of being rejected with Chrome's 'Cannot find context with specified id'. On a long-lived CDP connection the set also grew by one id per navigation and was never pruned. Clear valid_context_ids when executionContextsCleared is emitted, then re-register the default world (id 2) and the isolated worlds as their executionContextCreated events are emitted. Closes #407.
This commit is contained in:
1 parent
5ada3b404b
commit
358100aa63
2 files changed
+79
No files matched your search
@@ -73,12 +73,22 @@ pub fn emit_navigation_events(
|
||||
});
|
||||
}
|
||||
|
||||
// executionContextsCleared invalidates every prior context id, so a
|
||||
// Runtime.evaluate / callFunctionOn targeting a pre-navigation context
|
||||
// must be rejected (Chrome: "Cannot find context with specified id"). The
|
||||
// default world (id 2) and isolated worlds are re-registered below as their
|
||||
// executionContextCreated events are emitted. Issue #407: previously this
|
||||
// set was insert-only, so stale ids kept validating and grew unbounded.
|
||||
ctx.valid_context_ids.clear();
|
||||
let mut phase1 = vec![
|
||||
CdpEvent { method: "Page.lifecycleEvent".into(), params: json!({"frameId": frame_id, "loaderId": loader_id, "name": "init", "timestamp": ts}), session_id: es.clone() },
|
||||
CdpEvent { method: "Runtime.executionContextsCleared".into(), params: json!({}), session_id: es.clone() },
|
||||
CdpEvent { method: "Page.frameNavigated".into(), params: json!({"frame": {"id": frame_id, "loaderId": loader_id, "url": page_url, "domainAndRegistry": "", "securityOrigin": page_url, "mimeType": nav_mime, "adFrameStatus": {"adFrameType": "none"}}, "type": "Navigation"}), session_id: es.clone() },
|
||||
CdpEvent { method: "Runtime.executionContextCreated".into(), params: json!({"context": {"id": 2, "origin": page_url, "name": "", "uniqueId": format!("ctx-nav-{}", page_id), "auxData": {"isDefault": true, "type": "default", "frameId": frame_id}}}), session_id: es.clone() },
|
||||
];
|
||||
// The default world is re-created as context id 2; re-register it. Isolated
|
||||
// worlds register themselves via next_isolated_context in the loop below.
|
||||
ctx.valid_context_ids.insert(2);
|
||||
let world_names: Vec<String> = if ctx.isolated_worlds.is_empty() {
|
||||
vec!["__puppeteer_utility_world__24.40.0".to_string()]
|
||||
} else {
|
||||
|
||||
@@ -0,0 +1,69 @@
|
||||
// Regression for issue #407: `CdpContext.valid_context_ids` was insert-only,
|
||||
// so `Runtime.executionContextsCleared` on navigation never pruned the old
|
||||
// context ids. A `Runtime.callFunctionOn` targeting a pre-navigation context
|
||||
// still ran (Chrome rejects it with "Cannot find context with specified id"),
|
||||
// and the set grew by one id per navigation on a long-lived connection.
|
||||
|
||||
use obscura_cdp::dispatch::CdpContext;
|
||||
use obscura_cdp::domains::page::emit_navigation_events;
|
||||
use obscura_browser::lifecycle::WaitUntil;
|
||||
|
||||
fn navigate(ctx: &mut CdpContext, page_id: &str) {
|
||||
emit_navigation_events(
|
||||
ctx,
|
||||
&Some("session-1".to_string()),
|
||||
"frame-1",
|
||||
"loader-1",
|
||||
"http://127.0.0.1/page",
|
||||
page_id,
|
||||
&[],
|
||||
WaitUntil::Load,
|
||||
true,
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn navigation_prunes_stale_execution_context_ids() {
|
||||
let mut ctx = CdpContext::new();
|
||||
// Seed a stale id that no navigation re-creates (simulates a context from
|
||||
// a prior page state, or simply the pre-navigation main id 1).
|
||||
ctx.valid_context_ids.insert(999);
|
||||
|
||||
navigate(&mut ctx, "page-1");
|
||||
|
||||
// The stale id is gone after executionContextsCleared; the default world
|
||||
// (id 2) and the first isolated world (id 100, counter starts at 100) remain.
|
||||
assert!(
|
||||
!ctx.valid_context_ids.contains(&999),
|
||||
"stale execution context id must be pruned on navigation"
|
||||
);
|
||||
assert!(ctx.valid_context_ids.contains(&2), "default world id 2 must be re-registered");
|
||||
assert!(
|
||||
ctx.valid_context_ids.contains(&100),
|
||||
"isolated world id 100 must be registered: {:?}",
|
||||
ctx.valid_context_ids
|
||||
);
|
||||
|
||||
navigate(&mut ctx, "page-1");
|
||||
|
||||
// Second navigation: the first nav's isolated id (100) is now stale, and a
|
||||
// fresh id (101) takes its place. The set must not grow across navigations.
|
||||
assert!(
|
||||
!ctx.valid_context_ids.contains(&100),
|
||||
"previous navigation's isolated context id must be pruned"
|
||||
);
|
||||
assert!(ctx.valid_context_ids.contains(&101), "fresh isolated id 101 must be registered");
|
||||
assert!(ctx.valid_context_ids.contains(&2), "default world id 2 must survive navigation");
|
||||
|
||||
// Unbounded-growth check: two navigations leave exactly the default world
|
||||
// plus the current isolated world(s), not an accumulating union.
|
||||
let count_after_two_navs = ctx.valid_context_ids.len();
|
||||
navigate(&mut ctx, "page-1");
|
||||
navigate(&mut ctx, "page-1");
|
||||
assert_eq!(
|
||||
ctx.valid_context_ids.len(),
|
||||
count_after_two_navs,
|
||||
"valid_context_ids must not grow across navigations (was {count_after_two_navs}, now {})",
|
||||
ctx.valid_context_ids.len()
|
||||
);
|
||||
}
|
||||
Reference in new issue
Block a user