From ca20a2c7f64c2804c2eee537743a74fe13daef19 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Mon, 18 May 2026 13:02:30 +0800 Subject: [PATCH] Use backup queue when one doesn't exist Turns a crash into a slightly delayed (incorrect) execution order which has some basis in the spec. --- src/browser/CustomElementReactions.zig | 64 ++++++++++++++++------ src/browser/Frame.zig | 4 ++ src/browser/js/Context.zig | 11 ++++ src/browser/webapi/element/html/Button.zig | 14 ++--- src/browser/webapi/element/html/Custom.zig | 12 ++-- 5 files changed, 75 insertions(+), 30 deletions(-) diff --git a/src/browser/CustomElementReactions.zig b/src/browser/CustomElementReactions.zig index 3e0cbc9c..70e5c94b 100644 --- a/src/browser/CustomElementReactions.zig +++ b/src/browser/CustomElementReactions.zig @@ -27,6 +27,12 @@ // inside a callback, a new scope captures its own checkpoint past the current // length, drains its own range, and the outer iteration continues from where // it left off. +// +// When a reaction is enqueued without an active scope (e.g. a Web API path +// that wasn't tagged `.ce_reactions = true`, or a non-WebIDL entry point), +// it goes on the backup queue instead and a microtask is scheduled to drain +// it. This matches the spec's "backup element queue" so missing bridge tags +// degrade to delayed reactions rather than crashes. const std = @import("std"); const lp = @import("lightpanda"); @@ -38,15 +44,18 @@ const Custom = @import("webapi/element/html/Custom.zig"); const String = lp.String; const Allocator = std.mem.Allocator; +const IS_DEBUG = @import("builtin").mode == .Debug; const Self = @This(); allocator: Allocator, queue: std.ArrayList(Reaction) = .empty, -// Number of currently-open scopes (push() that hasn't been pop'd). Every -// enqueue must happen inside a scope — that's the leak-detection invariant. -// Checked in debug at enqueue time so leaks surface where the bug is, not -// later at some unrelated boundary. + +backup_scheduled: bool = false, +backup_queue: std.ArrayList(Reaction) = .empty, + +// Number of currently-open scopes (push() that hasn't been pop'd). When 0, +// enqueues route to the backup queue and rely on a microtask to drain. active_scopes: u32 = 0, /// Open a new reactions scope. Returns a checkpoint to be passed to popAndInvoke. @@ -70,23 +79,44 @@ pub fn popAndInvoke(self: *Self, checkpoint: usize, frame: *Frame) void { self.active_scopes -= 1; } -inline fn assertScopeActive(self: *const Self) void { - lp.assert(self.active_scopes > 0, "ce_reactions enqueue without active scope", .{}); +/// Drain the backup queue. Called from the scheduled microtask. `backup_scheduled` +/// stays true while draining so new enqueues append to backup_queue and get picked +/// up by the same loop instead of scheduling a redundant microtask. +pub fn drainBackup(self: *Self, frame: *Frame) void { + var i: usize = 0; + const backup_queue = self.backup_queue; + while (i < backup_queue.items.len) : (i += 1) { + Custom.fireReaction(backup_queue.items[i], frame); + } + self.backup_queue.clearRetainingCapacity(); + self.backup_scheduled = false; } -pub fn enqueueConnected(self: *Self, element: *Element) !void { - self.assertScopeActive(); - try self.queue.append(self.allocator, .{ .connected = element }); +fn route(self: *Self, frame: *Frame, reaction: Reaction) !void { + if (self.active_scopes > 0) { + try self.queue.append(self.allocator, reaction); + return; + } + if (comptime IS_DEBUG) { + lp.log.err(.bug, "custom element scope", .{.note = "Missing explicit reaction scope, using fallback. This log is only generated in debug builds."}); + } + try self.backup_queue.append(self.allocator, reaction); + if (!self.backup_scheduled) { + try frame.scheduleCustomElementBackupDrain(); + self.backup_scheduled = true; + } } -pub fn enqueueDisconnected(self: *Self, element: *Element) !void { - self.assertScopeActive(); - try self.queue.append(self.allocator, .{ .disconnected = element }); +pub fn enqueueConnected(self: *Self, frame: *Frame, element: *Element) !void { + try self.route(frame, .{ .connected = element }); } -pub fn enqueueAdopted(self: *Self, element: *Element, old_document: *Document, new_document: *Document) !void { - self.assertScopeActive(); - try self.queue.append(self.allocator, .{ .adopted = .{ +pub fn enqueueDisconnected(self: *Self, frame: *Frame, element: *Element) !void { + try self.route(frame, .{ .disconnected = element }); +} + +pub fn enqueueAdopted(self: *Self, frame: *Frame, element: *Element, old_document: *Document, new_document: *Document) !void { + try self.route(frame, .{ .adopted = .{ .element = element, .old_document = old_document, .new_document = new_document, @@ -95,14 +125,14 @@ pub fn enqueueAdopted(self: *Self, element: *Element, old_document: *Document, n pub fn enqueueAttributeChanged( self: *Self, + frame: *Frame, element: *Element, name: String, old_value: ?String, new_value: ?String, namespace: ?String, ) !void { - self.assertScopeActive(); - try self.queue.append(self.allocator, .{ .attribute_changed = .{ + try self.route(frame, .{ .attribute_changed = .{ .name = name, .element = element, .old_value = old_value, diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index e92e00aa..09e45140 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -1825,6 +1825,10 @@ pub fn scheduleSlotchangeDelivery(self: *Frame) !void { try self.js.queueSlotchangeDelivery(); } +pub fn scheduleCustomElementBackupDrain(self: *Frame) !void { + try self.js.queueCustomElementBackupDrain(); +} + pub fn performScheduledIntersectionChecks(self: *Frame) void { if (!self._intersection_check_scheduled) { return; diff --git a/src/browser/js/Context.zig b/src/browser/js/Context.zig index fc7c3347..0f1350b6 100644 --- a/src/browser/js/Context.zig +++ b/src/browser/js/Context.zig @@ -989,6 +989,17 @@ pub fn queueSlotchangeDelivery(self: *Context) !void { }.run); } +pub fn queueCustomElementBackupDrain(self: *Context) !void { + self.enqueueMicrotask(struct { + fn run(ctx: *Context) void { + switch (ctx.global) { + .frame => |frame| frame._ce_reactions.drainBackup(frame), + .worker => unreachable, + } + } + }.run); +} + // Helper for executing a Microtask on this Context. In V8, microtasks aren't // associated to a Context - they are just functions to execute in an Isolate. // But for these Context microtasks, we want to (a) make sure the context isn't diff --git a/src/browser/webapi/element/html/Button.zig b/src/browser/webapi/element/html/Button.zig index e8a103fe..91729cae 100644 --- a/src/browser/webapi/element/html/Button.zig +++ b/src/browser/webapi/element/html/Button.zig @@ -241,13 +241,13 @@ pub const JsApi = struct { pub const name = bridge.accessor(Button.getName, Button.setName, .{ .ce_reactions = true }); pub const required = bridge.accessor(Button.getRequired, Button.setRequired, .{ .ce_reactions = true }); pub const form = bridge.accessor(Button.getForm, null, .{}); - pub const formAction = bridge.accessor(Button.getFormAction, Button.setFormAction, .{.ce_reactions = true}); - pub const formEnctype = bridge.accessor(Button.getFormEnctype, Button.setFormEnctype, .{.ce_reactions = true}); - pub const formMethod = bridge.accessor(Button.getFormMethod, Button.setFormMethod, .{.ce_reactions = true}); - pub const formNoValidate = bridge.accessor(Button.getFormNoValidate, Button.setFormNoValidate, .{.ce_reactions = true}); - pub const formTarget = bridge.accessor(Button.getFormTarget, Button.setFormTarget, .{.ce_reactions = true}); - pub const value = bridge.accessor(Button.getValue, Button.setValue, .{.ce_reactions = true}); - pub const @"type" = bridge.accessor(Button.getType, Button.setType, .{.ce_reactions = true}); + pub const formAction = bridge.accessor(Button.getFormAction, Button.setFormAction, .{ .ce_reactions = true }); + pub const formEnctype = bridge.accessor(Button.getFormEnctype, Button.setFormEnctype, .{ .ce_reactions = true }); + pub const formMethod = bridge.accessor(Button.getFormMethod, Button.setFormMethod, .{ .ce_reactions = true }); + pub const formNoValidate = bridge.accessor(Button.getFormNoValidate, Button.setFormNoValidate, .{ .ce_reactions = true }); + pub const formTarget = bridge.accessor(Button.getFormTarget, Button.setFormTarget, .{ .ce_reactions = true }); + pub const value = bridge.accessor(Button.getValue, Button.setValue, .{ .ce_reactions = true }); + pub const @"type" = bridge.accessor(Button.getType, Button.setType, .{ .ce_reactions = true }); pub const labels = bridge.accessor(Button.getLabels, null, .{}); pub const willValidate = bridge.accessor(Button.getWillValidate, null, .{}); pub const validity = bridge.accessor(Button.getValidity, null, .{}); diff --git a/src/browser/webapi/element/html/Custom.zig b/src/browser/webapi/element/html/Custom.zig index 4b1944f0..26b9e640 100644 --- a/src/browser/webapi/element/html/Custom.zig +++ b/src/browser/webapi/element/html/Custom.zig @@ -73,7 +73,7 @@ pub fn enqueueConnectedCallbackOnElement(comptime from_parser: bool, element: *E if (custom._connected_callback_invoked) return; custom._connected_callback_invoked = true; custom._disconnected_callback_invoked = false; - try frame._ce_reactions.enqueueConnected(element); + try frame._ce_reactions.enqueueConnected(frame, element); return; } @@ -101,7 +101,7 @@ pub fn enqueueConnectedCallbackOnElement(comptime from_parser: bool, element: *E } _ = frame._customized_builtin_disconnected_callback_invoked.remove(element); - try frame._ce_reactions.enqueueConnected(element); + try frame._ce_reactions.enqueueConnected(frame, element); } pub fn enqueueDisconnectedCallbackOnElement(element: *Element, frame: *Frame) void { @@ -110,7 +110,7 @@ pub fn enqueueDisconnectedCallbackOnElement(element: *Element, frame: *Frame) vo if (custom._disconnected_callback_invoked) return; custom._disconnected_callback_invoked = true; custom._connected_callback_invoked = false; - frame._ce_reactions.enqueueDisconnected(element) catch |err| { + frame._ce_reactions.enqueueDisconnected(frame, element) catch |err| { log.warn(.bug, "ce_reactions enqueue fail", .{ .err = err }); }; return; @@ -128,7 +128,7 @@ pub fn enqueueDisconnectedCallbackOnElement(element: *Element, frame: *Frame) vo gop.value_ptr.* = {}; _ = frame._customized_builtin_connected_callback_invoked.remove(element); - frame._ce_reactions.enqueueDisconnected(element) catch |err| { + frame._ce_reactions.enqueueDisconnected(frame, element) catch |err| { log.warn(.bug, "ce_reactions enqueue fail", .{ .err = err }); }; } @@ -139,7 +139,7 @@ pub fn enqueueAdoptedCallbackOnElement(element: *Element, old_document: *Documen } else { if (frame.getCustomizedBuiltInDefinition(element) == null) return; } - frame._ce_reactions.enqueueAdopted(element, old_document, new_document) catch |err| { + frame._ce_reactions.enqueueAdopted(frame, element, old_document, new_document) catch |err| { log.warn(.bug, "ce_reactions enqueue fail", .{ .err = err }); }; } @@ -152,7 +152,7 @@ pub fn enqueueAttributeChangedCallbackOnElement(element: *Element, name: String, const definition = frame.getCustomizedBuiltInDefinition(element) orelse return; if (!definition.isAttributeObserved(name)) return; } - frame._ce_reactions.enqueueAttributeChanged(element, name, old_value, new_value, namespace) catch |err| { + frame._ce_reactions.enqueueAttributeChanged(frame, element, name, old_value, new_value, namespace) catch |err| { log.warn(.bug, "ce_reactions enqueue fail", .{ .err = err }); }; }