From 2b945aa1f9463cef0edaf99a6964a430b3730489 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Fri, 24 Jul 2026 08:20:32 +0800 Subject: [PATCH 1/3] cdp: ignore duplicate enable calls CDP driver can send multiple Network.enable which would register the same listener multiple times. This commit makes it so that only one (the first) callback registered for a listener+eventtype is used. Subsequent registration for the same listener+eventtype are ignored. This is safe because all callbacks are currently static. It's a mistake (enforced by a debug-only assertion) for code to try to register a different callback for an already registered listener+ eventtype. This generalizes https://github.com/lightpanda-io/browser/pull/3038 --- src/Notification.zig | 20 +++++++++++++------- src/cdp/CDP.zig | 13 +------------ 2 files changed, 14 insertions(+), 19 deletions(-) diff --git a/src/Notification.zig b/src/Notification.zig index 55dcc4c1b..e8830f9a7 100644 --- a/src/Notification.zig +++ b/src/Notification.zig @@ -392,8 +392,20 @@ pub fn deinit(self: *Notification) void { } pub fn register(self: *Notification, comptime event: EventType, receiver: anytype, func: EventFunc(event)) !void { - var list = &@field(self.event_listeners, @tagName(event)); + const allocator = self.allocator; + const gop = try self.listeners.getOrPut(allocator, @intFromPtr(receiver)); + if (gop.found_existing) { + for (gop.value_ptr.items) |existing| { + if (existing.event == event) { + lp.assert(@as(*const anyopaque, @ptrCast(func)) == existing.func, "different notification callbacks per receiver", .{.event = event}); + return; + } + } + } else { + gop.value_ptr.* = .empty; + } + var list = &@field(self.event_listeners, @tagName(event)); var listener = try self.mem_pool.create(); errdefer self.mem_pool.destroy(listener); @@ -405,12 +417,6 @@ pub fn register(self: *Notification, comptime event: EventType, receiver: anytyp .func = @ptrCast(func), .struct_name = @typeName(@typeInfo(@TypeOf(receiver)).pointer.child), }; - - const allocator = self.allocator; - const gop = try self.listeners.getOrPut(allocator, @intFromPtr(receiver)); - if (gop.found_existing == false) { - gop.value_ptr.* = .empty; - } try gop.value_ptr.append(allocator, listener); // we don't add this until we've successfully added the entry to diff --git a/src/cdp/CDP.zig b/src/cdp/CDP.zig index 7cf27b991..bac8a64a6 100644 --- a/src/cdp/CDP.zig +++ b/src/cdp/CDP.zig @@ -569,10 +569,6 @@ pub const BrowserContext = struct { http_proxy_changed: bool = false, user_agent_changed: bool = false, - // True once we've registered for the download notifications, so repeated - // Browser.setDownloadBehavior calls don't add duplicate listeners. - download_events_registered: bool = false, - // Extra headers to add to all requests. extra_headers: std.ArrayList([*c]const u8) = .empty, @@ -826,6 +822,7 @@ pub const BrowserContext = struct { } pub fn fetchEnable(self: *BrowserContext, authRequests: bool) !void { + self.fetchDisable(); //in case of multiple calls try self.notification.register(.http_request_intercept, self, onHttpRequestIntercept); if (authRequests) { try self.notification.register(.http_request_auth_required, self, onHttpRequestAuthRequired); @@ -868,21 +865,13 @@ pub const BrowserContext = struct { // Registers for the download notifications dispatched by Frame when a // navigation is treated as a file download. Idempotent. See issue #2701. pub fn downloadEventsEnable(self: *BrowserContext) !void { - if (self.download_events_registered) { - return; - } try self.notification.register(.download_will_begin, self, onDownloadWillBegin); try self.notification.register(.download_progress, self, onDownloadProgress); - self.download_events_registered = true; } pub fn downloadEventsDisable(self: *BrowserContext) void { - if (self.download_events_registered == false) { - return; - } self.notification.unregister(.download_will_begin, self); self.notification.unregister(.download_progress, self); - self.download_events_registered = false; } pub fn onDownloadWillBegin(ctx: *anyopaque, msg: *const Notification.DownloadWillBegin) !void { From a6af331ab2bdc17eb333d40fb79c82ba5ff016aa Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Fri, 24 Jul 2026 08:25:21 +0800 Subject: [PATCH 2/3] zig fmt --- src/Notification.zig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Notification.zig b/src/Notification.zig index e8830f9a7..74fd5af3f 100644 --- a/src/Notification.zig +++ b/src/Notification.zig @@ -397,7 +397,7 @@ pub fn register(self: *Notification, comptime event: EventType, receiver: anytyp if (gop.found_existing) { for (gop.value_ptr.items) |existing| { if (existing.event == event) { - lp.assert(@as(*const anyopaque, @ptrCast(func)) == existing.func, "different notification callbacks per receiver", .{.event = event}); + lp.assert(@as(*const anyopaque, @ptrCast(func)) == existing.func, "different notification callbacks per receiver", .{ .event = event }); return; } } From ed9a25f470b2c7f66706721a4a5dc28ad8506b48 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Fri, 24 Jul 2026 10:18:59 +0800 Subject: [PATCH 3/3] fix test --- src/cdp/domains/browser.zig | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/cdp/domains/browser.zig b/src/cdp/domains/browser.zig index b690f8301..c05a4a5a8 100644 --- a/src/cdp/domains/browser.zig +++ b/src/cdp/domains/browser.zig @@ -320,7 +320,6 @@ test "cdp.browser: setDownloadBehavior stores config on the session" { try testing.expectEqual(.allow_and_name, bc.session.download_behavior); try testing.expectEqualSlices(u8, "/tmp/lp-downloads", bc.session.download_path.?); try testing.expect(bc.session.download_events_enabled); - try testing.expect(bc.download_events_registered); // `default` maps to `deny` and tears the registration down again. try ctx.processMessage(.{ @@ -331,7 +330,6 @@ test "cdp.browser: setDownloadBehavior stores config on the session" { try testing.expectEqual(.deny, bc.session.download_behavior); try testing.expect(bc.session.download_path == null); try testing.expect(bc.session.download_events_enabled == false); - try testing.expect(bc.download_events_registered == false); } test "cdp.browser: setDownloadBehavior is a no-op when no context is loaded" {