LibWeb: Store Promise::wait_for_all state in a JS-heap allocated object

The one current caller of this function always defers microtask
checkpoints before calling wait_for_all, ensuring that the promise
accept/reject handlers will always be called later in the Web event loop
processing. We need to store all the state for the closures in a heap
allocated object with HeapFunctions to keep it around while there are
still promises to resolve.

Userland/Libraries/LibWeb/HTML/Navigation.cpp

@@ -22,6 +22,7 @@
 #include <LibWeb/HTML/NavigationHistoryEntry.h>
 #include <LibWeb/HTML/NavigationTransition.h>
 #include <LibWeb/HTML/Scripting/ExceptionReporter.h>
+#include <LibWeb/HTML/Scripting/TemporaryExecutionContext.h>
 #include <LibWeb/HTML/TraversableNavigable.h>
 #include <LibWeb/HTML/Window.h>
 #include <LibWeb/WebIDL/AbstractOperations.h>
@@ -1068,7 +1069,7 @@ bool Navigation::inner_navigate_event_firing_algorithm(
 
     // 31. Prepare to run script given navigation's relevant settings object.
     // NOTE: There's a massive spec note here
-    relevant_settings_object(*this).prepare_to_run_script();
+    TemporaryExecutionContext execution_context { relevant_settings_object(*this) };
 
     // 32. If event's interception state is not "none":
     if (event->interception_state() != NavigateEvent::InterceptionState::None) {
@@ -1117,11 +1118,6 @@ bool Navigation::inner_navigate_event_firing_algorithm(
         for (auto const& handler : event->navigation_handler_list()) {
             // 1. Append the result of invoking handler with an empty arguments list to promisesList.
             auto result = WebIDL::invoke_callback(handler, {});
-            if (result.is_abrupt()) {
-                // FIXME: https://github.com/whatwg/html/issues/9774
-                report_exception(result.release_error(), realm);
-                continue;
-            }
             // This *should* be equivalent to converting a promise to a promise capability
             promises_list.append(WebIDL::create_resolved_promise(realm, result.value().value()));
         }
@@ -1139,9 +1135,12 @@ bool Navigation::inner_navigate_event_firing_algorithm(
 
         // 4. Wait for all of promisesList, with the following success steps:
         WebIDL::wait_for_all(
-            realm, promises_list, [&](JS::MarkedVector<JS::Value> const&) -> void {
+            realm, promises_list, [event, this, api_method_tracker](auto const&) -> void {
+
                 // FIXME: Spec issue: Event's relevant global objects' *associated document*
                 // 1. If event's relevant global object is not fully active, then abort these steps.
+                auto& relevant_global_object = verify_cast<HTML::Window>(HTML::relevant_global_object(*event));
+                auto& realm = event->realm();
                 if (!relevant_global_object.associated_document().is_fully_active())
                     return;
 
@@ -1170,12 +1169,14 @@ bool Navigation::inner_navigate_event_firing_algorithm(
                 m_transition = nullptr;
 
                 // 9. If apiMethodTracker is non-null, then resolve the finished promise for apiMethodTracker.
-                if (api_method_tracker)
+                if (api_method_tracker != nullptr)
                     resolve_the_finished_promise(*api_method_tracker); },
             // and the following failure steps given reason rejectionReason:
-            [&](JS::Value rejection_reason) -> void {
+            [event, this, api_method_tracker](JS::Value rejection_reason) -> void {
                 // FIXME: Spec issue: Event's relevant global objects' *associated document*
                 // 1. If event's relevant global object is not fully active, then abort these steps.
+                auto& relevant_global_object = verify_cast<HTML::Window>(HTML::relevant_global_object(*event));
+                auto& realm = event->realm();
                 if (!relevant_global_object.associated_document().is_fully_active())
                     return;
 
@@ -1213,18 +1214,18 @@ bool Navigation::inner_navigate_event_firing_algorithm(
                 m_transition = nullptr;
 
                 // 9. If apiMethodTracker is non-null, then reject the finished promise for apiMethodTracker with rejectionReason.
-                if (api_method_tracker)
+                if (api_method_tracker != nullptr)
                     reject_the_finished_promise(*api_method_tracker, rejection_reason);
             });
     }
 
     // 34. Otherwise, if apiMethodTracker is non-null, then clean up apiMethodTracker.
-    else if (api_method_tracker) {
+    else if (api_method_tracker != nullptr) {
         clean_up(*api_method_tracker);
     }
 
     // 35. Clean up after running script given navigation's relevant settings object.
-    relevant_settings_object(*this).clean_up_after_running_script();
+    // Handled by TemporaryExecutionContext destructor from step 31
 
     // 36. If event's interception state is "none", then return true.
     // 37. Return false.

Userland/Libraries/LibWeb/WebIDL/Promise.cpp

@@ -6,6 +6,7 @@
 
 #include <AK/Function.h>
 #include <AK/TypeCasts.h>
+#include <LibJS/Heap/HeapFunction.h>
 #include <LibJS/Runtime/PromiseCapability.h>
 #include <LibJS/Runtime/PromiseConstructor.h>
 #include <LibJS/Runtime/Realm.h>
@@ -182,18 +183,46 @@ void mark_promise_as_handled(Promise const& promise)
     promise_object->set_is_handled();
 }
 
+struct WaitForAllResults : JS::Cell {
+    JS_CELL(WaitForAllResults, JS::Cell);
+
+    WaitForAllResults(JS::NonnullGCPtr<JS::HeapFunction<void(Vector<JS::Value> const&)>> s, size_t t)
+        : success_steps(s)
+        , total(t)
+    {
+        // 8. Let result be a list containing total null values.
+        result.ensure_capacity(total);
+        for (size_t i = 0; i < total; ++i)
+            result.unchecked_append(JS::js_null());
+    }
+
+    virtual void visit_edges(JS::Cell::Visitor& visitor) override
+    {
+        Base::visit_edges(visitor);
+        visitor.visit(success_steps);
+        for (auto& value : result) {
+            visitor.visit(value);
+        }
+    }
+
+    JS::NonnullGCPtr<JS::HeapFunction<void(Vector<JS::Value> const&)>> success_steps;
+    Vector<JS::Value> result;
+    size_t total = 0;
+    size_t fulfilled_count = 0;
+};
+
 // https://webidl.spec.whatwg.org/#wait-for-all
-void wait_for_all(JS::Realm& realm, JS::MarkedVector<JS::NonnullGCPtr<Promise>> const& promises, JS::SafeFunction<void(JS::MarkedVector<JS::Value> const&)> success_steps, JS::SafeFunction<void(JS::Value)> failure_steps)
+void wait_for_all(JS::Realm& realm, Vector<JS::NonnullGCPtr<Promise>> const& promises, Function<void(Vector<JS::Value> const&)> success_steps, Function<void(JS::Value)> failure_steps)
 {
     // FIXME: Fix spec typo, fullfilled --> fulfilled
     // 1. Let fullfilledCount be 0.
-    size_t fulfilled_count = 0;
+    // Handled later in WaitForAllResults
 
     // 2. Let rejected be false.
     auto rejected = false;
 
     // 3. Let rejectionHandlerSteps be the following steps given arg:
-    auto rejection_handler_steps = [&rejected, failure_steps = move(failure_steps)](JS::VM& vm) -> JS::ThrowCompletionOr<JS::Value> {
+    auto rejection_handler_steps = [rejected, failure_steps = JS::create_heap_function(realm.heap(), move(failure_steps))](JS::VM& vm) mutable -> JS::ThrowCompletionOr<JS::Value> {
         // 1. If rejected is true, abort these steps.
         if (rejected)
             return JS::js_undefined();
@@ -202,7 +231,7 @@ void wait_for_all(JS::Realm& realm, JS::MarkedVector<JS::NonnullGCPtr<Promise>>
         rejected = true;
 
         // 3. Perform failureSteps given arg.
-        failure_steps(vm.argument(0));
+        failure_steps->function()(vm.argument(0));
 
         return JS::js_undefined();
     };
@@ -216,8 +245,8 @@ void wait_for_all(JS::Realm& realm, JS::MarkedVector<JS::NonnullGCPtr<Promise>>
     // 6. If total is 0, then:
     if (total == 0) {
         // 1. Queue a microtask to perform successSteps given « ».
-        HTML::queue_a_microtask(nullptr, [&realm, success_steps = move(success_steps)] {
+        HTML::queue_a_microtask(nullptr, [success_steps = JS::create_heap_function(realm.heap(), move(success_steps))] {
-            success_steps(JS::MarkedVector<JS::Value> { realm.heap() });
+            success_steps->function()({});
         });
 
         // 2. Return.
@@ -228,10 +257,9 @@ void wait_for_all(JS::Realm& realm, JS::MarkedVector<JS::NonnullGCPtr<Promise>>
     auto index = 0;
 
     // 8. Let result be a list containing total null values.
-    auto result = JS::MarkedVector<JS::Value>(realm.heap());
+    // Handled in WaitForAllResults
-    result.ensure_capacity(total);
+
-    for (size_t i = 0; i < total; ++i)
+    auto results = realm.heap().allocate<WaitForAllResults>(realm, JS::create_heap_function(realm.heap(), move(success_steps)), total);
-        result.unchecked_append(JS::js_null());
 
     // 9. For each promise of promises:
     for (auto const& promise : promises) {
@@ -240,18 +268,18 @@ void wait_for_all(JS::Realm& realm, JS::MarkedVector<JS::NonnullGCPtr<Promise>>
 
         // FIXME: This should be fulfillmentHandlerSteps
         // 2. Let fulfillmentHandler be the following steps given arg:
-        auto fulfillment_handler_steps = [&](JS::VM& vm) -> JS::ThrowCompletionOr<JS::Value> {
+        auto fulfillment_handler_steps = [results, promise_index](JS::VM& vm) -> JS::ThrowCompletionOr<JS::Value> {
             auto arg = vm.argument(0);
 
             // 1. Set result[promiseIndex] to arg.
-            result[promise_index] = arg;
+            results->result[promise_index] = arg;
 
             // 2. Set fullfilledCount to fullfilledCount + 1.
-            ++fulfilled_count;
+            ++results->fulfilled_count;
 
             // 3. If fullfilledCount equals total, then perform successSteps given result.
-            if (fulfilled_count == total)
+            if (results->fulfilled_count == results->total)
-                success_steps(result);
+                results->success_steps->function()(results->result);
 
             return JS::js_undefined();
         };

Userland/Libraries/LibWeb/WebIDL/Promise.h

@@ -29,6 +29,6 @@ JS::NonnullGCPtr<JS::Promise> react_to_promise(Promise const&, Optional<Reaction
 JS::NonnullGCPtr<JS::Promise> upon_fulfillment(Promise const&, ReactionSteps);
 JS::NonnullGCPtr<JS::Promise> upon_rejection(Promise const&, ReactionSteps);
 void mark_promise_as_handled(Promise const&);
-void wait_for_all(JS::Realm&, JS::MarkedVector<JS::NonnullGCPtr<Promise>> const& promises, JS::SafeFunction<void(JS::MarkedVector<JS::Value> const&)> success_steps, JS::SafeFunction<void(JS::Value)> failure_steps);
+void wait_for_all(JS::Realm&, Vector<JS::NonnullGCPtr<Promise>> const& promises, Function<void(Vector<JS::Value> const&)> success_steps, Function<void(JS::Value)> failure_steps);
 
 }