From 912ffc3f84fdb75d82e2847612c067fe3b3ed71e Mon Sep 17 00:00:00 2001 From: Callum Law Date: Mon, 25 Aug 2025 16:55:07 +1200 Subject: [PATCH] LibWeb: Remove unnecessary `ComputedProperties::maybe_null_property` We know that all (longhand) properties have a value so this is unnecessary. --- Libraries/LibWeb/CSS/CSSStyleProperties.cpp | 8 +------ Libraries/LibWeb/CSS/ComputedProperties.cpp | 10 --------- Libraries/LibWeb/CSS/ComputedProperties.h | 1 - Libraries/LibWeb/CSS/StyleComputer.cpp | 22 +++++++++----------- Libraries/LibWeb/DOM/Element.cpp | 10 +++------ Services/WebContent/ConnectionFromClient.cpp | 3 +-- 6 files changed, 15 insertions(+), 39 deletions(-) diff --git a/Libraries/LibWeb/CSS/CSSStyleProperties.cpp b/Libraries/LibWeb/CSS/CSSStyleProperties.cpp index 8560ac6a5cd..c23289a7976 100644 --- a/Libraries/LibWeb/CSS/CSSStyleProperties.cpp +++ b/Libraries/LibWeb/CSS/CSSStyleProperties.cpp @@ -187,15 +187,9 @@ Optional CSSStyleProperties::property(PropertyID property_id) con if (!layout_node) { auto style = element.document().style_computer().compute_style(element, pseudo_element); - // FIXME: This is a stopgap until we implement shorthand -> longhand conversion. - auto const* value = style->maybe_null_property(property_id); - if (!value) { - dbgln("FIXME: CSSStyleProperties::property(property_id={:#x}) No value for property ID in newly computed style case.", to_underlying(property_id)); - return {}; - } return StyleProperty { .property_id = property_id, - .value = *value, + .value = style->property(property_id), }; } diff --git a/Libraries/LibWeb/CSS/ComputedProperties.cpp b/Libraries/LibWeb/CSS/ComputedProperties.cpp index f60e4a9b373..a25fe809495 100644 --- a/Libraries/LibWeb/CSS/ComputedProperties.cpp +++ b/Libraries/LibWeb/CSS/ComputedProperties.cpp @@ -159,16 +159,6 @@ StyleValue const& ComputedProperties::property(PropertyID property_id, WithAnima return *m_property_values[to_underlying(property_id) - to_underlying(first_longhand_property_id)]; } -StyleValue const* ComputedProperties::maybe_null_property(PropertyID property_id) const -{ - VERIFY(property_id >= first_longhand_property_id && property_id <= last_longhand_property_id); - - if (auto animated_value = m_animated_property_values.get(property_id); animated_value.has_value()) - return animated_value.value(); - - return m_property_values[to_underlying(property_id) - to_underlying(first_longhand_property_id)]; -} - Variant ComputedProperties::gap_value(PropertyID id) const { auto const& value = property(id); diff --git a/Libraries/LibWeb/CSS/ComputedProperties.h b/Libraries/LibWeb/CSS/ComputedProperties.h index dd811b5f5d8..4963f589642 100644 --- a/Libraries/LibWeb/CSS/ComputedProperties.h +++ b/Libraries/LibWeb/CSS/ComputedProperties.h @@ -66,7 +66,6 @@ public: Yes, }; StyleValue const& property(PropertyID, WithAnimationsApplied = WithAnimationsApplied::Yes) const; - StyleValue const* maybe_null_property(PropertyID) const; void revert_property(PropertyID, ComputedProperties const& style_for_revert); GC::Ptr animation_name_source() const { return m_animation_name_source; } diff --git a/Libraries/LibWeb/CSS/StyleComputer.cpp b/Libraries/LibWeb/CSS/StyleComputer.cpp index 8a4c6dc486f..2e50b80e8bd 100644 --- a/Libraries/LibWeb/CSS/StyleComputer.cpp +++ b/Libraries/LibWeb/CSS/StyleComputer.cpp @@ -1348,23 +1348,23 @@ static void compute_transitioned_properties(ComputedProperties const& style, DOM } auto normalize_transition_length_list = [&properties, &style](PropertyID property, auto make_default_value) { - auto const* style_value = style.maybe_null_property(property); + auto const& style_value = style.property(property); StyleValueVector list; - if (style_value && !style_value->is_value_list()) { + if (!style_value.is_value_list()) { for (size_t i = 0; i < properties.size(); i++) - list.append(*style_value); + list.append(style_value); return list; } - if (!style_value || !style_value->is_value_list() || style_value->as_value_list().size() == 0) { + if (style_value.as_value_list().size() == 0) { auto default_value = make_default_value(); for (size_t i = 0; i < properties.size(); i++) list.append(default_value); return list; } - auto const& value_list = style_value->as_value_list(); + auto const& value_list = style_value.as_value_list(); for (size_t i = 0; i < properties.size(); i++) list.append(value_list.value_at(i, true)); @@ -2617,14 +2617,12 @@ GC::Ref StyleComputer::compute_properties(DOM::Element& elem // Animation declarations [css-animations-2] auto animation_name = [&]() -> Optional { - auto const animation_name = computed_style->maybe_null_property(PropertyID::AnimationName); - if (!animation_name) + auto const& animation_name = computed_style->property(PropertyID::AnimationName); + if (animation_name.is_keyword() && animation_name.to_keyword() == Keyword::None) return OptionalNone {}; - if (animation_name->is_keyword() && animation_name->to_keyword() == Keyword::None) - return OptionalNone {}; - if (animation_name->is_string()) - return animation_name->as_string().string_value().to_string(); - return animation_name->to_string(SerializationMode::Normal); + if (animation_name.is_string()) + return animation_name.as_string().string_value().to_string(); + return animation_name.to_string(SerializationMode::Normal); }(); if (animation_name.has_value()) { diff --git a/Libraries/LibWeb/DOM/Element.cpp b/Libraries/LibWeb/DOM/Element.cpp index d6905a63e16..758f1d27252 100644 --- a/Libraries/LibWeb/DOM/Element.cpp +++ b/Libraries/LibWeb/DOM/Element.cpp @@ -676,12 +676,8 @@ static CSS::RequiredInvalidationAfterStyleChange compute_required_invalidation(C for (auto i = to_underlying(CSS::first_longhand_property_id); i <= to_underlying(CSS::last_longhand_property_id); ++i) { auto property_id = static_cast(i); - auto old_value = old_style.maybe_null_property(property_id); - auto new_value = new_style.maybe_null_property(property_id); - if (!old_value && !new_value) - continue; - invalidation |= CSS::compute_property_invalidation(property_id, old_value, new_value); + invalidation |= CSS::compute_property_invalidation(property_id, old_style.property(property_id), new_style.property(property_id)); } return invalidation; } @@ -810,7 +806,7 @@ CSS::RequiredInvalidationAfterStyleChange Element::recompute_inherited_style() // FIXME: We should use the specified value rather than the cascaded value as the cascaded value may include // unresolved CSS-wide keywords (e.g. 'initial' or 'inherit') rather than the resolved value. auto const& preabsolutized_value = m_cascaded_properties->property(property_id); - RefPtr old_value = computed_properties->maybe_null_property(property_id); + RefPtr old_value = computed_properties->property(property_id); // Update property if it uses relative units as it might have been affected by a change in ancestor element style. if (preabsolutized_value && preabsolutized_value->is_length() && preabsolutized_value->as_length().length().is_font_relative()) { auto is_inherited = computed_properties->is_property_inherited(property_id); @@ -842,7 +838,7 @@ CSS::RequiredInvalidationAfterStyleChange Element::recompute_inherited_style() document().style_computer().absolutize_values(*computed_properties); for (auto [property_id, old_value] : old_values_with_relative_units) { - auto new_value = computed_properties->maybe_null_property(static_cast(property_id)); + auto const& new_value = computed_properties->property(static_cast(property_id)); invalidation |= CSS::compute_property_invalidation(static_cast(property_id), old_value, new_value); } diff --git a/Services/WebContent/ConnectionFromClient.cpp b/Services/WebContent/ConnectionFromClient.cpp index fb2ddf637d7..9086d84739e 100644 --- a/Services/WebContent/ConnectionFromClient.cpp +++ b/Services/WebContent/ConnectionFromClient.cpp @@ -328,8 +328,7 @@ void ConnectionFromClient::debug_request(u64 page_id, ByteString request, ByteSt auto dump_style = [](String const& title, Web::CSS::ComputedProperties const& style, HashMap const& custom_properties) { dbgln("+ {}", title); for (size_t i = to_underlying(Web::CSS::first_longhand_property_id); i < to_underlying(Web::CSS::last_longhand_property_id); ++i) { - auto property = style.maybe_null_property(static_cast(i)); - dbgln("| {} = {}", Web::CSS::string_from_property_id(static_cast(i)), property ? property->to_string(Web::CSS::SerializationMode::Normal) : ""_string); + dbgln("| {} = {}", Web::CSS::string_from_property_id(static_cast(i)), style.property(static_cast(i)).to_string(Web::CSS::SerializationMode::Normal)); } for (auto const& [name, property] : custom_properties) { dbgln("| {} = {}", name, property.value->to_string(Web::CSS::SerializationMode::Normal));