Browse Source

LibWeb/Fetch: Use `JS::HeapFunction` for callback in `FetchAlgorithms`

In FetchAlgorithms, it is common for callbacks to capture realms. This
can indirectly keep objects alive that hold FetchController with these
callbacks. This creates a cyclic dependency. However, when
JS::HeapFunction is used, this is not a problem, as captured by
callbacks values do not create new roots.
Aliaksandr Kalenik 1 year ago
parent
commit
9a07ac0b6a

+ 8 - 8
Userland/Libraries/LibWeb/Fetch/Fetching/Fetching.cpp

@@ -65,7 +65,7 @@ WebIDL::ExceptionOr<JS::NonnullGCPtr<Infrastructure::FetchController>> fetch(JS:
     auto& vm = realm.vm();
 
     // 1. Assert: request’s mode is "navigate" or processEarlyHintsResponse is null.
-    VERIFY(request.mode() == Infrastructure::Request::Mode::Navigate || !algorithms.process_early_hints_response().has_value());
+    VERIFY(request.mode() == Infrastructure::Request::Mode::Navigate || !algorithms.process_early_hints_response());
 
     // 2. Let taskDestination be null.
     JS::GCPtr<JS::Object> task_destination;
@@ -608,8 +608,8 @@ WebIDL::ExceptionOr<void> fetch_response_handover(JS::Realm& realm, Infrastructu
 
             // 2. If fetchParams’s process response end-of-body is non-null, then run fetchParams’s process response
             //    end-of-body given response.
-            if (fetch_params.algorithms()->process_response_end_of_body().has_value())
-                (*fetch_params.algorithms()->process_response_end_of_body())(response);
+            if (fetch_params.algorithms()->process_response_end_of_body())
+                (fetch_params.algorithms()->process_response_end_of_body())(response);
 
             // 3. If fetchParams’s request’s initiator type is non-null and fetchParams’s request’s client’s global
             //    object is fetchParams’s task destination, then run fetchParams’s controller’s report timing steps
@@ -634,9 +634,9 @@ WebIDL::ExceptionOr<void> fetch_response_handover(JS::Realm& realm, Infrastructu
 
     // 4. If fetchParams’s process response is non-null, then queue a fetch task to run fetchParams’s process response
     //    given response, with fetchParams’s task destination.
-    if (fetch_params.algorithms()->process_response().has_value()) {
+    if (fetch_params.algorithms()->process_response()) {
         Infrastructure::queue_fetch_task(task_destination, [&fetch_params, &response]() {
-            (*fetch_params.algorithms()->process_response())(response);
+            fetch_params.algorithms()->process_response()(response);
         });
     }
 
@@ -657,17 +657,17 @@ WebIDL::ExceptionOr<void> fetch_response_handover(JS::Realm& realm, Infrastructu
     }
 
     // 8. If fetchParams’s process response consume body is non-null, then:
-    if (fetch_params.algorithms()->process_response_consume_body().has_value()) {
+    if (fetch_params.algorithms()->process_response_consume_body()) {
         // 1. Let processBody given nullOrBytes be this step: run fetchParams’s process response consume body given
         //    response and nullOrBytes.
         auto process_body = [&fetch_params, &response](Variant<ByteBuffer, Empty> const& null_or_bytes) {
-            (*fetch_params.algorithms()->process_response_consume_body())(response, null_or_bytes);
+            (fetch_params.algorithms()->process_response_consume_body())(response, null_or_bytes);
         };
 
         // 2. Let processBodyError be this step: run fetchParams’s process response consume body given response and
         //    failure.
         auto process_body_error = [&fetch_params, &response](auto) {
-            (*fetch_params.algorithms()->process_response_consume_body())(response, Infrastructure::FetchAlgorithms::ConsumeBodyFailureTag {});
+            (fetch_params.algorithms()->process_response_consume_body())(response, Infrastructure::FetchAlgorithms::ConsumeBodyFailureTag {});
         };
 
         // 3. If internalResponse's body is null, then queue a fetch task to run processBody given null, with

+ 18 - 8
Userland/Libraries/LibWeb/Fetch/Infrastructure/FetchAlgorithms.cpp

@@ -12,17 +12,27 @@ namespace Web::Fetch::Infrastructure {
 
 JS::NonnullGCPtr<FetchAlgorithms> FetchAlgorithms::create(JS::VM& vm, Input input)
 {
-    return vm.heap().allocate_without_realm<FetchAlgorithms>(move(input));
+    return vm.heap().allocate_without_realm<FetchAlgorithms>(vm, move(input));
 }
 
-FetchAlgorithms::FetchAlgorithms(Input input)
-    : m_process_request_body_chunk_length(move(input.process_request_body_chunk_length))
-    , m_process_request_end_of_body(move(input.process_request_end_of_body))
-    , m_process_early_hints_response(move(input.process_early_hints_response))
-    , m_process_response(move(input.process_response))
-    , m_process_response_end_of_body(move(input.process_response_end_of_body))
-    , m_process_response_consume_body(move(input.process_response_consume_body))
+FetchAlgorithms::FetchAlgorithms(JS::VM& vm, Input input)
+    : m_process_request_body_chunk_length(JS::create_heap_function(vm.heap(), move(input.process_request_body_chunk_length)))
+    , m_process_request_end_of_body(JS::create_heap_function(vm.heap(), move(input.process_request_end_of_body)))
+    , m_process_early_hints_response(JS::create_heap_function(vm.heap(), move(input.process_early_hints_response)))
+    , m_process_response(JS::create_heap_function(vm.heap(), move(input.process_response)))
+    , m_process_response_end_of_body(JS::create_heap_function(vm.heap(), move(input.process_response_end_of_body)))
+    , m_process_response_consume_body(JS::create_heap_function(vm.heap(), move(input.process_response_consume_body)))
 {
 }
 
+void FetchAlgorithms::visit_edges(JS::Cell::Visitor& visitor)
+{
+    visitor.visit(m_process_request_body_chunk_length);
+    visitor.visit(m_process_request_end_of_body);
+    visitor.visit(m_process_early_hints_response);
+    visitor.visit(m_process_response);
+    visitor.visit(m_process_response_end_of_body);
+    visitor.visit(m_process_response_consume_body);
+}
+
 }

+ 29 - 26
Userland/Libraries/LibWeb/Fetch/Infrastructure/FetchAlgorithms.h

@@ -9,6 +9,7 @@
 #include <AK/Optional.h>
 #include <LibJS/Heap/Cell.h>
 #include <LibJS/Heap/GCPtr.h>
+#include <LibJS/Heap/HeapFunction.h>
 #include <LibJS/SafeFunction.h>
 #include <LibWeb/Forward.h>
 
@@ -22,40 +23,42 @@ public:
     struct ConsumeBodyFailureTag { };
     using BodyBytes = Variant<Empty, ConsumeBodyFailureTag, ByteBuffer>;
 
-    using ProcessRequestBodyChunkLengthFunction = JS::SafeFunction<void(u64)>;
-    using ProcessRequestEndOfBodyFunction = JS::SafeFunction<void()>;
-    using ProcessEarlyHintsResponseFunction = JS::SafeFunction<void(JS::NonnullGCPtr<Infrastructure::Response>)>;
-    using ProcessResponseFunction = JS::SafeFunction<void(JS::NonnullGCPtr<Infrastructure::Response>)>;
-    using ProcessResponseEndOfBodyFunction = JS::SafeFunction<void(JS::NonnullGCPtr<Infrastructure::Response>)>;
-    using ProcessResponseConsumeBodyFunction = JS::SafeFunction<void(JS::NonnullGCPtr<Infrastructure::Response>, BodyBytes)>;
+    using ProcessRequestBodyChunkLengthFunction = Function<void(u64)>;
+    using ProcessRequestEndOfBodyFunction = Function<void()>;
+    using ProcessEarlyHintsResponseFunction = Function<void(JS::NonnullGCPtr<Infrastructure::Response>)>;
+    using ProcessResponseFunction = Function<void(JS::NonnullGCPtr<Infrastructure::Response>)>;
+    using ProcessResponseEndOfBodyFunction = Function<void(JS::NonnullGCPtr<Infrastructure::Response>)>;
+    using ProcessResponseConsumeBodyFunction = Function<void(JS::NonnullGCPtr<Infrastructure::Response>, BodyBytes)>;
 
     struct Input {
-        Optional<ProcessRequestBodyChunkLengthFunction> process_request_body_chunk_length;
-        Optional<ProcessRequestEndOfBodyFunction> process_request_end_of_body;
-        Optional<ProcessEarlyHintsResponseFunction> process_early_hints_response;
-        Optional<ProcessResponseFunction> process_response;
-        Optional<ProcessResponseEndOfBodyFunction> process_response_end_of_body;
-        Optional<ProcessResponseConsumeBodyFunction> process_response_consume_body;
+        ProcessRequestBodyChunkLengthFunction process_request_body_chunk_length;
+        ProcessRequestEndOfBodyFunction process_request_end_of_body;
+        ProcessEarlyHintsResponseFunction process_early_hints_response;
+        ProcessResponseFunction process_response;
+        ProcessResponseEndOfBodyFunction process_response_end_of_body;
+        ProcessResponseConsumeBodyFunction process_response_consume_body;
     };
 
     [[nodiscard]] static JS::NonnullGCPtr<FetchAlgorithms> create(JS::VM&, Input);
 
-    Optional<ProcessRequestBodyChunkLengthFunction> const& process_request_body_chunk_length() const { return m_process_request_body_chunk_length; }
-    Optional<ProcessRequestEndOfBodyFunction> const& process_request_end_of_body() const { return m_process_request_end_of_body; }
-    Optional<ProcessEarlyHintsResponseFunction> const& process_early_hints_response() const { return m_process_early_hints_response; }
-    Optional<ProcessResponseFunction> const& process_response() const { return m_process_response; }
-    Optional<ProcessResponseEndOfBodyFunction> const& process_response_end_of_body() const { return m_process_response_end_of_body; }
-    Optional<ProcessResponseConsumeBodyFunction> const& process_response_consume_body() const { return m_process_response_consume_body; }
+    ProcessRequestBodyChunkLengthFunction const& process_request_body_chunk_length() const { return m_process_request_body_chunk_length->function(); }
+    ProcessRequestEndOfBodyFunction const& process_request_end_of_body() const { return m_process_request_end_of_body->function(); }
+    ProcessEarlyHintsResponseFunction const& process_early_hints_response() const { return m_process_early_hints_response->function(); }
+    ProcessResponseFunction const& process_response() const { return m_process_response->function(); }
+    ProcessResponseEndOfBodyFunction const& process_response_end_of_body() const { return m_process_response_end_of_body->function(); }
+    ProcessResponseConsumeBodyFunction const& process_response_consume_body() const { return m_process_response_consume_body->function(); }
+
+    virtual void visit_edges(JS::Cell::Visitor&) override;
 
 private:
-    explicit FetchAlgorithms(Input);
-
-    Optional<ProcessRequestBodyChunkLengthFunction> m_process_request_body_chunk_length;
-    Optional<ProcessRequestEndOfBodyFunction> m_process_request_end_of_body;
-    Optional<ProcessEarlyHintsResponseFunction> m_process_early_hints_response;
-    Optional<ProcessResponseFunction> m_process_response;
-    Optional<ProcessResponseEndOfBodyFunction> m_process_response_end_of_body;
-    Optional<ProcessResponseConsumeBodyFunction> m_process_response_consume_body;
+    explicit FetchAlgorithms(JS::VM&, Input);
+
+    JS::NonnullGCPtr<JS::HeapFunction<void(u64)>> m_process_request_body_chunk_length;
+    JS::NonnullGCPtr<JS::HeapFunction<void()>> m_process_request_end_of_body;
+    JS::NonnullGCPtr<JS::HeapFunction<void(JS::NonnullGCPtr<Infrastructure::Response>)>> m_process_early_hints_response;
+    JS::NonnullGCPtr<JS::HeapFunction<void(JS::NonnullGCPtr<Infrastructure::Response>)>> m_process_response;
+    JS::NonnullGCPtr<JS::HeapFunction<void(JS::NonnullGCPtr<Infrastructure::Response>)>> m_process_response_end_of_body;
+    JS::NonnullGCPtr<JS::HeapFunction<void(JS::NonnullGCPtr<Infrastructure::Response>, BodyBytes)>> m_process_response_consume_body;
 };
 
 }