mem: Optional CSSStyleDeclaration materialization

StyleManager ultimately ends up calling el.getOrCreateStyle() which either returns the element's CSSStyleProperties OR (creates it AND stores it in the Frame._element_styles for future lookups). The goal behind this caching is twofold: 1 - Performance of not having to reparse the "style" attribute 2 - Identity: two calls from JS to get the properties should return the same value (2) is non-negotiable, so the 'getOrCreate' _has_ to exist for JS-facing APIs. But (1) is CPU vs memory optimization that we've decided should always favor the CPU. But, in any case where we dump an entire tree, that memory cost can be significant (# of elements with a style attribute) and the CPU gains are questionable (it isn't like a JS loop re-checking an element's properties, it's a one-time dump). So, the StyleManager now takes a comptime `InlineAccess` which is either `.scan` or `.materialize`. When it's `.materialize` it behaves as before. When it's `.scan` is will use an existing `_element_styles` if available else it will re-parse but not store the value.
2026-09-14 15:03:27 -04:00 · 2026-08-26 13:23:11 +08:00
1 parent e76cc816c0
commit e4a36552b8
@@ -131,7 +131,7 @@ fn walk(
        if (tag == .datalist or tag == .option or tag == .optgroup) return;

        // Check visibility using the engine's checkVisibility which handles CSS display: none
-        if (!el.checkVisibilityCached(ctx.visibility_cache, self.frame)) {
+        if (!el.checkVisibilityCached(ctx.visibility_cache, self.frame, .scan)) {
            return;
        }

@@ -33,7 +33,6 @@ const SelectorList = @import("webapi/selector/List.zig");
 const CSSStyleRule = @import("webapi/css/CSSStyleRule.zig");
 const CSSStyleSheet = @import("webapi/css/CSSStyleSheet.zig");
 const CSSStyleProperties = @import("webapi/css/CSSStyleProperties.zig");
-const CSSStyleProperty = @import("webapi/css/CSSStyleDeclaration.zig").Property;

 const log = lp.log;
 const String = lp.String;
@@ -573,7 +572,7 @@ fn rebuildIfDirty(self: *StyleManager) !void {
 // Check if an element is hidden based on options.
 // By default only checks display:none.
 // Walks up the tree to check ancestors.
-pub fn isHidden(self: *StyleManager, el: *Element, cache: ?*VisibilityCache, options: CheckVisibilityOptions) bool {
+pub fn isHidden(self: *StyleManager, el: *Element, cache: ?*VisibilityCache, options: CheckVisibilityOptions, comptime access: InlineAccess) bool {
    self.rebuildIfDirty() catch return false;

    var current: ?*Element = el;
@@ -590,7 +589,7 @@ pub fn isHidden(self: *StyleManager, el: *Element, cache: ?*VisibilityCache, opt
            }
        }

-        const hidden = self.isElementHidden(elem, options);
+        const hidden = self.isElementHidden(elem, options, access);

        // Store in cache
        if (cache) |c| {
@@ -611,18 +610,18 @@ pub fn isHidden(self: *StyleManager, el: *Element, cache: ?*VisibilityCache, opt
 /// Computed display:none for a single element (own property, no ancestor walk).
 /// Honors the UA stylesheet rules per HTML Rendering §15.3.1 "Hidden elements"
 /// via `isElementHidden`.
-pub fn hasDisplayNone(self: *StyleManager, el: *Element) bool {
+pub fn hasDisplayNone(self: *StyleManager, el: *Element, comptime access: InlineAccess) bool {
    self.rebuildIfDirty() catch return false;
-    return self.isElementHidden(el, .{});
+    return self.isElementHidden(el, .{}, access);
 }

 /// Computed display:none coming only from inline style or an author stylesheet
 /// rule — the UA stylesheet's hidden elements (<head>, <script>, [hidden], …)
 /// are NOT counted, so document scaffolding is preserved. Used by the HTML
 /// dump's "invisible" strip mode.
-pub fn hasAuthorDisplayNone(self: *StyleManager, el: *Element) bool {
+pub fn hasAuthorDisplayNone(self: *StyleManager, el: *Element, comptime access: InlineAccess) bool {
    self.rebuildIfDirty() catch return false;
-    return self.isElementHidden(el, .{ .ua_display_none = false });
+    return self.isElementHidden(el, .{ .ua_display_none = false }, access);
 }

 /// Centralizes UA-stylesheet display:none truth so `getComputedStyle().display`
@@ -667,7 +666,7 @@ pub fn hasVisibilityHiddenInherited(self: *StyleManager, el: *Element) bool {
    self.rebuildIfDirty() catch return false;
    var current: ?*Element = el;
    while (current) |elem| {
-        if (self.isElementHidden(elem, .{ .check_display = false, .check_visibility = true })) {
+        if (self.isElementHidden(elem, .{ .check_display = false, .check_visibility = true }, .materialize)) {
            return true;
        }
        current = elem.parentElement();
@@ -676,7 +675,7 @@ pub fn hasVisibilityHiddenInherited(self: *StyleManager, el: *Element) bool {
 }

 /// Check if a single element (not ancestors) is hidden.
-fn isElementHidden(self: *StyleManager, el: *Element, options: CheckVisibilityOptions) bool {
+fn isElementHidden(self: *StyleManager, el: *Element, options: CheckVisibilityOptions, comptime access: InlineAccess) bool {
    // Track best match per property (value + priority)
    // Initialize priority to INLINE_PRIORITY for properties we don't care about - this makes
    // the loop naturally skip them since no stylesheet rule can have priority >= INLINE_PRIORITY
@@ -691,8 +690,8 @@ fn isElementHidden(self: *StyleManager, el: *Element, options: CheckVisibilityOp

    // Check inline styles FIRST - they use INLINE_PRIORITY so no stylesheet can beat them
    if (options.check_display) {
-        if (getInlineStyleProperty(el, comptime .wrap("display"), self.frame)) |property| {
-            if (property._value.eqlSliceIgnoreCase("none")) {
+        if (inlineValue(el, comptime .wrap("display"), self.frame, access)) |value| {
+            if (std.ascii.eqlIgnoreCase(value, "none")) {
                return true; // Early exit for hiding value
            }
            display_none = false;
@@ -704,8 +703,8 @@ fn isElementHidden(self: *StyleManager, el: *Element, options: CheckVisibilityOp
    }

    if (options.check_visibility) {
-        if (getInlineStyleProperty(el, comptime .wrap("visibility"), self.frame)) |property| {
-            if (property._value.eqlSliceIgnoreCase("hidden") or property._value.eqlSliceIgnoreCase("collapse")) {
+        if (inlineValue(el, comptime .wrap("visibility"), self.frame, access)) |value| {
+            if (std.ascii.eqlIgnoreCase(value, "hidden") or std.ascii.eqlIgnoreCase(value, "collapse")) {
                return true;
            }
            visibility_hidden = false;
@@ -719,8 +718,8 @@ fn isElementHidden(self: *StyleManager, el: *Element, options: CheckVisibilityOp
    }

    if (options.check_opacity) {
-        if (getInlineStyleProperty(el, comptime .wrap("opacity"), self.frame)) |property| {
-            if (property._value.eqlSliceIgnoreCase("0")) {
+        if (inlineValue(el, comptime .wrap("opacity"), self.frame, access)) |value| {
+            if (std.ascii.eqlIgnoreCase(value, "0")) {
                return true;
            }
            opacity_zero = false;
@@ -874,8 +873,8 @@ fn elementHasPointerEventsNone(self: *StyleManager, el: *Element) bool {
    const frame = self.frame;

    // Check inline style first
-    if (getInlineStyleProperty(el, .wrap("pointer-events"), frame)) |property| {
-        if (property._value.eqlSliceIgnoreCase("none")) {
+    if (inlineValue(el, .wrap("pointer-events"), frame, .materialize)) |value| {
+        if (std.ascii.eqlIgnoreCase(value, "none")) {
            return true;
        }
        return false;
@@ -1194,22 +1193,67 @@ const CheckVisibilityOptions = struct {
 // its field max, so a real rule can never pack to all-ones.
 const INLINE_PRIORITY: u64 = std.math.maxInt(u64);

+/// How a probe reads an element's inline `style=` attribute.
+pub const InlineAccess = enum {
+    /// Parse the attribute into the element's CSSStyleProperties (the object
+    /// `el.style` hands out) and keep it, so repeat probes on the same element
+    /// cost a list lookup. Layout reads that object directly (getElementAxis,
+    /// horizontalPosition), so every JS-reachable path must materialize.
+    materialize,
+    /// Fold the attribute text on each call; allocates nothing. For
+    /// Zig-initiated tooling that walks the whole document once per turn
+    /// (markdown, dump, tree), where pinning a parsed declaration list in the
+    /// page arena for every inline-styled element outlives its use. An
+    /// object JS already created is still read.
+    scan,
+};
+
 // `frame` is the StyleManager's frame, which callers guarantee is el's owner
 // frame (el.ownerFrame) — the map where the materialized style lives.
-fn getInlineStyleProperty(el: *Element, property_name: String, frame: *Frame) ?*CSSStyleProperty {
+fn inlineValue(el: *Element, property_name: String, comptime access: InlineAccess, frame: *Frame) ?[]const u8 {
    if (!el._flags.has_inline_style) {
        // Neither a style object nor a style attribute; skip both lookups.
        return null;
    }
-    const style = el.getStyle(frame) orelse blk: {
-        // No JS-set style object and no style attribute -> nothing inline to read.
-        if (el.getAttributeSafe(comptime .wrap("style")) == null) return null;
-        break :blk el.getOrCreateStyle(frame) catch |err| {
-            log.err(.browser, "StyleManager getOrCreateStyle", .{ .err = err });
-            return null;
-        };
-    };
-    return style.asCSSStyleDeclaration().findProperty(property_name);
+    if (el.getStyle(frame)) |style| {
+        return styleValue(style, property_name);
+    }
+    // No JS-set style object and no style attribute -> nothing inline to read.
+    const attr = el.getAttributeSafe(comptime .wrap("style")) orelse return null;
+    switch (access) {
+        .materialize => {
+            const style = el.getOrCreateStyle(frame) catch |err| {
+                log.err(.browser, "StyleManager getOrCreateStyle", .{ .err = err });
+                return null;
+            };
+            return styleValue(style, property_name);
+        },
+        .scan => return scanInlineValue(attr, property_name.str()),
+    }
+}
+
+fn styleValue(style: *CSSStyleProperties, property_name: String) ?[]const u8 {
+    const property = style.asCSSStyleDeclaration().findProperty(property_name) orelse return null;
+    return property._value.str();
+}
+
+// Must agree with CSSStyleDeclaration.applyDeclarations, which is what the
+// materialized object was built from.
+fn scanInlineValue(attr: []const u8, property_name: []const u8) ?[]const u8 {
+    var value: ?[]const u8 = null;
+    var important = false;
+    var it = CssParser.parseDeclarationsList(attr);
+    while (it.next()) |declaration| {
+        if (std.ascii.eqlIgnoreCase(declaration.name, property_name) == false) {
+            continue;
+        }
+        if (important and declaration.important == false) {
+            continue;
+        }
+        value = declaration.value;
+        important = declaration.important;
+    }
+    return value;
 }

 /// Resolved value of an element's inline `style=` declaration for `property_name`,
@@ -1217,8 +1261,7 @@ fn getInlineStyleProperty(el: *Element, property_name: String, frame: *Frame) ?*
 /// inline style (the same source `el.style` exposes), so `getComputedStyle` and
 /// `el.style` agree on inline values instead of resolving them independently.
 pub fn inlineStyleValue(self: *StyleManager, el: *Element, property_name: String) ?[]const u8 {
-    const property = getInlineStyleProperty(el, property_name, self.frame) orelse return null;
-    return property._value.str();
+    return inlineValue(el, property_name, self.frame, .materialize);
 }

 /// Bounds computedFontSize's ancestor recursion (the parent walk and
@@ -1440,3 +1483,39 @@ test "StyleManager: packed priority bounds" {
    // Real layer ranks fit the 12 rank bits below the unlayered sentinel.
    try testing.expect(MAX_LAYERS < UNLAYERED_RANK);
 }
+
+test "StyleManager: inlineValue: scan matches materialize" {
+    const frame = try testing.createFrame();
+    defer testing.test_session.closeAllPages();
+
+    const div = try frame.window._document.createElement("div", null, frame);
+    try Frame.parse.htmlAsChildren(frame, div.asNode(),
+        \\<i style="display:none"></i>
+        \\<i style="color:red; DISPLAY : none !important ; display:block"></i>
+        \\<i style="display:block;display:none"></i>
+        \\<i style="display: none; display: /* c */ block"></i>
+        \\<i style="visibility:hidden"></i>
+        \\<i style="display:"></i>
+        \\<i></i>
+    );
+    const expected = [_]?[]const u8{ "none", "none", "none", "block", null, null, null };
+
+    var i: usize = 0;
+    var child = div.asNode().firstChild();
+    while (child) |node| : (child = node.nextSibling()) {
+        const el = node.is(Element) orelse continue;
+        const scanned = inlineValue(el, comptime .wrap("display"), frame, .scan);
+        // scanning never creates the style object
+        try testing.expectEqual(null, el.getStyle(frame));
+        const materialized = inlineValue(el, comptime .wrap("display"), frame, .materialize);
+        if (expected[i]) |value| {
+            try testing.expectEqual(value, scanned);
+            try testing.expectEqual(value, materialized);
+        } else {
+            try testing.expectEqual(null, scanned);
+            try testing.expectEqual(null, materialized);
+        }
+        i += 1;
+    }
+    try testing.expectEqual(expected.len, i);
+}
@@ -384,7 +384,7 @@ fn shouldStripElement(el: *Node.Element, opts: Opts, frame: *Frame) bool {
        if (std.mem.eql(u8, tag_name, "iframe")) return true;
    }

-    if (opts.strip.invisible and frame._style_manager.hasAuthorDisplayNone(el)) {
+    if (opts.strip.invisible and frame._style_manager.hasAuthorDisplayNone(el, .scan)) {
        return true;
    }

@@ -150,7 +150,7 @@ fn isSignificantText(node: *Node) bool {
 fn isVisibleElement(el: *Element, frame: *Frame) bool {
    const tag = el.getTag();
    if (tag.isMetadata() or tag == .svg) return false;
-    if (frame._style_manager.hasDisplayNone(el)) return false;
+    if (frame._style_manager.hasDisplayNone(el, .scan)) return false;
    if (el.getAttributeSafe(comptime .wrap("aria-hidden"))) |v| {
        if (std.ascii.eqlIgnoreCase(v, "true")) return false;
    }
@@ -916,7 +916,7 @@ fn elementFromPointImpl(self: *Document, x: f64, y: f64, ignore_x: bool, frame:

        preorder_index += 1;
        if (node.is(Element)) |element| {
-            if (element.checkVisibilityCached(&visibility_cache, frame)) {
+            if (element.checkVisibilityCached(&visibility_cache, frame, .materialize)) {
                if (y >= pos and y <= pos + element.boxAxis(frame, .height)) {
                    if (ignore_x) {
                        topmost = element;
@@ -1128,7 +1128,7 @@ pub fn focus(self: *Element, frame: *Frame) !void {

    // Per HTML spec §6.4.4, an element must be "being rendered" (not
    // display:none on self or any ancestor) to be focusable.
-    if (!self.checkVisibilityCached(null, frame)) {
+    if (!self.checkVisibilityCached(null, frame, .materialize)) {
        return;
    }

@@ -1329,8 +1329,8 @@ pub fn hasPointerEventsNone(self: *Element, cache: ?*PointerEventsCache, frame:
    return self.ownerFrame(frame)._style_manager.hasPointerEventsNone(self, cache);
 }

-pub fn checkVisibilityCached(self: *Element, cache: ?*VisibilityCache, frame: *Frame) bool {
-    return !self.ownerFrame(frame)._style_manager.isHidden(self, cache, .{});
+pub fn checkVisibilityCached(self: *Element, cache: ?*VisibilityCache, frame: *Frame, comptime access: StyleManager.InlineAccess) bool {
+    return !self.ownerFrame(frame)._style_manager.isHidden(self, cache, .{}, access);
 }

 // The element's own display:none only, no ancestor walk. For a child or
@@ -1338,7 +1338,7 @@ pub fn checkVisibilityCached(self: *Element, cache: ?*VisibilityCache, frame: *F
 // answer: they share the visible ancestor chain — and the owner frame, which
 // the caller resolves once rather than per element.
 fn isVisibleSelf(self: *Element, style_manager: *StyleManager) bool {
-    return !style_manager.hasDisplayNone(self);
+    return !style_manager.hasDisplayNone(self, .materialize);
 }

 const CheckVisibilityOpts = struct {
@@ -1352,7 +1352,7 @@ pub fn checkVisibility(self: *Element, opts_: ?CheckVisibilityOpts, frame: *Fram
    return !self.ownerFrame(frame)._style_manager.isHidden(self, null, .{
        .check_opacity = opts.checkOpacity or opts.opacityProperty,
        .check_visibility = opts.visibilityProperty or opts.checkVisibilityCSS,
-    });
+    }, .materialize);
 }

 pub const Axis = enum {
@@ -1398,14 +1398,14 @@ pub fn getElementAxis(self: *Element, frame: *Frame, comptime axis: Axis) Axis.S
 // width / height treshold is reached. If the size isn't explicit, we fallback
 // to the content size.
 pub fn getClientWidth(self: *Element, frame: *Frame) f64 {
-    if (!self.checkVisibilityCached(null, frame)) {
+    if (!self.checkVisibilityCached(null, frame, .materialize)) {
        return 0.0;
    }
    return self.boxAxis(frame, .width);
 }

 pub fn getClientHeight(self: *Element, frame: *Frame) f64 {
-    if (!self.checkVisibilityCached(null, frame)) {
+    if (!self.checkVisibilityCached(null, frame, .materialize)) {
        return 0.0;
    }
    return self.boxAxis(frame, .height);
@@ -1436,7 +1436,7 @@ pub fn getBoundingClientRect(self: *Element, frame: *Frame) !*DOMRect {
 // getBoundingClientRect, getClientRects, and IntersectionObserver. A DOMRect is
 // only materialized at the JS boundary.
 pub fn boundingClientRectValues(self: *Element, frame: *Frame) DOMRect.Data {
-    if (!self.checkVisibilityCached(null, frame)) {
+    if (!self.checkVisibilityCached(null, frame, .materialize)) {
        return .{};
    }
    return self.boundingClientRectValuesForVisible(frame);
@@ -1453,7 +1453,7 @@ pub fn boundingClientRectValuesForVisible(self: *Element, frame: *Frame) DOMRect
 }

 pub fn getClientRects(self: *Element, frame: *Frame) ![]*DOMRect {
-    if (!self.checkVisibilityCached(null, frame)) {
+    if (!self.checkVisibilityCached(null, frame, .materialize)) {
        return &.{};
    }
    const rects = try frame.local_arena.alloc(*DOMRect, 1);
@@ -1505,7 +1505,7 @@ pub fn setScrollLeft(self: *Element, value: i32, frame: *Frame) !void {
 }

 pub fn getScrollHeight(self: *Element, frame: *Frame) f64 {
-    if (!self.checkVisibilityCached(null, frame)) {
+    if (!self.checkVisibilityCached(null, frame, .materialize)) {
        return 0.0;
    }

@@ -1522,7 +1522,7 @@ pub fn getScrollHeight(self: *Element, frame: *Frame) f64 {
 }

 pub fn getScrollWidth(self: *Element, frame: *Frame) f64 {
-    if (!self.checkVisibilityCached(null, frame)) {
+    if (!self.checkVisibilityCached(null, frame, .materialize)) {
        return 0.0;
    }

@@ -1590,21 +1590,21 @@ pub fn getOffsetWidth(self: *Element, frame: *Frame) f64 {
 }

 pub fn getOffsetTop(self: *Element, frame: *Frame) f64 {
-    if (!self.checkVisibilityCached(null, frame)) {
+    if (!self.checkVisibilityCached(null, frame, .materialize)) {
        return 0.0;
    }
    return calculateDocumentPosition(self.asNode());
 }

 pub fn getOffsetLeft(self: *Element, frame: *Frame) f64 {
-    if (!self.checkVisibilityCached(null, frame)) {
+    if (!self.checkVisibilityCached(null, frame, .materialize)) {
        return 0.0;
    }
    return self.horizontalPosition(frame);
 }

 pub fn getOffsetParent(self: *Element, frame: *Frame) ?*Element {
-    if (!self.asNode().isConnected() or !self.checkVisibilityCached(null, frame)) {
+    if (!self.asNode().isConnected() or !self.checkVisibilityCached(null, frame, .materialize)) {
        return null;
    }

@@ -162,7 +162,7 @@ pub fn deliverEntries(self: *ResizeObserver, frame: *Frame) !void {
        obs.connected = connected;

        const width, const height = blk: {
-            if (!connected or !target.checkVisibilityCached(&visibility_cache, frame)) {
+            if (!connected or !target.checkVisibilityCached(&visibility_cache, frame, .materialize)) {
                break :blk .{ 0, 0 };
            }
            break :blk .{
@@ -84,7 +84,7 @@ pub fn getPropertyValue(self: *const CSSStyleDeclaration, property_name: []const
        if (self._element) |element| {
            const style_manager = &element.ownerFrame(frame)._style_manager;
            if (wrapped.eql(comptime .wrap("display"))) {
-                if (style_manager.hasDisplayNone(element)) return "none";
+                if (style_manager.hasDisplayNone(element, .materialize)) return "none";
            } else if (wrapped.eql(comptime .wrap("visibility"))) {
                if (style_manager.hasVisibilityHiddenInherited(element)) return "hidden";
            }
@@ -122,7 +122,7 @@ pub fn getPropertyValue(self: *const CSSStyleDeclaration, property_name: []const
 }

 fn resolvedDimension(element: *Element, dimension: enum { width, height }, frame: *Frame) []const u8 {
-    if (!element.checkVisibilityCached(null, frame)) {
+    if (!element.checkVisibilityCached(null, frame, .materialize)) {
        return "auto";
    }
    const value = switch (dimension) {
@@ -1612,7 +1612,7 @@ fn handleChildElement(
    // is hidden through its parent. If you can el.innerText on an element, the
    // visibility of el.parent doesn't matter. So we only care about visibility
    // on the element itself and then on each child. This is much simpler too.
-    if (state.frame._style_manager.hasDisplayNone(he.asElement())) {
+    if (state.frame._style_manager.hasDisplayNone(he.asElement(), .materialize)) {
        return;
    }

@@ -1305,7 +1305,7 @@ fn isHidden(elt: *DOMNode.Element, frame: *Frame, cache: *DOMNode.Element.Visibi

    // CSS display:none and visibility:hidden (both inherited from ancestors via
    // style computation). Matches Chromium's AX tree which prunes both.
-    if (frame._style_manager.isHidden(elt, cache, .{ .check_visibility = true })) {
+    if (frame._style_manager.isHidden(elt, cache, .{ .check_visibility = true }, .scan)) {
        return true;
    }