Merge pull request #2032 from lightpanda-io/cdp_navigation_event_order

Improve/Fix CDP navigation event order

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,
 };

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;
     }
 

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;
 }
 

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);

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);
 
@@ -472,9 +471,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 +483,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 });
 
@@ -554,18 +565,22 @@ 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 });
+}
+
+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;
 
-    // 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) {
+        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",
@@ -573,16 +588,23 @@ pub fn pageNavigated(arena: Allocator, bc: anytype, event: *const Notification.P
             .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);
 
-    // loadEventFired event
     try cdp.sendEvent(
         "Page.loadEventFired",
         .{ .timestamp = timestamp },
         .{ .session_id = session_id },
     );
 
-    // lifecycle DOMContentLoaded event
     if (bc.page_life_cycle_events) {
+        const loader_id = &id.toLoaderId(event.req_id);
         try cdp.sendEvent("Page.lifecycleEvent", LifecycleEvent{
             .timestamp = timestamp,
             .name = "load",
@@ -591,7 +613,6 @@ pub fn pageNavigated(arena: Allocator, bc: anytype, event: *const Notification.P
         }, .{ .session_id = session_id });
     }
 
-    // frameStoppedLoading
     return cdp.sendEvent("Page.frameStoppedLoading", .{
         .frameId = frame_id,
     }, .{ .session_id = session_id });