Merge pull request #761 from lightpanda-io/pozo_for_custom_state

Improve usability of NodeWrapper

Author: karlseguin

src/browser/State.zig

@@ -0,0 +1,65 @@
+// Copyright (C) 2023-2024  Lightpanda (Selecy SAS)
+//
+// Francis Bouvier <[email protected]>
+// Pierre Tachoire <[email protected]>
+//
+// 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 <https://www.gnu.org/licenses/>.
+
+// Sometimes we need to extend libdom. For example, its HTMLDocument doesn't
+// have a readyState. We have a couple different options, such as making the
+// correction in libdom directly. Another option stems from the fact that every
+// libdom node has an opaque embedder_data field. This is the struct that we
+// lazily load into that field.
+//
+// It didn't originally start off as a collection of every single extension, but
+// this quickly proved necessary, since different fields are needed on the same
+// data at different levels of the prototype chain. This isn't memory efficient.
+
+const Env = @import("env.zig").Env;
+const parser = @import("netsurf.zig");
+const CSSStyleDeclaration = @import("cssom/css_style_declaration.zig").CSSStyleDeclaration;
+
+// for HTMLScript (but probably needs to be added to more)
+onload: ?Env.Function = null,
+onerror: ?Env.Function = null,
+
+// for HTMLElement
+style: CSSStyleDeclaration = .empty,
+
+// for html/document
+ready_state: ReadyState = .loading,
+
+// for dom/document
+active_element: ?*parser.Element = null,
+
+// for HTMLSelectElement
+// By default, if no option is explicitly selected, the first option should
+// be selected. However, libdom doesn't do this, and it sets the
+// selectedIndex to -1, which is a valid value for "nothing selected".
+// Therefore, when libdom says the selectedIndex == -1, we don't know if
+// it means that nothing is selected, or if the first option is selected by
+// default.
+// There are cases where this won't work, but when selectedIndex is
+// explicitly set, we set this boolean flag. Then, when we're getting then
+// selectedIndex, if this flag is == false, which is to say that if
+// selectedIndex hasn't been explicitly set AND if we have at least 1 option
+// AND if it isn't a multi select, we can make the 1st item selected by
+// default (by returning selectedIndex == 0).
+explicit_index_set: bool = false,
+
+const ReadyState = enum {
+    loading,
+    interactive,
+    complete,
+};

src/browser/browser.zig

@@ -21,6 +21,7 @@ const std = @import("std");
 const Allocator = std.mem.Allocator;
 const ArenaAllocator = std.heap.ArenaAllocator;
 
+const State = @import("State.zig");
 const Env = @import("env.zig").Env;
 const App = @import("../app.zig").App;
 const Session = @import("session.zig").Session;
