Rework Inspector usage

V8's inspector world is made up of 4 components: Inspector, Client, Channel and
Session. Currently, we treat all 4 components as a single unit which is tied to
the lifetime of CDP BrowserContext - or, loosely speaking, 1 "Inspector Unit"
per page / v8::Context.

According to https://web.archive.org/web/20210622022956/https://hyperandroid.com/2020/02/12/v8-inspector-from-an-embedder-standpoint/
and conversation with Gemini, it's more typical to have 1 inspector per isolate.
The general breakdown is the Inspector is the top-level manager, the Client is
our implementation which control how the Inspector works (its function we expose
that v8 calls into). These should be tied to the Isolate. Channels and Sessions
are more closely tied to Context, where the Channel is v8->zig and the Session
us zig->v8.

This PR does a few things
1 - It creates 1 Inspector and Client per Isolate (Env.js)
2 - It creates 1 Session/Channel per BrowserContext
3 - It merges v8::Session and v8::Channel into Inspector.Session
4 - It moves the Inspector instance directly into the Env
5 - BrowserContext interacts with the Inspector.Session, not the Inspector

4 is arguably unnecessary with respect to the main goal of this commit, but
the end-goal is to tighten the integration. Specifically, rather than CDP having
to inform the inspector that a context was created/destroyed, the Env which
manages Contexts directly (https://github.com/lightpanda-io/browser/pull/1432)
and which now has direct access to the Inspector, is now equipped to keep this
in sync.
This commit is contained in:
Karl Seguin
2026-01-30 15:35:18 +08:00
parent 34dda780d9
commit 181f265de5
11 changed files with 216 additions and 273 deletions

View File

@@ -80,7 +80,9 @@ pub fn CDPT(comptime TypeProvider: type) type {
pub fn init(app: *App, client: TypeProvider.Client) !Self {
const allocator = app.allocator;
const browser = try Browser.init(app);
const browser = try Browser.init(app, .{
.env = .{ .with_inspector = true },
});
errdefer browser.deinit();
return .{
@@ -352,7 +354,7 @@ pub fn BrowserContext(comptime CDP_T: type) type {
node_registry: Node.Registry,
node_search_list: Node.Search.List,
inspector: *js.Inspector,
inspector_session: *js.Inspector.Session,
isolated_worlds: std.ArrayListUnmanaged(IsolatedWorld),
http_proxy_changed: bool = false,
@@ -381,7 +383,9 @@ pub fn BrowserContext(comptime CDP_T: type) type {
const session = try cdp.browser.newSession();
const arena = session.arena;
const inspector = try cdp.browser.env.newInspector(arena, self);
const browser = &cdp.browser;
const inspector_session = browser.env.inspector.?.startSession(self);
errdefer browser.env.inspector.?.stopSession();
var registry = Node.Registry.init(allocator);
errdefer registry.deinit();
@@ -400,7 +404,7 @@ pub fn BrowserContext(comptime CDP_T: type) type {
.node_registry = registry,
.node_search_list = undefined,
.isolated_worlds = .empty,
.inspector = inspector,
.inspector_session = inspector_session,
.notification_arena = cdp.notification_arena.allocator(),
.intercept_state = try InterceptState.init(allocator),
.captured_responses = .empty,
@@ -409,27 +413,28 @@ pub fn BrowserContext(comptime CDP_T: type) type {
self.node_search_list = Node.Search.List.init(allocator, &self.node_registry);
errdefer self.deinit();
try cdp.browser.notification.register(.page_remove, self, onPageRemove);
try cdp.browser.notification.register(.page_created, self, onPageCreated);
try cdp.browser.notification.register(.page_navigate, self, onPageNavigate);
try cdp.browser.notification.register(.page_navigated, self, onPageNavigated);
try browser.notification.register(.page_remove, self, onPageRemove);
try browser.notification.register(.page_created, self, onPageCreated);
try browser.notification.register(.page_navigate, self, onPageNavigate);
try browser.notification.register(.page_navigated, self, onPageNavigated);
}
pub fn deinit(self: *Self) void {
// safe to call even if never registered
log.unregisterInterceptor();
self.log_interceptor.deinit();
const browser = &self.cdp.browser;
// Drain microtasks makes sure we don't have inspector's callback
// in progress before deinit.
self.cdp.browser.env.runMicrotasks();
browser.env.runMicrotasks();
// resetContextGroup detach the inspector from all contexts.
// It append async tasks, so we make sure we run the message loop
// before deinit it.
self.inspector.resetContextGroup();
self.session.browser.runMessageLoop();
self.inspector.deinit();
browser.env.inspector.?.resetContextGroup();
browser.runMessageLoop();
browser.env.inspector.?.stopSession();
// abort all intercepted requests before closing the sesion/page
// since some of these might callback into the page/scriptmanager
@@ -445,16 +450,16 @@ pub fn BrowserContext(comptime CDP_T: type) type {
// If the session has a page, we need to clear it first. The page
// context is always nested inside of the isolated world context,
// so we need to shutdown the page one first.
self.cdp.browser.closeSession();
browser.closeSession();
self.node_registry.deinit();
self.node_search_list.deinit();
self.cdp.browser.notification.unregisterAll(self);
browser.notification.unregisterAll(self);
if (self.http_proxy_changed) {
// has to be called after browser.closeSession, since it won't
// work if there are active connections.
self.cdp.browser.http_client.restoreOriginalProxy() catch |err| {
browser.http_client.restoreOriginalProxy() catch |err| {
log.warn(.http, "restoreOriginalProxy", .{ .err = err });
};
}
@@ -650,9 +655,7 @@ pub fn BrowserContext(comptime CDP_T: type) type {
}
pub fn callInspector(self: *const Self, msg: []const u8) void {
self.inspector.send(msg);
// force running micro tasks after send input to the inspector.
self.cdp.browser.runMicrotasks();
self.inspector_session.send(msg);
}
pub fn onInspectorResponse(ctx: *anyopaque, _: u32, msg: []const u8) void {
@@ -678,18 +681,6 @@ pub fn BrowserContext(comptime CDP_T: type) type {
};
}
// debugger events
pub fn onRunMessageLoopOnPause(_: *anyopaque, _: u32) void {
// onRunMessageLoopOnPause is called when a breakpoint is hit.
// Until quit pause, we must continue to run a nested message loop
// to interact with the the debugger ony (ie. Chrome DevTools).
}
pub fn onQuitMessageLoopOnPause(_: *anyopaque) void {
// Quit breakpoint pause.
}
// 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.

View File

@@ -305,7 +305,7 @@ fn resolveNode(cmd: anytype) !void {
// 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.getRemoteObject(
const remote_object = try bc.inspector_session.getRemoteObject(
&ls.?.local,
params.objectGroup orelse "",
node.dom,
@@ -404,7 +404,7 @@ fn getNode(arena: Allocator, bc: anytype, node_id: ?Node.Id, backend_node_id: ?N
defer ls.deinit();
// Retrieve the object from which ever context it is in.
const parser_node = try bc.inspector.getNodePtr(arena, object_id_, &ls.local);
const parser_node = try bc.inspector_session.getNodePtr(arena, object_id_, &ls.local);
return try bc.node_registry.register(@ptrCast(@alignCast(parser_node)));
}
return error.MissingParams;

View File

@@ -189,17 +189,9 @@ fn createIsolatedWorld(cmd: anytype) !void {
const world = try bc.createIsolatedWorld(params.worldName, params.grantUniveralAccess);
const page = bc.session.currentPage() orelse return error.PageNotLoaded;
const js_context = try world.createContext(page);
// Create the auxdata json for the contextCreated event
// Calling contextCreated will assign a Id to the context and send the contextCreated event
const aux_data = try std.fmt.allocPrint(cmd.arena, "{{\"isDefault\":false,\"type\":\"isolated\",\"frameId\":\"{s}\"}}", .{params.frameId});
var ls: js.Local.Scope = undefined;
js_context.localScope(&ls);
defer ls.deinit();
bc.inspector.contextCreated(&ls.local, world.name, "", aux_data, false);
return cmd.sendResult(.{ .executionContextId = ls.local.debugContextId() }, .{});
return cmd.sendResult(.{ .executionContextId = js_context.id }, .{});
}
fn navigate(cmd: anytype) !void {
@@ -359,7 +351,7 @@ pub fn pageNavigated(arena: Allocator, bc: anytype, event: *const Notification.P
page.js.localScope(&ls);
defer ls.deinit();
bc.inspector.contextCreated(
bc.inspector_session.inspector.contextCreated(
&ls.local,
"",
try page.getOrigin(arena) orelse "",
@@ -376,7 +368,7 @@ pub fn pageNavigated(arena: Allocator, bc: anytype, event: *const Notification.P
(isolated_world.context orelse continue).localScope(&ls);
defer ls.deinit();
bc.inspector.contextCreated(
bc.inspector_session.inspector.contextCreated(
&ls.local,
isolated_world.name,
"://",

View File

@@ -182,7 +182,7 @@ fn createTarget(cmd: anytype) !void {
defer ls.deinit();
const aux_data = try std.fmt.allocPrint(cmd.arena, "{{\"isDefault\":true,\"type\":\"default\",\"frameId\":\"{s}\"}}", .{target_id});
bc.inspector.contextCreated(
bc.inspector_session.inspector.contextCreated(
&ls.local,
"",
"", // @ZIGDOM