diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index 763cbf94..29c70aaf 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -1246,6 +1246,24 @@ pub fn isGoingAway(self: *const Frame) bool { return parent.isGoingAway(); } +// True if this frame, any descendant frame, or any worker owned by any +// of those frames is currently inside script evaluation. Used as a +// reentrancy guard before tearing down a page from a CDP message that +// may have been drained while a Zig->JS->Zig stack (e.g. Worker +// importScripts -> syncRequest -> blocking_read) is mid-flight. +// Recursive over child frames so an evaluating subframe also defers +// parent teardown. +pub fn anyScriptEvaluating(self: *const Frame) bool { + if (self._script_manager.base.is_evaluating) return true; + for (self.workers.items) |worker| { + if (worker._worker_scope._script_manager.is_evaluating) return true; + } + for (self.child_frames.items) |child| { + if (child.anyScriptEvaluating()) return true; + } + return false; +} + pub fn scriptAddedCallback(self: *Frame, comptime from_parser: bool, script: *Element.Html.Script) !void { if (self.isGoingAway()) { // if we're planning on navigating to another frame, don't run this script diff --git a/src/browser/Session.zig b/src/browser/Session.zig index 9d6f8c85..faf86ad2 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -200,8 +200,12 @@ pub fn removePage(self: *Session) void { lp.assert(false, "Session.removePage - page is null", .{}); }; - if (page.frame._script_manager.base.is_evaluating) { + if (page.frame.anyScriptEvaluating()) { // Reentrant teardown from a CDP message drained inside syncRequest; + // either the page's own script (frame ScriptManager.is_evaluating) + // or a Worker eval (Worker.loadInitialScript marks its + // _worker_scope._script_manager.is_evaluating). Tearing down here + // would free the arena/identity_map underneath the active eval. // Session.deinit reclaims the page when the connection closes. return; } diff --git a/src/browser/js/Scheduler.zig b/src/browser/js/Scheduler.zig index a4ed54fd..fa64229a 100644 --- a/src/browser/js/Scheduler.zig +++ b/src/browser/js/Scheduler.zig @@ -80,9 +80,8 @@ pub fn add(self: *Scheduler, ctx: *anyopaque, cb: Callback, run_in_ms: u32, opts } pub fn run(self: *Scheduler) !void { - const now = milliTimestamp(.monotonic); - try self.runQueue(&self.low_priority, now); - try self.runQueue(&self.high_priority, now); + try self.runQueue(&self.low_priority); + try self.runQueue(&self.high_priority); } pub fn hasReadyTasks(self: *Scheduler) bool { @@ -99,10 +98,12 @@ pub fn msToNextHigh(self: *Scheduler) ?u64 { return @intCast(task.run_at - now); } -fn runQueue(self: *Scheduler, queue: *Queue, now: u64) !void { +fn runQueue(self: *Scheduler, queue: *Queue) !void { if (queue.count() == 0) { return; } + const start = milliTimestamp(.monotonic); + var now = start; while (queue.peek()) |*task_| { if (task_.run_at > now) { @@ -126,6 +127,11 @@ fn runQueue(self: *Scheduler, queue: *Queue, now: u64) !void { task.run_at = now + ms; try self.low_priority.add(task); } + + now = milliTimestamp(.monotonic); + if (now - start > 500) { + return; + } } return; } diff --git a/src/browser/webapi/Worker.zig b/src/browser/webapi/Worker.zig index 5a271793..67d24526 100644 --- a/src/browser/webapi/Worker.zig +++ b/src/browser/webapi/Worker.zig @@ -187,6 +187,21 @@ fn loadInitialScript(self: *Worker, script: []const u8) !void { try_catch.init(&ls.local); defer try_catch.deinit(); + // Mark this worker's ScriptManager as evaluating for the lifetime of + // the eval. Worker scripts can call importScripts() which performs a + // synchronous HTTP request that pumps the CDP socket while waiting + // (HttpClient.syncRequest -> cdp.blocking_read). A CDP message such + // as Target.closeTarget arriving on that socket would otherwise tear + // down the page (Session.removePage -> Page.deinit -> Frame.deinit -> + // Worker.deinit) while this eval is mid-flight, freeing the worker's + // arena and identity_map underneath us. Session.removePage walks + // every frame's workers and bails out when any is_evaluating, so the + // teardown is deferred until the eval unwinds. + const sm = &self._worker_scope._script_manager; + const was_evaluating = sm.is_evaluating; + sm.is_evaluating = true; + defer sm.is_evaluating = was_evaluating; + _ = ls.local.eval(script, self._url) catch |err| { const caught = try_catch.caughtOrError(self._arena, err); log.err(.browser, "worker script error", .{ .url = self._url, .caught = caught }); diff --git a/src/browser/webapi/WorkerGlobalScope.zig b/src/browser/webapi/WorkerGlobalScope.zig index e9b8fe13..ea194382 100644 --- a/src/browser/webapi/WorkerGlobalScope.zig +++ b/src/browser/webapi/WorkerGlobalScope.zig @@ -374,6 +374,26 @@ fn importScript(self: *WorkerGlobalScope, arena: Allocator, url: [:0]const u8) ! var headers = try http_client.newHeaders(); try self.headersForRequest(&headers); + // Mark the worker's ScriptManager as evaluating for the duration of + // the synchronous fetch. syncRequest pumps the CDP socket while + // waiting (HttpClient.syncRequest -> cdp.blocking_read). A CDP + // message such as Target.closeTarget arriving on that socket would + // otherwise tear down the page (Session.removePage -> Page.deinit -> + // Frame.deinit -> Worker.deinit) while we're mid-fetch, freeing the + // worker's arena and identity_map underneath us. Frame.anyScriptEvaluating + // walks every frame's workers, so the teardown is deferred until the + // outer call unwinds. + // + // The typical caller (Worker.loadInitialScript) already sets this + // around its own eval, so this is a defense-in-depth nesting: a worker + // script that calls importScripts() from a setTimeout callback or a + // microtask wouldn't have the outer guard, but would still be safe + // because of this one. + const sm = &self._script_manager; + const was_evaluating = sm.is_evaluating; + sm.is_evaluating = true; + defer sm.is_evaluating = was_evaluating; + const response = http_client.syncRequest(arena, .{ .url = resolved_url, .method = .GET, diff --git a/src/browser/webapi/net/Fetch.zig b/src/browser/webapi/net/Fetch.zig index c186b818..bb9e97a7 100644 --- a/src/browser/webapi/net/Fetch.zig +++ b/src/browser/webapi/net/Fetch.zig @@ -94,7 +94,12 @@ pub fn init(input: Input, options: ?InitOpts, exec: *const Execution) !js.Promis .@"same-origin" => if (exec.isSameOrigin(request._url)) &session.cookie_jar else null, }; - try http_client.request(.{ + // Synchronous failures from request layers (e.g. RobotsLayer returning + // RobotsBlocked when robots.txt is already cached) are dispatched to + // httpErrorCallback by Client.request, which rejects the promise and + // releases response._arena. Propagating the error from here would also + // fire the `errdefer response.deinit` above and double-free the arena. + http_client.request(.{ .ctx = fetch, .params = .{ .url = request._url, @@ -114,7 +119,7 @@ pub fn init(input: Input, options: ?InitOpts, exec: *const Execution) !js.Promis .done_callback = httpDoneCallback, .error_callback = httpErrorCallback, .shutdown_callback = httpShutdownCallback, - }); + }) catch {}; return resolver.promise(); } diff --git a/src/cdp/CDP.zig b/src/cdp/CDP.zig index a235fda2..38f7c5ea 100644 --- a/src/cdp/CDP.zig +++ b/src/cdp/CDP.zig @@ -371,7 +371,7 @@ pub fn disposeBrowserContext(self: *CDP, browser_context_id: []const u8) bool { // (see Session.removePage's matching guard). Defer cleanup to // CDP.deinit at connection close, by which time eval has unwound. if (bc.session.currentPage()) |page| { - if (page.frame._script_manager.base.is_evaluating) { + if (page.frame.anyScriptEvaluating()) { return true; } } diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index b714da1e..a06a2908 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -39,11 +39,13 @@ pub fn processMessage(cmd: *CDP.Command) !void { const action = std.meta.stringToEnum(enum { enable, getFrameTree, + getNavigationHistory, setLifecycleEventsEnabled, addScriptToEvaluateOnNewDocument, removeScriptToEvaluateOnNewDocument, createIsolatedWorld, navigate, + navigateToHistoryEntry, reload, stopLoading, close, @@ -56,11 +58,13 @@ pub fn processMessage(cmd: *CDP.Command) !void { switch (action) { .enable => return cmd.sendResult(null, .{}), .getFrameTree => return getFrameTree(cmd), + .getNavigationHistory => return getNavigationHistory(cmd), .setLifecycleEventsEnabled => return setLifecycleEventsEnabled(cmd), .addScriptToEvaluateOnNewDocument => return addScriptToEvaluateOnNewDocument(cmd), .removeScriptToEvaluateOnNewDocument => return removeScriptToEvaluateOnNewDocument(cmd), .createIsolatedWorld => return createIsolatedWorld(cmd), .navigate => return navigate(cmd), + .navigateToHistoryEntry => return navigateToHistoryEntry(cmd), .reload => return doReload(cmd), .stopLoading => return cmd.sendResult(null, .{}), .close => return close(cmd), @@ -362,6 +366,86 @@ fn doReload(cmd: *CDP.Command) !void { }); } +const NavigationEntry = struct { + id: i64, + url: []const u8, + userTypedURL: []const u8, + title: []const u8, + transitionType: []const u8, +}; + +fn getNavigationHistory(cmd: *CDP.Command) !void { + const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; + if (bc.session_id == null) { + return error.SessionIdNotLoaded; + } + + const nav = &bc.session.navigation; + const entries_in = nav._entries.items; + + const entries_out = try cmd.arena.alloc(NavigationEntry, entries_in.len); + for (entries_in, 0..) |entry, i| { + // Navigation.pushEntry always formats _id as a decimal usize counter, + // so parse failure here is an internal invariant violation, not a + // recoverable runtime error. + const eid = std.fmt.parseInt(i64, entry._id, 10) catch @panic("Navigation entry _id is not a base-10 integer"); + entries_out[i] = .{ + .id = eid, + .url = entry._url orelse "", + .userTypedURL = entry._url orelse "", + .title = "", + .transitionType = "other", + }; + } + + return cmd.sendResult(.{ + .currentIndex = @as(i64, @intCast(nav._index)), + .entries = entries_out, + }, .{}); +} + +fn navigateToHistoryEntry(cmd: *CDP.Command) !void { + const params = (try cmd.params(struct { + entryId: i64, + })) orelse return error.InvalidParams; + + const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; + if (bc.session_id == null) { + return error.SessionIdNotLoaded; + } + + const session = bc.session; + const nav = &session.navigation; + + var target_index: ?usize = null; + var target_url: ?[:0]const u8 = null; + for (nav._entries.items, 0..) |entry, i| { + const eid = std.fmt.parseInt(i64, entry._id, 10) catch @panic("Navigation entry _id is not a base-10 integer"); + if (eid == params.entryId) { + target_index = i; + target_url = entry._url; + break; + } + } + + const idx = target_index orelse return error.InvalidParams; + const url = target_url orelse return error.InvalidParams; + + const frame = session.currentFrame() orelse return error.FrameNotLoaded; + + const opts = Frame.NavigateOpts{ + .reason = .history, + .cdp_id = cmd.input.id, + .kind = .{ .traverse = idx }, + }; + + if (frame._load_state == .waiting) { + return frame.navigate(url, opts); + } + + try session.initiateRootNavigation(frame._frame_id, url, opts); +} + pub fn frameNavigate(bc: *CDP.BrowserContext, event: *const Notification.FrameNavigate) !void { // detachTarget could be called, in which case, we still have a frame doing // things, but no session. @@ -1356,3 +1440,109 @@ test "cdp.frame: addScriptToEvaluateOnNewDocument" { try testing.expectEqual(2, try test_val.toI32()); } } + +test "cdp.frame: getNavigationHistory + navigateToHistoryEntry" { + var ctx = try testing.context(); + defer ctx.deinit(); + + { + // No browser context — should error. + try ctx.processMessage(.{ .id = 10, .method = "Page.getNavigationHistory" }); + try ctx.expectSentError(-31998, "BrowserContextNotLoaded", .{ .id = 10 }); + } + { + try ctx.processMessage(.{ .id = 11, .method = "Page.navigateToHistoryEntry", .params = .{ .entryId = 0 } }); + try ctx.expectSentError(-31998, "BrowserContextNotLoaded", .{ .id = 11 }); + } + + var bc = try ctx.loadBrowserContext(.{ .id = "BID-B2", .url = "cdp/dom1.html", .target_id = "TID-B2-0000000".* }); + + // Build up history: dom1.html (from loadBrowserContext) → dom2.html → dom3.html. + { + try ctx.processMessage(.{ + .id = 20, + .method = "Page.navigate", + .params = .{ .url = "http://127.0.0.1:9582/src/browser/tests/cdp/dom2.html" }, + }); + var runner = try bc.session.runner(.{}); + try runner.wait(.{ .ms = 2000 }); + } + { + try ctx.processMessage(.{ + .id = 21, + .method = "Page.navigate", + .params = .{ .url = "http://127.0.0.1:9582/src/browser/tests/cdp/dom3.html" }, + }); + var runner = try bc.session.runner(.{}); + try runner.wait(.{ .ms = 2000 }); + } + + // Three entries (ids 0, 1, 2), currentIndex points at the most-recent. + { + try ctx.processMessage(.{ .id = 30, .method = "Page.getNavigationHistory" }); + try ctx.expectSentResult(.{ + .currentIndex = 2, + .entries = &[_]NavigationEntry{ + .{ + .id = 0, + .url = "http://127.0.0.1:9582/src/browser/tests/cdp/dom1.html", + .userTypedURL = "http://127.0.0.1:9582/src/browser/tests/cdp/dom1.html", + .title = "", + .transitionType = "other", + }, + .{ + .id = 1, + .url = "http://127.0.0.1:9582/src/browser/tests/cdp/dom2.html", + .userTypedURL = "http://127.0.0.1:9582/src/browser/tests/cdp/dom2.html", + .title = "", + .transitionType = "other", + }, + .{ + .id = 2, + .url = "http://127.0.0.1:9582/src/browser/tests/cdp/dom3.html", + .userTypedURL = "http://127.0.0.1:9582/src/browser/tests/cdp/dom3.html", + .title = "", + .transitionType = "other", + }, + }, + }, .{ .id = 30 }); + } + + // Traverse back to the first entry. + { + try ctx.processMessage(.{ + .id = 40, + .method = "Page.navigateToHistoryEntry", + .params = .{ .entryId = 0 }, + }); + var runner = try bc.session.runner(.{}); + try runner.wait(.{ .ms = 2000 }); + + const f = bc.session.currentFrame() orelse unreachable; + try testing.expectEqualSlices(u8, "http://127.0.0.1:9582/src/browser/tests/cdp/dom1.html", f.url); + } + + // Traverse forward to the middle entry. + { + try ctx.processMessage(.{ + .id = 41, + .method = "Page.navigateToHistoryEntry", + .params = .{ .entryId = 1 }, + }); + var runner = try bc.session.runner(.{}); + try runner.wait(.{ .ms = 2000 }); + + const f = bc.session.currentFrame() orelse unreachable; + try testing.expectEqualSlices(u8, "http://127.0.0.1:9582/src/browser/tests/cdp/dom2.html", f.url); + } + + // Unknown entryId — InvalidParams. + { + try ctx.processMessage(.{ + .id = 42, + .method = "Page.navigateToHistoryEntry", + .params = .{ .entryId = 9999 }, + }); + try ctx.expectSentError(-31998, "InvalidParams", .{ .id = 42 }); + } +}