LibWeb: Make StyleComputer and FontLoader GC-allocated

This allows them to keep style sheets alive while loading fonts for
them. Fixes some GC crashes seen on the WPT WOFF2 tests after
66a19b8550 stopped FetchRecord leaks from
keeping various other things alive.

Libraries/LibWeb/CSS/CSSStyleSheet.cpp

@@ -128,6 +128,7 @@ void CSSStyleSheet::visit_edges(Cell::Visitor& visitor)
     visitor.visit(m_namespace_rules);
     visitor.visit(m_import_rules);
     visitor.visit(m_owning_documents_or_shadow_roots);
+    visitor.visit(m_associated_font_loaders);
 }
 
 // https://www.w3.org/TR/cssom/#dom-cssstylesheet-insertrule

Libraries/LibWeb/CSS/CSSStyleSheet.h

@@ -91,7 +91,7 @@ public:
     void set_source_text(String);
     Optional<String> source_text(Badge<DOM::Document>) const;
 
-    void add_associated_font_loader(WeakPtr<FontLoader const> font_loader)
+    void add_associated_font_loader(GC::Ref<FontLoader const> font_loader)
     {
         m_associated_font_loaders.append(font_loader);
     }
@@ -126,7 +126,7 @@ private:
     bool m_disallow_modification { false };
     Optional<bool> m_did_match;
 
-    Vector<WeakPtr<FontLoader const>> m_associated_font_loaders;
+    Vector<GC::Ptr<FontLoader const>> m_associated_font_loaders;
 };
 
 }

Libraries/LibWeb/CSS/FontFace.cpp

@@ -499,7 +499,7 @@ GC::Ref<WebIDL::Promise> FontFace::load()
                 {},                // FIXME: feature_settings
                 {},                // FIXME: variation_settings
             };
