From 752184b12b6ed490694860d5a48fe7b6383ccc2d Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 30 Mar 2026 11:01:44 +0800 Subject: [PATCH 1/2] Improve/Fix CDP navigation event order These changes all better align with chrome's event ordering/timing. There are two big changes. The first is that our internal page_navigated event, which is kind of our heavy hitter, is sent once the header is received as opposed to (much later) on document load. The main goal of this internal event is to trigger the "Page.frameNavigated" CDP event which is meant to happen once the URL is committed, which _is_ on header response. To accommodate this earlier trigger, new explicit events for DOMContentLoaded and load have be added. This drastically changes the flow of events as things go from: Start Page Navigation Response Received Start Frame Navigation Response Received End Frame Navigation End Page Navigation context clear + reset DOMContentLoaded Loaded TO: Start Page Navigation Response Received End Page Navigation context clear + reset Start Frame Navigation Response Received End Frame Navigation DOMContentLoaded Loaded So not only does it remove the nesting, but it ensures that the context are cleared and reset once the main page's navigation is locked in, and before any frame is created. --- src/Notification.zig | 16 +++++++++ src/browser/HttpClient.zig | 8 ++--- src/browser/Page.zig | 41 ++++++++++++++-------- src/cdp/CDP.zig | 12 +++++++ src/cdp/domains/page.zig | 69 ++++++++++++++++++++++++++++++++++++-- 5 files changed, 125 insertions(+), 21 deletions(-) diff --git a/src/Notification.zig b/src/Notification.zig index e025820a..6adbfd81 100644 --- a/src/Notification.zig +++ b/src/Notification.zig @@ -74,6 +74,8 @@ const EventListeners = struct { page_network_idle: List = .{}, page_network_almost_idle: List = .{}, page_frame_created: List = .{}, + page_dom_content_loaded: List = .{}, + page_loaded: List = .{}, http_request_fail: List = .{}, http_request_start: List = .{}, http_request_intercept: List = .{}, @@ -91,6 +93,8 @@ const Events = union(enum) { page_network_idle: *const PageNetworkIdle, page_network_almost_idle: *const PageNetworkAlmostIdle, page_frame_created: *const PageFrameCreated, + page_dom_content_loaded: *const PageDOMContentLoaded, + page_loaded: *const PageLoaded, http_request_fail: *const RequestFail, http_request_start: *const RequestStart, http_request_intercept: *const RequestIntercept, @@ -137,6 +141,18 @@ pub const PageFrameCreated = struct { timestamp: u64, }; +pub const PageDOMContentLoaded = struct { + req_id: u32, + frame_id: u32, + timestamp: u64, +}; + +pub const PageLoaded = struct { + req_id: u32, + frame_id: u32, + timestamp: u64, +}; + pub const RequestStart = struct { transfer: *Transfer, }; diff --git a/src/browser/HttpClient.zig b/src/browser/HttpClient.zig index c49d7e1f..db735474 100644 --- a/src/browser/HttpClient.zig +++ b/src/browser/HttpClient.zig @@ -1348,15 +1348,15 @@ pub const Transfer = struct { } } + transfer.req.notification.dispatch(.http_response_header_done, &.{ + .transfer = transfer, + }); + const proceed = transfer.req.header_callback(transfer) catch |err| { log.err(.http, "header_callback", .{ .err = err, .req = transfer }); return err; }; - transfer.req.notification.dispatch(.http_response_header_done, &.{ - .transfer = transfer, - }); - return proceed and transfer.aborted == false; } diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 314d20f0..3bf8962b 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -487,7 +487,6 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi return error.InjectBlankFailed; }; } - self.documentIsComplete(); session.notification.dispatch(.page_navigate, &.{ .frame_id = self._frame_id, @@ -519,6 +518,8 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi // force next request id manually b/c we won't create a real req. _ = session.browser.http_client.incrReqId(); + + self.documentIsComplete(); return; } @@ -738,6 +739,12 @@ pub fn _documentIsLoaded(self: *Page) !void { self.document.asEventTarget(), event, ); + + self._session.notification.dispatch(.page_dom_content_loaded, &.{ + .frame_id = self._frame_id, + .req_id = self._req_id, + .timestamp = timestamp(.monotonic), + }); } pub fn scriptsCompletedLoading(self: *Page) void { @@ -796,19 +803,6 @@ pub fn documentIsComplete(self: *Page) void { self._documentIsComplete() catch |err| { log.err(.page, "document is complete", .{ .err = err, .type = self._type, .url = self.url }); }; - - if (self._navigated_options) |no| { - // _navigated_options will be null in special short-circuit cases, like - // "navigating" to about:blank, in which case this notification has - // already been sent - self._session.notification.dispatch(.page_navigated, &.{ - .frame_id = self._frame_id, - .req_id = self._req_id, - .opts = no, - .url = self.url, - .timestamp = timestamp(.monotonic), - }); - } } fn _documentIsComplete(self: *Page) !void { @@ -827,6 +821,12 @@ fn _documentIsComplete(self: *Page) !void { try self._event_manager.dispatchDirect(window_target, event, self.window._on_load, .{ .inject_target = false, .context = "page load" }); } + self._session.notification.dispatch(.page_loaded, &.{ + .frame_id = self._frame_id, + .req_id = self._req_id, + .timestamp = timestamp(.monotonic), + }); + if (self._event_manager.hasDirectListeners(window_target, "pageshow", self.window._on_pageshow)) { const pageshow_event = (try PageTransitionEvent.initTrusted(comptime .wrap("pageshow"), .{}, self)).asEvent(); try self._event_manager.dispatchDirect(window_target, pageshow_event, self.window._on_pageshow, .{ .context = "page show" }); @@ -879,6 +879,19 @@ fn pageHeaderDoneCallback(transfer: *HttpClient.Transfer) !bool { }); } + if (self._navigated_options) |no| { + // _navigated_options will be null in special short-circuit cases, like + // "navigating" to about:blank, in which case this notification has + // already been sent + self._session.notification.dispatch(.page_navigated, &.{ + .frame_id = self._frame_id, + .req_id = self._req_id, + .opts = no, + .url = self.url, + .timestamp = timestamp(.monotonic), + }); + } + return true; } diff --git a/src/cdp/CDP.zig b/src/cdp/CDP.zig index 21005de4..bf94d5aa 100644 --- a/src/cdp/CDP.zig +++ b/src/cdp/CDP.zig @@ -432,6 +432,8 @@ pub fn BrowserContext(comptime CDP_T: type) type { try notification.register(.page_navigate, self, onPageNavigate); try notification.register(.page_navigated, self, onPageNavigated); try notification.register(.page_frame_created, self, onPageFrameCreated); + try notification.register(.page_dom_content_loaded, self, onPageDOMContentLoaded); + try notification.register(.page_loaded, self, onPageLoaded); } pub fn deinit(self: *Self) void { @@ -607,6 +609,16 @@ pub fn BrowserContext(comptime CDP_T: type) type { return @import("domains/page.zig").pageFrameCreated(self, msg); } + pub fn onPageDOMContentLoaded(ctx: *anyopaque, msg: *const Notification.PageDOMContentLoaded) !void { + const self: *Self = @ptrCast(@alignCast(ctx)); + return @import("domains/page.zig").pageDOMContentLoaded(self, msg); + } + + pub fn onPageLoaded(ctx: *anyopaque, msg: *const Notification.PageLoaded) !void { + const self: *Self = @ptrCast(@alignCast(ctx)); + return @import("domains/page.zig").pageLoaded(self, msg); + } + pub fn onPageNetworkIdle(ctx: *anyopaque, msg: *const Notification.PageNetworkIdle) !void { const self: *Self = @ptrCast(@alignCast(ctx)); return @import("domains/page.zig").pageNetworkIdle(self, msg); diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index 108d1e3b..bfac766b 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -472,9 +472,9 @@ pub fn pageNavigated(arena: Allocator, bc: anytype, event: *const Notification.P const page = bc.session.currentPage() orelse return error.PageNotLoaded; // When we actually recreated the context we should have the inspector send - // this event, see: resetContextGroup Sending this event will tell the - // client that the context ids they had are invalid and the context shouls - // be dropped The client will expect us to send new contextCreated events, + // this event, see: resetContextGroup. Sending this event will tell the + // client that the context ids they had are invalid and the context should + // be dropped. The client will expect us to send new contextCreated events, // such that the client has new id's for the active contexts. // Only send executionContextsCleared for main frame navigations. For child // frames (iframes), clearing all contexts would destroy the main frame's @@ -484,6 +484,18 @@ pub fn pageNavigated(arena: Allocator, bc: anytype, event: *const Notification.P try cdp.sendEvent("Runtime.executionContextsCleared", null, .{ .session_id = session_id }); } + // frameNavigated event + try cdp.sendEvent("Page.frameNavigated", .{ + .type = "Navigation", + .frame = Frame{ + .id = frame_id, + .url = event.url, + .loaderId = loader_id, + .securityOrigin = bc.security_origin, + .secureContextType = bc.secure_context_type, + }, + }, .{ .session_id = session_id }); + { const aux_data = try std.fmt.allocPrint(arena, "{{\"isDefault\":true,\"type\":\"default\",\"frameId\":\"{s}\",\"loaderId\":\"{s}\"}}", .{ frame_id, loader_id }); @@ -597,6 +609,57 @@ pub fn pageNavigated(arena: Allocator, bc: anytype, event: *const Notification.P }, .{ .session_id = session_id }); } +pub fn pageDOMContentLoaded(bc: anytype, event: *const Notification.PageDOMContentLoaded) !void { + const session_id = bc.session_id orelse return; + const timestamp = event.timestamp; + var cdp = bc.cdp; + + try cdp.sendEvent( + "Page.domContentEventFired", + .{ .timestamp = timestamp }, + .{ .session_id = session_id }, + ); + + if (bc.page_life_cycle_events) { + const frame_id = &id.toFrameId(event.frame_id); + const loader_id = &id.toLoaderId(event.req_id); + try cdp.sendEvent("Page.lifecycleEvent", LifecycleEvent{ + .timestamp = timestamp, + .name = "DOMContentLoaded", + .frameId = frame_id, + .loaderId = loader_id, + }, .{ .session_id = session_id }); + } +} + +pub fn pageLoaded(bc: anytype, event: *const Notification.PageLoaded) !void { + const session_id = bc.session_id orelse return; + const timestamp = event.timestamp; + var cdp = bc.cdp; + + const frame_id = &id.toFrameId(event.frame_id); + + try cdp.sendEvent( + "Page.loadEventFired", + .{ .timestamp = timestamp }, + .{ .session_id = session_id }, + ); + + if (bc.page_life_cycle_events) { + const loader_id = &id.toLoaderId(event.req_id); + try cdp.sendEvent("Page.lifecycleEvent", LifecycleEvent{ + .timestamp = timestamp, + .name = "load", + .frameId = frame_id, + .loaderId = loader_id, + }, .{ .session_id = session_id }); + } + + return cdp.sendEvent("Page.frameStoppedLoading", .{ + .frameId = frame_id, + }, .{ .session_id = session_id }); +} + pub fn pageNetworkIdle(bc: anytype, event: *const Notification.PageNetworkIdle) !void { return sendPageLifecycle(bc, "networkIdle", event.timestamp, &id.toFrameId(event.frame_id), &id.toLoaderId(event.req_id)); } From 568fa25add19e34309bbd2b04fc5806e88ba0acb Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 30 Mar 2026 11:39:38 +0800 Subject: [PATCH 2/2] Remove DOMContentLoaded and Loaded events from page_navigated These were moved to their own distinct events, and should have been removed from here. --- src/cdp/domains/page.zig | 42 ---------------------------------------- 1 file changed, 42 deletions(-) diff --git a/src/cdp/domains/page.zig b/src/cdp/domains/page.zig index bfac766b..9d26f000 100644 --- a/src/cdp/domains/page.zig +++ b/src/cdp/domains/page.zig @@ -420,7 +420,6 @@ pub fn pageNavigated(arena: Allocator, bc: anytype, event: *const Notification.P // things, but no session. const session_id = bc.session_id orelse return; - const timestamp = event.timestamp; const frame_id = &id.toFrameId(event.frame_id); const loader_id = &id.toLoaderId(event.req_id); @@ -566,47 +565,6 @@ pub fn pageNavigated(arena: Allocator, bc: anytype, event: *const Notification.P // chromedp client expects to receive the events is this order. // see https://github.com/chromedp/chromedp/issues/1558 try cdp.sendEvent("DOM.documentUpdated", null, .{ .session_id = session_id }); - - // domContentEventFired event - // TODO: partially hard coded - try cdp.sendEvent( - "Page.domContentEventFired", - .{ .timestamp = timestamp }, - .{ .session_id = session_id }, - ); - - // lifecycle DOMContentLoaded event - // TODO: partially hard coded - if (bc.page_life_cycle_events) { - try cdp.sendEvent("Page.lifecycleEvent", LifecycleEvent{ - .timestamp = timestamp, - .name = "DOMContentLoaded", - .frameId = frame_id, - .loaderId = loader_id, - }, .{ .session_id = session_id }); - } - - // loadEventFired event - try cdp.sendEvent( - "Page.loadEventFired", - .{ .timestamp = timestamp }, - .{ .session_id = session_id }, - ); - - // lifecycle DOMContentLoaded event - if (bc.page_life_cycle_events) { - try cdp.sendEvent("Page.lifecycleEvent", LifecycleEvent{ - .timestamp = timestamp, - .name = "load", - .frameId = frame_id, - .loaderId = loader_id, - }, .{ .session_id = session_id }); - } - - // frameStoppedLoading - return cdp.sendEvent("Page.frameStoppedLoading", .{ - .frameId = frame_id, - }, .{ .session_id = session_id }); } pub fn pageDOMContentLoaded(bc: anytype, event: *const Notification.PageDOMContentLoaded) !void {