1
Fork 0
mirror of https://github.com/RGBCube/serenity synced 2025-07-25 16:07:47 +00:00

LibWeb: Remove unnecessary ErrorOr<> from StyleComputer

All of this error propogation came from a single call to
HashMap::try_ensure_capacity! As part of the ongoing effort to ignore
small allocation failures, lets just assert this works. This has the
nice side-effect of propogating out to a few other classes.
This commit is contained in:
Matthew Olsson 2024-03-05 18:10:02 -07:00 committed by Andreas Kling
parent a511f1ef85
commit 1ca31e0dc1
7 changed files with 45 additions and 65 deletions

View file

@ -534,12 +534,7 @@ Optional<StyleProperty> ResolvedCSSStyleDeclaration::property(PropertyID propert
}
if (!m_element->layout_node()) {
auto style_or_error = m_element->document().style_computer().compute_style(const_cast<DOM::Element&>(*m_element));
if (style_or_error.is_error()) {
dbgln("ResolvedCSSStyleDeclaration::property style computer failed");
return {};
}
auto style = style_or_error.release_value();
auto style = m_element->document().style_computer().compute_style(const_cast<DOM::Element&>(*m_element));
// FIXME: This is a stopgap until we implement shorthand -> longhand conversion.
auto value = style->maybe_null_property(property_id);

View file

@ -729,7 +729,7 @@ void StyleComputer::cascade_declarations(StyleProperties& style, DOM::Element& e
}
}
static ErrorOr<void> cascade_custom_properties(DOM::Element& element, Optional<CSS::Selector::PseudoElement::Type> pseudo_element, Vector<MatchingRule> const& matching_rules)
static void cascade_custom_properties(DOM::Element& element, Optional<CSS::Selector::PseudoElement::Type> pseudo_element, Vector<MatchingRule> const& matching_rules)
{
size_t needed_capacity = 0;
for (auto const& matching_rule : matching_rules)
@ -741,7 +741,7 @@ static ErrorOr<void> cascade_custom_properties(DOM::Element& element, Optional<C
}
HashMap<FlyString, StyleProperty> custom_properties;
TRY(custom_properties.try_ensure_capacity(needed_capacity));
custom_properties.ensure_capacity(needed_capacity);
for (auto const& matching_rule : matching_rules) {
for (auto const& it : verify_cast<PropertyOwningCSSStyleDeclaration>(matching_rule.rule->declaration()).custom_properties())
@ -756,8 +756,6 @@ static ErrorOr<void> cascade_custom_properties(DOM::Element& element, Optional<C
}
element.set_custom_properties(pseudo_element, move(custom_properties));
return {};
}
static NonnullRefPtr<StyleValue const> interpolate_value(DOM::Element& element, StyleValue const& from, StyleValue const& to, float delta);
@ -1257,18 +1255,18 @@ static ValueComparingNonnullRefPtr<StyleValue const> interpolate_property(DOM::E
}
}
ErrorOr<void> StyleComputer::collect_animation_into(JS::NonnullGCPtr<Animations::KeyframeEffect> effect, StyleProperties& style_properties) const
void StyleComputer::collect_animation_into(JS::NonnullGCPtr<Animations::KeyframeEffect> effect, StyleProperties& style_properties) const
{
auto animation = effect->associated_animation();
if (!animation)
return {};
return;
auto output_progress = effect->transformed_progress();
if (!output_progress.has_value())
return {};
return;
if (!effect->key_frame_set())
return {};
return;
auto& keyframes = effect->key_frame_set()->keyframes_by_key;
@ -1281,7 +1279,7 @@ ErrorOr<void> StyleComputer::collect_animation_into(JS::NonnullGCPtr<Animations:
for (auto it = keyframes.begin(); it != keyframes.end(); ++it)
dbgln(" - {}", it.key());
}
return {};
return;
}
auto keyframe_start = matching_keyframe_it.key();
@ -1341,8 +1339,6 @@ ErrorOr<void> StyleComputer::collect_animation_into(JS::NonnullGCPtr<Animations:
dbgln_if(LIBWEB_CSS_ANIMATION_DEBUG, "Interpolated value for property {} at {}: {} -> {} = {}", string_from_property_id(it.key), progress_in_keyframe, start->to_string(), end->to_string(), next_value->to_string());
style_properties.set_property(it.key, next_value);
}
return {};
}
static void apply_animation_properties(DOM::Document& document, StyleProperties& style, Animations::Animation& animation)
@ -1412,7 +1408,7 @@ static void apply_animation_properties(DOM::Document& document, StyleProperties&
}
// https://www.w3.org/TR/css-cascade/#cascading
ErrorOr<void> StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element& element, Optional<CSS::Selector::PseudoElement::Type> pseudo_element, bool& did_match_any_pseudo_element_rules, ComputeStyleMode mode) const
void StyleComputer::compute_cascaded_values(StyleProperties& style, DOM::Element& element, Optional<CSS::Selector::PseudoElement::Type> pseudo_element, bool& did_match_any_pseudo_element_rules, ComputeStyleMode mode) const
{
// First, we collect all the CSS rules whose selectors match `element`:
MatchingRuleSet matching_rule_set;
@ -1427,13 +1423,13 @@ ErrorOr<void> StyleComputer::compute_cascaded_values(StyleProperties& style, DOM
VERIFY(pseudo_element.has_value());
if (matching_rule_set.author_rules.is_empty() && matching_rule_set.user_rules.is_empty() && matching_rule_set.user_agent_rules.is_empty()) {
did_match_any_pseudo_element_rules = false;
return {};
return;
}
did_match_any_pseudo_element_rules = true;
}
// Then we resolve all the CSS custom properties ("variables") for this element:
TRY(cascade_custom_properties(element, pseudo_element, matching_rule_set.author_rules));
cascade_custom_properties(element, pseudo_element, matching_rule_set.author_rules);
// Then we apply the declarations from the matched rules in cascade order:
@ -1520,7 +1516,7 @@ ErrorOr<void> StyleComputer::compute_cascaded_values(StyleProperties& style, DOM
if (auto effect = animation->effect(); effect && effect->is_keyframe_effect()) {
auto& keyframe_effect = *static_cast<Animations::KeyframeEffect*>(effect.ptr());
if (keyframe_effect.pseudo_element_type() == pseudo_element)
TRY(collect_animation_into(keyframe_effect, style));
collect_animation_into(keyframe_effect, style);
}
}
@ -1534,8 +1530,6 @@ ErrorOr<void> StyleComputer::compute_cascaded_values(StyleProperties& style, DOM
cascade_declarations(style, element, pseudo_element, matching_rule_set.user_agent_rules, CascadeOrigin::UserAgent, Important::Yes);
// FIXME: Transition declarations [css-transitions-1]
return {};
}
DOM::Element const* element_to_inherit_style_from(DOM::Element const* element, Optional<CSS::Selector::PseudoElement::Type> pseudo_element)
@ -2199,25 +2193,24 @@ NonnullRefPtr<StyleProperties> StyleComputer::create_document_style() const
return style;
}
ErrorOr<NonnullRefPtr<StyleProperties>> StyleComputer::compute_style(DOM::Element& element, Optional<CSS::Selector::PseudoElement::Type> pseudo_element) const
NonnullRefPtr<StyleProperties> StyleComputer::compute_style(DOM::Element& element, Optional<CSS::Selector::PseudoElement::Type> pseudo_element) const
{
auto style = TRY(compute_style_impl(element, move(pseudo_element), ComputeStyleMode::Normal));
return style.release_nonnull();
return compute_style_impl(element, move(pseudo_element), ComputeStyleMode::Normal).release_nonnull();
}
ErrorOr<RefPtr<StyleProperties>> StyleComputer::compute_pseudo_element_style_if_needed(DOM::Element& element, Optional<CSS::Selector::PseudoElement::Type> pseudo_element) const
RefPtr<StyleProperties> StyleComputer::compute_pseudo_element_style_if_needed(DOM::Element& element, Optional<CSS::Selector::PseudoElement::Type> pseudo_element) const
{
return compute_style_impl(element, move(pseudo_element), ComputeStyleMode::CreatePseudoElementStyleIfNeeded);
}
ErrorOr<RefPtr<StyleProperties>> StyleComputer::compute_style_impl(DOM::Element& element, Optional<CSS::Selector::PseudoElement::Type> pseudo_element, ComputeStyleMode mode) const
RefPtr<StyleProperties> StyleComputer::compute_style_impl(DOM::Element& element, Optional<CSS::Selector::PseudoElement::Type> pseudo_element, ComputeStyleMode mode) const
{
build_rule_cache_if_needed();
// Special path for elements that use pseudo element as style selector
if (element.use_pseudo_element().has_value()) {
auto& parent_element = verify_cast<HTML::HTMLElement>(*element.root().parent_or_shadow_host());
auto style = TRY(compute_style(parent_element, *element.use_pseudo_element()));
auto style = compute_style(parent_element, *element.use_pseudo_element());
// Merge back inline styles
if (element.has_attribute(HTML::AttributeNames::style)) {
@ -2231,7 +2224,7 @@ ErrorOr<RefPtr<StyleProperties>> StyleComputer::compute_style_impl(DOM::Element&
auto style = StyleProperties::create();
// 1. Perform the cascade. This produces the "specified style"
bool did_match_any_pseudo_element_rules = false;
TRY(compute_cascaded_values(style, element, pseudo_element, did_match_any_pseudo_element_rules, mode));
compute_cascaded_values(style, element, pseudo_element, did_match_any_pseudo_element_rules, mode);
if (mode == ComputeStyleMode::CreatePseudoElementStyleIfNeeded && !did_match_any_pseudo_element_rules)
return nullptr;

View file

@ -52,8 +52,8 @@ public:
NonnullRefPtr<StyleProperties> create_document_style() const;
ErrorOr<NonnullRefPtr<StyleProperties>> compute_style(DOM::Element&, Optional<CSS::Selector::PseudoElement::Type> = {}) const;
ErrorOr<RefPtr<StyleProperties>> compute_pseudo_element_style_if_needed(DOM::Element&, Optional<CSS::Selector::PseudoElement::Type>) const;
NonnullRefPtr<StyleProperties> compute_style(DOM::Element&, Optional<CSS::Selector::PseudoElement::Type> = {}) const;
RefPtr<StyleProperties> compute_pseudo_element_style_if_needed(DOM::Element&, Optional<CSS::Selector::PseudoElement::Type>) const;
// https://www.w3.org/TR/css-cascade/#origin
enum class CascadeOrigin {
@ -87,8 +87,8 @@ private:
class FontLoader;
struct MatchingFontCandidate;
ErrorOr<RefPtr<StyleProperties>> compute_style_impl(DOM::Element&, Optional<CSS::Selector::PseudoElement::Type>, ComputeStyleMode) const;
ErrorOr<void> compute_cascaded_values(StyleProperties&, DOM::Element&, Optional<CSS::Selector::PseudoElement::Type>, bool& did_match_any_pseudo_element_rules, ComputeStyleMode) const;
RefPtr<StyleProperties> compute_style_impl(DOM::Element&, Optional<CSS::Selector::PseudoElement::Type>, ComputeStyleMode) const;
void compute_cascaded_values(StyleProperties&, DOM::Element&, Optional<CSS::Selector::PseudoElement::Type>, bool& did_match_any_pseudo_element_rules, ComputeStyleMode) 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);
RefPtr<Gfx::FontCascadeList const> font_matching_algorithm(FontFaceKey const& key, float font_size_in_pt) const;
@ -136,7 +136,7 @@ private:
RuleCache const& rule_cache_for_cascade_origin(CascadeOrigin) const;
ErrorOr<void> collect_animation_into(JS::NonnullGCPtr<Animations::KeyframeEffect> animation, StyleProperties& style_properties) const;
void collect_animation_into(JS::NonnullGCPtr<Animations::KeyframeEffect> animation, StyleProperties& style_properties) const;
OwnPtr<RuleCache> m_author_rule_cache;
OwnPtr<RuleCache> m_user_rule_cache;

View file

@ -572,8 +572,7 @@ Element::RequiredInvalidationAfterStyleChange Element::recompute_style()
set_needs_style_update(false);
VERIFY(parent());
// FIXME propagate errors
auto new_computed_css_values = MUST(document().style_computer().compute_style(*this));
auto new_computed_css_values = document().style_computer().compute_style(*this);
// Tables must not inherit -libweb-* values for text-align.
// FIXME: Find the spec for this.

View file

@ -189,14 +189,14 @@ void TreeBuilder::insert_node_into_inline_or_block_ancestor(Layout::Node& node,
}
}
ErrorOr<void> TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element, CSS::Selector::PseudoElement::Type pseudo_element, AppendOrPrepend mode)
void TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element, CSS::Selector::PseudoElement::Type pseudo_element, AppendOrPrepend mode)
{
auto& document = element.document();
auto& style_computer = document.style_computer();
auto pseudo_element_style = TRY(style_computer.compute_pseudo_element_style_if_needed(element, pseudo_element));
auto pseudo_element_style = style_computer.compute_pseudo_element_style_if_needed(element, pseudo_element);
if (!pseudo_element_style)
return {};
return;
auto initial_quote_nesting_level = m_quote_nesting_level;
auto [pseudo_element_content, final_quote_nesting_level] = pseudo_element_style->content(initial_quote_nesting_level);
@ -207,11 +207,11 @@ ErrorOr<void> TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element
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;
auto generated_for = Node::GeneratedFor::NotGenerated;
if (pseudo_element == CSS::Selector::PseudoElement::Type::Before) {
@ -240,8 +240,6 @@ ErrorOr<void> TreeBuilder::create_pseudo_element_if_needed(DOM::Element& element
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 {};
}
static bool is_ignorable_whitespace(Layout::Node const& node)
@ -289,7 +287,7 @@ i32 TreeBuilder::calculate_list_item_index(DOM::Node& dom_node)
return 1;
}
ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& context)
void TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::Context& context)
{
JS::GCPtr<Layout::Node> layout_node;
Optional<TemporaryChange<bool>> has_svg_root_change;
@ -311,7 +309,7 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
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();
@ -326,7 +324,7 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
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::Document>(dom_node)) {
style = style_computer.create_document_style();
@ -338,7 +336,7 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
}
if (!layout_node)
return {};
return;
if (!dom_node.parent_or_shadow_host()) {
m_layout_root = layout_node;
@ -354,7 +352,7 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
if (is<DOM::Element>(dom_node) && layout_node->can_have_children()) {
auto& element = static_cast<DOM::Element&>(dom_node);
push_parent(verify_cast<NodeWithStyle>(*layout_node));
TRY(create_pseudo_element_if_needed(element, CSS::Selector::PseudoElement::Type::Before, AppendOrPrepend::Prepend));
create_pseudo_element_if_needed(element, CSS::Selector::PseudoElement::Type::Before, AppendOrPrepend::Prepend);
pop_parent();
}
@ -362,19 +360,19 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
push_parent(verify_cast<NodeWithStyle>(*layout_node));
if (shadow_root) {
for (auto* node = shadow_root->first_child(); node; node = node->next_sibling()) {
TRY(create_layout_tree(*node, context));
create_layout_tree(*node, context);
}
} else {
// This is the same as verify_cast<DOM::ParentNode>(dom_node).for_each_child
for (auto* node = verify_cast<DOM::ParentNode>(dom_node).first_child(); node; node = node->next_sibling())
TRY(create_layout_tree(*node, context));
create_layout_tree(*node, context);
}
pop_parent();
}
if (is<ListItemBox>(*layout_node)) {
auto& element = static_cast<DOM::Element&>(dom_node);
auto marker_style = TRY(style_computer.compute_style(element, CSS::Selector::PseudoElement::Type::Marker));
auto marker_style = style_computer.compute_style(element, CSS::Selector::PseudoElement::Type::Marker);
auto list_item_marker = document.heap().allocate_without_realm<ListItemMarkerBox>(document, layout_node->computed_values().list_style_type(), layout_node->computed_values().list_style_position(), calculate_list_item_index(dom_node), *marker_style);
static_cast<ListItemBox&>(*layout_node).set_marker(list_item_marker);
element.set_pseudo_element_node({}, CSS::Selector::PseudoElement::Type::Marker, list_item_marker);
@ -385,11 +383,8 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
auto slottables = static_cast<HTML::HTMLSlotElement&>(dom_node).assigned_nodes_internal();
push_parent(verify_cast<NodeWithStyle>(*layout_node));
for (auto const& slottable : slottables) {
TRY(slottable.visit([&](auto& node) -> ErrorOr<void> {
return create_layout_tree(node, context);
}));
}
for (auto const& slottable : slottables)
slottable.visit([&](auto& node) { create_layout_tree(node, context); });
pop_parent();
}
@ -445,11 +440,9 @@ ErrorOr<void> TreeBuilder::create_layout_tree(DOM::Node& dom_node, TreeBuilder::
if (is<DOM::Element>(dom_node) && layout_node->can_have_children()) {
auto& element = static_cast<DOM::Element&>(dom_node);
push_parent(verify_cast<NodeWithStyle>(*layout_node));
TRY(create_pseudo_element_if_needed(element, CSS::Selector::PseudoElement::Type::After, AppendOrPrepend::Append));
create_pseudo_element_if_needed(element, CSS::Selector::PseudoElement::Type::After, AppendOrPrepend::Append);
pop_parent();
}
return {};
}
JS::GCPtr<Layout::Node> TreeBuilder::build(DOM::Node& dom_node)
@ -458,7 +451,7 @@ JS::GCPtr<Layout::Node> TreeBuilder::build(DOM::Node& dom_node)
Context context;
m_quote_nesting_level = 0;
MUST(create_layout_tree(dom_node, context)); // FIXME propagate errors
create_layout_tree(dom_node, context);
if (auto* root = dom_node.document().layout_node())
fixup_tables(*root);

