Merge pull request #1788 from lightpanda-io/range_cleanup

Add cleanup to Range

src/browser/Factory.zig

@@ -265,13 +265,15 @@ pub fn blob(_: *const Factory, arena: Allocator, child: anytype) !*@TypeOf(child
     return chain.get(1);
 }
 
-pub fn abstractRange(self: *Factory, child: anytype, page: *Page) !*@TypeOf(child) {
+pub fn abstractRange(_: *const Factory, arena: Allocator, child: anytype, page: *Page) !*@TypeOf(child) {
-    const allocator = self._slab.allocator();
+    const chain = try PrototypeChain(&.{ AbstractRange, @TypeOf(child) }).allocate(arena);
-    const chain = try PrototypeChain(&.{ AbstractRange, @TypeOf(child) }).allocate(allocator);
 
     const doc = page.document.asNode();
     const abstract_range = chain.get(0);
     abstract_range.* = AbstractRange{
+        ._rc = 0,
+        ._arena = arena,
+        ._page_id = page.id,
         ._type = unionInit(AbstractRange.Type, chain.get(1)),
         ._end_offset = 0,
         ._start_offset = 0,

src/browser/Page.zig

@@ -80,6 +80,8 @@ pub const BUF_SIZE = 1024;
 
 const Page = @This();
 
+id: u32,
+
 // This is the "id" of the frame. It can be re-used from page-to-page, e.g.
 // when navigating.
 _frame_id: u32,
@@ -254,6 +256,7 @@ pub fn init(self: *Page, frame_id: u32, session: *Session, parent: ?*Page) !void
     })).asDocument();
 
     self.* = .{
+        .id = session.nextPageId(),
         .js = undefined,
         .parent = parent,
         .arena = session.page_arena,

src/browser/Session.zig

@@ -84,6 +84,7 @@ queued_navigation: std.ArrayList(*Page),
 // about:blank navigations (which may add to queued_navigation).
 queued_queued_navigation: std.ArrayList(*Page),
 
+page_id_gen: u32,
 frame_id_gen: u32,
 
 pub fn init(self: *Session, browser: *Browser, notification: *Notification) !void {
@@ -103,6 +104,7 @@ pub fn init(self: *Session, browser: *Browser, notification: *Notification) !voi
         .page_arena = page_arena,
         .factory = Factory.init(page_arena),
         .history = .{},
+        .page_id_gen = 0,
         .frame_id_gen = 0,
         // The prototype (EventTarget) for Navigation is created when a Page is created.
         .navigation = .{ ._proto = undefined },
@@ -297,9 +299,24 @@ pub const WaitResult = enum {
     cdp_socket,
 };
 
-pub fn findPage(self: *Session, frame_id: u32) ?*Page {
+pub fn findPageByFrameId(self: *Session, frame_id: u32) ?*Page {
     const page = self.currentPage() orelse return null;
-    return if (page._frame_id == frame_id) page else null;
+    return findPageBy(page, "_frame_id", frame_id);
+}
+
+pub fn findPageById(self: *Session, id: u32) ?*Page {
+    const page = self.currentPage() orelse return null;
+    return findPageBy(page, "id", id);
+}
+
+fn findPageBy(page: *Page, comptime field: []const u8, id: u32) ?*Page {
+    if (@field(page, field) == id) return page;
+    for (page.frames.items) |f| {
+        if (findPageBy(f, field, id)) |found| {
+            return found;
+        }
+    }
+    return null;
 }
 
 pub fn wait(self: *Session, wait_ms: u32) WaitResult {
@@ -636,3 +653,9 @@ pub fn nextFrameId(self: *Session) u32 {
     self.frame_id_gen = id;
     return id;
 }
+
+pub fn nextPageId(self: *Session) u32 {
+    const id = self.page_id_gen +% 1;
+    self.page_id_gen = id;
+    return id;
+}

src/browser/webapi/AbstractRange.zig

@@ -19,15 +19,22 @@
 const std = @import("std");
 const js = @import("../js/js.zig");
 
+const Session = @import("../Session.zig");
+
 const Node = @import("Node.zig");
 const Range = @import("Range.zig");
 
+const Allocator = std.mem.Allocator;
+const IS_DEBUG = @import("builtin").mode == .Debug;
+
 const AbstractRange = @This();
 
 pub const _prototype_root = true;
 
+_rc: u8,
 _type: Type,
-
+_page_id: u32,
+_arena: Allocator,
 _end_offset: u32,
 _start_offset: u32,
 _end_container: *Node,
@@ -36,6 +43,27 @@ _start_container: *Node,
 // Intrusive linked list node for tracking live ranges on the Page.
 _range_link: std.DoublyLinkedList.Node = .{},
 
+pub fn acquireRef(self: *AbstractRange) void {
+    self._rc += 1;
+}
+
+pub fn deinit(self: *AbstractRange, shutdown: bool, session: *Session) void {
+    _ = shutdown;
+    const rc = self._rc;
+    if (comptime IS_DEBUG) {
+        std.debug.assert(rc != 0);
+    }
+
+    if (rc == 1) {
+        if (session.findPageById(self._page_id)) |page| {
+            page._live_ranges.remove(&self._range_link);
+        }
+        session.releaseArena(self._arena);
+        return;
+    }
+    self._rc = rc - 1;
+}
+
 pub const Type = union(enum) {
     range: *Range,
     // TODO: static_range: *StaticRange,
@@ -310,6 +338,8 @@ pub const JsApi = struct {
         pub const name = "AbstractRange";
         pub const prototype_chain = bridge.prototypeChain();
         pub var class_id: bridge.ClassId = undefined;
+        pub const weak = true;
+        pub const finalizer = bridge.finalizer(AbstractRange.deinit);
     };
 
     pub const startContainer = bridge.accessor(AbstractRange.getStartContainer, null, .{});

src/browser/webapi/Range.zig

@@ -21,22 +21,31 @@ const String = @import("../../string.zig").String;
 
 const js = @import("../js/js.zig");
 const Page = @import("../Page.zig");
+const Session = @import("../Session.zig");
 
 const Node = @import("Node.zig");
 const DocumentFragment = @import("DocumentFragment.zig");
 const AbstractRange = @import("AbstractRange.zig");
 const DOMRect = @import("DOMRect.zig");
 
+const Allocator = std.mem.Allocator;
+
 const Range = @This();
 
 _proto: *AbstractRange,
 
-pub fn asAbstractRange(self: *Range) *AbstractRange {
+pub fn init(page: *Page) !*Range {
-    return self._proto;
+    const arena = try page.getArena(.{ .debug = "Range" });
+    errdefer page.releaseArena(arena);
+    return page._factory.abstractRange(arena, Range{ ._proto = undefined }, page);
 }
 
-pub fn init(page: *Page) !*Range {
+pub fn deinit(self: *Range, shutdown: bool, session: *Session) void {
-    return page._factory.abstractRange(Range{ ._proto = undefined }, page);
+    self._proto.deinit(shutdown, session);
+}
+
+pub fn asAbstractRange(self: *Range) *AbstractRange {
+    return self._proto;
 }
 
 pub fn setStart(self: *Range, node: *Node, offset: u32) !void {
@@ -309,7 +318,10 @@ pub fn intersectsNode(self: *const Range, node: *Node) bool {
 }
 
 pub fn cloneRange(self: *const Range, page: *Page) !*Range {
-    const clone = try page._factory.abstractRange(Range{ ._proto = undefined }, page);
+    const arena = try page.getArena(.{ .debug = "Range.clone" });
+    errdefer page.releaseArena(arena);
+
+    const clone = try page._factory.abstractRange(arena, Range{ ._proto = undefined }, page);
     clone._proto._end_offset = self._proto._end_offset;
     clone._proto._start_offset = self._proto._start_offset;
     clone._proto._end_container = self._proto._end_container;
@@ -687,6 +699,8 @@ pub const JsApi = struct {
         pub const name = "Range";
         pub const prototype_chain = bridge.prototypeChain();
         pub var class_id: bridge.ClassId = undefined;
+        pub const weak = true;
+        pub const finalizer = bridge.finalizer(Range.deinit);
     };
 
     // Constants for compareBoundaryPoints

src/browser/webapi/Selection.zig

@@ -21,6 +21,8 @@ const log = @import("../../log.zig");
 
 const js = @import("../js/js.zig");
 const Page = @import("../Page.zig");
+const Session = @import("../Session.zig");
+
 const Range = @import("Range.zig");
 const AbstractRange = @import("AbstractRange.zig");
 const Node = @import("Node.zig");
@@ -37,13 +39,22 @@ _direction: SelectionDirection = .none,
 
 pub const init: Selection = .{};
 
+pub fn deinit(self: *Selection, shutdown: bool, session: *Session) void {
+    if (self._range) |r| {
+        r.deinit(shutdown, session);
+        self._range = null;
+    }
+}
+
 fn dispatchSelectionChangeEvent(page: *Page) !void {
     const event = try Event.init("selectionchange", .{}, page);
     try page._event_manager.dispatch(page.document.asEventTarget(), event);
 }
 
 fn isInTree(self: *const Selection) bool {
-    if (self._range == null) return false;
+    if (self._range == null) {
+        return false;
+    }
     const anchor_node = self.getAnchorNode() orelse return false;
     const focus_node = self.getFocusNode() orelse return false;
     return anchor_node.isConnected() and focus_node.isConnected();
@@ -104,21 +115,33 @@ pub fn getIsCollapsed(self: *const Selection) bool {
 }
 
 pub fn getRangeCount(self: *const Selection) u32 {
-    if (self._range == null) return 0;
+    if (self._range == null) {
-    if (!self.isInTree()) return 0;
+        return 0;
+    }
+    if (!self.isInTree()) {
+        return 0;
+    }
 
     return 1;
 }
 
 pub fn getType(self: *const Selection) []const u8 {
-    if (self._range == null) return "None";
+    if (self._range == null) {
-    if (!self.isInTree()) return "None";
+        return "None";
-    if (self.getIsCollapsed()) return "Caret";
+    }
+    if (!self.isInTree()) {
+        return "None";
+    }
+    if (self.getIsCollapsed()) {
+        return "Caret";
+    }
     return "Range";
 }
 
 pub fn addRange(self: *Selection, range: *Range, page: *Page) !void {
-    if (self._range != null) return;
+    if (self._range != null) {
+        return;
+    }
 
     // Only add the range if its root node is in the document associated with this selection
     const start_node = range.asAbstractRange().getStartContainer();
@@ -126,22 +149,25 @@ pub fn addRange(self: *Selection, range: *Range, page: *Page) !void {
         return;
     }
 
-    self._range = range;
+    self.setRange(range, page);
     try dispatchSelectionChangeEvent(page);
 }
 
 pub fn removeRange(self: *Selection, range: *Range, page: *Page) !void {
-    if (self._range == range) {
+    const existing_range = self._range orelse return error.NotFound;
-        self._range = null;
+    if (existing_range != range) {
-        try dispatchSelectionChangeEvent(page);
-        return;
-    } else {
         return error.NotFound;
     }
+    self.setRange(null, page);
+    try dispatchSelectionChangeEvent(page);
 }
 
 pub fn removeAllRanges(self: *Selection, page: *Page) !void {
-    self._range = null;
+    if (self._range == null) {
+        return;
+    }
+
+    self.setRange(null, page);
     self._direction = .none;
     try dispatchSelectionChangeEvent(page);
 }
@@ -157,7 +183,7 @@ pub fn collapseToEnd(self: *Selection, page: *Page) !void {
     try new_range.setStart(last_node, last_offset);
     try new_range.setEnd(last_node, last_offset);
 
-    self._range = new_range;
+    self.setRange(new_range, page);
     self._direction = .none;
     try dispatchSelectionChangeEvent(page);
 }
@@ -173,7 +199,7 @@ pub fn collapseToStart(self: *Selection, page: *Page) !void {
     try new_range.setStart(first_node, first_offset);
     try new_range.setEnd(first_node, first_offset);
 
-    self._range = new_range;
+    self.setRange(new_range, page);
     self._direction = .none;
     try dispatchSelectionChangeEvent(page);
 }
@@ -255,7 +281,7 @@ pub fn extend(self: *Selection, node: *Node, _offset: ?u32, page: *Page) !void {
         },
     }
 
-    self._range = new_range;
+    self.setRange(new_range, page);
     try dispatchSelectionChangeEvent(page);
 }
 
@@ -560,7 +586,8 @@ fn applyModify(self: *Selection, alter: ModifyAlter, new_node: *Node, new_offset
             const new_range = try Range.init(page);
             try new_range.setStart(new_node, new_offset);
             try new_range.setEnd(new_node, new_offset);
-            self._range = new_range;
+
+            self.setRange(new_range, page);
             self._direction = .none;
             try dispatchSelectionChangeEvent(page);
         },
@@ -582,7 +609,7 @@ pub fn selectAllChildren(self: *Selection, parent: *Node, page: *Page) !void {
     const child_count = parent.getChildrenCount();
     try range.setEnd(parent, @intCast(child_count));
 
-    self._range = range;
+    self.setRange(range, page);
     self._direction = .forward;
     try dispatchSelectionChangeEvent(page);
 }
@@ -630,7 +657,7 @@ pub fn setBaseAndExtent(
         },
     }
 
-    self._range = range;
+    self.setRange(range, page);
     try dispatchSelectionChangeEvent(page);
 }
 
@@ -656,7 +683,7 @@ pub fn collapse(self: *Selection, _node: ?*Node, _offset: ?u32, page: *Page) !vo
     try range.setStart(node, offset);
     try range.setEnd(node, offset);
 
-    self._range = range;
+    self.setRange(range, page);
     self._direction = .none;
     try dispatchSelectionChangeEvent(page);
 }
@@ -666,6 +693,16 @@ pub fn toString(self: *const Selection, page: *Page) ![]const u8 {
     return try range.toString(page);
 }
 
+fn setRange(self: *Selection, new_range: ?*Range, page: *Page) void {
+    if (self._range) |existing| {
+        existing.deinit(false, page._session);
+    }
+    if (new_range) |nr| {
+        nr.asAbstractRange().acquireRef();
+    }
+    self._range = new_range;
+}
+
 pub const JsApi = struct {
     pub const bridge = js.Bridge(Selection);
 
@@ -673,6 +710,7 @@ pub const JsApi = struct {
         pub const name = "Selection";
         pub const prototype_chain = bridge.prototypeChain();
         pub var class_id: bridge.ClassId = undefined;
+        pub const finalizer = bridge.finalizer(Selection.deinit);
     };
 
     pub const anchorNode = bridge.accessor(Selection.getAnchorNode, null, .{});

src/cdp/domains/accessibility.zig

@@ -53,8 +53,8 @@ fn getFullAXTree(cmd: anytype) !void {
         const frame_id = params.frameId orelse {
             break :blk session.currentPage() orelse return error.PageNotLoaded;
         };
-        const page_id = try id.toPageId(.frame_id, frame_id);
+        const page_frame_id = try id.toPageId(.frame_id, frame_id);
-        break :blk session.findPage(page_id) orelse {
+        break :blk session.findPageByFrameId(page_frame_id) orelse {
             return cmd.sendError(-32000, "Frame with the given id does not belong to the target.", .{});
         };
     };

src/cdp/domains/dom.zig

@@ -502,9 +502,9 @@ fn getFrameOwner(cmd: anytype) !void {
     })) orelse return error.InvalidParams;
 
     const bc = cmd.browser_context orelse return error.BrowserContextNotLoaded;
-    const page_id = try id.toPageId(.frame_id, params.frameId);
+    const page_frame_id = try id.toPageId(.frame_id, params.frameId);
 
-    const page = bc.session.findPage(page_id) orelse {
+    const page = bc.session.findPageByFrameId(page_frame_id) orelse {
         return cmd.sendError(-32000, "Frame with the given id does not belong to the target.", .{});
     };
 

src/cdp/domains/network.zig

@@ -238,7 +238,7 @@ pub fn httpRequestStart(bc: anytype, msg: *const Notification.RequestStart) !voi
     const transfer = msg.transfer;
     const req = &transfer.req;
     const frame_id = req.frame_id;
-    const page = bc.session.findPage(frame_id) orelse return;
+    const page = bc.session.findPageByFrameId(frame_id) orelse return;
 
     // Modify request with extra CDP headers
     for (bc.extra_headers.items) |extra| {