diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index ffafef0d..095999c0 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -3038,24 +3038,11 @@ pub fn removeNode(self: *Frame, parent: *Node, child: *Node, opts: RemoveNodeOpt null; const children = parent._children.?; - switch (children.*) { - .one => |n| { - lp.assert(n == child, "Frame.removeNode.one", .{}); - parent._children = null; - self._factory.destroy(children); - }, - .list => |list| { - list.remove(&child._child_link); - - // Should not be possible to get a child list with a single node. - // While it doesn't cause any problems, it indicates an bug in the - // code as these should always be represented as .{.one = node} - const first = list.first.?; - if (first.next == null) { - children.* = .{ .one = Node.linkToNode(first) }; - self._factory.destroy(list); - } - }, + children.remove(&child._child_link); + if (children.first == null) { + // last child removed; drop the list so a childless node holds no allocation + parent._children = null; + self._factory.destroy(children); } // grab this before we null the parent const was_connected = child.isConnected(); @@ -3198,44 +3185,23 @@ pub fn _insertNodeRelative(self: *Frame, comptime from_parser: bool, parent: *No lp.assert(child._parent == null, "Frame.insertNodeRelative parent", .{}); - const children = blk: { - // expand parent._children so that it can take another child - if (parent._children) |c| { - switch (c.*) { - .list => {}, - .one => |node| { - const list = try self._factory.create(std.DoublyLinkedList{}); - list.append(&node._child_link); - c.* = .{ .list = list }; - }, - } - break :blk c; - } else { - const Children = @import("webapi/children.zig").Children; - const c = try self._factory.create(Children{ .one = child }); - parent._children = c; - break :blk c; - } + const children = parent._children orelse blk: { + const list = try self._factory.create(std.DoublyLinkedList{}); + parent._children = list; + break :blk list; }; switch (relative) { - .append => switch (children.*) { - .one => {}, // already set in the expansion above - .list => |list| list.append(&child._child_link), - }, + .append => children.append(&child._child_link), .after => |ref_node| { // caller should have made sure this was the case lp.assert(ref_node._parent.? == parent, "Frame.insertNodeRelative after", .{ .url = self.url }); - // if ref_node is in parent, and expanded _children above to - // accommodate another child, then `children` must be a list - children.list.insertAfter(&ref_node._child_link, &child._child_link); + children.insertAfter(&ref_node._child_link, &child._child_link); }, .before => |ref_node| { // caller should have made sure this was the case lp.assert(ref_node._parent.? == parent, "Frame.insertNodeRelative before", .{ .url = self.url }); - // if ref_node is in parent, and expanded _children above to - // accommodate another child, then `children` must be a list - children.list.insertBefore(&ref_node._child_link, &child._child_link); + children.insertBefore(&ref_node._child_link, &child._child_link); }, } child._parent = parent; @@ -3603,7 +3569,7 @@ fn parseHtmlAsChildrenInner(self: *Frame, node: *Node, html: []const u8, allow_d // we expect, and nodes might be altogether removed. We deal with this in a // few different places, but always the same way: leave it as-is. const children = node._children orelse return; - const first = children.first(); + const first = Node.linkToNode(children.first.?); if (first.is(Element.Html.Html) == null) { return; } diff --git a/src/browser/webapi/DOMNodeIterator.zig b/src/browser/webapi/DOMNodeIterator.zig index adc59633..da46691f 100644 --- a/src/browser/webapi/DOMNodeIterator.zig +++ b/src/browser/webapi/DOMNodeIterator.zig @@ -153,7 +153,7 @@ fn filterNode(self: *const DOMNodeIterator, node: *Node, frame: *Frame) !i32 { fn getNextInTree(self: *const DOMNodeIterator, node: *Node) ?*Node { // Depth-first traversal within the root subtree if (node._children) |children| { - return children.first(); + return Node.linkToNode(children.first.?); } var current = node; diff --git a/src/browser/webapi/Document.zig b/src/browser/webapi/Document.zig index 93f3dc38..cc6e4a75 100644 --- a/src/browser/webapi/Document.zig +++ b/src/browser/webapi/Document.zig @@ -806,7 +806,7 @@ fn writeInternal(self: *Document, text: []const []const u8, append_newline: bool // Extract children from wrapper HTML element (html5ever wraps fragments) // https://github.com/servo/html5ever/issues/583 const children = fragment_node._children orelse return; - const first = children.first(); + const first = Node.linkToNode(children.first.?); // Collect all children to insert (to avoid iterator invalidation) var children_to_insert: std.ArrayList(*Node) = .empty; diff --git a/src/browser/webapi/Node.zig b/src/browser/webapi/Node.zig index c4e0fe0a..f7cb6688 100644 --- a/src/browser/webapi/Node.zig +++ b/src/browser/webapi/Node.zig @@ -31,7 +31,6 @@ pub const CData = @import("CData.zig"); pub const Element = @import("Element.zig"); pub const Document = @import("Document.zig"); pub const HTMLDocument = @import("HTMLDocument.zig"); -pub const Children = @import("children.zig").Children; pub const DocumentFragment = @import("DocumentFragment.zig"); pub const DocumentType = @import("DocumentType.zig"); pub const ShadowRoot = @import("ShadowRoot.zig"); @@ -46,7 +45,9 @@ const Node = @This(); _type: Type, _proto: *EventTarget, _parent: ?*Node = null, -_children: ?*Children = null, +// A node with no children leaves this null (no allocation). Otherwise it +// points to a heap-allocated intrusive list of the node's `_child_link`s. +_children: ?*LinkedList = null, _child_link: LinkedList.Node = .{}, // Lookup for nodes that have a different owner document than frame.document @@ -171,12 +172,12 @@ pub fn findAdjacentNodes(self: *Node, position: []const u8) !struct { *Node, ?*N pub fn firstChild(self: *const Node) ?*Node { const children = self._children orelse return null; - return children.first(); + return linkToNodeOrNull(children.first); } pub fn lastChild(self: *const Node) ?*Node { const children = self._children orelse return null; - return children.last(); + return linkToNodeOrNull(children.last); } pub fn nextSibling(self: *const Node) ?*Node { @@ -714,7 +715,7 @@ pub fn childrenIterator(self: *Node) NodeIterator { }; return .{ - .node = children.first(), + .node = linkToNodeOrNull(children.first), }; } diff --git a/src/browser/webapi/children.zig b/src/browser/webapi/children.zig deleted file mode 100644 index fe920e26..00000000 --- a/src/browser/webapi/children.zig +++ /dev/null @@ -1,39 +0,0 @@ -const std = @import("std"); - -const Node = @import("Node.zig"); - -const LinkedList = std.DoublyLinkedList; - -// Our node._children is of type ?*NodeList. The extra (extra) indirection is to -// keep memory size down. -// First, a lot of nodes have no children. For these nodes, `?*NodeList = null` -// will take 8 bytes and require no allocations (because an optional pointer in -// Zig uses the address 0 to represent null, rather than a separate field). -// Second, a lot of nodes will have one child. For these nodes, we'll also only -// use 8 bytes, because @sizeOf(NodeList) == 8. This is the reason the -// list: *LinkedList is behind a pointer. -pub const Children = union(enum) { - one: *Node, - list: *LinkedList, - - pub fn first(self: *const Children) *Node { - return switch (self.*) { - .one => |n| n, - .list => |list| Node.linkToNode(list.first.?), - }; - } - - pub fn last(self: *const Children) *Node { - return switch (self.*) { - .one => |n| n, - .list => |list| Node.linkToNode(list.last.?), - }; - } - - pub fn len(self: *const Children) u32 { - return switch (self.*) { - .one => 1, - .list => |list| @intCast(list.len()), - }; - } -}; diff --git a/src/browser/webapi/collections/ChildNodes.zig b/src/browser/webapi/collections/ChildNodes.zig index 115b8829..08e4096a 100644 --- a/src/browser/webapi/collections/ChildNodes.zig +++ b/src/browser/webapi/collections/ChildNodes.zig @@ -69,7 +69,7 @@ pub fn length(self: *ChildNodes, frame: *const Frame) !u32 { const children = self._node._children orelse return 0; // O(N) - const len = children.len(); + const len: u32 = @intCast(children.len()); self._last_length = len; return len; } @@ -100,7 +100,7 @@ pub fn getAtIndex(self: *ChildNodes, index: usize, frame: *const Frame) !?*Node } pub fn first(self: *const ChildNodes) ?*std.DoublyLinkedList.Node { - return &(self._node._children orelse return null).first()._child_link; + return (self._node._children orelse return null).first; } pub fn keys(self: *ChildNodes, frame: *Frame) !*KeyIterator {