View file

@ -27,7 +27,7 @@ private:
i32 calculate_list_item_index(DOM::Node&);
ErrorOr<void> create_layout_tree(DOM::Node&, Context&);
void 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(); }
@ -49,7 +49,7 @@ private:
Prepend,
};
void insert_node_into_inline_or_block_ancestor(Layout::Node&, CSS::Display, AppendOrPrepend);
ErrorOr<void> create_pseudo_element_if_needed(DOM::Element&, CSS::Selector::PseudoElement::Type, AppendOrPrepend);
void create_pseudo_element_if_needed(DOM::Element&, CSS::Selector::PseudoElement::Type, AppendOrPrepend);
JS::GCPtr<Layout::Node> m_layout_root;
Vector<JS::NonnullGCPtr<Layout::NodeWithStyle>> m_ancestor_stack;

View file

@ -481,7 +481,7 @@ void ConnectionFromClient::debug_request(u64 page_id, ByteString const& request,
for (auto& child : element->children_as_vector())
elements_to_visit.enqueue(child.ptr());
if (element->is_element()) {
auto styles = doc->style_computer().compute_style(*static_cast<Web::DOM::Element*>(element)).release_value_but_fixme_should_propagate_errors();
auto styles = doc->style_computer().compute_style(*static_cast<Web::DOM::Element*>(element));
dbgln("+ Element {}", element->debug_description());
auto& properties = styles->properties();
for (size_t i = 0; i < properties.size(); ++i)
@ -704,7 +704,7 @@ void ConnectionFromClient::inspect_dom_node(u64 page_id, i32 node_id, Optional<W
// 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 = MUST(page.page().focused_context().active_document()->style_computer().compute_style(element, pseudo_element));
auto pseudo_element_style = page.page().focused_context().active_document()->style_computer().compute_style(element, pseudo_element);
ByteString computed_values = serialize_json(pseudo_element_style);
ByteString resolved_values = "{}";
ByteString custom_properties_json = serialize_custom_properties_json(element, pseudo_element);