Skip to content

Commit 4f163c6

Browse files
committed
CustomElement Reactions
While this PR touches a lot of files, and isn't trivial, many of the changes are either: 1 - removing guards added in previous PRs, e.g. #1969 #2172 #2313 #2366 2 - Adding the `.ce_reactions = true` flag to various WebAPIs CustomElements have callbacks, e.g. connectedCallback. Also, many WebAPI calls are implemented as a series of mutations, e.g. appendChild = remove from current + append to new. These two things interact in an important way: when should callbacks execute? Before this PR, we were invoking callbacks at each individual step. This is (a) technically wrong and (b) breaks a lot of assumptions (the reason the above 4 PRs were needed to fix bugs). This PR adds a `_ce_reactions` queue to the frame. And, instead of invoking callbacks, we "enqueue" the reaction. At various boundaries, a scope is created the DOM manipulation is done, and then we pop the scope, invoking all queued reactions.
1 parent 8bede86 commit 4f163c6

19 files changed

Lines changed: 447 additions & 291 deletions
Lines changed: 129 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,129 @@
1+
// Copyright (C) 2023-2026 Lightpanda (Selecy SAS)
2+
//
3+
// Francis Bouvier <francis@lightpanda.io>
4+
// Pierre Tachoire <pierre@lightpanda.io>
5+
//
6+
// This program is free software: you can redistribute it and/or modify
7+
// it under the terms of the GNU Affero General Public License as
8+
// published by the Free Software Foundation, either version 3 of the
9+
// License, or (at your option) any later version.
10+
//
11+
// This program is distributed in the hope that it will be useful,
12+
// but WITHOUT ANY WARRANTY; without even the implied warranty of
13+
// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
14+
// GNU Affero General Public License for more details.
15+
//
16+
// You should have received a copy of the GNU Affero General Public License
17+
// along with this program. If not, see <https://www.gnu.org/licenses/>.
18+
19+
// Implements the spec's "custom element reactions" mechanism: callbacks
20+
// (connectedCallback, disconnectedCallback, adoptedCallback,
21+
// attributeChangedCallback) are enqueued during DOM mutation and invoked at
22+
// the outer algorithm boundary, not synchronously mid-mutation.
23+
//
24+
// The "stack of element queues" is collapsed to a single flat ArrayList plus
25+
// per-scope checkpoint indices: push() captures items.len, popAndInvoke()
26+
// drains items[checkpoint..] and truncates. Nested scopes work naturally —
27+
// inside a callback, a new scope captures its own checkpoint past the current
28+
// length, drains its own range, and the outer iteration continues from where
29+
// it left off.
30+
31+
const std = @import("std");
32+
const lp = @import("lightpanda");
33+
34+
const Frame = @import("Frame.zig");
35+
const Element = @import("webapi/Element.zig");
36+
const Document = @import("webapi/Document.zig");
37+
const Custom = @import("webapi/element/html/Custom.zig");
38+
39+
const String = lp.String;
40+
const Allocator = std.mem.Allocator;
41+
42+
const Self = @This();
43+
44+
allocator: Allocator,
45+
queue: std.ArrayList(Reaction) = .empty,
46+
// Number of currently-open scopes (push() that hasn't been pop'd). Every
47+
// enqueue must happen inside a scope — that's the leak-detection invariant.
48+
// Checked in debug at enqueue time so leaks surface where the bug is, not
49+
// later at some unrelated boundary.
50+
active_scopes: u32 = 0,
51+
52+
/// Open a new reactions scope. Returns a checkpoint to be passed to popAndInvoke.
53+
pub fn push(self: *Self) usize {
54+
self.active_scopes += 1;
55+
return self.queue.items.len;
56+
}
57+
58+
/// Drain reactions queued at indices >= checkpoint, then truncate. Reactions
59+
/// enqueued within a nested scope drain at that scope's pop, before this loop
60+
/// sees them.
61+
pub fn popAndInvoke(self: *Self, checkpoint: usize, frame: *Frame) void {
62+
for (self.queue.items[checkpoint..]) |reaction| {
63+
Custom.fireReaction(reaction, frame);
64+
}
65+
self.queue.items.len = checkpoint;
66+
self.active_scopes -= 1;
67+
}
68+
69+
inline fn assertScopeActive(self: *const Self) void {
70+
lp.assert(self.active_scopes > 0, "ce_reactions enqueue without active scope", .{});
71+
}
72+
73+
pub fn enqueueConnected(self: *Self, element: *Element) !void {
74+
self.assertScopeActive();
75+
try self.queue.append(self.allocator, .{ .connected = element });
76+
}
77+
78+
pub fn enqueueDisconnected(self: *Self, element: *Element) !void {
79+
self.assertScopeActive();
80+
try self.queue.append(self.allocator, .{ .disconnected = element });
81+
}
82+
83+
pub fn enqueueAdopted(self: *Self, element: *Element, old_document: *Document, new_document: *Document) !void {
84+
self.assertScopeActive();
85+
try self.queue.append(self.allocator, .{ .adopted = .{
86+
.element = element,
87+
.old_document = old_document,
88+
.new_document = new_document,
89+
} });
90+
}
91+
92+
pub fn enqueueAttributeChanged(
93+
self: *Self,
94+
element: *Element,
95+
name: String,
96+
old_value: ?String,
97+
new_value: ?String,
98+
namespace: ?String,
99+
) !void {
100+
self.assertScopeActive();
101+
try self.queue.append(self.allocator, .{ .attribute_changed = .{
102+
.name = name,
103+
.element = element,
104+
.old_value = old_value,
105+
.new_value = new_value,
106+
.namespace = namespace,
107+
} });
108+
}
109+
110+
pub const Reaction = union(enum) {
111+
connected: *Element,
112+
disconnected: *Element,
113+
adopted: Adopted,
114+
attribute_changed: AttributeChanged,
115+
116+
pub const Adopted = struct {
117+
element: *Element,
118+
old_document: *Document,
119+
new_document: *Document,
120+
};
121+
122+
pub const AttributeChanged = struct {
123+
element: *Element,
124+
name: String,
125+
old_value: ?String,
126+
new_value: ?String,
127+
namespace: ?String,
128+
};
129+
};

