LibWeb: Invalidate less elements affected by CSS custom properties

Before this change, whenever element's attributes changed, we would add
a flag to "pending invalidation", indicating that all descendants whose
style uses CSS custom properties needed to be recomputed. This resulted
in severe overinvalidation, because we would run invalidation regardless
of whether any custom property on affected element actually changed.

This change takes another approach, and now we decide whether
descendant's style needs to be recomputed based on whether ancestor's
style recomputation results in a change of custom properties, though
this approach adds a little overhead to style computation as now we have
to compare old vs new hashmap of custom properties.

This brings substantial improvement on discord and x.com where, before
this change, advantage of using invalidation sets was lost and we had
to recompute all descendants, because almost all of them use custom
properties.

Author: kalenikaliaksandr

Libraries/LibWeb/CSS/Parser/ValueParsing.cpp

@@ -4339,8 +4339,10 @@ NonnullRefPtr<CSSStyleValue const> Parser::resolve_unresolved_style_value(Parsin
 NonnullRefPtr<CSSStyleValue const> Parser::resolve_unresolved_style_value(DOM::AbstractElement& element, GuardedSubstitutionContexts& guarded_contexts, PropertyIDOrCustomPropertyName property, UnresolvedStyleValue const& unresolved)
 {
     // AD-HOC: Report that we might rely on custom properties.
-    // FIXME: This over-invalidates. Find a way of invalidating only when we need to - specifically, when var() is used.
-    element.element().set_style_uses_css_custom_properties(true);
+    if (unresolved.includes_attr_function())
+        element.element().set_style_uses_attr_css_function();
+    if (unresolved.includes_var_function())
+        element.element().set_style_uses_var_css_function();
 
     // To replace substitution functions in a property prop:
     auto const& property_name = property.visit(

Libraries/LibWeb/CSS/StyleComputer.cpp

@@ -2454,17 +2454,17 @@ GC::Ref<ComputedProperties> StyleComputer::create_document_style() const
     return style;
 }
 
-GC::Ref<ComputedProperties> StyleComputer::compute_style(DOM::Element& element, Optional<CSS::PseudoElement> pseudo_element) const
+GC::Ref<ComputedProperties> StyleComputer::compute_style(DOM::Element& element, Optional<CSS::PseudoElement> pseudo_element, Optional<bool&> did_change_custom_properties) const
 {
-    return *compute_style_impl(element, move(pseudo_element), ComputeStyleMode::Normal);
+    return *compute_style_impl(element, move(pseudo_element), ComputeStyleMode::Normal, did_change_custom_properties);
 }
 
-GC::Ptr<ComputedProperties> StyleComputer::compute_pseudo_element_style_if_needed(DOM::Element& element, Optional<CSS::PseudoElement> pseudo_element) const
+GC::Ptr<ComputedProperties> StyleComputer::compute_pseudo_element_style_if_needed(DOM::Element& element, Optional<CSS::PseudoElement> pseudo_element, Optional<bool&> did_change_custom_properties) const
 {
-    return compute_style_impl(element, move(pseudo_element), ComputeStyleMode::CreatePseudoElementStyleIfNeeded);
+    return compute_style_impl(element, move(pseudo_element), ComputeStyleMode::CreatePseudoElementStyleIfNeeded, did_change_custom_properties);
 }
 
-GC::Ptr<ComputedProperties> StyleComputer::compute_style_impl(DOM::Element& element, Optional<CSS::PseudoElement> pseudo_element, ComputeStyleMode mode) const
+GC::Ptr<ComputedProperties> StyleComputer::compute_style_impl(DOM::Element& element, Optional<CSS::PseudoElement> pseudo_element, ComputeStyleMode mode, Optional<bool&> did_change_custom_properties) const
 {
     build_rule_cache_if_needed();
 
@@ -2488,6 +2488,9 @@ GC::Ptr<ComputedProperties> StyleComputer::compute_style_impl(DOM::Element& elem
     PseudoClassBitmap attempted_pseudo_class_matches;
     auto matching_rule_set = build_matching_rule_set(element, pseudo_element, attempted_pseudo_class_matches, did_match_any_pseudo_element_rules, mode);
 
+    DOM::AbstractElement abstract_element { element, pseudo_element };
+    auto old_custom_properties = abstract_element.custom_properties();
+
     // Resolve all the CSS custom properties ("variables") for this element:
     // FIXME: Also resolve !important custom properties, in a second cascade.
     if (!pseudo_element.has_value() || pseudo_element_supports_property(*pseudo_element, PropertyID::Custom)) {
@@ -2533,6 +2536,11 @@ GC::Ptr<ComputedProperties> StyleComputer::compute_style_impl(DOM::Element& elem
 
     auto computed_properties = compute_properties(element, pseudo_element, cascaded_properties);
     computed_properties->set_attempted_pseudo_class_matches(attempted_pseudo_class_matches);
+
+    if (did_change_custom_properties.has_value() && abstract_element.custom_properties() != old_custom_properties) {
+        *did_change_custom_properties = true;
+    }
+
     return computed_properties;
 }
 

Libraries/LibWeb/CSS/StyleComputer.h

@@ -145,8 +145,8 @@ class StyleComputer {
 
     [[nodiscard]] GC::Ref<ComputedProperties> create_document_style() const;
 
-    [[nodiscard]] GC::Ref<ComputedProperties> compute_style(DOM::Element&, Optional<CSS::PseudoElement> = {}) const;
-    [[nodiscard]] GC::Ptr<ComputedProperties> compute_pseudo_element_style_if_needed(DOM::Element&, Optional<CSS::PseudoElement>) const;
+    [[nodiscard]] GC::Ref<ComputedProperties> compute_style(DOM::Element&, Optional<CSS::PseudoElement> = {}, Optional<bool&> did_change_custom_properties = {}) const;
+    [[nodiscard]] GC::Ptr<ComputedProperties> compute_pseudo_element_style_if_needed(DOM::Element&, Optional<CSS::PseudoElement>, Optional<bool&> did_change_custom_properties) const;
 
     [[nodiscard]] RuleCache const& get_pseudo_class_rule_cache(PseudoClass) const;
 
@@ -217,7 +217,7 @@ class StyleComputer {
     [[nodiscard]] MatchingRuleSet build_matching_rule_set(DOM::Element const&, Optional<PseudoElement>, PseudoClassBitmap& attempted_pseudo_class_matches, bool& did_match_any_pseudo_element_rules, ComputeStyleMode) const;
 
     LogicalAliasMappingContext compute_logical_alias_mapping_context(DOM::Element&, Optional<CSS::PseudoElement>, ComputeStyleMode, MatchingRuleSet const&) const;
-    [[nodiscard]] GC::Ptr<ComputedProperties> compute_style_impl(DOM::Element&, Optional<CSS::PseudoElement>, ComputeStyleMode) const;
+    [[nodiscard]] GC::Ptr<ComputedProperties> compute_style_impl(DOM::Element&, Optional<CSS::PseudoElement>, ComputeStyleMode, Optional<bool&> did_change_custom_properties) const;
     [[nodiscard]] GC::Ref<CascadedProperties> compute_cascaded_values(DOM::Element&, Optional<CSS::PseudoElement>, bool did_match_any_pseudo_element_rules, ComputeStyleMode, MatchingRuleSet const&, Optional<LogicalAliasMappingContext>, ReadonlySpan<PropertyID> properties_to_cascade) const;
     static RefPtr<Gfx::FontCascadeList const> find_matching_font_weight_ascending(Vector<MatchingFontCandidate> const& candidates, int target_weight, float font_size_in_pt, bool inclusive);
     static RefPtr<Gfx::FontCascadeList const> find_matching_font_weight_descending(Vector<MatchingFontCandidate> const& candidates, int target_weight, float font_size_in_pt, bool inclusive);

Libraries/LibWeb/CSS/StyleProperty.cpp

@@ -11,4 +11,11 @@ namespace Web::CSS {
 
 StyleProperty::~StyleProperty() = default;
 
+bool StyleProperty::operator==(StyleProperty const& other) const
+{
+    if (important != other.important || property_id != other.property_id || custom_name != other.custom_name)
+        return false;
+    return value->equals(*other.value);
+}
+
 }

Libraries/LibWeb/CSS/StyleProperty.h

@@ -23,6 +23,8 @@ struct StyleProperty {
     CSS::PropertyID property_id;
     NonnullRefPtr<CSSStyleValue const> value;
     FlyString custom_name {};
+
+    bool operator==(StyleProperty const& other) const;
 };
 
 }

Libraries/LibWeb/CSS/StyleValues/UnresolvedStyleValue.h

@@ -25,6 +25,8 @@ class UnresolvedStyleValue final : public CSSStyleValue {
 
     Vector<Parser::ComponentValue> const& values() const { return m_values; }
     bool contains_arbitrary_substitution_function() const { return m_substitution_functions_presence.has_any(); }
+    bool includes_attr_function() const { return m_substitution_functions_presence.attr; }
+    bool includes_var_function() const { return m_substitution_functions_presence.var; }
 
     virtual bool equals(CSSStyleValue const& other) const override;
 

Libraries/LibWeb/DOM/Document.cpp

@@ -1427,7 +1427,7 @@ void Document::update_layout(UpdateLayoutReason reason)
     }
 }
 
-[[nodiscard]] static CSS::RequiredInvalidationAfterStyleChange update_style_recursively(Node& node, CSS::StyleComputer& style_computer, bool needs_inherited_style_update)
+[[nodiscard]] static CSS::RequiredInvalidationAfterStyleChange update_style_recursively(Node& node, CSS::StyleComputer& style_computer, bool needs_inherited_style_update, bool recompute_elements_depending_on_custom_properties)
 {
     bool const needs_full_style_update = node.document().needs_full_style_update();
     CSS::RequiredInvalidationAfterStyleChange invalidation;
@@ -1440,12 +1440,14 @@ void Document::update_layout(UpdateLayoutReason reason)
     //       We will still recompute style for the children, though.
     bool is_display_none = false;
 
+    bool did_change_custom_properties = false;
     CSS::RequiredInvalidationAfterStyleChange node_invalidation;
     if (is<Element>(node)) {
-        if (needs_full_style_update || node.needs_style_update()) {
-            node_invalidation = static_cast<Element&>(node).recompute_style();
+        auto& element = static_cast<Element&>(node);
+        if (needs_full_style_update || node.needs_style_update() || (recompute_elements_depending_on_custom_properties && element.style_uses_var_css_function())) {
+            node_invalidation = element.recompute_style(did_change_custom_properties);
         } else if (needs_inherited_style_update) {
-            node_invalidation = static_cast<Element&>(node).recompute_inherited_style();
+            node_invalidation = element.recompute_inherited_style();
         }
         is_display_none = static_cast<Element&>(node).computed_properties()->display().is_none();
     }
@@ -1464,21 +1466,25 @@ void Document::update_layout(UpdateLayoutReason reason)
     node.set_needs_style_update(false);
     invalidation |= node_invalidation;
 
+    if (did_change_custom_properties) {
+        recompute_elements_depending_on_custom_properties = true;
+    }
+
     bool children_need_inherited_style_update = !invalidation.is_none();
-    if (needs_full_style_update || node.child_needs_style_update() || children_need_inherited_style_update) {
+    if (needs_full_style_update || node.child_needs_style_update() || children_need_inherited_style_update || recompute_elements_depending_on_custom_properties) {
         if (node.is_element()) {
             if (auto shadow_root = static_cast<DOM::Element&>(node).shadow_root()) {
                 if (needs_full_style_update || shadow_root->needs_style_update() || shadow_root->child_needs_style_update()) {
-                    auto subtree_invalidation = update_style_recursively(*shadow_root, style_computer, children_need_inherited_style_update);
+                    auto subtree_invalidation = update_style_recursively(*shadow_root, style_computer, children_need_inherited_style_update, recompute_elements_depending_on_custom_properties);
                     if (!is_display_none)
                         invalidation |= subtree_invalidation;
                 }
             }
         }
 
         node.for_each_child([&](auto& child) {
-            if (needs_full_style_update || child.needs_style_update() || children_need_inherited_style_update || child.child_needs_style_update()) {
-                auto subtree_invalidation = update_style_recursively(child, style_computer, children_need_inherited_style_update);
+            if (needs_full_style_update || child.needs_style_update() || children_need_inherited_style_update || child.child_needs_style_update() || recompute_elements_depending_on_custom_properties) {
+                auto subtree_invalidation = update_style_recursively(child, style_computer, children_need_inherited_style_update, recompute_elements_depending_on_custom_properties);
                 if (!is_display_none)
                     invalidation |= subtree_invalidation;
             }
@@ -1523,7 +1529,7 @@ void Document::update_style()
 
     style_computer().reset_ancestor_filter();
 
-    auto invalidation = update_style_recursively(*this, style_computer(), false);
+    auto invalidation = update_style_recursively(*this, style_computer(), false, false);
     if (!invalidation.is_none())
         invalidate_display_list();
     if (invalidation.rebuild_stacking_context_tree)

Libraries/LibWeb/DOM/Element.cpp

@@ -683,10 +683,12 @@ static CSS::RequiredInvalidationAfterStyleChange compute_required_invalidation(C
     return invalidation;
 }
 
-CSS::RequiredInvalidationAfterStyleChange Element::recompute_style()
+CSS::RequiredInvalidationAfterStyleChange Element::recompute_style(bool& did_change_custom_properties)
 {
     VERIFY(parent());
 
+    m_style_uses_attr_css_function = false;
+    m_style_uses_var_css_function = false;
     m_affected_by_has_pseudo_class_in_subject_position = false;
     m_affected_by_has_pseudo_class_in_non_subject_position = false;
     m_affected_by_has_pseudo_class_with_relative_selector_that_has_sibling_combinator = false;
@@ -697,7 +699,7 @@ CSS::RequiredInvalidationAfterStyleChange Element::recompute_style()
     m_sibling_invalidation_distance = 0;
 
     auto& style_computer = document().style_computer();
-    auto new_computed_properties = style_computer.compute_style(*this);
+    auto new_computed_properties = style_computer.compute_style(*this, {}, did_change_custom_properties);
 
     // Tables must not inherit -libweb-* values for text-align.
     // FIXME: Find the spec for this.
@@ -737,7 +739,7 @@ CSS::RequiredInvalidationAfterStyleChange Element::recompute_style()
         style_computer.push_ancestor(*this);
 
         auto pseudo_element_style = computed_properties(pseudo_element);
-        auto new_pseudo_element_style = style_computer.compute_pseudo_element_style_if_needed(*this, pseudo_element);
+        auto new_pseudo_element_style = style_computer.compute_pseudo_element_style_if_needed(*this, pseudo_element, did_change_custom_properties);
 
         // TODO: Can we be smarter about invalidation?
         if (pseudo_element_style && new_pseudo_element_style) {
@@ -2472,24 +2474,13 @@ void Element::invalidate_style_after_attribute_change(FlyString const& attribute
 {
     Vector<CSS::InvalidationSet::Property, 1> changed_properties;
     StyleInvalidationOptions style_invalidation_options;
-    if (is_presentational_hint(attribute_name)) {
+    if (is_presentational_hint(attribute_name) || style_uses_attr_css_function()) {
         style_invalidation_options.invalidate_self = true;
     }
 
-    if (style_uses_css_custom_properties()) {
-        // A css custom property can be hooked on to this element by any attribute
-        // so invalidate elements and rerender them in that scenario
-        style_invalidation_options.invalidate_elements_that_use_css_custom_properties = true;
-    }
-
     if (attribute_name == HTML::AttributeNames::style) {
         style_invalidation_options.invalidate_self = true;
-        // even if we don't have custom properties, the new "style" attribute could add one
-        style_invalidation_options.invalidate_elements_that_use_css_custom_properties = true;
     } else if (attribute_name == HTML::AttributeNames::class_) {
-        // adding or removing classes can add new custom properties to this element
-        style_invalidation_options.invalidate_elements_that_use_css_custom_properties = true;
-
         Vector<StringView> old_classes;
         Vector<StringView> new_classes;
         if (old_value.has_value())

Libraries/LibWeb/DOM/Element.h

@@ -201,7 +201,7 @@ class Element
 
     void run_attribute_change_steps(FlyString const& local_name, Optional<String> const& old_value, Optional<String> const& value, Optional<FlyString> const& namespace_);
 
-    CSS::RequiredInvalidationAfterStyleChange recompute_style();
+    CSS::RequiredInvalidationAfterStyleChange recompute_style(bool& did_change_custom_properties);
     CSS::RequiredInvalidationAfterStyleChange recompute_inherited_style();
 
     Optional<CSS::PseudoElement> use_pseudo_element() const { return m_use_pseudo_element; }
@@ -256,8 +256,10 @@ class Element
     void set_custom_properties(Optional<CSS::PseudoElement>, HashMap<FlyString, CSS::StyleProperty> custom_properties);
     [[nodiscard]] HashMap<FlyString, CSS::StyleProperty> const& custom_properties(Optional<CSS::PseudoElement>) const;
 
-    bool style_uses_css_custom_properties() const { return m_style_uses_css_custom_properties; }
-    void set_style_uses_css_custom_properties(bool value) { m_style_uses_css_custom_properties = value; }
+    bool style_uses_attr_css_function() const { return m_style_uses_attr_css_function; }
+    void set_style_uses_attr_css_function() { m_style_uses_attr_css_function = true; }
+    bool style_uses_var_css_function() const { return m_style_uses_var_css_function; }
+    void set_style_uses_var_css_function() { m_style_uses_var_css_function = true; }
 
     // NOTE: The function is wrapped in a GC::HeapFunction immediately.
     HTML::TaskID queue_an_element_task(HTML::Task::Source, Function<void()>);
@@ -605,7 +607,8 @@ class Element
 
     bool m_in_top_layer : 1 { false };
     bool m_rendered_in_top_layer : 1 { false };
-    bool m_style_uses_css_custom_properties : 1 { false };
+    bool m_style_uses_attr_css_function : 1 { false };
+    bool m_style_uses_var_css_function : 1 { false };
     bool m_affected_by_has_pseudo_class_in_subject_position : 1 { false };
     bool m_affected_by_has_pseudo_class_in_non_subject_position : 1 { false };
     bool m_affected_by_direct_sibling_combinator : 1 { false };

Libraries/LibWeb/DOM/Node.cpp

@@ -515,11 +515,11 @@ void Node::invalidate_style(StyleInvalidationReason reason, Vector<CSS::Invalida
         set_needs_style_update(true);
     }
 
-    if (!invalidation_set.has_properties() && !options.invalidate_elements_that_use_css_custom_properties) {
+    if (!invalidation_set.has_properties()) {
         return;
     }
 
-    document().style_invalidator().add_pending_invalidation(*this, move(invalidation_set), options.invalidate_elements_that_use_css_custom_properties);
+    document().style_invalidator().add_pending_invalidation(*this, move(invalidation_set));
 }
 
 Utf16String Node::child_text_content() const