Merge pull request #2365 from lightpanda-io/dom_version_node_frame

Track DOM version based on the node's owning frame
This commit is contained in:
Karl Seguin
2026-05-06 07:53:13 +08:00
committed by GitHub
16 changed files with 168 additions and 37 deletions

View File

@@ -2976,7 +2976,7 @@ pub fn appendNode(self: *Frame, parent: *Node, child: *Node, opts: InsertNodeOpt
}
pub fn appendAllChildren(self: *Frame, parent: *Node, target: *Node) !void {
self.domChanged();
target.bumpDomVersion(self);
const dest_connected = target.isConnected();
// Use firstChild() instead of iterator to handle cases where callbacks
@@ -2991,7 +2991,7 @@ pub fn appendAllChildren(self: *Frame, parent: *Node, target: *Node) !void {
}
pub fn insertAllChildrenBefore(self: *Frame, fragment: *Node, parent: *Node, ref_node: *Node) !void {
self.domChanged();
parent.bumpDomVersion(self);
const dest_connected = parent.isConnected();
// Use firstChild() instead of iterator to handle cases where callbacks

View File

@@ -0,0 +1,30 @@
<!DOCTYPE html>
<head></head>
<body></body>
<script src="../../../testing.js"></script>
<!--
Per the HTML spec, the "already started" flag must be propagated when a script
element is cloned. A clone of a script that has already run must NOT run again
when inserted into a document. This guards a regression where cloneNode lost
the flag and re-ran every cloned <script> on each insertion.
-->
<script id=cloned_inline_does_not_rerun>
window.cloned_inline_count = 0;
const c1_original = document.createElement('script');
c1_original.textContent = 'window.cloned_inline_count++;';
document.head.appendChild(c1_original);
testing.expectEqual(1, window.cloned_inline_count);
// Clone the executed script and insert the clone. The clone must not re-run.
const c1_clone = c1_original.cloneNode(true);
document.head.appendChild(c1_clone);
testing.expectEqual(1, window.cloned_inline_count);
// A deep clone of a wrapper containing the script must keep the flag too.
const c1_wrapper = document.createElement('div');
c1_wrapper.appendChild(c1_original.cloneNode(true));
document.body.appendChild(c1_wrapper);
testing.expectEqual(1, window.cloned_inline_count);
</script>

View File

@@ -0,0 +1,58 @@
<!DOCTYPE html>
<head></head>
<body>
<script src="../testing.js"></script>
<!--
Live collections (NodeList from .childNodes, HTMLCollection, NodeLive variants
like getElementsByTagName) cache state and invalidate on the owning frame's
DOM version. Cross-realm callers — parent JS reading `iframe.x.childNodes` —
must still see mutations applied through the iframe's frame, even though the
caller's own frame.version doesn't bump on those mutations.
This guards a regression where the cache was tied to the calling frame and
returned stale results after iframe-side mutations.
-->
<iframe id=if_collection src="support/cross_realm_collection.html"></iframe>
<script id=childNodes_invalidates_after_iframe_mutation>
testing.onload(() => {
const idoc = document.getElementById('if_collection').contentDocument;
const iwin = document.getElementById('if_collection').contentWindow;
const testDiv = idoc.querySelector('#test');
const p0 = idoc.querySelector('#p0');
const p1 = idoc.querySelector('#p1');
// Prime the parent-realm view of the live NodeList.
const cn = testDiv.childNodes;
testing.expectEqual(3, cn.length);
testing.expectEqual(p0, cn[0]);
// Mutate the iframe's DOM through parent-realm code. The mutation only
// bumps the iframe frame's version; the parent's stays unchanged. The
// cached collection must still reflect the new state.
testDiv.removeChild(p0);
testing.expectEqual(2, cn.length);
testing.expectEqual(p1, cn[0]);
testing.expectEqual(undefined, cn[2]);
});
</script>
<script id=getElementsByTagName_invalidates_after_iframe_mutation>
testing.onload(() => {
const idoc = document.getElementById('if_collection').contentDocument;
const ps = idoc.getElementsByTagName('p');
const seen_initial = ps.length;
// Append a new <p> through the iframe's document.
const fresh = idoc.createElement('p');
fresh.id = 'pfresh';
idoc.querySelector('#test').appendChild(fresh);
testing.expectEqual(seen_initial + 1, ps.length);
testing.expectEqual(fresh, ps[ps.length - 1]);
});
</script>
</body>

View File

@@ -0,0 +1,3 @@
<!DOCTYPE html>
<head></head>
<body><div id=test><p id=p0>p0</p><p id=p1>p1</p><p id=p2>p2</p></div></body>

View File

@@ -488,8 +488,8 @@ pub fn importNode(_: *const Document, node: *Node, deep_: ?bool, frame: *Frame)
pub fn append(self: *Document, nodes: []const Node.NodeOrText, frame: *Frame) !void {
try validateDocumentNodes(self, nodes, false);
frame.domChanged();
const parent = self.asNode();
parent.bumpDomVersion(frame);
const parent_is_connected = parent.isConnected();
for (nodes) |node_or_text| {
@@ -513,8 +513,8 @@ pub fn append(self: *Document, nodes: []const Node.NodeOrText, frame: *Frame) !v
pub fn prepend(self: *Document, nodes: []const Node.NodeOrText, frame: *Frame) !void {
try validateDocumentNodes(self, nodes, false);
frame.domChanged();
const parent = self.asNode();
parent.bumpDomVersion(frame);
const parent_is_connected = parent.isConnected();
var i = nodes.len;
@@ -736,7 +736,7 @@ fn writeInternal(self: *Document, text: []const []const u8, append_newline: bool
insert_after = child;
}
frame.domChanged();
parent.bumpDomVersion(frame);
self._write_insertion_point = children_to_insert.getLast();
}

View File

@@ -154,7 +154,7 @@ pub fn getInnerHTML(self: *DocumentFragment, writer: *std.Io.Writer, frame: *Fra
pub fn setInnerHTML(self: *DocumentFragment, html: []const u8, frame: *Frame) !void {
const parent = self.asNode();
frame.domChanged();
parent.bumpDomVersion(frame);
var it = parent.childrenIterator();
while (it.next()) |child| {
frame.removeNode(parent, child, .{ .will_be_reconnected = false });

View File

@@ -467,7 +467,7 @@ pub fn setOuterHTML(self: *Element, html: []const u8, frame: *Frame) !void {
const node = self.asNode();
const parent = node._parent orelse return;
frame.domChanged();
parent.bumpDomVersion(frame);
if (html.len > 0) {
const fragment = (try Node.DocumentFragment.init(frame)).asNode();
try frame.parseHtmlAsChildren(fragment, html);
@@ -493,7 +493,7 @@ pub fn setInnerHTML(self: *Element, html: []const u8, frame: *Frame) !void {
// fires disconnectedCallback for custom elements, which can mutate
// the child list and dangle any cached next-pointer the iterator
// would otherwise hold.
frame.domChanged();
parent.bumpDomVersion(frame);
while (parent.firstChild()) |child| {
frame.removeNode(parent, child, .{ .will_be_reconnected = false });
}
@@ -879,10 +879,9 @@ pub fn replaceChildren(self: *Element, nodes: []const Node.NodeOrText, frame: *F
}
pub fn replaceWith(self: *Element, nodes: []const Node.NodeOrText, frame: *Frame) !void {
frame.domChanged();
const ref_node = self.asNode();
const parent = ref_node._parent orelse return;
parent.bumpDomVersion(frame);
const parent_is_connected = parent.isConnected();
@@ -919,9 +918,9 @@ pub fn replaceWith(self: *Element, nodes: []const Node.NodeOrText, frame: *Frame
}
pub fn remove(self: *Element, frame: *Frame) void {
frame.domChanged();
const node = self.asNode();
const parent = node._parent orelse return;
parent.bumpDomVersion(frame);
frame.removeNode(parent, node, .{ .will_be_reconnected = false });
}

View File

@@ -226,7 +226,7 @@ pub fn appendChild(self: *Node, child: *Node, frame: *Frame) !*Node {
try validateNodeInsertion(self, child);
frame.domChanged();
self.bumpDomVersion(frame);
// If the child is currently connected, and if its new parent is connected,
// then we can remove + add a bit more efficiently (we don't have to fully
@@ -512,11 +512,30 @@ pub fn ownerDocument(self: *const Node, frame: *const Frame) ?*Document {
return frame.document;
}
// Returns the Frame that owns this node's tree. Used to tie cached state of
// "live" collections (NodeList, HTMLCollection, etc.) to the right frame's DOM
// version: cross-realm callers must invalidate based on mutations through the
// node's owning frame, not the caller's frame.
//
// Falls back to `default` when the node has no associated document yet (e.g.,
// freshly created and detached) or its document has no frame.
pub fn ownerFrame(self: *const Node, default: *Frame) *Frame {
if (self._type == .document) {
return self._type.document._frame orelse default;
}
const doc = self.ownerDocument(default) orelse return default;
return doc._frame orelse default;
}
// Tells the owning frame that this node's subtree has changed. Use this from
// mutation paths instead of `frame.domChanged()` so cross-realm mutations
// (e.g., parent JS mutating an iframe's DOM) bump the iframe's version, not
// the caller's. Otherwise, live collections that key off the owning frame's
// version won't see the change and will return stale cached state.
pub fn bumpDomVersion(self: *const Node, default: *Frame) void {
self.ownerFrame(default).domChanged();
}
pub const ResolveURLOpts = struct {
allocator: ?Allocator = null,
};
@@ -548,7 +567,7 @@ pub fn removeChild(self: *Node, child: *Node, frame: *Frame) !*Node {
var it = self.childrenIterator();
while (it.next()) |n| {
if (n == child) {
frame.domChanged();
self.bumpDomVersion(frame);
frame.removeNode(self, child, .{ .will_be_reconnected = false });
return child;
}
@@ -563,7 +582,7 @@ pub fn insertBefore(self: *Node, new_node: *Node, ref_node_: ?*Node, frame: *Fra
// special case: if nodes are the same, ignore the change.
if (new_node == ref_node_) {
frame.domChanged();
self.bumpDomVersion(frame);
if (frame.hasMutationObservers()) {
const parent = new_node._parent.?;
@@ -594,7 +613,7 @@ pub fn insertBefore(self: *Node, new_node: *Node, ref_node_: ?*Node, frame: *Fra
const parent_owner = self.ownerDocument(frame) orelse self.as(Document);
const adopting_to_new_document = child_owner != null and child_owner.? != parent_owner;
frame.domChanged();
self.bumpDomVersion(frame);
const will_be_reconnected = self.isConnected() and !adopting_to_new_document;
if (new_node._parent) |parent| {
frame.removeNode(parent, new_node, .{ .will_be_reconnected = will_be_reconnected });
@@ -1049,7 +1068,7 @@ pub fn replaceChildren(self: *Node, nodes: []const NodeOrText, frame: *Frame) !v
}
}
frame.domChanged();
self.bumpDomVersion(frame);
// Remove all existing children
var it = self.childrenIterator();

View File

@@ -374,7 +374,7 @@ pub fn deleteContents(self: *Range, frame: *Frame) !void {
if (self._proto.getCollapsed()) {
return;
}
frame.domChanged();
self._proto._start_container.bumpDomVersion(frame);
// Simple case: same container
if (self._proto._start_container == self._proto._end_container) {

View File

@@ -34,8 +34,11 @@ _arena: std.mem.Allocator,
_last_index: usize,
_last_length: ?u32,
_last_node: ?*std.DoublyLinkedList.Node,
// Version observed on `_owning_frame` the last time we refreshed our cache.
// Compare against `_owning_frame.version` to detect DOM mutations.
_cached_version: usize,
_node: *Node,
_owning_frame: *Frame,
pub const KeyIterator = GenericIterator(Iterator, "0");
pub const ValueIterator = GenericIterator(Iterator, "1");
@@ -45,6 +48,8 @@ pub fn init(node: *Node, frame: *Frame) !*ChildNodes {
const arena = try frame.getArena(.small, "ChildNodes");
errdefer frame.releaseArena(arena);
const owning_frame = node.ownerFrame(frame);
const self = try arena.create(ChildNodes);
self.* = .{
._node = node,
@@ -52,7 +57,8 @@ pub fn init(node: *Node, frame: *Frame) !*ChildNodes {
._last_index = 0,
._last_node = null,
._last_length = null,
._cached_version = frame.version,
._cached_version = owning_frame.version,
._owning_frame = owning_frame,
};
return self;
}
@@ -61,8 +67,8 @@ pub fn deinit(self: *const ChildNodes, page: *Page) void {
page.releaseArena(self._arena);
}
pub fn length(self: *ChildNodes, frame: *Frame) !u32 {
if (self.versionCheck(frame)) {
pub fn length(self: *ChildNodes, _: *Frame) !u32 {
if (self.versionCheck()) {
if (self._last_length) |cached_length| {
return cached_length;
}
@@ -75,16 +81,16 @@ pub fn length(self: *ChildNodes, frame: *Frame) !u32 {
return len;
}
pub fn getAtIndex(self: *ChildNodes, index: usize, frame: *Frame) !?*Node {
_ = self.versionCheck(frame);
pub fn getAtIndex(self: *ChildNodes, index: usize, _: *Frame) !?*Node {
_ = self.versionCheck();
var current = self._last_index;
var node: ?*std.DoublyLinkedList.Node = null;
if (index < current) {
if (index < current or self._last_node == null) {
current = 0;
node = self.first() orelse return null;
} else {
node = self._last_node orelse self.first() orelse return null;
node = self._last_node;
}
defer self._last_index = current;
@@ -116,8 +122,8 @@ pub fn entries(self: *ChildNodes, frame: *Frame) !*EntryIterator {
return .init(.{ .list = self }, frame);
}
fn versionCheck(self: *ChildNodes, frame: *Frame) bool {
const current = frame.version;
fn versionCheck(self: *ChildNodes) bool {
const current = self._owning_frame.version;
if (current == self._cached_version) {
return true;
}

View File

@@ -32,19 +32,23 @@ _tw: TreeWalker.FullExcludeSelf,
_last_index: usize,
_last_length: ?u32,
_cached_version: usize,
_owning_frame: *Frame,
pub fn init(root: *Node, frame: *Frame) HTMLAllCollection {
const owning_frame = root.ownerFrame(frame);
return .{
._last_index = 0,
._last_length = null,
._tw = TreeWalker.FullExcludeSelf.init(root, .{}),
._cached_version = frame.version,
._cached_version = owning_frame.version,
._owning_frame = owning_frame,
};
}
fn versionCheck(self: *HTMLAllCollection, frame: *const Frame) bool {
if (self._cached_version != frame.version) {
self._cached_version = frame.version;
fn versionCheck(self: *HTMLAllCollection, _: *const Frame) bool {
const current = self._owning_frame.version;
if (self._cached_version != current) {
self._cached_version = current;
self._last_index = 0;
self._last_length = null;
self._tw.reset();

View File

@@ -99,16 +99,19 @@ pub fn NodeLive(comptime mode: Mode) type {
_last_index: usize,
_last_length: ?u32,
_cached_version: usize,
_owning_frame: *Frame,
const Self = @This();
pub fn init(root: *Node, filter: Filter, frame: *Frame) Self {
const owning_frame = root.ownerFrame(frame);
return .{
._last_index = 0,
._last_length = null,
._filter = filter,
._tw = TW.init(root, .{}),
._cached_version = frame.version,
._cached_version = owning_frame.version,
._owning_frame = owning_frame,
};
}
@@ -342,8 +345,8 @@ pub fn NodeLive(comptime mode: Mode) type {
};
}
fn versionCheck(self: *Self, frame: *const Frame) bool {
const current = frame.version;
fn versionCheck(self: *Self, _: *const Frame) bool {
const current = self._owning_frame.version;
if (current == self._cached_version) {
return true;
}

View File

@@ -229,7 +229,7 @@ pub const List = struct {
};
try frame.addElementId(parent, element, entry._value.str());
}
frame.domChanged();
element.asNode().bumpDomVersion(frame);
frame.attributeChange(element, result.normalized, entry._value, old_value);
return entry;
}
@@ -292,7 +292,7 @@ pub const List = struct {
frame.removeElementId(element, entry._value.str());
}
frame.domChanged();
element.asNode().bumpDomVersion(frame);
frame.attributeRemove(element, result.normalized, old_value);
_ = frame._attribute_lookup.remove(@intFromPtr(entry));
self._list.remove(&entry._node);

View File

@@ -273,7 +273,7 @@ pub fn setInnerText(self: *HtmlElement, text: []const u8, frame: *Frame) !void {
const parent = self.asElement().asNode();
// Remove all existing children
frame.domChanged();
parent.bumpDomVersion(frame);
var it = parent.childrenIterator();
while (it.next()) |child| {
frame.removeNode(parent, child, .{ .will_be_reconnected = false });

View File

@@ -80,7 +80,7 @@ pub fn setSelected(self: *Option, selected: bool, frame: *Frame) !void {
// TODO: When setting selected=true, may need to unselect other options
// in the parent <select> if it doesn't have multiple attribute
self._selected = selected;
frame.domChanged();
self.asElement().asNode().bumpDomVersion(frame);
}
pub fn getDefaultSelected(self: *const Option) bool {

View File

@@ -151,6 +151,15 @@ pub const Build = struct {
const element = self.asElement();
self._src = element.getAttributeSafe(comptime .wrap("src")) orelse "";
}
// Per the HTML spec, the "already started" flag must be propagated to the
// clone so that re-inserting a cloned <script> doesn't run it again.
pub fn cloned(source_element: *Element, cloned_element: *Element, _: *Frame) !void {
const source = source_element.as(Script);
const clone = cloned_element.as(Script);
clone._executed = source._executed;
clone._force_async = source._force_async;
}
};
const testing = @import("../../../../testing.zig");