src/browser/Frame.zig

Lines changed: 26 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ const StyleManager = @import("StyleManager.zig");
3232
const Parser = @import("parser/Parser.zig");
3333
const h5e = @import("parser/html5ever.zig");
3434

35+
const CustomElementReactions = @import("CustomElementReactions.zig");
36+
3537
const URL = @import("URL.zig");
3638
const Blob = @import("webapi/Blob.zig");
3739
const Node = @import("webapi/Node.zig");
@@ -188,6 +190,12 @@ _upgrading_element: ?*Node = null,
188190
// List of custom elements that were created before their definition was registered
189191
_undefined_custom_elements: std.ArrayList(*Element.Html.Custom) = .{},
190192

193+
// Pending custom-element reactions (connected/disconnected/adopted/attribute
194+
// changed). Reactions are enqueued during DOM mutation and drained at the
195+
// outer algorithm boundary — set up by the JS bridge for [CEReactions]
196+
// methods and by the parser pump on each yield.
197+
_ce_reactions: CustomElementReactions,
198+
191199
// for heap allocations and managing WebAPI objects
192200
_factory: *Factory,
193201

@@ -287,6 +295,7 @@ pub fn init(self: *Frame, frame_id: u32, page: *Page, parent: ?*Frame) !void {
287295
._type = if (parent == null) .root else .frame,
288296
._style_manager = undefined,
289297
._script_manager = undefined,
298+
._ce_reactions = .{ .allocator = arena },
290299
._event_manager = EventManager.init(arena, self),
291300
};
292301
self._to_load = &self._to_load_1;
@@ -1852,7 +1861,7 @@ pub fn adoptNodeTree(self: *Frame, node: *Node, old_owner: *Document, new_owner:
18521861

18531862
// Per spec, adopted steps run on each element after its document is set.
18541863
if (node.is(Element)) |el| {
1855-
Element.Html.Custom.invokeAdoptedCallbackOnElement(el, old_owner, new_owner, self);
1864+
Element.Html.Custom.enqueueAdoptedCallbackOnElement(el, old_owner, new_owner, self);
18561865
}
18571866

18581867
var it = node.childrenIterator();
@@ -2588,7 +2597,7 @@ pub fn createElementNS(self: *Frame, namespace: Element.Namespace, name: []const
25882597
if (element._attributes) |attributes| {
25892598
var it = attributes.iterator();
25902599
while (it.next()) |attr| {
2591-
Element.Html.Custom.invokeAttributeChangedCallbackOnElement(
2600+
Element.Html.Custom.enqueueAttributeChangedCallbackOnElement(
25922601
element,
25932602
attr._name,
25942603
null, // old_value is null for initial attributes
@@ -2981,7 +2990,7 @@ pub fn removeNode(self: *Frame, parent: *Node, child: *Node, opts: RemoveNodeOpt
29812990
self.removeElementIdWithMaps(id_maps.?, id);
29822991
}
29832992

2984-
Element.Html.Custom.invokeDisconnectedCallbackOnElement(el, self);
2993+
Element.Html.Custom.enqueueDisconnectedCallbackOnElement(el, self);
29852994

29862995
// If a <style> element is being removed, remove its sheet from the list
29872996
if (el.is(Element.Html.Style)) |style| {
@@ -3004,11 +3013,8 @@ pub fn appendAllChildren(self: *Frame, parent: *Node, target: *Node) !void {
30043013
self.domChanged();
30053014
const dest_connected = target.isConnected();
30063015

3007-
// Use firstChild() instead of iterator to handle cases where callbacks
3008-
// (like custom element connectedCallback) modify the parent during iteration.
3009-
// The iterator captures "next" pointers that can become stale.
3010-
while (parent.firstChild()) |child| {
3011-
// Check if child was connected BEFORE removing it from parent
3016+
var it = parent.childrenIterator();
3017+
while (it.next()) |child| {
30123018
const child_was_connected = child.isConnected();
30133019
self.removeNode(parent, child, .{ .will_be_reconnected = dest_connected });
30143020
try self.appendNode(target, child, .{ .child_already_connected = child_was_connected });
@@ -3019,31 +3025,16 @@ pub fn insertAllChildrenBefore(self: *Frame, fragment: *Node, parent: *Node, ref
30193025
self.domChanged();
30203026
const dest_connected = parent.isConnected();
30213027

3022-
// Use firstChild() instead of iterator to handle cases where callbacks
3023-
// (like custom element connectedCallback) modify the fragment during iteration.
3024-
// The iterator captures "next" pointers that can become stale.
3025-
while (fragment.firstChild()) |child| {
3026-
// Check if child was connected BEFORE removing it from fragment
3028+
var it = fragment.childrenIterator();
3029+
while (it.next()) |child| {
30273030
const child_was_connected = child.isConnected();
30283031
self.removeNode(fragment, child, .{ .will_be_reconnected = dest_connected });
3029-
// A callback fired by a previous iteration's insert (e.g. a custom
3030-
// element's connectedCallback) may have detached ref_node from
3031-
// parent. In that case, fall back to append so the remaining
3032-
// children still land in `parent` in source order.
3033-
if (ref_node._parent == parent) {
3034-
try self.insertNodeRelative(
3035-
parent,
3036-
child,
3037-
.{ .before = ref_node },
3038-
.{ .child_already_connected = child_was_connected },
3039-
);
3040-
} else {
3041-
try self.appendNode(
3042-
parent,
3043-
child,
3044-
.{ .child_already_connected = child_was_connected },
3045-
);
3046-
}
3032+
try self.insertNodeRelative(
3033+
parent,
3034+
child,
3035+
.{ .before = ref_node },
3036+
.{ .child_already_connected = child_was_connected },
3037+
);
30473038
}
30483039
}
30493040

@@ -3167,7 +3158,7 @@ pub fn _insertNodeRelative(self: *Frame, comptime from_parser: bool, parent: *No
31673158
if (el.getAttributeSafe(comptime .wrap("id"))) |id| {
31683159
try self.addElementId(parent, el, id);
31693160
}
3170-
try Element.Html.Custom.invokeConnectedCallbackOnElement(true, el, self);
3161+
try Element.Html.Custom.enqueueConnectedCallbackOnElement(true, el, self);
31713162
}
31723163
}
31733164
return;
@@ -3213,7 +3204,7 @@ pub fn _insertNodeRelative(self: *Frame, comptime from_parser: bool, parent: *No
32133204
}
32143205

32153206
if (should_invoke_connected) {
3216-
try Element.Html.Custom.invokeConnectedCallbackOnElement(false, el, self);
3207+
try Element.Html.Custom.enqueueConnectedCallbackOnElement(false, el, self);
32173208
}
32183209
}
32193210
}
@@ -3223,7 +3214,7 @@ pub fn attributeChange(self: *Frame, element: *Element, name: String, value: Str
32233214
log.err(.bug, "build.attributeChange", .{ .tag = element.getTag(), .name = name, .value = value, .err = err, .type = self._type, .url = self.url });
32243215
};
32253216

3226-
Element.Html.Custom.invokeAttributeChangedCallbackOnElement(element, name, old_value, value, null, self);
3217+
Element.Html.Custom.enqueueAttributeChangedCallbackOnElement(element, name, old_value, value, null, self);
32273218

32283219
var it: ?*std.DoublyLinkedList.Node = self._mutation_observers.first;
32293220
while (it) |node| : (it = node.next) {
@@ -3249,7 +3240,7 @@ pub fn attributeRemove(self: *Frame, element: *Element, name: String, old_value:
32493240
log.err(.bug, "build.attributeRemove", .{ .tag = element.getTag(), .name = name, .err = err, .type = self._type, .url = self.url });
32503241
};
32513242

3252-
Element.Html.Custom.invokeAttributeChangedCallbackOnElement(element, name, old_value, null, null, self);
3243+
Element.Html.Custom.enqueueAttributeChangedCallbackOnElement(element, name, old_value, null, null, self);
32533244

32543245
var it: ?*std.DoublyLinkedList.Node = self._mutation_observers.first;
32553246
while (it) |node| : (it = node.next) {

src/browser/ScriptManagerBase.zig

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -749,6 +749,13 @@ pub const Script = struct {
749749
try_catch.init(local);
750750
defer try_catch.deinit();
751751

752+
// Custom-element reactions: the script body is a JS-execution
753+
// boundary. Open a scope so any reactions it queues (or that were
754+
// queued by the parser since the previous boundary) drain at the
755+
// end of the script, before the parser resumes.
756+
const ce_checkpoint = frame._ce_reactions.push();
757+
defer frame._ce_reactions.popAndInvoke(ce_checkpoint, frame);
758+
752759
const success = blk: {
753760
const content = self.source.content();
754761
switch (fe.kind) {

src/browser/js/Caller.zig

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -569,6 +569,7 @@ pub const Function = struct {
569569
null_as_undefined: bool = false,
570570
cache: ?Caching = null,
571571
embedded_receiver: bool = false,
572+
ce_reactions: bool = false,
572573

573574
// We support two ways to cache a value directly into a v8::Object. The
574575
// difference between the two is like the difference between a Map
@@ -621,6 +622,26 @@ pub const Function = struct {
621622
caller.initWithContext(ctx, v8_context);
622623
defer caller.deinit();
623624

625+
// [CEReactions] entry: open a reactions scope so any custom-element
626+
// callbacks queued by DOM mutation inside `func` fire after it
627+
// returns, never mid-algorithm.
628+
var ce_checkpoint: usize = undefined;
629+
const ce_frame: ?*Frame = if (comptime opts.ce_reactions) switch (ctx.global) {
630+
.frame => |frame| frame,
631+
.worker => null,
632+
} else null;
633+
634+
if (comptime opts.ce_reactions) {
635+
if (ce_frame) |frame| {
636+
ce_checkpoint = frame._ce_reactions.push();
637+
}
638+
}
639+
defer if (comptime opts.ce_reactions) {
640+
if (ce_frame) |frame| {
641+
frame._ce_reactions.popAndInvoke(ce_checkpoint, frame);
642+
}
643+
};
644+
624645
const js_value = _call(T, &caller.local, info, func, opts) catch |err| {
625646
handleError(T, @TypeOf(func), &caller.local, err, info, .{
626647
.dom_exception = opts.dom_exception,

src/browser/js/bridge.zig

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,18 @@ pub const Constructor = struct {
130130
}
131131
defer caller.deinit();
132132

133+
// Constructors are a JS-execution boundary, just like
134+
// [CEReactions] methods. Open a reactions scope so any
135+
// callbacks queued by the user's constructor body (or
136+
// by attribute_changed reactions queued before invocation)
137+
// drain at the constructor's exit, not later.
138+
const ce_frame: ?*Frame = switch (caller.local.ctx.global) {
139+
.frame => |frame| frame,
140+
.worker => null,
141+
};
142+
const ce_checkpoint: usize = if (ce_frame) |frame| frame._ce_reactions.push() else 0;
143+
defer if (ce_frame) |frame| frame._ce_reactions.popAndInvoke(ce_checkpoint, frame);
144+
133145
caller.constructor(T, func, handle.?, .{
134146
.dom_exception = opts.dom_exception,
135147
.new_target = opts.new_target,

0 commit comments

Comments
 (0)