From a4cb5031d160fc6993171f3cd7e4dc01b8a6b09a Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Thu, 19 Mar 2026 20:29:20 +0800 Subject: [PATCH] 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; }