From df86e52d75207a20033a8e4e6d82438c94160aa6 Mon Sep 17 00:00:00 2001
From: Aliaksandr Kalenik <kalenik.aliaksandr@gmail.com>
Date: Mon, 25 Sep 2023 14:27:11 +0200
Subject: [PATCH] LibWeb: Use JS::HeapFunction for callbacks in
 SharedImageRequest

If a function that captures a GC-allocated object is owned by another
GC-allocated object, it is more preferable to use JS::HeapFunction.
This is because JS::HeapFunction is visited, unlike introducing a new
heap root as JS::SafeFunction does.
---
 .../Libraries/LibWeb/HTML/ImageRequest.cpp    |  2 +-
 Userland/Libraries/LibWeb/HTML/ImageRequest.h |  2 +-
 .../LibWeb/HTML/SharedImageRequest.cpp        | 20 ++++++++++++++-----
 .../LibWeb/HTML/SharedImageRequest.h          |  7 ++++---
 4 files changed, 21 insertions(+), 10 deletions(-)

diff --git a/Userland/Libraries/LibWeb/HTML/ImageRequest.cpp b/Userland/Libraries/LibWeb/HTML/ImageRequest.cpp
index 09d577702d..7763009ce7 100644
--- a/Userland/Libraries/LibWeb/HTML/ImageRequest.cpp
+++ b/Userland/Libraries/LibWeb/HTML/ImageRequest.cpp
@@ -125,7 +125,7 @@ void ImageRequest::fetch_image(JS::Realm& realm, JS::NonnullGCPtr<Fetch::Infrast
     m_shared_image_request->fetch_image(realm, request);
 }
 
-void ImageRequest::add_callbacks(JS::SafeFunction<void()> on_finish, JS::SafeFunction<void()> on_fail)
+void ImageRequest::add_callbacks(Function<void()> on_finish, Function<void()> on_fail)
 {
     VERIFY(m_shared_image_request);
     m_shared_image_request->add_callbacks(move(on_finish), move(on_fail));
diff --git a/Userland/Libraries/LibWeb/HTML/ImageRequest.h b/Userland/Libraries/LibWeb/HTML/ImageRequest.h
index c97812c918..bdb28e5a37 100644
--- a/Userland/Libraries/LibWeb/HTML/ImageRequest.h
+++ b/Userland/Libraries/LibWeb/HTML/ImageRequest.h
@@ -55,7 +55,7 @@ public:
     void prepare_for_presentation(HTMLImageElement&);
 
     void fetch_image(JS::Realm&, JS::NonnullGCPtr<Fetch::Infrastructure::Request>);
-    void add_callbacks(JS::SafeFunction<void()> on_finish, JS::SafeFunction<void()> on_fail);
+    void add_callbacks(Function<void()> on_finish, Function<void()> on_fail);
 
     SharedImageRequest const* shared_image_request() const { return m_shared_image_request; }
 
diff --git a/Userland/Libraries/LibWeb/HTML/SharedImageRequest.cpp b/Userland/Libraries/LibWeb/HTML/SharedImageRequest.cpp
index 3abb5c7b3b..2426c5dfff 100644
--- a/Userland/Libraries/LibWeb/HTML/SharedImageRequest.cpp
+++ b/Userland/Libraries/LibWeb/HTML/SharedImageRequest.cpp
@@ -48,6 +48,10 @@ void SharedImageRequest::visit_edges(JS::Cell::Visitor& visitor)
 {
     Base::visit_edges(visitor);
     visitor.visit(m_fetch_controller);
+    for (auto& callback : m_callbacks) {
+        visitor.visit(callback.on_finish);
+        visitor.visit(callback.on_fail);
+    }
 }
 
 RefPtr<DecodedImageData const> SharedImageRequest::image_data() const
@@ -99,7 +103,7 @@ void SharedImageRequest::fetch_image(JS::Realm& realm, JS::NonnullGCPtr<Fetch::I
     set_fetch_controller(fetch_controller);
 }
 
-void SharedImageRequest::add_callbacks(JS::SafeFunction<void()> on_finish, JS::SafeFunction<void()> on_fail)
+void SharedImageRequest::add_callbacks(Function<void()> on_finish, Function<void()> on_fail)
 {
     if (m_state == State::Finished) {
         if (on_finish)
@@ -113,7 +117,13 @@ void SharedImageRequest::add_callbacks(JS::SafeFunction<void()> on_finish, JS::S
         return;
     }
 
-    m_callbacks.append({ move(on_finish), move(on_fail) });
+    Callbacks callbacks;
+    if (on_finish)
+        callbacks.on_finish = JS::create_heap_function(vm().heap(), move(on_finish));
+    if (on_fail)
+        callbacks.on_fail = JS::create_heap_function(vm().heap(), move(on_fail));
+
+    m_callbacks.append(move(callbacks));
 }
 
 void SharedImageRequest::handle_successful_fetch(AK::URL const& url_string, StringView mime_type, ByteBuffer data)
@@ -129,7 +139,7 @@ void SharedImageRequest::handle_successful_fetch(AK::URL const& url_string, Stri
         m_state = State::Failed;
         for (auto& callback : m_callbacks) {
             if (callback.on_fail)
-                callback.on_fail();
+                callback.on_fail->function()();
         }
     };
 
@@ -160,7 +170,7 @@ void SharedImageRequest::handle_successful_fetch(AK::URL const& url_string, Stri
 
     for (auto& callback : m_callbacks) {
         if (callback.on_finish)
-            callback.on_finish();
+            callback.on_finish->function()();
     }
     m_callbacks.clear();
 }
@@ -170,7 +180,7 @@ void SharedImageRequest::handle_failed_fetch()
     m_state = State::Failed;
     for (auto& callback : m_callbacks) {
         if (callback.on_fail)
-            callback.on_fail();
+            callback.on_fail->function()();
     }
     m_callbacks.clear();
 }
diff --git a/Userland/Libraries/LibWeb/HTML/SharedImageRequest.h b/Userland/Libraries/LibWeb/HTML/SharedImageRequest.h
index e788ef534e..8b2780fddd 100644
--- a/Userland/Libraries/LibWeb/HTML/SharedImageRequest.h
+++ b/Userland/Libraries/LibWeb/HTML/SharedImageRequest.h
@@ -11,6 +11,7 @@
 #include <AK/URL.h>
 #include <LibGfx/Size.h>
 #include <LibJS/Heap/Handle.h>
+#include <LibJS/Heap/HeapFunction.h>
 #include <LibJS/SafeFunction.h>
 #include <LibWeb/Forward.h>
 
@@ -33,7 +34,7 @@ public:
 
     void fetch_image(JS::Realm&, JS::NonnullGCPtr<Fetch::Infrastructure::Request>);
 
-    void add_callbacks(JS::SafeFunction<void()> on_finish, JS::SafeFunction<void()> on_fail);
+    void add_callbacks(Function<void()> on_finish, Function<void()> on_fail);
 
     bool is_fetching() const;
     bool needs_fetching() const;
@@ -58,8 +59,8 @@ private:
     Page& m_page;
 
     struct Callbacks {
-        JS::SafeFunction<void()> on_finish;
-        JS::SafeFunction<void()> on_fail;
+        JS::GCPtr<JS::HeapFunction<void()>> on_finish;
+        JS::GCPtr<JS::HeapFunction<void()>> on_fail;
     };
     Vector<Callbacks> m_callbacks;