RGBCube/serenity
1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-25 05:47:35 +00:00
Code Issues Activity Actions

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

Browse source 
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.
This commit is contained in:
Andreas Kling 2022-03-13 17:07:14 +01:00
parent f88d65d9cb
commit 39389b5704
2 changed files with 30 additions and 34 deletions
Download patch file Download diff file Expand all files Collapse all files

58
Userland/Libraries/LibWeb/CSS/StyleComputer.cpp
View file

@ -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]
}