LibWeb: Don't make deep copy of custom properties for every element

Previously we were making a copy of the full set of custom properties
that applied to a DOM element. This was very costly and dominated the
profile when mousing around on GitHub.

Note that this may break custom properties on pseudo elements a little
bit, and that's something we'll have to look into.

2 files changed, 30 insertions, 34 deletions

diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp
index 5760a49f50..e69675a1bf 100644
--- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp
+++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp
@@ -441,7 +441,16 @@ static void set_property_expanding_shorthands(StyleProperties& style, CSS::Prope
     style.set_property(property_id, value);
 }
 
-bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView property_name, HashMap<FlyString, NonnullRefPtr<PropertyDependencyNode>>& dependencies, Vector<StyleComponentValueRule> const& source, Vector<StyleComponentValueRule>& dest, size_t source_start_index, HashMap<FlyString, StyleProperty> const& custom_properties) const
+static RefPtr<StyleValue> get_custom_property(DOM::Element const& element, FlyString const& custom_property_name)
+{
+    for (auto const* current_element = &element; current_element; current_element = current_element->parent_element()) {
+        if (auto it = current_element->custom_properties().find(custom_property_name); it != current_element->custom_properties().end())
+            return it->value.value;
+    }
+    return nullptr;
+}
+
+bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView property_name, HashMap<FlyString, NonnullRefPtr<PropertyDependencyNode>>& dependencies, Vector<StyleComponentValueRule> const& source, Vector<StyleComponentValueRule>& dest, size_t source_start_index) const
 {
     // FIXME: Do this better!
     // We build a copy of the tree of StyleComponentValueRules, with all var()s replaced with their contents.
@@ -455,13 +464,6 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p
         return false;
     }
 
