From bb773c6c13a6da8c611fcf78ebafc163bd82ee0c Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Tue, 24 Feb 2026 12:57:05 +0800 Subject: [PATCH] All frames must share the same Arena/Factory This is a bit sad, but at least for now, all frames must share the same page.arena and page.factory (they still get their own v8::Context and call_arena). Consider this case (from WPT /css/cssom-view/scrollingElement.html): ``` let nonQuirksFrame = document.createElement("iframe"); nonQuirksFrame.onload = this.step_func_done(function() { var nonQuirksDoc = nonQuirksFrame.contentDocument; nonQuirksDoc.removeChild(nonQuirksDoc.documentElement); }); nonQuirksFrame.src = URL.createObjectURL(new Blob([``], { type: "text/html" })); document.body.append(nonQuirksFrame); ``` We have the root page (p0) and the frame page (p1). When the frame (p1) is created, it's [currently] given its own arena/factory and parses the page. Those nodes are created with p1's factory. The onload callback executes on p0, so when we call removeChild that's executing with p0's arena/factory and tries to release memory using that factory - which is NOT the factory that created them. A better approach might be that _memory_ operations aren't tied to the current calling context, but rather the owning document's page. But: 1 - That would mean we have 2 execution contexts: the v8 context where the code is running, and the memory context that owns the code 2 - Nodes can be disconnected, what page should we use? 3 - Some operations can behave across frames, p0 could adoptNode on p1, so we'd have to carefully use p1's factory when cleaning up and re-create the node in p0's factory. So much hassle. Using a shared factory/arena solves these problems at the cost of bloat - when a frame goes away or navigates, we can't free its old memory. At some point, we should fix that. But I don't have a quick and easy solution, and sharing the arena/factory is _really_ quick and easy. --- src/browser/Factory.zig | 183 ++++++++++++++++++---------------------- src/browser/Page.zig | 16 ++-- 2 files changed, 94 insertions(+), 105 deletions(-) diff --git a/src/browser/Factory.zig b/src/browser/Factory.zig index 44bfcb1e..21d4d839 100644 --- a/src/browser/Factory.zig +++ b/src/browser/Factory.zig @@ -42,12 +42,93 @@ const Allocator = std.mem.Allocator; const IS_DEBUG = builtin.mode == .Debug; const assert = std.debug.assert; +// Shared across all frames of a Page. const Factory = @This(); -_page: *Page, _arena: Allocator, _slab: SlabAllocator, +pub fn init(arena: Allocator) !*Factory { + const self = try arena.create(Factory); + self.* = .{ + ._arena = arena, + ._slab = SlabAllocator.init(arena, 128), + }; + return self; +} + +// this is a root object +pub fn eventTarget(self: *Factory, child: anytype) !*@TypeOf(child) { + const allocator = self._slab.allocator(); + const chain = try PrototypeChain( + &.{ EventTarget, @TypeOf(child) }, + ).allocate(allocator); + + const event_ptr = chain.get(0); + event_ptr.* = .{ + ._type = unionInit(EventTarget.Type, chain.get(1)), + }; + chain.setLeaf(1, child); + + return chain.get(1); +} + +pub fn standaloneEventTarget(self: *Factory, child: anytype) !*EventTarget { + const allocator = self._slab.allocator(); + const et = try allocator.create(EventTarget); + et.* = .{ ._type = unionInit(EventTarget.Type, child) }; + return et; +} + +// this is a root object +pub fn event(_: *const Factory, arena: Allocator, typ: String, child: anytype) !*@TypeOf(child) { + const chain = try PrototypeChain( + &.{ Event, @TypeOf(child) }, + ).allocate(arena); + + // Special case: Event has a _type_string field, so we need manual setup + const event_ptr = chain.get(0); + event_ptr.* = try eventInit(arena, typ, chain.get(1)); + chain.setLeaf(1, child); + + return chain.get(1); +} + +pub fn uiEvent(_: *const Factory, arena: Allocator, typ: String, child: anytype) !*@TypeOf(child) { + const chain = try PrototypeChain( + &.{ Event, UIEvent, @TypeOf(child) }, + ).allocate(arena); + + // Special case: Event has a _type_string field, so we need manual setup + const event_ptr = chain.get(0); + event_ptr.* = try eventInit(arena, typ, chain.get(1)); + chain.setMiddle(1, UIEvent.Type); + chain.setLeaf(2, child); + + return chain.get(2); +} + +pub fn mouseEvent(_: *const Factory, arena: Allocator, typ: String, mouse: MouseEvent, child: anytype) !*@TypeOf(child) { + const chain = try PrototypeChain( + &.{ Event, UIEvent, MouseEvent, @TypeOf(child) }, + ).allocate(arena); + + // Special case: Event has a _type_string field, so we need manual setup + const event_ptr = chain.get(0); + event_ptr.* = try eventInit(arena, typ, chain.get(1)); + chain.setMiddle(1, UIEvent.Type); + + // Set MouseEvent with all its fields + const mouse_ptr = chain.get(2); + mouse_ptr.* = mouse; + mouse_ptr._proto = chain.get(1); + mouse_ptr._type = unionInit(MouseEvent.Type, chain.get(3)); + + chain.setLeaf(3, child); + + return chain.get(3); +} + fn PrototypeChain(comptime types: []const type) type { return struct { const Self = @This(); @@ -151,86 +232,6 @@ fn AutoPrototypeChain(comptime types: []const type) type { }; } -pub fn init(arena: Allocator, page: *Page) Factory { - return .{ - ._page = page, - ._arena = arena, - ._slab = SlabAllocator.init(arena, 128), - }; -} - -// this is a root object -pub fn eventTarget(self: *Factory, child: anytype) !*@TypeOf(child) { - const allocator = self._slab.allocator(); - const chain = try PrototypeChain( - &.{ EventTarget, @TypeOf(child) }, - ).allocate(allocator); - - const event_ptr = chain.get(0); - event_ptr.* = .{ - ._type = unionInit(EventTarget.Type, chain.get(1)), - }; - chain.setLeaf(1, child); - - return chain.get(1); -} - -pub fn standaloneEventTarget(self: *Factory, child: anytype) !*EventTarget { - const allocator = self._slab.allocator(); - const et = try allocator.create(EventTarget); - et.* = .{ ._type = unionInit(EventTarget.Type, child) }; - return et; -} - -// this is a root object -pub fn event(_: *const Factory, arena: Allocator, typ: String, child: anytype) !*@TypeOf(child) { - const chain = try PrototypeChain( - &.{ Event, @TypeOf(child) }, - ).allocate(arena); - - // Special case: Event has a _type_string field, so we need manual setup - const event_ptr = chain.get(0); - event_ptr.* = try eventInit(arena, typ, chain.get(1)); - chain.setLeaf(1, child); - - return chain.get(1); -} - -pub fn uiEvent(_: *const Factory, arena: Allocator, typ: String, child: anytype) !*@TypeOf(child) { - const chain = try PrototypeChain( - &.{ Event, UIEvent, @TypeOf(child) }, - ).allocate(arena); - - // Special case: Event has a _type_string field, so we need manual setup - const event_ptr = chain.get(0); - event_ptr.* = try eventInit(arena, typ, chain.get(1)); - chain.setMiddle(1, UIEvent.Type); - chain.setLeaf(2, child); - - return chain.get(2); -} - -pub fn mouseEvent(_: *const Factory, arena: Allocator, typ: String, mouse: MouseEvent, child: anytype) !*@TypeOf(child) { - const chain = try PrototypeChain( - &.{ Event, UIEvent, MouseEvent, @TypeOf(child) }, - ).allocate(arena); - - // Special case: Event has a _type_string field, so we need manual setup - const event_ptr = chain.get(0); - event_ptr.* = try eventInit(arena, typ, chain.get(1)); - chain.setMiddle(1, UIEvent.Type); - - // Set MouseEvent with all its fields - const mouse_ptr = chain.get(2); - mouse_ptr.* = mouse; - mouse_ptr._proto = chain.get(1); - mouse_ptr._type = unionInit(MouseEvent.Type, chain.get(3)); - - chain.setLeaf(3, child); - - return chain.get(3); -} - fn eventInit(arena: Allocator, typ: String, value: anytype) !Event { // Round to 2ms for privacy (browsers do this) const raw_timestamp = @import("../datetime.zig").milliTimestamp(.monotonic); @@ -383,7 +384,7 @@ pub fn destroy(self: *Factory, value: anytype) void { } if (comptime @hasField(S, "_proto")) { - self.destroyChain(value, true, 0, std.mem.Alignment.@"1"); + self.destroyChain(value, 0, std.mem.Alignment.@"1"); } else { self.destroyStandalone(value); } @@ -397,7 +398,6 @@ pub fn destroyStandalone(self: *Factory, value: anytype) void { fn destroyChain( self: *Factory, value: anytype, - comptime first: bool, old_size: usize, old_align: std.mem.Alignment, ) void { @@ -409,23 +409,8 @@ fn destroyChain( const new_size = current_size + @sizeOf(S); const new_align = std.mem.Alignment.max(old_align, std.mem.Alignment.of(S)); - // This is initially called from a deinit. We don't want to call that - // same deinit. So when this is the first time destroyChain is called - // we don't call deinit (because we're in that deinit) - if (!comptime first) { - // But if it isn't the first time - if (@hasDecl(S, "deinit")) { - // And it has a deinit, we'll call it - switch (@typeInfo(@TypeOf(S.deinit)).@"fn".params.len) { - 1 => value.deinit(), - 2 => value.deinit(self._page), - else => @compileLog(@typeName(S) ++ " has an invalid deinit function"), - } - } - } - if (@hasField(S, "_proto")) { - self.destroyChain(value._proto, false, new_size, new_align); + self.destroyChain(value._proto, new_size, new_align); } else { // no proto so this is the head of the chain. // we use this as the ptr to the start of the chain. diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 699e5cb0..08ef2e7d 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -173,7 +173,7 @@ _upgrading_element: ?*Node = null, _undefined_custom_elements: std.ArrayList(*Element.Html.Custom) = .{}, // for heap allocations and managing WebAPI objects -_factory: Factory, +_factory: *Factory, _load_state: LoadState = .waiting, @@ -247,14 +247,15 @@ pub fn init(self: *Page, id: u32, session: *Session, parent: ?*Page) !void { } const browser = session.browser; const arena_pool = browser.arena_pool; - const page_arena = try arena_pool.acquire(); - errdefer arena_pool.release(page_arena); + + const page_arena = if (parent) |p| p.arena else try arena_pool.acquire(); + errdefer if (parent == null) arena_pool.release(page_arena); + + var factory = if (parent) |p| p._factory else try Factory.init(page_arena); const call_arena = try arena_pool.acquire(); errdefer arena_pool.release(call_arena); - var factory = Factory.init(page_arena, self); - const document = (try factory.document(Node.Document.HTMLDocument{ ._proto = undefined, })).asDocument(); @@ -355,7 +356,10 @@ pub fn deinit(self: *Page) void { } self.arena_pool.release(self.call_arena); - self.arena_pool.release(self.arena); + + if (self.parent == null) { + self.arena_pool.release(self.arena); + } } pub fn base(self: *const Page) [:0]const u8 {