mirror/browser
1
0
Fork 0
mirror of https://github.com/lightpanda-io/browser.git synced 2026-06-11 01:25:53 -04:00
Code Issues Packages Projects Releases Wiki Activity

Track DOM version based on the node's owning frame

Browse Source 
This fixes 2 separate issues, driven by the [very] broken
/dom/ranges/Range-insertNode.html WPT test.

The first, and simplest, is that when a script element is cloned, its executing
_executed and _force_async flags need to be copied. This prevents double-
execution.

The second is more complicated: dom version tracking (as a caching mechanism
used in various live collection) has to be against the node's owning frame, not
the frame that's executing the JavaScript. This includes both the version
checking and version change (on dom mutation). This introduces a relatively
expensive node -> document -> frame on every mutation and cache read. This is
potentially worth optimizing somehow, in a follow up PR. Two options come to
mind:

1 - Track a single dom version on the Page. Mutation in frame1 invalidates
    frame2.
2 - Optimize for the frame-less case. If a page has no frame, then the calling
    context should be equal to the owning document's frame.
This commit is contained in:
Karl Seguin
2026-05-05 18:30:56 +08:00
parent d360fcc07b
commit becbd54f4e
16 changed files with 168 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/browser/Frame.zig
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

30
src/browser/tests/element/html/script/clone_already_started.html Normal file
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>

58
src/browser/tests/frames/cross_realm_collection.html Normal file
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>

3
src/browser/tests/frames/support/cross_realm_collection.html Normal file
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>

6
src/browser/webapi/Document.zig
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();
}

2
src/browser/webapi/DocumentFragment.zig
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 });

9
src/browser/webapi/Element.zig
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 });
}

29
src/browser/webapi/Node.zig
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();

2
src/browser/webapi/Range.zig
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) {

24
src/browser/webapi/collections/ChildNodes.zig
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;
}

12
src/browser/webapi/collections/HTMLAllCollection.zig
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();

9
src/browser/webapi/collections/node_live.zig
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;
}

4
src/browser/webapi/element/Attribute.zig
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);

2
src/browser/webapi/element/Html.zig
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 });

2
src/browser/webapi/element/html/Option.zig
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 {

9
src/browser/webapi/element/html/Script.zig
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");