From 806497c02b798b720ce4e258d5e230001a25fe47 Mon Sep 17 00:00:00 2001 From: Pierre Tachoire Date: Tue, 12 May 2026 14:18:24 +0200 Subject: [PATCH 1/3] fix(cdp): remove duplicate Page.frameNavigated and fix context registration for iframes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The frameNavigated CDP handler sent Page.frameNavigated twice per navigation and always used the root frame's V8 context for inspector.contextCreated, even during iframe navigations. This caused the root frame's inspector context id to be silently re-registered (and the previous id invalidated) whenever an iframe navigated. The duplicate Page.frameNavigated masked this bug — Puppeteer happened to pick up the re-registered context id. Removing the duplicate exposed the underlying issue: callFunctionOn with the original id failed with "Cannot find context with specified id". --- src/cdp/domains/page.zig | 57 +++++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 27 deletions(-) diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index a06a2908..92ce7e95 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -624,7 +624,8 @@ pub fn frameNavigated(arena: Allocator, bc: *CDP.BrowserContext, event: *const N }, .{ .session_id = session_id }); } - const frame = bc.session.currentFrame() orelse return error.FrameNotLoaded; + const root_frame = bc.session.currentFrame() orelse return error.FrameNotLoaded; + const is_root_frame = event.frame_id == root_frame._frame_id; // When we actually recreated the context we should have the inspector send // this event, see: resetContextGroup. Sending this event will tell the @@ -635,10 +636,17 @@ pub fn frameNavigated(arena: Allocator, bc: *CDP.BrowserContext, event: *const N // frames (iframes), clearing all contexts would destroy the main frame's // context, causing Puppeteer's frame.evaluate()/frame.content() to hang // forever. - if (event.frame_id == frame._frame_id) { + if (is_root_frame) { try cdp.sendEvent("Runtime.executionContextsCleared", null, .{ .session_id = session_id }); } + // Look up the actual navigated frame. For main frame navigations this is + // the root frame; for iframes it is the child frame. Using the correct + // frame's JS context for inspector.contextCreated prevents re-registering + // the root context under a new id (which silently invalidates the + // previous id on the V8 side). + const frame = bc.session.findFrameByFrameId(event.frame_id) orelse root_frame; + // frameNavigated event try cdp.sendEvent("Page.frameNavigated", .{ .type = "Navigation", @@ -666,22 +674,29 @@ pub fn frameNavigated(arena: Allocator, bc: *CDP.BrowserContext, event: *const N true, ); } - for (bc.isolated_worlds.items) |isolated_world| { - const aux_json = try std.fmt.allocPrint(arena, "{{\"isDefault\":false,\"type\":\"isolated\",\"frameId\":\"{s}\",\"loaderId\":\"{s}\"}}", .{ frame_id, loader_id }); + // Isolated worlds are session-wide (single V8 context shared across + // navigations). Only re-register them for main frame navigations; + // re-registering during child frame (iframe) navigations would + // re-register the same V8 context under a new inspector id, silently + // invalidating the id the main frame is using. + if (is_root_frame) { + for (bc.isolated_worlds.items) |isolated_world| { + const aux_json = try std.fmt.allocPrint(arena, "{{\"isDefault\":false,\"type\":\"isolated\",\"frameId\":\"{s}\",\"loaderId\":\"{s}\"}}", .{ frame_id, loader_id }); - // Calling contextCreated will assign a new Id to the context and send the contextCreated event + // Calling contextCreated will assign a new Id to the context and send the contextCreated event - var ls: js.Local.Scope = undefined; - (isolated_world.context orelse continue).localScope(&ls); - defer ls.deinit(); + var ls: js.Local.Scope = undefined; + (isolated_world.context orelse continue).localScope(&ls); + defer ls.deinit(); - bc.inspector_session.inspector.contextCreated( - &ls.local, - isolated_world.name, - "://", - aux_json, - false, - ); + bc.inspector_session.inspector.contextCreated( + &ls.local, + isolated_world.name, + "://", + aux_json, + false, + ); + } } // Evaluate scripts registered via Page.addScriptToEvaluateOnNewDocument. @@ -705,18 +720,6 @@ pub fn frameNavigated(arena: Allocator, bc: *CDP.BrowserContext, event: *const N } } - // frameNavigated event - try cdp.sendEvent("Page.frameNavigated", .{ - .type = "Navigation", - .frame = CDPFrame{ - .id = frame_id, - .url = event.url, - .loaderId = loader_id, - .securityOrigin = bc.security_origin, - .secureContextType = bc.secure_context_type, - }, - }, .{ .session_id = session_id }); - // The DOM.documentUpdated event must be send after the frameNavigated one. // chromedp client expects to receive the events is this order. // see https://github.com/chromedp/chromedp/issues/1558 From 8432cfbfbafb86ae85393cda3ac7a81fa77be896 Mon Sep 17 00:00:00 2001 From: Pierre Tachoire Date: Wed, 13 May 2026 12:29:11 +0200 Subject: [PATCH 2/3] cdp: return error in case of missing event's frame Instead of using the root_frame --- src/cdp/domains/page.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index 92ce7e95..708c9d00 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -645,7 +645,7 @@ pub fn frameNavigated(arena: Allocator, bc: *CDP.BrowserContext, event: *const N // frame's JS context for inspector.contextCreated prevents re-registering // the root context under a new id (which silently invalidates the // previous id on the V8 side). - const frame = bc.session.findFrameByFrameId(event.frame_id) orelse root_frame; + const frame = bc.session.findFrameByFrameId(event.frame_id) orelse return error.FrameNotFound; // frameNavigated event try cdp.sendEvent("Page.frameNavigated", .{ From 5d73d82bf64691bac59f0e4f72e4ee3eb3f3f341 Mon Sep 17 00:00:00 2001 From: Pierre Tachoire Date: Wed, 13 May 2026 14:11:53 +0200 Subject: [PATCH 3/3] cdp: call context created w/ correct is_default_context value Co-authored-by: Navid EMAD --- src/cdp/domains/page.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index 708c9d00..bb0274e1 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -671,7 +671,7 @@ pub fn frameNavigated(arena: Allocator, bc: *CDP.BrowserContext, event: *const N "", frame.origin orelse "", aux_data, - true, + is_root_frame, ); } // Isolated worlds are session-wide (single V8 context shared across