From bf02e41919dfb4ecb45cdd4295cb63fb8b22958b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Fri, 7 Aug 2026 08:36:50 +0200 Subject: [PATCH] runner: advance idle notifications outside the wait-condition loop checkIdleNotifications was only called for conditions still pending, but an idle notification needs a check 500ms+ after the condition first held (Frame.IdleNotification). On a quiet page the same tick both starts that hold and resolves the condition via is_done, so nothing advanced the state machine for the rest of the wait. The CDP pump waits in 1s slices (CDP.pageWait), so Page.lifecycleEvent networkIdle/networkAlmostIdle was starved until a later slice built a fresh condition. That event is what puppeteer's networkidle0/networkidle2 and playwright's networkidle block on. Measured with puppeteer against lightpanda serve, goto(waitUntil: networkidle0) on example.com, median of 5: 2001ms before, 595ms after (the page loads in ~100ms, so ~600ms is the floor: load plus the 500ms hold). Every pre-fix run landed within 1999-2195ms - the stall is quantized to whole pump slices, not network variance. An ad-heavy page is unaffected either way (3196ms vs 2859ms, overlapping ranges): pending timers keep is_done false, so its condition never resolved early to begin with. Non-CDP waits are deliberately unchanged: waitForFrame(.networkidle) on a quiet page still resolves immediately via is_done rather than serving the 500ms hold. Agent/MCP waitForState and fetch --wait-until networkidle want "settled now", not chrome's lifecycle heuristic. --- src/browser/Runner.zig | 44 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 42 insertions(+), 2 deletions(-) diff --git a/src/browser/Runner.zig b/src/browser/Runner.zig index 26f95c28f..25ab29217 100644 --- a/src/browser/Runner.zig +++ b/src/browser/Runner.zig @@ -230,6 +230,18 @@ fn _tick(self: *Runner, comptime is_cdp: bool, timeout_ms: u32, conditions: []Wa const network_idle = activity.idle(); const is_done = browser.hasMacrotasks() == false and network_idle; + // Outside the condition loop: it skips resolved conditions, but an idle + // notification needs a check 500ms+ after the hold starts, and on a quiet + // page one tick both starts the hold and resolves the condition. Before + // it, so `.networkidle` conditions read fresh state. + var page_index: usize = 0; + while (page_index < session.pages.items.len) : (page_index += 1) { + // Indexed: notifyNetworkIdle dispatches to listeners. + const page = session.pages.items[page_index]; + if (page.replacement != null) continue; // frozen; the replacement is live + page.frame.checkIdleNotifications(total_http_activity); + } + // _we_ have nothing to run, but v8 is working on background tasks. We'll // wait for them. Don't do this for CDP, since new CDP messages can always // come in at any time. @@ -272,8 +284,6 @@ fn _tick(self: *Runner, comptime is_cdp: bool, timeout_ms: u32, conditions: []Wa } }, .html, .complete => { - frame.checkIdleNotifications(total_http_activity); - const met = switch (condition.until) { .done => is_done, .domcontentloaded => frame._load_state == .load or frame._load_state == .complete, @@ -296,6 +306,8 @@ fn _tick(self: *Runner, comptime is_cdp: bool, timeout_ms: u32, conditions: []Wa } } + // Always taken for is_cdp and every exit returns .ok, so _tick never yields + // .done to the CDP pump: _wait's .done/is_cdp arm is dormant. if ((comptime is_cdp) or want_http_tick) { const ms_to_next_task = blk: { if (has_runnable_page == false) { @@ -543,3 +555,31 @@ test "Runner: lazy iframe does not delay the load event" { try testing.expectEqual(true, lazy_child._load_state == .complete); try testing.expectEqual(true, lazy_child._parent_notified); } + +test "Runner: idle notifications advance past a resolved condition" { + const page = try testing.pageTest("runner/runner1.html", .{}); + defer page.close(); + + const frame = page.frame().?; + + // What a quiet page looks like one tick in: the hold has started, and the + // same tick resolved the wait condition. Seeded past the 500ms hold so the + // test doesn't spend it. + const held_since = lp.datetime.milliTimestamp(.boot) -| 600; + frame._notified_network_idle = .{ .triggered = held_since }; + frame._notified_network_almost_idle = .{ .triggered = held_since }; + + var conditions = [_]WaitCondition{.{ + .frame_id = page.frame_id, + .until = .done, + .status = .complete, + }}; + + // is_cdp mirrors CDP.pageWait, which keeps ticking after the condition + // resolves. + var runner = page.session.runner(.{}); + _ = try runner._wait(true, 50, &conditions); + + try testing.expectEqual(true, frame._notified_network_idle == .done); + try testing.expectEqual(true, frame._notified_network_almost_idle == .done); +}