LibWeb: Account for animated values when computing font

Computing the font for an element in `compute_font` is premature since
we are yet to apply animated properties - instead we should compute the
value on the fly (with a cache to avoid unnecessary work) to ensure we
are respecting the latest values

Author: Calme1709

Libraries/LibWeb/CSS/ComputedProperties.cpp

@@ -11,6 +11,7 @@
 #include <LibGC/CellAllocator.h>
 #include <LibWeb/CSS/Clip.h>
 #include <LibWeb/CSS/ComputedProperties.h>
+#include <LibWeb/CSS/FontComputer.h>
 #include <LibWeb/CSS/StyleValues/ColorSchemeStyleValue.h>
 #include <LibWeb/CSS/StyleValues/ContentStyleValue.h>
 #include <LibWeb/CSS/StyleValues/CounterDefinitionsStyleValue.h>
@@ -47,6 +48,21 @@ namespace Web::CSS {
 
 GC_DEFINE_ALLOCATOR(ComputedProperties);
 
+static HashTable<PropertyID> font_affecting_properties()
+{
+    static HashTable<PropertyID> font_affecting_properties;
+    if (font_affecting_properties.is_empty()) {
+        font_affecting_properties.set(PropertyID::FontFamily);
+        font_affecting_properties.set(PropertyID::FontSize);
+        font_affecting_properties.set(PropertyID::FontStyle);
+        font_affecting_properties.set(PropertyID::FontWeight);
+        font_affecting_properties.set(PropertyID::FontWidth);
+        font_affecting_properties.set(PropertyID::FontVariationSettings);
+    }
+
+    return font_affecting_properties;
+}
+
 ComputedProperties::ComputedProperties() = default;
 
 ComputedProperties::~ComputedProperties() = default;
@@ -128,6 +144,9 @@ void ComputedProperties::set_property_without_modifying_flags(PropertyID id, Non
     VERIFY(id >= first_longhand_property_id && id <= last_longhand_property_id);
 
     m_property_values[to_underlying(id) - to_underlying(first_longhand_property_id)] = move(value);
+
+    if (font_affecting_properties().contains(id))
+        clear_computed_font_list_cache();
 }
 
 void ComputedProperties::revert_property(PropertyID id, ComputedProperties const& style_for_revert)
@@ -153,6 +172,9 @@ void ComputedProperties::set_animated_property(PropertyID id, NonnullRefPtr<Styl
 {
     m_animated_property_values.set(id, move(value));
     set_animated_property_inherited(id, inherited);
+
+    if (font_affecting_properties().contains(id))
+        clear_computed_font_list_cache();
 }
 
 void ComputedProperties::remove_animated_property(PropertyID id)
@@ -175,7 +197,7 @@ StyleValue const& ComputedProperties::property(PropertyID property_id, WithAnima
             return *animated_value.value();
     }
 
-    // By the time we call this method, all properties have values assigned.
+    // By the time we call this method, the property should have been assigned
     return *m_property_values[to_underlying(property_id) - to_underlying(first_longhand_property_id)];
 }
 
@@ -2517,6 +2539,26 @@ WillChange ComputedProperties::will_change() const
     return WillChange::make_auto();
 }
 
