From dbf18b90a79916ce25c76fc5b4995607f1c4701a Mon Sep 17 00:00:00 2001 From: Nikolay Govorov Date: Tue, 3 Feb 2026 16:43:25 +0000 Subject: [PATCH 1/2] Removes telemetry dependence on notifications --- src/App.zig | 4 +--- src/Notification.zig | 34 +++++++++------------------------- src/browser/Browser.zig | 2 +- src/browser/Page.zig | 12 ++++++++++++ src/telemetry/telemetry.zig | 27 --------------------------- 5 files changed, 23 insertions(+), 56 deletions(-) diff --git a/src/App.zig b/src/App.zig index fca528cd..0a18457d 100644 --- a/src/App.zig +++ b/src/App.zig @@ -70,7 +70,7 @@ pub fn init(allocator: Allocator, config: Config) !*App { app.config = config; app.allocator = allocator; - app.notification = try Notification.init(allocator, null); + app.notification = try Notification.init(allocator); errdefer app.notification.deinit(); app.http = try Http.init(allocator, .{ @@ -96,8 +96,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(); diff --git a/src/Notification.zig b/src/Notification.zig index 008c0e08..bc11f38a 100644 --- a/src/Notification.zig +++ b/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 -// because everything's just one big global, that gets picked up by the -// registered CDP listener, and the telemetry network activity gets sent to the -// CDP client. +// That is, it would work until multiple CDP clients connect, and because +// everything's just one big global, events from one CDP session would be sent +// to all CDP clients. // // 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 -// notification instances at a given time: one in a Browser and one in the App. -// What about something like Telemetry, which lives outside of a Browser but -// still cares about Browser-events (like .page_navigate)? When the Browser -// 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. +// CDP connection (BrowserContext). Each CDP connection has its own notification +// that is shared across all Sessions (tabs) within that connection. This ensures +// proper isolation between different CDP clients while allowing a single client +// to receive events from all its tabs. 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 { - - // 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. +pub fn init(allocator: Allocator) !*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; } @@ -313,7 +297,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 diff --git a/src/browser/Browser.zig b/src/browser/Browser.zig index 408af7e4..b560e3f5 100644 --- a/src/browser/Browser.zig +++ b/src/browser/Browser.zig @@ -60,7 +60,7 @@ 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); + const notification = try Notification.init(allocator); app.http.client.notification = notification; app.http.client.next_request_id = 0; // Should we track ids in CDP only? errdefer notification.deinit(); diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 37c87605..932eae33 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -485,6 +485,12 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi .timestamp = timestamp(.monotonic), }); + // 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.browser.notification.dispatch(.page_navigated, &.{ .req_id = req_id, .opts = .{ @@ -527,6 +533,12 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi .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(.{ diff --git a/src/telemetry/telemetry.zig b/src/telemetry/telemetry.zig index cffa8768..ff60ac1f 100644 --- a/src/telemetry/telemetry.zig +++ b/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://"), - } }); - } }; } From fd8c488dbdf5aab277a3cb896e6a645335542ea4 Mon Sep 17 00:00:00 2001 From: Nikolay Govorov Date: Tue, 3 Feb 2026 17:02:09 +0000 Subject: [PATCH 2/2] Move Notification from App to BrowserContext --- src/App.zig | 6 -- src/Notification.zig | 3 + src/browser/Browser.zig | 15 +--- src/browser/Page.zig | 23 +++--- src/browser/ScriptManager.zig | 3 + src/browser/Session.zig | 22 ++++-- src/browser/webapi/net/Fetch.zig | 1 + src/browser/webapi/net/XMLHttpRequest.zig | 1 + src/cdp/cdp.zig | 56 +++++++------ src/http/Client.zig | 96 ++++++++++------------- src/lightpanda.zig | 5 +- src/main_legacy_test.zig | 4 +- src/main_wpt.zig | 5 +- src/testing.zig | 9 ++- 14 files changed, 130 insertions(+), 119 deletions(-) diff --git a/src/App.zig b/src/App.zig index 0a18457d..41ff0f30 100644 --- a/src/App.zig +++ b/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); - 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, @@ -113,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(); diff --git a/src/Notification.zig b/src/Notification.zig index bc11f38a..0dc867c7 100644 --- a/src/Notification.zig +++ b/src/Notification.zig @@ -240,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; diff --git a/src/browser/Browser.zig b/src/browser/Browser.zig index b560e3f5..54668987 100644 --- a/src/browser/Browser.zig +++ b/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.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; } diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 932eae33..c0469fa0 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -478,7 +478,7 @@ 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, @@ -486,12 +486,14 @@ pub fn navigate(self: *Page, request_url: [:0]const u8, opts: NavigateOpts) !voi }); // 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.browser.app.telemetry.record(.{ + .navigate = .{ + .tls = false, // about:blank is not TLS + .proxy = self._session.browser.app.config.http_proxy != null, + }, + }); - self._session.browser.notification.dispatch(.page_navigated, &.{ + self._session.notification.dispatch(.page_navigated, &.{ .req_id = req_id, .opts = .{ .cdp_id = opts.cdp_id, @@ -526,7 +528,7 @@ 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, @@ -549,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, @@ -676,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, @@ -1425,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), }); } diff --git a/src/browser/ScriptManager.zig b/src/browser/ScriptManager.zig index 95c6150e..6f55d12a 100644 --- a/src/browser/ScriptManager.zig +++ b/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, diff --git a/src/browser/Session.zig b/src/browser/Session.zig index 3e426b97..53702830 100644 --- a/src/browser/Session.zig +++ b/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); - switch (page.wait(wait_ms)) { - .navigate => self.processScheduledNavigation() catch return .done, - else => |result| return result, + if (self.page) |*page| { + switch (page.wait(wait_ms)) { + .navigate => self.processScheduledNavigation() catch return .done, + 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 diff --git a/src/browser/webapi/net/Fetch.zig b/src/browser/webapi/net/Fetch.zig index 78d77016..a66fb311 100644 --- a/src/browser/webapi/net/Fetch.zig +++ b/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, diff --git a/src/browser/webapi/net/XMLHttpRequest.zig b/src/browser/webapi/net/XMLHttpRequest.zig index cb9f9a3a..7c266e1a 100644 --- a/src/browser/webapi/net/XMLHttpRequest.zig +++ b/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, diff --git a/src/cdp/cdp.zig b/src/cdp/cdp.zig index 77e7071c..99310255 100644 --- a/src/cdp/cdp.zig +++ b/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 browser.notification.register(.page_created, self, onPageCreated); - try browser.notification.register(.page_navigate, self, onPageNavigate); - try browser.notification.register(.page_navigated, self, onPageNavigated); + try notification.register(.page_remove, self, onPageRemove); + try notification.register(.page_created, self, onPageCreated); + try notification.register(.page_navigate, self, onPageNavigate); + 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.cdp.browser.notification.register(.http_request_start, self, onHttpRequestStart); - try self.cdp.browser.notification.register(.http_request_done, self, onHttpRequestDone); - try self.cdp.browser.notification.register(.http_response_data, self, onHttpResponseData); - try self.cdp.browser.notification.register(.http_response_header_done, self, onHttpResponseHeadersDone); + try self.notification.register(.http_request_fail, self, onHttpRequestFail); + try self.notification.register(.http_request_start, self, onHttpRequestStart); + try self.notification.register(.http_request_done, self, onHttpRequestDone); + try self.notification.register(.http_response_data, self, onHttpResponseData); + 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.cdp.browser.notification.unregister(.http_request_start, self); - self.cdp.browser.notification.unregister(.http_request_done, self); - self.cdp.browser.notification.unregister(.http_response_data, self); - self.cdp.browser.notification.unregister(.http_response_header_done, self); + self.notification.unregister(.http_request_fail, self); + self.notification.unregister(.http_request_start, self); + self.notification.unregister(.http_request_done, self); + self.notification.unregister(.http_response_data, 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.cdp.browser.notification.unregister(.http_request_auth_required, self); + self.notification.unregister(.http_request_intercept, 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.cdp.browser.notification.register(.page_network_almost_idle, self, onPageNetworkAlmostIdle); + try self.notification.register(.page_network_idle, self, onPageNetworkIdle); + 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.cdp.browser.notification.unregister(.page_network_almost_idle, self); + self.notification.unregister(.page_network_idle, self); + self.notification.unregister(.page_network_almost_idle, self); } pub fn logEnable(self: *Self) void { diff --git a/src/http/Client.zig b/src/http/Client.zig index 9d497597..97103d07 100644 --- a/src/http/Client.zig +++ b/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); - - notification.dispatch(.http_request_start, &.{ .transfer = transfer }); + transfer.req.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| { - notification.dispatch(.http_request_fail, &.{ - .transfer = transfer, - .err = err, - }); - } + transfer.req.notification.dispatch(.http_request_fail, &.{ + .transfer = transfer, + .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; - notification.dispatch(.http_request_auth_required, &.{ .transfer = transfer, .wait_for_interception = &wait_for_interception }); - if (wait_for_interception) { - self.intercepted += 1; - if (comptime IS_DEBUG) { - 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); + var wait_for_interception = false; + transfer.req.notification.dispatch(.http_request_auth_required, &.{ .transfer = transfer, .wait_for_interception = &wait_for_interception }); + if (wait_for_interception) { + self.intercepted += 1; + if (comptime IS_DEBUG) { + 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); } } @@ -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| { - notification.dispatch(.http_request_done, &.{ - .transfer = transfer, - }); - } + transfer.req.notification.dispatch(.http_request_done, &.{ + .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| { - notification.dispatch(.http_response_header_done, &.{ - .transfer = transfer, - }); - } + transfer.req.notification.dispatch(.http_response_header_done, &.{ + .transfer = transfer, + }); return proceed; } @@ -1249,12 +1237,10 @@ pub const Transfer = struct { return c.CURL_WRITEFUNC_ERROR; }; - if (transfer.client.notification) |notification| { - notification.dispatch(.http_response_data, &.{ - .data = chunk, - .transfer = transfer, - }); - } + transfer.req.notification.dispatch(.http_response_data, &.{ + .data = chunk, + .transfer = transfer, + }); return @intCast(chunk_len); } diff --git a/src/lightpanda.zig b/src/lightpanda.zig index f016b76d..d7c1e3ad 100644 --- a/src/lightpanda.zig +++ b/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. diff --git a/src/main_legacy_test.zig b/src/main_legacy_test.zig index beaacef3..7fc2448c 100644 --- a/src/main_legacy_test.zig +++ b/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(); diff --git a/src/main_wpt.zig b/src/main_wpt.zig index 45a6e26f..ee2ee2ac 100644 --- a/src/main_wpt.zig +++ b/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(); diff --git a/src/testing.zig b/src/testing.zig index 8520e683..b164d52f 100644 --- a/src/testing.zig +++ b/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(); }