LibWeb/CSS: Fetch ImageStyleValue images closer to spec

We now don't absolutize the URL during parsing, keeping it as a CSS::URL
object which we then pass to the "fetch an external image for a
stylesheet" algorithm. Our version of this algorithm is a bit ad-hoc,
in order to make use of SharedResourceRequest. To try and reduce
duplication, I've pulled all of fetch_a_style_resource() into a static
function, apart from the "actually do the fetch" step.

Libraries/LibWeb/CSS/Fetch.cpp

@@ -9,11 +9,12 @@
 #include <LibWeb/CSS/Fetch.h>
 #include <LibWeb/DOM/Document.h>
 #include <LibWeb/Fetch/Fetching/Fetching.h>
+#include <LibWeb/HTML/SharedResourceRequest.h>
 
 namespace Web::CSS {
 
 // https://drafts.csswg.org/css-values-4/#fetch-a-style-resource
-void fetch_a_style_resource(StyleResourceURL const& url_value, StyleSheetOrDocument sheet_or_document, Fetch::Infrastructure::Request::Destination destination, CorsMode cors_mode, Fetch::Infrastructure::FetchAlgorithms::ProcessResponseConsumeBodyFunction process_response)
+static GC::Ptr<Fetch::Infrastructure::Request> fetch_a_style_resource_impl(StyleResourceURL const& url_value, StyleSheetOrDocument sheet_or_document, Fetch::Infrastructure::Request::Destination destination, CorsMode cors_mode)
 {
     // AD-HOC: Not every caller has a CSSStyleSheet, so allow passing a Document in instead for URL completion.
     //         Spec issue: https://github.com/w3c/csswg-drafts/issues/12065
@@ -39,7 +40,7 @@ void fetch_a_style_resource(StyleResourceURL const& url_value, StyleSheetOrDocum
         [](CSS::URL const& url) { return url.url(); });
     auto parsed_url = ::URL::Parser::basic_parse(url_string, base);
     if (!parsed_url.has_value())
-        return;
+        return {};
 
     // 4. Let req be a new request whose url is parsedUrl, whose destination is destination, mode is corsMode,
     //    origin is environmentSettings’s origin, credentials mode is "same-origin", use-url-credentials flag is set,
@@ -70,10 +71,65 @@ void fetch_a_style_resource(StyleResourceURL const& url_value, StyleSheetOrDocum
         request->set_initiator_type(Fetch::Infrastructure::Request::InitiatorType::CSS);
 
     // 8. Fetch req, with processresponseconsumebody set to processResponse.
-    Fetch::Infrastructure::FetchAlgorithms::Input fetch_algorithms_input {};
-    fetch_algorithms_input.process_response_consume_body = move(process_response);
+    // NB: Implemented by caller.
+    return request;
+}
 
-    (void)Fetch::Fetching::fetch(environment_settings.realm(), request, Fetch::Infrastructure::FetchAlgorithms::create(vm, move(fetch_algorithms_input)));
+// https://drafts.csswg.org/css-values-4/#fetch-a-style-resource
+void fetch_a_style_resource(StyleResourceURL const& url_value, StyleSheetOrDocument sheet_or_document, Fetch::Infrastructure::Request::Destination destination, CorsMode cors_mode, Fetch::Infrastructure::FetchAlgorithms::ProcessResponseConsumeBodyFunction process_response)
+{
+    if (auto request = fetch_a_style_resource_impl(url_value, sheet_or_document, destination, cors_mode)) {
+        auto& environment_settings = HTML::relevant_settings_object(sheet_or_document.visit([](auto& it) -> JS::Object& { return it; }));
+        auto& vm = environment_settings.vm();
+
+        Fetch::Infrastructure::FetchAlgorithms::Input fetch_algorithms_input {};
+        fetch_algorithms_input.process_response_consume_body = move(process_response);
+
+        (void)Fetch::Fetching::fetch(environment_settings.realm(), *request, Fetch::Infrastructure::FetchAlgorithms::create(vm, move(fetch_algorithms_input)));
+    }
+}
+
+// https://drafts.csswg.org/css-images-4/#fetch-an-external-image-for-a-stylesheet
+GC::Ptr<HTML::SharedResourceRequest> fetch_an_external_image_for_a_stylesheet(StyleResourceURL const& url_value, StyleSheetOrDocument sheet_or_document)
+{
+    // To fetch an external image for a stylesheet, given a <url> url and CSSStyleSheet sheet, fetch a style resource
+    // given url, with stylesheet CSSStyleSheet, destination "image", CORS mode "no-cors", and processResponse being
+    // the following steps given response res and null, failure or a byte stream byteStream: If byteStream is a byte
+    // stream, load the image from the byte stream.
+
+    // NB: We can't directly call fetch_a_style_resource() because we want to make use of SharedResourceRequest to
+    //     deduplicate image requests.
+
+    if (auto request = fetch_a_style_resource_impl(url_value, sheet_or_document, Fetch::Infrastructure::Request::Destination::Image, CorsMode::NoCors)) {
+
+        auto document = sheet_or_document.visit(
+            [&](GC::Ref<CSSStyleSheet> const& sheet) -> GC::Ref<DOM::Document> { return *sheet->owning_document(); },
+            [](GC::Ref<DOM::Document> const& document) -> GC::Ref<DOM::Document> { return document; });
+        auto& realm = document->realm();
+
+        auto shared_resource_request = HTML::SharedResourceRequest::get_or_create(realm, document->page(), request->url());
+        shared_resource_request->add_callbacks(
+            [document, weak_document = document->make_weak_ptr<DOM::Document>()] {
+                if (!weak_document)
+                    return;
+
+                if (auto navigable = document->navigable()) {
+                    // Once the image has loaded, we need to re-resolve CSS properties that depend on the image's dimensions.
+                    document->set_needs_to_resolve_paint_only_properties();
+
+                    // FIXME: Do less than a full repaint if possible?
+                    document->set_needs_display();
+                }
+            },
+            nullptr);
+
+        if (shared_resource_request->needs_fetching())
+            shared_resource_request->fetch_resource(realm, *request);
+
+        return shared_resource_request;
+    }
+
+    return nullptr;
 }
 
 }

Libraries/LibWeb/CSS/Fetch.h

@@ -10,6 +10,7 @@
 #include <LibWeb/CSS/URL.h>
 #include <LibWeb/Fetch/Infrastructure/FetchAlgorithms.h>
 #include <LibWeb/Fetch/Infrastructure/HTTP/Requests.h>
+#include <LibWeb/Forward.h>
 
 namespace Web::CSS {
 
@@ -26,4 +27,7 @@ using StyleSheetOrDocument = Variant<GC::Ref<CSSStyleSheet>, GC::Ref<DOM::Docume
 // https://drafts.csswg.org/css-values-4/#fetch-a-style-resource
 void fetch_a_style_resource(StyleResourceURL const& url, StyleSheetOrDocument, Fetch::Infrastructure::Request::Destination, CorsMode, Fetch::Infrastructure::FetchAlgorithms::ProcessResponseConsumeBodyFunction process_response);
 
+// https://drafts.csswg.org/css-images-4/#fetch-an-external-image-for-a-stylesheet
+GC::Ptr<HTML::SharedResourceRequest> fetch_an_external_image_for_a_stylesheet(StyleResourceURL const&, StyleSheetOrDocument);
+
 }

Libraries/LibWeb/CSS/Parser/ValueParsing.cpp

@@ -2013,13 +2013,10 @@ RefPtr<AbstractImageStyleValue> Parser::parse_image_value(TokenStream<ComponentV
     if (url.has_value()) {
         // If the value is a 'url(..)' parse as image, but if it is just a reference 'url(#xx)', leave it alone,
         // so we can parse as URL further on. These URLs are used as references inside SVG documents for masks.
+        // FIXME: Remove this special case once mask-image accepts `<image>`.
         if (!url->url().starts_with('#')) {
-            // FIXME: Stop completing the URL here
-            auto completed_url = complete_url(url->url());
-            if (completed_url.has_value()) {
-                tokens.discard_a_mark();
-                return ImageStyleValue::create(completed_url.release_value());
-            }
+            tokens.discard_a_mark();
+            return ImageStyleValue::create(url.release_value());
         }
         tokens.restore_a_mark();
         return nullptr;

Libraries/LibWeb/CSS/StyleValues/ImageStyleValue.cpp

@@ -1,18 +1,17 @@
 /*
  * Copyright (c) 2018-2023, Andreas Kling <andreas@ladybird.org>
  * Copyright (c) 2021, Tobias Christiansen <tobyase@serenityos.org>
- * Copyright (c) 2021-2023, Sam Atkins <atkinssj@serenityos.org>
+ * Copyright (c) 2021-2025, Sam Atkins <sam@ladybird.org>
  * Copyright (c) 2022-2023, MacDue <macdue@dueutil.tech>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
-#include "ImageStyleValue.h"
 #include <LibWeb/CSS/ComputedValues.h>
-#include <LibWeb/CSS/Serialize.h>
+#include <LibWeb/CSS/Fetch.h>
+#include <LibWeb/CSS/StyleValues/ImageStyleValue.h>
 #include <LibWeb/DOM/Document.h>
 #include <LibWeb/HTML/DecodedImageData.h>
-#include <LibWeb/HTML/ImageRequest.h>
 #include <LibWeb/HTML/PotentialCORSRequest.h>
 #include <LibWeb/HTML/SharedResourceRequest.h>
 #include <LibWeb/Painting/DisplayListRecorder.h>
@@ -21,7 +20,17 @@
 
 namespace Web::CSS {
 
-ImageStyleValue::ImageStyleValue(::URL::URL const& url)
+ValueComparingNonnullRefPtr<ImageStyleValue> ImageStyleValue::create(URL const& url)
+{
+    return adopt_ref(*new (nothrow) ImageStyleValue(url));
+}
+
+ValueComparingNonnullRefPtr<ImageStyleValue> ImageStyleValue::create(::URL::URL const& url)
+{
+    return adopt_ref(*new (nothrow) ImageStyleValue(URL { url.to_string() }));
+}
+
+ImageStyleValue::ImageStyleValue(URL const& url)
     : AbstractImageStyleValue(Type::Image)
     , m_url(url)
 {
@@ -35,6 +44,7 @@ void ImageStyleValue::visit_edges(JS::Cell::Visitor& visitor) const
     // FIXME: visit_edges in non-GC allocated classes is confusing pattern.
     //        Consider making CSSStyleValue to be GC allocated instead.
     visitor.visit(m_resource_request);
+    visitor.visit(m_style_sheet);
     visitor.visit(m_timer);
 }
 
@@ -44,37 +54,26 @@ void ImageStyleValue::load_any_resources(DOM::Document& document)
         return;
     m_document = &document;
 
-    m_resource_request = HTML::SharedResourceRequest::get_or_create(document.realm(), document.page(), m_url);
-    m_resource_request->add_callbacks(
-        [this, weak_this = make_weak_ptr()] {
-            if (!weak_this)
-                return;
+    if (m_style_sheet) {
+        m_resource_request = fetch_an_external_image_for_a_stylesheet(m_url, { *m_style_sheet });
+    } else {
+        m_resource_request = fetch_an_external_image_for_a_stylesheet(m_url, { document });
+    }
+    if (m_resource_request) {
+        m_resource_request->add_callbacks(
+            [this, weak_this = make_weak_ptr()] {
+                if (!weak_this || !m_document)
+                    return;
 
-            if (!m_document)
-                return;
-
-            if (auto navigable = m_document->navigable()) {
-                // Once the image has loaded, we need to re-resolve CSS properties that depend on the image's dimensions.
-                m_document->set_needs_to_resolve_paint_only_properties();
-
-                // FIXME: Do less than a full repaint if possible?
-                m_document->set_needs_display();
-            }
-
-            auto image_data = m_resource_request->image_data();
-            if (image_data->is_animated() && image_data->frame_count() > 1) {
-                m_timer = Platform::Timer::create(m_document->heap());
-                m_timer->set_interval(image_data->frame_duration(0));
-                m_timer->on_timeout = GC::create_function(m_document->heap(), [this] { animate(); });
-                m_timer->start();
-            }
-        },
-        nullptr);
-
-    if (m_resource_request->needs_fetching()) {
-        auto request = HTML::create_potential_CORS_request(document.vm(), m_url, Fetch::Infrastructure::Request::Destination::Image, HTML::CORSSettingAttribute::NoCORS);
-        request->set_client(&document.relevant_settings_object());
-        m_resource_request->fetch_resource(document.realm(), request);
+                auto image_data = m_resource_request->image_data();
+                if (image_data->is_animated() && image_data->frame_count() > 1) {
+                    m_timer = Platform::Timer::create(m_document->heap());
+                    m_timer->set_interval(image_data->frame_duration(0));
+                    m_timer->on_timeout = GC::create_function(m_document->heap(), [this] { animate(); });
+                    m_timer->start();
+                }
+            },
+            nullptr);
     }
 }
 
@@ -116,7 +115,7 @@ Gfx::ImmutableBitmap const* ImageStyleValue::bitmap(size_t frame_index, Gfx::Int
 
 String ImageStyleValue::to_string(SerializationMode) const
 {
-    return serialize_a_url(m_url.to_string());
+    return m_url.to_string();
 }
 
 bool ImageStyleValue::equals(CSSStyleValue const& other) const
@@ -177,4 +176,10 @@ Optional<Gfx::Color> ImageStyleValue::color_if_single_pixel_bitmap() const
     return {};
 }
 
+void ImageStyleValue::set_style_sheet(GC::Ptr<CSSStyleSheet> style_sheet)
+{
+    Base::set_style_sheet(style_sheet);
+    m_style_sheet = style_sheet;
+}
+
 }

Libraries/LibWeb/CSS/StyleValues/ImageStyleValue.h

@@ -1,7 +1,7 @@
 /*
  * Copyright (c) 2018-2020, Andreas Kling <andreas@ladybird.org>
  * Copyright (c) 2021, Tobias Christiansen <tobyase@serenityos.org>
- * Copyright (c) 2021-2023, Sam Atkins <atkinssj@serenityos.org>
+ * Copyright (c) 2021-2025, Sam Atkins <sam@ladybird.org>
  * Copyright (c) 2022-2023, MacDue <macdue@dueutil.tech>
  *
  * SPDX-License-Identifier: BSD-2-Clause
@@ -9,11 +9,10 @@
 
 #pragma once
 
-#include <LibGC/Root.h>
 #include <LibJS/Heap/Cell.h>
-#include <LibURL/URL.h>
 #include <LibWeb/CSS/Enums.h>
 #include <LibWeb/CSS/StyleValues/AbstractImageStyleValue.h>
+#include <LibWeb/CSS/URL.h>
 #include <LibWeb/Forward.h>
 
 namespace Web::CSS {
@@ -25,10 +24,9 @@ class ImageStyleValue final
     using Base = AbstractImageStyleValue;
 
 public:
-    static ValueComparingNonnullRefPtr<ImageStyleValue> create(::URL::URL const& url)
-    {
-        return adopt_ref(*new (nothrow) ImageStyleValue(url));
-    }
+    static ValueComparingNonnullRefPtr<ImageStyleValue> create(URL const&);
+    static ValueComparingNonnullRefPtr<ImageStyleValue> create(::URL::URL const&);
+
     virtual ~ImageStyleValue() override;
 
     virtual void visit_edges(JS::Cell::Visitor& visitor) const override;
@@ -53,14 +51,17 @@ public:
     GC::Ptr<HTML::DecodedImageData> image_data() const;
 
 private:
-    ImageStyleValue(::URL::URL const&);
+    ImageStyleValue(URL const&);
 
-    GC::Ptr<HTML::SharedResourceRequest> m_resource_request;
+    virtual void set_style_sheet(GC::Ptr<CSSStyleSheet>) override;
 
     void animate();
     Gfx::ImmutableBitmap const* bitmap(size_t frame_index, Gfx::IntSize = {}) const;
 
-    ::URL::URL m_url;
+    GC::Ptr<HTML::SharedResourceRequest> m_resource_request;
+    GC::Ptr<CSSStyleSheet> m_style_sheet;
+
+    URL m_url;
     WeakPtr<DOM::Document> m_document;
 
     size_t m_current_frame_index { 0 };