Merge pull request #1465 from lightpanda-io/wp/mrdimidium/direct-telemetry

Telemetry without notifications

src/App.zig

@@ -27,7 +27,6 @@ const Telemetry = @import("telemetry/telemetry.zig").Telemetry;
 
 pub const Http = @import("http/Http.zig");
 pub const ArenaPool = @import("ArenaPool.zig");
-pub const Notification = @import("Notification.zig");
 
 // Container for global state / objects that various parts of the system
 // might need.
@@ -41,7 +40,6 @@ telemetry: Telemetry,
 allocator: Allocator,
 arena_pool: ArenaPool,
 app_dir_path: ?[]const u8,
-notification: *Notification,
 shutdown: bool = false,
 
 pub const RunMode = enum {
@@ -70,9 +68,6 @@ pub fn init(allocator: Allocator, config: Config) !*App {
     app.config = config;
     app.allocator = allocator;
 
-    app.notification = try Notification.init(allocator, null);
-    errdefer app.notification.deinit();
-
     app.http = try Http.init(allocator, .{
         .max_host_open = config.http_max_host_open orelse 4,
         .max_concurrent = config.http_max_concurrent orelse 10,
@@ -96,8 +91,6 @@ pub fn init(allocator: Allocator, config: Config) !*App {
     app.telemetry = try Telemetry.init(app, config.run_mode);
     errdefer app.telemetry.deinit();
 
-    try app.telemetry.register(app.notification);
-
     app.arena_pool = ArenaPool.init(allocator);
     errdefer app.arena_pool.deinit();
 
@@ -115,7 +108,6 @@ pub fn deinit(self: *App) void {
         self.app_dir_path = null;
     }
     self.telemetry.deinit();
-    self.notification.deinit();
     self.http.deinit();
     self.snapshot.deinit();
     self.platform.deinit();

src/Notification.zig

@@ -39,10 +39,9 @@ const List = std.DoublyLinkedList;
 // CDP code registers for the "network_bytes_sent" event, because it needs to
 // send messages to the client when this happens. Our HTTP client could then
 // emit a "network_bytes_sent" message. It would be easy, and it would work.
-// That is, it would work until the Telemetry code makes an HTTP request, and
+// That is, it would work until multiple CDP clients connect, and because
-// because everything's just one big global, that gets picked up by the
+// everything's just one big global, events from one CDP session would be sent
-// registered CDP listener, and the telemetry network activity gets sent to the
+// to all CDP clients.
-// CDP client.
 //
 // To avoid this, one way or another, we need scoping. We could still have
 // a global registry but every "register" and every "emit" has some type of
@@ -50,14 +49,10 @@ const List = std.DoublyLinkedList;
 // between components to share a common scope.
 //
 // Instead, the approach that we take is to have a notification instance per
-// scope. This makes some things harder, but we only plan on having 2
+// CDP connection (BrowserContext). Each CDP connection has its own notification
-// notification instances  at a given time: one in a Browser and one in the App.
+// that is shared across all Sessions (tabs) within that connection. This ensures
-// What about something like Telemetry, which lives outside of a Browser but
+// proper isolation between different CDP clients while allowing a single client
-// still cares about Browser-events (like .page_navigate)? When the Browser
+// to receive events from all its tabs.
-// notification is created, a `notification_created` event is raised in the
-// App's notification, which Telemetry is registered for. This allows Telemetry
-// to register for events in the Browser notification. See the Telemetry's
-// register function.
 const Notification = @This();
 // Every event type (which are hard-coded), has a list of Listeners.
 // When the event happens, we dispatch to those listener.
@@ -85,7 +80,6 @@ const EventListeners = struct {
     http_request_auth_required: List = .{},
     http_response_data: List = .{},
     http_response_header_done: List = .{},
-    notification_created: List = .{},
 };
 
 const Events = union(enum) {
@@ -102,7 +96,6 @@ const Events = union(enum) {
     http_request_done: *const RequestDone,
     http_response_data: *const ResponseData,
     http_response_header_done: *const ResponseHeaderDone,
-    notification_created: *Notification,
 };
 const EventType = std.meta.FieldEnum(Events);
 
@@ -162,12 +155,7 @@ pub const RequestFail = struct {
     err: anyerror,
 };
 
-pub fn init(allocator: Allocator, parent: ?*Notification) !*Notification {
+pub fn init(allocator: Allocator) !*Notification {
-
-    // This is put on the heap because we want to raise a .notification_created
-    // event, so that, something like Telemetry, can receive the
-    // .page_navigate event on all notification instances. That can only work
-    // if we dispatch .notification_created with a *Notification.
     const notification = try allocator.create(Notification);
     errdefer allocator.destroy(notification);
 
@@ -178,10 +166,6 @@ pub fn init(allocator: Allocator, parent: ?*Notification) !*Notification {
         .mem_pool = std.heap.MemoryPool(Listener).init(allocator),
     };
 
-    if (parent) |pn| {
-        pn.dispatch(.notification_created, notification);
-    }
-
     return notification;
 }
 
@@ -256,6 +240,9 @@ pub fn unregisterAll(self: *Notification, receiver: *anyopaque) void {
 }
 
 pub fn dispatch(self: *Notification, comptime event: EventType, data: ArgType(event)) void {
+    if (self.listeners.count() == 0) {
+        return;
+    }
     const list = &@field(self.event_listeners, @tagName(event));
 
     var node = list.first;
@@ -313,7 +300,7 @@ const Listener = struct {
 
 const testing = std.testing;
 test "Notification" {
-    var notifier = try Notification.init(testing.allocator, null);
+    var notifier = try Notification.init(testing.allocator);
     defer notifier.deinit();
 
     // noop

src/browser/Browser.zig

@@ -27,11 +27,11 @@ const App = @import("../App.zig");
 
 const ArenaPool = App.ArenaPool;
 const HttpClient = App.Http.Client;
-const Notification = App.Notification;
 
 const IS_DEBUG = @import("builtin").mode == .Debug;
 
 const Session = @import("Session.zig");
+const Notification = @import("../Notification.zig");
 
 // Browser is an instance of the browser.
 // You can create multiple browser instances.
@@ -48,7 +48,6 @@ call_arena: ArenaAllocator,
 page_arena: ArenaAllocator,
 session_arena: ArenaAllocator,
 transfer_arena: ArenaAllocator,
-notification: *Notification,
 
 const InitOpts = struct {
     env: js.Env.InitOpts = .{},
@@ -60,17 +59,11 @@ pub fn init(app: *App, opts: InitOpts) !Browser {
     var env = try js.Env.init(app, opts.env);
     errdefer env.deinit();
 
-    const notification = try Notification.init(allocator, app.notification);
-    app.http.client.notification = notification;
-    app.http.client.next_request_id = 0; // Should we track ids in CDP only?
-    errdefer notification.deinit();
-
     return .{
         .app = app,
         .env = env,
         .session = null,
         .allocator = allocator,
-        .notification = notification,
         .arena_pool = &app.arena_pool,
         .http_client = app.http.client,
         .call_arena = ArenaAllocator.init(allocator),
@@ -87,15 +80,13 @@ pub fn deinit(self: *Browser) void {
     self.page_arena.deinit();
     self.session_arena.deinit();
     self.transfer_arena.deinit();
-    self.http_client.notification = null;
-    self.notification.deinit();
 }
 
-pub fn newSession(self: *Browser) !*Session {
+pub fn newSession(self: *Browser, notification: *Notification) !*Session {
     self.closeSession();
     self.session = @as(Session, undefined);
     const session = &self.session.?;
-    try Session.init(session, self);
+    try Session.init(session, self, notification);
     return session;
 }
 

src/browser/Page.zig

@@ -478,14 +478,22 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi
         // This assumption may be false when CDP Page.addScriptToEvaluateOnNewDocument is implemented
         self.documentIsComplete();
 
-        self._session.browser.notification.dispatch(.page_navigate, &.{
+        self._session.notification.dispatch(.page_navigate, &.{
             .req_id = req_id,
             .opts = opts,
             .url = request_url,
             .timestamp = timestamp(.monotonic),
         });
 
-        self._session.browser.notification.dispatch(.page_navigated, &.{
+        // Record telemetry for navigation
+        self._session.browser.app.telemetry.record(.{
+            .navigate = .{
+                .tls = false, // about:blank is not TLS
+                .proxy = self._session.browser.app.config.http_proxy != null,
+            },
+        });
+
+        self._session.notification.dispatch(.page_navigated, &.{
             .req_id = req_id,
             .opts = .{
                 .cdp_id = opts.cdp_id,
@@ -520,13 +528,19 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi
 
     // We dispatch page_navigate event before sending the request.
     // It ensures the event page_navigated is not dispatched before this one.
-    self._session.browser.notification.dispatch(.page_navigate, &.{
+    self._session.notification.dispatch(.page_navigate, &.{
         .req_id = req_id,
         .opts = opts,
         .url = self.url,
         .timestamp = timestamp(.monotonic),
     });
 
+    // Record telemetry for navigation
+    self._session.browser.app.telemetry.record(.{ .navigate = .{
+        .tls = std.ascii.startsWithIgnoreCase(self.url, "https://"),
+        .proxy = self._session.browser.app.config.http_proxy != null,
+    } });
+
     session.navigation._current_navigation_kind = opts.kind;
 
     http_client.request(.{
@@ -537,6 +551,7 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi
         .body = opts.body,
         .cookie_jar = &self._session.cookie_jar,
         .resource_type = .document,
+        .notification = self._session.notification,
         .header_callback = pageHeaderDoneCallback,
         .data_callback = pageDataCallback,
         .done_callback = pageDoneCallback,
@@ -664,7 +679,7 @@ pub fn documentIsComplete(self: *Page) void {
         std.debug.assert(self._navigated_options != null);
     }
 
-    self._session.browser.notification.dispatch(.page_navigated, &.{
+    self._session.notification.dispatch(.page_navigated, &.{
         .req_id = self._req_id.?,
         .opts = self._navigated_options.?,
         .url = self.url,
@@ -1413,14 +1428,14 @@ pub fn deliverSlotchangeEvents(self: *Page) void {
 
 fn notifyNetworkIdle(self: *Page) void {
     lp.assert(self._notified_network_idle == .done, "Page.notifyNetworkIdle", .{});
-    self._session.browser.notification.dispatch(.page_network_idle, &.{
+    self._session.notification.dispatch(.page_network_idle, &.{
         .timestamp = timestamp(.monotonic),
     });
 }
 
 fn notifyNetworkAlmostIdle(self: *Page) void {
     lp.assert(self._notified_network_almost_idle == .done, "Page.notifyNetworkAlmostIdle", .{});
-    self._session.browser.notification.dispatch(.page_network_almost_idle, &.{
+    self._session.notification.dispatch(.page_network_almost_idle, &.{
         .timestamp = timestamp(.monotonic),
     });
 }

src/browser/ScriptManager.zig

@@ -266,6 +266,7 @@ pub fn addFromElement(self: *ScriptManager, comptime from_parser: bool, script_e
             .blocking = is_blocking,
             .cookie_jar = &page._session.cookie_jar,
             .resource_type = .script,
+            .notification = page._session.notification,
             .start_callback = if (log.enabled(.http, .debug)) Script.startCallback else null,
             .header_callback = Script.headerCallback,
             .data_callback = Script.dataCallback,
@@ -380,6 +381,7 @@ pub fn preloadImport(self: *ScriptManager, url: [:0]const u8, referrer: []const
         .headers = try self.getHeaders(url),
         .cookie_jar = &self.page._session.cookie_jar,
         .resource_type = .script,
+        .notification = self.page._session.notification,
         .start_callback = if (log.enabled(.http, .debug)) Script.startCallback else null,
         .header_callback = Script.headerCallback,
         .data_callback = Script.dataCallback,
@@ -481,6 +483,7 @@ pub fn getAsyncImport(self: *ScriptManager, url: [:0]const u8, cb: ImportAsync.C
         .ctx = script,
         .resource_type = .script,
         .cookie_jar = &self.page._session.cookie_jar,
+        .notification = self.page._session.notification,
         .start_callback = if (log.enabled(.http, .debug)) Script.startCallback else null,
         .header_callback = Script.headerCallback,
         .data_callback = Script.dataCallback,

src/browser/Session.zig

@@ -28,6 +28,7 @@ const History = @import("webapi/History.zig");
 
 const Page = @import("Page.zig");
 const Browser = @import("Browser.zig");
+const Notification = @import("../Notification.zig");
 
 const Allocator = std.mem.Allocator;
 const IS_DEBUG = @import("builtin").mode == .Debug;
@@ -39,6 +40,7 @@ const IS_DEBUG = @import("builtin").mode == .Debug;
 const Session = @This();
 
 browser: *Browser,
+notification: *Notification,
 
 // Used to create our Inspector and in the BrowserContext.
 arena: Allocator,
@@ -61,7 +63,7 @@ navigation: Navigation,
 
 page: ?Page,
 
-pub fn init(self: *Session, browser: *Browser) !void {
+pub fn init(self: *Session, browser: *Browser, notification: *Notification) !void {
     const allocator = browser.app.allocator;
     const session_allocator = browser.session_arena.allocator();
 
@@ -71,6 +73,7 @@ pub fn init(self: *Session, browser: *Browser) !void {
         .navigation = .{},
         .storage_shed = .{},
         .browser = browser,
+        .notification = notification,
         .arena = session_allocator,
         .cookie_jar = storage.Cookie.Jar.init(allocator),
         .transfer_arena = browser.transfer_arena.allocator(),
@@ -104,14 +107,14 @@ pub fn createPage(self: *Session) !*Page {
     }
     // start JS env
     // Inform CDP the main page has been created such that additional context for other Worlds can be created as well
-    self.browser.notification.dispatch(.page_created, page);
+    self.notification.dispatch(.page_created, page);
 
     return page;
 }
 
 pub fn removePage(self: *Session) void {
     // Inform CDP the page is going to be removed, allowing other worlds to remove themselves before the main one
-    self.browser.notification.dispatch(.page_remove, .{});
+    self.notification.dispatch(.page_remove, .{});
     lp.assert(self.page != null, "Session.removePage - page is null", .{});
 
     self.page.?.deinit();
@@ -137,10 +140,13 @@ pub const WaitResult = enum {
 
 pub fn wait(self: *Session, wait_ms: u32) WaitResult {
     while (true) {
-        const page = &(self.page orelse return .no_page);
+        if (self.page) |*page| {
-        switch (page.wait(wait_ms)) {
+            switch (page.wait(wait_ms)) {
-            .navigate => self.processScheduledNavigation() catch return .done,
+                .navigate => self.processScheduledNavigation() catch return .done,
-            else => |result| return result,
+                else => |result| return result,
+            }
+        } else {
+            return .no_page;
         }
         // if we've successfull navigated, we'll give the new page another
         // page.wait(wait_ms)
@@ -148,7 +154,7 @@ pub fn wait(self: *Session, wait_ms: u32) WaitResult {
 }
 
 fn processScheduledNavigation(self: *Session) !void {
-    errdefer _ = self.browser.transfer_arena.reset(.{ .retain_with_limit = 8 * 1024 });
+    defer _ = self.browser.transfer_arena.reset(.{ .retain_with_limit = 8 * 1024 });
     const url, const opts = blk: {
         const qn = self.page.?._queued_navigation.?;
         // qn might not be safe to use after self.removePage is called, hence

src/browser/webapi/net/Fetch.zig

@@ -78,6 +78,7 @@ pub fn init(input: Input, options: ?InitOpts, page: *Page) !js.Promise {
         .headers = headers,
         .resource_type = .fetch,
         .cookie_jar = &page._session.cookie_jar,
+        .notification = page._session.notification,
         .start_callback = httpStartCallback,
         .header_callback = httpHeaderDoneCallback,
         .data_callback = httpDataCallback,

src/browser/webapi/net/XMLHttpRequest.zig

@@ -209,6 +209,7 @@ pub fn send(self: *XMLHttpRequest, body_: ?[]const u8) !void {
         .body = self._request_body,
         .cookie_jar = &page._session.cookie_jar,
         .resource_type = .xhr,
+        .notification = page._session.notification,
         .start_callback = httpStartCallback,
         .header_callback = httpHeaderDoneCallback,
         .data_callback = httpDataCallback,

src/cdp/cdp.zig

@@ -389,12 +389,18 @@ pub fn BrowserContext(comptime CDP_T: type) type {
         // to store the,
         captured_responses: std.AutoHashMapUnmanaged(usize, std.ArrayList(u8)),
 
+        notification: *Notification,
+
         const Self = @This();
 
         fn init(self: *Self, id: []const u8, cdp: *CDP_T) !void {
             const allocator = cdp.allocator;
 
-            const session = try cdp.browser.newSession();
+            // Create notification for this BrowserContext
+            const notification = try Notification.init(allocator);
+            errdefer notification.deinit();
+
+            const session = try cdp.browser.newSession(notification);
 
             const browser = &cdp.browser;
             const inspector_session = browser.env.inspector.?.startSession(self);
@@ -423,14 +429,15 @@ pub fn BrowserContext(comptime CDP_T: type) type {
                 .intercept_state = try InterceptState.init(allocator),
                 .captured_responses = .empty,
                 .log_interceptor = LogInterceptor(Self).init(allocator, self),
+                .notification = notification,
             };
             self.node_search_list = Node.Search.List.init(allocator, &self.node_registry);
             errdefer self.deinit();
 
-            try browser.notification.register(.page_remove, self, onPageRemove);
+            try notification.register(.page_remove, self, onPageRemove);
-            try browser.notification.register(.page_created, self, onPageCreated);
+            try notification.register(.page_created, self, onPageCreated);
-            try browser.notification.register(.page_navigate, self, onPageNavigate);
+            try notification.register(.page_navigate, self, onPageNavigate);
-            try browser.notification.register(.page_navigated, self, onPageNavigated);
+            try notification.register(.page_navigated, self, onPageNavigated);
         }
 
         pub fn deinit(self: *Self) void {
@@ -468,7 +475,8 @@ pub fn BrowserContext(comptime CDP_T: type) type {
 
             self.node_registry.deinit();
             self.node_search_list.deinit();
-            browser.notification.unregisterAll(self);
+            self.notification.unregisterAll(self);
+            self.notification.deinit();
 
             if (self.http_proxy_changed) {
                 // has to be called after browser.closeSession, since it won't
@@ -538,43 +546,43 @@ pub fn BrowserContext(comptime CDP_T: type) type {
         }
 
         pub fn networkEnable(self: *Self) !void {
-            try self.cdp.browser.notification.register(.http_request_fail, self, onHttpRequestFail);
+            try self.notification.register(.http_request_fail, self, onHttpRequestFail);
-            try self.cdp.browser.notification.register(.http_request_start, self, onHttpRequestStart);
+            try self.notification.register(.http_request_start, self, onHttpRequestStart);
-            try self.cdp.browser.notification.register(.http_request_done, self, onHttpRequestDone);
+            try self.notification.register(.http_request_done, self, onHttpRequestDone);
-            try self.cdp.browser.notification.register(.http_response_data, self, onHttpResponseData);
+            try self.notification.register(.http_response_data, self, onHttpResponseData);
-            try self.cdp.browser.notification.register(.http_response_header_done, self, onHttpResponseHeadersDone);
+            try self.notification.register(.http_response_header_done, self, onHttpResponseHeadersDone);
         }
 
         pub fn networkDisable(self: *Self) void {
-            self.cdp.browser.notification.unregister(.http_request_fail, self);
+            self.notification.unregister(.http_request_fail, self);
-            self.cdp.browser.notification.unregister(.http_request_start, self);
+            self.notification.unregister(.http_request_start, self);
-            self.cdp.browser.notification.unregister(.http_request_done, self);
+            self.notification.unregister(.http_request_done, self);
-            self.cdp.browser.notification.unregister(.http_response_data, self);
+            self.notification.unregister(.http_response_data, self);
-            self.cdp.browser.notification.unregister(.http_response_header_done, self);
+            self.notification.unregister(.http_response_header_done, self);
         }
 
         pub fn fetchEnable(self: *Self, authRequests: bool) !void {
-            try self.cdp.browser.notification.register(.http_request_intercept, self, onHttpRequestIntercept);
+            try self.notification.register(.http_request_intercept, self, onHttpRequestIntercept);
             if (authRequests) {
-                try self.cdp.browser.notification.register(.http_request_auth_required, self, onHttpRequestAuthRequired);
+                try self.notification.register(.http_request_auth_required, self, onHttpRequestAuthRequired);
             }
         }
 
         pub fn fetchDisable(self: *Self) void {
-            self.cdp.browser.notification.unregister(.http_request_intercept, self);
+            self.notification.unregister(.http_request_intercept, self);
-            self.cdp.browser.notification.unregister(.http_request_auth_required, self);
+            self.notification.unregister(.http_request_auth_required, self);
         }
 
         pub fn lifecycleEventsEnable(self: *Self) !void {
             self.page_life_cycle_events = true;
-            try self.cdp.browser.notification.register(.page_network_idle, self, onPageNetworkIdle);
+            try self.notification.register(.page_network_idle, self, onPageNetworkIdle);
-            try self.cdp.browser.notification.register(.page_network_almost_idle, self, onPageNetworkAlmostIdle);
+            try self.notification.register(.page_network_almost_idle, self, onPageNetworkAlmostIdle);
         }
 
         pub fn lifecycleEventsDisable(self: *Self) void {
             self.page_life_cycle_events = false;
-            self.cdp.browser.notification.unregister(.page_network_idle, self);
+            self.notification.unregister(.page_network_idle, self);
-            self.cdp.browser.notification.unregister(.page_network_almost_idle, self);
+            self.notification.unregister(.page_network_almost_idle, self);
         }
 
         pub fn logEnable(self: *Self) void {

src/http/Client.zig

@@ -87,9 +87,6 @@ allocator: Allocator,
 // request. These wil come and go with each request.
 transfer_pool: std.heap.MemoryPool(Transfer),
 
-// To notify registered subscribers of events, the browser sets/nulls this for us.
-notification: ?*Notification = null,
-
 // only needed for CDP which can change the proxy and then restore it. When
 // restoring, this originally-configured value is what it goes to.
 http_proxy: ?[:0]const u8 = null,
@@ -218,12 +215,10 @@ pub fn tick(self: *Client, timeout_ms: u32) !PerformStatus {
 pub fn request(self: *Client, req: Request) !void {
     const transfer = try self.makeTransfer(req);
 
-    const notification = self.notification orelse return self.process(transfer);
+    transfer.req.notification.dispatch(.http_request_start, &.{ .transfer = transfer });
-
-    notification.dispatch(.http_request_start, &.{ .transfer = transfer });
 
     var wait_for_interception = false;
-    notification.dispatch(.http_request_intercept, &.{ .transfer = transfer, .wait_for_interception = &wait_for_interception });
+    transfer.req.notification.dispatch(.http_request_intercept, &.{ .transfer = transfer, .wait_for_interception = &wait_for_interception });
     if (wait_for_interception == false) {
         // request not intercepted, process it normally
         return self.process(transfer);
@@ -371,7 +366,7 @@ fn makeTransfer(self: *Client, req: Request) !*Transfer {
     return transfer;
 }
 
-fn requestFailed(self: *Client, transfer: *Transfer, err: anyerror, comptime execute_callback: bool) void {
+fn requestFailed(transfer: *Transfer, err: anyerror, comptime execute_callback: bool) void {
     // this shouldn't happen, we'll crash in debug mode. But in release, we'll
     // just noop this state.
     if (comptime IS_DEBUG) {
@@ -383,12 +378,10 @@ fn requestFailed(self: *Client, transfer: *Transfer, err: anyerror, comptime exe
 
     transfer._notified_fail = true;
 
-    if (self.notification) |notification| {
+    transfer.req.notification.dispatch(.http_request_fail, &.{
-        notification.dispatch(.http_request_fail, &.{
+        .transfer = transfer,
-            .transfer = transfer,
+        .err = err,
-            .err = err,
+    });
-        });
-    }
 
     if (execute_callback) {
         transfer.req.error_callback(transfer.ctx, err);
@@ -569,26 +562,24 @@ fn processMessages(self: *Client) !bool {
         // In case of auth challenge
         // TODO give a way to configure the number of auth retries.
         if (transfer._auth_challenge != null and transfer._tries < 10) {
-            if (transfer.client.notification) |notification| {
+            var wait_for_interception = false;
-                var wait_for_interception = false;
+            transfer.req.notification.dispatch(.http_request_auth_required, &.{ .transfer = transfer, .wait_for_interception = &wait_for_interception });
-                notification.dispatch(.http_request_auth_required, &.{ .transfer = transfer, .wait_for_interception = &wait_for_interception });
+            if (wait_for_interception) {
-                if (wait_for_interception) {
+                self.intercepted += 1;
-                    self.intercepted += 1;
+                if (comptime IS_DEBUG) {
-                    if (comptime IS_DEBUG) {
+                    log.debug(.http, "wait for auth interception", .{ .intercepted = self.intercepted });
-                        log.debug(.http, "wait for auth interception", .{ .intercepted = self.intercepted });
-                    }
-                    transfer._intercept_state = .pending;
-                    if (!transfer.req.blocking) {
-                        // the request is put on hold to be intercepted.
-                        // In this case we ignore callbacks for now.
-                        // Note: we don't deinit transfer on purpose: we want to keep
-                        // using it for the following request
-                        self.endTransfer(transfer);
-                        continue;
-                    }
-
-                    _ = try self.waitForInterceptedResponse(transfer);
                 }
+                transfer._intercept_state = .pending;
+                if (!transfer.req.blocking) {
+                    // the request is put on hold to be intercepted.
+                    // In this case we ignore callbacks for now.
+                    // Note: we don't deinit transfer on purpose: we want to keep
+                    // using it for the following request
+                    self.endTransfer(transfer);
+                    continue;
+                }
+
+                _ = try self.waitForInterceptedResponse(transfer);
             }
         }
 
@@ -604,29 +595,27 @@ fn processMessages(self: *Client) !bool {
             if (!transfer._header_done_called) {
                 const proceed = transfer.headerDoneCallback(easy) catch |err| {
                     log.err(.http, "header_done_callback", .{ .err = err });
-                    self.requestFailed(transfer, err, true);
+                    requestFailed(transfer, err, true);
                     continue;
                 };
                 if (!proceed) {
-                    self.requestFailed(transfer, error.Abort, true);
+                    requestFailed(transfer, error.Abort, true);
                     break :blk;
                 }
             }
             transfer.req.done_callback(transfer.ctx) catch |err| {
                 // transfer isn't valid at this point, don't use it.
                 log.err(.http, "done_callback", .{ .err = err });
-                self.requestFailed(transfer, err, true);
+                requestFailed(transfer, err, true);
                 continue;
             };
 
-            if (transfer.client.notification) |notification| {
+            transfer.req.notification.dispatch(.http_request_done, &.{
-                notification.dispatch(.http_request_done, &.{
+                .transfer = transfer,
-                    .transfer = transfer,
+            });
-                });
-            }
             processed = true;
         } else |err| {
-            self.requestFailed(transfer, err, true);
+            requestFailed(transfer, err, true);
         }
     }
     return processed;
@@ -767,6 +756,7 @@ pub const Request = struct {
     cookie_jar: *CookieJar,
     resource_type: ResourceType,
     credentials: ?[:0]const u8 = null,
+    notification: *Notification,
 
     // This is only relevant for intercepted requests. If a request is flagged
     // as blocking AND is intercepted, then it'll be up to us to wait until
@@ -977,7 +967,7 @@ pub const Transfer = struct {
     }
 
     pub fn abort(self: *Transfer, err: anyerror) void {
-        self.client.requestFailed(self, err, true);
+        requestFailed(self, err, true);
         if (self._handle != null) {
             self.client.endTransfer(self);
         }
@@ -985,7 +975,7 @@ pub const Transfer = struct {
     }
 
     pub fn terminate(self: *Transfer) void {
-        self.client.requestFailed(self, error.Shutdown, false);
+        requestFailed(self, error.Shutdown, false);
         if (self._handle != null) {
             self.client.endTransfer(self);
         }
@@ -1095,11 +1085,9 @@ pub const Transfer = struct {
             return err;
         };
 
-        if (transfer.client.notification) |notification| {
+        transfer.req.notification.dispatch(.http_response_header_done, &.{
-            notification.dispatch(.http_response_header_done, &.{
+            .transfer = transfer,
-                .transfer = transfer,
+        });
-            });
-        }
 
         return proceed;
     }
@@ -1249,12 +1237,10 @@ pub const Transfer = struct {
             return c.CURL_WRITEFUNC_ERROR;
         };
 
-        if (transfer.client.notification) |notification| {
+        transfer.req.notification.dispatch(.http_response_data, &.{
-            notification.dispatch(.http_response_data, &.{
+            .data = chunk,
-                .data = chunk,
+            .transfer = transfer,
-                .transfer = transfer,
+        });
-            });
-        }
 
         return @intCast(chunk_len);
     }

src/lightpanda.zig

@@ -22,6 +22,7 @@ pub const Server = @import("Server.zig");
 pub const Page = @import("browser/Page.zig");
 pub const Browser = @import("browser/Browser.zig");
 pub const Session = @import("browser/Session.zig");
+pub const Notification = @import("Notification.zig");
 
 pub const log = @import("log.zig");
 pub const js = @import("browser/js/js.zig");
@@ -38,9 +39,11 @@ pub const FetchOpts = struct {
 };
 pub fn fetch(app: *App, url: [:0]const u8, opts: FetchOpts) !void {
     var browser = try Browser.init(app, .{});
+    const notification = try Notification.init(app.allocator);
+    defer notification.deinit();
     defer browser.deinit();
 
-    var session = try browser.newSession();
+    var session = try browser.newSession(notification);
     const page = try session.createPage();
 
     // // Comment this out to get a profile of the JS code in v8/profile.json.

src/main_legacy_test.zig

@@ -44,9 +44,11 @@ pub fn main() !void {
     defer test_arena.deinit();
 
     var browser = try lp.Browser.init(app, .{});
+    const notification = try lp.Notification.init(app.allocator);
+    defer notification.deinit();
     defer browser.deinit();
 
-    const session = try browser.newSession();
+    const session = try browser.newSession(notification);
 
     var dir = try std.fs.cwd().openDir("src/browser/tests/legacy/", .{ .iterate = true, .no_follow = true });
     defer dir.close();

src/main_wpt.zig

@@ -107,7 +107,10 @@ fn run(
     test_file: []const u8,
     err_out: *?[]const u8,
 ) ![]const u8 {
-    const session = try browser.newSession();
+    const notification = try lp.Notification.init(browser.allocator);
+    defer notification.deinit();
+
+    const session = try browser.newSession(notification);
     defer browser.closeSession();
 
     const page = try session.createPage();

src/telemetry/telemetry.zig

@@ -5,7 +5,6 @@ const Allocator = std.mem.Allocator;
 
 const log = @import("../log.zig");
 const App = @import("../App.zig");
-const Notification = @import("../Notification.zig");
 
 const uuidv4 = @import("../id.zig").uuidv4;
 const IID_FILE = "iid";
@@ -63,32 +62,6 @@ fn TelemetryT(comptime P: type) type {
                 log.warn(.telemetry, "record error", .{ .err = err, .type = @tagName(std.meta.activeTag(event)) });
             };
         }
-
-        // Called outside of `init` because we need a stable pointer for self.
-        // We care page_navigate events, but those happen on a Browser's
-        // notification. This doesn't exist yet, and there isn't only going to
-        // be 1, browsers come and go.
-        // What we can do is register for the `notification_created` event.
-        // In the callback for that, `onNotificationCreated`, we can then register
-        // for the browser-events that we care about.
-        pub fn register(self: *Self, notification: *Notification) !void {
-            if (self.disabled) {
-                return;
-            }
-            try notification.register(.notification_created, self, onNotificationCreated);
-        }
-
-        fn onNotificationCreated(ctx: *anyopaque, new: *Notification) !void {
-            return new.register(.page_navigate, ctx, onPageNavigate);
-        }
-
-        fn onPageNavigate(ctx: *anyopaque, data: *const Notification.PageNavigate) !void {
-            const self: *Self = @ptrCast(@alignCast(ctx));
-            self.record(.{ .navigate = .{
-                .proxy = false,
-                .tls = std.ascii.startsWithIgnoreCase(data.url, "https://"),
-            } });
-        }
     };
 }
 

src/testing.zig

@@ -40,6 +40,7 @@ const App = @import("App.zig");
 const js = @import("browser/js/js.zig");
 const Browser = @import("browser/Browser.zig");
 const Session = @import("browser/Session.zig");
+const Notification = @import("Notification.zig");
 const Page = @import("browser/Page.zig");
 
 // Merged std.testing.expectEqual and std.testing.expectString
@@ -333,6 +334,7 @@ fn isJsonValue(a: std.json.Value, b: std.json.Value) bool {
 
 pub var test_app: *App = undefined;
 pub var test_browser: Browser = undefined;
+pub var test_notification: *Notification = undefined;
 pub var test_session: *Session = undefined;
 
 const WEB_API_TEST_ROOT = "src/browser/tests/";
@@ -463,7 +465,11 @@ test "tests:beforeAll" {
     test_browser = try Browser.init(test_app, .{});
     errdefer test_browser.deinit();
 
-    test_session = try test_browser.newSession();
+    // Create notification for testing
+    test_notification = try Notification.init(test_app.allocator);
+    errdefer test_notification.deinit();
+
+    test_session = try test_browser.newSession(test_notification);
 
     var wg: std.Thread.WaitGroup = .{};
     wg.startMany(2);
@@ -494,6 +500,7 @@ test "tests:afterAll" {
 
     @import("root").v8_peak_memory = test_browser.env.isolate.getHeapStatistics().total_physical_size;
 
+    test_notification.deinit();
     test_browser.deinit();
     test_app.deinit();
 }