From a0fc38eb6db628bc08a6f5a8657f3b5d6dd272b0 Mon Sep 17 00:00:00 2001 From: Rohit <71192000+rohitsux@users.noreply.github.com> Date: Wed, 17 Jun 2026 01:02:30 +0530 Subject: [PATCH] fix(cdp): store permissions at the browser level Address review: permissions set via Browser.grantPermissions / setPermission now live on the Browser instead of the active Page, so they persist across page navigations (previously a navigation dropped them) and match how Chrome scopes permissions to the browser context. navigator.permissions.query() reads the browser-level state. Also log not_implemented for the origin and browserContextId params, which are accepted but not yet honored. --- src/browser/Browser.zig | 34 +++++++++++++++++++ src/browser/Page.zig | 18 ----------- src/browser/webapi/Permissions.zig | 2 +- src/cdp/domains/browser.zig | 52 +++++++++++++++++++----------- 4 files changed, 68 insertions(+), 38 deletions(-) diff --git a/src/browser/Browser.zig b/src/browser/Browser.zig index c52ddadbd..1f08a08bf 100644 --- a/src/browser/Browser.zig +++ b/src/browser/Browser.zig @@ -26,6 +26,7 @@ const js = @import("js/js.zig"); const Page = @import("Page.zig"); const Session = @import("Session.zig"); const HttpClient = @import("HttpClient.zig"); +const PermissionState = @import("webapi/Permissions.zig").State; const ArenaPool = App.ArenaPool; const Allocator = std.mem.Allocator; @@ -42,6 +43,13 @@ allocator: Allocator, arena_pool: *ArenaPool, http_client: HttpClient, +// Permission state set via CDP Browser.grantPermissions / setPermission / +// resetPermissions, keyed by permission name (e.g. "geolocation"). Read back +// by navigator.permissions.query(). Scoped to the Browser so it persists +// across page navigations, mirroring how Chrome scopes permissions to the +// browser context. Keys are owned by `allocator`; values are enum tags. +permissions: std.StringHashMapUnmanaged(PermissionState) = .empty, + // used by sessions to allocate pages. page_pool: std.heap.MemoryPool(Page), @@ -104,6 +112,32 @@ pub fn deinit(self: *Browser) void { self.fc_identity_pool.deinit(); self.page_pool.deinit(); self.http_client.deinit(); + self.clearPermissions(); + self.permissions.deinit(self.allocator); +} + +// Set (or overwrite) the stored state for a permission. The name is duped into +// `allocator`; the state is a plain enum tag. Used by CDP +// Browser.grantPermissions / setPermission. +pub fn setPermission(self: *Browser, name: []const u8, state: PermissionState) !void { + const gop = try self.permissions.getOrPut(self.allocator, name); + if (!gop.found_existing) { + gop.key_ptr.* = self.allocator.dupe(u8, name) catch |err| { + _ = self.permissions.remove(name); + return err; + }; + } + gop.value_ptr.* = state; +} + +// Clear all stored permissions, freeing the keys. Used by CDP +// Browser.resetPermissions and on teardown. +pub fn clearPermissions(self: *Browser) void { + var it = self.permissions.keyIterator(); + while (it.next()) |key| { + self.allocator.free(key.*); + } + self.permissions.clearRetainingCapacity(); } pub fn newSession(self: *Browser, notification: *Notification) !*Session { diff --git a/src/browser/Page.zig b/src/browser/Page.zig index f785d78fb..76e739b76 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -25,7 +25,6 @@ const v8 = js.v8; const Frame = @import("Frame.zig"); const Session = @import("Session.zig"); const Factory = @import("Factory.zig"); -const PermissionState = @import("webapi/Permissions.zig").State; const Allocator = std.mem.Allocator; const IS_DEBUG = builtin.mode == .Debug; @@ -71,11 +70,6 @@ frame_arena: Allocator, // lifetime. origins: std.StringHashMapUnmanaged(*js.Origin) = .empty, -// Permission state set via CDP Browser.grantPermissions / setPermission / -// resetPermissions, keyed by permission name (e.g. "geolocation"). Read back -// by navigator.permissions.query(). -permissions: std.StringHashMapUnmanaged(PermissionState) = .empty, - // Identity tracking for the main world. All main-world contexts in this Page // share this, ensuring object identity works across same-origin frames. identity: js.Identity = .{}, @@ -193,10 +187,6 @@ pub fn deinit(self: *Page) void { self.origins = .empty; } - // Keys and values are duped into frame_arena, released just below; drop - // the map so we don't keep a dangling reference. - self.permissions = .empty; - session.arena_pool.release(self.frame_arena); } @@ -208,14 +198,6 @@ pub fn releaseArena(self: *Page, allocator: Allocator) void { return self.session.releaseArena(allocator); } -pub fn setPermission(self: *Page, name: []const u8, state: PermissionState) !void { - const gop = try self.permissions.getOrPut(self.frame_arena, name); - if (!gop.found_existing) { - gop.key_ptr.* = try self.frame_arena.dupe(u8, name); - } - gop.value_ptr.* = state; -} - pub fn getOrCreateOrigin(self: *Page, key_: ?[]const u8) !*js.Origin { const session = self.session; const key = key_ orelse { diff --git a/src/browser/webapi/Permissions.zig b/src/browser/webapi/Permissions.zig index effe590aa..62a0920f0 100644 --- a/src/browser/webapi/Permissions.zig +++ b/src/browser/webapi/Permissions.zig @@ -50,7 +50,7 @@ pub fn query(_: *const Permissions, qd: QueryDescriptor, exec: *const Execution) const arena = try exec.getArena(.tiny, "PermissionStatus"); errdefer exec.releaseArena(arena); - const state = exec.page.permissions.get(qd.name) orelse .prompt; + const state = exec.session.browser.permissions.get(qd.name) orelse .prompt; const status = try arena.create(PermissionStatus); status.* = .{ ._arena = arena, diff --git a/src/cdp/domains/browser.zig b/src/cdp/domains/browser.zig index 5d121bb0e..b384c5616 100644 --- a/src/cdp/domains/browser.zig +++ b/src/cdp/domains/browser.zig @@ -17,8 +17,12 @@ // along with this program. If not, see . const std = @import("std"); +const lp = @import("lightpanda"); const CDP = @import("../CDP.zig"); +const log = lp.log; +const PermissionState = @import("../../browser/webapi/Permissions.zig").State; + // TODO: hard coded data const PROTOCOL_VERSION = "1.3"; const REVISION = "@9e6ded5ac1ff5e38d930ae52bd9aec09bd1a68e4"; @@ -97,7 +101,8 @@ fn setWindowBounds(cmd: *CDP.Command) !void { } // Grant the listed permissions so navigator.permissions.query() reports them -// as "granted". State is stored on the active Page and resets on navigation. +// as "granted". State is stored on the Browser, so it persists across page +// navigations. fn grantPermissions(cmd: *CDP.Command) !void { const params = (try cmd.params(struct { permissions: []const []const u8, @@ -105,10 +110,16 @@ fn grantPermissions(cmd: *CDP.Command) !void { browserContextId: ?[]const u8 = null, })) orelse return error.InvalidParams; - const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; - const page = bc.session.currentPage() orelse return error.PageNotLoaded; + if (params.origin) |v| { + log.warn(.not_implemented, "Browser.grantPermissions", .{ .param = "origin", .value = v }); + } + if (params.browserContextId) |v| { + log.warn(.not_implemented, "Browser.grantPermissions", .{ .param = "browserContextId", .value = v }); + } + + const browser = &cmd.cdp.browser; for (params.permissions) |name| { - try page.setPermission(name, .granted); + try browser.setPermission(name, .granted); } return cmd.sendResult(null, .{ .include_session_id = false }); @@ -124,25 +135,24 @@ fn setPermission(cmd: *CDP.Command) !void { browserContextId: ?[]const u8 = null, })) orelse return error.InvalidParams; - const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; - const page = bc.session.currentPage() orelse return error.PageNotLoaded; + if (params.origin) |v| { + log.warn(.not_implemented, "Browser.setPermission", .{ .param = "origin", .value = v }); + } + if (params.browserContextId) |v| { + log.warn(.not_implemented, "Browser.setPermission", .{ .param = "browserContextId", .value = v }); + } - const PermissionState = @import("../../browser/webapi/Permissions.zig").State; const state = std.meta.stringToEnum(PermissionState, params.setting) orelse { return error.InvalidPermissionSetting; }; - try page.setPermission(params.permission.name, state); + try cmd.cdp.browser.setPermission(params.permission.name, state); return cmd.sendResult(null, .{ .include_session_id = false }); } // Clear all granted permissions; navigator.permissions.query() falls back to // the default "prompt". fn resetPermissions(cmd: *CDP.Command) !void { - const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded; - if (bc.session.currentPage()) |page| { - page.permissions.clearRetainingCapacity(); - } - + cmd.cdp.browser.clearPermissions(); return cmd.sendResult(null, .{ .include_session_id = false }); } @@ -187,17 +197,21 @@ test "cdp.browser: grant/set/reset permissions reach navigator.permissions" { defer ctx.deinit(); const bc = try ctx.loadBrowserContext(.{ .id = "BID-PERM", .url = "cdp/dom1.html" }); - const page = bc.session.currentPage() orelse unreachable; + const browser = bc.session.browser; - // grantPermissions: each listed permission becomes "granted". + // grantPermissions: each listed permission becomes "granted". State lives + // on the Browser, not the Page. try ctx.processMessage(.{ .id = 40, .method = "Browser.grantPermissions", .params = .{ .permissions = &[_][]const u8{ "geolocation", "notifications" } }, }); try ctx.expectSentResult(null, .{ .id = 40, .session_id = null }); - try testing.expectEqual(.granted, page.permissions.get("geolocation").?); - try testing.expectEqual(.granted, page.permissions.get("notifications").?); + // State lives on the Browser, not the Page, so it persists for the whole + // CDP connection (across page navigations) rather than being lost when the + // page is replaced. + try testing.expectEqual(.granted, browser.permissions.get("geolocation").?); + try testing.expectEqual(.granted, browser.permissions.get("notifications").?); // setPermission: override a single permission to an explicit state. try ctx.processMessage(.{ @@ -206,7 +220,7 @@ test "cdp.browser: grant/set/reset permissions reach navigator.permissions" { .params = .{ .permission = .{ .name = "geolocation" }, .setting = "denied" }, }); try ctx.expectSentResult(null, .{ .id = 41, .session_id = null }); - try testing.expectEqual(.denied, page.permissions.get("geolocation").?); + try testing.expectEqual(.denied, browser.permissions.get("geolocation").?); // resetPermissions: clears everything; query falls back to "prompt". try ctx.processMessage(.{ @@ -214,5 +228,5 @@ test "cdp.browser: grant/set/reset permissions reach navigator.permissions" { .method = "Browser.resetPermissions", }); try ctx.expectSentResult(null, .{ .id = 42, .session_id = null }); - try testing.expectEqual(@as(usize, 0), page.permissions.count()); + try testing.expectEqual(@as(usize, 0), browser.permissions.count()); }