LibWeb: Make HTML::SharedImageRequest GC allocated

This allows to partially solve the problem of cyclic dependency between
HTMLImageElement and SharedImageRequest that prevents all image
elements from being deallocated.

Userland/Libraries/LibWeb/CSS/StyleValues/ImageStyleValue.cpp

@@ -31,7 +31,7 @@ void ImageStyleValue::load_any_resources(DOM::Document& document)
         return;
     m_document = &document;
 
-    m_image_request = HTML::SharedImageRequest::get_or_create(*document.page(), m_url).release_value_but_fixme_should_propagate_errors();
+    m_image_request = HTML::SharedImageRequest::get_or_create(document.realm(), *document.page(), m_url);
     m_image_request->add_callbacks(
         [this, weak_this = make_weak_ptr()] {
             if (!weak_this)

Userland/Libraries/LibWeb/CSS/StyleValues/ImageStyleValue.h

@@ -10,6 +10,7 @@
 #pragma once
 
 #include <AK/URL.h>
+#include <LibJS/Heap/Handle.h>
 #include <LibWeb/CSS/Enums.h>
 #include <LibWeb/CSS/StyleValues/AbstractImageStyleValue.h>
 
@@ -43,7 +44,7 @@ public:
 private:
     ImageStyleValue(AK::URL const&);
 
-    RefPtr<HTML::SharedImageRequest> m_image_request;
+    JS::Handle<HTML::SharedImageRequest> m_image_request;
 
     void animate();
     Gfx::Bitmap const* bitmap(size_t frame_index, Gfx::IntSize = {}) const;

Userland/Libraries/LibWeb/HTML/HTMLImageElement.cpp

@@ -394,7 +394,7 @@ ErrorOr<void> HTMLImageElement::update_the_image_data(bool restart_animations, b
                     restart_the_animation();
 
                 // 2. Set current request's current URL to urlString.
-                m_current_request->set_current_url(url_string);
+                m_current_request->set_current_url(realm(), url_string);
 
                 // 3. If maybe omit events is not set or previousURL is not equal to urlString, then fire an event named load at the img element.
                 if (!maybe_omit_events || previous_url != url_string)
@@ -433,7 +433,7 @@ after_step_7:
             // 2. Queue an element task on the DOM manipulation task source given the img element and the following steps:
             queue_an_element_task(HTML::Task::Source::DOMManipulation, [this, maybe_omit_events, previous_url] {
                 // 1. Change the current request's current URL to the empty string.
-                m_current_request->set_current_url(""sv);
+                m_current_request->set_current_url(realm(), ""sv);
 
                 // 2. If all of the following conditions are true:
                 //    - the element has a src attribute or it uses srcset or picture; and
@@ -466,7 +466,7 @@ after_step_7:
             // 4. Queue an element task on the DOM manipulation task source given the img element and the following steps:
             queue_an_element_task(HTML::Task::Source::DOMManipulation, [this, selected_source, maybe_omit_events, previous_url] {
                 // 1. Change the current request's current URL to selected source.
-                m_current_request->set_current_url(selected_source.value().url);
+                m_current_request->set_current_url(realm(), selected_source.value().url);
 
                 // 2. If maybe omit events is not set or previousURL is not equal to selected source, then fire an event named error at the img element.
                 if (!maybe_omit_events || previous_url != selected_source.value().url)
@@ -503,7 +503,7 @@ after_step_7:
 
         // 16. Set image request to a new image request whose current URL is urlString.
         auto image_request = ImageRequest::create(realm(), *document().page());
-        image_request->set_current_url(url_string);
+        image_request->set_current_url(realm(), url_string);
 
         // 17. If current request's state is unavailable or broken, then set the current request to image request.
         //     Otherwise, set the pending request to image request.
@@ -698,7 +698,7 @@ void HTMLImageElement::react_to_changes_in_the_environment()
 
     // 11. ⌛ Let image request be a new image request whose current URL is urlString
     auto image_request = ImageRequest::create(realm(), *document().page());
-    image_request->set_current_url(url_string);
+    image_request->set_current_url(realm(), url_string);
 
     // 12. ⌛ Let the element's pending request be image request.
     m_pending_request = image_request;

Userland/Libraries/LibWeb/HTML/HTMLObjectElement.cpp

@@ -39,6 +39,12 @@ void HTMLObjectElement::initialize(JS::Realm& realm)
     set_prototype(&Bindings::ensure_web_prototype<Bindings::HTMLObjectElementPrototype>(realm, "HTMLObjectElement"));
 }
 
+void HTMLObjectElement::visit_edges(Cell::Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_image_request);
+}
+
 void HTMLObjectElement::attribute_changed(DeprecatedFlyString const& name, DeprecatedString const& value)
 {
     NavigableContainer::attribute_changed(name, value);
@@ -316,7 +322,7 @@ void HTMLObjectElement::load_image()
     // NOTE: This currently reloads the image instead of reusing the resource we've already downloaded.
     auto data = attribute(HTML::AttributeNames::data);
     auto url = document().parse_url(data);
-    m_image_request = HTML::SharedImageRequest::get_or_create(*document().page(), url).release_value_but_fixme_should_propagate_errors();
+    m_image_request = HTML::SharedImageRequest::get_or_create(realm(), *document().page(), url);
     m_image_request->add_callbacks(
         [this] {
             run_object_representation_completed_steps(Representation::Image);

Userland/Libraries/LibWeb/HTML/HTMLObjectElement.h

@@ -45,6 +45,8 @@ public:
     // https://html.spec.whatwg.org/multipage/forms.html#category-listed
     virtual bool is_listed() const override { return true; }
 
+    virtual void visit_edges(Cell::Visitor&) override;
+
 private:
     HTMLObjectElement(DOM::Document&, DOM::QualifiedName);
 
@@ -80,7 +82,7 @@ private:
 
     RefPtr<DecodedImageData const> image_data() const;
 
-    RefPtr<SharedImageRequest> m_image_request;
+    JS::GCPtr<SharedImageRequest> m_image_request;
 };
 
 }

Userland/Libraries/LibWeb/HTML/ImageRequest.cpp

@@ -33,6 +33,12 @@ ImageRequest::~ImageRequest()
 {
 }
 
+void ImageRequest::visit_edges(JS::Cell::Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_shared_image_request);
+}
+
 // https://html.spec.whatwg.org/multipage/images.html#img-available
 bool ImageRequest::is_available() const
 {
@@ -60,11 +66,11 @@ AK::URL const& ImageRequest::current_url() const
     return m_current_url;
 }
 
-void ImageRequest::set_current_url(AK::URL url)
+void ImageRequest::set_current_url(JS::Realm& realm, AK::URL url)
 {
     m_current_url = move(url);
     if (m_current_url.is_valid())
-        m_shared_image_request = SharedImageRequest::get_or_create(m_page, m_current_url).release_value_but_fixme_should_propagate_errors();
+        m_shared_image_request = SharedImageRequest::get_or_create(realm, m_page, m_current_url);
 }
 
 // https://html.spec.whatwg.org/multipage/images.html#abort-the-image-request

Userland/Libraries/LibWeb/HTML/ImageRequest.h

@@ -40,7 +40,7 @@ public:
     void set_state(State);
 
     AK::URL const& current_url() const;
-    void set_current_url(AK::URL);
+    void set_current_url(JS::Realm&, AK::URL);
 
     [[nodiscard]] RefPtr<DecodedImageData const> image_data() const;
     void set_image_data(RefPtr<DecodedImageData const>);
@@ -59,6 +59,8 @@ public:
 
     SharedImageRequest const* shared_image_request() const { return m_shared_image_request; }
 
+    virtual void visit_edges(JS::Cell::Visitor&) override;
+
 private:
     explicit ImageRequest(Page&);
 
@@ -84,7 +86,7 @@ private:
     // which is either a struct consisting of a width and a height or is null. It must initially be null.
     Optional<Gfx::FloatSize> m_preferred_density_corrected_dimensions;
 
-    RefPtr<SharedImageRequest> m_shared_image_request;
+    JS::GCPtr<SharedImageRequest> m_shared_image_request;
 };
 
 // https://html.spec.whatwg.org/multipage/images.html#abort-the-image-request

Userland/Libraries/LibWeb/HTML/SharedImageRequest.cpp

@@ -24,11 +24,11 @@ static HashMap<AK::URL, SharedImageRequest*>& shared_image_requests()
     return requests;
 }
 
-ErrorOr<NonnullRefPtr<SharedImageRequest>> SharedImageRequest::get_or_create(Page& page, AK::URL const& url)
+JS::NonnullGCPtr<SharedImageRequest> SharedImageRequest::get_or_create(JS::Realm& realm, Page& page, AK::URL const& url)
 {
     if (auto it = shared_image_requests().find(url); it != shared_image_requests().end())
         return *it->value;
-    auto request = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) SharedImageRequest(page, url)));
+    auto request = realm.heap().allocate<SharedImageRequest>(realm, page, url);
     shared_image_requests().set(url, request);
     return request;
 }
