From 736be5b35b7d40243754b894cc5bbfa0e6846049 Mon Sep 17 00:00:00 2001 From: nikneym Date: Thu, 17 Sep 2026 16:08:30 +0300 Subject: [PATCH 1/4] `cdp`: answer inspector commands on the session that sent them Playwright's `browserContext.newCDPSession` must get its response on that session, not the primary one. Playwright keys its pending callbacks by session, finds none for that id, and throws "Assertion error", which takes the whole process down. Addresses #1838 and #1839. --- src/server/cdp/CDP.zig | 41 ++++++++++++++++++++++-------- src/server/cdp/domains/runtime.zig | 28 +++++++++++++++++++- 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/src/server/cdp/CDP.zig b/src/server/cdp/CDP.zig index cf1dd2e84..93052b35b 100644 --- a/src/server/cdp/CDP.zig +++ b/src/server/cdp/CDP.zig @@ -470,6 +470,7 @@ pub const BrowserContext = struct { // entries evicted by resetFrame can linger harmlessly until reset. set_child_nodes_sent: std.AutoHashMapUnmanaged(NodeRegistry.Id, void) = .empty, + inspector_call_sessions: std.AutoHashMapUnmanaged(i64, []const u8) = .empty, inspector_session: *js.Inspector.Session, isolated_worlds: std.ArrayList(*IsolatedWorld), @@ -622,6 +623,7 @@ pub const BrowserContext = struct { self.node_registry.deinit(); self.node_search_list.deinit(); self.set_child_nodes_sent.deinit(self.cdp.allocator); + self.inspector_call_sessions.deinit(self.cdp.allocator); // Session.deinit (called via closeSession above) already cleared this // notification off any ownerless CorsGate/RobotsGate transfers. @@ -1174,13 +1176,31 @@ pub const BrowserContext = struct { } } - pub fn callInspector(self: *const BrowserContext, msg: []const u8) void { - self.inspector_session.send(msg); + /// Forwards `cmd`'s raw JSON to the V8 inspector, which answers through onInspectorResponse. + pub fn callInspector(self: *BrowserContext, cmd: *const Command) !void { + try self.trackInspectorCall(cmd); + self.inspector_session.send(cmd.input.json); self.session.browser.env.runMicrotasks(); } - pub fn onInspectorResponse(ctx: *anyopaque, _: u32, msg: []const u8) void { - sendInspectorMessage(@ptrCast(@alignCast(ctx)), msg) catch |err| { + // Remembers which non-primary session `cmd` came from, so that its response + // can be stamped with that session. + fn trackInspectorCall(self: *BrowserContext, cmd: *const Command) !void { + const id = cmd.input.id orelse return; + const input_session_id = cmd.input.session_id orelse return; + if (self.session_id) |primary| { + if (std.mem.eql(u8, primary, input_session_id)) { + return; + } + } + const session_id = self.cdp.resolveSessionId(input_session_id) orelse return; + try self.inspector_call_sessions.put(self.cdp.allocator, id, session_id); + } + + pub fn onInspectorResponse(ctx: *anyopaque, call_id: u32, msg: []const u8) void { + const self: *BrowserContext = @ptrCast(@alignCast(ctx)); + const session_id = self.inspector_call_sessions.fetchRemove(@intCast(call_id)); + sendInspectorMessage(self, msg, if (session_id) |kv| kv.value else null) catch |err| { log.err(.cdp, "send inspector response", .{ .err = err }); }; } @@ -1197,16 +1217,17 @@ pub const BrowserContext = struct { log.debug(.cdp, "inspector event", .{ .method = method }); } - sendInspectorMessage(@ptrCast(@alignCast(ctx)), msg) catch |err| { + sendInspectorMessage(@ptrCast(@alignCast(ctx)), msg, null) catch |err| { log.err(.cdp, "send inspector event", .{ .err = err }); }; } - // This is hacky x 2. First, we create the JSON payload by gluing our - // session_id onto it. Second, we're much more client/websocket aware than - // we should be. - fn sendInspectorMessage(self: *BrowserContext, msg: []const u8) !void { - const session_id = self.session_id orelse { + // This is hacky x 2. First, we create the JSON payload by gluing a + // session_id onto it: `explicit_session_id` (the session a response + // belongs to) or else the primary session (all events). Second, we're + // much more client/websocket aware than we should be. + fn sendInspectorMessage(self: *BrowserContext, msg: []const u8, explicit_session_id: ?[]const u8) !void { + const session_id = explicit_session_id orelse self.session_id orelse { // We no longer have an active session. What should we do // in this case? return; diff --git a/src/server/cdp/domains/runtime.zig b/src/server/cdp/domains/runtime.zig index e36f1466e..8e4d4df5f 100644 --- a/src/server/cdp/domains/runtime.zig +++ b/src/server/cdp/domains/runtime.zig @@ -80,7 +80,7 @@ fn sendInspector(cmd: *CDP.Command) !void { const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; // the result to return is handled directly by the inspector. - bc.callInspector(cmd.input.json); + try bc.callInspector(cmd); } // Object arguments stay remote handles; serializing them would execute page JS. @@ -180,6 +180,32 @@ test "cdp.runtime: inspector-handled methods pass through" { try ctx.expectSentResult(null, .{ .id = 52 }); } +// Playwright's browserContext.newCDPSession attaches a second session to the +// page and sends Runtime commands through it. The inspector's answer must +// carry that session's id: the driver keys its pending callbacks by session +// and asserts on a response it can't match (lightpanda-io/browser#1838). +test "cdp.runtime: inspector responses go to the session that sent the command" { + var ctx = try testing.context(); + defer ctx.deinit(); + + const bc = try ctx.loadBrowserContext(.{ .id = "BID-RT2", .url = "hi.html", .target_id = "FID-0000000RT2".*, .session_id = "SID-PRIMARY" }); + try bc.attached_sessions.append(bc.arena, .{ .id = "SID-AUX", .parent_id = null }); + + try ctx.processMessage(.{ .id = 60, .method = "Runtime.evaluate", .sessionId = "SID-AUX", .params = .{ .expression = "1 + 1", .returnByValue = true } }); + try ctx.expectSentResult(.{ .result = .{ .type = "number", .value = 2, .description = "2" } }, .{ .id = 60, .session_id = "SID-AUX" }); + try testing.expectEqual(0, bc.inspector_call_sessions.count()); + + // The primary session keeps working as before. + try ctx.processMessage(.{ .id = 61, .method = "Runtime.evaluate", .sessionId = "SID-PRIMARY", .params = .{ .expression = "2 + 2", .returnByValue = true } }); + try ctx.expectSentResult(.{ .result = .{ .type = "number", .value = 4, .description = "4" } }, .{ .id = 61, .session_id = "SID-PRIMARY" }); + + // A response the inspector produces after the dispatch (awaitPromise + // answers from a microtask) still finds its session. + try ctx.processMessage(.{ .id = 62, .method = "Runtime.evaluate", .sessionId = "SID-AUX", .params = .{ .expression = "Promise.resolve('late')", .awaitPromise = true, .returnByValue = true } }); + try ctx.expectSentResult(.{ .result = .{ .type = "string", .value = "late" } }, .{ .id = 62, .session_id = "SID-AUX" }); + try testing.expectEqual(0, bc.inspector_call_sessions.count()); +} + test "cdp.runtime: consoleAPICalled type matches the console method" { testing.silenceLog(&.{.js}); From 56cb4205aeb32a4b2f52361a1c444903394c41b0 Mon Sep 17 00:00:00 2001 From: nikneym Date: Mon, 21 Sep 2026 12:16:26 +0300 Subject: [PATCH 2/4] `cdp`: sessions own their inspector session `Inspector` now supports several sessions; `startSession` allocates one (the V8 channel keeps its address) and `stopSession` frees it. They all connect to the same context group so every session see every context. `BrowserContext.session_id` stays the primary session's id for the many event call sites. --- src/browser/js/Env.zig | 2 +- src/browser/js/Inspector.zig | 44 +++-- src/server/cdp/CDP.zig | 225 +++++++++++++---------- src/server/cdp/domains/accessibility.zig | 4 +- src/server/cdp/domains/dom.zig | 66 +++++-- src/server/cdp/domains/fetch.zig | 5 +- src/server/cdp/domains/page.zig | 20 +- src/server/cdp/domains/runtime.zig | 58 +++++- src/server/cdp/domains/target.zig | 97 +++++----- src/server/cdp/testing.zig | 4 +- 10 files changed, 332 insertions(+), 193 deletions(-) diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index 799ed3b9f..ef43ba993 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -245,7 +245,7 @@ pub fn deinit(self: *Env) void { const allocator = app.allocator; if (self.inspector) |i| { - i.deinit(allocator); + i.deinit(); } allocator.free(self.templates); diff --git a/src/browser/js/Inspector.zig b/src/browser/js/Inspector.zig index 5d176095f..22c876f7d 100644 --- a/src/browser/js/Inspector.zig +++ b/src/browser/js/Inspector.zig @@ -37,24 +37,29 @@ const CLIENT_TRUST_LEVEL = 1; // (not much at all) const Inspector = @This(); +allocator: Allocator, unique_id: i64, isolate: *v8.Isolate, handle: *v8.Inspector, client: *v8.InspectorClientImpl, default_context: ?v8.Global, -session: ?Session, +/// One per CDP session attached to the page target; all connect to the same +/// `CONTEXT_GROUP_ID`, so every session sees every context. Heap allocated +/// because the V8 channel keeps the session's address (SET_DATA). +sessions: std.ArrayListUnmanaged(*Session), pub fn init(allocator: Allocator, isolate: *v8.Isolate) !*Inspector { const self = try allocator.create(Inspector); errdefer allocator.destroy(self); self.* = .{ + .allocator = allocator, .unique_id = 1, - .session = null, .isolate = isolate, .client = undefined, .handle = undefined, .default_context = null, + .sessions = .empty, }; self.client = v8.v8_inspector__Client__IMPL__CREATE(); @@ -67,32 +72,39 @@ pub fn init(allocator: Allocator, isolate: *v8.Isolate) !*Inspector { return self; } -pub fn deinit(self: *const Inspector, allocator: Allocator) void { +pub fn deinit(self: *Inspector) void { var hs: v8.HandleScope = undefined; v8.v8__HandleScope__CONSTRUCT(&hs, self.isolate); defer v8.v8__HandleScope__DESTRUCT(&hs); - if (self.session) |*s| { - s.deinit(); + for (self.sessions.items) |session| { + session.deinit(); + self.allocator.destroy(session); } + self.sessions.deinit(self.allocator); + v8.v8_inspector__Client__IMPL__DELETE(self.client); v8.v8_inspector__Inspector__DELETE(self.handle); - allocator.destroy(self); + self.allocator.destroy(self); } -pub fn startSession(self: *Inspector, ctx: anytype) *Session { - if (comptime lp.IS_DEBUG) { - std.debug.assert(self.session == null); - } +pub fn startSession(self: *Inspector, ctx: anytype) !*Session { + const session = try self.allocator.create(Session); + errdefer self.allocator.destroy(session); - self.session = @as(Session, undefined); - Session.init(&self.session.?, self, ctx); - return &self.session.?; + Session.init(session, self, ctx); + errdefer session.deinit(); + + try self.sessions.append(self.allocator, session); + return session; } -pub fn stopSession(self: *Inspector) void { - self.session.?.deinit(); - self.session = null; +pub fn stopSession(self: *Inspector, session: *Session) void { + const index = std.mem.findScalar(*Session, self.sessions.items, session); + lp.assert(index != null, "Inspector.stopSession unknown session", .{}); + _ = self.sessions.swapRemove(index.?); + session.deinit(); + self.allocator.destroy(session); } // From CDP docs diff --git a/src/server/cdp/CDP.zig b/src/server/cdp/CDP.zig index 93052b35b..99d54f603 100644 --- a/src/server/cdp/CDP.zig +++ b/src/server/cdp/CDP.zig @@ -333,18 +333,8 @@ pub fn resolveSessionId(self: *const CDP, input_session_id: []const u8) ?[]const return browser_session_id; } } - const browser_context = &(self.browser_context orelse return null); - if (browser_context.session_id) |session_id| { - if (std.mem.eql(u8, session_id, input_session_id)) { - return session_id; - } - } - for (browser_context.attached_sessions.items) |session| { - if (std.mem.eql(u8, session.id, input_session_id)) { - return session.id; - } - } - return null; + const browser_context = if (self.browser_context) |*bc| bc else return null; + return browser_context.attached_sessions.getKey(input_session_id); } fn isValidSessionId(self: *const CDP, input_session_id: []const u8) bool { @@ -407,9 +397,41 @@ pub const BrowserContext = struct { id: u32, }; - const AttachedSession = struct { + // A CDP session attached to this context's page target. Each owns a V8 + // inspector session; V8 keeps `Runtime.enable` state and remote object ids + // per session, so a command is answered on the session it was sent + // through and a session only gets the inspector events it enabled. + pub const AttachedSession = struct { id: []const u8, parent_id: ?[]const u8, + bc: *BrowserContext, + inspector_session: *js.Inspector.Session, + + /// V8 -> this session. Stamp OUR id on responses and events. + pub fn onInspectorResponse(ctx: *anyopaque, _: u32, msg: []const u8) void { + const self: *AttachedSession = @ptrCast(@alignCast(ctx)); + self.bc.sendInspectorMessage(msg, self.id) catch |err| { + log.err(.cdp, "send inspector response", .{ .err = err }); + }; + } + + pub fn onInspectorEvent(ctx: *anyopaque, msg: []const u8) void { + const self: *AttachedSession = @ptrCast(@alignCast(ctx)); + if (log.enabled(.cdp, .debug)) { + // msg should be {"method":,... + lp.assert(std.mem.startsWith(u8, msg, "{\"method\":"), "onInspectorEvent prefix", .{}); + const method_end = std.mem.indexOfScalar(u8, msg, ',') orelse { + log.err(.cdp, "invalid inspector event", .{ .msg = msg }); + return; + }; + const method = msg[10..method_end]; + log.debug(.cdp, "inspector event", .{ .method = method, .session_id = self.id }); + } + + self.bc.sendInspectorMessage(msg, self.id) catch |err| { + log.err(.cdp, "send inspector event", .{ .err = err }); + }; + } }; id: []const u8, cdp: *CDP, @@ -444,16 +466,18 @@ pub const BrowserContext = struct { // it). null until the first page is created. page_handle: ?Session.PageHandle = null, - // The CDP session_id. After the target/page is created, the client - // "attaches" to it (either explicitly or automatically). We return a - // "sessionId" which identifies this link. `sessionId` is the how - // the CDP client informs us what it's trying to manipulate. Because we - // only support 1 BrowserContext at a time, and 1 page at a time, this - // is all pretty straightforward, but it still needs to be enforced, i.e. - // if we get a request with a sessionId that doesn't match the current one - // we should reject it. + // The primary CDP session's id. After the target/page is created, the + // client "attaches" to it (either explicitly or automatically). We return + // a "sessionId" which identifies this link. `sessionId` is the how the + // CDP client informs us what it's trying to manipulate. Also included in + // `attached_sessions`. session_id: ?[]const u8, - attached_sessions: std.ArrayList(AttachedSession) = .empty, + + // Every session attached to the page target, the primary included, by + // session id. Insertion-ordered so detach events come out in attach order. + // + // Request with a session id that isn't there is rejected. + attached_sessions: std.StringArrayHashMapUnmanaged(*AttachedSession) = .empty, // A cancelled text-less keyDown drops the char message that follows it // (chromedp's keyDown/char/keyUp split), as Chrome does. @@ -470,8 +494,6 @@ pub const BrowserContext = struct { // entries evicted by resetFrame can linger harmlessly until reset. set_child_nodes_sent: std.AutoHashMapUnmanaged(NodeRegistry.Id, void) = .empty, - inspector_call_sessions: std.AutoHashMapUnmanaged(i64, []const u8) = .empty, - inspector_session: *js.Inspector.Session, isolated_worlds: std.ArrayList(*IsolatedWorld), // True when Runtime.evaluate has been run in the main world. Optimization @@ -534,10 +556,6 @@ pub const BrowserContext = struct { lp.cookies.loadFromFile(session, cookie_path); } - const browser = &cdp.browser; - const inspector_session = browser.env.inspector.?.startSession(self); - errdefer browser.env.inspector.?.stopSession(); - var registry = NodeRegistry.init(allocator); errdefer registry.deinit(); @@ -553,7 +571,6 @@ pub const BrowserContext = struct { .node_registry = registry, .node_search_list = undefined, .isolated_worlds = .empty, - .inspector_session = inspector_session, .frame_arena = cdp.frame_arena.allocator(), .arena = cdp.browser_context_arena.allocator(), .notification_arena = cdp.notification_arena.allocator(), @@ -586,7 +603,7 @@ pub const BrowserContext = struct { // It appends async tasks, so we make sure we run the message loop // before deinit it. env.inspector.?.resetContextGroup(); - env.inspector.?.stopSession(); + self.detachAllSessions(); // abort all intercepted requests before closing the session/page // since some of these might callback into the page/scriptmanager. @@ -623,7 +640,7 @@ pub const BrowserContext = struct { self.node_registry.deinit(); self.node_search_list.deinit(); self.set_child_nodes_sent.deinit(self.cdp.allocator); - self.inspector_call_sessions.deinit(self.cdp.allocator); + self.attached_sessions.deinit(self.cdp.allocator); // Session.deinit (called via closeSession above) already cleared this // notification off any ownerless CorsGate/RobotsGate transfers. @@ -844,7 +861,7 @@ pub const BrowserContext = struct { pub fn closeTarget(self: *BrowserContext) !void { const target_id = self.target_id orelse return; const cdp = self.cdp; - for (self.attached_sessions.items) |session| { + for (self.attached_sessions.values()) |session| { self.fetchDisableForSession(session.id); try cdp.sendEvent("Inspector.detached", .{ .reason = "Render process gone.", @@ -855,21 +872,7 @@ pub const BrowserContext = struct { .reason = "Render process gone.", }, .{ .session_id = session.parent_id }); } - self.attached_sessions.clearRetainingCapacity(); - - // could be null, created but never attached - if (self.session_id) |session_id| { - self.fetchDisableForSession(session_id); - try cdp.sendEvent("Inspector.detached", .{ - .reason = "Render process gone.", - }, .{ .session_id = session_id }); - try cdp.sendEvent("Target.detachedFromTarget", .{ - .targetId = target_id, - .sessionId = session_id, - .reason = "Render process gone.", - }, .{}); - self.session_id = null; - } + self.detachAllSessions(); try cdp.sendEvent("Target.targetDestroyed", .{ .targetId = target_id }, .{}); @@ -1176,63 +1179,85 @@ pub const BrowserContext = struct { } } - /// Forwards `cmd`'s raw JSON to the V8 inspector, which answers through onInspectorResponse. + pub inline fn inspector(self: *const BrowserContext) *js.Inspector { + return self.cdp.browser.env.inspector.?; + } + + /// Attaches `session_id` to the page target with its own inspector + /// session. The id is copied into the context's arena. + pub fn attachSession(self: *BrowserContext, session_id: []const u8, parent_id: ?[]const u8) !*AttachedSession { + if (self.attached_sessions.contains(session_id)) { + return error.SessionAlreadyAttached; + } + const allocator = self.cdp.allocator; + + const attached = try allocator.create(AttachedSession); + errdefer allocator.destroy(attached); + + const inspector_session = try self.inspector().startSession(attached); + errdefer self.inspector().stopSession(inspector_session); + + attached.* = .{ + .id = try self.arena.dupe(u8, session_id), + .parent_id = parent_id, + .bc = self, + .inspector_session = inspector_session, + }; + try self.attached_sessions.put(allocator, attached.id, attached); + return attached; + } + + /// The first session attached to the target. + pub fn attachPrimarySession(self: *BrowserContext, session_id: []const u8) !*AttachedSession { + lp.assert(self.session_id == null, "CDP.BrowserContext.attachPrimarySession already attached", .{}); + const attached = try self.attachSession(session_id, null); + self.session_id = attached.id; + return attached; + } + + /// Stops the session's inspector session and forgets it. + /// Returns false when no such session is attached. + pub fn detachSession(self: *BrowserContext, session_id: []const u8) bool { + const kv = self.attached_sessions.fetchOrderedRemove(session_id) orelse return false; + if (self.session_id) |primary| { + if (std.mem.eql(u8, primary, session_id)) { + self.session_id = null; + } + } + self.destroySession(kv.value); + return true; + } + + pub fn detachAllSessions(self: *BrowserContext) void { + for (self.attached_sessions.values()) |attached| { + self.destroySession(attached); + } + self.attached_sessions.clearRetainingCapacity(); + self.session_id = null; + } + + fn destroySession(self: *BrowserContext, attached: *AttachedSession) void { + self.inspector().stopSession(attached.inspector_session); + self.cdp.allocator.destroy(attached); + } + + pub fn inspectorSession(self: *const BrowserContext, session_id: ?[]const u8) !*js.Inspector.Session { + const id = session_id orelse self.session_id orelse return error.SessionNotAttached; + const attached = self.attached_sessions.get(id) orelse return error.SessionNotAttached; + return attached.inspector_session; + } + + /// Forwards `cmd`'s raw JSON to the inspector session of the session it was sent through. pub fn callInspector(self: *BrowserContext, cmd: *const Command) !void { - try self.trackInspectorCall(cmd); - self.inspector_session.send(cmd.input.json); + const inspector_session = try self.inspectorSession(cmd.input.session_id); + inspector_session.send(cmd.input.json); self.session.browser.env.runMicrotasks(); } - // Remembers which non-primary session `cmd` came from, so that its response - // can be stamped with that session. - fn trackInspectorCall(self: *BrowserContext, cmd: *const Command) !void { - const id = cmd.input.id orelse return; - const input_session_id = cmd.input.session_id orelse return; - if (self.session_id) |primary| { - if (std.mem.eql(u8, primary, input_session_id)) { - return; - } - } - const session_id = self.cdp.resolveSessionId(input_session_id) orelse return; - try self.inspector_call_sessions.put(self.cdp.allocator, id, session_id); - } - - pub fn onInspectorResponse(ctx: *anyopaque, call_id: u32, msg: []const u8) void { - const self: *BrowserContext = @ptrCast(@alignCast(ctx)); - const session_id = self.inspector_call_sessions.fetchRemove(@intCast(call_id)); - sendInspectorMessage(self, msg, if (session_id) |kv| kv.value else null) catch |err| { - log.err(.cdp, "send inspector response", .{ .err = err }); - }; - } - - pub fn onInspectorEvent(ctx: *anyopaque, msg: []const u8) void { - if (log.enabled(.cdp, .debug)) { - // msg should be {"method":,... - lp.assert(std.mem.startsWith(u8, msg, "{\"method\":"), "onInspectorEvent prefix", .{}); - const method_end = std.mem.indexOfScalar(u8, msg, ',') orelse { - log.err(.cdp, "invalid inspector event", .{ .msg = msg }); - return; - }; - const method = msg[10..method_end]; - log.debug(.cdp, "inspector event", .{ .method = method }); - } - - sendInspectorMessage(@ptrCast(@alignCast(ctx)), msg, null) catch |err| { - log.err(.cdp, "send inspector event", .{ .err = err }); - }; - } - - // This is hacky x 2. First, we create the JSON payload by gluing a - // session_id onto it: `explicit_session_id` (the session a response - // belongs to) or else the primary session (all events). Second, we're - // much more client/websocket aware than we should be. - fn sendInspectorMessage(self: *BrowserContext, msg: []const u8, explicit_session_id: ?[]const u8) !void { - const session_id = explicit_session_id orelse self.session_id orelse { - // We no longer have an active session. What should we do - // in this case? - return; - }; - + // This is hacky x 2. First, we create the JSON payload by gluing the + // session_id onto it. Second, we're much more client/websocket aware than + // we should be. + fn sendInspectorMessage(self: *BrowserContext, msg: []const u8, session_id: []const u8) !void { const cdp = self.cdp; const allocator = cdp.link.acquireSendArena(); defer cdp.link.releaseSendArena(); diff --git a/src/server/cdp/domains/accessibility.zig b/src/server/cdp/domains/accessibility.zig index 9d46d796d..79346ec0a 100644 --- a/src/server/cdp/domains/accessibility.zig +++ b/src/server/cdp/domains/accessibility.zig @@ -89,7 +89,7 @@ fn queryAXTree(cmd: *CDP.Command) !void { const params = (try cmd.params(Params)) orelse return error.InvalidParams; const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; - const node = try dom.getNode(cmd.arena, bc, params.nodeId, params.backendNodeId, params.objectId); + const node = try dom.getNode(cmd.arena, bc, cmd.input.session_id, params.nodeId, params.backendNodeId, params.objectId); const frame = bc.mainFrame() orelse return error.FrameNotLoaded; const temp_arena = try frame.getArena(.medium, "AXNode"); @@ -124,7 +124,7 @@ fn getPartialAXTree(cmd: *CDP.Command) !void { } const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; - const node = try dom.getNode(cmd.arena, bc, params.nodeId, params.backendNodeId, params.objectId); + const node = try dom.getNode(cmd.arena, bc, cmd.input.session_id, params.nodeId, params.backendNodeId, params.objectId); const frame = bc.mainFrame() orelse return error.FrameNotLoaded; const temp_arena = try frame.getArena(.medium, "AXNode"); diff --git a/src/server/cdp/domains/dom.zig b/src/server/cdp/domains/dom.zig index 447362c59..23263314d 100644 --- a/src/server/cdp/domains/dom.zig +++ b/src/server/cdp/domains/dom.zig @@ -365,9 +365,12 @@ fn resolveNode(cmd: *CDP.Command) !void { js_context.localScope(&ls); defer ls.deinit(); + // The object id is minted on the command's session; only that session can unwrap it later. + const inspector_session = try bc.inspectorSession(cmd.input.session_id); + // node._node is a *DOMNode we need this to be able to find its most derived type e.g. Node -> Element -> HTMLElement // So we use the Node.Union when retrieve the value from the environment - const remote_object = try bc.inspector_session.getRemoteObject( + const remote_object = try inspector_session.getRemoteObject( &ls.local, params.objectGroup orelse "", node.dom, @@ -433,7 +436,7 @@ fn describeNode(cmd: *CDP.Command) !void { } const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; - const node = try getNode(cmd.arena, bc, params.nodeId, params.backendNodeId, params.objectId); + const node = try getNode(cmd.arena, bc, cmd.input.session_id, params.nodeId, params.backendNodeId, params.objectId); return cmd.sendResult(.{ .node = bc.nodeWriter(node, .{ .depth = params.depth }) }, .{}); } @@ -477,7 +480,7 @@ fn scrollIntoViewIfNeeded(cmd: *CDP.Command) !void { // We retrieve the node to at least check if it exists and is valid. const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; - const node = try getNode(cmd.arena, bc, params.nodeId, params.backendNodeId, params.objectId); + const node = try getNode(cmd.arena, bc, cmd.input.session_id, params.nodeId, params.backendNodeId, params.objectId); switch (node.dom._type) { .element => {}, @@ -489,7 +492,9 @@ fn scrollIntoViewIfNeeded(cmd: *CDP.Command) !void { return cmd.sendResult(null, .{}); } -pub fn getNode(arena: Allocator, bc: *CDP.BrowserContext, node_id: ?NodeRegistry.Id, backend_node_id: ?NodeRegistry.Id, object_id: ?[]const u8) !*NodeRegistry.Node { +/// `session_id` belongs to the command; a remote object id only resolves on +/// the inspector session that minted it. +pub fn getNode(arena: Allocator, bc: *CDP.BrowserContext, session_id: ?[]const u8, node_id: ?NodeRegistry.Id, backend_node_id: ?NodeRegistry.Id, object_id: ?[]const u8) !*NodeRegistry.Node { const input_node_id = node_id orelse backend_node_id; if (input_node_id) |input_node_id_| { return bc.node_registry.lookup_by_id.get(input_node_id_) orelse return error.NodeNotFound; @@ -501,7 +506,8 @@ pub fn getNode(arena: Allocator, bc: *CDP.BrowserContext, node_id: ?NodeRegistry defer ls.deinit(); // Retrieve the object from which ever context it is in. - const parser_node = try bc.inspector_session.getNodePtr(arena, object_id_, &ls.local); + const inspector_session = try bc.inspectorSession(session_id); + const parser_node = try inspector_session.getNodePtr(arena, object_id_, &ls.local); return try bc.node_registry.register(@ptrCast(@alignCast(parser_node))); } return error.MissingParams; @@ -519,7 +525,7 @@ fn getContentQuads(cmd: *CDP.Command) !void { const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; const frame = bc.mainFrame() orelse return error.FrameNotLoaded; - const node = try getNode(cmd.arena, bc, params.nodeId, params.backendNodeId, params.objectId); + const node = try getNode(cmd.arena, bc, cmd.input.session_id, params.nodeId, params.backendNodeId, params.objectId); // TODO likely if the following CSS properties are set the quads should be empty // visibility: hidden @@ -545,7 +551,7 @@ fn getBoxModel(cmd: *CDP.Command) !void { const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; const frame = bc.mainFrame() orelse return error.FrameNotLoaded; - const node = try getNode(cmd.arena, bc, params.nodeId, params.backendNodeId, params.objectId); + const node = try getNode(cmd.arena, bc, cmd.input.session_id, params.nodeId, params.backendNodeId, params.objectId); // TODO implement for document or text const element = node.dom.is(DOMNode.Element) orelse return error.NodeIsNotAnElement; @@ -626,7 +632,7 @@ fn getOuterHTML(cmd: *CDP.Command) !void { const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; const frame = bc.mainFrame() orelse return error.FrameNotLoaded; - const node = try getNode(cmd.arena, bc, params.nodeId, params.backendNodeId, params.objectId); + const node = try getNode(cmd.arena, bc, cmd.input.session_id, params.nodeId, params.backendNodeId, params.objectId); var aw = std.Io.Writer.Allocating.init(cmd.arena); try dump.deep(node.dom, .{}, &aw.writer, frame); @@ -640,7 +646,7 @@ fn requestNode(cmd: *CDP.Command) !void { })) orelse return error.InvalidParams; const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; - const node = try getNode(cmd.arena, bc, null, null, params.objectId); + const node = try getNode(cmd.arena, bc, cmd.input.session_id, null, null, params.objectId); return cmd.sendResult(.{ .nodeId = node.id }, .{}); } @@ -661,7 +667,7 @@ fn setFileInputFiles(cmd: *CDP.Command) !void { const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; const root = bc.mainFrame() orelse return error.FrameNotLoaded; - const node = try getNode(cmd.arena, bc, params.nodeId, params.backendNodeId, params.objectId); + const node = try getNode(cmd.arena, bc, cmd.input.session_id, params.nodeId, params.backendNodeId, params.objectId); const element = node.dom.is(DOMNode.Element) orelse return error.NodeIsNotAnElement; const input = element.is(Input) orelse return error.NotAnInputElement; if (input._input_type != .file) return error.NotAFileInput; @@ -694,7 +700,7 @@ fn focus(cmd: *CDP.Command) !void { const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; const frame = bc.mainFrame() orelse return error.FrameNotLoaded; - const node = try getNode(cmd.arena, bc, params.nodeId, params.backendNodeId, params.objectId); + const node = try getNode(cmd.arena, bc, cmd.input.session_id, params.nodeId, params.backendNodeId, params.objectId); const element = node.dom.is(DOMNode.Element) orelse return error.NodeIsNotAnElement; if (element.isFocusable(frame) == false) { return cmd.sendError(-32000, "Element is not focusable", .{}); @@ -1400,7 +1406,43 @@ fn mainWorldContextId(bc: *CDP.BrowserContext, frame: *const Frame) !i32 { var ls: js.Local.Scope = undefined; frame.js.localScope(&ls); defer ls.deinit(); - return bc.inspector_session.inspector.getContextId(&ls.local); + return bc.inspector().getContextId(&ls.local); +} + +test "cdp.dom: remote object ids belong to the session that minted them" { + var ctx = try testing.context(); + defer ctx.deinit(); + + const bc = try ctx.loadBrowserContext(.{ .id = "BID-RO", .url = "cdp/dom1.html", .target_id = "FID-000000000R".*, .session_id = "SID-PRIMARY" }); + _ = try bc.attachSession("SID-AUX", null); + + const root = bc.mainFrame() orelse unreachable; + const html = root.document.getDocumentElement() orelse unreachable; + const node = try bc.node_registry.register(html.asNode()); + + // The auxiliary session mints an id and resolves it. + try ctx.processMessage(.{ .id = 20, .method = "DOM.resolveNode", .sessionId = "SID-AUX", .params = .{ .backendNodeId = node.id } }); + const aux_object_id = try sentObjectId(&ctx, 20); + try ctx.processMessage(.{ .id = 21, .method = "DOM.requestNode", .sessionId = "SID-AUX", .params = .{ .objectId = aux_object_id } }); + try ctx.expectSentResult(.{ .nodeId = node.id }, .{ .id = 21, .session_id = "SID-AUX" }); + try ctx.processMessage(.{ .id = 22, .method = "DOM.describeNode", .sessionId = "SID-AUX", .params = .{ .objectId = aux_object_id } }); + try ctx.expectSentResult(.{ .node = .{ .nodeId = node.id, .localName = "html" } }, .{ .id = 22, .session_id = "SID-AUX" }); + + // The primary has minted nothing yet: the auxiliary's id is not its. + try ctx.processMessage(.{ .id = 23, .method = "Runtime.callFunctionOn", .sessionId = "SID-PRIMARY", .params = .{ + .objectId = aux_object_id, + .functionDeclaration = "function() { return this.localName; }", + .returnByValue = true, + } }); + try ctx.expectSentError(-32000, "Could not find object with given id", .{ .id = 23 }); + + // Each session's own ids keep working. + try ctx.processMessage(.{ .id = 24, .method = "DOM.resolveNode", .sessionId = "SID-PRIMARY", .params = .{ .backendNodeId = node.id } }); + const primary_object_id = try sentObjectId(&ctx, 24); + try ctx.processMessage(.{ .id = 25, .method = "DOM.requestNode", .sessionId = "SID-PRIMARY", .params = .{ .objectId = primary_object_id } }); + try ctx.expectSentResult(.{ .nodeId = node.id }, .{ .id = 25, .session_id = "SID-PRIMARY" }); + try ctx.processMessage(.{ .id = 26, .method = "DOM.requestNode", .sessionId = "SID-AUX", .params = .{ .objectId = aux_object_id } }); + try ctx.expectSentResult(.{ .nodeId = node.id }, .{ .id = 26, .session_id = "SID-AUX" }); } // The result.object.objectId of the response to command `msg_id`. diff --git a/src/server/cdp/domains/fetch.zig b/src/server/cdp/domains/fetch.zig index 0da0c325e..9cec0b738 100644 --- a/src/server/cdp/domains/fetch.zig +++ b/src/server/cdp/domains/fetch.zig @@ -595,10 +595,7 @@ test "cdp.Fetch: interception events belong to the enabling session" { .session_id = "SID-PRIMARY", .target_id = "TID-000000000B".*, }); - try bc.attached_sessions.append(bc.arena, .{ - .id = "SID-AUX", - .parent_id = null, - }); + _ = try bc.attachSession("SID-AUX", null); try ctx.processMessage(.{ .id = 1, diff --git a/src/server/cdp/domains/page.zig b/src/server/cdp/domains/page.zig index 71fa88d29..e2d5c66ed 100644 --- a/src/server/cdp/domains/page.zig +++ b/src/server/cdp/domains/page.zig @@ -284,7 +284,7 @@ fn createIsolatedWorld(cmd: *CDP.Command) !void { var ls: js.Local.Scope = undefined; js_context.localScope(&ls); defer ls.deinit(); - const context_id = bc.inspector_session.inspector.getContextId(&ls.local); + const context_id = bc.inspector().getContextId(&ls.local); return cmd.sendResult(.{ .executionContextId = context_id }, .{}); } @@ -315,7 +315,7 @@ fn registerIsolatedWorldContext(arena: Allocator, bc: *CDP.BrowserContext, world js_context.localScope(&ls); defer ls.deinit(); - bc.inspector_session.inspector.contextCreated( + bc.inspector().contextCreated( &ls.local, world.name, frame.origin orelse "", @@ -558,7 +558,7 @@ pub fn frameNavigate(bc: *CDP.BrowserContext, event: *const Notification.FrameNa pub fn frameRemove(bc: *CDP.BrowserContext) void { // Clear all remote object mappings to prevent stale objectIds from being used // after the context is destroy - bc.inspector_session.inspector.resetContextGroup(); + bc.inspector().resetContextGroup(); // The main frame is going to be removed, we need to remove contexts from other worlds first. for (bc.isolated_worlds.items) |isolated_world| { @@ -743,7 +743,7 @@ pub fn frameNavigated(arena: Allocator, bc: *CDP.BrowserContext, event: *const N frame.js.localScope(&ls); defer ls.deinit(); - bc.inspector_session.inspector.contextCreated( + bc.inspector().contextCreated( &ls.local, "", frame.origin orelse "", @@ -1735,7 +1735,7 @@ fn isolatedWorldContextId(bc: *CDP.BrowserContext, frame: *const Frame) !i32 { var ls: js.Local.Scope = undefined; js_context.localScope(&ls); defer ls.deinit(); - return bc.inspector_session.inspector.getContextId(&ls.local); + return bc.inspector().getContextId(&ls.local); } test "cdp.frame: child frame metadata" { @@ -2170,7 +2170,7 @@ test "cdp.frame: reload replays POST navigation" { _ = try cdp_inst.createBrowserContext(); var bc = &cdp_inst.browser_context.?; bc.id = "BID-A6"; - bc.session_id = "SID-X"; + _ = try bc.attachPrimarySession("SID-X"); bc.target_id = "TID-A6-0000000".*; // First navigation: POST a form-style payload to /echo_method. @@ -2222,7 +2222,7 @@ test "cdp.frame: reload after POST→redirect drops the POST" { _ = try cdp_inst.createBrowserContext(); var bc = &cdp_inst.browser_context.?; bc.id = "BID-A6R"; - bc.session_id = "SID-XR"; + _ = try bc.attachPrimarySession("SID-XR"); bc.target_id = "TID-A6R-000000".*; // First navigation: POST /redirect_to_echo → 302 → GET /echo_method. @@ -2486,7 +2486,7 @@ test "cdp.frame: first navigation of a pristine bootstrap about:blank navigates defer ctx.deinit(); var bc = try ctx.loadBrowserContext(.{ .id = "BID-PRS", .target_id = "TID-PRS-000000".* }); - bc.session_id = "SID-PRS"; + _ = try bc.attachPrimarySession("SID-PRS"); _ = try bc.session.createPage(); const before = bc.mainFrame() orelse unreachable; try testing.expectEqualSlices(u8, "about:blank", before.url); @@ -2515,7 +2515,7 @@ test "cdp.frame: anchor click sends Referer matching the originating page" { _ = try cdp_inst.createBrowserContext(); var bc = &cdp_inst.browser_context.?; bc.id = "BID-A18"; - bc.session_id = "SID-A18"; + _ = try bc.attachPrimarySession("SID-A18"); bc.target_id = "TID-A18-000000".*; // Initial navigation to the page hosting the anchor — driven directly via @@ -2564,7 +2564,7 @@ test "cdp.frame: address-bar Page.navigate sends no Referer" { _ = try cdp_inst.createBrowserContext(); var bc = &cdp_inst.browser_context.?; bc.id = "BID-A18B"; - bc.session_id = "SID-A18B"; + _ = try bc.attachPrimarySession("SID-A18B"); bc.target_id = "TID-A18B-00000".*; { diff --git a/src/server/cdp/domains/runtime.zig b/src/server/cdp/domains/runtime.zig index 8e4d4df5f..b5e1a1be3 100644 --- a/src/server/cdp/domains/runtime.zig +++ b/src/server/cdp/domains/runtime.zig @@ -127,18 +127,21 @@ const ConsoleMessage = struct { }; pub fn consoleMessage(arena: Allocator, bc: *CDP.BrowserContext, event: *const Notification.ConsoleMessage) !void { + // The event goes to the primary session, so its argument handles are + // minted on the primary's inspector session. const session_id = bc.session_id orelse return; + const inspector_session = try bc.inspectorSession(session_id); const frame = bc.mainFrame() orelse return error.FrameNotLoaded; var ls: js.Local.Scope = undefined; frame.js.localScope(&ls); defer ls.deinit(); - const context_id = bc.inspector_session.inspector.getContextId(&ls.local); + const context_id = bc.inspector().getContextId(&ls.local); var args: std.ArrayList(RemoteObject) = .empty; for (event.values) |value| { - const remote_object = try bc.inspector_session.getRemoteObject( + const remote_object = try inspector_session.getRemoteObject( &ls.local, "", value, @@ -189,11 +192,10 @@ test "cdp.runtime: inspector responses go to the session that sent the command" defer ctx.deinit(); const bc = try ctx.loadBrowserContext(.{ .id = "BID-RT2", .url = "hi.html", .target_id = "FID-0000000RT2".*, .session_id = "SID-PRIMARY" }); - try bc.attached_sessions.append(bc.arena, .{ .id = "SID-AUX", .parent_id = null }); + _ = try bc.attachSession("SID-AUX", null); try ctx.processMessage(.{ .id = 60, .method = "Runtime.evaluate", .sessionId = "SID-AUX", .params = .{ .expression = "1 + 1", .returnByValue = true } }); try ctx.expectSentResult(.{ .result = .{ .type = "number", .value = 2, .description = "2" } }, .{ .id = 60, .session_id = "SID-AUX" }); - try testing.expectEqual(0, bc.inspector_call_sessions.count()); // The primary session keeps working as before. try ctx.processMessage(.{ .id = 61, .method = "Runtime.evaluate", .sessionId = "SID-PRIMARY", .params = .{ .expression = "2 + 2", .returnByValue = true } }); @@ -203,7 +205,53 @@ test "cdp.runtime: inspector responses go to the session that sent the command" // answers from a microtask) still finds its session. try ctx.processMessage(.{ .id = 62, .method = "Runtime.evaluate", .sessionId = "SID-AUX", .params = .{ .expression = "Promise.resolve('late')", .awaitPromise = true, .returnByValue = true } }); try ctx.expectSentResult(.{ .result = .{ .type = "string", .value = "late" } }, .{ .id = 62, .session_id = "SID-AUX" }); - try testing.expectEqual(0, bc.inspector_call_sessions.count()); + + // A command with no sessionId is answered on the primary session. + try ctx.processMessage(.{ .id = 63, .method = "Runtime.evaluate", .params = .{ .expression = "3 + 3", .returnByValue = true } }); + try ctx.expectSentResult(.{ .result = .{ .type = "number", .value = 6, .description = "6" } }, .{ .id = 63, .session_id = "SID-PRIMARY" }); +} + +// Number of `method` events the client received on `session_id`. +fn countSentEvents(ctx: *testing.TestContext, method: []const u8, session_id: []const u8) !usize { + var count: usize = 0; + var i: usize = 0; + while (try ctx.getSentMessage(i)) |msg| : (i += 1) { + const obj = switch (msg) { + .object => |o| o, + else => continue, + }; + const sent_method = obj.get("method") orelse continue; + if (sent_method != .string or !std.mem.eql(u8, sent_method.string, method)) { + continue; + } + const sent_session_id = obj.get("sessionId") orelse continue; + if (sent_session_id == .string and std.mem.eql(u8, sent_session_id.string, session_id)) { + count += 1; + } + } + return count; +} + +// V8 keeps Runtime.enable per inspector session: a session only gets the +// executionContextCreated events it asked for, stamped with its own id. +test "cdp.runtime: inspector events go to the session that enabled them" { + var ctx = try testing.context(); + defer ctx.deinit(); + + const bc = try ctx.loadBrowserContext(.{ .id = "BID-RT3", .url = "hi.html", .target_id = "FID-0000000RT3".*, .session_id = "SID-PRIMARY" }); + _ = try bc.attachSession("SID-AUX", null); + + try ctx.processMessage(.{ .id = 70, .method = "Runtime.enable", .sessionId = "SID-AUX" }); + try ctx.expectSentResult(null, .{ .id = 70, .session_id = "SID-AUX" }); + try ctx.expectSentEvent("Runtime.executionContextCreated", .{ .context = .{ .auxData = .{ .isDefault = true, .type = "default" } } }, .{ .session_id = "SID-AUX" }); + try testing.expectEqual(1, try countSentEvents(&ctx, "Runtime.executionContextCreated", "SID-AUX")); + try testing.expectEqual(0, try countSentEvents(&ctx, "Runtime.executionContextCreated", "SID-PRIMARY")); + + try ctx.processMessage(.{ .id = 71, .method = "Runtime.enable", .sessionId = "SID-PRIMARY" }); + try ctx.expectSentResult(null, .{ .id = 71, .session_id = "SID-PRIMARY" }); + try ctx.expectSentEvent("Runtime.executionContextCreated", .{ .context = .{ .auxData = .{ .isDefault = true, .type = "default" } } }, .{ .session_id = "SID-PRIMARY" }); + try testing.expectEqual(1, try countSentEvents(&ctx, "Runtime.executionContextCreated", "SID-AUX")); + try testing.expectEqual(1, try countSentEvents(&ctx, "Runtime.executionContextCreated", "SID-PRIMARY")); } test "cdp.runtime: consoleAPICalled type matches the console method" { diff --git a/src/server/cdp/domains/target.zig b/src/server/cdp/domains/target.zig index 0457dc19f..c5fcc9528 100644 --- a/src/server/cdp/domains/target.zig +++ b/src/server/cdp/domains/target.zig @@ -196,7 +196,7 @@ fn createTarget(cmd: *CDP.Command) !void { defer ls.deinit(); const aux_data = try std.fmt.allocPrint(cmd.arena, "{{\"isDefault\":true,\"type\":\"default\",\"frameId\":\"{s}\"}}", .{target_id}); - bc.inspector_session.inspector.contextCreated( + bc.inspector().contextCreated( &ls.local, "", "", // @ZIGDOM @@ -267,14 +267,10 @@ fn attachToTarget(cmd: *CDP.Command) !void { cmd.cdp.resolveSessionId(session_id) orelse return error.UnknownSessionId else null; - const session_id = try bc.arena.dupe(u8, cmd.cdp.session_id_gen.next()); - try bc.attached_sessions.append(bc.arena, .{ - .id = session_id, - .parent_id = parent_id, - }); + const session = try bc.attachSession(cmd.cdp.session_id_gen.next(), parent_id); try cmd.sendEvent("Target.attachedToTarget", AttachToTarget{ - .sessionId = session_id, + .sessionId = session.id, .targetInfo = TargetInfo{ .targetId = target_id, .title = bc.getTitle() orelse "", @@ -283,7 +279,7 @@ fn attachToTarget(cmd: *CDP.Command) !void { }, }, .{ .session_id = parent_id }); - return cmd.sendResult(.{ .sessionId = session_id }, .{}); + return cmd.sendResult(.{ .sessionId = session.id }, .{}); } fn attachToBrowserTarget(cmd: *CDP.Command) !void { @@ -402,31 +398,18 @@ fn detachFromTarget(cmd: *CDP.Command) !void { const params = (try cmd.params(Params)) orelse Params{}; if (cmd.browser_context) |bc| { - if (params.sessionId) |requested_session_id| { - for (bc.attached_sessions.items, 0..) |session, index| { - if (!std.mem.eql(u8, session.id, requested_session_id)) continue; + // Without a sessionId, detach the primary session (if any). + if (params.sessionId orelse bc.session_id) |requested_session_id| { + const session = bc.attached_sessions.get(requested_session_id) orelse return error.UnknownSessionId; + const session_id = session.id; + const parent_id = session.parent_id; - _ = bc.attached_sessions.orderedRemove(index); - bc.fetchDisableForSession(session.id); - try cmd.sendEvent("Target.detachedFromTarget", .{ - .sessionId = session.id, - }, .{ .session_id = session.parent_id }); - return cmd.sendResult(null, .{}); - } - - const session_id = bc.session_id orelse return error.UnknownSessionId; - if (!std.mem.eql(u8, session_id, requested_session_id)) { - return error.UnknownSessionId; - } - } - - if (bc.session_id) |session_id| { bc.fetchDisableForSession(session_id); try cmd.sendEvent("Target.detachedFromTarget", .{ .sessionId = session_id, - }, .{}); + }, .{ .session_id = parent_id }); + _ = bc.detachSession(session_id); } - bc.session_id = null; } return cmd.sendResult(null, .{}); @@ -456,8 +439,8 @@ fn setAutoAttach(cmd: *CDP.Command) !void { try cmd.sendEvent("Target.detachedFromTarget", .{ .sessionId = session_id, }, .{}); + _ = bc.detachSession(session_id); } - bc.session_id = null; } try cmd.sendResult(null, .{}); return; @@ -500,27 +483,24 @@ fn setAutoAttach(cmd: *CDP.Command) !void { fn doAttachtoTarget(cmd: *CDP.Command, target_id: []const u8) !void { const bc = cmd.browser_context.?; - const session_id = bc.session_id orelse blk: { - break :blk try bc.arena.dupe(u8, cmd.cdp.session_id_gen.next()); - }; + const parent_id = bc.session_id; if (bc.session_id == null) { // extra_headers should not be kept on a new frame or tab, // currently we have only 1 frame, we clear it just in case bc.extra_headers.clearRetainingCapacity(); + _ = try bc.attachPrimarySession(cmd.cdp.session_id_gen.next()); } try cmd.sendEvent("Target.attachedToTarget", AttachToTarget{ - .sessionId = session_id, + .sessionId = bc.session_id.?, .targetInfo = TargetInfo{ .targetId = target_id, .title = bc.getTitle() orelse "", .url = bc.getURL() orelse "about:blank", .browserContextId = bc.id, }, - }, .{ .session_id = bc.session_id }); - - bc.session_id = session_id; + }, .{ .session_id = parent_id }); } const AttachToTarget = struct { @@ -629,7 +609,9 @@ test "cdp.target: disposeBrowserContext detaches target sessions" { .sessionId = "BSID-1", .params = .{ .targetId = target_id }, }); - const auxiliary_id = try testing.arena_allocator.dupe(u8, bc.attached_sessions.items[0].id); + // the primary is attached first, the auxiliary session after it + try testing.expectEqual(2, bc.attached_sessions.count()); + const auxiliary_id = try testing.arena_allocator.dupe(u8, bc.attached_sessions.keys()[1]); try ctx.processMessage(.{ .id = 5, @@ -909,7 +891,8 @@ test "cdp.target: attachToTarget" { const session_id = bc.session_id.?; try ctx.expectSentResult(.{ .sessionId = session_id }, .{ .id = 11 }); try ctx.expectSentEvent("Target.attachedToTarget", .{ .sessionId = session_id, .targetInfo = .{ .url = "about:blank", .title = "", .attached = true, .type = "page", .canAccessOpener = false, .browserContextId = "BID-9", .targetId = bc.target_id.? } }, .{}); - try testing.expectEqual(0, bc.attached_sessions.items.len); + try testing.expectEqual(1, bc.attached_sessions.count()); + try testing.expect(bc.attached_sessions.contains(session_id)); } } @@ -930,7 +913,9 @@ test "cdp.target: auxiliary session is unique and routed through its parent" { .params = .{ .targetId = "TID-000000000B" }, }); - const session_id = bc.attached_sessions.items[0].id; + // the primary is attached first, the auxiliary session after it + try testing.expectEqual(2, bc.attached_sessions.count()); + const session_id = bc.attached_sessions.keys()[1]; try testing.expect(!std.mem.eql(u8, session_id, bc.session_id.?)); try ctx.expectSentEvent("Target.attachedToTarget", .{ .sessionId = session_id, @@ -1045,16 +1030,46 @@ test "cdp.target: detachFromTarget auxiliary session" { }); try ctx.processMessage(.{ .id = 10, .method = "Target.attachToTarget", .params = .{ .targetId = "TID-000000000B" } }); - const session_id = bc.attached_sessions.items[0].id; + try testing.expectEqual(2, bc.attached_sessions.count()); + const session_id = bc.attached_sessions.keys()[1]; try testing.expect(!std.mem.eql(u8, session_id, bc.session_id.?)); try ctx.processMessage(.{ .id = 11, .method = "Target.detachFromTarget", .params = .{ .sessionId = session_id } }); try ctx.expectSentEvent("Target.detachedFromTarget", .{ .sessionId = session_id }, .{}); - try testing.expectEqual(0, bc.attached_sessions.items.len); + try testing.expectEqual(1, bc.attached_sessions.count()); try testing.expectEqual(true, bc.session_id != null); try ctx.expectSentResult(null, .{ .id = 11 }); } +// A detached session's inspector session goes with it: commands on the old +// id are rejected up front and the primary is unaffected. +test "cdp.target: detachFromTarget releases the auxiliary session's inspector session" { + var ctx = try testing.context(); + defer ctx.deinit(); + const bc = try ctx.loadBrowserContext(.{ + .id = "BID-9", + .url = "hi.html", + .session_id = "SID-PRIMARY", + .target_id = "TID-000000000B".*, + }); + _ = try bc.attachSession("SID-AUX", null); + + try ctx.processMessage(.{ .id = 10, .method = "Runtime.evaluate", .sessionId = "SID-AUX", .params = .{ .expression = "1 + 1", .returnByValue = true } }); + try ctx.expectSentResult(.{ .result = .{ .type = "number", .value = 2 } }, .{ .id = 10, .session_id = "SID-AUX" }); + + try ctx.processMessage(.{ .id = 11, .method = "Target.detachFromTarget", .params = .{ .sessionId = "SID-AUX" } }); + try ctx.expectSentEvent("Target.detachedFromTarget", .{ .sessionId = "SID-AUX" }, .{}); + try ctx.expectSentResult(null, .{ .id = 11 }); + try testing.expectEqual(1, bc.attached_sessions.count()); + try testing.expectEqual(1, bc.inspector().sessions.items.len); + + try ctx.processMessage(.{ .id = 12, .method = "Runtime.evaluate", .sessionId = "SID-AUX", .params = .{ .expression = "1 + 1", .returnByValue = true } }); + try ctx.expectSentError(-32001, "Unknown sessionId", .{ .id = 12 }); + + try ctx.processMessage(.{ .id = 13, .method = "Runtime.evaluate", .sessionId = "SID-PRIMARY", .params = .{ .expression = "2 + 2", .returnByValue = true } }); + try ctx.expectSentResult(.{ .result = .{ .type = "number", .value = 4 } }, .{ .id = 13, .session_id = "SID-PRIMARY" }); +} + test "cdp.target: detachFromTarget without session" { var ctx = try testing.context(); defer ctx.deinit(); diff --git a/src/server/cdp/testing.zig b/src/server/cdp/testing.zig index fff827169..3bac1b10f 100644 --- a/src/server/cdp/testing.zig +++ b/src/server/cdp/testing.zig @@ -95,12 +95,12 @@ pub const TestContext = struct { } if (opts.session_id) |sid| { - bc.session_id = sid; + _ = try bc.attachPrimarySession(sid); } if (opts.url) |url| { if (bc.session_id == null) { - bc.session_id = "SID-X"; + _ = try bc.attachPrimarySession("SID-X"); } if (bc.target_id == null) { bc.target_id = "TID-000000000Z".*; From 8ede89217aa369404dfb901ddb6670adf952ea1b Mon Sep 17 00:00:00 2001 From: nikneym Date: Mon, 21 Sep 2026 12:36:26 +0300 Subject: [PATCH 3/4] `cdp`: update tests --- src/server/cdp/domains/dom.zig | 36 +++++++++++++++++------------- src/server/cdp/domains/network.zig | 2 +- 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/server/cdp/domains/dom.zig b/src/server/cdp/domains/dom.zig index 23263314d..f113c83b5 100644 --- a/src/server/cdp/domains/dom.zig +++ b/src/server/cdp/domains/dom.zig @@ -1418,31 +1418,37 @@ test "cdp.dom: remote object ids belong to the session that minted them" { const root = bc.mainFrame() orelse unreachable; const html = root.document.getDocumentElement() orelse unreachable; - const node = try bc.node_registry.register(html.asNode()); + const html_node = try bc.node_registry.register(html.asNode()); + const document_node = try bc.node_registry.register(root.document.asNode()); - // The auxiliary session mints an id and resolves it. - try ctx.processMessage(.{ .id = 20, .method = "DOM.resolveNode", .sessionId = "SID-AUX", .params = .{ .backendNodeId = node.id } }); + // The auxiliary session mints an id for and resolves it. + try ctx.processMessage(.{ .id = 20, .method = "DOM.resolveNode", .sessionId = "SID-AUX", .params = .{ .backendNodeId = html_node.id } }); const aux_object_id = try sentObjectId(&ctx, 20); try ctx.processMessage(.{ .id = 21, .method = "DOM.requestNode", .sessionId = "SID-AUX", .params = .{ .objectId = aux_object_id } }); - try ctx.expectSentResult(.{ .nodeId = node.id }, .{ .id = 21, .session_id = "SID-AUX" }); - try ctx.processMessage(.{ .id = 22, .method = "DOM.describeNode", .sessionId = "SID-AUX", .params = .{ .objectId = aux_object_id } }); - try ctx.expectSentResult(.{ .node = .{ .nodeId = node.id, .localName = "html" } }, .{ .id = 22, .session_id = "SID-AUX" }); + try ctx.expectSentResult(.{ .nodeId = html_node.id }, .{ .id = 21, .session_id = "SID-AUX" }); // The primary has minted nothing yet: the auxiliary's id is not its. - try ctx.processMessage(.{ .id = 23, .method = "Runtime.callFunctionOn", .sessionId = "SID-PRIMARY", .params = .{ + try ctx.processMessage(.{ .id = 22, .method = "Runtime.callFunctionOn", .sessionId = "SID-PRIMARY", .params = .{ .objectId = aux_object_id, .functionDeclaration = "function() { return this.localName; }", .returnByValue = true, } }); - try ctx.expectSentError(-32000, "Could not find object with given id", .{ .id = 23 }); + try ctx.expectSentError(-32000, "Could not find object with given id", .{ .id = 22 }); - // Each session's own ids keep working. - try ctx.processMessage(.{ .id = 24, .method = "DOM.resolveNode", .sessionId = "SID-PRIMARY", .params = .{ .backendNodeId = node.id } }); - const primary_object_id = try sentObjectId(&ctx, 24); - try ctx.processMessage(.{ .id = 25, .method = "DOM.requestNode", .sessionId = "SID-PRIMARY", .params = .{ .objectId = primary_object_id } }); - try ctx.expectSentResult(.{ .nodeId = node.id }, .{ .id = 25, .session_id = "SID-PRIMARY" }); - try ctx.processMessage(.{ .id = 26, .method = "DOM.requestNode", .sessionId = "SID-AUX", .params = .{ .objectId = aux_object_id } }); - try ctx.expectSentResult(.{ .nodeId = node.id }, .{ .id = 26, .session_id = "SID-AUX" }); + // Each session numbers its ids by itself, so the primary's first id may + // well be the same string as the auxiliary's: the session a command comes + // through, not the id, tells the objects apart. Mint a different node on + // the primary and check that each id describes its own. + try ctx.processMessage(.{ .id = 23, .method = "DOM.resolveNode", .sessionId = "SID-PRIMARY", .params = .{ .backendNodeId = document_node.id } }); + const primary_object_id = try sentObjectId(&ctx, 23); + try ctx.processMessage(.{ .id = 24, .method = "DOM.describeNode", .sessionId = "SID-PRIMARY", .params = .{ .objectId = primary_object_id } }); + try ctx.expectSentResult(.{ .node = .{ .nodeId = document_node.id, .nodeName = "#document" } }, .{ .id = 24, .session_id = "SID-PRIMARY" }); + try ctx.processMessage(.{ .id = 25, .method = "DOM.describeNode", .sessionId = "SID-AUX", .params = .{ .objectId = aux_object_id } }); + try ctx.expectSentResult(.{ .node = .{ .nodeId = html_node.id, .localName = "html" } }, .{ .id = 25, .session_id = "SID-AUX" }); + try ctx.processMessage(.{ .id = 26, .method = "DOM.requestNode", .sessionId = "SID-PRIMARY", .params = .{ .objectId = primary_object_id } }); + try ctx.expectSentResult(.{ .nodeId = document_node.id }, .{ .id = 26, .session_id = "SID-PRIMARY" }); + try ctx.processMessage(.{ .id = 27, .method = "DOM.requestNode", .sessionId = "SID-AUX", .params = .{ .objectId = aux_object_id } }); + try ctx.expectSentResult(.{ .nodeId = html_node.id }, .{ .id = 27, .session_id = "SID-AUX" }); } // The result.object.objectId of the response to command `msg_id`. diff --git a/src/server/cdp/domains/network.zig b/src/server/cdp/domains/network.zig index b431d88bd..8b9b95190 100644 --- a/src/server/cdp/domains/network.zig +++ b/src/server/cdp/domains/network.zig @@ -1591,7 +1591,7 @@ test "cdp.Network: worker requests emit network events" { _ = try cdp.createBrowserContext(); var bc = &cdp.browser_context.?; bc.id = "BID-NW"; - bc.session_id = "SID-NW"; + _ = try bc.attachPrimarySession("SID-NW"); bc.target_id = "TID-NW-0000000".*; try ctx.processMessage(.{ .id = 1, .method = "Network.enable" }); From 9aa748616b00b5279eca6ef7665586d7eb1f339c Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 28 Sep 2026 12:18:33 +0800 Subject: [PATCH 4/4] Simplify Inspector.Session ownership Since AttachedSession is already heap-based, it's inspector_session is at a fixed address and thus can own the session. Also, hook in some noop callbacks on deinit. --- src/browser/js/Env.zig | 2 +- src/browser/js/Inspector.zig | 58 +++++++++++-------------------- src/server/cdp/CDP.zig | 14 ++++---- src/server/cdp/domains/target.zig | 34 +++++++++++++++++- 4 files changed, 61 insertions(+), 47 deletions(-) diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index ef43ba993..799ed3b9f 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -245,7 +245,7 @@ pub fn deinit(self: *Env) void { const allocator = app.allocator; if (self.inspector) |i| { - i.deinit(); + i.deinit(allocator); } allocator.free(self.templates); diff --git a/src/browser/js/Inspector.zig b/src/browser/js/Inspector.zig index 22c876f7d..94d467ec8 100644 --- a/src/browser/js/Inspector.zig +++ b/src/browser/js/Inspector.zig @@ -17,7 +17,6 @@ // along with this program. If not, see . const std = @import("std"); -const lp = @import("lightpanda"); const js = @import("js.zig"); const v8 = js.v8; @@ -37,29 +36,22 @@ const CLIENT_TRUST_LEVEL = 1; // (not much at all) const Inspector = @This(); -allocator: Allocator, unique_id: i64, isolate: *v8.Isolate, handle: *v8.Inspector, client: *v8.InspectorClientImpl, default_context: ?v8.Global, -/// One per CDP session attached to the page target; all connect to the same -/// `CONTEXT_GROUP_ID`, so every session sees every context. Heap allocated -/// because the V8 channel keeps the session's address (SET_DATA). -sessions: std.ArrayListUnmanaged(*Session), pub fn init(allocator: Allocator, isolate: *v8.Isolate) !*Inspector { const self = try allocator.create(Inspector); errdefer allocator.destroy(self); self.* = .{ - .allocator = allocator, .unique_id = 1, .isolate = isolate, .client = undefined, .handle = undefined, .default_context = null, - .sessions = .empty, }; self.client = v8.v8_inspector__Client__IMPL__CREATE(); @@ -72,39 +64,14 @@ pub fn init(allocator: Allocator, isolate: *v8.Isolate) !*Inspector { return self; } -pub fn deinit(self: *Inspector) void { +pub fn deinit(self: *const Inspector, allocator: Allocator) void { var hs: v8.HandleScope = undefined; v8.v8__HandleScope__CONSTRUCT(&hs, self.isolate); defer v8.v8__HandleScope__DESTRUCT(&hs); - for (self.sessions.items) |session| { - session.deinit(); - self.allocator.destroy(session); - } - self.sessions.deinit(self.allocator); - v8.v8_inspector__Client__IMPL__DELETE(self.client); v8.v8_inspector__Inspector__DELETE(self.handle); - self.allocator.destroy(self); -} - -pub fn startSession(self: *Inspector, ctx: anytype) !*Session { - const session = try self.allocator.create(Session); - errdefer self.allocator.destroy(session); - - Session.init(session, self, ctx); - errdefer session.deinit(); - - try self.sessions.append(self.allocator, session); - return session; -} - -pub fn stopSession(self: *Inspector, session: *Session) void { - const index = std.mem.findScalar(*Session, self.sessions.items, session); - lp.assert(index != null, "Inspector.stopSession unknown session", .{}); - _ = self.sessions.swapRemove(index.?); - session.deinit(); - self.allocator.destroy(session); + allocator.destroy(self); } // From CDP docs @@ -211,7 +178,10 @@ const RemoteObject = struct { // Combines a v8::InspectorSession and a v8::InspectorChannelImpl. The // InspectorSession is for zig -> v8 (sending messages to the inspector). The // Channel is for v8 -> zig, getting events from the Inspector (that we'll pass -// back to some opaque context, i.e the CDP BrowserContext). +// back to some opaque context, i.e the CDP AttachedSession). +// The channel keeps the Session's address, so the owner must not move it +// between init and deinit. Every Session connects to the same +// CONTEXT_GROUP_ID and so sees every context. // The channel callbacks are defined below, as: // pub export fn v8_inspector__Channel__IMPL__XYZ pub const Session = struct { @@ -224,7 +194,7 @@ pub const Session = struct { onNotif: *const fn (ctx: *anyopaque, msg: []const u8) void, onResp: *const fn (ctx: *anyopaque, call_id: u32, msg: []const u8) void, - fn init(self: *Session, inspector: *Inspector, ctx: anytype) void { + pub fn init(self: *Session, inspector: *Inspector, ctx: anytype) void { const Container = @typeInfo(@TypeOf(ctx)).pointer.child; const channel = v8.v8_inspector__Channel__IMPL__CREATE(inspector.isolate); @@ -246,11 +216,23 @@ pub const Session = struct { }; } - fn deinit(self: *const Session) void { + pub fn deinit(self: *Session) void { + // Deleting the V8 session fails its pending evaluations, which V8 + // answers through the channel. The client is gone: drop them. + self.onResp = dropResponse; + self.onNotif = dropNotification; + + var hs: v8.HandleScope = undefined; + v8.v8__HandleScope__CONSTRUCT(&hs, self.inspector.isolate); + defer v8.v8__HandleScope__DESTRUCT(&hs); + v8.v8_inspector__Session__DELETE(self.handle); v8.v8_inspector__Channel__IMPL__DELETE(self.channel); } + fn dropResponse(_: *anyopaque, _: u32, _: []const u8) void {} + fn dropNotification(_: *anyopaque, _: []const u8) void {} + pub fn send(self: *const Session, msg: []const u8) void { const isolate = self.inspector.isolate; var hs: v8.HandleScope = undefined; diff --git a/src/server/cdp/CDP.zig b/src/server/cdp/CDP.zig index 99d54f603..0a4e74cf9 100644 --- a/src/server/cdp/CDP.zig +++ b/src/server/cdp/CDP.zig @@ -405,7 +405,7 @@ pub const BrowserContext = struct { id: []const u8, parent_id: ?[]const u8, bc: *BrowserContext, - inspector_session: *js.Inspector.Session, + inspector_session: js.Inspector.Session, /// V8 -> this session. Stamp OUR id on responses and events. pub fn onInspectorResponse(ctx: *anyopaque, _: u32, msg: []const u8) void { @@ -1194,15 +1194,15 @@ pub const BrowserContext = struct { const attached = try allocator.create(AttachedSession); errdefer allocator.destroy(attached); - const inspector_session = try self.inspector().startSession(attached); - errdefer self.inspector().stopSession(inspector_session); - attached.* = .{ .id = try self.arena.dupe(u8, session_id), .parent_id = parent_id, .bc = self, - .inspector_session = inspector_session, + .inspector_session = undefined, }; + attached.inspector_session.init(self.inspector(), attached); + errdefer attached.inspector_session.deinit(); + try self.attached_sessions.put(allocator, attached.id, attached); return attached; } @@ -1237,14 +1237,14 @@ pub const BrowserContext = struct { } fn destroySession(self: *BrowserContext, attached: *AttachedSession) void { - self.inspector().stopSession(attached.inspector_session); + attached.inspector_session.deinit(); self.cdp.allocator.destroy(attached); } pub fn inspectorSession(self: *const BrowserContext, session_id: ?[]const u8) !*js.Inspector.Session { const id = session_id orelse self.session_id orelse return error.SessionNotAttached; const attached = self.attached_sessions.get(id) orelse return error.SessionNotAttached; - return attached.inspector_session; + return &attached.inspector_session; } /// Forwards `cmd`'s raw JSON to the inspector session of the session it was sent through. diff --git a/src/server/cdp/domains/target.zig b/src/server/cdp/domains/target.zig index c5fcc9528..1958b7d08 100644 --- a/src/server/cdp/domains/target.zig +++ b/src/server/cdp/domains/target.zig @@ -1061,7 +1061,6 @@ test "cdp.target: detachFromTarget releases the auxiliary session's inspector se try ctx.expectSentEvent("Target.detachedFromTarget", .{ .sessionId = "SID-AUX" }, .{}); try ctx.expectSentResult(null, .{ .id = 11 }); try testing.expectEqual(1, bc.attached_sessions.count()); - try testing.expectEqual(1, bc.inspector().sessions.items.len); try ctx.processMessage(.{ .id = 12, .method = "Runtime.evaluate", .sessionId = "SID-AUX", .params = .{ .expression = "1 + 1", .returnByValue = true } }); try ctx.expectSentError(-32001, "Unknown sessionId", .{ .id = 12 }); @@ -1070,6 +1069,39 @@ test "cdp.target: detachFromTarget releases the auxiliary session's inspector se try ctx.expectSentResult(.{ .result = .{ .type = "number", .value = 4 } }, .{ .id = 13, .session_id = "SID-PRIMARY" }); } +// Stopping an inspector session fails its pending evaluations. The client was +// already told the session is detached, so, like Chrome, nothing more is sent. +test "cdp.target: detachFromTarget drops the auxiliary session's pending responses" { + var ctx = try testing.context(); + defer ctx.deinit(); + const bc = try ctx.loadBrowserContext(.{ + .id = "BID-9", + .url = "hi.html", + .session_id = "SID-PRIMARY", + .target_id = "TID-000000000B".*, + }); + _ = try bc.attachSession("SID-AUX", null); + + try ctx.processMessage(.{ .id = 10, .method = "Runtime.enable", .sessionId = "SID-AUX" }); + try ctx.expectSentResult(null, .{ .id = 10, .session_id = "SID-AUX" }); + try ctx.processMessage(.{ .id = 11, .method = "Runtime.evaluate", .sessionId = "SID-AUX", .params = .{ .expression = "({a: 1})" } }); + try ctx.expectSentResult(.{ .result = .{ .type = "object" } }, .{ .id = 11, .session_id = "SID-AUX" }); + try ctx.processMessage(.{ .id = 12, .method = "Runtime.evaluate", .sessionId = "SID-AUX", .params = .{ .expression = "new Promise(() => {})", .awaitPromise = true } }); + + try ctx.processMessage(.{ .id = 13, .method = "Target.detachFromTarget", .params = .{ .sessionId = "SID-AUX" } }); + try ctx.expectSentEvent("Target.detachedFromTarget", .{ .sessionId = "SID-AUX" }, .{}); + try ctx.expectSentResult(null, .{ .id = 13 }); + + var i: usize = 0; + while (try ctx.getSentMessage(i)) |msg| : (i += 1) { + const msg_id = msg.object.get("id") orelse continue; + try testing.expect(msg_id != .integer or msg_id.integer != 12); + } + + try ctx.processMessage(.{ .id = 14, .method = "Runtime.evaluate", .sessionId = "SID-PRIMARY", .params = .{ .expression = "2 + 2", .returnByValue = true } }); + try ctx.expectSentResult(.{ .result = .{ .type = "number", .value = 4 } }, .{ .id = 14, .session_id = "SID-PRIMARY" }); +} + test "cdp.target: detachFromTarget without session" { var ctx = try testing.context(); defer ctx.deinit();