From a639ae20478f337b8f99f9c6b47896bde4b7c912 Mon Sep 17 00:00:00 2001 From: Navid EMAD Date: Mon, 4 May 2026 15:08:52 +0200 Subject: [PATCH 01/10] net: multipart-encode FormData bodies in fetch and XMLHttpRequest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wire the existing multipart encoder (FormData.multipartEncode) into the fetch and XHR body paths. Previously both entry points typed their body parameter as ?[]const u8, so the JS->Zig bridge silently coerced any FormData JSValue via toString() and emitted "[object FormData]" with Content-Type: application/x-www-form-urlencoded. Introduces a BodyInit tagged union covering FormData, URLSearchParams, Blob, and bytes. A small extract() helper returns the serialized bytes plus the spec-mandated default Content-Type (Fetch §6.5 step 8 / XHR §4.7.6 step 4). Both Request.init and XMLHttpRequest.send apply the default Content-Type only when the caller hasn't already set one. Closes #2357 --- src/browser/webapi/net/BodyInit.zig | 154 ++++++++++++++++++++++ src/browser/webapi/net/Request.zig | 23 +++- src/browser/webapi/net/XMLHttpRequest.zig | 15 ++- 3 files changed, 185 insertions(+), 7 deletions(-) create mode 100644 src/browser/webapi/net/BodyInit.zig diff --git a/src/browser/webapi/net/BodyInit.zig b/src/browser/webapi/net/BodyInit.zig new file mode 100644 index 00000000..dfcc1981 --- /dev/null +++ b/src/browser/webapi/net/BodyInit.zig @@ -0,0 +1,154 @@ +// Copyright (C) 2026 Lightpanda (Selecy SAS) +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as +// published by the Free Software Foundation, either version 3 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +// BodyInit — accepted body shapes for fetch(Request) / XHR.send(). +// +// Per Fetch §6.5 "extract a body" (https://fetch.spec.whatwg.org/#concept-bodyinit-extract) +// and XHR §4.7.6 "send()" (https://xhr.spec.whatwg.org/#dom-xmlhttprequest-send), +// the runtime must serialize the body and select the matching default +// Content-Type. Without this layer the JS→Zig bridge falls back to +// toStringSmart() on the JSValue, which sends "[object FormData]" for +// FormData (issue #2357) and skips the multipart encoding wired up at +// FormData.multipartEncode (./FormData.zig:198). +// +// The union arms are ordered so the bridge's tagged-union prober matches +// the most specific JsApi class first; the trailing `bytes: []const u8` +// arm soaks up strings (and via .coerce, anything string-like) so plain +// text bodies still work unchanged. + +const std = @import("std"); + +const FormData = @import("FormData.zig"); +const URLSearchParams = @import("URLSearchParams.zig"); +const Blob = @import("../Blob.zig"); + +const Allocator = std.mem.Allocator; + +pub const BodyInit = union(enum) { + form_data: *FormData, + url_search_params: *URLSearchParams, + blob: *Blob, + bytes: []const u8, +}; + +// Result of extracting a body. `bytes` is duped into the caller's arena. +// `content_type`, when non-null, is the spec-mandated default Content-Type +// for the body source — callers MUST only apply it if the user has not +// already set a Content-Type header (per Fetch §6.5). +pub const Extracted = struct { + bytes: []const u8, + content_type: ?[]const u8, +}; + +// Boundary length in bytes (32 hex chars = 128 bits of entropy). Chrome +// uses ~16 bytes; the multipart spec only requires it be unique enough to +// not collide with any payload bytes, which 128 bits comfortably is. +const BOUNDARY_HEX_LEN: usize = 32; +const BOUNDARY_PREFIX: []const u8 = "----LightpandaFormBoundary"; + +pub fn extract(body: BodyInit, arena: Allocator) !Extracted { + switch (body) { + .bytes => |b| { + // String bodies: dupe as-is. Per Fetch §6.5 step 4, the default + // Content-Type for USVString is "text/plain;charset=UTF-8"; + // emit it so callers without an explicit header still pass spec + // checks. Pre-fix behaviour also omitted this; tests that depend + // on no Content-Type for string bodies should set one explicitly. + return .{ + .bytes = try arena.dupe(u8, b), + .content_type = "text/plain;charset=UTF-8", + }; + }, + .url_search_params => |usp| { + var buf = std.Io.Writer.Allocating.init(arena); + try usp.toString(&buf.writer); + return .{ + .bytes = buf.written(), + .content_type = "application/x-www-form-urlencoded;charset=UTF-8", + }; + }, + .form_data => |fd| { + const boundary = try randomBoundary(arena); + var buf = std.Io.Writer.Allocating.init(arena); + try fd.write(.{ + .encoding = .{ .formdata = boundary }, + .allocator = arena, + }, &buf.writer); + const ct = try std.fmt.allocPrint(arena, "multipart/form-data; boundary={s}", .{boundary}); + return .{ + .bytes = buf.written(), + .content_type = ct, + }; + }, + .blob => |blob| { + return .{ + .bytes = try arena.dupe(u8, blob._slice), + .content_type = if (blob._mime.len > 0) try arena.dupe(u8, blob._mime) else null, + }; + }, + } +} + +fn randomBoundary(arena: Allocator) ![]const u8 { + var rand_bytes: [BOUNDARY_HEX_LEN / 2]u8 = undefined; + std.crypto.random.bytes(&rand_bytes); + const hex = std.fmt.bytesToHex(rand_bytes, .lower); + return std.fmt.allocPrint(arena, "{s}{s}", .{ BOUNDARY_PREFIX, hex }); +} + +const testing = @import("../../../testing.zig"); + +test "BodyInit: bytes pass through with text/plain" { + const arena = testing.arena_allocator; + const r = try extract(.{ .bytes = "hello" }, arena); + try testing.expectString("hello", r.bytes); + try testing.expectString("text/plain;charset=UTF-8", r.content_type.?); +} + +test "BodyInit: URLSearchParams emit urlencoded body + content-type" { + const arena = testing.arena_allocator; + const usp = try arena.create(URLSearchParams); + usp.* = .{ ._arena = arena, ._params = .empty }; + try usp.append("a", "1"); + try usp.append("b", "2"); + const r = try extract(.{ .url_search_params = usp }, arena); + try testing.expectString("a=1&b=2", r.bytes); + try testing.expectString("application/x-www-form-urlencoded;charset=UTF-8", r.content_type.?); +} + +test "BodyInit: FormData emits multipart with random boundary" { + const arena = testing.arena_allocator; + const fd = try arena.create(FormData); + fd.* = .{ ._arena = arena, ._entries = .empty }; + try fd.append("username", "alice"); + try fd.append("email", "alice@example.com"); + const r = try extract(.{ .form_data = fd }, arena); + const ct = r.content_type.?; + try testing.expect(std.mem.startsWith(u8, ct, "multipart/form-data; boundary=" ++ BOUNDARY_PREFIX)); + // Body must contain the entries' Content-Disposition lines and end with + // the closing boundary marker. + const boundary = ct["multipart/form-data; boundary=".len..]; + try testing.expect(std.mem.indexOf(u8, r.bytes, "Content-Disposition: form-data; name=\"username\"") != null); + try testing.expect(std.mem.indexOf(u8, r.bytes, "Content-Disposition: form-data; name=\"email\"") != null); + try testing.expect(std.mem.indexOf(u8, r.bytes, "alice") != null); + try testing.expect(std.mem.indexOf(u8, r.bytes, "alice@example.com") != null); + const closer = try std.fmt.allocPrint(arena, "--{s}--\r\n", .{boundary}); + try testing.expect(std.mem.endsWith(u8, r.bytes, closer)); +} + +// Blob.extract is exercised end-to-end by the Request/XHR HTML fixture +// tests rather than constructed ad-hoc here — Blob owns `_type`, `_rc`, +// and `_arena` fields that need a Page-backed allocator to initialise +// safely. diff --git a/src/browser/webapi/net/Request.zig b/src/browser/webapi/net/Request.zig index 50dfc87f..e021a3a8 100644 --- a/src/browser/webapi/net/Request.zig +++ b/src/browser/webapi/net/Request.zig @@ -25,6 +25,9 @@ const URL = @import("../URL.zig"); const Headers = @import("Headers.zig"); const Blob = @import("../Blob.zig"); const AbortSignal = @import("../AbortSignal.zig"); +const body_init = @import("BodyInit.zig"); + +pub const BodyInit = body_init.BodyInit; const Execution = js.Execution; const Allocator = std.mem.Allocator; @@ -48,7 +51,7 @@ pub const Input = union(enum) { pub const InitOpts = struct { method: ?[]const u8 = null, headers: ?Headers.InitOpts = null, - body: ?[]const u8 = null, + body: ?BodyInit = null, cache: Cache = .default, credentials: Credentials = .@"same-origin", signal: ?*AbortSignal = null, @@ -86,7 +89,7 @@ pub fn init(input: Input, opts_: ?InitOpts, exec: *const Execution) !*Request { .request => |r| r._method, }; - const headers = if (opts.headers) |headers_init| switch (headers_init) { + var headers = if (opts.headers) |headers_init| switch (headers_init) { .obj => |h| h, else => try Headers.init(headers_init, exec), } else switch (input) { @@ -94,9 +97,19 @@ pub fn init(input: Input, opts_: ?InitOpts, exec: *const Execution) !*Request { .request => |r| r._headers, }; - const body = if (opts.body) |b| - try arena.dupe(u8, b) - else switch (input) { + const body = if (opts.body) |b| blk: { + const extracted = try body_init.extract(b, arena); + // Per Fetch §6.5 step 11, the default Content-Type only applies if + // the user has not already set one via the headers init dict. + if (extracted.content_type) |ct| { + const hs = headers orelse try Headers.init(null, exec); + if (!hs.has("content-type", exec)) { + try hs.append("content-type", ct, exec); + } + headers = hs; + } + break :blk extracted.bytes; + } else switch (input) { .url => null, .request => |r| r._body, }; diff --git a/src/browser/webapi/net/XMLHttpRequest.zig b/src/browser/webapi/net/XMLHttpRequest.zig index 31a72cd0..b80dac86 100644 --- a/src/browser/webapi/net/XMLHttpRequest.zig +++ b/src/browser/webapi/net/XMLHttpRequest.zig @@ -30,6 +30,8 @@ const Page = @import("../../Page.zig"); const Node = @import("../Node.zig"); const Event = @import("../Event.zig"); const Headers = @import("Headers.zig"); +const Request = @import("Request.zig"); +const body_init = @import("BodyInit.zig"); const EventTarget = @import("../EventTarget.zig"); const XMLHttpRequestEventTarget = @import("XMLHttpRequestEventTarget.zig"); @@ -221,7 +223,7 @@ pub fn setRequestHeader(self: *XMLHttpRequest, name: []const u8, value: []const return self._request_headers.append(name, value, exec); } -pub fn send(self: *XMLHttpRequest, body_: ?[]const u8) !void { +pub fn send(self: *XMLHttpRequest, body_: ?Request.BodyInit, exec_: *const Execution) !void { if (comptime IS_DEBUG) { log.debug(.http, "XMLHttpRequest.send", .{ .url = self._url }); } @@ -231,7 +233,16 @@ pub fn send(self: *XMLHttpRequest, body_: ?[]const u8) !void { if (body_) |b| { if (self._method != .GET and self._method != .HEAD) { - self._request_body = try self._arena.dupe(u8, b); + const extracted = try body_init.extract(b, self._arena); + self._request_body = extracted.bytes; + // Per XHR §4.7.6 "send()" step 4, the default Content-Type only + // applies if the author hasn't already set one via + // setRequestHeader. + if (extracted.content_type) |ct| { + if (!self._request_headers.has("content-type", exec_)) { + try self._request_headers.append("content-type", ct, exec_); + } + } } } From 5cc8a0daff900f65b45725db063e0186db580ee6 Mon Sep 17 00:00:00 2001 From: Nikolay Govorov Date: Mon, 4 May 2026 17:21:15 +0100 Subject: [PATCH 02/10] Fix v8 crash --- src/browser/Browser.zig | 1 - src/browser/Session.zig | 6 ++++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/browser/Browser.zig b/src/browser/Browser.zig index c29c796e..18a674da 100644 --- a/src/browser/Browser.zig +++ b/src/browser/Browser.zig @@ -86,7 +86,6 @@ pub fn closeSession(self: *Browser) void { if (self.session) |*session| { session.deinit(); self.session = null; - self.env.memoryPressureNotification(.critical); } } diff --git a/src/browser/Session.zig b/src/browser/Session.zig index 5e8141ca..e8f80e38 100644 --- a/src/browser/Session.zig +++ b/src/browser/Session.zig @@ -105,6 +105,12 @@ pub fn deinit(self: *Session) void { self.removePage(); } self.cookie_jar.deinit(); + + // Force V8 to flush any remaining weak callbacks while + // fc_identity_pool is still alive. Identity structs allocated from + // this pool back V8 weak-callback parameters; freeing the pool first + // would leave dangling pointers that segfault on the next GC. + self.browser.env.memoryPressureNotification(.critical); self.fc_identity_pool.deinit(); self.storage_shed.deinit(self.browser.app.allocator); From e5a9f8ba2ef040ca64bac6db3f3ce3251131824c Mon Sep 17 00:00:00 2001 From: Nikolay Govorov Date: Mon, 4 May 2026 18:12:47 +0100 Subject: [PATCH 03/10] Fix ony more crash --- src/cdp/CDP.zig | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/cdp/CDP.zig b/src/cdp/CDP.zig index a56c5f30..a235fda2 100644 --- a/src/cdp/CDP.zig +++ b/src/cdp/CDP.zig @@ -365,6 +365,16 @@ pub fn disposeBrowserContext(self: *CDP, browser_context_id: []const u8) bool { if (std.mem.eql(u8, bc.id, browser_context_id) == false) { return false; } + // Reentrant teardown from a CDP message drained inside HttpClient.syncRequest. + // Tearing down the browser context here would free Session/Page state + // that the unwinding script-eval frame above us is about to dereference + // (see Session.removePage's matching guard). Defer cleanup to + // CDP.deinit at connection close, by which time eval has unwound. + if (bc.session.currentPage()) |page| { + if (page.frame._script_manager.base.is_evaluating) { + return true; + } + } bc.deinit(); self.browser.closeSession(); self.browser_context = null; From bdc9013241bac98f5de4808c57da8db55fe8cacd Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Tue, 5 May 2026 07:52:37 +0800 Subject: [PATCH 04/10] silence an unecessary test log --- src/TestHTTPServer.zig | 7 +++++-- src/browser/tests/page/meta.html | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/src/TestHTTPServer.zig b/src/TestHTTPServer.zig index 44736db1..a6d157c2 100644 --- a/src/TestHTTPServer.zig +++ b/src/TestHTTPServer.zig @@ -100,7 +100,10 @@ fn handleConnection(self: *TestHTTPServer, conn: std.net.Server.Connection) !voi pub fn sendFile(req: *std.http.Server.Request, file_path: []const u8) !void { var url_buf: [1024]u8 = undefined; var fba = std.heap.FixedBufferAllocator.init(&url_buf); - const unescaped_file_path = try URL.unescape(fba.allocator(), file_path); + var unescaped_file_path = try URL.unescape(fba.allocator(), file_path); + if (std.mem.indexOfScalarPos(u8, unescaped_file_path, 0, '?')) |pos| { + unescaped_file_path = unescaped_file_path[0..pos]; + } var file = std.fs.cwd().openFile(unescaped_file_path, .{}) catch |err| switch (err) { error.FileNotFound => return req.respond("server error", .{ .status = .not_found }), else => return err, @@ -114,7 +117,7 @@ pub fn sendFile(req: *std.http.Server.Request, file_path: []const u8) !void { .content_length = stat.size, .respond_options = .{ .extra_headers = &.{ - .{ .name = "content-type", .value = getContentType(file_path) }, + .{ .name = "content-type", .value = getContentType(unescaped_file_path) }, }, }, }); diff --git a/src/browser/tests/page/meta.html b/src/browser/tests/page/meta.html index 41e354f1..3c03f403 100644 --- a/src/browser/tests/page/meta.html +++ b/src/browser/tests/page/meta.html @@ -42,4 +42,4 @@ - + From 8ee644d9bf6a890b5bf1fd53438934f1fcc4eca4 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Tue, 5 May 2026 15:19:10 +0800 Subject: [PATCH 05/10] Protect against response double-free When Fetch.httpErrorCallback rejects the promise, microtasks are run. Those microtasks could do anything, including triggering a frame shutdown (e.g. via a navigation event). The result is that we end up with httpErrorCallback httpShutdownCallback And the code simply isn't designed to handle that. httpErrorCallback now takes control of the cleanup, capturing the `self._owns_response` in a local, and disabling so that any nested `httpShutdownCallback` are noops. --- src/browser/webapi/net/Fetch.zig | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/browser/webapi/net/Fetch.zig b/src/browser/webapi/net/Fetch.zig index 6fe47ad7..c186b818 100644 --- a/src/browser/webapi/net/Fetch.zig +++ b/src/browser/webapi/net/Fetch.zig @@ -249,10 +249,17 @@ fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void { var response = self._response; response._http_response = null; + + // Capture this before we reject. Rejection could trigger httpShutdownCallback + // (via a microtask callback). But if we're here, then we'll take care of + // cleaning up when we're done. + const owns_response = self._owns_response; + self._owns_response = false; + // the response is only passed on v8 on success, if we're here, it's safe to // clear this. (defer since `self is in the response's arena). - defer if (self._owns_response) { + defer if (owns_response) { response.deinit(self._exec.context.page); }; @@ -266,10 +273,6 @@ fn httpErrorCallback(ctx: *anyopaque, err: anyerror) void { fn httpShutdownCallback(ctx: *anyopaque) void { const self: *Fetch = @ptrCast(@alignCast(ctx)); - if (comptime IS_DEBUG) { - // should always be true - std.debug.assert(self._owns_response); - } if (self._owns_response) { var response = self._response; From becbd54f4ee4ded6120d2a3e3ed4a01a0a440de3 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Tue, 5 May 2026 18:30:56 +0800 Subject: [PATCH 06/10] Track DOM version based on the node's owning frame 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. --- src/browser/Frame.zig | 4 +- .../html/script/clone_already_started.html | 30 ++++++++++ .../tests/frames/cross_realm_collection.html | 58 +++++++++++++++++++ .../support/cross_realm_collection.html | 3 + src/browser/webapi/Document.zig | 6 +- src/browser/webapi/DocumentFragment.zig | 2 +- src/browser/webapi/Element.zig | 9 ++- src/browser/webapi/Node.zig | 29 ++++++++-- src/browser/webapi/Range.zig | 2 +- src/browser/webapi/collections/ChildNodes.zig | 24 +++++--- .../webapi/collections/HTMLAllCollection.zig | 12 ++-- src/browser/webapi/collections/node_live.zig | 9 ++- src/browser/webapi/element/Attribute.zig | 4 +- src/browser/webapi/element/Html.zig | 2 +- src/browser/webapi/element/html/Option.zig | 2 +- src/browser/webapi/element/html/Script.zig | 9 +++ 16 files changed, 168 insertions(+), 37 deletions(-) create mode 100644 src/browser/tests/element/html/script/clone_already_started.html create mode 100644 src/browser/tests/frames/cross_realm_collection.html create mode 100644 src/browser/tests/frames/support/cross_realm_collection.html diff --git a/src/browser/Frame.zig b/src/browser/Frame.zig index 422fa955..51c6254b 100644 --- a/src/browser/Frame.zig +++ b/src/browser/Frame.zig @@ -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 diff --git a/src/browser/tests/element/html/script/clone_already_started.html b/src/browser/tests/element/html/script/clone_already_started.html new file mode 100644 index 00000000..5748459d --- /dev/null +++ b/src/browser/tests/element/html/script/clone_already_started.html @@ -0,0 +1,30 @@ + + + + + + + + diff --git a/src/browser/tests/frames/cross_realm_collection.html b/src/browser/tests/frames/cross_realm_collection.html new file mode 100644 index 00000000..9d14b1b9 --- /dev/null +++ b/src/browser/tests/frames/cross_realm_collection.html @@ -0,0 +1,58 @@ + + + + + + + + + + + + + diff --git a/src/browser/tests/frames/support/cross_realm_collection.html b/src/browser/tests/frames/support/cross_realm_collection.html new file mode 100644 index 00000000..f247803c --- /dev/null +++ b/src/browser/tests/frames/support/cross_realm_collection.html @@ -0,0 +1,3 @@ + + +

