From eacfcac93288af50fd53ff09173884f92b099613 Mon Sep 17 00:00:00 2001 From: Thomas Queiroz Date: Tue, 6 Dec 2022 03:08:20 -0300 Subject: [PATCH] LibWeb: Use HashMap::try_ensure_capacity in StyleComputer --- .../CSS/ResolvedCSSStyleDeclaration.cpp | 9 +++- .../Libraries/LibWeb/CSS/StyleComputer.cpp | 17 ++++--- Userland/Libraries/LibWeb/CSS/StyleComputer.h | 4 +- Userland/Libraries/LibWeb/DOM/Element.cpp | 4 +- .../Libraries/LibWeb/Layout/TreeBuilder.cpp | 46 +++++++++++-------- .../Libraries/LibWeb/Layout/TreeBuilder.h | 4 +- .../WebContent/ConnectionFromClient.cpp | 2 +- 7 files changed, 53 insertions(+), 33 deletions(-) diff --git a/Userland/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp b/Userland/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp index a32ef6e2ca..8392db5018 100644 --- a/Userland/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp +++ b/Userland/Libraries/LibWeb/CSS/ResolvedCSSStyleDeclaration.cpp @@ -7,6 +7,7 @@ */ #include +#include #include #include #include @@ -531,7 +532,13 @@ Optional ResolvedCSSStyleDeclaration::property(PropertyID propert } if (!m_element->layout_node()) { - auto style = m_element->document().style_computer().compute_style(const_cast(*m_element)); + auto style_or_error = m_element->document().style_computer().compute_style(const_cast(*m_element)); + if (style_or_error.is_error()) { + dbgln("ResolvedCSSStyleDeclaration::property style computer failed"); + return {}; + } + auto style = style_or_error.release_value(); + // FIXME: This is a stopgap until we implement shorthand -> longhand conversion. auto value = style->maybe_null_property(property_id); if (!value) { diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp index 35937a906c..5aec82f4fb 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -7,6 +7,7 @@ */ #include +#include #include #include #include @@ -806,7 +807,7 @@ void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& e } } -static void cascade_custom_properties(DOM::Element& element, Vector const& matching_rules) +static ErrorOr cascade_custom_properties(DOM::Element& element, Vector const& matching_rules) { size_t needed_capacity = 0; for (auto const& matching_rule : matching_rules) @@ -815,7 +816,7 @@ static void cascade_custom_properties(DOM::Element& element, Vectorcustom_properties().size(); HashMap custom_properties; - custom_properties.ensure_capacity(needed_capacity); + TRY(custom_properties.try_ensure_capacity(needed_capacity)); for (auto const& matching_rule : matching_rules) { for (auto const& it : verify_cast(matching_rule.rule->declaration()).custom_properties()) @@ -828,10 +829,12 @@ static void cascade_custom_properties(DOM::Element& element, Vector pseudo_element) const +ErrorOr StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element& element, Optional pseudo_element) const { // First, we collect all the CSS rules whose selectors match `element`: MatchingRuleSet matching_rule_set; @@ -843,7 +846,7 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element // Then we resolve all the CSS custom properties ("variables") for this element: // 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); + TRY(cascade_custom_properties(element, matching_rule_set.author_rules)); // Then we apply the declarations from the matched rules in cascade order: @@ -869,6 +872,8 @@ void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element cascade_declarations(style, element, matching_rule_set.user_agent_rules, CascadeOrigin::UserAgent, Important::Yes); // FIXME: Transition declarations [css-transitions-1] + + return {}; } static DOM::Element const* element_to_inherit_style_from(DOM::Element const* element, Optional pseudo_element) @@ -1293,13 +1298,13 @@ NonnullRefPtr StyleComputer::create_document_style() const return style; } -NonnullRefPtr StyleComputer::compute_style(DOM::Element& element, Optional pseudo_element) const +ErrorOr> StyleComputer::compute_style(DOM::Element& element, Optional pseudo_element) const { build_rule_cache_if_needed(); auto style = StyleProperties::create(); // 1. Perform the cascade. This produces the "specified style" - compute_cascaded_values(style, element, pseudo_element); + TRY(compute_cascaded_values(style, element, pseudo_element)); // 2. Compute the font, since that may be needed for font-relative CSS units compute_font(style, &element, pseudo_element); diff --git a/Userland/Libraries/LibWeb/CSS/StyleComputer.h b/Userland/Libraries/LibWeb/CSS/StyleComputer.h index 44f1a789cc..df8ecdd6e7 100644 --- a/Userland/Libraries/LibWeb/CSS/StyleComputer.h +++ b/Userland/Libraries/LibWeb/CSS/StyleComputer.h @@ -56,7 +56,7 @@ public: DOM::Document const& document() const { return m_document; } NonnullRefPtr create_document_style() const; - NonnullRefPtr compute_style(DOM::Element&, Optional = {}) const; + ErrorOr> compute_style(DOM::Element&, Optional = {}) const; // https://www.w3.org/TR/css-cascade/#origin enum class CascadeOrigin { @@ -78,7 +78,7 @@ public: void load_fonts_from_sheet(CSSStyleSheet const&); private: - void compute_cascaded_values(StyleProperties&, DOM::Element&, Optional) const; + ErrorOr compute_cascaded_values(StyleProperties&, DOM::Element&, Optional) const; void compute_font(StyleProperties&, DOM::Element const*, Optional) const; void compute_defaulted_values(StyleProperties&, DOM::Element const*, Optional) const; void absolutize_values(StyleProperties&, DOM::Element const*, Optional) const; diff --git a/Userland/Libraries/LibWeb/DOM/Element.cpp b/Userland/Libraries/LibWeb/DOM/Element.cpp index e7071bb7ec..eae666b646 100644 --- a/Userland/Libraries/LibWeb/DOM/Element.cpp +++ b/Userland/Libraries/LibWeb/DOM/Element.cpp @@ -400,7 +400,9 @@ Element::NeedsRelayout Element::recompute_style() { set_needs_style_update(false); VERIFY(parent()); - auto new_computed_css_values = document().style_computer().compute_style(*this); + + // FIXME propagate errors + auto new_computed_css_values = MUST(document().style_computer().compute_style(*this)); auto required_invalidation = RequiredInvalidation::Relayout; diff --git a/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp b/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp index ba5a78380d..759fe1132c 100644 --- a/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp +++ b/Userland/Libraries/LibWeb/Layout/TreeBuilder.cpp @@ -6,6 +6,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include #include @@ -147,12 +148,12 @@ void TreeBuilder::insert_node_into_inline_or_block_ancestor(Layout::Node& node, } } -void TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element, CSS::Selector::PseudoElement pseudo_element, AppendOrPrepend mode) +ErrorOr TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element, CSS::Selector::PseudoElement pseudo_element, AppendOrPrepend mode) { auto& document = element.document(); auto& style_computer = document.style_computer(); - auto pseudo_element_style = style_computer.compute_style(element, pseudo_element); + auto pseudo_element_style = TRY(style_computer.compute_style(element, pseudo_element)); auto pseudo_element_content = pseudo_element_style->content(); auto pseudo_element_display = pseudo_element_style->display(); // ::before and ::after only exist if they have content. `content: normal` computes to `none` for them. @@ -160,11 +161,11 @@ void TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element, CSS::Se if (pseudo_element_display.is_none() || pseudo_element_content.type == CSS::ContentData::Type::Normal || pseudo_element_content.type == CSS::ContentData::Type::None) - return; + return {}; auto pseudo_element_node = DOM::Element::create_layout_node_for_display_type(document, pseudo_element_display, pseudo_element_style, nullptr); if (!pseudo_element_node) - return; + return {}; pseudo_element_node->set_generated(true); // FIXME: Handle images, and multiple values @@ -181,20 +182,22 @@ void TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element, CSS::Se element.set_pseudo_element_node({}, pseudo_element, pseudo_element_node); insert_node_into_inline_or_block_ancestor(*pseudo_element_node, pseudo_element_display, mode); + + return {}; } -void TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& context) +ErrorOr TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& context) { // If the parent doesn't have a layout node, we don't need one either. if (dom_node.parent_or_shadow_host() && !dom_node.parent_or_shadow_host()->layout_node()) - return; + return {}; Optional> has_svg_root_change; if (dom_node.is_svg_container()) { has_svg_root_change.emplace(context.has_svg_root, true); } else if (dom_node.requires_svg_container() && !context.has_svg_root) { - return; + return {}; } auto& document = dom_node.document(); @@ -210,7 +213,7 @@ void TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& style = element.computed_css_values(); display = style->display(); if (display.is_none()) - return; + return {}; layout_node = element.create_layout_node(*style); } else if (is(dom_node)) { style = style_computer.create_document_style(); @@ -225,7 +228,7 @@ void TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& } if (!layout_node) - return; + return {}; if (!dom_node.parent_or_shadow_host()) { m_layout_root = layout_node; @@ -240,10 +243,11 @@ void TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& if ((dom_node.has_children() || shadow_root) && layout_node->can_have_children()) { push_parent(verify_cast(*layout_node)); if (shadow_root) - create_layout_tree(*shadow_root, context); - verify_cast(dom_node).for_each_child([&](auto& dom_child) { - create_layout_tree(dom_child, context); - }); + TRY(create_layout_tree(*shadow_root, context)); + + // This is the same as verify_cast(dom_node).for_each_child + for (auto* node = verify_cast(dom_node).first_child(); node; node = node->next_sibling()) + TRY(create_layout_tree(*node, context)); pop_parent(); } @@ -251,15 +255,15 @@ void TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& if (is(dom_node)) { auto& element = static_cast(dom_node); push_parent(verify_cast(*layout_node)); - create_pseudo_element_if_needed(element, CSS::Selector::PseudoElement::Before, AppendOrPrepend::Prepend); - create_pseudo_element_if_needed(element, CSS::Selector::PseudoElement::After, AppendOrPrepend::Append); + TRY(create_pseudo_element_if_needed(element, CSS::Selector::PseudoElement::Before, AppendOrPrepend::Prepend)); + TRY(create_pseudo_element_if_needed(element, CSS::Selector::PseudoElement::After, AppendOrPrepend::Append)); pop_parent(); } if (is(*layout_node)) { auto& element = static_cast(dom_node); int child_index = layout_node->parent()->index_of_child(*layout_node).value(); - auto marker_style = style_computer.compute_style(element, CSS::Selector::PseudoElement::Marker); + auto marker_style = TRY(style_computer.compute_style(element, CSS::Selector::PseudoElement::Marker)); auto list_item_marker = document.heap().allocate_without_realm(document, layout_node->computed_values().list_style_type(), child_index + 1, *marker_style); static_cast(*layout_node).set_marker(list_item_marker); element.set_pseudo_element_node({}, CSS::Selector::PseudoElement::Marker, list_item_marker); @@ -269,9 +273,9 @@ void TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& if (is(dom_node)) { auto& progress = static_cast(dom_node); if (!progress.using_system_appearance()) { - auto bar_style = style_computer.compute_style(progress, CSS::Selector::PseudoElement::ProgressBar); + auto bar_style = TRY(style_computer.compute_style(progress, CSS::Selector::PseudoElement::ProgressBar)); bar_style->set_property(CSS::PropertyID::Display, CSS::IdentifierStyleValue::create(CSS::ValueID::InlineBlock)); - auto value_style = style_computer.compute_style(progress, CSS::Selector::PseudoElement::ProgressValue); + auto value_style = TRY(style_computer.compute_style(progress, CSS::Selector::PseudoElement::ProgressValue)); value_style->set_property(CSS::PropertyID::Display, CSS::IdentifierStyleValue::create(CSS::ValueID::Block)); auto position = progress.position(); value_style->set_property(CSS::PropertyID::Width, CSS::PercentageStyleValue::create(CSS::Percentage(position >= 0 ? round_to(100 * position) : 0))); @@ -294,7 +298,7 @@ void TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& auto& input_element = static_cast(dom_node); if (auto placeholder_value = input_element.placeholder_value(); placeholder_value.has_value()) { - auto placeholder_style = style_computer.compute_style(input_element, CSS::Selector::PseudoElement::Placeholder); + auto placeholder_style = TRY(style_computer.compute_style(input_element, CSS::Selector::PseudoElement::Placeholder)); auto placeholder = DOM::Element::create_layout_node_for_display_type(document, placeholder_style->display(), placeholder_style, nullptr); auto* text = document.heap().allocate(document.realm(), document, *placeholder_value); @@ -311,6 +315,8 @@ void TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& input_element.set_pseudo_element_node({}, CSS::Selector::PseudoElement::Placeholder, placeholder); } } + + return {}; } JS::GCPtr TreeBuilder::build(DOM::Node& dom_node) @@ -318,7 +324,7 @@ JS::GCPtr TreeBuilder::build(DOM::Node& dom_node) VERIFY(dom_node.is_document()); Context context; - create_layout_tree(dom_node, context); + MUST(create_layout_tree(dom_node, context)); // FIXME propagate errors if (auto* root = dom_node.document().layout_node()) fixup_tables(*root); diff --git a/Userland/Libraries/LibWeb/Layout/TreeBuilder.h b/Userland/Libraries/LibWeb/Layout/TreeBuilder.h index 0c0c238ad4..367986359a 100644 --- a/Userland/Libraries/LibWeb/Layout/TreeBuilder.h +++ b/Userland/Libraries/LibWeb/Layout/TreeBuilder.h @@ -24,7 +24,7 @@ private: bool has_svg_root = false; }; - void create_layout_tree(DOM::Node&, Context&); + ErrorOr create_layout_tree(DOM::Node&, Context&); void push_parent(Layout::NodeWithStyle& node) { m_ancestor_stack.append(node); } void pop_parent() { m_ancestor_stack.take_last(); } @@ -45,7 +45,7 @@ private: Prepend, }; void insert_node_into_inline_or_block_ancestor(Layout::Node&, CSS::Display, AppendOrPrepend); - void create_pseudo_element_if_needed(DOM::Element&, CSS::Selector::PseudoElement, AppendOrPrepend); + ErrorOr create_pseudo_element_if_needed(DOM::Element&, CSS::Selector::PseudoElement, AppendOrPrepend); JS::GCPtr m_layout_root; Vector m_ancestor_stack; diff --git a/Userland/Services/WebContent/ConnectionFromClient.cpp b/Userland/Services/WebContent/ConnectionFromClient.cpp index 0e1f7bf59d..a899b5db95 100644 --- a/Userland/Services/WebContent/ConnectionFromClient.cpp +++ b/Userland/Services/WebContent/ConnectionFromClient.cpp @@ -374,7 +374,7 @@ Messages::WebContentServer::InspectDomNodeResponse ConnectionFromClient::inspect // FIXME: Pseudo-elements only exist as Layout::Nodes, which don't have style information // in a format we can use. So, we run the StyleComputer again to get the specified // values, and have to ignore the computed values and custom properties. - auto pseudo_element_style = page().focused_context().active_document()->style_computer().compute_style(element, pseudo_element); + auto pseudo_element_style = MUST(page().focused_context().active_document()->style_computer().compute_style(element, pseudo_element)); DeprecatedString computed_values = serialize_json(pseudo_element_style); DeprecatedString resolved_values = "{}"; DeprecatedString custom_properties_json = "{}";