From 09850d75005b9242417c840472c07872c0d2fb21 Mon Sep 17 00:00:00 2001 From: sjorsdonkers <72333389+sjorsdonkers@users.noreply.github.com> Date: Tue, 22 Apr 2025 11:11:46 +0200 Subject: [PATCH] Fix executor used in resolveNode --- src/browser/browser.zig | 4 ++-- src/cdp/cdp.zig | 46 ++++++++++++++++++++++++---------------- src/cdp/domains/dom.zig | 3 ++- src/cdp/domains/page.zig | 14 ++++++++---- src/runtime/js.zig | 15 ++++++------- 5 files changed, 49 insertions(+), 33 deletions(-) diff --git a/src/browser/browser.zig b/src/browser/browser.zig index 0af2b423..f524d2c8 100644 --- a/src/browser/browser.zig +++ b/src/browser/browser.zig @@ -192,7 +192,7 @@ pub const Session = struct { errdefer self.arena.deinit(); self.executor = try browser.env.startExecutor(Window, &self.state, self); - errdefer browser.env.stopExecutor(self.executor); + errdefer browser.env.stopExecutor(self.executor, true); self.inspector = try Env.Inspector.init(self.arena.allocator(), self.executor, ctx); self.microtaskLoop(); @@ -207,7 +207,7 @@ pub const Session = struct { self.arena.deinit(); self.cookie_jar.deinit(); self.storage_shed.deinit(); - self.browser.env.stopExecutor(self.executor); + self.browser.env.stopExecutor(self.executor, true); } fn microtaskLoop(self: *Session) void { diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig index a32e61ae..dcdaf045 100644 --- a/src/cdp/cdp.zig +++ b/src/cdp/cdp.zig @@ -308,24 +308,6 @@ pub fn BrowserContext(comptime CDP_T: type) type { isolated_world: ?IsolatedWorld, - pub fn createIsolatedWorld( - self: *Self, - world_name: []const u8, - grant_universal_access: bool, - ) !void { - if (self.isolated_world != null) return error.AlreadyExists; - - const executor = try self.cdp.browser.env.startExecutor(@import("../browser/html/window.zig").Window, &self.session.state, self.session); - errdefer self.cdp.browser.env.stopExecutor(executor); - executor.context.exit(); - - self.isolated_world = .{ - .name = try self.session.arena.allocator().dupe(u8, world_name), // TODO allocator - .grant_universal_access = grant_universal_access, - .executor = executor, - }; - } - const Self = @This(); fn init(self: *Self, id: []const u8, cdp: *CDP_T) !void { @@ -352,6 +334,11 @@ pub fn BrowserContext(comptime CDP_T: type) type { } pub fn deinit(self: *Self) void { + if (self.isolated_world) |isolated_world| { + isolated_world.executor.endScope(); + self.cdp.browser.env.stopExecutor(isolated_world.executor, false); + self.isolated_world = null; + } self.node_registry.deinit(); self.node_search_list.deinit(); } @@ -361,6 +348,29 @@ pub fn BrowserContext(comptime CDP_T: type) type { self.node_search_list.reset(); } + pub fn createIsolatedWorld( + self: *Self, + world_name: []const u8, + grant_universal_access: bool, + ) !void { + if (self.isolated_world != null) return error.CurrentlyOnly1IsolatedWorldSupported; + + const executor = try self.cdp.browser.env.startExecutor(@import("../browser/html/window.zig").Window, &self.session.state, self.session); + errdefer self.cdp.browser.env.stopExecutor(executor, true); + + // TBD should we endScope on removePage and re-startScope on createPage? + // Window will be refactored into the executor so we leave it ugly here for now as a reminder. + try executor.startScope(@import("../browser/html/window.zig").Window{}); + + executor.context.exit(); // The default context should remain open + + self.isolated_world = .{ + .name = try self.session.arena.allocator().dupe(u8, world_name), // TODO allocator + .grant_universal_access = grant_universal_access, + .executor = executor, + }; + } + pub fn nodeWriter(self: *Self, node: *const Node, opts: Node.Writer.Opts) Node.Writer { return .{ .node = node, diff --git a/src/cdp/domains/dom.zig b/src/cdp/domains/dom.zig index 110f11c5..0f594fd3 100644 --- a/src/cdp/domains/dom.zig +++ b/src/cdp/domains/dom.zig @@ -134,6 +134,7 @@ fn resolveNode(cmd: anytype) !void { if (executor.context.debugContextId() != context_id) { const isolated_world = bc.isolated_world orelse return error.ContextNotFound; executor = isolated_world.executor; + if (executor.context.debugContextId() != context_id) return error.ContextNotFound; } } @@ -143,7 +144,7 @@ fn resolveNode(cmd: anytype) !void { // node._node is a *parser.Node 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.session.inspector.getRemoteObject( - bc.session.executor, + executor, params.objectGroup orelse "", try dom_node.Node.toInterface(node._node), ); diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index 868b5499..302b8f3d 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -84,6 +84,8 @@ fn setLifecycleEventsEnabled(cmd: anytype) !void { } // TODO: hard coded method +// With the command we receive a script we need to store and run for each new document. +// Note that the worldName refers to the name given to the isolated world. fn addScriptToEvaluateOnNewDocument(cmd: anytype) !void { // const params = (try cmd.params(struct { // source: []const u8, @@ -110,9 +112,10 @@ fn createIsolatedWorld(cmd: anytype) !void { } const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; - try bc.createIsolatedWorld(params.worldName, params.grantUniveralAccess); // orelse return error.IsolatedWorldAlreadyExists; + try bc.createIsolatedWorld(params.worldName, params.grantUniveralAccess); - // Create the auxdata json from + // Create the auxdata json for the contextCreated event + // Calling contextCreated will assign a Id to the context and send the contextCreated event const aux_json = try std.fmt.allocPrint(cmd.arena, "{{\"isDefault\":false,\"type\":\"isolated\",\"frameId\":\"{s}\"}}", .{params.frameId}); bc.session.inspector.contextCreated(bc.isolated_world.?.executor, bc.isolated_world.?.name, "", aux_json, false); @@ -213,16 +216,19 @@ pub fn pageNavigate(bc: anytype, event: *const Notification.PageNavigate) !void // Send Runtime.executionContextsCleared event // TODO: noop event, we have no env context at this point, is it necesarry? - // When we actually recreated the context we should have the inspector send this event, see: resetContextGroup + // When we actually recreated the context we should have the inspector send this event, see: resetContextGroup + // Sending this event will tell the client that the context ids they had are invalid and the context shouls be dropped + // The client will expect us to send new contextCreated events, such that the client has new id's for the active contexts. try cdp.sendEvent("Runtime.executionContextsCleared", null, .{ .session_id = session_id }); if (bc.isolated_world != null) { const aux_json = try std.fmt.allocPrint( bc.session.arena.allocator(), // TODO change this "{{\"isDefault\":false,\"type\":\"isolated\",\"frameId\":\"{s}\"}}", - .{bc.target_id.?}, // TODO check this + .{bc.target_id.?}, ); + // Calling contextCreated will assign a new Id to the context and send the contextCreated event bc.session.inspector.contextCreated( bc.isolated_world.?.executor, bc.isolated_world.?.name, diff --git a/src/runtime/js.zig b/src/runtime/js.zig index 4d7770e6..4c3eeed6 100644 --- a/src/runtime/js.zig +++ b/src/runtime/js.zig @@ -364,7 +364,7 @@ pub fn Env(comptime S: type, comptime types: anytype) type { executor.call_arena = executor._call_arena_instance.allocator(); executor.scope_arena = executor._scope_arena_instance.allocator(); - errdefer self.stopExecutor(executor); + errdefer self.stopExecutor(executor, false); // Note: This likely has issues as context.exit() is errdefered as well // Custom exception // NOTE: there is no way in v8 to subclass the Error built-in type @@ -385,8 +385,8 @@ pub fn Env(comptime S: type, comptime types: anytype) type { // a Context, it's managed by the garbage collector. So, when the // `gc_hints` option is enabled, we'll use the `lowMemoryNotification` // call on the isolate to encourage v8 to free the context. - pub fn stopExecutor(self: *Self, executor: *Executor) void { - executor.deinit(); + pub fn stopExecutor(self: *Self, executor: *Executor, exit_context: bool) void { + executor.deinit(exit_context); self.executor_pool.destroy(executor); if (self.gc_hints) { self.isolate.lowMemoryNotification(); @@ -818,11 +818,10 @@ pub fn Env(comptime S: type, comptime types: anytype) type { // no init, must be initialized via env.startExecutor() // not public, must be destroyed via env.stopExecutor() - fn deinit(self: *Executor) void { - if (self.scope != null) { - self.endScope(); - } - self.context.exit(); + + fn deinit(self: *Executor, exit_context: bool) void { + if (self.scope != null) self.endScope(); + if (exit_context) self.context.exit(); self.handle_scope.deinit(); self._call_arena_instance.deinit();