From 6b9dc906390c8c973052e4cf07414b4b4135b004 Mon Sep 17 00:00:00 2001 From: Karl Seguin Date: Wed, 17 Sep 2025 12:08:26 +0800 Subject: [PATCH] Introduces an Env.String for persistent strings If a webapi has a []const u8 parameter, then the page.call_arena is used. This is our favorite arena to use, but if the string value has a lifetime beyond the call, it then needs to be duped again (using page.arena). When a webapi has a Env.String parameter, the page.arena will be used directly to get the value from V8, removing the need for an intermediary dupe in the call_arena. This allows HTMLCollections to be streamlined. They no longer need to dupe the filter (tag name, class name, ...), which means they can no longer fail. It also means that we no longer need to needlessly dupe the string literals. --- src/browser/dom/document.zig | 16 ++---- src/browser/dom/element.zig | 21 +++----- src/browser/dom/html_collection.zig | 51 ++++++++----------- src/browser/dom/nodelist.zig | 18 +++++-- src/browser/dom/performance.zig | 8 ++- src/browser/html/document.zig | 46 ++++++++--------- src/browser/html/window.zig | 4 +- src/runtime/js.zig | 13 +++++ src/tests/dom/html_collection.html | 77 ++++++++++++++++------------- 9 files changed, 127 insertions(+), 127 deletions(-) diff --git a/src/browser/dom/document.zig b/src/browser/dom/document.zig index 3e91b7cb..40ab1d60 100644 --- a/src/browser/dom/document.zig +++ b/src/browser/dom/document.zig @@ -156,22 +156,14 @@ pub const Document = struct { // the spec changed to return an HTMLCollection instead. // That's why we reimplemented getElementsByTagName by using an // HTMLCollection in zig here. - pub fn _getElementsByTagName( - self: *parser.Document, - tag_name: []const u8, - page: *Page, - ) !collection.HTMLCollection { - return try collection.HTMLCollectionByTagName(page.arena, parser.documentToNode(self), tag_name, .{ + pub fn _getElementsByTagName(self: *parser.Document, tag_name: Env.String) !collection.HTMLCollection { + return collection.HTMLCollectionByTagName(parser.documentToNode(self), tag_name.string, .{ .include_root = true, }); } - pub fn _getElementsByClassName( - self: *parser.Document, - classNames: []const u8, - page: *Page, - ) !collection.HTMLCollection { - return try collection.HTMLCollectionByClassName(page.arena, parser.documentToNode(self), classNames, .{ + pub fn _getElementsByClassName(self: *parser.Document, class_names: Env.String) !collection.HTMLCollection { + return collection.HTMLCollectionByClassName(parser.documentToNode(self), class_names.string, .{ .include_root = true, }); } diff --git a/src/browser/dom/element.zig b/src/browser/dom/element.zig index 5482dc65..4bd987af 100644 --- a/src/browser/dom/element.zig +++ b/src/browser/dom/element.zig @@ -19,6 +19,7 @@ const std = @import("std"); const parser = @import("../netsurf.zig"); +const Env = @import("../env.zig").Env; const Page = @import("../page.zig").Page; const css = @import("css.zig"); @@ -350,28 +351,18 @@ pub const Element = struct { return try parser.elementRemoveAttributeNode(self, attr); } - pub fn _getElementsByTagName( - self: *parser.Element, - tag_name: []const u8, - page: *Page, - ) !collection.HTMLCollection { - return try collection.HTMLCollectionByTagName( - page.arena, + pub fn _getElementsByTagName(self: *parser.Element, tag_name: Env.String) !collection.HTMLCollection { + return collection.HTMLCollectionByTagName( parser.elementToNode(self), - tag_name, + tag_name.string, .{ .include_root = false }, ); } - pub fn _getElementsByClassName( - self: *parser.Element, - classNames: []const u8, - page: *Page, - ) !collection.HTMLCollection { + pub fn _getElementsByClassName(self: *parser.Element, class_names: Env.String) !collection.HTMLCollection { return try collection.HTMLCollectionByClassName( - page.arena, parser.elementToNode(self), - classNames, + class_names.string, .{ .include_root = false }, ); } diff --git a/src/browser/dom/html_collection.zig b/src/browser/dom/html_collection.zig index 11dd4158..133aae87 100644 --- a/src/browser/dom/html_collection.zig +++ b/src/browser/dom/html_collection.zig @@ -52,13 +52,13 @@ pub const MatchByTagName = struct { tag: []const u8, is_wildcard: bool, - fn init(arena: Allocator, tag_name: []const u8) !MatchByTagName { + fn init(tag_name: []const u8) MatchByTagName { if (std.mem.eql(u8, tag_name, "*")) { return .{ .tag = "*", .is_wildcard = true }; } return .{ - .tag = try arena.dupe(u8, tag_name), + .tag = tag_name, .is_wildcard = false, }; } @@ -69,15 +69,14 @@ pub const MatchByTagName = struct { }; pub fn HTMLCollectionByTagName( - arena: Allocator, root: ?*parser.Node, tag_name: []const u8, opts: Opts, -) !HTMLCollection { - return HTMLCollection{ +) HTMLCollection { + return .{ .root = root, .walker = .{ .walkerDepthFirst = .{} }, - .matcher = .{ .matchByTagName = try MatchByTagName.init(arena, tag_name) }, + .matcher = .{ .matchByTagName = MatchByTagName.init(tag_name) }, .mutable = opts.mutable, .include_root = opts.include_root, }; @@ -86,9 +85,9 @@ pub fn HTMLCollectionByTagName( pub const MatchByClassName = struct { class_names: []const u8, - fn init(arena: Allocator, class_names: []const u8) !MatchByClassName { + fn init(class_names: []const u8) !MatchByClassName { return .{ - .class_names = try arena.dupe(u8, class_names), + .class_names = class_names, }; } @@ -107,15 +106,14 @@ pub const MatchByClassName = struct { }; pub fn HTMLCollectionByClassName( - arena: Allocator, root: ?*parser.Node, - classNames: []const u8, + class_names: []const u8, opts: Opts, ) !HTMLCollection { return HTMLCollection{ .root = root, .walker = .{ .walkerDepthFirst = .{} }, - .matcher = .{ .matchByClassName = try MatchByClassName.init(arena, classNames) }, + .matcher = .{ .matchByClassName = try MatchByClassName.init(class_names) }, .mutable = opts.mutable, .include_root = opts.include_root, }; @@ -124,10 +122,8 @@ pub fn HTMLCollectionByClassName( pub const MatchByName = struct { name: []const u8, - fn init(arena: Allocator, name: []const u8) !MatchByName { - return .{ - .name = try arena.dupe(u8, name), - }; + fn init(name: []const u8) !MatchByName { + return .{ .name = name }; } pub fn match(self: MatchByName, node: *parser.Node) !bool { @@ -138,7 +134,6 @@ pub const MatchByName = struct { }; pub fn HTMLCollectionByName( - arena: Allocator, root: ?*parser.Node, name: []const u8, opts: Opts, @@ -146,7 +141,7 @@ pub fn HTMLCollectionByName( return HTMLCollection{ .root = root, .walker = .{ .walkerDepthFirst = .{} }, - .matcher = .{ .matchByName = try MatchByName.init(arena, name) }, + .matcher = .{ .matchByName = try MatchByName.init(name) }, .mutable = opts.mutable, .include_root = opts.include_root, }; @@ -203,8 +198,8 @@ pub fn HTMLCollectionChildren( }; } -pub fn HTMLCollectionEmpty() !HTMLCollection { - return HTMLCollection{ +pub fn HTMLCollectionEmpty() HTMLCollection { + return .{ .root = null, .walker = .{ .walkerNone = .{} }, .matcher = .{ .matchFalse = .{} }, @@ -226,14 +221,11 @@ pub const MatchByLinks = struct { } }; -pub fn HTMLCollectionByLinks( - root: ?*parser.Node, - opts: Opts, -) !HTMLCollection { - return HTMLCollection{ +pub fn HTMLCollectionByLinks(root: ?*parser.Node, opts: Opts) HTMLCollection { + return .{ .root = root, .walker = .{ .walkerDepthFirst = .{} }, - .matcher = .{ .matchByLinks = MatchByLinks{} }, + .matcher = .{ .matchByLinks = .{} }, .mutable = opts.mutable, .include_root = opts.include_root, }; @@ -252,14 +244,11 @@ pub const MatchByAnchors = struct { } }; -pub fn HTMLCollectionByAnchors( - root: ?*parser.Node, - opts: Opts, -) !HTMLCollection { - return HTMLCollection{ +pub fn HTMLCollectionByAnchors(root: ?*parser.Node, opts: Opts) HTMLCollection { + return .{ .root = root, .walker = .{ .walkerDepthFirst = .{} }, - .matcher = .{ .matchByAnchors = MatchByAnchors{} }, + .matcher = .{ .matchByAnchors = .{} }, .mutable = opts.mutable, .include_root = opts.include_root, }; diff --git a/src/browser/dom/nodelist.zig b/src/browser/dom/nodelist.zig index d098cfd3..6ee5bca4 100644 --- a/src/browser/dom/nodelist.zig +++ b/src/browser/dom/nodelist.zig @@ -17,6 +17,7 @@ // along with this program. If not, see . const std = @import("std"); +const Allocator = std.mem.Allocator; const log = @import("../../log.zig"); const parser = @import("../netsurf.zig"); @@ -101,13 +102,20 @@ pub const NodeList = struct { nodes: NodesArrayList = .{}, - pub fn deinit(self: *NodeList, alloc: std.mem.Allocator) void { - // TODO unref all nodes - self.nodes.deinit(alloc); + pub fn deinit(self: *NodeList, allocator: Allocator) void { + self.nodes.deinit(allocator); } - pub fn append(self: *NodeList, alloc: std.mem.Allocator, node: *parser.Node) !void { - try self.nodes.append(alloc, node); + pub fn ensureTotalCapacity(self: *NodeList, allocator: Allocator, n: usize) !void { + return self.nodes.ensureTotalCapacity(allocator, n); + } + + pub fn append(self: *NodeList, allocator: Allocator, node: *parser.Node) !void { + try self.nodes.append(allocator, node); + } + + pub fn appendAssumeCapacity(self: *NodeList, node: *parser.Node) void { + self.nodes.appendAssumeCapacity(node); } pub fn get_length(self: *const NodeList) u32 { diff --git a/src/browser/dom/performance.zig b/src/browser/dom/performance.zig index 6a971b4d..09c0c6f8 100644 --- a/src/browser/dom/performance.zig +++ b/src/browser/dom/performance.zig @@ -61,7 +61,7 @@ pub const Performance = struct { return milliTimestamp() - self.time_origin; } - pub fn _mark(_: *Performance, name: []const u8, _options: ?PerformanceMark.Options, page: *Page) !PerformanceMark { + pub fn _mark(_: *Performance, name: Env.String, _options: ?PerformanceMark.Options, page: *Page) !PerformanceMark { const mark: PerformanceMark = try .constructor(name, _options, page); // TODO: Should store this in an entries list return mark; @@ -155,7 +155,7 @@ pub const PerformanceMark = struct { startTime: ?f64 = null, }; - pub fn constructor(name: []const u8, _options: ?Options, page: *Page) !PerformanceMark { + pub fn constructor(name: Env.String, _options: ?Options, page: *Page) !PerformanceMark { const perf = &page.window.performance; const options = _options orelse Options{}; @@ -166,9 +166,7 @@ pub const PerformanceMark = struct { } const detail = if (options.detail) |d| try d.persist() else null; - - const duped_name = try page.arena.dupe(u8, name); - const proto = PerformanceEntry{ .name = duped_name, .entry_type = .mark, .start_time = start_time }; + const proto = PerformanceEntry{ .name = name.string, .entry_type = .mark, .start_time = start_time }; return .{ .proto = proto, .detail = detail }; } diff --git a/src/browser/html/document.zig b/src/browser/html/document.zig index 55b94ed6..4020b498 100644 --- a/src/browser/html/document.zig +++ b/src/browser/html/document.zig @@ -115,67 +115,69 @@ pub const HTMLDocument = struct { } pub fn _getElementsByName(self: *parser.DocumentHTML, name: []const u8, page: *Page) !NodeList { - const arena = page.arena; var list: NodeList = .{}; - if (name.len == 0) return list; + if (name.len == 0) { + return list; + } const root = parser.documentHTMLToNode(self); - var c = try collection.HTMLCollectionByName(arena, root, name, .{ + var c = try collection.HTMLCollectionByName(root, name, .{ .include_root = false, }); const ln = try c.get_length(); + try list.ensureTotalCapacity(page.arena, ln); + var i: u32 = 0; - while (i < ln) { + while (i < ln) : (i += 1) { const n = try c.item(i) orelse break; - try list.append(arena, n); - i += 1; + list.appendAssumeCapacity(n); } return list; } - pub fn get_images(self: *parser.DocumentHTML, page: *Page) !collection.HTMLCollection { - return try collection.HTMLCollectionByTagName(page.arena, parser.documentHTMLToNode(self), "img", .{ + pub fn get_images(self: *parser.DocumentHTML) collection.HTMLCollection { + return collection.HTMLCollectionByTagName(parser.documentHTMLToNode(self), "img", .{ .include_root = false, }); } - pub fn get_embeds(self: *parser.DocumentHTML, page: *Page) !collection.HTMLCollection { - return try collection.HTMLCollectionByTagName(page.arena, parser.documentHTMLToNode(self), "embed", .{ + pub fn get_embeds(self: *parser.DocumentHTML) collection.HTMLCollection { + return collection.HTMLCollectionByTagName(parser.documentHTMLToNode(self), "embed", .{ .include_root = false, }); } - pub fn get_plugins(self: *parser.DocumentHTML, page: *Page) !collection.HTMLCollection { - return get_embeds(self, page); + pub fn get_plugins(self: *parser.DocumentHTML) collection.HTMLCollection { + return get_embeds(self); } - pub fn get_forms(self: *parser.DocumentHTML, page: *Page) !collection.HTMLCollection { - return try collection.HTMLCollectionByTagName(page.arena, parser.documentHTMLToNode(self), "form", .{ + pub fn get_forms(self: *parser.DocumentHTML) collection.HTMLCollection { + return collection.HTMLCollectionByTagName(parser.documentHTMLToNode(self), "form", .{ .include_root = false, }); } - pub fn get_scripts(self: *parser.DocumentHTML, page: *Page) !collection.HTMLCollection { - return try collection.HTMLCollectionByTagName(page.arena, parser.documentHTMLToNode(self), "script", .{ + pub fn get_scripts(self: *parser.DocumentHTML) collection.HTMLCollection { + return collection.HTMLCollectionByTagName(parser.documentHTMLToNode(self), "script", .{ .include_root = false, }); } - pub fn get_applets(_: *parser.DocumentHTML) !collection.HTMLCollection { - return try collection.HTMLCollectionEmpty(); + pub fn get_applets(_: *parser.DocumentHTML) collection.HTMLCollection { + return collection.HTMLCollectionEmpty(); } - pub fn get_links(self: *parser.DocumentHTML) !collection.HTMLCollection { - return try collection.HTMLCollectionByLinks(parser.documentHTMLToNode(self), .{ + pub fn get_links(self: *parser.DocumentHTML) collection.HTMLCollection { + return collection.HTMLCollectionByLinks(parser.documentHTMLToNode(self), .{ .include_root = false, }); } - pub fn get_anchors(self: *parser.DocumentHTML) !collection.HTMLCollection { - return try collection.HTMLCollectionByAnchors(parser.documentHTMLToNode(self), .{ + pub fn get_anchors(self: *parser.DocumentHTML) collection.HTMLCollection { + return collection.HTMLCollectionByAnchors(parser.documentHTMLToNode(self), .{ .include_root = false, }); } diff --git a/src/browser/html/window.zig b/src/browser/html/window.zig index 6f1d5fe8..3ad2e846 100644 --- a/src/browser/html/window.zig +++ b/src/browser/html/window.zig @@ -246,10 +246,10 @@ pub const Window = struct { return self.createTimeout(cbk, 0, page, .{ .name = "queueMicrotask" }); } - pub fn _matchMedia(_: *const Window, media: []const u8, page: *Page) !MediaQueryList { + pub fn _matchMedia(_: *const Window, media: Env.String) !MediaQueryList { return .{ .matches = false, // TODO? - .media = try page.arena.dupe(u8, media), + .media = media.string, }; } diff --git a/src/runtime/js.zig b/src/runtime/js.zig index dbbc26a2..4d554f31 100644 --- a/src/runtime/js.zig +++ b/src/runtime/js.zig @@ -1196,6 +1196,10 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { return try self.createFunction(js_value); } + if (T == String) { + return .{ .string = try valueToString(self.context_arena, js_value, self.isolate, self.v8_context) }; + } + const js_obj = js_value.castTo(v8.Object); if (comptime isJsObject(T)) { @@ -2184,6 +2188,15 @@ pub fn Env(comptime State: type, comptime WebApis: type) type { promise: v8.Promise, }; + // When doing jsValueToZig, string ([]const u8) are managed by the + // call_arena. That means that if the API wants to persist the string + // (which is relatively common), it needs to dupe it again. + // If the parameter is an Env.String rather than a []const u8, then + // the page's arena will be used (rather than the call arena). + pub const String = struct { + string: []const u8, + }; + pub const Inspector = struct { isolate: v8.Isolate, inner: *v8.Inspector, diff --git a/src/tests/dom/html_collection.html b/src/tests/dom/html_collection.html index 0f65c715..baac8424 100644 --- a/src/tests/dom/html_collection.html +++ b/src/tests/dom/html_collection.html @@ -10,50 +10,57 @@ - - let getElementsByTagName = document.getElementsByTagName('p'); - testing.expectEqual(2, getElementsByTagName.length); + + + +