@@ -44,6 +44,12 @@ SharedImageRequest::~SharedImageRequest()
     shared_image_requests().remove(m_url);
 }
 
+void SharedImageRequest::visit_edges(JS::Cell::Visitor& visitor)
+{
+    Base::visit_edges(visitor);
+    visitor.visit(m_fetch_controller);
+}
+
 RefPtr<DecodedImageData const> SharedImageRequest::image_data() const
 {
     return m_image_data;

Userland/Libraries/LibWeb/HTML/SharedImageRequest.h

@@ -16,9 +16,12 @@
 
 namespace Web::HTML {
 
-class SharedImageRequest : public RefCounted<SharedImageRequest> {
+class SharedImageRequest : public JS::Cell {
+    JS_CELL(ImageRequest, JS::Cell);
+
 public:
-    static ErrorOr<NonnullRefPtr<SharedImageRequest>> get_or_create(Page&, AK::URL const&);
+    [[nodiscard]] static JS::NonnullGCPtr<SharedImageRequest> get_or_create(JS::Realm&, Page&, AK::URL const&);
+
     ~SharedImageRequest();
 
     AK::URL const& url() const { return m_url; }
@@ -35,6 +38,8 @@ public:
     bool is_fetching() const;
     bool needs_fetching() const;
 
+    virtual void visit_edges(JS::Cell::Visitor&) override;
+
 private:
     explicit SharedImageRequest(Page&, AK::URL);
 
@@ -60,7 +65,7 @@ private:
 
     AK::URL m_url;
     RefPtr<DecodedImageData const> m_image_data;
-    JS::Handle<Fetch::Infrastructure::FetchController> m_fetch_controller;
+    JS::GCPtr<Fetch::Infrastructure::FetchController> m_fetch_controller;
 };
 
 }