Merge pull request #2954 from lightpanda-io/dom-pr-13-traversal

webapi: TreeWalker, NodeFilter and NodeIterator spec conformance

src/browser/Frame.zig

@@ -51,6 +51,7 @@ const Performance = @import("webapi/Performance.zig");
 const Screen = @import("webapi/Screen.zig");
 const VisualViewport = @import("webapi/VisualViewport.zig");
 const AbstractRange = @import("webapi/AbstractRange.zig");
+const DOMNodeIterator = @import("webapi/DOMNodeIterator.zig");
 const Worker = @import("webapi/Worker.zig");
 const CSSStyleSheet = @import("webapi/css/CSSStyleSheet.zig");
 const CustomElementDefinition = @import("webapi/CustomElementDefinition.zig");
@@ -189,6 +190,9 @@ _http_owner: HttpClient.Owner = .{},
 
 // List of active live ranges (for mutation updates per DOM spec)
 _live_ranges: std.DoublyLinkedList = .{},
+// Live NodeIterators for the DOM pre-removing steps. Iterators are
+// slab-allocated (frame lifetime) and never unlinked.
+_live_node_iterators: std.DoublyLinkedList = .{},
 
 // List of open BroadcastChannels, used to route postMessage between same-named
 // channels in this frame's origin
@@ -2379,6 +2383,15 @@ const RemoveNodeOpts = struct {
     notify_observers: bool = true,
 };
 pub fn removeNode(self: *Frame, parent: *Node, child: *Node, opts: RemoveNodeOpts) void {
+    // NodeIterator pre-removing steps must run while the tree is intact.
+    if (self._live_node_iterators.first != null) {
+        var it: ?*std.DoublyLinkedList.Node = self._live_node_iterators.first;
+        while (it) |link| : (it = link.next) {
+            const iterator: *DOMNodeIterator = @fieldParentPtr("_iterator_link", link);
+            iterator.nodeWillBeRemoved(child);
+        }
+    }
+
     // Capture siblings before removing
     const previous_sibling = child.previousSibling();
     const next_sibling = child.nextSibling();

src/browser/webapi/DOMNodeIterator.zig

@@ -16,6 +16,7 @@
 // You should have received a copy of the GNU Affero General Public License
 // along with this program.  If not, see <https://www.gnu.org/licenses/>.
 
+const std = @import("std");
 const lp = @import("lightpanda");
 
 const js = @import("../js/js.zig");
@@ -35,19 +36,27 @@ _filter: NodeFilter,
 _reference_node: *Node,
 _pointer_before_reference_node: bool,
 _active: bool = false,
+_frame_loader_id: u32,
+_iterator_link: std.DoublyLinkedList.Node = .{},
 
 pub fn init(root: *Node, what_to_show: u32, filter: ?FilterOpts, frame: *Frame) !*DOMNodeIterator {
     const node_filter = try NodeFilter.init(filter);
-    return frame._factory.create(DOMNodeIterator{
+    const iterator = try frame._factory.create(DOMNodeIterator{
         ._root = root,
         ._filter = node_filter,
         ._reference_node = root,
         ._what_to_show = what_to_show,
+        ._frame_loader_id = frame._loader_id,
         ._pointer_before_reference_node = true,
     });
+    frame._live_node_iterators.append(&iterator._iterator_link);
+    return iterator;
 }
 
 pub fn deinit(self: *DOMNodeIterator, page: *Page) void {
+    if (page.findFrameByLoaderId(self._frame_loader_id)) |frame| {
+        frame._live_node_iterators.remove(&self._iterator_link);
+    }
     self._filter.deinit();
     page.factory.destroy(self);
 }
@@ -64,6 +73,43 @@ pub fn getRoot(self: *const DOMNodeIterator) *Node {
     return self._root;
 }
 
+// DOM "node iterator pre-removing steps", run while the tree still contains
+// to_be_removed.
+pub fn nodeWillBeRemoved(self: *DOMNodeIterator, to_be_removed: *Node) void {
+    if (to_be_removed.contains(self._root)) {
+        // Removing the root or one of its ancestors leaves the iterator alone.
+        return;
+    }
+    if (to_be_removed != self._reference_node and to_be_removed.contains(self._reference_node) == false) {
+        return;
+    }
+
+    if (self._pointer_before_reference_node) {
+        // The first node following to_be_removed's subtree, if any.
+        var node = to_be_removed;
+        while (node != self._root) {
+            if (node.nextSibling()) |sibling| {
+                self._reference_node = sibling;
+                return;
+            }
+            node = node.parentNode() orelse break;
+        }
+        self._pointer_before_reference_node = false;
+    }
+
+    // The node immediately preceding to_be_removed in tree order: the
+    // previous sibling's last inclusive descendant, or the parent.
+    if (to_be_removed.previousSibling()) |prev| {
+        var node = prev;
+        while (node.lastChild()) |child| {
+            node = child;
+        }
+        self._reference_node = node;
+    } else {
+        self._reference_node = to_be_removed.parentNode() orelse self._root;
+    }
+}
+
 pub fn getReferenceNode(self: *const DOMNodeIterator) *Node {
     return self._reference_node;
 }
@@ -77,7 +123,7 @@ pub fn getWhatToShow(self: *const DOMNodeIterator) u32 {
 }
 
 pub fn getFilter(self: *const DOMNodeIterator) ?FilterOpts {
-    return self._filter._original_filter;
+    return self._filter._opts;
 }
 
 pub fn nextNode(self: *DOMNodeIterator, frame: *Frame) !?*Node {

src/browser/webapi/DOMTreeWalker.zig

@@ -33,6 +33,7 @@ _root: *Node,
 _what_to_show: u32,
 _filter: NodeFilter,
 _current: *Node,
+_active: bool = false,
 
 pub fn init(root: *Node, what_to_show: u32, filter: ?FilterOpts, frame: *Frame) !*DOMTreeWalker {
     const node_filter = try NodeFilter.init(filter);
@@ -66,7 +67,7 @@ pub fn getWhatToShow(self: *const DOMTreeWalker) u32 {
 }
 
 pub fn getFilter(self: *const DOMTreeWalker) ?FilterOpts {
-    return self._filter._original_filter;
+    return self._filter._opts;
 }
 
 pub fn getCurrentNode(self: *const DOMTreeWalker) *Node {
@@ -174,27 +175,52 @@ pub fn lastChild(self: *DOMTreeWalker, frame: *Frame) !?*Node {
 }
 
 pub fn previousSibling(self: *DOMTreeWalker, frame: *Frame) !?*Node {
-    var node = self.previousSiblingOrNull(self._current);
-    while (node) |n| {
-        if (try self.acceptNode(n, frame) == NodeFilter.FILTER_ACCEPT) {
-            self._current = n;
-            return n;
-        }
-        node = self.previousSiblingOrNull(n);
-    }
-    return null;
+    return self.traverseSiblings(.previous, frame);
 }
 
 pub fn nextSibling(self: *DOMTreeWalker, frame: *Frame) !?*Node {
-    var node = self.nextSiblingOrNull(self._current);
-    while (node) |n| {
-        if (try self.acceptNode(n, frame) == NodeFilter.FILTER_ACCEPT) {
-            self._current = n;
-            return n;
+    return self.traverseSiblings(.next, frame);
+}
+
+// The spec's "traverse siblings" algorithm: a skipped (but not rejected)
+// sibling's children are still candidates, and when the siblings run out the
+// walk climbs to the parent and continues from its siblings, stopping at the
+// root or at an accepted parent.
+fn traverseSiblings(self: *DOMTreeWalker, comptime direction: enum { next, previous }, frame: *Frame) !?*Node {
+    var node = self._current;
+    if (node == self._root) return null;
+
+    while (true) {
+        var sibling: ?*Node = if (direction == .next)
+            self.nextSiblingOrNull(node)
+        else
+            self.previousSiblingOrNull(node);
+
+        while (sibling) |sib| {
+            node = sib;
+            const result = try self.acceptNode(node, frame);
+            if (result == NodeFilter.FILTER_ACCEPT) {
+                self._current = node;
+                return node;
+            }
+            sibling = if (direction == .next)
+                self.firstChildOrNull(node)
+            else
+                self.lastChildOrNull(node);
+            if (result == NodeFilter.FILTER_REJECT or sibling == null) {
+                sibling = if (direction == .next)
+                    self.nextSiblingOrNull(node)
+                else
+                    self.previousSiblingOrNull(node);
+            }
+        }
+
+        node = node.parentNode() orelse return null;
+        if (node == self._root) return null;
+        if (try self.acceptNode(node, frame) == NodeFilter.FILTER_ACCEPT) {
+            return null;
         }
-        node = self.nextSiblingOrNull(n);
     }
-    return null;
 }
 
 pub fn previousNode(self: *DOMTreeWalker, frame: *Frame) !?*Node {
@@ -309,7 +335,11 @@ pub fn nextNode(self: *DOMTreeWalker, frame: *Frame) !?*Node {
 }
 
 // Helper methods
-fn acceptNode(self: *const DOMTreeWalker, node: *Node, frame: *Frame) !i32 {
+fn acceptNode(self: *DOMTreeWalker, node: *Node, frame: *Frame) !i32 {
+    if (self._active) {
+        return error.InvalidStateError;
+    }
+
     // First check whatToShow
     if (!NodeFilter.shouldShow(node, self._what_to_show)) {
         return NodeFilter.FILTER_SKIP;
@@ -319,6 +349,8 @@ fn acceptNode(self: *const DOMTreeWalker, node: *Node, frame: *Frame) !i32 {
     // For TreeWalker, REJECT means reject node and its descendants
     // SKIP means skip node but check its descendants
     // ACCEPT means accept the node
+    self._active = true;
+    defer self._active = false;
     return try self._filter.acceptNode(node, frame.js.local.?);
 }
 

src/browser/webapi/NodeFilter.zig

@@ -21,32 +21,23 @@ const Node = @import("Node.zig");
 
 const NodeFilter = @This();
 
-_func: ?js.Function.Global,
-_original_filter: ?FilterOpts,
+_opts: ?FilterOpts,
 
 pub const FilterOpts = union(enum) {
     function: js.Function.Global,
-    object: struct {
-        pub const js_as_object = true;
-        acceptNode: js.Function.Global,
-    },
+    // Per spec, the validity of this has to be checked in each acceptNode call.
+    object: js.Object.Global,
 };
 
 pub fn init(opts_: ?FilterOpts) !NodeFilter {
-    const opts = opts_ orelse return .{ ._func = null, ._original_filter = null };
-    const func = switch (opts) {
-        .function => |func| func,
-        .object => |obj| obj.acceptNode,
-    };
-    return .{
-        ._func = func,
-        ._original_filter = opts_,
-    };
+    return .{ ._opts = opts_ };
 }
 
 pub fn deinit(self: *const NodeFilter) void {
-    if (self._func) |func| {
-        func.release();
+    const opts = self._opts orelse return;
+    switch (opts) {
+        .function => |func| func.release(),
+        .object => |obj| obj.release(),
     }
 }
 
@@ -71,8 +62,20 @@ pub const SHOW_DOCUMENT_FRAGMENT: u32 = 0x400;
 pub const SHOW_NOTATION: u32 = 0x800;
 
 pub fn acceptNode(self: *const NodeFilter, node: *Node, local: *const js.Local) !i32 {
-    const func = self._func orelse return FILTER_ACCEPT;
-    return local.toLocal(func).callRethrow(i32, .{node});
+    const opts = self._opts orelse return FILTER_ACCEPT;
+    switch (opts) {
+        .function => |func| return local.toLocal(func).callRethrow(i32, .{node}),
+        .object => |obj| {
+            // Per spec, the acceptNode member is looked up on every invocation
+            const filter_obj = obj.local(local);
+            const member = try filter_obj.get("acceptNode");
+            if (!member.isFunction()) {
+                return error.TypeError;
+            }
+            const func = js.Function{ .local = local, .handle = @ptrCast(member.handle) };
+            return func.callWithThisRethrow(i32, filter_obj, .{node});
+        },
+    }
 }
 
 pub fn shouldShow(node: *const Node, what_to_show: u32) bool {