Merge pull request #1459 from lightpanda-io/browser_context_arenas

Add a dedicated browser_context and page_arena to CDP.

src/cdp/cdp.zig

@@ -76,6 +76,13 @@ pub fn CDPT(comptime TypeProvider: type) type {
         // Used for processing notifications within a browser context.
         notification_arena: std.heap.ArenaAllocator,
 
+        // Valid for 1 page navigation (what CDP calls a "renderer")
+        page_arena: std.heap.ArenaAllocator,
+
+        // Valid for the entire lifetime of the BrowserContext. Should minimize
+        // (or altogether elimiate) our use of this.
+        browser_context_arena: std.heap.ArenaAllocator,
+
         const Self = @This();
 
         pub fn init(app: *App, client: TypeProvider.Client) !Self {
@@ -90,8 +97,10 @@ pub fn CDPT(comptime TypeProvider: type) type {
                 .browser = browser,
                 .allocator = allocator,
                 .browser_context = null,
+                .page_arena = std.heap.ArenaAllocator.init(allocator),
                 .message_arena = std.heap.ArenaAllocator.init(allocator),
                 .notification_arena = std.heap.ArenaAllocator.init(allocator),
+                .browser_context_arena = std.heap.ArenaAllocator.init(allocator),
             };
         }
 
@@ -100,8 +109,10 @@ pub fn CDPT(comptime TypeProvider: type) type {
                 bc.deinit();
             }
             self.browser.deinit();
+            self.page_arena.deinit();
             self.message_arena.deinit();
             self.notification_arena.deinit();
+            self.browser_context_arena.deinit();
         }
 
         pub fn handleMessage(self: *Self, msg: []const u8) bool {
@@ -323,9 +334,12 @@ pub fn BrowserContext(comptime CDP_T: type) type {
         // RELATION TO SESSION_ID
         session: *Session,
 
-        // Points to the session arena
+        // Tied to the lifetime of the BrowserContext
         arena: Allocator,
 
+        // Tied to the lifetime of 1 page rendered in the BrowserContext.
+        page_arena: Allocator,
+
         // From the parent's notification_arena.allocator(). Most of the CDP
         // code paths deal with a cmd which has its own arena (from the
         // message_arena). But notifications happen outside of the typical CDP
@@ -355,12 +369,12 @@ pub fn BrowserContext(comptime CDP_T: type) type {
         node_search_list: Node.Search.List,
 
         inspector_session: *js.Inspector.Session,
-        isolated_worlds: std.ArrayListUnmanaged(IsolatedWorld),
+        isolated_worlds: std.ArrayList(*IsolatedWorld),
 
         http_proxy_changed: bool = false,
 
         // Extra headers to add to all requests.
-        extra_headers: std.ArrayListUnmanaged([*c]const u8) = .empty,
+        extra_headers: std.ArrayList([*c]const u8) = .empty,
 
         intercept_state: InterceptState,
 
@@ -373,7 +387,7 @@ pub fn BrowserContext(comptime CDP_T: type) type {
         // ever streamed. So if CDP is the only thing that needs bodies in
         // memory for an arbitrary amount of time, then that's where we're going
         // to store the,
-        captured_responses: std.AutoHashMapUnmanaged(usize, std.ArrayListUnmanaged(u8)),
+        captured_responses: std.AutoHashMapUnmanaged(usize, std.ArrayList(u8)),
 
         const Self = @This();
 
@@ -381,7 +395,6 @@ pub fn BrowserContext(comptime CDP_T: type) type {
             const allocator = cdp.allocator;
 
             const session = try cdp.browser.newSession();
-            const arena = session.arena;
 
             const browser = &cdp.browser;
             const inspector_session = browser.env.inspector.?.startSession(self);
@@ -393,7 +406,6 @@ pub fn BrowserContext(comptime CDP_T: type) type {
             self.* = .{
                 .id = id,
                 .cdp = cdp,
-                .arena = arena,
                 .target_id = null,
                 .session_id = null,
                 .session = session,
@@ -405,6 +417,8 @@ pub fn BrowserContext(comptime CDP_T: type) type {
                 .node_search_list = undefined,
                 .isolated_worlds = .empty,
                 .inspector_session = inspector_session,
+                .page_arena = cdp.page_arena.allocator(),
+                .arena = cdp.browser_context_arena.allocator(),
                 .notification_arena = cdp.notification_arena.allocator(),
                 .intercept_state = try InterceptState.init(allocator),
                 .captured_responses = .empty,
@@ -442,7 +456,7 @@ pub fn BrowserContext(comptime CDP_T: type) type {
                 transfer.abort(error.ClientDisconnect);
             }
 
-            for (self.isolated_worlds.items) |*world| {
+            for (self.isolated_worlds.items) |world| {
                 world.deinit();
             }
             self.isolated_worlds.clearRetainingCapacity();
@@ -472,16 +486,21 @@ pub fn BrowserContext(comptime CDP_T: type) type {
         }
 
         pub fn createIsolatedWorld(self: *Self, world_name: []const u8, grant_universal_access: bool) !*IsolatedWorld {
-            const owned_name = try self.arena.dupe(u8, world_name);
+            const browser = &self.cdp.browser;
-            const world = try self.isolated_worlds.addOne(self.arena);
+            const arena = try browser.arena_pool.acquire();
+            errdefer browser.arena_pool.release(arena);
 
+            const world = try arena.create(IsolatedWorld);
             world.* = .{
+                .arena = arena,
                 .context = null,
-                .name = owned_name,
+                .browser = browser,
-                .env = &self.cdp.browser.env,
+                .name = try arena.dupe(u8, world_name),
                 .grant_universal_access = grant_universal_access,
             };
 
+            try self.isolated_worlds.append(self.arena, world);
+
             return world;
         }
 
@@ -634,7 +653,7 @@ pub fn BrowserContext(comptime CDP_T: type) type {
 
         pub fn onHttpResponseData(ctx: *anyopaque, msg: *const Notification.ResponseData) !void {
             const self: *Self = @ptrCast(@alignCast(ctx));
-            const arena = self.arena;
+            const arena = self.page_arena;
 
             const id = msg.transfer.id;
             const gop = try self.captured_responses.getOrPut(arena, id);
@@ -700,7 +719,7 @@ pub fn BrowserContext(comptime CDP_T: type) type {
             // + 10 for the max websocket header
             const message_len = msg.len + session_id.len + 1 + field.len + 10;
 
-            var buf: std.ArrayListUnmanaged(u8) = .{};
+            var buf: std.ArrayList(u8) = .{};
             buf.ensureTotalCapacity(allocator, message_len) catch |err| {
                 log.err(.cdp, "inspector buffer", .{ .err = err });
                 return;
@@ -734,20 +753,20 @@ pub fn BrowserContext(comptime CDP_T: type) type {
 /// Generally the client needs to resolve a node into the isolated world to be able to work with it.
 /// An object id is unique across all contexts, different object ids can refer to the same Node in different contexts.
 const IsolatedWorld = struct {
+    arena: Allocator,
+    browser: *Browser,
     name: []const u8,
-    env: *js.Env,
     context: ?*js.Context = null,
     grant_universal_access: bool,
 
     pub fn deinit(self: *IsolatedWorld) void {
-        if (self.context) |ctx| {
+        self.removeContext() catch {};
-            self.env.destroyContext(ctx);
+        self.browser.arena_pool.release(self.arena);
-            self.context = null;
-        }
     }
+
     pub fn removeContext(self: *IsolatedWorld) !void {
         const ctx = self.context orelse return error.NoIsolatedContextToRemove;
-        self.env.destroyContext(ctx);
+        self.browser.env.destroyContext(ctx);
         self.context = null;
     }
 
@@ -758,7 +777,7 @@ const IsolatedWorld = struct {
     // Currently we have only 1 page/frame and thus also only 1 state in the isolate world.
     pub fn createContext(self: *IsolatedWorld, page: *Page) !*js.Context {
         if (self.context == null) {
-            self.context = try self.env.createContext(page, false);
+            self.context = try self.browser.env.createContext(page, false);
         } else {
             log.warn(.cdp, "not implemented", .{
                 .feature = "createContext: Not implemented second isolated context creation",

src/cdp/domains/dom.zig

@@ -284,7 +284,7 @@ fn resolveNode(cmd: anytype) !void {
             break :blk;
         }
         // not the default scope, check the other ones
-        for (bc.isolated_worlds.items) |*isolated_world| {
+        for (bc.isolated_worlds.items) |isolated_world| {
             ls.?.deinit();
             ls = null;
 

src/cdp/domains/page.zig

@@ -167,7 +167,7 @@ fn close(cmd: anytype) !void {
     }
 
     bc.session.removePage();
-    for (bc.isolated_worlds.items) |*world| {
+    for (bc.isolated_worlds.items) |world| {
         world.deinit();
     }
     bc.isolated_worlds.clearRetainingCapacity();
@@ -275,15 +275,21 @@ pub fn pageNavigate(arena: Allocator, bc: anytype, event: *const Notification.Pa
 
 pub fn pageRemove(bc: anytype) !void {
     // The main page is going to be removed, we need to remove contexts from other worlds first.
-    for (bc.isolated_worlds.items) |*isolated_world| {
+    for (bc.isolated_worlds.items) |isolated_world| {
         try isolated_world.removeContext();
     }
 }
 
 pub fn pageCreated(bc: anytype, page: *Page) !void {
-    for (bc.isolated_worlds.items) |*isolated_world| {
+    _ = bc.cdp.page_arena.reset(.{ .retain_with_limit = 1024 * 512 });
+
+    for (bc.isolated_worlds.items) |isolated_world| {
         _ = try isolated_world.createContext(page);
     }
+    // Only retain captured responses until a navigation event. In CDP term,
+    // this is called a "renderer" and the cache-duration can be controlled via
+    // the Network.configureDurableMessages message (which we don't support)
+    bc.captured_responses = .empty;
 }
 
 pub fn pageNavigated(arena: Allocator, bc: anytype, event: *const Notification.PageNavigated) !void {
@@ -359,7 +365,7 @@ pub fn pageNavigated(arena: Allocator, bc: anytype, event: *const Notification.P
             true,
         );
     }
-    for (bc.isolated_worlds.items) |*isolated_world| {
+    for (bc.isolated_worlds.items) |isolated_world| {
         const aux_json = try std.fmt.allocPrint(arena, "{{\"isDefault\":false,\"type\":\"isolated\",\"frameId\":\"{s}\"}}", .{target_id});
 
         // Calling contextCreated will assign a new Id to the context and send the contextCreated event

src/cdp/domains/target.zig

@@ -283,7 +283,7 @@ fn closeTarget(cmd: anytype) !void {
     }
 
     bc.session.removePage();
-    for (bc.isolated_worlds.items) |*world| {
+    for (bc.isolated_worlds.items) |world| {
         world.deinit();
     }
     bc.isolated_worlds.clearRetainingCapacity();