-            if (auto loader = style_computer.load_font_face(parsed_font_face, move(on_load)); loader.has_value())
+            if (auto loader = style_computer.load_font_face(parsed_font_face, move(on_load)))
                 loader->start_loading_next_url();
         } else {
             // FIXME: Don't know how to load fonts in workers! They don't have a StyleComputer

Libraries/LibWeb/CSS/StyleComputer.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018-2023, Andreas Kling <andreas@ladybird.org>
+ * Copyright (c) 2018-2025, Andreas Kling <andreas@ladybird.org>
  * Copyright (c) 2021, the SerenityOS developers.
  * Copyright (c) 2021-2025, Sam Atkins <sam@ladybird.org>
  * Copyright (c) 2024, Matthew Olsson <mattco@serenityos.org>
@@ -94,6 +94,9 @@
 
 namespace Web::CSS {
 
+GC_DEFINE_ALLOCATOR(StyleComputer);
+GC_DEFINE_ALLOCATOR(FontLoader);
+
 struct FontFaceKey {
     NonnullRawPtr<FlyString const> family_name;
     int weight { 0 };
@@ -184,11 +187,20 @@ StyleComputer::StyleComputer(DOM::Document& document)
     , m_default_font_metrics(16, Platform::FontPlugin::the().default_font(16)->pixel_metrics())
     , m_root_element_font_metrics(m_default_font_metrics)
 {
+    m_ancestor_filter = make<CountingBloomFilter<u8, 14>>();
     m_qualified_layer_names_in_order.append({});
 }
 
 StyleComputer::~StyleComputer() = default;
 
+void StyleComputer::visit_edges(Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_document);
+    visitor.visit(m_loaded_fonts);
+    visitor.visit(m_user_style_sheet);
+}
+
 FontLoader::FontLoader(StyleComputer& style_computer, GC::Ptr<CSSStyleSheet> parent_style_sheet, FlyString family_name, Vector<Gfx::UnicodeRange> unicode_ranges, Vector<URL> urls, Function<void(RefPtr<Gfx::Typeface const>)> on_load)
     : m_style_computer(style_computer)
     , m_parent_style_sheet(parent_style_sheet)
@@ -201,6 +213,14 @@ FontLoader::FontLoader(StyleComputer& style_computer, GC::Ptr<CSSStyleSheet> par
 
 FontLoader::~FontLoader() = default;
 
+void FontLoader::visit_edges(Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_style_computer);
+    visitor.visit(m_parent_style_sheet);
+    visitor.visit(m_fetch_controller);
+}
+
 bool FontLoader::is_loading() const
 {
     return m_fetch_controller && !m_vector_font;
@@ -228,34 +248,29 @@ void FontLoader::start_loading_next_url()
     // To fetch a font given a selected <url> url for @font-face rule, fetch url, with stylesheet being rule’s parent
     // CSS style sheet, destination "font", CORS mode "cors", and processResponse being the following steps given
     // response res and null, failure or a byte stream stream:
-    auto style_sheet_or_document = m_parent_style_sheet ? StyleSheetOrDocument { *m_parent_style_sheet } : StyleSheetOrDocument { m_style_computer.document() };
+    auto style_sheet_or_document = m_parent_style_sheet ? StyleSheetOrDocument { *m_parent_style_sheet } : StyleSheetOrDocument { m_style_computer->document() };
     auto maybe_fetch_controller = fetch_a_style_resource(m_urls.take_first(), style_sheet_or_document, Fetch::Infrastructure::Request::Destination::Font, CorsMode::Cors,
-        [weak_loader = make_weak_ptr()](auto response, auto stream) {
-            // NB: If the FontLoader died before this fetch completed, nobody wants the data.
-            if (weak_loader.is_null())
-                return;
-            auto& loader = *weak_loader;
-
+        [loader = this](auto response, auto stream) {
             // 1. If stream is null, return.
             // 2. Load a font from stream according to its type.
 
             // NB: We need to fetch the next source if this one fails to fetch OR decode. So, first try to decode it.
             RefPtr<Gfx::Typeface const> typeface;
             if (auto* bytes = stream.template get_pointer<ByteBuffer>()) {
-                if (auto maybe_typeface = loader.try_load_font(response, *bytes); !maybe_typeface.is_error())
+                if (auto maybe_typeface = loader->try_load_font(response, *bytes); !maybe_typeface.is_error())
                     typeface = maybe_typeface.release_value();
             }
 
             if (!typeface) {
                 // NB: If we have other sources available, try the next one.
-                if (loader.m_urls.is_empty()) {
-                    loader.font_did_load_or_fail(nullptr);
+                if (loader->m_urls.is_empty()) {
+                    loader->font_did_load_or_fail(nullptr);
                 } else {
-                    loader.m_fetch_controller = nullptr;
-                    loader.start_loading_next_url();
+                    loader->m_fetch_controller = nullptr;
+                    loader->start_loading_next_url();
                 }
             } else {
-                loader.font_did_load_or_fail(move(typeface));
+                loader->font_did_load_or_fail(move(typeface));
             }
         });
 
@@ -270,7 +285,7 @@ void FontLoader::font_did_load_or_fail(RefPtr<Gfx::Typeface const> typeface)
 {
     if (typeface) {
         m_vector_font = typeface.release_nonnull();
-        m_style_computer.did_load_font(m_family_name);
+        m_style_computer->did_load_font(m_family_name);
         if (m_on_load)
             m_on_load(m_vector_font);
     } else {
@@ -3088,7 +3103,7 @@ void StyleComputer::did_load_font(FlyString const&)
     document().invalidate_style(DOM::StyleInvalidationReason::CSSFontLoaded);
 }
 
-Optional<FontLoader&> StyleComputer::load_font_face(ParsedFontFace const& font_face, Function<void(RefPtr<Gfx::Typeface const>)> on_load)
+GC::Ptr<FontLoader> StyleComputer::load_font_face(ParsedFontFace const& font_face, Function<void(RefPtr<Gfx::Typeface const>)> on_load)
 {
     if (font_face.sources().is_empty()) {
         if (on_load)
@@ -3116,14 +3131,14 @@ Optional<FontLoader&> StyleComputer::load_font_face(ParsedFontFace const& font_f
         return {};
     }
 
-    auto loader = make<FontLoader>(*this, font_face.parent_style_sheet(), font_face.font_family(), font_face.unicode_ranges(), move(urls), move(on_load));
+    auto loader = heap().allocate<FontLoader>(*this, font_face.parent_style_sheet(), font_face.font_family(), font_face.unicode_ranges(), move(urls), move(on_load));
     auto& loader_ref = *loader;
     auto maybe_font_loaders_list = m_loaded_fonts.get(key);
     if (maybe_font_loaders_list.has_value()) {
         maybe_font_loaders_list->append(move(loader));
     } else {
         FontLoaderList loaders;
-        loaders.append(move(loader));
+        loaders.append(loader);
         m_loaded_fonts.set(OwnFontFaceKey(key), move(loaders));
     }
     // Actual object owned by font loader list inside m_loaded_fonts, this isn't use-after-move/free
@@ -3138,8 +3153,8 @@ void StyleComputer::load_fonts_from_sheet(CSSStyleSheet& sheet)
         auto const& font_face_rule = static_cast<CSSFontFaceRule const&>(*rule);
         if (!font_face_rule.is_valid())
             continue;
-        if (auto font_loader = load_font_face(font_face_rule.font_face()); font_loader.has_value()) {
-            sheet.add_associated_font_loader(font_loader.value());
+        if (auto font_loader = load_font_face(font_face_rule.font_face())) {
+            sheet.add_associated_font_loader(*font_loader);
         }
     }
 }
@@ -3276,20 +3291,20 @@ static void for_each_element_hash(DOM::Element const& element, auto callback)
 
 void StyleComputer::reset_ancestor_filter()
 {
-    m_ancestor_filter.clear();
+    m_ancestor_filter->clear();
 }
 
 void StyleComputer::push_ancestor(DOM::Element const& element)
 {
     for_each_element_hash(element, [&](u32 hash) {
-        m_ancestor_filter.increment(hash);
+        m_ancestor_filter->increment(hash);
     });
 }
 
 void StyleComputer::pop_ancestor(DOM::Element const& element)
 {
     for_each_element_hash(element, [&](u32 hash) {
-        m_ancestor_filter.decrement(hash);
+        m_ancestor_filter->decrement(hash);
     });
 }
 

Libraries/LibWeb/CSS/StyleComputer.h

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018-2024, Andreas Kling <andreas@ladybird.org>
+ * Copyright (c) 2018-2025, Andreas Kling <andreas@ladybird.org>
  * Copyright (c) 2021-2024, Sam Atkins <sam@ladybird.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
@@ -126,7 +126,10 @@ struct RuleCache {
 
 class FontLoader;
 
-class StyleComputer {
+class StyleComputer final : public GC::Cell {
+    GC_CELL(StyleComputer, GC::Cell);
+    GC_DECLARE_ALLOCATOR(StyleComputer);
+
 public:
     static void for_each_property_expanding_shorthands(PropertyID, CSSStyleValue const&, Function<void(PropertyID, CSSStyleValue const&)> const& set_longhand_property);
     static NonnullRefPtr<CSSStyleValue const> get_inherit_value(CSS::PropertyID, DOM::Element const*, Optional<CSS::PseudoElement> = {});
@@ -162,7 +165,7 @@ public:
 
     void did_load_font(FlyString const& family_name);
 
-    Optional<FontLoader&> load_font_face(ParsedFontFace const&, ESCAPING Function<void(RefPtr<Gfx::Typeface const>)> on_load = {});
+    GC::Ptr<FontLoader> load_font_face(ParsedFontFace const&, ESCAPING Function<void(RefPtr<Gfx::Typeface const>)> on_load = {});
 
     void load_fonts_from_sheet(CSSStyleSheet&);
     void unload_fonts_from_sheet(CSSStyleSheet&);
@@ -196,6 +199,8 @@ public:
     static NonnullRefPtr<CSSStyleValue const> compute_value_of_custom_property(DOM::AbstractElement, FlyString const& custom_property, Optional<Parser::GuardedSubstitutionContexts&> = {});
 
 private:
+    virtual void visit_edges(Visitor&) override;
+
     enum class ComputeStyleMode {
         Normal,
         CreatePseudoElementStyleIfNeeded,
@@ -283,9 +288,9 @@ private:
     OwnPtr<RuleCachesForDocumentAndShadowRoots> m_author_rule_cache;
     OwnPtr<RuleCachesForDocumentAndShadowRoots> m_user_rule_cache;
     OwnPtr<RuleCachesForDocumentAndShadowRoots> m_user_agent_rule_cache;
-    GC::Root<CSSStyleSheet> m_user_style_sheet;
+    GC::Ptr<CSSStyleSheet> m_user_style_sheet;
 
-    using FontLoaderList = Vector<NonnullOwnPtr<FontLoader>>;
+    using FontLoaderList = Vector<GC::Ref<FontLoader>>;
     HashMap<OwnFontFaceKey, FontLoaderList> m_loaded_fonts;
 
     [[nodiscard]] Length::FontMetrics const& root_element_font_metrics_for_element(GC::Ptr<DOM::Element const>) const;
@@ -295,10 +300,13 @@ private:
 
     CSSPixelRect m_viewport_rect;
 
-    CountingBloomFilter<u8, 14> m_ancestor_filter;
+    OwnPtr<CountingBloomFilter<u8, 14>> m_ancestor_filter;
 };
 
-class FontLoader : public Weakable<FontLoader> {
+class FontLoader final : public GC::Cell {
+    GC_CELL(FontLoader, GC::Cell);
+    GC_DECLARE_ALLOCATOR(FontLoader);
+
 public:
     FontLoader(StyleComputer& style_computer, GC::Ptr<CSSStyleSheet> parent_style_sheet, FlyString family_name, Vector<Gfx::UnicodeRange> unicode_ranges, Vector<URL> urls, ESCAPING Function<void(RefPtr<Gfx::Typeface const>)> on_load = {});
 
@@ -313,17 +321,19 @@ public:
     bool is_loading() const;
 
 private:
+    virtual void visit_edges(Visitor&) override;
+
     ErrorOr<NonnullRefPtr<Gfx::Typeface const>> try_load_font(Fetch::Infrastructure::Response const&, ByteBuffer const&);
 
     void font_did_load_or_fail(RefPtr<Gfx::Typeface const>);
 
-    StyleComputer& m_style_computer;
+    GC::Ref<StyleComputer> m_style_computer;
     GC::Ptr<CSSStyleSheet> m_parent_style_sheet;
     FlyString m_family_name;
     Vector<Gfx::UnicodeRange> m_unicode_ranges;
     RefPtr<Gfx::Typeface const> m_vector_font;
     Vector<URL> m_urls;
-    GC::Root<Fetch::Infrastructure::FetchController> m_fetch_controller;
+    GC::Ptr<Fetch::Infrastructure::FetchController> m_fetch_controller;
     Function<void(RefPtr<Gfx::Typeface const>)> m_on_load;
 };
 
@@ -332,7 +342,7 @@ inline bool StyleComputer::should_reject_with_ancestor_filter(Selector const& se
     for (u32 hash : selector.ancestor_hashes()) {
         if (hash == 0)
             break;
-        if (!m_ancestor_filter.may_contain(hash))
+        if (!m_ancestor_filter->may_contain(hash))
             return true;
     }
     return false;

Libraries/LibWeb/DOM/Document.cpp

@@ -459,7 +459,7 @@ GC::Ref<Document> Document::create_for_fragment_parsing(JS::Realm& realm)
 Document::Document(JS::Realm& realm, const URL::URL& url, TemporaryDocumentForFragmentParsing temporary_document_for_fragment_parsing)
     : ParentNode(realm, *this, NodeType::DOCUMENT_NODE)
     , m_page(Bindings::principal_host_defined_page(realm))
-    , m_style_computer(make<CSS::StyleComputer>(*this))
+    , m_style_computer(realm.heap().allocate<CSS::StyleComputer>(*this))
     , m_url(url)
     , m_temporary_document_for_fragment_parsing(temporary_document_for_fragment_parsing)
     , m_editing_host_manager(EditingHostManager::create(realm, *this))
@@ -557,7 +557,7 @@ void Document::visit_edges(Cell::Visitor& visitor)
     visitor.visit(m_appropriate_template_contents_owner_document);
     visitor.visit(m_pending_parsing_blocking_script);
     visitor.visit(m_history);
-
+    visitor.visit(m_style_computer);
     visitor.visit(m_browsing_context);
 
     visitor.visit(m_applets);

Libraries/LibWeb/DOM/Document.h

@@ -965,7 +965,7 @@ private:
     void run_csp_initialization() const;
 
     GC::Ref<Page> m_page;
-    OwnPtr<CSS::StyleComputer> m_style_computer;
+    GC::Ptr<CSS::StyleComputer> m_style_computer;
     GC::Ptr<CSS::StyleSheetList> m_style_sheets;
     GC::Ptr<Node> m_active_favicon;
     GC::Ptr<HTML::BrowsingContext> m_browsing_context;