-    auto get_custom_property = [&custom_properties](auto& name) -> RefPtr<StyleValue> {
-        auto it = custom_properties.find(name);
-        if (it != custom_properties.end())
-            return it->value.value;
-        return nullptr;
-    };
-
     auto get_dependency_node = [&](auto name) -> NonnullRefPtr<PropertyDependencyNode> {
         if (auto existing = dependencies.get(name); existing.has_value())
             return *existing.value();
@@ -496,16 +498,16 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p
                 if (parent->has_cycles())
                     return false;
 
-                if (auto custom_property_value = get_custom_property(custom_property_name)) {
+                if (auto custom_property_value = get_custom_property(element, custom_property_name)) {
                     VERIFY(custom_property_value->is_unresolved());
-                    if (!expand_unresolved_values(element, custom_property_name, dependencies, custom_property_value->as_unresolved().values(), dest, 0, custom_properties))
+                    if (!expand_unresolved_values(element, custom_property_name, dependencies, custom_property_value->as_unresolved().values(), dest, 0))
                         return false;
                     continue;
                 }
 
                 // Use the provided fallback value, if any.
                 if (var_contents.size() > 2 && var_contents[1].is(Token::Type::Comma)) {
-                    if (!expand_unresolved_values(element, property_name, dependencies, var_contents, dest, 2, custom_properties))
+                    if (!expand_unresolved_values(element, property_name, dependencies, var_contents, dest, 2))
                         return false;
                     continue;
                 }
@@ -513,7 +515,7 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p
 
             auto const& source_function = value.function();
             Vector<StyleComponentValueRule> function_values;
-            if (!expand_unresolved_values(element, property_name, dependencies, source_function.values(), function_values, 0, custom_properties))
+            if (!expand_unresolved_values(element, property_name, dependencies, source_function.values(), function_values, 0))
                 return false;
             NonnullRefPtr<StyleFunctionRule> function = adopt_ref(*new StyleFunctionRule(source_function.name(), move(function_values)));
             dest.empend(function);
@@ -522,7 +524,7 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p
         if (value.is_block()) {
             auto const& source_block = value.block();
             Vector<StyleComponentValueRule> block_values;
-            if (!expand_unresolved_values(element, property_name, dependencies, source_block.values(), block_values, 0, custom_properties))
+            if (!expand_unresolved_values(element, property_name, dependencies, source_block.values(), block_values, 0))
                 return false;
             NonnullRefPtr<StyleBlockRule> block = adopt_ref(*new StyleBlockRule(source_block.token(), move(block_values)));
             dest.empend(block);
@@ -534,7 +536,7 @@ bool StyleComputer::expand_unresolved_values(DOM::Element& element, StringView p
     return true;
 }
 
-RefPtr<StyleValue> StyleComputer::resolve_unresolved_style_value(DOM::Element& element, PropertyID property_id, UnresolvedStyleValue const& unresolved, HashMap<FlyString, StyleProperty> const& custom_properties) const
+RefPtr<StyleValue> StyleComputer::resolve_unresolved_style_value(DOM::Element& element, PropertyID property_id, UnresolvedStyleValue const& unresolved) const
 {
     // Unresolved always contains a var(), unless it is a custom property's value, in which case we shouldn't be trying
     // to produce a different StyleValue from it.
@@ -542,7 +544,7 @@ RefPtr<StyleValue> StyleComputer::resolve_unresolved_style_value(DOM::Element& e
 
     Vector<StyleComponentValueRule> expanded_values;
     HashMap<FlyString, NonnullRefPtr<PropertyDependencyNode>> dependencies;
-    if (!expand_unresolved_values(element, string_from_property_id(property_id), dependencies, unresolved.values(), expanded_values, 0, custom_properties))
+    if (!expand_unresolved_values(element, string_from_property_id(property_id), dependencies, unresolved.values(), expanded_values, 0))
         return {};
 
     if (auto parsed_value = Parser::parse_css_value({}, ParsingContext { document() }, property_id, expanded_values))
@@ -551,7 +553,7 @@ RefPtr<StyleValue> StyleComputer::resolve_unresolved_style_value(DOM::Element& e
     return {};
 }
 
-void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& element, Vector<MatchingRule> const& matching_rules, CascadeOrigin cascade_origin, Important important, HashMap<FlyString, StyleProperty> const& custom_properties) const
+void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& element, Vector<MatchingRule> const& matching_rules, CascadeOrigin cascade_origin, Important important) const
 {
     for (auto const& match : matching_rules) {
         for (auto const& property : verify_cast<PropertyOwningCSSStyleDeclaration>(match.rule->declaration()).properties()) {
@@ -559,7 +561,7 @@ void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& e
                 continue;
             auto property_value = property.value;
             if (property.value->is_unresolved()) {
-                if (auto resolved = resolve_unresolved_style_value(element, property.property_id, property.value->as_unresolved(), custom_properties))
+                if (auto resolved = resolve_unresolved_style_value(element, property.property_id, property.value->as_unresolved()))
                     property_value = resolved.release_nonnull();
             }
             set_property_expanding_shorthands(style, property.property_id, property_value, m_document);
@@ -577,29 +579,21 @@ void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& e
     }
 }
 
-static HashMap<FlyString, StyleProperty> const& cascade_custom_properties(DOM::Element& element, Vector<MatchingRule> const& matching_rules)
+static void cascade_custom_properties(DOM::Element& element, Vector<MatchingRule> const& matching_rules)
 {
     size_t needed_capacity = 0;
-    if (auto* parent_element = element.parent_element())
-        needed_capacity += parent_element->custom_properties().size();
     for (auto const& matching_rule : matching_rules)
         needed_capacity += verify_cast<PropertyOwningCSSStyleDeclaration>(matching_rule.rule->declaration()).custom_properties().size();
 
     HashMap<FlyString, StyleProperty> custom_properties;
     custom_properties.ensure_capacity(needed_capacity);
 
-    if (auto* parent_element = element.parent_element()) {
-        for (auto const& it : parent_element->custom_properties())
-            custom_properties.set(it.key, it.value);
-    }
-
     for (auto const& matching_rule : matching_rules) {
         for (auto const& it : verify_cast<PropertyOwningCSSStyleDeclaration>(matching_rule.rule->declaration()).custom_properties())
             custom_properties.set(it.key, it.value);
     }
 
     element.set_custom_properties(move(custom_properties));
-    return element.custom_properties();
 }
 
 // https://www.w3.org/TR/css-cascade/#cascading
@@ -613,12 +607,14 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element
     sort_matching_rules(matching_rule_set.author_rules);
 
     // Then we resolve all the CSS custom properties ("variables") for this element:
-    auto const& custom_properties = cascade_custom_properties(element, matching_rule_set.author_rules);
+    // FIXME: Look into how custom properties should interact with pseudo elements and support that properly.
+    if (!pseudo_element.has_value())
+        cascade_custom_properties(element, matching_rule_set.author_rules);
 
     // Then we apply the declarations from the matched rules in cascade order:
 
     // Normal user agent declarations
-    cascade_declarations(style, element, matching_rule_set.user_agent_rules, CascadeOrigin::UserAgent, Important::No, custom_properties);
+    cascade_declarations(style, element, matching_rule_set.user_agent_rules, CascadeOrigin::UserAgent, Important::No);
 
     // FIXME: Normal user declarations
 
@@ -626,17 +622,17 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element
     element.apply_presentational_hints(style);
 
     // Normal author declarations
-    cascade_declarations(style, element, matching_rule_set.author_rules, CascadeOrigin::Author, Important::No, custom_properties);
+    cascade_declarations(style, element, matching_rule_set.author_rules, CascadeOrigin::Author, Important::No);
 
     // FIXME: Animation declarations [css-animations-1]
 
     // Important author declarations
-    cascade_declarations(style, element, matching_rule_set.author_rules, CascadeOrigin::Author, Important::Yes, custom_properties);
+    cascade_declarations(style, element, matching_rule_set.author_rules, CascadeOrigin::Author, Important::Yes);
 
     // FIXME: Important user declarations
 
     // Important user agent declarations
-    cascade_declarations(style, element, matching_rule_set.user_agent_rules, CascadeOrigin::UserAgent, Important::Yes, custom_properties);
+    cascade_declarations(style, element, matching_rule_set.user_agent_rules, CascadeOrigin::UserAgent, Important::Yes);
 
     // FIXME: Transition declarations [css-transitions-1]
 }
diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.h b/Userland/Libraries/LibWeb/CSS/StyleComputer.h
index 107efc5194..6a1e3976a7 100644
--- a/Userland/Libraries/LibWeb/CSS/StyleComputer.h
+++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.h
@@ -80,8 +80,8 @@ private:
 
     void compute_defaulted_property_value(StyleProperties&, DOM::Element const*, CSS::PropertyID, Optional<CSS::Selector::PseudoElement>) const;
 
-    RefPtr<StyleValue> resolve_unresolved_style_value(DOM::Element&, PropertyID, UnresolvedStyleValue const&, HashMap<FlyString, StyleProperty> const&) const;
-    bool expand_unresolved_values(DOM::Element&, StringView property_name, HashMap<FlyString, NonnullRefPtr<PropertyDependencyNode>>& dependencies, Vector<StyleComponentValueRule> const& source, Vector<StyleComponentValueRule>& dest, size_t source_start_index, HashMap<FlyString, StyleProperty> const& custom_properties) const;
+    RefPtr<StyleValue> resolve_unresolved_style_value(DOM::Element&, PropertyID, UnresolvedStyleValue const&) const;
+    bool expand_unresolved_values(DOM::Element&, StringView property_name, HashMap<FlyString, NonnullRefPtr<PropertyDependencyNode>>& dependencies, Vector<StyleComponentValueRule> const& source, Vector<StyleComponentValueRule>& dest, size_t source_start_index) const;
 
     template<typename Callback>
     void for_each_stylesheet(CascadeOrigin, Callback) const;
@@ -91,7 +91,7 @@ private:
         Vector<MatchingRule> author_rules;
     };
 
-    void cascade_declarations(StyleProperties&, DOM::Element&, Vector<MatchingRule> const&, CascadeOrigin, Important important, HashMap<FlyString, StyleProperty> const&) const;
+    void cascade_declarations(StyleProperties&, DOM::Element&, Vector<MatchingRule> const&, CascadeOrigin, Important important) const;
 
     void build_rule_cache();
     void build_rule_cache_if_needed() const;