mirror/browser
1
0
Fork 0
mirror of https://github.com/lightpanda-io/browser.git synced 2026-07-31 17:55:59 -04:00
Code Issues Packages Projects Releases Wiki Activity

script: fix segfault when a tool call triggers navigation

Browse Source 
A press/click that submits a form or follows a link commits a
replacement Page inside the tool call itself (finalizeAction ->
awaitQueuedNavigation), freeing the Frame that
Session._tool_frame_override still pointed at; finalizeAction then
dereferenced it via requireFrame -> currentFrame. Any script whose
press/click navigated crashed on replay.

Store a frame id instead of a pointer and resolve it at every
currentFrame call. The replacement page keeps the frame id (see
commitPendingPage), so the same handle stays routable across the swap.
This commit is contained in:
Adrià Arrufat
2026-07-03 23:52:29 +02:00
parent 933dc61bf6
commit ae2359c1af
2 changed files with 32 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
src/browser/Session.zig
View File

@@ -72,7 +72,10 @@ _nav_cursor: usize = 0,
// Set by the agent script Runtime around one tool call so each `Page` handle's
// tools act on its own frame, not `pages[0]`. Null for all other callers.
_tool_frame_override: ?*Frame = null,
// A frame id, not a pointer: a tool-triggered navigation commits a replacement
// Page mid-call, freeing the old Frame while keeping its frame id (see
// `commitPendingPage`).
_tool_frame_override: ?u32 = null,
// Loader IDs are scoped to the Session: each new BrowserContext gets a
// fresh counter. Frame IDs (`frame_id_gen`) live on `Browser` instead so
@@ -444,8 +447,9 @@ pub fn primaryPage(self: *Session) ?PageHandle {
// DEPRECATED. Exists during our transition to multi-page sessions.
pub fn currentFrame(self: *Session) ?*Frame {
if (self._tool_frame_override) |frame| {
return frame;
if (self._tool_frame_override) |frame_id| {
// No pages[0] fallthrough: the override targets one specific page.
return self.findFrameByFrameId(frame_id);
}
if (self.pages.items.len == 0) {
return null;
@@ -458,8 +462,8 @@ pub fn currentFrame(self: *Session) ?*Frame {
}
/// See `_tool_frame_override`. Pass null to clear.
pub fn setToolFrameOverride(self: *Session, frame: ?*Frame) void {
self._tool_frame_override = frame;
pub fn setToolFrameOverride(self: *Session, frame_id: ?u32) void {
self._tool_frame_override = frame_id;
}
// Multi-page aware: frame ids are globally unique (monotonic on `Browser`).

25
src/script/Runtime.zig
View File

@@ -449,10 +449,10 @@ fn invoke(self: *Runtime, tool: BrowserTool, info: *const v8.FunctionCallbackInf
// `pages[0]` is not necessarily this handle's page.
const frame_id = self.receiverFrameId(context, info) orelse
return self.throwError("page is not navigated or has been closed; call page.goto(url) first");
const frame = self.session.findFrameByFrameId(frame_id) orelse
if (self.session.findFrameByFrameId(frame_id) == null)
return self.throwError("page handle is no longer valid; the page was closed");
self.session.setToolFrameOverride(frame);
self.session.setToolFrameOverride(frame_id);
defer self.session.setToolFrameOverride(null);
const result = self.callTool(arena, tool, args) catch |err| switch (err) {
@@ -1083,6 +1083,27 @@ test "agent script runtime: parallel gotos coexist and route per page" {
);
}
test "agent script runtime: a tool-triggered navigation keeps the handle routable" {
defer testing.reset();
defer testing.test_session.closeAllPages();
var registry = CDPNode.Registry.init(testing.allocator);
defer registry.deinit();
const runtime = try Runtime.init(testing.allocator, testing.test_app, testing.test_session, &registry);
defer runtime.deinit();
// click commits a replacement Page mid-call; the frame override and the
// handle's frame id must both survive the swap.
try runTestScript(runtime,
\\const page = new Page();
\\await page.goto("http://localhost:9582/src/browser/tests/mcp_nav.html");
\\page.click("#navlink");
\\const href = page.evaluate("location.href");
\\if (href !== "about:blank") throw new Error("expected about:blank, got: " + href);
);
}
test "agent script runtime: re-goto on the same page object replaces its page" {
defer testing.reset();
defer testing.test_session.closeAllPages();