p0

p1

p2

diff --git a/src/browser/webapi/Document.zig b/src/browser/webapi/Document.zig index 8ffec5ce..b18211f5 100644 --- a/src/browser/webapi/Document.zig +++ b/src/browser/webapi/Document.zig @@ -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(); } diff --git a/src/browser/webapi/DocumentFragment.zig b/src/browser/webapi/DocumentFragment.zig index 186bc68a..106cb427 100644 --- a/src/browser/webapi/DocumentFragment.zig +++ b/src/browser/webapi/DocumentFragment.zig @@ -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 }); diff --git a/src/browser/webapi/Element.zig b/src/browser/webapi/Element.zig index a5aa19d5..91f47758 100644 --- a/src/browser/webapi/Element.zig +++ b/src/browser/webapi/Element.zig @@ -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 }); } diff --git a/src/browser/webapi/Node.zig b/src/browser/webapi/Node.zig index fafa32c6..718c065a 100644 --- a/src/browser/webapi/Node.zig +++ b/src/browser/webapi/Node.zig @@ -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(); diff --git a/src/browser/webapi/Range.zig b/src/browser/webapi/Range.zig index c1eb3fde..686c69df 100644 --- a/src/browser/webapi/Range.zig +++ b/src/browser/webapi/Range.zig @@ -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) { diff --git a/src/browser/webapi/collections/ChildNodes.zig b/src/browser/webapi/collections/ChildNodes.zig index 1fa3b180..c94e7e53 100644 --- a/src/browser/webapi/collections/ChildNodes.zig +++ b/src/browser/webapi/collections/ChildNodes.zig @@ -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; } diff --git a/src/browser/webapi/collections/HTMLAllCollection.zig b/src/browser/webapi/collections/HTMLAllCollection.zig index 835de18c..7e9758b7 100644 --- a/src/browser/webapi/collections/HTMLAllCollection.zig +++ b/src/browser/webapi/collections/HTMLAllCollection.zig @@ -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(); diff --git a/src/browser/webapi/collections/node_live.zig b/src/browser/webapi/collections/node_live.zig index 60e2e40c..33962d8e 100644 --- a/src/browser/webapi/collections/node_live.zig +++ b/src/browser/webapi/collections/node_live.zig @@ -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; } diff --git a/src/browser/webapi/element/Attribute.zig b/src/browser/webapi/element/Attribute.zig index c0d4d053..24b6477c 100644 --- a/src/browser/webapi/element/Attribute.zig +++ b/src/browser/webapi/element/Attribute.zig @@ -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); diff --git a/src/browser/webapi/element/Html.zig b/src/browser/webapi/element/Html.zig index 3ed96f57..ffaba715 100644 --- a/src/browser/webapi/element/Html.zig +++ b/src/browser/webapi/element/Html.zig @@ -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 }); diff --git a/src/browser/webapi/element/html/Option.zig b/src/browser/webapi/element/html/Option.zig index 9ca53f25..494573cf 100644 --- a/src/browser/webapi/element/html/Option.zig +++ b/src/browser/webapi/element/html/Option.zig @@ -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