@@ -41,6 +42,7 @@ pub const Browser = struct {
     session_arena: ArenaAllocator,
     transfer_arena: ArenaAllocator,
     notification: *Notification,
+    state_pool: std.heap.MemoryPool(State),
 
     pub fn init(app: *App) !Browser {
         const allocator = app.allocator;
@@ -61,6 +63,7 @@ pub const Browser = struct {
             .page_arena = ArenaAllocator.init(allocator),
             .session_arena = ArenaAllocator.init(allocator),
             .transfer_arena = ArenaAllocator.init(allocator),
+            .state_pool = std.heap.MemoryPool(State).init(allocator),
         };
     }
 
@@ -71,6 +74,7 @@ pub const Browser = struct {
         self.session_arena.deinit();
         self.transfer_arena.deinit();
         self.notification.deinit();
+        self.state_pool.deinit();
     }
 
     pub fn newSession(self: *Browser) !*Session {

src/browser/dom/document.zig

@@ -42,8 +42,6 @@ pub const Document = struct {
     pub const prototype = *Node;
     pub const subtype = .node;
 
-    active_element: ?*parser.Element = null,
-
     pub fn constructor(page: *const Page) !*parser.DocumentHTML {
         const doc = try parser.documentCreateDocument(
             try parser.documentHTMLGetTitle(page.window.document),
@@ -245,17 +243,26 @@ pub const Document = struct {
         return try TreeWalker.init(root, what_to_show, filter);
     }
 
-    pub fn get_activeElement(doc: *parser.Document, page: *Page) !?ElementUnion {
-        const self = try page.getOrCreateNodeWrapper(Document, @ptrCast(doc));
-        if (self.active_element) |ae| {
+    pub fn get_activeElement(self: *parser.Document, page: *Page) !?ElementUnion {
+        const state = try page.getOrCreateNodeState(@ptrCast(self));
+        if (state.active_element) |ae| {
             return try Element.toInterface(ae);
         }
 
         if (try parser.documentHTMLBody(page.window.document)) |body| {
             return try Element.toInterface(@ptrCast(body));
         }
 
-        return get_documentElement(doc);
+        return get_documentElement(self);
+    }
+
+    // TODO: some elements can't be focused, like if they're disabled
+    // but there doesn't seem to be a generic way to check this. For example
+    // we could look for the "disabled" attribute, but that's only meaningful
+    // on certain types, and libdom's vtable doesn't seem to expose this.
+    pub fn setFocus(self: *parser.Document, e: *parser.ElementHTML, page: *Page) !void {
+        const state = try page.getOrCreateNodeState(@ptrCast(self));
+        state.active_element = @ptrCast(e);
     }
 };
 

src/browser/html/document.zig

@@ -39,14 +39,6 @@ pub const HTMLDocument = struct {
     pub const prototype = *Document;
     pub const subtype = .node;
 
-    ready_state: ReadyState = .loading,
-
-    const ReadyState = enum {
-        loading,
-        interactive,
-        complete,
-    };
-
     // JS funcs
     // --------
 
@@ -191,9 +183,9 @@ pub const HTMLDocument = struct {
         return &page.window;
     }
 
-    pub fn get_readyState(node: *parser.DocumentHTML, page: *Page) ![]const u8 {
-        const self = try page.getOrCreateNodeWrapper(HTMLDocument, @ptrCast(node));
-        return @tagName(self.ready_state);
+    pub fn get_readyState(self: *parser.DocumentHTML, page: *Page) ![]const u8 {
+        const state = try page.getOrCreateNodeState(@ptrCast(self));
+        return @tagName(state.ready_state);
     }
 
     // noop legacy functions
@@ -270,9 +262,9 @@ pub const HTMLDocument = struct {
         return list.items;
     }
 
-    pub fn documentIsLoaded(html_doc: *parser.DocumentHTML, page: *Page) !void {
-        const self = try page.getOrCreateNodeWrapper(HTMLDocument, @ptrCast(html_doc));
-        self.ready_state = .interactive;
+    pub fn documentIsLoaded(self: *parser.DocumentHTML, page: *Page) !void {
+        const state = try page.getOrCreateNodeState(@ptrCast(self));
+        state.ready_state = .interactive;
 
         const evt = try parser.eventCreate();
         defer parser.eventDestroy(evt);
@@ -282,12 +274,12 @@ pub const HTMLDocument = struct {
             .source = "document",
         });
         try parser.eventInit(evt, "DOMContentLoaded", .{ .bubbles = true, .cancelable = true });
-        _ = try parser.eventTargetDispatchEvent(parser.toEventTarget(parser.DocumentHTML, html_doc), evt);
+        _ = try parser.eventTargetDispatchEvent(parser.toEventTarget(parser.DocumentHTML, self), evt);
     }
 
-    pub fn documentIsComplete(html_doc: *parser.DocumentHTML, page: *Page) !void {
-        const self = try page.getOrCreateNodeWrapper(HTMLDocument, @ptrCast(html_doc));
-        self.ready_state = .complete;
+    pub fn documentIsComplete(self: *parser.DocumentHTML, page: *Page) !void {
+        const state = try page.getOrCreateNodeState(@ptrCast(self));
+        state.ready_state = .complete;
     }
 };
 

src/browser/html/elements.zig

@@ -112,11 +112,9 @@ pub const HTMLElement = struct {
     pub const prototype = *Element;
     pub const subtype = .node;
 
-    style: CSSStyleDeclaration = .empty,
-
     pub fn get_style(e: *parser.ElementHTML, page: *Page) !*CSSStyleDeclaration {
-        const self = try page.getOrCreateNodeWrapper(HTMLElement, @ptrCast(e));
-        return &self.style;
+        const state = try page.getOrCreateNodeState(@ptrCast(e));
+        return &state.style;
     }
 
     pub fn get_innerText(e: *parser.ElementHTML) ![]const u8 {
@@ -159,16 +157,9 @@ pub const HTMLElement = struct {
             return;
         }
 
-        const root_node = try parser.nodeGetRootNode(@ptrCast(e));
-
         const Document = @import("../dom/document.zig").Document;
-        const document = try page.getOrCreateNodeWrapper(Document, @ptrCast(root_node));
-
-        // TODO: some elements can't be focused, like if they're disabled
-        // but there doesn't seem to be a generic way to check this. For example
-        // we could look for the "disabled" attribute, but that's only meaningful
-        // on certain types, and libdom's vtable doesn't seem to expose this.
-        document.active_element = @ptrCast(e);
+        const root_node = try parser.nodeGetRootNode(@ptrCast(e));
+        try Document.setFocus(@ptrCast(root_node), e, page);
     }
 };
 
@@ -852,9 +843,6 @@ pub const HTMLScriptElement = struct {
     pub const prototype = *HTMLElement;
     pub const subtype = .node;
 
-    onload: ?Env.Function = null,
-    onerror: ?Env.Function = null,
-
     pub fn get_src(self: *parser.Script) !?[]const u8 {
         return try parser.elementGetAttribute(
             parser.scriptToElt(self),
@@ -964,24 +952,24 @@ pub const HTMLScriptElement = struct {
         return try parser.elementRemoveAttribute(parser.scriptToElt(self), "nomodule");
     }
 
-    pub fn get_onload(script: *parser.Script, page: *Page) !?Env.Function {
-        const self = page.getNodeWrapper(HTMLScriptElement, @ptrCast(script)) orelse return null;
-        return self.onload;
+    pub fn get_onload(self: *parser.Script, page: *Page) !?Env.Function {
+        const state = page.getNodeState(@ptrCast(self)) orelse return null;
+        return state.onload;
     }
 
-    pub fn set_onload(script: *parser.Script, function: ?Env.Function, page: *Page) !void {
-        const self = try page.getOrCreateNodeWrapper(HTMLScriptElement, @ptrCast(script));
-        self.onload = function;
+    pub fn set_onload(self: *parser.Script, function: ?Env.Function, page: *Page) !void {
+        const state = try page.getOrCreateNodeState(@ptrCast(self));
+        state.onload = function;
     }
 
-    pub fn get_onerror(script: *parser.Script, page: *Page) !?Env.Function {
-        const self = page.getNodeWrapper(HTMLScriptElement, @ptrCast(script)) orelse return null;
-        return self.onerror;
+    pub fn get_onerror(self: *parser.Script, page: *Page) !?Env.Function {
+        const state = page.getNodeState(@ptrCast(self)) orelse return null;
+        return state.onerror;
     }
 
-    pub fn set_onerror(script: *parser.Script, function: ?Env.Function, page: *Page) !void {
-        const self = try page.getOrCreateNodeWrapper(HTMLScriptElement, @ptrCast(script));
-        self.onerror = function;
+    pub fn set_onerror(self: *parser.Script, function: ?Env.Function, page: *Page) !void {
+        const state = try page.getOrCreateNodeState(@ptrCast(self));
+        state.onerror = function;
     }
 };
 

src/browser/html/select.zig

@@ -26,20 +26,6 @@ pub const HTMLSelectElement = struct {
     pub const prototype = *HTMLElement;
     pub const subtype = .node;
 
-    // By default, if no option is explicitly selected, the first option should
-    // be selected. However, libdom doesn't do this, and it sets the
-    // selectedIndex to -1, which is a valid value for "nothing selected".
-    // Therefore, when libdom says the selectedIndex == -1, we don't know if
-    // it means that nothing is selected, or if the first option is selected by
-    // default.
-    // There are cases where this won't work, but when selectedIndex is
-    // explicitly set, we set this boolean flag. Then, when we're getting then
-    // selectedIndex, if this flag is == false, which is to say that if
-    // selectedIndex hasn't been explicitly set AND if we have at least 1 option
-    // AND if it isn't a multi select, we can make the 1st item selected by
-    // default (by returning selectedIndex == 0).
-    explicit_index_set: bool = false,
-
     pub fn get_length(select: *parser.Select) !u32 {
         return parser.selectGetLength(select);
     }
@@ -70,11 +56,11 @@ pub const HTMLSelectElement = struct {
     }
 
     pub fn get_selectedIndex(select: *parser.Select, page: *Page) !i32 {
-        const self = try page.getOrCreateNodeWrapper(HTMLSelectElement, @ptrCast(select));
+        const state = try page.getOrCreateNodeState(@ptrCast(select));
         const selected_index = try parser.selectGetSelectedIndex(select);
 
         // See the explicit_index_set field documentation
-        if (!self.explicit_index_set) {
+        if (!state.explicit_index_set) {
             if (selected_index == -1) {
                 if (try parser.selectGetMultiple(select) == false) {
                     if (try get_length(select) > 0) {
@@ -89,8 +75,8 @@ pub const HTMLSelectElement = struct {
     // Libdom's dom_html_select_select_set_selected_index will crash if index
     // is out of range, and it doesn't properly unset options
     pub fn set_selectedIndex(select: *parser.Select, index: i32, page: *Page) !void {
-        var self = try page.getOrCreateNodeWrapper(HTMLSelectElement, @ptrCast(select));
-        self.explicit_index_set = true;
+        var state = try page.getOrCreateNodeState(@ptrCast(select));
+        state.explicit_index_set = true;
 
         const options = try parser.selectGetOptions(select);
         const len = try parser.optionCollectionGetLength(options);

src/browser/page.zig

@@ -22,6 +22,7 @@ const builtin = @import("builtin");
 const Allocator = std.mem.Allocator;
 
 const Dump = @import("dump.zig");
+const State = @import("State.zig");
 const Env = @import("env.zig").Env;
 const Mime = @import("mime.zig").Mime;
 const DataURI = @import("datauri.zig").DataURI;
@@ -95,6 +96,8 @@ pub const Page = struct {
     // indicates intention to navigate to another page on the next loop execution.
     delayed_navigation: bool = false,
 
+    state_pool: *std.heap.MemoryPool(State),
+
     pub fn init(self: *Page, arena: Allocator, session: *Session) !void {
         const browser = session.browser;
         self.* = .{
@@ -106,6 +109,7 @@ pub const Page = struct {
             .call_arena = undefined,
             .loop = browser.app.loop,
             .renderer = Renderer.init(arena),
+            .state_pool = &browser.state_pool,
             .cookie_jar = &session.cookie_jar,
             .microtask_node = .{ .func = microtaskCallback },
             .window_clicked_event_node = .{ .func = windowClicked },
@@ -597,21 +601,21 @@ pub const Page = struct {
         _ = try self.loop.timeout(0, &navi.navigate_node);
     }
 
-    pub fn getOrCreateNodeWrapper(self: *Page, comptime T: type, node: *parser.Node) !*T {
-        if (self.getNodeWrapper(T, node)) |wrap| {
+    pub fn getOrCreateNodeState(self: *Page, node: *parser.Node) !*State {
+        if (self.getNodeState(node)) |wrap| {
             return wrap;
         }
 
-        const wrap = try self.arena.create(T);
-        wrap.* = T{};
+        const state = try self.state_pool.create();
+        state.* = .{};
 
-        parser.nodeSetEmbedderData(node, wrap);
-        return wrap;
+        parser.nodeSetEmbedderData(node, state);
+        return state;
     }
 
-    pub fn getNodeWrapper(_: *const Page, comptime T: type, node: *parser.Node) ?*T {
-        if (parser.nodeGetEmbedderData(node)) |wrap| {
-            return @alignCast(@ptrCast(wrap));
+    pub fn getNodeState(_: *const Page, node: *parser.Node) ?*State {
+        if (parser.nodeGetEmbedderData(node)) |state| {
+            return @alignCast(@ptrCast(state));
         }
         return null;
     }
@@ -743,8 +747,7 @@ const Script = struct {
             // attached to it. But this seems quite unlikely and it does help
             // optimize loading scripts, of which there can be hundreds for a
             // page.
-            const HTMLScriptElement = @import("html/elements.zig").HTMLScriptElement;
-            if (page.getNodeWrapper(HTMLScriptElement, @ptrCast(e))) |se| {
+            if (page.getNodeState(@ptrCast(e))) |se| {
                 if (se.onload) |function| {
                     onload = .{ .function = function };
                 }

src/browser/session.zig

@@ -90,6 +90,7 @@ pub const Session = struct {
 
         const page_arena = &self.browser.page_arena;
         _ = page_arena.reset(.{ .retain_with_limit = 1 * 1024 * 1024 });
+        _ = self.browser.state_pool.reset(.{ .retain_with_limit = 4 * 1024 });
 
         self.page = @as(Page, undefined);
         const page = &self.page.?;