From 14dc19edb16be1c4e2a2b155acea8808f9c0b055 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Tue, 27 Jan 2026 15:28:38 +0800 Subject: [PATCH] Leverage finalizers and ArenaPool in Intersction and Mutation Observer Both of these become self-contained with their own arena, which can be cleaned up when (a) we don't need it anymore and (b) v8 doens't need it. The "we don't need it anymore" is true when there's nothing being observed. The underlying records also get their own arena and are handed off to v8 to finalize whenever they fall out of scope. --- src/browser/js/Context.zig | 12 +++ src/browser/webapi/IntersectionObserver.zig | 79 +++++++++++++---- src/browser/webapi/MutationObserver.zig | 97 ++++++++++++++++----- 3 files changed, 150 insertions(+), 38 deletions(-) diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index d2676c84..061bd2c9 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -234,6 +234,18 @@ pub fn weakRef(self: *Context, obj: anytype) void { v8.v8__Global__SetWeakFinalizer(global, obj, bridge.Struct(@TypeOf(obj)).JsApi.Meta.finalizer.from_v8, v8.kParameter); } +pub fn safeWeakRef(self: *Context, obj: anytype) void { + const global = self.identity_map.getPtr(@intFromPtr(obj)) orelse { + if (comptime IS_DEBUG) { + // should not be possible + std.debug.assert(false); + } + return; + }; + v8.v8__Global__ClearWeak(global); + v8.v8__Global__SetWeakFinalizer(global, obj, bridge.Struct(@TypeOf(obj)).JsApi.Meta.finalizer.from_v8, v8.kParameter); +} + pub fn strongRef(self: *Context, obj: anytype) void { const global = self.identity_map.getPtr(@intFromPtr(obj)) orelse { if (comptime IS_DEBUG) { diff --git a/src/browser/webapi/IntersectionObserver.zig b/src/browser/webapi/IntersectionObserver.zig index bc793206..8f0b338e 100644 --- a/src/browser/webapi/IntersectionObserver.zig +++ b/src/browser/webapi/IntersectionObserver.zig @@ -1,4 +1,4 @@ -// Copyright (C) 2023-2025 Lightpanda (Selecy SAS) +// Copyright (C) 2023-2026 Lightpanda (Selecy SAS) // // Francis Bouvier // Pierre Tachoire @@ -19,6 +19,10 @@ const std = @import("std"); const js = @import("../js/js.zig"); const log = @import("../../log.zig"); +const IS_DEBUG = @import("builtin").mode == .Debug; + +const Allocator = std.mem.Allocator; + const Page = @import("../Page.zig"); const Element = @import("Element.zig"); const DOMRect = @import("DOMRect.zig"); @@ -32,7 +36,9 @@ pub fn registerTypes() []const type { const IntersectionObserver = @This(); -_callback: js.Function.Global, +_page: *Page, +_arena: Allocator, +_callback: js.Function.Temp, _observing: std.ArrayList(*Element) = .{}, _root: ?*Element = null, _root_margin: []const u8 = "0px", @@ -59,25 +65,42 @@ pub const ObserverInit = struct { }; }; -pub fn init(callback: js.Function.Global, options: ?ObserverInit, page: *Page) !*IntersectionObserver { +pub fn init(callback: js.Function.Temp, options: ?ObserverInit, page: *Page) !*IntersectionObserver { + const arena = try page.getArena(.{ .debug = "IntersectionObserver" }); + errdefer page.releaseArena(arena); + const opts = options orelse ObserverInit{}; - const root_margin = if (opts.rootMargin) |rm| try page.arena.dupe(u8, rm) else "0px"; + const root_margin = if (opts.rootMargin) |rm| try arena.dupe(u8, rm) else "0px"; const threshold = switch (opts.threshold) { .scalar => |s| blk: { - const arr = try page.arena.alloc(f64, 1); + const arr = try arena.alloc(f64, 1); arr[0] = s; break :blk arr; }, - .array => |arr| try page.arena.dupe(f64, arr), + .array => |arr| try arena.dupe(f64, arr), }; - return page._factory.create(IntersectionObserver{ + const self = try arena.create(IntersectionObserver); + self.* = .{ + ._page = page, + ._arena = arena, ._callback = callback, ._root = opts.root, ._root_margin = root_margin, ._threshold = threshold, - }); + }; + return self; +} + +pub fn deinit(self: *IntersectionObserver, shutdown: bool) void { + const page = self._page; + page.js.release(self._callback); + if ((comptime IS_DEBUG) and !shutdown) { + std.debug.assert(self._observing.items.len == 0); + } + + page.releaseArena(self._arena); } pub fn observe(self: *IntersectionObserver, target: *Element, page: *Page) !void { @@ -90,10 +113,11 @@ pub fn observe(self: *IntersectionObserver, target: *Element, page: *Page) !void // Register with page if this is our first observation if (self._observing.items.len == 0) { + page.js.strongRef(self); try page.registerIntersectionObserver(self); } - try self._observing.append(page.arena, target); + try self._observing.append(self._arena, target); // Don't initialize previous state yet - let checkIntersection do it // This ensures we get an entry on first observation @@ -105,7 +129,7 @@ pub fn observe(self: *IntersectionObserver, target: *Element, page: *Page) !void } } -pub fn unobserve(self: *IntersectionObserver, target: *Element) void { +pub fn unobserve(self: *IntersectionObserver, target: *Element, page: *Page) void { for (self._observing.items, 0..) |elem, i| { if (elem == target) { _ = self._observing.swapRemove(i); @@ -115,21 +139,31 @@ pub fn unobserve(self: *IntersectionObserver, target: *Element) void { var j: usize = 0; while (j < self._pending_entries.items.len) { if (self._pending_entries.items[j]._target == target) { - _ = self._pending_entries.swapRemove(j); + const entry = self._pending_entries.swapRemove(j); + entry.deinit(false); } else { j += 1; } } - return; + break; } } + + if (self._observing.items.len == 0) { + page.js.safeWeakRef(self); + } } pub fn disconnect(self: *IntersectionObserver, page: *Page) void { page.unregisterIntersectionObserver(self); self._observing.clearRetainingCapacity(); self._previous_states.clearRetainingCapacity(); + + for (self._pending_entries.items) |entry| { + entry.deinit(false); + } self._pending_entries.clearRetainingCapacity(); + page.js.safeWeakRef(self); } pub fn takeRecords(self: *IntersectionObserver, page: *Page) ![]*IntersectionObserverEntry { @@ -206,8 +240,11 @@ fn checkIntersection(self: *IntersectionObserver, target: *Element, page: *Page) (was_intersecting_opt != null and was_intersecting_opt.? != is_now_intersecting); if (should_report) { - const entry = try page.arena.create(IntersectionObserverEntry); + const arena = try page.getArena(.{ .debug = "IntersectionObserverEntry" }); + const entry = try arena.create(IntersectionObserverEntry); entry.* = .{ + ._page = page, + ._arena = arena, ._target = target, ._time = 0.0, // TODO: Get actual timestamp ._bounding_client_rect = data.bounding_client_rect, @@ -217,12 +254,12 @@ fn checkIntersection(self: *IntersectionObserver, target: *Element, page: *Page) ._is_intersecting = is_now_intersecting, }; - try self._pending_entries.append(page.arena, entry); + try self._pending_entries.append(self._arena, entry); } // Always update the previous state, even if we didn't report // This ensures we can detect state changes on subsequent checks - try self._previous_states.put(page.arena, target, is_now_intersecting); + try self._previous_states.put(self._arena, target, is_now_intersecting); } pub fn checkIntersections(self: *IntersectionObserver, page: *Page) !void { @@ -258,14 +295,20 @@ pub fn deliverEntries(self: *IntersectionObserver, page: *Page) !void { } pub const IntersectionObserverEntry = struct { - _target: *Element, + _page: *Page, + _arena: Allocator, _time: f64, + _target: *Element, _bounding_client_rect: *DOMRect, _intersection_rect: *DOMRect, _root_bounds: *DOMRect, _intersection_ratio: f64, _is_intersecting: bool, + pub fn deinit(self: *const IntersectionObserverEntry, _: bool) void { + self._page.releaseArena(self._arena); + } + pub fn getTarget(self: *const IntersectionObserverEntry) *Element { return self._target; } @@ -301,6 +344,8 @@ pub const IntersectionObserverEntry = struct { pub const name = "IntersectionObserverEntry"; pub const prototype_chain = bridge.prototypeChain(); pub var class_id: bridge.ClassId = undefined; + pub const weak = true; + pub const finalizer = bridge.finalizer(IntersectionObserverEntry.deinit); }; pub const target = bridge.accessor(IntersectionObserverEntry.getTarget, null, .{}); @@ -320,6 +365,8 @@ pub const JsApi = struct { pub const name = "IntersectionObserver"; pub const prototype_chain = bridge.prototypeChain(); pub var class_id: bridge.ClassId = undefined; + pub const weak = true; + pub const finalizer = bridge.finalizer(IntersectionObserver.deinit); }; pub const constructor = bridge.constructor(init, .{}); diff --git a/src/browser/webapi/MutationObserver.zig b/src/browser/webapi/MutationObserver.zig index 8d01cb42..7d548ee7 100644 --- a/src/browser/webapi/MutationObserver.zig +++ b/src/browser/webapi/MutationObserver.zig @@ -1,4 +1,4 @@ -// Copyright (C) 2023-2025 Lightpanda (Selecy SAS) +// Copyright (C) 2023-2026 Lightpanda (Selecy SAS) // // Francis Bouvier // Pierre Tachoire @@ -25,6 +25,10 @@ const Node = @import("Node.zig"); const Element = @import("Element.zig"); const log = @import("../../log.zig"); +const IS_DEBUG = @import("builtin").mode == .Debug; + +const Allocator = std.mem.Allocator; + pub fn registerTypes() []const type { return &.{ MutationObserver, @@ -34,9 +38,12 @@ pub fn registerTypes() []const type { const MutationObserver = @This(); -_callback: js.Function.Global, +_page: *Page, +_arena: Allocator, +_callback: js.Function.Temp, _observing: std.ArrayList(Observing) = .{}, _pending_records: std.ArrayList(*MutationRecord) = .{}, + /// Intrusively linked to next element (see Page.zig). node: std.DoublyLinkedList.Node = .{}, @@ -55,19 +62,38 @@ pub const ObserveOptions = struct { attributeFilter: ?[]const []const u8 = null, }; -pub fn init(callback: js.Function.Global, page: *Page) !*MutationObserver { - return page._factory.create(MutationObserver{ +pub fn init(callback: js.Function.Temp, page: *Page) !*MutationObserver { + const arena = try page.getArena(.{ .debug = "MutationObserver" }); + errdefer page.releaseArena(arena); + + const self = try arena.create(MutationObserver); + self.* = .{ + ._page = page, + ._arena = arena, ._callback = callback, - }); + }; + return self; +} + +pub fn deinit(self: *MutationObserver, shutdown: bool) void { + const page = self._page; + page.js.release(self._callback); + if ((comptime IS_DEBUG) and !shutdown) { + std.debug.assert(self._observing.items.len == 0); + } + + page.releaseArena(self._arena); } pub fn observe(self: *MutationObserver, target: *Node, options: ObserveOptions, page: *Page) !void { + const arena = self._arena; + // Deep copy attributeFilter if present var copied_options = options; if (options.attributeFilter) |filter| { - const filter_copy = try page.arena.alloc([]const u8, filter.len); + const filter_copy = try arena.alloc([]const u8, filter.len); for (filter, 0..) |name, i| { - filter_copy[i] = try page.arena.dupe(u8, name); + filter_copy[i] = try arena.dupe(u8, name); } copied_options.attributeFilter = filter_copy; } @@ -86,10 +112,11 @@ pub fn observe(self: *MutationObserver, target: *Node, options: ObserveOptions, // Register with page if this is our first observation if (self._observing.items.len == 0) { + page.js.strongRef(self); try page.registerMutationObserver(self); } - try self._observing.append(page.arena, .{ + try self._observing.append(arena, .{ .target = target, .options = copied_options, }); @@ -98,7 +125,11 @@ pub fn observe(self: *MutationObserver, target: *Node, options: ObserveOptions, pub fn disconnect(self: *MutationObserver, page: *Page) void { page.unregisterMutationObserver(self); self._observing.clearRetainingCapacity(); + for (self._pending_records.items) |record| { + record.deinit(false); + } self._pending_records.clearRetainingCapacity(); + page.js.safeWeakRef(self); } pub fn takeRecords(self: *MutationObserver, page: *Page) ![]*MutationRecord { @@ -139,21 +170,25 @@ pub fn notifyAttributeChange( } } - const record = try page._factory.create(MutationRecord{ + const arena = try self._page.getArena(.{ .debug = "MutationRecord" }); + const record = try arena.create(MutationRecord); + record.* = .{ + ._page = page, + ._arena = arena, ._type = .attributes, ._target = target_node, - ._attribute_name = try page.arena.dupe(u8, attribute_name.str()), + ._attribute_name = try arena.dupe(u8, attribute_name.str()), ._old_value = if (obs.options.attributeOldValue and old_value != null) - try page.arena.dupe(u8, old_value.?.str()) + try arena.dupe(u8, old_value.?.str()) else null, ._added_nodes = &.{}, ._removed_nodes = &.{}, ._previous_sibling = null, ._next_sibling = null, - }); + }; - try self._pending_records.append(page.arena, record); + try self._pending_records.append(self._arena, record); try page.scheduleMutationDelivery(); break; @@ -180,21 +215,25 @@ pub fn notifyCharacterDataChange( continue; } - const record = try page._factory.create(MutationRecord{ + const arena = try self._page.getArena(.{ .debug = "MutationRecord" }); + const record = try arena.create(MutationRecord); + record.* = .{ + ._page = page, + ._arena = arena, ._type = .characterData, ._target = target, ._attribute_name = null, ._old_value = if (obs.options.characterDataOldValue and old_value != null) - try page.arena.dupe(u8, old_value.?) + try arena.dupe(u8, old_value.?) else null, ._added_nodes = &.{}, ._removed_nodes = &.{}, ._previous_sibling = null, ._next_sibling = null, - }); + }; - try self._pending_records.append(page.arena, record); + try self._pending_records.append(self._arena, record); try page.scheduleMutationDelivery(); break; @@ -224,18 +263,22 @@ pub fn notifyChildListChange( continue; } - const record = try page._factory.create(MutationRecord{ + const arena = try self._page.getArena(.{ .debug = "MutationRecord" }); + const record = try arena.create(MutationRecord); + record.* = .{ + ._page = page, + ._arena = arena, ._type = .childList, ._target = target, ._attribute_name = null, ._old_value = null, - ._added_nodes = try page.arena.dupe(*Node, added_nodes), - ._removed_nodes = try page.arena.dupe(*Node, removed_nodes), + ._added_nodes = try arena.dupe(*Node, added_nodes), + ._removed_nodes = try arena.dupe(*Node, removed_nodes), ._previous_sibling = previous_sibling, ._next_sibling = next_sibling, - }); + }; - try self._pending_records.append(page.arena, record); + try self._pending_records.append(self._arena, record); try page.scheduleMutationDelivery(); break; @@ -263,7 +306,9 @@ pub fn deliverRecords(self: *MutationObserver, page: *Page) !void { pub const MutationRecord = struct { _type: Type, + _page: *Page, _target: *Node, + _arena: Allocator, _attribute_name: ?[]const u8, _old_value: ?[]const u8, _added_nodes: []const *Node, @@ -277,6 +322,10 @@ pub const MutationRecord = struct { characterData, }; + pub fn deinit(self: *const MutationRecord, _: bool) void { + self._page.releaseArena(self._arena); + } + pub fn getType(self: *const MutationRecord) []const u8 { return switch (self._type) { .attributes => "attributes", @@ -327,6 +376,8 @@ pub const MutationRecord = struct { pub const name = "MutationRecord"; pub const prototype_chain = bridge.prototypeChain(); pub var class_id: bridge.ClassId = undefined; + pub const weak = true; + pub const finalizer = bridge.finalizer(MutationRecord.deinit); }; pub const @"type" = bridge.accessor(MutationRecord.getType, null, .{}); @@ -348,6 +399,8 @@ pub const JsApi = struct { pub const name = "MutationObserver"; pub const prototype_chain = bridge.prototypeChain(); pub var class_id: bridge.ClassId = undefined; + pub const weak = true; + pub const finalizer = bridge.finalizer(MutationObserver.deinit); }; pub const constructor = bridge.constructor(MutationObserver.init, .{});