+ValueComparingNonnullRefPtr<Gfx::FontCascadeList const> ComputedProperties::computed_font_list(FontComputer const& font_computer) const
+{
+    if (!m_cached_computed_font_list) {
+        const_cast<ComputedProperties*>(this)->m_cached_computed_font_list = font_computer.compute_font_for_style_values(property(PropertyID::FontFamily), font_size(), font_slope(), font_weight(), font_width(), font_variation_settings());
+        VERIFY(!m_cached_computed_font_list->is_empty());
+    }
+
+    return *m_cached_computed_font_list;
+}
+
+ValueComparingNonnullRefPtr<Gfx::Font const> ComputedProperties::first_available_computed_font(FontComputer const& font_computer) const
+{
+    if (!m_cached_first_available_computed_font)
+        // https://drafts.csswg.org/css-fonts/#first-available-font
+        // First font for which the character U+0020 (space) is not excluded by a unicode-range
+        const_cast<ComputedProperties*>(this)->m_cached_first_available_computed_font = computed_font_list(font_computer)->font_for_code_point(' ');
+
+    return *m_cached_first_available_computed_font;
+}
+
 CSSPixels ComputedProperties::font_size() const
 {
     return property(PropertyID::FontSize).as_length().length().absolute_length_to_px();

Libraries/LibWeb/CSS/ComputedProperties.h

@@ -228,25 +228,9 @@ class WEB_API ComputedProperties final : public JS::Cell {
 
     WillChange will_change() const;
 
-    Gfx::FontCascadeList const& computed_font_list() const
-    {
-        VERIFY(m_font_list);
-        return *m_font_list;
-    }
-
-    Gfx::Font const& first_available_computed_font() const
-    {
-        VERIFY(m_first_available_computed_font);
-        return *m_first_available_computed_font;
-    }
-
-    void set_computed_font_list(NonnullRefPtr<Gfx::FontCascadeList const> font_list)
-    {
-        m_font_list = move(font_list);
-        // https://drafts.csswg.org/css-fonts/#first-available-font
-        // First font for which the character U+0020 (space) is not excluded by a unicode-range
-        m_first_available_computed_font = m_font_list->font_for_code_point(' ');
-    }
+    ValueComparingRefPtr<Gfx::FontCascadeList const> cached_computed_font_list() const { return m_cached_computed_font_list; }
+    ValueComparingNonnullRefPtr<Gfx::FontCascadeList const> computed_font_list(FontComputer const&) const;
+    ValueComparingNonnullRefPtr<Gfx::Font const> first_available_computed_font(FontComputer const&) const;
 
     [[nodiscard]] CSSPixels line_height() const;
     [[nodiscard]] CSSPixels font_size() const;
@@ -300,8 +284,14 @@ class WEB_API ComputedProperties final : public JS::Cell {
     Display m_display_before_box_type_transformation { InitialValues::display() };
 
     int m_math_depth { InitialValues::math_depth() };
-    RefPtr<Gfx::FontCascadeList const> m_font_list;
-    RefPtr<Gfx::Font const> m_first_available_computed_font;
+
+    RefPtr<Gfx::FontCascadeList const> m_cached_computed_font_list;
+    RefPtr<Gfx::Font const> m_cached_first_available_computed_font;
+    void clear_computed_font_list_cache()
+    {
+        m_cached_computed_font_list = nullptr;
+        m_cached_first_available_computed_font = nullptr;
+    }
 
     Optional<CSSPixels> m_line_height;
 

Libraries/LibWeb/CSS/FontComputer.cpp

@@ -352,7 +352,7 @@ RefPtr<Gfx::FontCascadeList const> FontComputer::font_matching_algorithm_impl(Fl
     return {};
 }
 
-RefPtr<Gfx::FontCascadeList const> FontComputer::compute_font_for_style_values(StyleValue const& font_family, CSSPixels const& font_size, int slope, double font_weight, Percentage const& font_width, HashMap<FlyString, double> const& font_variation_settings) const
+NonnullRefPtr<Gfx::FontCascadeList const> FontComputer::compute_font_for_style_values(StyleValue const& font_family, CSSPixels const& font_size, int slope, double font_weight, Percentage const& font_width, HashMap<FlyString, double> const& font_variation_settings) const
 {
     // FIXME: We round to int here as that is what is expected by our font infrastructure below
     auto width = round_to<int>(font_width.value());

Libraries/LibWeb/CSS/FontComputer.h

@@ -100,7 +100,7 @@ class WEB_API FontComputer final : public GC::Cell {
     void load_fonts_from_sheet(CSSStyleSheet&);
     void unload_fonts_from_sheet(CSSStyleSheet&);
 
-    RefPtr<Gfx::FontCascadeList const> compute_font_for_style_values(StyleValue const& font_family, CSSPixels const& font_size, int font_slope, double font_weight, Percentage const& font_width, HashMap<FlyString, double> const& font_variation_settings) const;
+    NonnullRefPtr<Gfx::FontCascadeList const> compute_font_for_style_values(StyleValue const& font_family, CSSPixels const& font_size, int font_slope, double font_weight, Percentage const& font_width, HashMap<FlyString, double> const& font_variation_settings) const;
 
     size_t number_of_css_font_faces_with_loading_in_progress() const;
 

Libraries/LibWeb/CSS/Length.cpp

@@ -132,8 +132,8 @@ Length::ResolutionContext Length::ResolutionContext::for_element(DOM::AbstractEl
 
     return Length::ResolutionContext {
         .viewport_rect = element.element().navigable()->viewport_rect(),
-        .font_metrics = { element.computed_properties()->font_size(), element.computed_properties()->first_available_computed_font().pixel_metrics(), element.computed_properties()->line_height() },
-        .root_font_metrics = { root_element->computed_properties()->font_size(), root_element->computed_properties()->first_available_computed_font().pixel_metrics(), element.computed_properties()->line_height() }
+        .font_metrics = { element.computed_properties()->font_size(), element.computed_properties()->first_available_computed_font(element.document().font_computer())->pixel_metrics(), element.computed_properties()->line_height() },
+        .root_font_metrics = { root_element->computed_properties()->font_size(), root_element->computed_properties()->first_available_computed_font(element.document().font_computer())->pixel_metrics(), element.computed_properties()->line_height() }
     };
 }
 

Libraries/LibWeb/CSS/StyleComputer.cpp

@@ -683,7 +683,7 @@ void StyleComputer::collect_animation_into(DOM::AbstractElement abstract_element
 
         Length::FontMetrics font_metrics {
             computed_properties.font_size(),
-            computed_properties.first_available_computed_font().pixel_metrics(),
+            computed_properties.first_available_computed_font(document().font_computer())->pixel_metrics(),
             computed_properties.line_height()
         };
 
@@ -797,7 +797,7 @@ void StyleComputer::collect_animation_into(DOM::AbstractElement abstract_element
                     .viewport_rect = viewport_rect(),
                     .font_metrics = {
                         computed_properties.font_size(),
-                        computed_properties.first_available_computed_font().pixel_metrics(),
+                        computed_properties.first_available_computed_font(document().font_computer())->pixel_metrics(),
                         inheritance_parent_has_computed_properties ? inheritance_parent->computed_properties()->line_height() : InitialValues::line_height() },
                     .root_font_metrics = m_root_element_font_metrics },
                 .abstract_element = abstract_element
@@ -1389,7 +1389,7 @@ Length::FontMetrics StyleComputer::calculate_root_element_font_metrics(ComputedP
 {
     auto const& root_value = style.property(CSS::PropertyID::FontSize);
 
-    auto font_pixel_metrics = style.first_available_computed_font().pixel_metrics();
+    auto font_pixel_metrics = style.first_available_computed_font(document().font_computer())->pixel_metrics();
     Length::FontMetrics font_metrics { m_default_font_metrics.font_size, font_pixel_metrics, InitialValues::line_height() };
     font_metrics.font_size = root_value.as_length().length().to_px(viewport_rect(), font_metrics, font_metrics);
     font_metrics.line_height = style.line_height();
@@ -1499,15 +1499,7 @@ void StyleComputer::compute_font(ComputedProperties& style, Optional<DOM::Abstra
         PropertyID::FontVariationSettings,
         compute_font_variation_settings(font_variation_settings_value, font_computation_context));
 
-    auto const& font_family = style.property(CSS::PropertyID::FontFamily);
-
-    auto font_list = document().font_computer().compute_font_for_style_values(font_family, style.font_size(), style.font_slope(), style.font_weight(), style.font_width(), style.font_variation_settings());
-    VERIFY(font_list);
-    VERIFY(!font_list->is_empty());
-
-    RefPtr<Gfx::Font const> const found_font = font_list->first();
-
-    style.set_computed_font_list(*font_list);
+    RefPtr<Gfx::Font const> const found_font = style.first_available_computed_font(m_document->font_computer());
 
     Length::FontMetrics line_height_font_metrics {
         style.font_size(),
@@ -1578,7 +1570,7 @@ void StyleComputer::compute_property_values(ComputedProperties& style, Optional<
 {
     Length::FontMetrics font_metrics {
         style.font_size(),
-        style.first_available_computed_font().pixel_metrics(),
+        style.first_available_computed_font(document().font_computer())->pixel_metrics(),
         style.line_height()
     };
 

Libraries/LibWeb/DOM/Element.cpp

@@ -697,7 +697,7 @@ static CSS::RequiredInvalidationAfterStyleChange compute_required_invalidation(C
 {
     CSS::RequiredInvalidationAfterStyleChange invalidation;
 
-    if (!old_style.computed_font_list().equals(new_style.computed_font_list()))
+    if (old_style.cached_computed_font_list() != new_style.cached_computed_font_list())
         invalidation.relayout = true;
 
     for (auto i = to_underlying(CSS::first_longhand_property_id); i <= to_underlying(CSS::last_longhand_property_id); ++i) {

Libraries/LibWeb/HTML/HTMLInputElement.cpp

@@ -149,7 +149,7 @@ void HTMLInputElement::adjust_computed_style(CSS::ComputedProperties& style)
     // NOTE: Other browsers apply a minimum height of a single line's line-height to single-line input elements.
     if (is_single_line() && style.property(CSS::PropertyID::Height).has_auto()) {
         auto current_line_height = style.line_height().to_double();
-        auto minimum_line_height = style.first_available_computed_font().pixel_size() * CSS::ComputedProperties::normal_line_height_scale;
+        auto minimum_line_height = style.first_available_computed_font(document().font_computer())->pixel_size() * CSS::ComputedProperties::normal_line_height_scale;
 
         // FIXME: Instead of overriding line-height, we should set height here instead.
         if (current_line_height < minimum_line_height)

Libraries/LibWeb/Layout/Node.cpp

@@ -397,7 +397,7 @@ void NodeWithStyle::apply_style(CSS::ComputedProperties const& computed_style)
     // NOTE: We have to be careful that font-related properties get set in the right order.
     //       m_font is used by Length::to_px() when resolving sizes against this layout node.
     //       That's why it has to be set before everything else.
-    computed_values.set_font_list(computed_style.computed_font_list());
+    computed_values.set_font_list(computed_style.computed_font_list(document().font_computer()));
     computed_values.set_font_size(computed_style.font_size());
     computed_values.set_font_weight(computed_style.font_weight());
     computed_values.set_line_height(computed_style.line_height());

Services/WebContent/ConnectionFromClient.cpp

@@ -540,7 +540,7 @@ void ConnectionFromClient::inspect_dom_node(u64 page_id, WebView::DOMNodePropert
     auto serialize_used_fonts = [&]() {
         JsonArray serialized;
 
-        properties->computed_font_list().for_each_font_entry([&](Gfx::FontCascadeList::Entry const& entry) {
+        properties->computed_font_list(node->document().font_computer())->for_each_font_entry([&](Gfx::FontCascadeList::Entry const& entry) {
             auto const& font = *entry.font;
 
             JsonObject font_object;