Merge pull request #2632 from lightpanda-io/remove_children

Remove the fancy Children
This commit is contained in:
Karl Seguin
2026-06-04 14:56:45 +08:00
committed by GitHub
6 changed files with 23 additions and 95 deletions

View File

@@ -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;
}

View File

@@ -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;

View File

@@ -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;

View File

@@ -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),
};
}

View File

@@ -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()),
};
}
};

View File

@@ -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 {