From 09327c3897c934c98f03adcf3b7fff0da6a4528c Mon Sep 17 00:00:00 2001 From: shaewe180 Date: Wed, 18 Mar 2026 15:02:00 +0800 Subject: [PATCH 01/15] feat: fetch add wait_until parameter for page loads options Add `--wait_until` and `--wait_ms` CLI arguments to configure session wait behavior. Updates `Session.wait` to evaluate specific page load states (`load`, `domcontentloaded`, `networkidle`, `fixed`) before completing the wait loop. --- src/Config.zig | 39 +++++++++++++++++++++++++ src/browser/Session.zig | 61 ++++++++++++++++++++++------------------ src/cdp/cdp.zig | 2 +- src/cdp/domains/lp.zig | 2 +- src/cdp/testing.zig | 2 +- src/lightpanda.zig | 3 +- src/main.zig | 3 +- src/main_legacy_test.zig | 2 +- src/mcp/tools.zig | 4 +-- src/testing.zig | 4 +-- 10 files changed, 85 insertions(+), 37 deletions(-) diff --git a/src/Config.zig b/src/Config.zig index 629df32b..f65ecd5d 100644 --- a/src/Config.zig +++ b/src/Config.zig @@ -217,6 +217,15 @@ pub const DumpFormat = enum { semantic_tree_text, }; +pub const WaitUntil = enum { + load, + domcontentloaded, + networkidle, + fixed, + + pub const js_enum_from_string = true; +}; + pub const Fetch = struct { url: [:0]const u8, dump_mode: ?DumpFormat = null, @@ -224,6 +233,8 @@ pub const Fetch = struct { with_base: bool = false, with_frames: bool = false, strip: dump.Opts.Strip = .{}, + wait_ms: u32 = 5000, + wait_until: WaitUntil = .load, }; pub const Common = struct { @@ -619,8 +630,34 @@ fn parseFetchArgs( var url: ?[:0]const u8 = null; var common: Common = .{}; var strip: dump.Opts.Strip = .{}; + var wait_ms: u32 = 5000; + var wait_until: WaitUntil = .load; while (args.next()) |opt| { + if (std.mem.eql(u8, "--wait_ms", opt)) { + const str = args.next() orelse { + log.fatal(.app, "missing argument value", .{ .arg = "--wait_ms" }); + return error.InvalidArgument; + }; + wait_ms = std.fmt.parseInt(u32, str, 10) catch |err| { + log.fatal(.app, "invalid argument value", .{ .arg = "--wait_ms", .err = err }); + return error.InvalidArgument; + }; + continue; + } + + if (std.mem.eql(u8, "--wait_until", opt)) { + const str = args.next() orelse { + log.fatal(.app, "missing argument value", .{ .arg = "--wait_until" }); + return error.InvalidArgument; + }; + wait_until = std.meta.stringToEnum(WaitUntil, str) orelse { + log.fatal(.app, "invalid argument value", .{ .arg = "--wait_until", .val = str }); + return error.InvalidArgument; + }; + continue; + } + if (std.mem.eql(u8, "--dump", opt)) { var peek_args = args.*; if (peek_args.next()) |next_arg| { @@ -709,6 +746,8 @@ fn parseFetchArgs( .common = common, .with_base = with_base, .with_frames = with_frames, + .wait_ms = wait_ms, + .wait_until = wait_until, }; } diff --git a/src/browser/Session.zig b/src/browser/Session.zig index 73b6b26e..cc965757 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -319,15 +319,15 @@ fn findPageBy(page: *Page, comptime field: []const u8, id: u32) ?*Page { return null; } -pub fn wait(self: *Session, wait_ms: u32) WaitResult { +pub fn wait(self: *Session, wait_ms: u32, wait_until: lp.Config.WaitUntil) WaitResult { var page = &(self.page orelse return .no_page); while (true) { - const wait_result = self._wait(page, wait_ms) catch |err| { + const wait_result = self._wait(&page, wait_ms, wait_until) catch |err| { switch (err) { error.JsError => {}, // already logged (with hopefully more context) else => log.err(.browser, "session wait", .{ .err = err, - .url = page.url, + .url = page.*.url, }), } return .done; @@ -346,7 +346,7 @@ pub fn wait(self: *Session, wait_ms: u32) WaitResult { } } -fn _wait(self: *Session, page: *Page, wait_ms: u32) !WaitResult { +fn _wait(self: *Session, page: **Page, wait_ms: u32, wait_until: lp.Config.WaitUntil) !WaitResult { var timer = try std.time.Timer.start(); var ms_remaining = wait_ms; @@ -366,13 +366,15 @@ fn _wait(self: *Session, page: *Page, wait_ms: u32) !WaitResult { const exit_when_done = http_client.cdp_client == null; while (true) { - switch (page._parse_state) { + switch (page.*._parse_state) { .pre, .raw, .text, .image => { // The main page hasn't started/finished navigating. // There's no JS to run, and no reason to run the scheduler. if (http_client.active == 0 and exit_when_done) { // haven't started navigating, I guess. - return .done; + if (wait_until != .fixed) { + return .done; + } } // Either we have active http connections, or we're in CDP // mode with an extra socket. Either way, we're waiting @@ -404,15 +406,15 @@ fn _wait(self: *Session, page: *Page, wait_ms: u32) !WaitResult { try browser.runMacrotasks(); // Each call to this runs scheduled load events. - try page.dispatchLoad(); + try page.*.dispatchLoad(); const http_active = http_client.active; const total_network_activity = http_active + http_client.intercepted; - if (page._notified_network_almost_idle.check(total_network_activity <= 2)) { - page.notifyNetworkAlmostIdle(); + if (page.*._notified_network_almost_idle.check(total_network_activity <= 2)) { + page.*.notifyNetworkAlmostIdle(); } - if (page._notified_network_idle.check(total_network_activity == 0)) { - page.notifyNetworkIdle(); + if (page.*._notified_network_idle.check(total_network_activity == 0)) { + page.*.notifyNetworkIdle(); } if (http_active == 0 and exit_when_done) { @@ -423,17 +425,14 @@ fn _wait(self: *Session, page: *Page, wait_ms: u32) !WaitResult { std.debug.assert(http_client.intercepted == 0); } - var ms = blk: { - // if (wait_ms - ms_remaining < 100) { - // if (comptime builtin.is_test) { - // return .done; - // } - // // Look, we want to exit ASAP, but we don't want - // // to exit so fast that we've run none of the - // // background jobs. - // break :blk 50; - // } + const is_event_done = switch (wait_until) { + .fixed => false, + .domcontentloaded => (page.*._load_state == .load or page.*._load_state == .complete), + .load => (page.*._load_state == .complete), + .networkidle => (page.*._load_state == .complete and http_active == 0), + }; + var ms = blk: { if (browser.hasBackgroundTasks()) { // _we_ have nothing to run, but v8 is working on // background tasks. We'll wait for them. @@ -441,19 +440,27 @@ fn _wait(self: *Session, page: *Page, wait_ms: u32) !WaitResult { break :blk 20; } - break :blk browser.msToNextMacrotask() orelse return .done; + const next_task = browser.msToNextMacrotask(); + if (next_task == null and is_event_done) { + return .done; + } + break :blk next_task orelse 20; }; if (ms > ms_remaining) { + if (is_event_done) { + return .done; + } // Same as above, except we have a scheduled task, // it just happens to be too far into the future // compared to how long we were told to wait. if (!browser.hasBackgroundTasks()) { - return .done; + if (is_event_done) return .done; + } else { + // _we_ have nothing to run, but v8 is working on + // background tasks. We'll wait for them. + browser.waitForBackgroundTasks(); } - // _we_ have nothing to run, but v8 is working on - // background tasks. We'll wait for them. - browser.waitForBackgroundTasks(); ms = 20; } @@ -484,7 +491,7 @@ fn _wait(self: *Session, page: *Page, wait_ms: u32) !WaitResult { } }, .err => |err| { - page._parse_state = .{ .raw_done = @errorName(err) }; + page.*._parse_state = .{ .raw_done = @errorName(err) }; return err; }, .raw_done => { diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig index 58ed11b9..e3456226 100644 --- a/src/cdp/cdp.zig +++ b/src/cdp/cdp.zig @@ -131,7 +131,7 @@ pub fn CDPT(comptime TypeProvider: type) type { // timeouts (or http events) which are ready to be processed. pub fn pageWait(self: *Self, ms: u32) Session.WaitResult { const session = &(self.browser.session orelse return .no_page); - return session.wait(ms); + return session.wait(ms, .load); } // Called from above, in processMessage which handles client messages diff --git a/src/cdp/domains/lp.zig b/src/cdp/domains/lp.zig index 19fc8cac..6c90571e 100644 --- a/src/cdp/domains/lp.zig +++ b/src/cdp/domains/lp.zig @@ -280,7 +280,7 @@ test "cdp.lp: action tools" { const page = try bc.session.createPage(); const url = "http://localhost:9582/src/browser/tests/mcp_actions.html"; try page.navigate(url, .{ .reason = .address_bar, .kind = .{ .push = null } }); - _ = bc.session.wait(5000); + _ = bc.session.wait(5000, .load); // Test Click const btn = page.document.getElementById("btn", page).?.asNode(); diff --git a/src/cdp/testing.zig b/src/cdp/testing.zig index e07cab44..9e739a31 100644 --- a/src/cdp/testing.zig +++ b/src/cdp/testing.zig @@ -136,7 +136,7 @@ const TestContext = struct { 0, ); try page.navigate(full_url, .{}); - _ = bc.session.wait(2000); + _ = bc.session.wait(2000, .load); } return bc; } diff --git a/src/lightpanda.zig b/src/lightpanda.zig index a9c7a1f0..6096f2a8 100644 --- a/src/lightpanda.zig +++ b/src/lightpanda.zig @@ -46,6 +46,7 @@ const IS_DEBUG = @import("builtin").mode == .Debug; pub const FetchOpts = struct { wait_ms: u32 = 5000, + wait_until: Config.WaitUntil = .load, dump: dump.Opts, dump_mode: ?Config.DumpFormat = null, writer: ?*std.Io.Writer = null, @@ -107,7 +108,7 @@ pub fn fetch(app: *App, url: [:0]const u8, opts: FetchOpts) !void { .reason = .address_bar, .kind = .{ .push = null }, }); - _ = session.wait(opts.wait_ms); + _ = session.wait(opts.wait_ms, opts.wait_until); const writer = opts.writer orelse return; if (opts.dump_mode) |mode| { diff --git a/src/main.zig b/src/main.zig index 1b5ffdb2..93404acc 100644 --- a/src/main.zig +++ b/src/main.zig @@ -120,7 +120,8 @@ fn run(allocator: Allocator, main_arena: Allocator) !void { log.debug(.app, "startup", .{ .mode = "fetch", .dump_mode = opts.dump_mode, .url = url, .snapshot = app.snapshot.fromEmbedded() }); var fetch_opts = lp.FetchOpts{ - .wait_ms = 5000, + .wait_ms = opts.wait_ms, + .wait_until = opts.wait_until, .dump_mode = opts.dump_mode, .dump = .{ .strip = opts.strip, diff --git a/src/main_legacy_test.zig b/src/main_legacy_test.zig index a6d1593f..839fd484 100644 --- a/src/main_legacy_test.zig +++ b/src/main_legacy_test.zig @@ -106,7 +106,7 @@ pub fn run(allocator: Allocator, file: []const u8, session: *lp.Session) !void { defer try_catch.deinit(); try page.navigate(url, .{}); - _ = session.wait(2000); + _ = session.wait(2000, .load); ls.local.eval("testing.assertOk()", "testing.assertOk()") catch |err| { const caught = try_catch.caughtOrError(allocator, err); diff --git a/src/mcp/tools.zig b/src/mcp/tools.zig index d8fd4ead..f1b6cf7c 100644 --- a/src/mcp/tools.zig +++ b/src/mcp/tools.zig @@ -538,7 +538,7 @@ fn performGoto(server: *Server, url: [:0]const u8, id: std.json.Value) !void { return error.NavigationFailed; }; - _ = server.session.wait(5000); + _ = server.session.wait(5000, .load); } const testing = @import("../testing.zig"); @@ -603,7 +603,7 @@ test "MCP - Actions: click, fill, scroll" { const page = try server.session.createPage(); const url = "http://localhost:9582/src/browser/tests/mcp_actions.html"; try page.navigate(url, .{ .reason = .address_bar, .kind = .{ .push = null } }); - _ = server.session.wait(5000); + _ = server.session.wait(5000, .load); // Test Click const btn = page.document.getElementById("btn", page).?.asNode(); diff --git a/src/testing.zig b/src/testing.zig index adebdc32..0f9747d0 100644 --- a/src/testing.zig +++ b/src/testing.zig @@ -415,7 +415,7 @@ fn runWebApiTest(test_file: [:0]const u8) !void { defer try_catch.deinit(); try page.navigate(url, .{}); - _ = test_session.wait(2000); + _ = test_session.wait(2000, .load); test_browser.runMicrotasks(); @@ -439,7 +439,7 @@ pub fn pageTest(comptime test_file: []const u8) !*Page { ); try page.navigate(url, .{}); - _ = test_session.wait(2000); + _ = test_session.wait(2000, .load); return page; } From c15afa23cad4f46b56df0b1e7ae435955e89e034 Mon Sep 17 00:00:00 2001 From: shaewe180 Date: Thu, 19 Mar 2026 09:36:42 +0800 Subject: [PATCH 02/15] Session: fix page pointer handling in wait loop - Refactor `wait` and `_wait` to handle `page` as `*Page` instead of `**Page`, preventing stale references during navigations. - Update `networkidle` wait condition to use `_notified_network_idle == .done`. - Document `--wait_ms` and `--wait_until` options in `Config.zig` help text. --- src/Config.zig | 7 +++++++ src/browser/Session.zig | 30 +++++++++++++++--------------- 2 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/Config.zig b/src/Config.zig index f65ecd5d..d082e92e 100644 --- a/src/Config.zig +++ b/src/Config.zig @@ -398,6 +398,13 @@ pub fn printUsageAndExit(self: *const Config, success: bool) void { \\ \\--with_frames Includes the contents of iframes. Defaults to false. \\ + \\--wait_ms Wait time in milliseconds. + \\ Defaults to 5000. + \\ + \\--wait_until Wait until the specified event. + \\ Supported events: load, domcontentloaded, networkidle, fixed. + \\ Defaults to 'load'. + \\ ++ common_options ++ \\ \\serve command diff --git a/src/browser/Session.zig b/src/browser/Session.zig index cc965757..43576bf5 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -320,14 +320,15 @@ fn findPageBy(page: *Page, comptime field: []const u8, id: u32) ?*Page { } pub fn wait(self: *Session, wait_ms: u32, wait_until: lp.Config.WaitUntil) WaitResult { - var page = &(self.page orelse return .no_page); while (true) { - const wait_result = self._wait(&page, wait_ms, wait_until) catch |err| { + if (self.page == null) return .no_page; + const page = &self.page.?; + const wait_result = self._wait(page, wait_ms, wait_until) catch |err| { switch (err) { error.JsError => {}, // already logged (with hopefully more context) else => log.err(.browser, "session wait", .{ .err = err, - .url = page.*.url, + .url = page.url, }), } return .done; @@ -339,14 +340,13 @@ pub fn wait(self: *Session, wait_ms: u32, wait_until: lp.Config.WaitUntil) WaitR return .done; } self.processQueuedNavigation() catch return .done; - page = &self.page.?; // might have changed }, else => |result| return result, } } } -fn _wait(self: *Session, page: **Page, wait_ms: u32, wait_until: lp.Config.WaitUntil) !WaitResult { +fn _wait(self: *Session, page: *Page, wait_ms: u32, wait_until: lp.Config.WaitUntil) !WaitResult { var timer = try std.time.Timer.start(); var ms_remaining = wait_ms; @@ -366,7 +366,7 @@ fn _wait(self: *Session, page: **Page, wait_ms: u32, wait_until: lp.Config.WaitU const exit_when_done = http_client.cdp_client == null; while (true) { - switch (page.*._parse_state) { + switch (page._parse_state) { .pre, .raw, .text, .image => { // The main page hasn't started/finished navigating. // There's no JS to run, and no reason to run the scheduler. @@ -406,15 +406,15 @@ fn _wait(self: *Session, page: **Page, wait_ms: u32, wait_until: lp.Config.WaitU try browser.runMacrotasks(); // Each call to this runs scheduled load events. - try page.*.dispatchLoad(); + try page.dispatchLoad(); const http_active = http_client.active; const total_network_activity = http_active + http_client.intercepted; - if (page.*._notified_network_almost_idle.check(total_network_activity <= 2)) { - page.*.notifyNetworkAlmostIdle(); + if (page._notified_network_almost_idle.check(total_network_activity <= 2)) { + page.notifyNetworkAlmostIdle(); } - if (page.*._notified_network_idle.check(total_network_activity == 0)) { - page.*.notifyNetworkIdle(); + if (page._notified_network_idle.check(total_network_activity == 0)) { + page.notifyNetworkIdle(); } if (http_active == 0 and exit_when_done) { @@ -427,9 +427,9 @@ fn _wait(self: *Session, page: **Page, wait_ms: u32, wait_until: lp.Config.WaitU const is_event_done = switch (wait_until) { .fixed => false, - .domcontentloaded => (page.*._load_state == .load or page.*._load_state == .complete), - .load => (page.*._load_state == .complete), - .networkidle => (page.*._load_state == .complete and http_active == 0), + .domcontentloaded => (page._load_state == .load or page._load_state == .complete), + .load => (page._load_state == .complete), + .networkidle => (page._notified_network_idle == .done), }; var ms = blk: { @@ -491,7 +491,7 @@ fn _wait(self: *Session, page: **Page, wait_ms: u32, wait_until: lp.Config.WaitU } }, .err => |err| { - page.*._parse_state = .{ .raw_done = @errorName(err) }; + page._parse_state = .{ .raw_done = @errorName(err) }; return err; }, .raw_done => { From e2be8525c44688cf6006c4f42a06bfef9b6f135a Mon Sep 17 00:00:00 2001 From: shaewe180 Date: Thu, 19 Mar 2026 09:40:40 +0800 Subject: [PATCH 03/15] Config: remove js_enum_from_string constant --- src/Config.zig | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/Config.zig b/src/Config.zig index d082e92e..0bec5b7a 100644 --- a/src/Config.zig +++ b/src/Config.zig @@ -222,8 +222,6 @@ pub const WaitUntil = enum { domcontentloaded, networkidle, fixed, - - pub const js_enum_from_string = true; }; pub const Fetch = struct { From db01158d2d9830293ff55ea26f56851b4abe97ca Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 19 Mar 2026 11:36:39 +0800 Subject: [PATCH 04/15] Improve unhandled rejection We now pay attention to the type of event that causes the unhandled exception. This allows us to trigger the window.rejectionhandled event when that is the correct type. It also lets us no-op for other event types which should not trigger rejectionhandled or unhandledrejection. Fixes stackoverflow in github integration. --- src/browser/js/Env.zig | 7 ++++++- src/browser/webapi/Window.zig | 27 ++++++++++++++++++++++----- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index 09117eb0..d1aed2cc 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -495,6 +495,11 @@ pub fn terminate(self: *const Env) void { } fn promiseRejectCallback(message_handle: v8.PromiseRejectMessage) callconv(.c) void { + const promise_event = v8.v8__PromiseRejectMessage__GetEvent(&message_handle); + if (promise_event != v8.kPromiseRejectWithNoHandler and promise_event != v8.kPromiseHandlerAddedAfterReject) { + return; + } + const promise_handle = v8.v8__PromiseRejectMessage__GetPromise(&message_handle).?; const v8_isolate = v8.v8__Object__GetIsolate(@ptrCast(promise_handle)).?; const isolate = js.Isolate{ .handle = v8_isolate }; @@ -508,7 +513,7 @@ fn promiseRejectCallback(message_handle: v8.PromiseRejectMessage) callconv(.c) v }; const page = ctx.page; - page.window.unhandledPromiseRejection(.{ + page.window.unhandledPromiseRejection(promise_event == v8.kPromiseRejectWithNoHandler, .{ .local = &local, .handle = &message_handle, }, page) catch |err| { diff --git a/src/browser/webapi/Window.zig b/src/browser/webapi/Window.zig index d3a8ebfc..bbed1010 100644 --- a/src/browser/webapi/Window.zig +++ b/src/browser/webapi/Window.zig @@ -67,7 +67,8 @@ _on_pageshow: ?js.Function.Global = null, _on_popstate: ?js.Function.Global = null, _on_error: ?js.Function.Global = null, _on_message: ?js.Function.Global = null, -_on_unhandled_rejection: ?js.Function.Global = null, // TODO: invoke on error +_on_rejection_handled: ?js.Function.Global = null, +_on_unhandled_rejection: ?js.Function.Global = null, _current_event: ?*Event = null, _location: *Location, _timer_id: u30 = 0, @@ -222,6 +223,14 @@ pub fn setOnMessage(self: *Window, setter: ?FunctionSetter) void { self._on_message = getFunctionFromSetter(setter); } +pub fn getOnRejectionHandled(self: *const Window) ?js.Function.Global { + return self._on_rejection_handled; +} + +pub fn setOnRejectionHandled(self: *Window, setter: ?FunctionSetter) void { + self._on_rejection_handled = getFunctionFromSetter(setter); +} + pub fn getOnUnhandledRejection(self: *const Window) ?js.Function.Global { return self._on_unhandled_rejection; } @@ -572,7 +581,7 @@ pub fn scrollBy(self: *Window, opts: ScrollToOpts, y: ?i32, page: *Page) !void { return self.scrollTo(.{ .x = absx }, absy, page); } -pub fn unhandledPromiseRejection(self: *Window, rejection: js.PromiseRejection, page: *Page) !void { +pub fn unhandledPromiseRejection(self: *Window, no_handler: bool, rejection: js.PromiseRejection, page: *Page) !void { if (comptime IS_DEBUG) { log.debug(.js, "unhandled rejection", .{ .value = rejection.reason(), @@ -580,13 +589,20 @@ pub fn unhandledPromiseRejection(self: *Window, rejection: js.PromiseRejection, }); } + const event_name, const attribute_callback = blk: { + if (no_handler) { + break :blk .{ "unhandledrejection", self._on_unhandled_rejection }; + } + break :blk .{ "rejectionhandled", self._on_rejection_handled }; + }; + const target = self.asEventTarget(); - if (page._event_manager.hasDirectListeners(target, "unhandledrejection", self._on_unhandled_rejection)) { - const event = (try @import("event/PromiseRejectionEvent.zig").init("unhandledrejection", .{ + if (page._event_manager.hasDirectListeners(target, event_name, attribute_callback)) { + const event = (try @import("event/PromiseRejectionEvent.zig").init(event_name, .{ .reason = if (rejection.reason()) |r| try r.temp() else null, .promise = try rejection.promise().temp(), }, page)).asEvent(); - try page._event_manager.dispatchDirect(target, event, self._on_unhandled_rejection, .{ .context = "window.unhandledrejection" }); + try page._event_manager.dispatchDirect(target, event, attribute_callback, .{ .context = "window.unhandledrejection" }); } } @@ -813,6 +829,7 @@ pub const JsApi = struct { pub const onpopstate = bridge.accessor(Window.getOnPopState, Window.setOnPopState, .{}); pub const onerror = bridge.accessor(Window.getOnError, Window.setOnError, .{}); pub const onmessage = bridge.accessor(Window.getOnMessage, Window.setOnMessage, .{}); + pub const onrejectionhandled = bridge.accessor(Window.getOnRejectionHandled, Window.setOnRejectionHandled, .{}); pub const onunhandledrejection = bridge.accessor(Window.getOnUnhandledRejection, Window.setOnUnhandledRejection, .{}); pub const event = bridge.accessor(Window.getEvent, null, .{ .null_as_undefined = true }); pub const fetch = bridge.function(Window.fetch, .{}); From 2b9d5fd4d998da1e168dca140292b221ffeb15e2 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 19 Mar 2026 12:09:10 +0800 Subject: [PATCH 05/15] Add `adoptedStyleSheets` property to ShadowRoot, just like Document Used in github. --- src/browser/tests/shadowroot/basic.html | 17 +++++++++++++++-- src/browser/webapi/ShadowRoot.zig | 16 ++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/browser/tests/shadowroot/basic.html b/src/browser/tests/shadowroot/basic.html index ed82ab18..3516fa79 100644 --- a/src/browser/tests/shadowroot/basic.html +++ b/src/browser/tests/shadowroot/basic.html @@ -5,7 +5,7 @@
- + + + + diff --git a/src/browser/webapi/ShadowRoot.zig b/src/browser/webapi/ShadowRoot.zig index 09bfa2d9..a5e54923 100644 --- a/src/browser/webapi/ShadowRoot.zig +++ b/src/browser/webapi/ShadowRoot.zig @@ -40,6 +40,7 @@ _mode: Mode, _host: *Element, _elements_by_id: std.StringHashMapUnmanaged(*Element) = .{}, _removed_ids: std.StringHashMapUnmanaged(void) = .{}, +_adopted_style_sheets: ?js.Object.Global = null, pub fn init(host: *Element, mode: Mode, page: *Page) !*ShadowRoot { return page._factory.documentFragment(ShadowRoot{ @@ -99,6 +100,20 @@ pub fn getElementById(self: *ShadowRoot, id: []const u8, page: *Page) ?*Element return null; } +pub fn getAdoptedStyleSheets(self: *ShadowRoot, page: *Page) !js.Object.Global { + if (self._adopted_style_sheets) |ass| { + return ass; + } + const js_arr = page.js.local.?.newArray(0); + const js_obj = js_arr.toObject(); + self._adopted_style_sheets = try js_obj.persist(); + return self._adopted_style_sheets.?; +} + +pub fn setAdoptedStyleSheets(self: *ShadowRoot, sheets: js.Object) !void { + self._adopted_style_sheets = try sheets.persist(); +} + pub const JsApi = struct { pub const bridge = js.Bridge(ShadowRoot); @@ -121,6 +136,7 @@ pub const JsApi = struct { } return self.getElementById(try value.toZig([]const u8), page); } + pub const adoptedStyleSheets = bridge.accessor(ShadowRoot.getAdoptedStyleSheets, ShadowRoot.setAdoptedStyleSheets, .{}); }; const testing = @import("../../testing.zig"); From d9c5f5650085f9bfabb131312fb10cc28fb66c42 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 18 Mar 2026 18:59:09 +0800 Subject: [PATCH 06/15] Remove Origins js.Origin was added to allow frames on the same origin to share our zig<->js maps / identity. It assumes that scripts on different origins will never be allowed (by v8) to access the same zig instances. If two different origins DID access the same zig instance, we'd have a few different problems. First, while the mapping would exist in Origin1's identity_map, when the zig instance was returned to a script in Origin2, it would not be found in Origin2's identity_map, and thus create a new v8::Object. Thus we'd end up with 2 v8::Objects for the same Zig instance. This is potentially not the end of the world, but not great either as any zig-native data _would_ be shared (it's the same instance after all), but js-native data wouldn't. The real problem this introduces though is with Finalizers. A weak reference that falls out of scope in Origin1 will get cleaned up, even though it's still referenced from Origin2. Now, under normal circumstances, this isn't an issue; v8 _does_ ensure that cross-origin access isn't allowed (because we set a SecurityToken on the v8::Context). But it seems like the v8::Inspector isn't bound by these restrictions and can happily access and share objects across origin. The simplest solution I can come up with is to move the mapping from the Origin to the Session. This does mean that objects might live longer than they have to. When all references to an origin go out of scope, we can do some cleanup. Not so when the Session owns this data. But really, how often are iframes on different origins being created and deleted within the lifetime of a page? When Origins were first introduces, the Session got burdened with having to manage multiple lifecycles: 1 - The page-surviving data (e.g. history) 2 - The root page lifecycle (e.g. page_arena, queuedNavigation) 3 - The origin lookup This commit doesn't change that, but it makes the session responsible for _a lot_ more of the root page lifecycle (#2 above). I lied. js.Origin still exists, but it's a shell of its former self. It only exists to store the SecurityToken name that is re-used for every context with the same origin. The v8 namespace leaks into Session. MutationObserver and IntersectionObserver are now back to using weak/strong refs which was one of the failing cases before this change. --- src/browser/Session.zig | 146 +++++++++++++- src/browser/js/Context.zig | 13 +- src/browser/js/Env.zig | 17 +- src/browser/js/Function.zig | 9 +- src/browser/js/Local.zig | 13 +- src/browser/js/Origin.zig | 204 +------------------- src/browser/js/Promise.zig | 10 +- src/browser/js/String.zig | 2 +- src/browser/js/Value.zig | 9 +- src/browser/js/bridge.zig | 11 +- src/browser/webapi/IntersectionObserver.zig | 22 ++- src/browser/webapi/MutationObserver.zig | 18 +- 12 files changed, 215 insertions(+), 259 deletions(-) diff --git a/src/browser/Session.zig b/src/browser/Session.zig index 73b6b26e..6376ff03 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -24,6 +24,7 @@ const log = @import("../log.zig"); const App = @import("../App.zig"); const js = @import("js/js.zig"); +const v8 = js.v8; const storage = @import("webapi/storage/storage.zig"); const Navigation = @import("webapi/navigation/Navigation.zig"); const History = @import("webapi/History.zig"); @@ -65,6 +66,19 @@ page_arena: Allocator, // Origin map for same-origin context sharing. Scoped to the root page lifetime. origins: std.StringHashMapUnmanaged(*js.Origin) = .empty, +// Session-scoped identity tracking (for the lifetime of the root page). +// Maps Zig instance pointers to their v8::Global(Object) wrappers. +identity_map: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, + +// Tracked global v8 objects that need to be released when the page is reset. +globals: std.ArrayList(v8.Global) = .empty, + +// Temporary v8 globals that can be released early. Key is global.data_ptr. +temps: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, + +// Finalizer callbacks for weak references. Key is @intFromPtr of the Zig instance. +finalizer_callbacks: std.AutoHashMapUnmanaged(usize, *FinalizerCallback) = .empty, + // Shared resources for all pages in this session. // These live for the duration of the page tree (root + frames). arena_pool: *ArenaPool, @@ -84,8 +98,8 @@ queued_navigation: std.ArrayList(*Page), // about:blank navigations (which may add to queued_navigation). queued_queued_navigation: std.ArrayList(*Page), -page_id_gen: u32, -frame_id_gen: u32, +page_id_gen: u32 = 0, +frame_id_gen: u32 = 0, pub fn init(self: *Session, browser: *Browser, notification: *Notification) !void { const allocator = browser.app.allocator; @@ -104,8 +118,6 @@ pub fn init(self: *Session, browser: *Browser, notification: *Notification) !voi .page_arena = page_arena, .factory = Factory.init(page_arena), .history = .{}, - .page_id_gen = 0, - .frame_id_gen = 0, // The prototype (EventTarget) for Navigation is created when a Page is created. .navigation = .{ ._proto = undefined }, .storage_shed = .{}, @@ -174,7 +186,7 @@ pub fn getArena(self: *Session, opts: GetArenaOpts) !Allocator { const allocator = try self.arena_pool.acquire(); if (comptime IS_DEBUG) { // Use session's arena (not page_arena) since page_arena gets reset between pages - const gop = try self._arena_pool_leak_track.getOrPut(self.arena, @intFromPtr(allocator.ptr)); + const gop = try self._arena_pool_leak_track.getOrPut(self.page_arena, @intFromPtr(allocator.ptr)); if (gop.found_existing and gop.value_ptr.count != 0) { log.err(.bug, "ArenaPool Double Use", .{ .owner = gop.value_ptr.*.owner }); @panic("ArenaPool Double Use"); @@ -237,7 +249,35 @@ pub fn releaseOrigin(self: *Session, origin: *js.Origin) void { /// Reset page_arena and factory for a clean slate. /// Called when root page is removed. fn resetPageResources(self: *Session) void { - // Check for arena leaks before releasing + { + var it = self.finalizer_callbacks.valueIterator(); + while (it.next()) |finalizer| { + finalizer.*.deinit(); + } + self.finalizer_callbacks = .empty; + } + + { + var it = self.identity_map.valueIterator(); + while (it.next()) |global| { + v8.v8__Global__Reset(global); + } + self.identity_map = .empty; + } + + for (self.globals.items) |*global| { + v8.v8__Global__Reset(global); + } + self.globals = .empty; + + { + var it = self.temps.valueIterator(); + while (it.next()) |global| { + v8.v8__Global__Reset(global); + } + self.temps = .empty; + } + if (comptime IS_DEBUG) { var it = self._arena_pool_leak_track.valueIterator(); while (it.next()) |value_ptr| { @@ -245,10 +285,9 @@ fn resetPageResources(self: *Session) void { log.err(.bug, "ArenaPool Leak", .{ .owner = value_ptr.owner }); } } - self._arena_pool_leak_track.clearRetainingCapacity(); + self._arena_pool_leak_track = .empty; } - // All origins should have been released when contexts were destroyed if (comptime IS_DEBUG) { std.debug.assert(self.origins.count() == 0); } @@ -259,10 +298,9 @@ fn resetPageResources(self: *Session) void { while (it.next()) |value| { value.*.deinit(app); } - self.origins.clearRetainingCapacity(); + self.origins = .empty; } - // Release old page_arena and acquire fresh one self.frame_id_gen = 0; self.arena_pool.reset(self.page_arena, 64 * 1024); self.factory = Factory.init(self.page_arena); @@ -672,3 +710,91 @@ pub fn nextPageId(self: *Session) u32 { self.page_id_gen = id; return id; } + +// These methods manage the mapping between Zig instances and v8 objects, +// scoped to the lifetime of the root page. +pub fn trackGlobal(self: *Session, global: v8.Global) !void { + return self.globals.append(self.page_arena, global); +} + +pub const IdentityResult = struct { + value_ptr: *v8.Global, + found_existing: bool, +}; + +pub fn addIdentity(self: *Session, ptr: usize) !IdentityResult { + const gop = try self.identity_map.getOrPut(self.page_arena, ptr); + return .{ + .value_ptr = gop.value_ptr, + .found_existing = gop.found_existing, + }; +} + +pub fn trackTemp(self: *Session, global: v8.Global) !void { + return self.temps.put(self.page_arena, global.data_ptr, global); +} + +pub fn releaseTemp(self: *Session, global: v8.Global) void { + if (self.temps.fetchRemove(global.data_ptr)) |kv| { + var g = kv.value; + v8.v8__Global__Reset(&g); + } +} + +/// Release an item from the identity_map (called after finalizer runs from V8) +pub fn releaseIdentity(self: *Session, item: *anyopaque) void { + var global = self.identity_map.fetchRemove(@intFromPtr(item)) orelse { + if (comptime IS_DEBUG) { + std.debug.assert(false); + } + return; + }; + v8.v8__Global__Reset(&global.value); + + // The item has been finalized, remove it from the finalizer callback so that + // we don't try to call it again on shutdown. + const kv = self.finalizer_callbacks.fetchRemove(@intFromPtr(item)) orelse { + if (comptime IS_DEBUG) { + std.debug.assert(false); + } + return; + }; + const fc = kv.value; + self.releaseArena(fc.arena); +} + +pub fn createFinalizerCallback( + self: *Session, + global: v8.Global, + ptr: *anyopaque, + zig_finalizer: *const fn (ptr: *anyopaque, session: *Session) void, +) !*FinalizerCallback { + const arena = try self.getArena(.{ .debug = "FinalizerCallback" }); + errdefer self.releaseArena(arena); + const fc = try arena.create(FinalizerCallback); + fc.* = .{ + .arena = arena, + .session = self, + .ptr = ptr, + .global = global, + .zig_finalizer = zig_finalizer, + }; + return fc; +} + +// A type that has a finalizer can have its finalizer called one of two ways. +// The first is from V8 via the WeakCallback we give to weakRef. But that isn't +// guaranteed to fire, so we track this in finalizer_callbacks and call them on +// page reset. +pub const FinalizerCallback = struct { + arena: Allocator, + session: *Session, + ptr: *anyopaque, + global: v8.Global, + zig_finalizer: *const fn (ptr: *anyopaque, session: *Session) void, + + pub fn deinit(self: *FinalizerCallback) void { + self.zig_finalizer(self.ptr, self.session); + self.session.releaseArena(self.arena); + } +}; diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index da7362aa..5ea9454f 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -185,9 +185,8 @@ pub fn setOrigin(self: *Context, key: ?[]const u8) !void { lp.assert(self.origin.rc == 1, "Ref opaque origin", .{ .rc = self.origin.rc }); const origin = try self.session.getOrCreateOrigin(key); - errdefer self.session.releaseOrigin(origin); - try origin.takeover(self.origin); + self.session.releaseOrigin(self.origin); self.origin = origin; { @@ -203,16 +202,16 @@ pub fn setOrigin(self: *Context, key: ?[]const u8) !void { } pub fn trackGlobal(self: *Context, global: v8.Global) !void { - return self.origin.trackGlobal(global); + return self.session.trackGlobal(global); } pub fn trackTemp(self: *Context, global: v8.Global) !void { - return self.origin.trackTemp(global); + return self.session.trackTemp(global); } pub fn weakRef(self: *Context, obj: anytype) void { const resolved = js.Local.resolveValue(obj); - const fc = self.origin.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { + const fc = self.session.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { if (comptime IS_DEBUG) { // should not be possible std.debug.assert(false); @@ -224,7 +223,7 @@ pub fn weakRef(self: *Context, obj: anytype) void { pub fn safeWeakRef(self: *Context, obj: anytype) void { const resolved = js.Local.resolveValue(obj); - const fc = self.origin.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { + const fc = self.session.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { if (comptime IS_DEBUG) { // should not be possible std.debug.assert(false); @@ -237,7 +236,7 @@ pub fn safeWeakRef(self: *Context, obj: anytype) void { pub fn strongRef(self: *Context, obj: anytype) void { const resolved = js.Local.resolveValue(obj); - const fc = self.origin.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { + const fc = self.session.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { if (comptime IS_DEBUG) { // should not be possible std.debug.assert(false); diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index 09117eb0..c417a6e5 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -307,16 +307,17 @@ pub fn createContext(self: *Env, page: *Page) !*Context { const context_id = self.context_id; self.context_id = context_id + 1; - const origin = try page._session.getOrCreateOrigin(null); - errdefer page._session.releaseOrigin(origin); + const session = page._session; + const origin = try session.getOrCreateOrigin(null); + errdefer session.releaseOrigin(origin); const context = try context_arena.create(Context); context.* = .{ .env = self, .page = page, - .session = page._session, .origin = origin, .id = context_id, + .session = session, .isolate = isolate, .arena = context_arena, .handle = context_global, @@ -326,7 +327,15 @@ pub fn createContext(self: *Env, page: *Page) !*Context { .script_manager = &page._script_manager, .scheduler = .init(context_arena), }; - try context.origin.identity_map.putNoClobber(origin.arena, @intFromPtr(page.window), global_global); + + { + // Multiple contexts can be created for the same Window (via CDP). We only + // need to register the first one. + const gop = try session.identity_map.getOrPut(session.page_arena, @intFromPtr(page.window)); + if (gop.found_existing == false) { + gop.value_ptr.* = global_global; + } + } // Store a pointer to our context inside the v8 context so that, given // a v8 context, we can get our context out diff --git a/src/browser/js/Function.zig b/src/browser/js/Function.zig index bfb5e53d..e2c10e9f 100644 --- a/src/browser/js/Function.zig +++ b/src/browser/js/Function.zig @@ -21,6 +21,7 @@ const js = @import("js.zig"); const v8 = js.v8; const log = @import("../../log.zig"); +const Session = @import("../Session.zig"); const Function = @This(); @@ -210,10 +211,10 @@ fn _persist(self: *const Function, comptime is_global: bool) !(if (is_global) Gl v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); if (comptime is_global) { try ctx.trackGlobal(global); - return .{ .handle = global, .origin = {} }; + return .{ .handle = global, .session = {} }; } try ctx.trackTemp(global); - return .{ .handle = global, .origin = ctx.origin }; + return .{ .handle = global, .session = ctx.session }; } pub fn tempWithThis(self: *const Function, value: anytype) !Temp { @@ -237,7 +238,7 @@ const GlobalType = enum(u8) { fn G(comptime global_type: GlobalType) type { return struct { handle: v8.Global, - origin: if (global_type == .temp) *js.Origin else void, + session: if (global_type == .temp) *Session else void, const Self = @This(); @@ -257,7 +258,7 @@ fn G(comptime global_type: GlobalType) type { } pub fn release(self: *const Self) void { - self.origin.releaseTemp(self.handle); + self.session.releaseTemp(self.handle); } }; } diff --git a/src/browser/js/Local.zig b/src/browser/js/Local.zig index a45b35df..ca09fc91 100644 --- a/src/browser/js/Local.zig +++ b/src/browser/js/Local.zig @@ -202,20 +202,21 @@ pub fn compileAndRun(self: *const Local, src: []const u8, name: ?[]const u8) !js // we can just grab it from the identity_map) pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object, value: anytype) !js.Object { const ctx = self.ctx; - const origin_arena = ctx.origin.arena; + const session = ctx.session; + const page_arena = session.page_arena; const T = @TypeOf(value); switch (@typeInfo(T)) { .@"struct" => { // Struct, has to be placed on the heap - const heap = try origin_arena.create(T); + const heap = try page_arena.create(T); heap.* = value; return self.mapZigInstanceToJs(js_obj_handle, heap); }, .pointer => |ptr| { const resolved = resolveValue(value); - const gop = try ctx.origin.addIdentity(@intFromPtr(resolved.ptr)); + const gop = try session.addIdentity(@intFromPtr(resolved.ptr)); if (gop.found_existing) { // we've seen this instance before, return the same object return (js.Object.Global{ .handle = gop.value_ptr.* }).local(self); @@ -244,7 +245,7 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object, // The TAO contains the pointer to our Zig instance as // well as any meta data we'll need to use it later. // See the TaggedOpaque struct for more details. - const tao = try origin_arena.create(TaggedOpaque); + const tao = try page_arena.create(TaggedOpaque); tao.* = .{ .value = resolved.ptr, .prototype_chain = resolved.prototype_chain.ptr, @@ -276,10 +277,10 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object, // Instead, we check if the base has finalizer. The assumption // here is that if a resolve type has a finalizer, then the base // should have a finalizer too. - const fc = try ctx.origin.createFinalizerCallback(ctx.session, gop.value_ptr.*, resolved.ptr, resolved.finalizer_from_zig.?); + const fc = try session.createFinalizerCallback(gop.value_ptr.*, resolved.ptr, resolved.finalizer_from_zig.?); { errdefer fc.deinit(); - try ctx.origin.finalizer_callbacks.put(ctx.origin.arena, @intFromPtr(resolved.ptr), fc); + try session.finalizer_callbacks.put(page_arena, @intFromPtr(resolved.ptr), fc); } conditionallyReference(value); diff --git a/src/browser/js/Origin.zig b/src/browser/js/Origin.zig index 9dc5857b..6f79b005 100644 --- a/src/browser/js/Origin.zig +++ b/src/browser/js/Origin.zig @@ -16,19 +16,20 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -// Origin represents the shared Zig<->JS bridge state for all contexts within -// the same origin. Multiple contexts (frames) from the same origin share a -// single Origin, ensuring that JS objects maintain their identity across frames. +// Origin represents the security token for contexts within the same origin. +// Multiple contexts (frames) from the same origin share a single Origin, +// which provides the V8 SecurityToken that allows cross-context access. +// +// Note: Identity tracking (mapping Zig instances to v8::Objects) is now +// handled at the Session level, scoped to the root page lifetime. const std = @import("std"); const js = @import("js.zig"); const App = @import("../../App.zig"); -const Session = @import("../Session.zig"); const v8 = js.v8; const Allocator = std.mem.Allocator; -const IS_DEBUG = @import("builtin").mode == .Debug; const Origin = @This(); @@ -38,38 +39,10 @@ arena: Allocator, // The key, e.g. lightpanda.io:443 key: []const u8, -// Security token - all contexts in this realm must use the same v8::Value instance +// Security token - all contexts in this origin must use the same v8::Value instance // as their security token for V8 to allow cross-context access security_token: v8.Global, -// Serves two purposes. Like `global_objects`, this is used to free -// every Global(Object) we've created during the lifetime of the realm. -// More importantly, it serves as an identity map - for a given Zig -// instance, we map it to the same Global(Object). -// The key is the @intFromPtr of the Zig value -identity_map: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, - -// Some web APIs have to manage opaque values. Ideally, they use an -// js.Object, but the js.Object has no lifetime guarantee beyond the -// current call. They can call .persist() on their js.Object to get -// a `Global(Object)`. We need to track these to free them. -// This used to be a map and acted like identity_map; the key was -// the @intFromPtr(js_obj.handle). But v8 can re-use address. Without -// a reliable way to know if an object has already been persisted, -// we now simply persist every time persist() is called. -globals: std.ArrayList(v8.Global) = .empty, - -// Temp variants stored in HashMaps for O(1) early cleanup. -// Key is global.data_ptr. -temps: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, - -// Any type that is stored in the identity_map which has a finalizer declared -// will have its finalizer stored here. This is only used when shutting down -// if v8 hasn't called the finalizer directly itself. -finalizer_callbacks: std.AutoHashMapUnmanaged(usize, *FinalizerCallback) = .empty, - -taken_over: std.ArrayList(*Origin), - pub fn init(app: *App, isolate: js.Isolate, key: []const u8) !*Origin { const arena = try app.arena_pool.acquire(); errdefer app.arena_pool.release(arena); @@ -88,175 +61,12 @@ pub fn init(app: *App, isolate: js.Isolate, key: []const u8) !*Origin { .rc = 1, .arena = arena, .key = owned_key, - .temps = .empty, - .globals = .empty, - .taken_over = .empty, .security_token = token_global, }; return self; } pub fn deinit(self: *Origin, app: *App) void { - for (self.taken_over.items) |o| { - o.deinit(app); - } - - // Call finalizers before releasing anything - { - var it = self.finalizer_callbacks.valueIterator(); - while (it.next()) |finalizer| { - finalizer.*.deinit(); - } - } - v8.v8__Global__Reset(&self.security_token); - - { - var it = self.identity_map.valueIterator(); - while (it.next()) |global| { - v8.v8__Global__Reset(global); - } - } - - for (self.globals.items) |*global| { - v8.v8__Global__Reset(global); - } - - { - var it = self.temps.valueIterator(); - while (it.next()) |global| { - v8.v8__Global__Reset(global); - } - } - app.arena_pool.release(self.arena); } - -pub fn trackGlobal(self: *Origin, global: v8.Global) !void { - return self.globals.append(self.arena, global); -} - -pub const IdentityResult = struct { - value_ptr: *v8.Global, - found_existing: bool, -}; - -pub fn addIdentity(self: *Origin, ptr: usize) !IdentityResult { - const gop = try self.identity_map.getOrPut(self.arena, ptr); - return .{ - .value_ptr = gop.value_ptr, - .found_existing = gop.found_existing, - }; -} - -pub fn trackTemp(self: *Origin, global: v8.Global) !void { - return self.temps.put(self.arena, global.data_ptr, global); -} - -pub fn releaseTemp(self: *Origin, global: v8.Global) void { - if (self.temps.fetchRemove(global.data_ptr)) |kv| { - var g = kv.value; - v8.v8__Global__Reset(&g); - } -} - -/// Release an item from the identity_map (called after finalizer runs from V8) -pub fn release(self: *Origin, item: *anyopaque) void { - var global = self.identity_map.fetchRemove(@intFromPtr(item)) orelse { - if (comptime IS_DEBUG) { - std.debug.assert(false); - } - return; - }; - v8.v8__Global__Reset(&global.value); - - // The item has been finalized, remove it from the finalizer callback so that - // we don't try to call it again on shutdown. - const kv = self.finalizer_callbacks.fetchRemove(@intFromPtr(item)) orelse { - if (comptime IS_DEBUG) { - std.debug.assert(false); - } - return; - }; - const fc = kv.value; - fc.session.releaseArena(fc.arena); -} - -pub fn createFinalizerCallback( - self: *Origin, - session: *Session, - global: v8.Global, - ptr: *anyopaque, - zig_finalizer: *const fn (ptr: *anyopaque, session: *Session) void, -) !*FinalizerCallback { - const arena = try session.getArena(.{ .debug = "FinalizerCallback" }); - errdefer session.releaseArena(arena); - const fc = try arena.create(FinalizerCallback); - fc.* = .{ - .arena = arena, - .origin = self, - .session = session, - .ptr = ptr, - .global = global, - .zig_finalizer = zig_finalizer, - }; - return fc; -} - -pub fn takeover(self: *Origin, original: *Origin) !void { - const arena = self.arena; - - try self.globals.ensureUnusedCapacity(arena, original.globals.items.len); - for (original.globals.items) |obj| { - self.globals.appendAssumeCapacity(obj); - } - original.globals.clearRetainingCapacity(); - - { - try self.temps.ensureUnusedCapacity(arena, original.temps.count()); - var it = original.temps.iterator(); - while (it.next()) |kv| { - try self.temps.put(arena, kv.key_ptr.*, kv.value_ptr.*); - } - original.temps.clearRetainingCapacity(); - } - - { - try self.finalizer_callbacks.ensureUnusedCapacity(arena, original.finalizer_callbacks.count()); - var it = original.finalizer_callbacks.iterator(); - while (it.next()) |kv| { - kv.value_ptr.*.origin = self; - try self.finalizer_callbacks.put(arena, kv.key_ptr.*, kv.value_ptr.*); - } - original.finalizer_callbacks.clearRetainingCapacity(); - } - - { - try self.identity_map.ensureUnusedCapacity(arena, original.identity_map.count()); - var it = original.identity_map.iterator(); - while (it.next()) |kv| { - try self.identity_map.put(arena, kv.key_ptr.*, kv.value_ptr.*); - } - original.identity_map.clearRetainingCapacity(); - } - - try self.taken_over.append(self.arena, original); -} - -// A type that has a finalizer can have its finalizer called one of two ways. -// The first is from V8 via the WeakCallback we give to weakRef. But that isn't -// guaranteed to fire, so we track this in finalizer_callbacks and call them on -// origin shutdown. -pub const FinalizerCallback = struct { - arena: Allocator, - origin: *Origin, - session: *Session, - ptr: *anyopaque, - global: v8.Global, - zig_finalizer: *const fn (ptr: *anyopaque, session: *Session) void, - - pub fn deinit(self: *FinalizerCallback) void { - self.zig_finalizer(self.ptr, self.session); - self.session.releaseArena(self.arena); - } -}; diff --git a/src/browser/js/Promise.zig b/src/browser/js/Promise.zig index 372d2578..0a08f424 100644 --- a/src/browser/js/Promise.zig +++ b/src/browser/js/Promise.zig @@ -19,6 +19,8 @@ const js = @import("js.zig"); const v8 = js.v8; +const Session = @import("../Session.zig"); + const Promise = @This(); local: *const js.Local, @@ -63,10 +65,10 @@ fn _persist(self: *const Promise, comptime is_global: bool) !(if (is_global) Glo v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); if (comptime is_global) { try ctx.trackGlobal(global); - return .{ .handle = global, .origin = {} }; + return .{ .handle = global, .session = {} }; } try ctx.trackTemp(global); - return .{ .handle = global, .origin = ctx.origin }; + return .{ .handle = global, .session = ctx.session }; } pub const Temp = G(.temp); @@ -80,7 +82,7 @@ const GlobalType = enum(u8) { fn G(comptime global_type: GlobalType) type { return struct { handle: v8.Global, - origin: if (global_type == .temp) *js.Origin else void, + session: if (global_type == .temp) *Session else void, const Self = @This(); @@ -96,7 +98,7 @@ fn G(comptime global_type: GlobalType) type { } pub fn release(self: *const Self) void { - self.origin.releaseTemp(self.handle); + self.session.releaseTemp(self.handle); } }; } diff --git a/src/browser/js/String.zig b/src/browser/js/String.zig index 47be227a..2cbe6a17 100644 --- a/src/browser/js/String.zig +++ b/src/browser/js/String.zig @@ -56,7 +56,7 @@ fn _toSlice(self: String, comptime null_terminate: bool, allocator: Allocator) ! pub fn toSSO(self: String, comptime global: bool) !(if (global) SSO.Global else SSO) { if (comptime global) { - return .{ .str = try self.toSSOWithAlloc(self.local.ctx.origin.arena) }; + return .{ .str = try self.toSSOWithAlloc(self.local.ctx.session.page_arena) }; } return self.toSSOWithAlloc(self.local.call_arena); } diff --git a/src/browser/js/Value.zig b/src/browser/js/Value.zig index 8e05690b..edbbe4e3 100644 --- a/src/browser/js/Value.zig +++ b/src/browser/js/Value.zig @@ -25,6 +25,7 @@ const v8 = js.v8; const IS_DEBUG = @import("builtin").mode == .Debug; const Allocator = std.mem.Allocator; +const Session = @import("../Session.zig"); const Value = @This(); @@ -300,10 +301,10 @@ fn _persist(self: *const Value, comptime is_global: bool) !(if (is_global) Globa v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); if (comptime is_global) { try ctx.trackGlobal(global); - return .{ .handle = global, .origin = {} }; + return .{ .handle = global, .session = {} }; } try ctx.trackTemp(global); - return .{ .handle = global, .origin = ctx.origin }; + return .{ .handle = global, .session = ctx.session }; } pub fn toZig(self: Value, comptime T: type) !T { @@ -361,7 +362,7 @@ const GlobalType = enum(u8) { fn G(comptime global_type: GlobalType) type { return struct { handle: v8.Global, - origin: if (global_type == .temp) *js.Origin else void, + session: if (global_type == .temp) *Session else void, const Self = @This(); @@ -381,7 +382,7 @@ fn G(comptime global_type: GlobalType) type { } pub fn release(self: *const Self) void { - self.origin.releaseTemp(self.handle); + self.session.releaseTemp(self.handle); } }; } diff --git a/src/browser/js/bridge.zig b/src/browser/js/bridge.zig index 4fc96b7e..57cff1fa 100644 --- a/src/browser/js/bridge.zig +++ b/src/browser/js/bridge.zig @@ -27,7 +27,6 @@ const v8 = js.v8; const Caller = @import("Caller.zig"); const Context = @import("Context.zig"); -const Origin = @import("Origin.zig"); const IS_DEBUG = @import("builtin").mode == .Debug; @@ -117,13 +116,13 @@ pub fn Builder(comptime T: type) type { .from_v8 = struct { fn wrap(handle: ?*const v8.WeakCallbackInfo) callconv(.c) void { const ptr = v8.v8__WeakCallbackInfo__GetParameter(handle.?).?; - const fc: *Origin.FinalizerCallback = @ptrCast(@alignCast(ptr)); + const fc: *Session.FinalizerCallback = @ptrCast(@alignCast(ptr)); - const origin = fc.origin; + const session = fc.session; const value_ptr = fc.ptr; - if (origin.finalizer_callbacks.contains(@intFromPtr(value_ptr))) { - func(@ptrCast(@alignCast(value_ptr)), false, fc.session); - origin.release(value_ptr); + if (session.finalizer_callbacks.contains(@intFromPtr(value_ptr))) { + func(@ptrCast(@alignCast(value_ptr)), false, session); + session.releaseIdentity(value_ptr); } else { // A bit weird, but v8 _requires_ that we release it // If we don't. We'll 100% crash. diff --git a/src/browser/webapi/IntersectionObserver.zig b/src/browser/webapi/IntersectionObserver.zig index 8586a11d..74a5d79e 100644 --- a/src/browser/webapi/IntersectionObserver.zig +++ b/src/browser/webapi/IntersectionObserver.zig @@ -93,12 +93,12 @@ pub fn init(callback: js.Function.Temp, options: ?ObserverInit, page: *Page) !*I } pub fn deinit(self: *IntersectionObserver, shutdown: bool, session: *Session) void { - if (shutdown) { - self._callback.release(); - session.releaseArena(self._arena); - } else if (comptime IS_DEBUG) { - std.debug.assert(false); + self._callback.release(); + if ((comptime IS_DEBUG) and !shutdown) { + std.debug.assert(self._observing.items.len == 0); } + + session.releaseArena(self._arena); } pub fn observe(self: *IntersectionObserver, target: *Element, page: *Page) !void { @@ -111,6 +111,7 @@ pub fn observe(self: *IntersectionObserver, target: *Element, page: *Page) !void // Register with page if this is our first observation if (self._observing.items.len == 0) { + page.js.strongRef(self); try page.registerIntersectionObserver(self); } @@ -145,18 +146,22 @@ pub fn unobserve(self: *IntersectionObserver, target: *Element, page: *Page) voi break; } } + + if (self._observing.items.len == 0) { + page.js.safeWeakRef(self); + } } pub fn disconnect(self: *IntersectionObserver, page: *Page) void { + page.unregisterIntersectionObserver(self); + self._observing.clearRetainingCapacity(); self._previous_states.clearRetainingCapacity(); for (self._pending_entries.items) |entry| { entry.deinit(false, page._session); } self._pending_entries.clearRetainingCapacity(); - - self._observing.clearRetainingCapacity(); - page.unregisterIntersectionObserver(self); + page.js.safeWeakRef(self); } pub fn takeRecords(self: *IntersectionObserver, page: *Page) ![]*IntersectionObserverEntry { @@ -358,6 +363,7 @@ pub const JsApi = struct { pub const name = "IntersectionObserver"; pub const prototype_chain = bridge.prototypeChain(); pub var class_id: bridge.ClassId = undefined; + pub const weak = true; pub const finalizer = bridge.finalizer(IntersectionObserver.deinit); }; diff --git a/src/browser/webapi/MutationObserver.zig b/src/browser/webapi/MutationObserver.zig index df86d1e1..b8608381 100644 --- a/src/browser/webapi/MutationObserver.zig +++ b/src/browser/webapi/MutationObserver.zig @@ -86,12 +86,12 @@ pub fn init(callback: js.Function.Temp, page: *Page) !*MutationObserver { } pub fn deinit(self: *MutationObserver, shutdown: bool, session: *Session) void { - if (shutdown) { - self._callback.release(); - session.releaseArena(self._arena); - } else if (comptime IS_DEBUG) { - std.debug.assert(false); + self._callback.release(); + if ((comptime IS_DEBUG) and !shutdown) { + std.debug.assert(self._observing.items.len == 0); } + + session.releaseArena(self._arena); } pub fn observe(self: *MutationObserver, target: *Node, options: ObserveOptions, page: *Page) !void { @@ -158,6 +158,7 @@ pub fn observe(self: *MutationObserver, target: *Node, options: ObserveOptions, // Register with page if this is our first observation if (self._observing.items.len == 0) { + page.js.strongRef(self); try page.registerMutationObserver(self); } @@ -168,13 +169,13 @@ pub fn observe(self: *MutationObserver, target: *Node, options: ObserveOptions, } pub fn disconnect(self: *MutationObserver, page: *Page) void { + page.unregisterMutationObserver(self); + self._observing.clearRetainingCapacity(); for (self._pending_records.items) |record| { record.deinit(false, page._session); } self._pending_records.clearRetainingCapacity(); - - self._observing.clearRetainingCapacity(); - page.unregisterMutationObserver(self); + page.js.safeWeakRef(self); } pub fn takeRecords(self: *MutationObserver, page: *Page) ![]*MutationRecord { @@ -440,6 +441,7 @@ pub const JsApi = struct { pub const name = "MutationObserver"; pub const prototype_chain = bridge.prototypeChain(); pub var class_id: bridge.ClassId = undefined; + pub const weak = true; pub const finalizer = bridge.finalizer(MutationObserver.deinit); }; From 38e9f86088c6158eb2de4302787e169437271327 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 19 Mar 2026 15:42:29 +0800 Subject: [PATCH 07/15] fix context-leak --- src/browser/js/Env.zig | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index c417a6e5..07f1a479 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -300,10 +300,6 @@ pub fn createContext(self: *Env, page: *Page) !*Context { v8.v8__Object__SetAlignedPointerInInternalField(global_obj, 0, tao); } - // our window wrapped in a v8::Global - var global_global: v8.Global = undefined; - v8.v8__Global__New(isolate.handle, global_obj, &global_global); - const context_id = self.context_id; self.context_id = context_id + 1; @@ -333,6 +329,9 @@ pub fn createContext(self: *Env, page: *Page) !*Context { // need to register the first one. const gop = try session.identity_map.getOrPut(session.page_arena, @intFromPtr(page.window)); if (gop.found_existing == false) { + // our window wrapped in a v8::Global + var global_global: v8.Global = undefined; + v8.v8__Global__New(isolate.handle, global_obj, &global_global); gop.value_ptr.* = global_global; } } From f70865e1745862d91b5ed2c744fba7d5cbf3f82e Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 19 Mar 2026 18:46:35 +0800 Subject: [PATCH 08/15] Take 2. History: We started with 1 context and thus only had 1 identity map. Frames were added, and we tried to stick with 1 identity map per context. That didn't work - it breaks cross-frame scripting. We introduced "Origin" so that all frames on the same origin share the same objects. That almost worked, by the v8::Inspector isn't bound by a Context's SecurityToken. So we tried 1 global identity map. But that doesn't work. CDP IsolateWorlds do, in fact, need some isolation. They need new v8::Objects created in their context, even if the object already exists in the main context. In the end, you end up with something like this: A page (and all its frames) needs 1 view of the data. And each IsolateWorld needs it own view. This commit introduces a js.Identity which is referenced by the context. The Session has a js.Identity (used by all pages), and each IsolateWorld has its own js.Identity. As a bonus, the arena pool memory-leak detection has been moved out of the session and into the ArenaPool. This means _all_ arena pool access is audited (in debug mode). This seems superfluous, but it's actually necessary since IsolateWorlds (which now own their own identity) can outlive the Page so there's no clear place to "check" for leaks - except on ArenaPool deinit. --- src/ArenaPool.zig | 80 ++++++++++++--- src/browser/Page.zig | 5 +- src/browser/Session.zig | 188 +++++------------------------------- src/browser/js/Context.zig | 62 +++++++++++- src/browser/js/Env.zig | 15 ++- src/browser/js/Function.zig | 11 ++- src/browser/js/Identity.zig | 76 +++++++++++++++ src/browser/js/Local.zig | 13 ++- src/browser/js/Origin.zig | 7 +- src/browser/js/Promise.zig | 12 ++- src/browser/js/Value.zig | 11 ++- src/browser/js/bridge.zig | 7 +- src/browser/js/js.zig | 1 + src/cdp/cdp.zig | 14 ++- 14 files changed, 290 insertions(+), 212 deletions(-) create mode 100644 src/browser/js/Identity.zig diff --git a/src/ArenaPool.zig b/src/ArenaPool.zig index 2f8de17f..998838df 100644 --- a/src/ArenaPool.zig +++ b/src/ArenaPool.zig @@ -17,12 +17,15 @@ // along with this program. If not, see . const std = @import("std"); +const builtin = @import("builtin"); const Allocator = std.mem.Allocator; const ArenaAllocator = std.heap.ArenaAllocator; const ArenaPool = @This(); +const IS_DEBUG = builtin.mode == .Debug; + allocator: Allocator, retain_bytes: usize, free_list_len: u16 = 0, @@ -30,10 +33,17 @@ free_list: ?*Entry = null, free_list_max: u16, entry_pool: std.heap.MemoryPool(Entry), mutex: std.Thread.Mutex = .{}, +// Debug mode: track acquire/release counts per debug name to detect leaks and double-frees +_leak_track: if (IS_DEBUG) std.StringHashMapUnmanaged(isize) else void = if (IS_DEBUG) .empty else {}, const Entry = struct { next: ?*Entry, arena: ArenaAllocator, + debug: if (IS_DEBUG) []const u8 else void = if (IS_DEBUG) "" else {}, +}; + +pub const DebugInfo = struct { + debug: []const u8 = "", }; pub fn init(allocator: Allocator, free_list_max: u16, retain_bytes: usize) ArenaPool { @@ -42,10 +52,26 @@ pub fn init(allocator: Allocator, free_list_max: u16, retain_bytes: usize) Arena .free_list_max = free_list_max, .retain_bytes = retain_bytes, .entry_pool = .init(allocator), + ._leak_track = if (IS_DEBUG) .empty else {}, }; } pub fn deinit(self: *ArenaPool) void { + if (IS_DEBUG) { + var has_leaks = false; + var it = self._leak_track.iterator(); + while (it.next()) |kv| { + if (kv.value_ptr.* != 0) { + std.debug.print("ArenaPool leak detected: '{s}' count={d}\n", .{ kv.key_ptr.*, kv.value_ptr.* }); + has_leaks = true; + } + } + if (has_leaks) { + @panic("ArenaPool: leaked arenas detected"); + } + self._leak_track.deinit(self.allocator); + } + var entry = self.free_list; while (entry) |e| { entry = e.next; @@ -54,13 +80,21 @@ pub fn deinit(self: *ArenaPool) void { self.entry_pool.deinit(); } -pub fn acquire(self: *ArenaPool) !Allocator { +pub fn acquire(self: *ArenaPool, dbg: DebugInfo) !Allocator { self.mutex.lock(); defer self.mutex.unlock(); if (self.free_list) |entry| { self.free_list = entry.next; self.free_list_len -= 1; + if (IS_DEBUG) { + entry.debug = dbg.debug; + const gop = try self._leak_track.getOrPut(self.allocator, dbg.debug); + if (!gop.found_existing) { + gop.value_ptr.* = 0; + } + gop.value_ptr.* += 1; + } return entry.arena.allocator(); } @@ -68,8 +102,17 @@ pub fn acquire(self: *ArenaPool) !Allocator { entry.* = .{ .next = null, .arena = ArenaAllocator.init(self.allocator), + .debug = if (IS_DEBUG) dbg.debug else {}, }; + if (IS_DEBUG) { + const gop = try self._leak_track.getOrPut(self.allocator, dbg.debug); + if (!gop.found_existing) { + gop.value_ptr.* = 0; + } + gop.value_ptr.* += 1; + } + return entry.arena.allocator(); } @@ -83,6 +126,19 @@ pub fn release(self: *ArenaPool, allocator: Allocator) void { self.mutex.lock(); defer self.mutex.unlock(); + if (IS_DEBUG) { + if (self._leak_track.getPtr(entry.debug)) |count| { + count.* -= 1; + if (count.* < 0) { + std.debug.print("ArenaPool double-free detected: '{s}'\n", .{entry.debug}); + @panic("ArenaPool: double-free detected"); + } + } else { + std.debug.print("ArenaPool release of untracked arena: '{s}'\n", .{entry.debug}); + @panic("ArenaPool: release of untracked arena"); + } + } + const free_list_len = self.free_list_len; if (free_list_len == self.free_list_max) { arena.deinit(); @@ -106,7 +162,7 @@ test "arena pool - basic acquire and use" { var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16); defer pool.deinit(); - const alloc = try pool.acquire(); + const alloc = try pool.acquire(.{ .debug = "test" }); const buf = try alloc.alloc(u8, 64); @memset(buf, 0xAB); try testing.expectEqual(@as(u8, 0xAB), buf[0]); @@ -118,14 +174,14 @@ test "arena pool - reuse entry after release" { var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16); defer pool.deinit(); - const alloc1 = try pool.acquire(); + const alloc1 = try pool.acquire(.{ .debug = "test" }); try testing.expectEqual(@as(u16, 0), pool.free_list_len); pool.release(alloc1); try testing.expectEqual(@as(u16, 1), pool.free_list_len); // The same entry should be returned from the free list. - const alloc2 = try pool.acquire(); + const alloc2 = try pool.acquire(.{ .debug = "test" }); try testing.expectEqual(@as(u16, 0), pool.free_list_len); try testing.expectEqual(alloc1.ptr, alloc2.ptr); @@ -136,9 +192,9 @@ test "arena pool - multiple concurrent arenas" { var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16); defer pool.deinit(); - const a1 = try pool.acquire(); - const a2 = try pool.acquire(); - const a3 = try pool.acquire(); + const a1 = try pool.acquire(.{ .debug = "test1" }); + const a2 = try pool.acquire(.{ .debug = "test2" }); + const a3 = try pool.acquire(.{ .debug = "test3" }); // All three must be distinct arenas. try testing.expect(a1.ptr != a2.ptr); @@ -161,8 +217,8 @@ test "arena pool - free list respects max limit" { var pool = ArenaPool.init(testing.allocator, 1, 1024 * 16); defer pool.deinit(); - const a1 = try pool.acquire(); - const a2 = try pool.acquire(); + const a1 = try pool.acquire(.{ .debug = "test1" }); + const a2 = try pool.acquire(.{ .debug = "test2" }); pool.release(a1); try testing.expectEqual(@as(u16, 1), pool.free_list_len); @@ -176,7 +232,7 @@ test "arena pool - reset clears memory without releasing" { var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16); defer pool.deinit(); - const alloc = try pool.acquire(); + const alloc = try pool.acquire(.{ .debug = "test" }); const buf = try alloc.alloc(u8, 128); @memset(buf, 0xFF); @@ -200,8 +256,8 @@ test "arena pool - deinit with entries in free list" { // detected by the test allocator). var pool = ArenaPool.init(testing.allocator, 512, 1024 * 16); - const a1 = try pool.acquire(); - const a2 = try pool.acquire(); + const a1 = try pool.acquire(.{ .debug = "test1" }); + const a2 = try pool.acquire(.{ .debug = "test2" }); _ = try a1.alloc(u8, 256); _ = try a2.alloc(u8, 512); pool.release(a1); diff --git a/src/browser/Page.zig b/src/browser/Page.zig index c3a6b5a3..148654a0 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -302,7 +302,10 @@ pub fn init(self: *Page, frame_id: u32, session: *Session, parent: ?*Page) !void self._script_manager = ScriptManager.init(browser.allocator, browser.http_client, self); errdefer self._script_manager.deinit(); - self.js = try browser.env.createContext(self); + self.js = try browser.env.createContext(self, .{ + .identity = &session.identity, + .identity_arena = session.page_arena, + }); errdefer self.js.deinit(); document._page = self; diff --git a/src/browser/Session.zig b/src/browser/Session.zig index 6376ff03..e318e486 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -66,30 +66,14 @@ page_arena: Allocator, // Origin map for same-origin context sharing. Scoped to the root page lifetime. origins: std.StringHashMapUnmanaged(*js.Origin) = .empty, -// Session-scoped identity tracking (for the lifetime of the root page). -// Maps Zig instance pointers to their v8::Global(Object) wrappers. -identity_map: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, - -// Tracked global v8 objects that need to be released when the page is reset. -globals: std.ArrayList(v8.Global) = .empty, - -// Temporary v8 globals that can be released early. Key is global.data_ptr. -temps: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, - -// Finalizer callbacks for weak references. Key is @intFromPtr of the Zig instance. -finalizer_callbacks: std.AutoHashMapUnmanaged(usize, *FinalizerCallback) = .empty, +// Identity tracking for the main world. All main world contexts share this, +// ensuring object identity works across same-origin frames. +identity: js.Identity = .{}, // Shared resources for all pages in this session. // These live for the duration of the page tree (root + frames). arena_pool: *ArenaPool, -// In Debug, we use this to see if anything fails to release an arena back to -// the pool. -_arena_pool_leak_track: if (IS_DEBUG) std.AutoHashMapUnmanaged(usize, struct { - owner: []const u8, - count: usize, -}) else void = if (IS_DEBUG) .empty else {}, - page: ?Page, queued_navigation: std.ArrayList(*Page), @@ -105,10 +89,10 @@ pub fn init(self: *Session, browser: *Browser, notification: *Notification) !voi const allocator = browser.app.allocator; const arena_pool = browser.arena_pool; - const arena = try arena_pool.acquire(); + const arena = try arena_pool.acquire(.{ .debug = "Session" }); errdefer arena_pool.release(arena); - const page_arena = try arena_pool.acquire(); + const page_arena = try arena_pool.acquire(.{ .debug = "Session.page_arena" }); errdefer arena_pool.release(page_arena); self.* = .{ @@ -183,32 +167,11 @@ pub const GetArenaOpts = struct { }; pub fn getArena(self: *Session, opts: GetArenaOpts) !Allocator { - const allocator = try self.arena_pool.acquire(); - if (comptime IS_DEBUG) { - // Use session's arena (not page_arena) since page_arena gets reset between pages - const gop = try self._arena_pool_leak_track.getOrPut(self.page_arena, @intFromPtr(allocator.ptr)); - if (gop.found_existing and gop.value_ptr.count != 0) { - log.err(.bug, "ArenaPool Double Use", .{ .owner = gop.value_ptr.*.owner }); - @panic("ArenaPool Double Use"); - } - gop.value_ptr.* = .{ .owner = opts.debug, .count = 1 }; - } - return allocator; + return self.arena_pool.acquire(.{ .debug = opts.debug }); } pub fn releaseArena(self: *Session, allocator: Allocator) void { - if (comptime IS_DEBUG) { - const found = self._arena_pool_leak_track.getPtr(@intFromPtr(allocator.ptr)).?; - if (found.count != 1) { - log.err(.bug, "ArenaPool Double Free", .{ .owner = found.owner, .count = found.count }); - if (comptime builtin.is_test) { - @panic("ArenaPool Double Free"); - } - return; - } - found.count = 0; - } - return self.arena_pool.release(allocator); + self.arena_pool.release(allocator); } pub fn getOrCreateOrigin(self: *Session, key_: ?[]const u8) !*js.Origin { @@ -249,44 +212,8 @@ pub fn releaseOrigin(self: *Session, origin: *js.Origin) void { /// Reset page_arena and factory for a clean slate. /// Called when root page is removed. fn resetPageResources(self: *Session) void { - { - var it = self.finalizer_callbacks.valueIterator(); - while (it.next()) |finalizer| { - finalizer.*.deinit(); - } - self.finalizer_callbacks = .empty; - } - - { - var it = self.identity_map.valueIterator(); - while (it.next()) |global| { - v8.v8__Global__Reset(global); - } - self.identity_map = .empty; - } - - for (self.globals.items) |*global| { - v8.v8__Global__Reset(global); - } - self.globals = .empty; - - { - var it = self.temps.valueIterator(); - while (it.next()) |global| { - v8.v8__Global__Reset(global); - } - self.temps = .empty; - } - - if (comptime IS_DEBUG) { - var it = self._arena_pool_leak_track.valueIterator(); - while (it.next()) |value_ptr| { - if (value_ptr.count > 0) { - log.err(.bug, "ArenaPool Leak", .{ .owner = value_ptr.owner }); - } - } - self._arena_pool_leak_track = .empty; - } + self.identity.deinit(); + self.identity = .{}; if (comptime IS_DEBUG) { std.debug.assert(self.origins.count() == 0); @@ -670,16 +597,6 @@ fn processRootQueuedNavigation(self: *Session) !void { defer self.arena_pool.release(qn.arena); - // HACK - // Mark as released in tracking BEFORE removePage clears the map. - // We can't call releaseArena() because that would also return the arena - // to the pool, making the memory invalid before we use qn.url/qn.opts. - if (comptime IS_DEBUG) { - if (self._arena_pool_leak_track.getPtr(@intFromPtr(qn.arena.ptr))) |found| { - found.count = 0; - } - } - self.removePage(); self.page = @as(Page, undefined); @@ -711,77 +628,6 @@ pub fn nextPageId(self: *Session) u32 { return id; } -// These methods manage the mapping between Zig instances and v8 objects, -// scoped to the lifetime of the root page. -pub fn trackGlobal(self: *Session, global: v8.Global) !void { - return self.globals.append(self.page_arena, global); -} - -pub const IdentityResult = struct { - value_ptr: *v8.Global, - found_existing: bool, -}; - -pub fn addIdentity(self: *Session, ptr: usize) !IdentityResult { - const gop = try self.identity_map.getOrPut(self.page_arena, ptr); - return .{ - .value_ptr = gop.value_ptr, - .found_existing = gop.found_existing, - }; -} - -pub fn trackTemp(self: *Session, global: v8.Global) !void { - return self.temps.put(self.page_arena, global.data_ptr, global); -} - -pub fn releaseTemp(self: *Session, global: v8.Global) void { - if (self.temps.fetchRemove(global.data_ptr)) |kv| { - var g = kv.value; - v8.v8__Global__Reset(&g); - } -} - -/// Release an item from the identity_map (called after finalizer runs from V8) -pub fn releaseIdentity(self: *Session, item: *anyopaque) void { - var global = self.identity_map.fetchRemove(@intFromPtr(item)) orelse { - if (comptime IS_DEBUG) { - std.debug.assert(false); - } - return; - }; - v8.v8__Global__Reset(&global.value); - - // The item has been finalized, remove it from the finalizer callback so that - // we don't try to call it again on shutdown. - const kv = self.finalizer_callbacks.fetchRemove(@intFromPtr(item)) orelse { - if (comptime IS_DEBUG) { - std.debug.assert(false); - } - return; - }; - const fc = kv.value; - self.releaseArena(fc.arena); -} - -pub fn createFinalizerCallback( - self: *Session, - global: v8.Global, - ptr: *anyopaque, - zig_finalizer: *const fn (ptr: *anyopaque, session: *Session) void, -) !*FinalizerCallback { - const arena = try self.getArena(.{ .debug = "FinalizerCallback" }); - errdefer self.releaseArena(arena); - const fc = try arena.create(FinalizerCallback); - fc.* = .{ - .arena = arena, - .session = self, - .ptr = ptr, - .global = global, - .zig_finalizer = zig_finalizer, - }; - return fc; -} - // A type that has a finalizer can have its finalizer called one of two ways. // The first is from V8 via the WeakCallback we give to weakRef. But that isn't // guaranteed to fire, so we track this in finalizer_callbacks and call them on @@ -791,10 +637,26 @@ pub const FinalizerCallback = struct { session: *Session, ptr: *anyopaque, global: v8.Global, + identity: *js.Identity, zig_finalizer: *const fn (ptr: *anyopaque, session: *Session) void, pub fn deinit(self: *FinalizerCallback) void { self.zig_finalizer(self.ptr, self.session); self.session.releaseArena(self.arena); } + + /// Release this item from the identity tracking maps (called after finalizer runs from V8) + pub fn releaseIdentity(self: *FinalizerCallback) void { + const session = self.session; + const id = @intFromPtr(self.ptr); + + if (self.identity.identity_map.fetchRemove(id)) |kv| { + var global = kv.value; + v8.v8__Global__Reset(&global); + } + + _ = self.identity.finalizer_callbacks.remove(id); + + session.releaseArena(self.arena); + } }; diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index 5ea9454f..2b861e7a 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -79,6 +79,16 @@ local: ?*const js.Local = null, origin: *Origin, +// Identity tracking for this context. For main world contexts, this points to +// Session's Identity. For isolated world contexts (CDP inspector), this points +// to IsolatedWorld's Identity. This ensures same-origin frames share object +// identity while isolated worlds have separate identity tracking. +identity: *js.Identity, + +// Allocator to use for identity map operations. For main world contexts this is +// session.page_arena, for isolated worlds it's the isolated world's arena. +identity_arena: Allocator, + // Unlike other v8 types, like functions or objects, modules are not shared // across origins. global_modules: std.ArrayList(v8.Global) = .empty, @@ -202,16 +212,16 @@ pub fn setOrigin(self: *Context, key: ?[]const u8) !void { } pub fn trackGlobal(self: *Context, global: v8.Global) !void { - return self.session.trackGlobal(global); + return self.identity.globals.append(self.identity_arena, global); } pub fn trackTemp(self: *Context, global: v8.Global) !void { - return self.session.trackTemp(global); + return self.identity.temps.put(self.identity_arena, global.data_ptr, global); } pub fn weakRef(self: *Context, obj: anytype) void { const resolved = js.Local.resolveValue(obj); - const fc = self.session.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { + const fc = self.identity.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { if (comptime IS_DEBUG) { // should not be possible std.debug.assert(false); @@ -223,7 +233,7 @@ pub fn weakRef(self: *Context, obj: anytype) void { pub fn safeWeakRef(self: *Context, obj: anytype) void { const resolved = js.Local.resolveValue(obj); - const fc = self.session.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { + const fc = self.identity.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { if (comptime IS_DEBUG) { // should not be possible std.debug.assert(false); @@ -236,7 +246,7 @@ pub fn safeWeakRef(self: *Context, obj: anytype) void { pub fn strongRef(self: *Context, obj: anytype) void { const resolved = js.Local.resolveValue(obj); - const fc = self.session.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { + const fc = self.identity.finalizer_callbacks.get(@intFromPtr(resolved.ptr)) orelse { if (comptime IS_DEBUG) { // should not be possible std.debug.assert(false); @@ -246,6 +256,48 @@ pub fn strongRef(self: *Context, obj: anytype) void { v8.v8__Global__ClearWeak(&fc.global); } +pub const IdentityResult = struct { + value_ptr: *v8.Global, + found_existing: bool, +}; + +pub fn addIdentity(self: *Context, ptr: usize) !IdentityResult { + const gop = try self.identity.identity_map.getOrPut(self.identity_arena, ptr); + return .{ + .value_ptr = gop.value_ptr, + .found_existing = gop.found_existing, + }; +} + +pub fn releaseTemp(self: *Context, global: v8.Global) void { + if (self.identity.temps.fetchRemove(global.data_ptr)) |kv| { + var g = kv.value; + v8.v8__Global__Reset(&g); + } +} + +pub fn createFinalizerCallback( + self: *Context, + global: v8.Global, + ptr: *anyopaque, + zig_finalizer: *const fn (ptr: *anyopaque, session: *Session) void, +) !*Session.FinalizerCallback { + const session = self.session; + const arena = try session.getArena(.{ .debug = "FinalizerCallback" }); + errdefer session.releaseArena(arena); + const fc = try arena.create(Session.FinalizerCallback); + fc.* = .{ + .arena = arena, + .session = session, + .ptr = ptr, + .global = global, + .zig_finalizer = zig_finalizer, + // Store identity pointer for cleanup when V8 GCs the object + .identity = self.identity, + }; + return fc; +} + // Any operation on the context have to be made from a local. pub fn localScope(self: *Context, ls: *js.Local.Scope) void { const isolate = self.isolate; diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index 07f1a479..d248a411 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -34,6 +34,7 @@ const Snapshot = @import("Snapshot.zig"); const Inspector = @import("Inspector.zig"); const Page = @import("../Page.zig"); +const Session = @import("../Session.zig"); const Window = @import("../webapi/Window.zig"); const JsApis = bridge.JsApis; @@ -254,8 +255,14 @@ pub fn deinit(self: *Env) void { allocator.destroy(self.isolate_params); } -pub fn createContext(self: *Env, page: *Page) !*Context { - const context_arena = try self.app.arena_pool.acquire(); +pub const ContextParams = struct { + identity: *js.Identity, + identity_arena: Allocator, + debug_name: []const u8 = "Context", +}; + +pub fn createContext(self: *Env, page: *Page, params: ContextParams) !*Context { + const context_arena = try self.app.arena_pool.acquire(.{ .debug = params.debug_name }); errdefer self.app.arena_pool.release(context_arena); const isolate = self.isolate; @@ -322,12 +329,14 @@ pub fn createContext(self: *Env, page: *Page) !*Context { .microtask_queue = microtask_queue, .script_manager = &page._script_manager, .scheduler = .init(context_arena), + .identity = params.identity, + .identity_arena = params.identity_arena, }; { // Multiple contexts can be created for the same Window (via CDP). We only // need to register the first one. - const gop = try session.identity_map.getOrPut(session.page_arena, @intFromPtr(page.window)); + const gop = try params.identity.identity_map.getOrPut(params.identity_arena, @intFromPtr(page.window)); if (gop.found_existing == false) { // our window wrapped in a v8::Global var global_global: v8.Global = undefined; diff --git a/src/browser/js/Function.zig b/src/browser/js/Function.zig index e2c10e9f..4c84a08f 100644 --- a/src/browser/js/Function.zig +++ b/src/browser/js/Function.zig @@ -211,10 +211,10 @@ fn _persist(self: *const Function, comptime is_global: bool) !(if (is_global) Gl v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); if (comptime is_global) { try ctx.trackGlobal(global); - return .{ .handle = global, .session = {} }; + return .{ .handle = global, .temps = {} }; } try ctx.trackTemp(global); - return .{ .handle = global, .session = ctx.session }; + return .{ .handle = global, .temps = &ctx.identity.temps }; } pub fn tempWithThis(self: *const Function, value: anytype) !Temp { @@ -238,7 +238,7 @@ const GlobalType = enum(u8) { fn G(comptime global_type: GlobalType) type { return struct { handle: v8.Global, - session: if (global_type == .temp) *Session else void, + temps: if (global_type == .temp) *std.AutoHashMapUnmanaged(usize, v8.Global) else void, const Self = @This(); @@ -258,7 +258,10 @@ fn G(comptime global_type: GlobalType) type { } pub fn release(self: *const Self) void { - self.session.releaseTemp(self.handle); + if (self.temps.fetchRemove(self.handle.data_ptr)) |kv| { + var g = kv.value; + v8.v8__Global__Reset(&g); + } } }; } diff --git a/src/browser/js/Identity.zig b/src/browser/js/Identity.zig new file mode 100644 index 00000000..323ae909 --- /dev/null +++ b/src/browser/js/Identity.zig @@ -0,0 +1,76 @@ +// Copyright (C) 2023-2026 Lightpanda (Selecy SAS) +// +// Francis Bouvier +// Pierre Tachoire +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as +// published by the Free Software Foundation, either version 3 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +// Identity manages the mapping between Zig instances and their v8::Object wrappers. +// This provides object identity semantics - the same Zig instance always maps to +// the same JS object within a given Identity scope. +// +// Main world contexts share a single Identity (on Session), ensuring that +// `window.top.document === top's document` works across same-origin frames. +// +// Isolated worlds (CDP inspector) have their own Identity, ensuring their +// v8::Global wrappers don't leak into the main world. + +const std = @import("std"); +const js = @import("js.zig"); + +const Session = @import("../Session.zig"); + +const v8 = js.v8; +const Allocator = std.mem.Allocator; + +const Identity = @This(); + +// Maps Zig instance pointers to their v8::Global(Object) wrappers. +identity_map: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, + +// Tracked global v8 objects that need to be released on cleanup. +globals: std.ArrayList(v8.Global) = .empty, + +// Temporary v8 globals that can be released early. Key is global.data_ptr. +temps: std.AutoHashMapUnmanaged(usize, v8.Global) = .empty, + +// Finalizer callbacks for weak references. Key is @intFromPtr of the Zig instance. +finalizer_callbacks: std.AutoHashMapUnmanaged(usize, *Session.FinalizerCallback) = .empty, + +pub fn deinit(self: *Identity) void { + { + var it = self.finalizer_callbacks.valueIterator(); + while (it.next()) |finalizer| { + finalizer.*.deinit(); + } + } + + { + var it = self.identity_map.valueIterator(); + while (it.next()) |global| { + v8.v8__Global__Reset(global); + } + } + + for (self.globals.items) |*global| { + v8.v8__Global__Reset(global); + } + + { + var it = self.temps.valueIterator(); + while (it.next()) |global| { + v8.v8__Global__Reset(global); + } + } +} diff --git a/src/browser/js/Local.zig b/src/browser/js/Local.zig index ca09fc91..9080e3bc 100644 --- a/src/browser/js/Local.zig +++ b/src/browser/js/Local.zig @@ -202,21 +202,20 @@ pub fn compileAndRun(self: *const Local, src: []const u8, name: ?[]const u8) !js // we can just grab it from the identity_map) pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object, value: anytype) !js.Object { const ctx = self.ctx; - const session = ctx.session; - const page_arena = session.page_arena; + const context_arena = ctx.arena; const T = @TypeOf(value); switch (@typeInfo(T)) { .@"struct" => { // Struct, has to be placed on the heap - const heap = try page_arena.create(T); + const heap = try context_arena.create(T); heap.* = value; return self.mapZigInstanceToJs(js_obj_handle, heap); }, .pointer => |ptr| { const resolved = resolveValue(value); - const gop = try session.addIdentity(@intFromPtr(resolved.ptr)); + const gop = try ctx.addIdentity(@intFromPtr(resolved.ptr)); if (gop.found_existing) { // we've seen this instance before, return the same object return (js.Object.Global{ .handle = gop.value_ptr.* }).local(self); @@ -245,7 +244,7 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object, // The TAO contains the pointer to our Zig instance as // well as any meta data we'll need to use it later. // See the TaggedOpaque struct for more details. - const tao = try page_arena.create(TaggedOpaque); + const tao = try context_arena.create(TaggedOpaque); tao.* = .{ .value = resolved.ptr, .prototype_chain = resolved.prototype_chain.ptr, @@ -277,10 +276,10 @@ pub fn mapZigInstanceToJs(self: *const Local, js_obj_handle: ?*const v8.Object, // Instead, we check if the base has finalizer. The assumption // here is that if a resolve type has a finalizer, then the base // should have a finalizer too. - const fc = try session.createFinalizerCallback(gop.value_ptr.*, resolved.ptr, resolved.finalizer_from_zig.?); + const fc = try ctx.createFinalizerCallback(gop.value_ptr.*, resolved.ptr, resolved.finalizer_from_zig.?); { errdefer fc.deinit(); - try session.finalizer_callbacks.put(page_arena, @intFromPtr(resolved.ptr), fc); + try ctx.identity.finalizer_callbacks.put(ctx.identity_arena, @intFromPtr(resolved.ptr), fc); } conditionallyReference(value); diff --git a/src/browser/js/Origin.zig b/src/browser/js/Origin.zig index 6f79b005..c6c6bf81 100644 --- a/src/browser/js/Origin.zig +++ b/src/browser/js/Origin.zig @@ -20,8 +20,9 @@ // Multiple contexts (frames) from the same origin share a single Origin, // which provides the V8 SecurityToken that allows cross-context access. // -// Note: Identity tracking (mapping Zig instances to v8::Objects) is now -// handled at the Session level, scoped to the root page lifetime. +// Note: Identity tracking (mapping Zig instances to v8::Objects) is managed +// separately via js.Identity - Session has the main world Identity, and +// IsolatedWorlds have their own Identity instances. const std = @import("std"); const js = @import("js.zig"); @@ -44,7 +45,7 @@ key: []const u8, security_token: v8.Global, pub fn init(app: *App, isolate: js.Isolate, key: []const u8) !*Origin { - const arena = try app.arena_pool.acquire(); + const arena = try app.arena_pool.acquire(.{ .debug = "Origin" }); errdefer app.arena_pool.release(arena); var hs: js.HandleScope = undefined; diff --git a/src/browser/js/Promise.zig b/src/browser/js/Promise.zig index 0a08f424..4e83c098 100644 --- a/src/browser/js/Promise.zig +++ b/src/browser/js/Promise.zig @@ -16,6 +16,7 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . +const std = @import("std"); const js = @import("js.zig"); const v8 = js.v8; @@ -65,10 +66,10 @@ fn _persist(self: *const Promise, comptime is_global: bool) !(if (is_global) Glo v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); if (comptime is_global) { try ctx.trackGlobal(global); - return .{ .handle = global, .session = {} }; + return .{ .handle = global, .temps = {} }; } try ctx.trackTemp(global); - return .{ .handle = global, .session = ctx.session }; + return .{ .handle = global, .temps = &ctx.identity.temps }; } pub const Temp = G(.temp); @@ -82,7 +83,7 @@ const GlobalType = enum(u8) { fn G(comptime global_type: GlobalType) type { return struct { handle: v8.Global, - session: if (global_type == .temp) *Session else void, + temps: if (global_type == .temp) *std.AutoHashMapUnmanaged(usize, v8.Global) else void, const Self = @This(); @@ -98,7 +99,10 @@ fn G(comptime global_type: GlobalType) type { } pub fn release(self: *const Self) void { - self.session.releaseTemp(self.handle); + if (self.temps.fetchRemove(self.handle.data_ptr)) |kv| { + var g = kv.value; + v8.v8__Global__Reset(&g); + } } }; } diff --git a/src/browser/js/Value.zig b/src/browser/js/Value.zig index edbbe4e3..d71e2f83 100644 --- a/src/browser/js/Value.zig +++ b/src/browser/js/Value.zig @@ -301,10 +301,10 @@ fn _persist(self: *const Value, comptime is_global: bool) !(if (is_global) Globa v8.v8__Global__New(ctx.isolate.handle, self.handle, &global); if (comptime is_global) { try ctx.trackGlobal(global); - return .{ .handle = global, .session = {} }; + return .{ .handle = global, .temps = {} }; } try ctx.trackTemp(global); - return .{ .handle = global, .session = ctx.session }; + return .{ .handle = global, .temps = &ctx.identity.temps }; } pub fn toZig(self: Value, comptime T: type) !T { @@ -362,7 +362,7 @@ const GlobalType = enum(u8) { fn G(comptime global_type: GlobalType) type { return struct { handle: v8.Global, - session: if (global_type == .temp) *Session else void, + temps: if (global_type == .temp) *std.AutoHashMapUnmanaged(usize, v8.Global) else void, const Self = @This(); @@ -382,7 +382,10 @@ fn G(comptime global_type: GlobalType) type { } pub fn release(self: *const Self) void { - self.session.releaseTemp(self.handle); + if (self.temps.fetchRemove(self.handle.data_ptr)) |kv| { + var g = kv.value; + v8.v8__Global__Reset(&g); + } } }; } diff --git a/src/browser/js/bridge.zig b/src/browser/js/bridge.zig index 57cff1fa..8135e7d7 100644 --- a/src/browser/js/bridge.zig +++ b/src/browser/js/bridge.zig @@ -118,11 +118,10 @@ pub fn Builder(comptime T: type) type { const ptr = v8.v8__WeakCallbackInfo__GetParameter(handle.?).?; const fc: *Session.FinalizerCallback = @ptrCast(@alignCast(ptr)); - const session = fc.session; const value_ptr = fc.ptr; - if (session.finalizer_callbacks.contains(@intFromPtr(value_ptr))) { - func(@ptrCast(@alignCast(value_ptr)), false, session); - session.releaseIdentity(value_ptr); + if (fc.identity.finalizer_callbacks.contains(@intFromPtr(value_ptr))) { + func(@ptrCast(@alignCast(value_ptr)), false, fc.session); + fc.releaseIdentity(); } else { // A bit weird, but v8 _requires_ that we release it // If we don't. We'll 100% crash. diff --git a/src/browser/js/js.zig b/src/browser/js/js.zig index 0c196e5b..10867167 100644 --- a/src/browser/js/js.zig +++ b/src/browser/js/js.zig @@ -25,6 +25,7 @@ pub const Env = @import("Env.zig"); pub const bridge = @import("bridge.zig"); pub const Caller = @import("Caller.zig"); pub const Origin = @import("Origin.zig"); +pub const Identity = @import("Identity.zig"); pub const Context = @import("Context.zig"); pub const Local = @import("Local.zig"); pub const Inspector = @import("Inspector.zig"); diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig index 58ed11b9..d6b35d83 100644 --- a/src/cdp/cdp.zig +++ b/src/cdp/cdp.zig @@ -489,7 +489,7 @@ pub fn BrowserContext(comptime CDP_T: type) type { pub fn createIsolatedWorld(self: *Self, world_name: []const u8, grant_universal_access: bool) !*IsolatedWorld { const browser = &self.cdp.browser; - const arena = try browser.arena_pool.acquire(); + const arena = try browser.arena_pool.acquire(.{ .debug = "IsolatedWorld" }); errdefer browser.arena_pool.release(arena); const world = try arena.create(IsolatedWorld); @@ -750,8 +750,13 @@ const IsolatedWorld = struct { context: ?*js.Context = null, grant_universal_access: bool, + // Identity tracking for this isolated world (separate from main world). + // This ensures CDP inspector contexts don't share v8::Globals with main world. + identity: js.Identity = .{}, + pub fn deinit(self: *IsolatedWorld) void { self.removeContext() catch {}; + self.identity.deinit(); self.browser.arena_pool.release(self.arena); } @@ -768,7 +773,12 @@ const IsolatedWorld = struct { // Currently we have only 1 page/frame and thus also only 1 state in the isolate world. pub fn createContext(self: *IsolatedWorld, page: *Page) !*js.Context { if (self.context == null) { - self.context = try self.browser.env.createContext(page); + const ctx = try self.browser.env.createContext(page, .{ + .identity = &self.identity, + .identity_arena = self.arena, + .debug_name = "IsolatedContext", + }); + self.context = ctx; } else { log.warn(.cdp, "not implemented", .{ .feature = "createContext: Not implemented second isolated context creation", From a4cb5031d160fc6993171f3cd7e4dc01b8a6b09a Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 19 Mar 2026 20:29:20 +0800 Subject: [PATCH 09/15] Tweak wait_until option Small tweaks to https://github.com/lightpanda-io/browser/pull/1896 Improve the wait ergonomics with an Option with default parameter. Revert page pointer logic to original (don't think that change was necessary). --- src/browser/Session.zig | 25 ++++++++++++++++--------- src/cdp/cdp.zig | 2 +- src/cdp/domains/lp.zig | 2 +- src/cdp/testing.zig | 2 +- src/lightpanda.zig | 2 +- src/main_legacy_test.zig | 2 +- src/mcp/tools.zig | 4 ++-- src/testing.zig | 4 ++-- 8 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/browser/Session.zig b/src/browser/Session.zig index 43576bf5..66334894 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -319,11 +319,15 @@ fn findPageBy(page: *Page, comptime field: []const u8, id: u32) ?*Page { return null; } -pub fn wait(self: *Session, wait_ms: u32, wait_until: lp.Config.WaitUntil) WaitResult { +const WaitOpts = struct { + timeout_ms: u32 = 5000, + until: lp.Config.WaitUntil = .load, +}; + +pub fn wait(self: *Session, opts: WaitOpts) WaitResult { + var page = &(self.page orelse return .no_page); while (true) { - if (self.page == null) return .no_page; - const page = &self.page.?; - const wait_result = self._wait(page, wait_ms, wait_until) catch |err| { + const wait_result = self._wait(page, opts) catch |err| { switch (err) { error.JsError => {}, // already logged (with hopefully more context) else => log.err(.browser, "session wait", .{ @@ -340,15 +344,18 @@ pub fn wait(self: *Session, wait_ms: u32, wait_until: lp.Config.WaitUntil) WaitR return .done; } self.processQueuedNavigation() catch return .done; + page = &self.page.?; // might have changed }, else => |result| return result, } } } -fn _wait(self: *Session, page: *Page, wait_ms: u32, wait_until: lp.Config.WaitUntil) !WaitResult { +fn _wait(self: *Session, page: *Page, opts: WaitOpts) !WaitResult { + const wait_until = opts.until; + var timer = try std.time.Timer.start(); - var ms_remaining = wait_ms; + var ms_remaining = opts.timeout_ms; const browser = self.browser; var http_client = browser.http_client; @@ -454,13 +461,13 @@ fn _wait(self: *Session, page: *Page, wait_ms: u32, wait_until: lp.Config.WaitUn // Same as above, except we have a scheduled task, // it just happens to be too far into the future // compared to how long we were told to wait. - if (!browser.hasBackgroundTasks()) { - if (is_event_done) return .done; - } else { + if (browser.hasBackgroundTasks()) { // _we_ have nothing to run, but v8 is working on // background tasks. We'll wait for them. browser.waitForBackgroundTasks(); } + // We're still wait for our wait_until. Not sure for what + // but let's keep waiting. Worst case, we'll timeout. ms = 20; } diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig index e3456226..cef34e07 100644 --- a/src/cdp/cdp.zig +++ b/src/cdp/cdp.zig @@ -131,7 +131,7 @@ pub fn CDPT(comptime TypeProvider: type) type { // timeouts (or http events) which are ready to be processed. pub fn pageWait(self: *Self, ms: u32) Session.WaitResult { const session = &(self.browser.session orelse return .no_page); - return session.wait(ms, .load); + return session.wait(.{ .timeout_ms = ms }); } // Called from above, in processMessage which handles client messages diff --git a/src/cdp/domains/lp.zig b/src/cdp/domains/lp.zig index 6c90571e..64fb616d 100644 --- a/src/cdp/domains/lp.zig +++ b/src/cdp/domains/lp.zig @@ -280,7 +280,7 @@ test "cdp.lp: action tools" { const page = try bc.session.createPage(); const url = "http://localhost:9582/src/browser/tests/mcp_actions.html"; try page.navigate(url, .{ .reason = .address_bar, .kind = .{ .push = null } }); - _ = bc.session.wait(5000, .load); + _ = bc.session.wait(.{}); // Test Click const btn = page.document.getElementById("btn", page).?.asNode(); diff --git a/src/cdp/testing.zig b/src/cdp/testing.zig index 9e739a31..34f28c94 100644 --- a/src/cdp/testing.zig +++ b/src/cdp/testing.zig @@ -136,7 +136,7 @@ const TestContext = struct { 0, ); try page.navigate(full_url, .{}); - _ = bc.session.wait(2000, .load); + _ = bc.session.wait(.{}); } return bc; } diff --git a/src/lightpanda.zig b/src/lightpanda.zig index 6096f2a8..1add3dc4 100644 --- a/src/lightpanda.zig +++ b/src/lightpanda.zig @@ -108,7 +108,7 @@ pub fn fetch(app: *App, url: [:0]const u8, opts: FetchOpts) !void { .reason = .address_bar, .kind = .{ .push = null }, }); - _ = session.wait(opts.wait_ms, opts.wait_until); + _ = session.wait(.{ .timeout_ms = opts.wait_ms, .until = opts.wait_until }); const writer = opts.writer orelse return; if (opts.dump_mode) |mode| { diff --git a/src/main_legacy_test.zig b/src/main_legacy_test.zig index 839fd484..0805e100 100644 --- a/src/main_legacy_test.zig +++ b/src/main_legacy_test.zig @@ -106,7 +106,7 @@ pub fn run(allocator: Allocator, file: []const u8, session: *lp.Session) !void { defer try_catch.deinit(); try page.navigate(url, .{}); - _ = session.wait(2000, .load); + _ = session.wait(.{}); ls.local.eval("testing.assertOk()", "testing.assertOk()") catch |err| { const caught = try_catch.caughtOrError(allocator, err); diff --git a/src/mcp/tools.zig b/src/mcp/tools.zig index f1b6cf7c..60fca2ce 100644 --- a/src/mcp/tools.zig +++ b/src/mcp/tools.zig @@ -538,7 +538,7 @@ fn performGoto(server: *Server, url: [:0]const u8, id: std.json.Value) !void { return error.NavigationFailed; }; - _ = server.session.wait(5000, .load); + _ = server.session.wait(.{}); } const testing = @import("../testing.zig"); @@ -603,7 +603,7 @@ test "MCP - Actions: click, fill, scroll" { const page = try server.session.createPage(); const url = "http://localhost:9582/src/browser/tests/mcp_actions.html"; try page.navigate(url, .{ .reason = .address_bar, .kind = .{ .push = null } }); - _ = server.session.wait(5000, .load); + _ = server.session.wait(.{}); // Test Click const btn = page.document.getElementById("btn", page).?.asNode(); diff --git a/src/testing.zig b/src/testing.zig index 0f9747d0..bdfeb915 100644 --- a/src/testing.zig +++ b/src/testing.zig @@ -415,7 +415,7 @@ fn runWebApiTest(test_file: [:0]const u8) !void { defer try_catch.deinit(); try page.navigate(url, .{}); - _ = test_session.wait(2000, .load); + _ = test_session.wait(.{}); test_browser.runMicrotasks(); @@ -439,7 +439,7 @@ pub fn pageTest(comptime test_file: []const u8) !*Page { ); try page.navigate(url, .{}); - _ = test_session.wait(2000, .load); + _ = test_session.wait(.{}); return page; } From 93e239f6827883f03f6f9c3dc21c84995c6aceba Mon Sep 17 00:00:00 2001 From: Halil Durak Date: Thu, 19 Mar 2026 12:23:19 +0300 Subject: [PATCH 10/15] bind more ECMAScript errors --- src/browser/js/Isolate.zig | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/browser/js/Isolate.zig b/src/browser/js/Isolate.zig index 32b0f7d6..08df142a 100644 --- a/src/browser/js/Isolate.zig +++ b/src/browser/js/Isolate.zig @@ -78,6 +78,21 @@ pub fn createError(self: Isolate, msg: []const u8) *const v8.Value { return v8.v8__Exception__Error(message).?; } +pub fn createRangeError(self: Isolate, msg: []const u8) *const v8.Value { + const message = self.initStringHandle(msg); + return v8.v8__Exception__RangeError(message).?; +} + +pub fn createReferenceError(self: Isolate, msg: []const u8) *const v8.Value { + const message = self.initStringHandle(msg); + return v8.v8__Exception__ReferenceError(message).?; +} + +pub fn createSyntaxError(self: Isolate, msg: []const u8) *const v8.Value { + const message = self.initStringHandle(msg); + return v8.v8__Exception__SyntaxError(message).?; +} + pub fn createTypeError(self: Isolate, msg: []const u8) *const v8.Value { const message = self.initStringHandle(msg); return v8.v8__Exception__TypeError(message).?; From 94190f93af218fddbcd941edda937df9f018411a Mon Sep 17 00:00:00 2001 From: Halil Durak Date: Thu, 19 Mar 2026 12:24:39 +0300 Subject: [PATCH 11/15] return correct errors from promises --- src/browser/js/Context.zig | 8 ++-- src/browser/js/Local.zig | 4 +- src/browser/js/PromiseResolver.zig | 41 +++++++++++++++---- src/browser/webapi/CustomElementRegistry.zig | 4 +- src/browser/webapi/SubtleCrypto.zig | 29 +++++++------ src/browser/webapi/net/Fetch.zig | 2 +- src/browser/webapi/net/Request.zig | 4 +- src/browser/webapi/net/Response.zig | 4 +- src/browser/webapi/streams/ReadableStream.zig | 2 +- .../streams/ReadableStreamDefaultReader.zig | 8 ++-- .../streams/WritableStreamDefaultWriter.zig | 10 ++--- 11 files changed, 72 insertions(+), 44 deletions(-) diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index da7362aa..cd775332 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -545,13 +545,13 @@ pub fn dynamicModuleCallback( break :blk js.String.toSliceZ(.{ .local = &local, .handle = resource_name.? }) catch |err| { log.err(.app, "OOM", .{ .err = err, .src = "dynamicModuleCallback1" }); - return @constCast((local.rejectPromise("Out of memory") catch return null).handle); + return @constCast(local.rejectPromise(.{ .generic_error = "Out of memory" }).handle); }; }; const specifier = js.String.toSliceZ(.{ .local = &local, .handle = v8_specifier.? }) catch |err| { log.err(.app, "OOM", .{ .err = err, .src = "dynamicModuleCallback2" }); - return @constCast((local.rejectPromise("Out of memory") catch return null).handle); + return @constCast(local.rejectPromise(.{ .generic_error = "Out of memory" }).handle); }; const normalized_specifier = self.script_manager.?.resolveSpecifier( @@ -560,14 +560,14 @@ pub fn dynamicModuleCallback( specifier, ) catch |err| { log.err(.app, "OOM", .{ .err = err, .src = "dynamicModuleCallback3" }); - return @constCast((local.rejectPromise("Out of memory") catch return null).handle); + return @constCast(local.rejectPromise(.{ .generic_error = "Out of memory" }).handle); }; const promise = self._dynamicModuleCallback(normalized_specifier, resource, &local) catch |err| blk: { log.err(.js, "dynamic module callback", .{ .err = err, }); - break :blk local.rejectPromise("Failed to load module") catch return null; + break :blk local.rejectPromise(.{ .generic_error = "Out of memory" }); }; return @constCast(promise.handle); } diff --git a/src/browser/js/Local.zig b/src/browser/js/Local.zig index f913dafd..af7d1116 100644 --- a/src/browser/js/Local.zig +++ b/src/browser/js/Local.zig @@ -1206,9 +1206,9 @@ pub fn stackTrace(self: *const Local) !?[]const u8 { } // == Promise Helpers == -pub fn rejectPromise(self: *const Local, value: anytype) !js.Promise { +pub fn rejectPromise(self: *const Local, comptime kind: js.PromiseResolver.RejectError) js.Promise { var resolver = js.PromiseResolver.init(self); - resolver.reject("Local.rejectPromise", value); + resolver.rejectError("Local.rejectPromise", kind); return resolver.promise(); } diff --git a/src/browser/js/PromiseResolver.zig b/src/browser/js/PromiseResolver.zig index 6386569a..7743fa48 100644 --- a/src/browser/js/PromiseResolver.zig +++ b/src/browser/js/PromiseResolver.zig @@ -22,6 +22,8 @@ const v8 = js.v8; const log = @import("../../log.zig"); const DOMException = @import("../webapi/DOMException.zig"); +const DOMException = @import("../webapi/DOMException.zig"); + const PromiseResolver = @This(); local: *const js.Local, @@ -65,20 +67,43 @@ pub fn reject(self: PromiseResolver, comptime source: []const u8, value: anytype }; } -pub const RejectError = union(enum) { - generic: []const u8, +pub const RejectError = union(enum(u4)) { + /// Not to be confused with `DOMException`; this is bare `Error`. + generic_error: []const u8, + range_error: []const u8, + reference_error: []const u8, + syntax_error: []const u8, type_error: []const u8, - dom_exception: anyerror, + wasm_compile_error: void, // TODO. + wasm_link_error: void, // TODO. + wasm_runtime_error: void, // TODO. + wasm_suspend_error: void, // TODO. + /// DOM exceptions are unknown to V8, belongs to web standards. + dom_exception: struct { err: anyerror }, }; -pub fn rejectError(self: PromiseResolver, comptime source: []const u8, err: RejectError) void { - const handle = switch (err) { - .type_error => |str| self.local.isolate.createTypeError(str), - .generic => |str| self.local.isolate.createError(str), + +/// Rejects the promise w/ an error object. +pub fn rejectError( + self: PromiseResolver, + comptime source: []const u8, + comptime kind: RejectError, +) void { + const handle = switch (kind) { + .generic_error => |msg| self.local.isolate.createError(msg), + .range_error => |msg| self.local.isolate.createRangeError(msg), + .reference_error => |msg| self.local.isolate.createReferenceError(msg), + .syntax_error => |msg| self.local.isolate.createSyntaxError(msg), + .type_error => |msg| self.local.isolate.createTypeError(msg), + // "Exceptional". .dom_exception => |exception| { - self.reject(source, DOMException.fromError(exception)); + self._reject(DOMException.fromError(exception.err) orelse unreachable) catch |reject_err| { + log.err(.bug, "rejectDomException", .{ .source = source, .err = reject_err, .persistent = false }); + }; return; }, + inline else => unreachable, }; + self._reject(js.Value{ .handle = handle, .local = self.local }) catch |reject_err| { log.err(.bug, "rejectError", .{ .source = source, .err = reject_err, .persistent = false }); }; diff --git a/src/browser/webapi/CustomElementRegistry.zig b/src/browser/webapi/CustomElementRegistry.zig index 6702a52e..25ed8857 100644 --- a/src/browser/webapi/CustomElementRegistry.zig +++ b/src/browser/webapi/CustomElementRegistry.zig @@ -125,8 +125,8 @@ pub fn whenDefined(self: *CustomElementRegistry, name: []const u8, page: *Page) return local.resolvePromise(definition.constructor); } - validateName(name) catch |err| { - return local.rejectPromise(DOMException.fromError(err) orelse unreachable); + validateName(name) catch |err| switch (err) { + error.SyntaxError => return local.rejectPromise(.{ .dom_exception = .{ .err = error.SyntaxError } }), }; const gop = try self._when_defined.getOrPut(page.arena, name); diff --git a/src/browser/webapi/SubtleCrypto.zig b/src/browser/webapi/SubtleCrypto.zig index 4839a69a..7d6df2e3 100644 --- a/src/browser/webapi/SubtleCrypto.zig +++ b/src/browser/webapi/SubtleCrypto.zig @@ -96,8 +96,8 @@ pub fn generateKey( key_usages: []const []const u8, page: *Page, ) !js.Promise { - const key_or_pair = CryptoKey.init(algorithm, extractable, key_usages, page) catch |err| { - return page.js.local.?.rejectPromise(@errorName(err)); + const key_or_pair = CryptoKey.init(algorithm, extractable, key_usages, page) catch { + return page.js.local.?.rejectPromise(.{ .dom_exception = .{ .err = error.SyntaxError } }); }; return page.js.local.?.resolvePromise(key_or_pair); @@ -112,7 +112,7 @@ pub fn exportKey( page: *Page, ) !js.Promise { if (!key.canExportKey()) { - return error.InvalidAccessError; + return page.js.local.?.rejectPromise(.{ .dom_exception = .{ .err = error.InvalidAccessError } }); } if (std.mem.eql(u8, format, "raw")) { @@ -124,9 +124,10 @@ pub fn exportKey( if (is_unsupported) { log.warn(.not_implemented, "SubtleCrypto.exportKey", .{ .format = format }); + return page.js.local.?.rejectPromise(.{ .dom_exception = .{ .err = error.NotSupported } }); } - return page.js.local.?.rejectPromise(@errorName(error.NotSupported)); + return page.js.local.?.rejectPromise(.{ .type_error = "invalid format" }); } /// Derive a secret key from a master key. @@ -148,7 +149,7 @@ pub fn deriveBits( log.warn(.not_implemented, "SubtleCrypto.deriveBits", .{ .name = name }); } - return page.js.local.?.rejectPromise(@errorName(error.NotSupported)); + return page.js.local.?.rejectPromise(.{ .dom_exception = .{ .err = error.NotSupported } }); }, }; } @@ -185,19 +186,19 @@ pub fn sign( .hmac => { // Verify algorithm. if (!algorithm.isHMAC()) { - return page.js.local.?.rejectPromise(@errorName(error.InvalidAccessError)); + return page.js.local.?.rejectPromise(.{ .dom_exception = .{ .err = error.InvalidAccessError } }); } // Call sign for HMAC. - const result = key.signHMAC(data, page) catch |err| { - return page.js.local.?.rejectPromise(@errorName(err)); + const result = key.signHMAC(data, page) catch { + return page.js.local.?.rejectPromise(.{ .dom_exception = .{ .err = error.InvalidAccessError } }); }; return page.js.local.?.resolvePromise(result); }, else => { log.warn(.not_implemented, "SubtleCrypto.sign", .{ .key_type = key._type }); - return page.js.local.?.rejectPromise(@errorName(error.InvalidAccessError)); + return page.js.local.?.rejectPromise(.{ .dom_exception = .{ .err = error.InvalidAccessError } }); }, }; } @@ -211,18 +212,20 @@ pub fn verify( data: []const u8, // ArrayBuffer. page: *Page, ) !js.Promise { - if (!algorithm.isHMAC()) return error.InvalidAccessError; + if (!algorithm.isHMAC()) { + return page.js.local.?.rejectPromise(.{ .dom_exception = .{ .err = error.InvalidAccessError } }); + } return switch (key._type) { .hmac => key.verifyHMAC(signature, data, page), - else => return error.InvalidAccessError, + else => page.js.local.?.rejectPromise(.{ .dom_exception = .{ .err = error.InvalidAccessError } }), }; } pub fn digest(_: *const SubtleCrypto, algorithm: []const u8, data: js.TypedArray(u8), page: *Page) !js.Promise { const local = page.js.local.?; if (algorithm.len > 10) { - return local.rejectPromise(DOMException.fromError(error.NotSupported)); + return local.rejectPromise(.{ .dom_exception = .{ .err = error.NotSupported } }); } const normalized = std.ascii.lowerString(&page.buf, algorithm); if (std.mem.eql(u8, normalized, "sha-1")) { @@ -245,7 +248,7 @@ pub fn digest(_: *const SubtleCrypto, algorithm: []const u8, data: js.TypedArray Sha512.hash(data.values, page.buf[0..Sha512.digest_length], .{}); return local.resolvePromise(js.ArrayBuffer{ .values = page.buf[0..Sha512.digest_length] }); } - return local.rejectPromise(DOMException.fromError(error.NotSupported)); + return local.rejectPromise(.{ .dom_exception = .{ .err = error.NotSupported } }); } /// Returns the desired digest by its name. diff --git a/src/browser/webapi/net/Fetch.zig b/src/browser/webapi/net/Fetch.zig index 0a44aae2..de1af4ec 100644 --- a/src/browser/webapi/net/Fetch.zig +++ b/src/browser/webapi/net/Fetch.zig @@ -243,7 +243,7 @@ fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void { defer ls.deinit(); // fetch() must reject with a TypeError on network errors per spec - ls.toLocal(self._resolver).rejectError("fetch error", .{ .type_error = @errorName(err) }); + ls.toLocal(self._resolver).rejectError("fetch error", .{ .type_error = "fetch error" }); } fn httpShutdownCallback(ctx: *anyopaque) void { diff --git a/src/browser/webapi/net/Request.zig b/src/browser/webapi/net/Request.zig index 6ba8f224..81ef8ba9 100644 --- a/src/browser/webapi/net/Request.zig +++ b/src/browser/webapi/net/Request.zig @@ -192,8 +192,8 @@ pub fn text(self: *const Request, page: *Page) !js.Promise { pub fn json(self: *const Request, page: *Page) !js.Promise { const body = self._body orelse ""; const local = page.js.local.?; - const value = local.parseJSON(body) catch |err| { - return local.rejectPromise(.{@errorName(err)}); + const value = local.parseJSON(body) catch { + return local.rejectPromise(.{ .syntax_error = "failed to parse" }); }; return local.resolvePromise(try value.persist()); } diff --git a/src/browser/webapi/net/Response.zig b/src/browser/webapi/net/Response.zig index 6a926369..7020ac05 100644 --- a/src/browser/webapi/net/Response.zig +++ b/src/browser/webapi/net/Response.zig @@ -139,8 +139,8 @@ pub fn getText(self: *const Response, page: *Page) !js.Promise { pub fn getJson(self: *Response, page: *Page) !js.Promise { const body = self._body orelse ""; const local = page.js.local.?; - const value = local.parseJSON(body) catch |err| { - return local.rejectPromise(.{@errorName(err)}); + const value = local.parseJSON(body) catch { + return local.rejectPromise(.{ .syntax_error = "failed to parse" }); }; return local.resolvePromise(try value.persist()); } diff --git a/src/browser/webapi/streams/ReadableStream.zig b/src/browser/webapi/streams/ReadableStream.zig index e4e5d0f9..56d5e9eb 100644 --- a/src/browser/webapi/streams/ReadableStream.zig +++ b/src/browser/webapi/streams/ReadableStream.zig @@ -255,7 +255,7 @@ pub fn pipeThrough(self: *ReadableStream, transform: PipeTransform, page: *Page) /// Returns a promise that resolves when piping is complete. pub fn pipeTo(self: *ReadableStream, destination: *WritableStream, page: *Page) !js.Promise { if (self.getLocked()) { - return page.js.local.?.rejectPromise("ReadableStream is locked"); + return page.js.local.?.rejectPromise(.{ .type_error = "ReadableStream is locked" }); } const local = page.js.local.?; diff --git a/src/browser/webapi/streams/ReadableStreamDefaultReader.zig b/src/browser/webapi/streams/ReadableStreamDefaultReader.zig index 2d3c5bbe..7bd848b7 100644 --- a/src/browser/webapi/streams/ReadableStreamDefaultReader.zig +++ b/src/browser/webapi/streams/ReadableStreamDefaultReader.zig @@ -58,12 +58,12 @@ pub const ReadResult = struct { pub fn read(self: *ReadableStreamDefaultReader, page: *Page) !js.Promise { const stream = self._stream orelse { - return page.js.local.?.rejectPromise("Reader has been released"); + return page.js.local.?.rejectPromise(.{ .type_error = "Reader has been released" }); }; if (stream._state == .errored) { - const err = stream._stored_error orelse "Stream errored"; - return page.js.local.?.rejectPromise(err); + //const err = stream._stored_error orelse "Stream errored"; + return page.js.local.?.rejectPromise(.{ .type_error = "Stream errored" }); } if (stream._controller.dequeue()) |chunk| { @@ -95,7 +95,7 @@ pub fn releaseLock(self: *ReadableStreamDefaultReader) void { pub fn cancel(self: *ReadableStreamDefaultReader, reason_: ?[]const u8, page: *Page) !js.Promise { const stream = self._stream orelse { - return page.js.local.?.rejectPromise("Reader has been released"); + return page.js.local.?.rejectPromise(.{ .type_error = "Reader has been released" }); }; self.releaseLock(); diff --git a/src/browser/webapi/streams/WritableStreamDefaultWriter.zig b/src/browser/webapi/streams/WritableStreamDefaultWriter.zig index 8ed552ea..f5e5e610 100644 --- a/src/browser/webapi/streams/WritableStreamDefaultWriter.zig +++ b/src/browser/webapi/streams/WritableStreamDefaultWriter.zig @@ -32,11 +32,11 @@ pub fn init(stream: *WritableStream, page: *Page) !*WritableStreamDefaultWriter pub fn write(self: *WritableStreamDefaultWriter, chunk: js.Value, page: *Page) !js.Promise { const stream = self._stream orelse { - return page.js.local.?.rejectPromise("Writer has been released"); + return page.js.local.?.rejectPromise(.{ .type_error = "Writer has been released" }); }; if (stream._state != .writable) { - return page.js.local.?.rejectPromise("Stream is not writable"); + return page.js.local.?.rejectPromise(.{ .type_error = "Stream is not writable" }); } try stream.writeChunk(chunk, page); @@ -46,11 +46,11 @@ pub fn write(self: *WritableStreamDefaultWriter, chunk: js.Value, page: *Page) ! pub fn close(self: *WritableStreamDefaultWriter, page: *Page) !js.Promise { const stream = self._stream orelse { - return page.js.local.?.rejectPromise("Writer has been released"); + return page.js.local.?.rejectPromise(.{ .type_error = "Writer has been released" }); }; if (stream._state != .writable) { - return page.js.local.?.rejectPromise("Stream is not writable"); + return page.js.local.?.rejectPromise(.{ .type_error = "Stream is not writable" }); } try stream.closeStream(page); @@ -67,7 +67,7 @@ pub fn releaseLock(self: *WritableStreamDefaultWriter) void { pub fn getClosed(self: *WritableStreamDefaultWriter, page: *Page) !js.Promise { const stream = self._stream orelse { - return page.js.local.?.rejectPromise("Writer has been released"); + return page.js.local.?.rejectPromise(.{ .type_error = "Writer has been released" }); }; if (stream._state == .closed) { From 84557cb4e6e86598181d5646212de1d1024c82f3 Mon Sep 17 00:00:00 2001 From: Matt Van Horn <455140+mvanhorn@users.noreply.github.com> Date: Thu, 19 Mar 2026 16:42:32 -0700 Subject: [PATCH 12/15] fix(cdp): send Target.detachedFromTarget event on detach detachFromTarget and setAutoAttach(false) both null bc.session_id without notifying the client. Per the CDP spec, detaching a session must fire a Target.detachedFromTarget event so the driver stops sending messages on the stale session ID. Capture the session_id before nulling it and fire the event in both code paths. Add tests covering the event emission and the no-session edge case. Fixes #1819 This contribution was developed with AI assistance (Claude Code + Codex). Co-Authored-By: Claude Opus 4.6 --- src/cdp/domains/target.zig | 55 +++++++++++++++++++++++++++++++------- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/src/cdp/domains/target.zig b/src/cdp/domains/target.zig index 1ff23849..51af7c53 100644 --- a/src/cdp/domains/target.zig +++ b/src/cdp/domains/target.zig @@ -394,15 +394,13 @@ fn sendMessageToTarget(cmd: anytype) !void { } fn detachFromTarget(cmd: anytype) !void { - // TODO check if sessionId/targetId match. - // const params = (try cmd.params(struct { - // sessionId: ?[]const u8, - // targetId: ?[]const u8, - // })) orelse return error.InvalidParams; - if (cmd.browser_context) |bc| { + if (bc.session_id) |session_id| { + try cmd.sendEvent("Target.detachedFromTarget", .{ + .sessionId = session_id, + }, .{}); + } bc.session_id = null; - // TODO should we send a Target.detachedFromTarget event? } return cmd.sendResult(null, .{}); @@ -427,8 +425,12 @@ fn setAutoAttach(cmd: anytype) !void { if (cmd.cdp.target_auto_attach == false) { // detach from all currently attached targets. if (cmd.browser_context) |bc| { + if (bc.session_id) |session_id| { + try cmd.sendEvent("Target.detachedFromTarget", .{ + .sessionId = session_id, + }, .{}); + } bc.session_id = null; - // TODO should we send a Target.detachedFromTarget event? } try cmd.sendResult(null, .{}); return; @@ -759,9 +761,11 @@ test "cdp.target: detachFromTarget" { try ctx.expectSentResult(.{ .targetId = bc.target_id.? }, .{ .id = 10 }); try ctx.processMessage(.{ .id = 11, .method = "Target.attachToTarget", .params = .{ .targetId = bc.target_id.? } }); - try ctx.expectSentResult(.{ .sessionId = bc.session_id.? }, .{ .id = 11 }); + const session_id = bc.session_id.?; + try ctx.expectSentResult(.{ .sessionId = session_id }, .{ .id = 11 }); try ctx.processMessage(.{ .id = 12, .method = "Target.detachFromTarget", .params = .{ .targetId = bc.target_id.? } }); + try ctx.expectSentEvent("Target.detachedFromTarget", .{ .sessionId = session_id }, .{}); try testing.expectEqual(null, bc.session_id); try ctx.expectSentResult(null, .{ .id = 12 }); @@ -769,3 +773,36 @@ test "cdp.target: detachFromTarget" { try ctx.expectSentResult(.{ .sessionId = bc.session_id.? }, .{ .id = 13 }); } } + +test "cdp.target: detachFromTarget without session" { + var ctx = testing.context(); + defer ctx.deinit(); + _ = try ctx.loadBrowserContext(.{ .id = "BID-9" }); + { + // detach when no session is attached should not send event + try ctx.processMessage(.{ .id = 10, .method = "Target.detachFromTarget" }); + try ctx.expectSentResult(null, .{ .id = 10 }); + try ctx.expectSentCount(0); + } +} + +test "cdp.target: setAutoAttach false sends detachedFromTarget" { + var ctx = testing.context(); + defer ctx.deinit(); + const bc = try ctx.loadBrowserContext(.{ .id = "BID-9" }); + { + try ctx.processMessage(.{ .id = 10, .method = "Target.createTarget", .params = .{ .browserContextId = "BID-9" } }); + try testing.expectEqual(true, bc.target_id != null); + try ctx.expectSentResult(.{ .targetId = bc.target_id.? }, .{ .id = 10 }); + + try ctx.processMessage(.{ .id = 11, .method = "Target.attachToTarget", .params = .{ .targetId = bc.target_id.? } }); + const session_id = bc.session_id.?; + try ctx.expectSentResult(.{ .sessionId = session_id }, .{ .id = 11 }); + + // setAutoAttach false should fire detachedFromTarget event + try ctx.processMessage(.{ .id = 12, .method = "Target.setAutoAttach", .params = .{ .autoAttach = false, .waitForDebuggerOnStart = false } }); + try ctx.expectSentEvent("Target.detachedFromTarget", .{ .sessionId = session_id }, .{}); + try testing.expectEqual(null, bc.session_id); + try ctx.expectSentResult(null, .{ .id = 12 }); + } +} From 02d05ae464335aac2bb3b4878b49c005351fd31a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Fri, 20 Mar 2026 09:38:35 +0900 Subject: [PATCH 13/15] Fix MCP error responses missing jsonrpc field Closes #1928 --- src/mcp/Server.zig | 6 +++--- src/mcp/router.zig | 10 +++++----- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/src/mcp/Server.zig b/src/mcp/Server.zig index e3ef8097..ffba7396 100644 --- a/src/mcp/Server.zig +++ b/src/mcp/Server.zig @@ -87,7 +87,7 @@ pub fn sendResult(self: *Self, id: std.json.Value, result: anytype) !void { } pub fn sendError(self: *Self, id: std.json.Value, code: protocol.ErrorCode, message: []const u8) !void { - try self.sendResponse(.{ + try self.sendResponse(protocol.Response{ .id = id, .@"error" = protocol.Error{ .code = @intFromEnum(code), @@ -114,7 +114,7 @@ test "MCP.Server - Integration: synchronous smoke test" { try router.processRequests(server, &in_reader); - try testing.expectJson(.{ .id = 1 }, out_alloc.writer.buffered()); + try testing.expectJson(.{ .jsonrpc = "2.0", .id = 1 }, out_alloc.writer.buffered()); } test "MCP.Server - Integration: ping request returns an empty result" { @@ -135,5 +135,5 @@ test "MCP.Server - Integration: ping request returns an empty result" { try router.processRequests(server, &in_reader); - try testing.expectJson(.{ .id = "ping-1", .result = .{} }, out_alloc.writer.buffered()); + try testing.expectJson(.{ .jsonrpc = "2.0", .id = "ping-1", .result = .{} }, out_alloc.writer.buffered()); } diff --git a/src/mcp/router.zig b/src/mcp/router.zig index 1bc02624..38b2e206 100644 --- a/src/mcp/router.zig +++ b/src/mcp/router.zig @@ -120,7 +120,7 @@ test "MCP.router - handleMessage - synchronous unit tests" { \\{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"test-client","version":"1.0.0"}}} ); try testing.expectJson( - \\{ "id": 1, "result": { "capabilities": { "tools": {} } } } + \\{ "jsonrpc": "2.0", "id": 1, "result": { "capabilities": { "tools": {} } } } , out_alloc.writer.buffered()); out_alloc.writer.end = 0; @@ -128,14 +128,14 @@ test "MCP.router - handleMessage - synchronous unit tests" { try handleMessage(server, aa, \\{"jsonrpc":"2.0","id":2,"method":"ping"} ); - try testing.expectJson(.{ .id = 2, .result = .{} }, out_alloc.writer.buffered()); + try testing.expectJson(.{ .jsonrpc = "2.0", .id = 2, .result = .{} }, out_alloc.writer.buffered()); out_alloc.writer.end = 0; // 3. Tools list try handleMessage(server, aa, \\{"jsonrpc":"2.0","id":3,"method":"tools/list"} ); - try testing.expectJson(.{ .id = 3 }, out_alloc.writer.buffered()); + try testing.expectJson(.{ .jsonrpc = "2.0", .id = 3 }, out_alloc.writer.buffered()); try testing.expect(std.mem.indexOf(u8, out_alloc.writer.buffered(), "\"name\":\"goto\"") != null); out_alloc.writer.end = 0; @@ -143,7 +143,7 @@ test "MCP.router - handleMessage - synchronous unit tests" { try handleMessage(server, aa, \\{"jsonrpc":"2.0","id":4,"method":"unknown_method"} ); - try testing.expectJson(.{ .id = 4, .@"error" = .{ .code = -32601 } }, out_alloc.writer.buffered()); + try testing.expectJson(.{ .jsonrpc = "2.0", .id = 4, .@"error" = .{ .code = -32601 } }, out_alloc.writer.buffered()); out_alloc.writer.end = 0; // 5. Parse error @@ -152,6 +152,6 @@ test "MCP.router - handleMessage - synchronous unit tests" { defer filter.deinit(); try handleMessage(server, aa, "invalid json"); - try testing.expectJson("{\"id\": null, \"error\": {\"code\": -32700}}", out_alloc.writer.buffered()); + try testing.expectJson("{\"jsonrpc\": \"2.0\", \"id\": null, \"error\": {\"code\": -32700}}", out_alloc.writer.buffered()); } } From 88681b1fdba7464c6646bd530cdeee653978a9cc Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Fri, 20 Mar 2026 16:50:03 +0800 Subject: [PATCH 14/15] Fix Context's call_arena The Context's call_arena should be based on the source, e.g. the IsolateWorld or the Page, not always the page. There's no rule that says all Contexts have to be a subset of the Page, and thus some might live longer and by doing so outlive the page_arena. Also, on context cleanup, isolate worlds now cleanup their identity. --- src/ArenaPool.zig | 1 - src/browser/Page.zig | 1 + src/browser/js/Context.zig | 4 +++- src/browser/js/Env.zig | 3 ++- src/cdp/cdp.zig | 9 +++++++++ 5 files changed, 15 insertions(+), 3 deletions(-) diff --git a/src/ArenaPool.zig b/src/ArenaPool.zig index 998838df..2e3f25a4 100644 --- a/src/ArenaPool.zig +++ b/src/ArenaPool.zig @@ -112,7 +112,6 @@ pub fn acquire(self: *ArenaPool, dbg: DebugInfo) !Allocator { } gop.value_ptr.* += 1; } - return entry.arena.allocator(); } diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 148654a0..eef70bbc 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -305,6 +305,7 @@ pub fn init(self: *Page, frame_id: u32, session: *Session, parent: ?*Page) !void self.js = try browser.env.createContext(self, .{ .identity = &session.identity, .identity_arena = session.page_arena, + .call_arena = self.call_arena, }); errdefer self.js.deinit(); diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index 2b861e7a..b46ec11d 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -63,7 +63,9 @@ templates: []*const v8.FunctionTemplate, // Arena for the lifetime of the context arena: Allocator, -// The page.call_arena +// The call_arena for this context. For main world contexts this is +// page.call_arena. For isolated world contexts this is a separate arena +// owned by the IsolatedWorld. call_arena: Allocator, // Because calls can be nested (i.e.a function calling a callback), diff --git a/src/browser/js/Env.zig b/src/browser/js/Env.zig index d248a411..9b3a1b4c 100644 --- a/src/browser/js/Env.zig +++ b/src/browser/js/Env.zig @@ -258,6 +258,7 @@ pub fn deinit(self: *Env) void { pub const ContextParams = struct { identity: *js.Identity, identity_arena: Allocator, + call_arena: Allocator, debug_name: []const u8 = "Context", }; @@ -325,7 +326,7 @@ pub fn createContext(self: *Env, page: *Page, params: ContextParams) !*Context { .arena = context_arena, .handle = context_global, .templates = self.templates, - .call_arena = page.call_arena, + .call_arena = params.call_arena, .microtask_queue = microtask_queue, .script_manager = &page._script_manager, .scheduler = .init(context_arena), diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig index d6b35d83..d3a2f64e 100644 --- a/src/cdp/cdp.zig +++ b/src/cdp/cdp.zig @@ -492,9 +492,13 @@ pub fn BrowserContext(comptime CDP_T: type) type { const arena = try browser.arena_pool.acquire(.{ .debug = "IsolatedWorld" }); errdefer browser.arena_pool.release(arena); + const call_arena = try browser.arena_pool.acquire(.{ .debug = "IsolatedWorld.call_arena" }); + errdefer browser.arena_pool.release(call_arena); + const world = try arena.create(IsolatedWorld); world.* = .{ .arena = arena, + .call_arena = call_arena, .context = null, .browser = browser, .name = try arena.dupe(u8, world_name), @@ -745,6 +749,7 @@ pub fn BrowserContext(comptime CDP_T: type) type { /// An object id is unique across all contexts, different object ids can refer to the same Node in different contexts. const IsolatedWorld = struct { arena: Allocator, + call_arena: Allocator, browser: *Browser, name: []const u8, context: ?*js.Context = null, @@ -757,6 +762,7 @@ const IsolatedWorld = struct { pub fn deinit(self: *IsolatedWorld) void { self.removeContext() catch {}; self.identity.deinit(); + self.browser.arena_pool.release(self.call_arena); self.browser.arena_pool.release(self.arena); } @@ -764,6 +770,8 @@ const IsolatedWorld = struct { const ctx = self.context orelse return error.NoIsolatedContextToRemove; self.browser.env.destroyContext(ctx); self.context = null; + self.identity.deinit(); + self.identity = .{}; } // The isolate world must share at least some of the state with the related page, specifically the DocumentHTML @@ -776,6 +784,7 @@ const IsolatedWorld = struct { const ctx = try self.browser.env.createContext(page, .{ .identity = &self.identity, .identity_arena = self.arena, + .call_arena = self.call_arena, .debug_name = "IsolatedContext", }); self.context = ctx; From de28d14afff9e2f83f7a2c67e6e0b36c1b96fb32 Mon Sep 17 00:00:00 2001 From: Halil Durak Date: Fri, 20 Mar 2026 13:35:12 +0300 Subject: [PATCH 15/15] give up on `switch (comptime kind)`, prefer `union(enum)` --- src/browser/js/Local.zig | 4 ++-- src/browser/js/PromiseResolver.zig | 12 +++--------- src/browser/webapi/Navigator.zig | 2 +- 3 files changed, 6 insertions(+), 12 deletions(-) diff --git a/src/browser/js/Local.zig b/src/browser/js/Local.zig index af7d1116..8ef62701 100644 --- a/src/browser/js/Local.zig +++ b/src/browser/js/Local.zig @@ -1206,9 +1206,9 @@ pub fn stackTrace(self: *const Local) !?[]const u8 { } // == Promise Helpers == -pub fn rejectPromise(self: *const Local, comptime kind: js.PromiseResolver.RejectError) js.Promise { +pub fn rejectPromise(self: *const Local, err: js.PromiseResolver.RejectError) js.Promise { var resolver = js.PromiseResolver.init(self); - resolver.rejectError("Local.rejectPromise", kind); + resolver.rejectError("Local.rejectPromise", err); return resolver.promise(); } diff --git a/src/browser/js/PromiseResolver.zig b/src/browser/js/PromiseResolver.zig index 7743fa48..fb3192c0 100644 --- a/src/browser/js/PromiseResolver.zig +++ b/src/browser/js/PromiseResolver.zig @@ -20,7 +20,6 @@ const js = @import("js.zig"); const v8 = js.v8; const log = @import("../../log.zig"); -const DOMException = @import("../webapi/DOMException.zig"); const DOMException = @import("../webapi/DOMException.zig"); @@ -67,17 +66,13 @@ pub fn reject(self: PromiseResolver, comptime source: []const u8, value: anytype }; } -pub const RejectError = union(enum(u4)) { +pub const RejectError = union(enum) { /// Not to be confused with `DOMException`; this is bare `Error`. generic_error: []const u8, range_error: []const u8, reference_error: []const u8, syntax_error: []const u8, type_error: []const u8, - wasm_compile_error: void, // TODO. - wasm_link_error: void, // TODO. - wasm_runtime_error: void, // TODO. - wasm_suspend_error: void, // TODO. /// DOM exceptions are unknown to V8, belongs to web standards. dom_exception: struct { err: anyerror }, }; @@ -86,9 +81,9 @@ pub const RejectError = union(enum(u4)) { pub fn rejectError( self: PromiseResolver, comptime source: []const u8, - comptime kind: RejectError, + err: RejectError, ) void { - const handle = switch (kind) { + const handle = switch (err) { .generic_error => |msg| self.local.isolate.createError(msg), .range_error => |msg| self.local.isolate.createRangeError(msg), .reference_error => |msg| self.local.isolate.createReferenceError(msg), @@ -101,7 +96,6 @@ pub fn rejectError( }; return; }, - inline else => unreachable, }; self._reject(js.Value{ .handle = handle, .local = self.local }) catch |reject_err| { diff --git a/src/browser/webapi/Navigator.zig b/src/browser/webapi/Navigator.zig index 218110c9..49f1f716 100644 --- a/src/browser/webapi/Navigator.zig +++ b/src/browser/webapi/Navigator.zig @@ -73,7 +73,7 @@ pub fn getStorage(self: *Navigator) *StorageManager { pub fn getBattery(_: *const Navigator, page: *Page) !js.Promise { log.info(.not_implemented, "navigator.getBattery", .{}); - return page.js.local.?.rejectErrorPromise(.{ .dom_exception = error.NotSupported }); + return page.js.local.?.rejectErrorPromise(.{ .dom_exception = .{ .err = error.NotSupported } }); } pub fn registerProtocolHandler(_: *const Navigator, scheme: []const u8, url: [:0]const u8, page: *const Page) !void {