LibWeb: Properly reject abrupt completion in clean_up_on_return

7 files changed, 39 insertions, 23 deletions

diff --git a/Userland/Libraries/LibWeb/Streams/AbstractOperations.cpp b/Userland/Libraries/LibWeb/Streams/AbstractOperations.cpp
index a758a8257d..20a39cc4e0 100644
--- a/Userland/Libraries/LibWeb/Streams/AbstractOperations.cpp
+++ b/Userland/Libraries/LibWeb/Streams/AbstractOperations.cpp
@@ -2077,7 +2077,7 @@ bool is_close_sentinel(JS::Value value)
 
 // Non-standard function to aid in converting a user-provided function into a WebIDL::Callback. This is essentially
 // what the Bindings generator would do at compile time, but at runtime instead.
-JS::ThrowCompletionOr<JS::Handle<WebIDL::CallbackType>> property_to_callback(JS::VM& vm, JS::Value value, JS::PropertyKey const& property_key)
+JS::ThrowCompletionOr<JS::Handle<WebIDL::CallbackType>> property_to_callback(JS::VM& vm, JS::Value value, JS::PropertyKey const& property_key, WebIDL::OperationReturnsPromise operation_returns_promise)
 {
     auto property = TRY(value.get(vm, property_key));
 
@@ -2087,7 +2087,7 @@ JS::ThrowCompletionOr<JS::Handle<WebIDL::CallbackType>> property_to_callback(JS:
     if (!property.is_function())
         return vm.throw_completion<JS::TypeError>(JS::ErrorType::NotAFunction, TRY_OR_THROW_OOM(vm, property.to_string_without_side_effects()));
 
-    return vm.heap().allocate_without_realm<WebIDL::CallbackType>(property.as_object(), HTML::incumbent_settings_object());
+    return vm.heap().allocate_without_realm<WebIDL::CallbackType>(property.as_object(), HTML::incumbent_settings_object(), operation_returns_promise);
 }
 
 }
diff --git a/Userland/Libraries/LibWeb/Streams/AbstractOperations.h b/Userland/Libraries/LibWeb/Streams/AbstractOperations.h
index b46a2c5187..770c53a0db 100644
--- a/Userland/Libraries/LibWeb/Streams/AbstractOperations.h
+++ b/Userland/Libraries/LibWeb/Streams/AbstractOperations.h
@@ -9,6 +9,7 @@
 
 #include <LibJS/Heap/GCPtr.h>
 #include <LibWeb/Forward.h>
+#include <LibWeb/WebIDL/CallbackType.h>
 #include <LibWeb/WebIDL/ExceptionOr.h>
 #include <LibWeb/WebIDL/Promise.h>
 
@@ -107,7 +108,7 @@ bool is_non_negative_number(JS::Value);
 
 JS::Value create_close_sentinel();
 bool is_close_sentinel(JS::Value);
-JS::ThrowCompletionOr<JS::Handle<WebIDL::CallbackType>> property_to_callback(JS::VM& vm, JS::Value value, JS::PropertyKey const& property_key);
+JS::ThrowCompletionOr<JS::Handle<WebIDL::CallbackType>> property_to_callback(JS::VM& vm, JS::Value value, JS::PropertyKey const& property_key, WebIDL::OperationReturnsPromise);
 
 // https://streams.spec.whatwg.org/#value-with-size
 struct ValueWithSize {
diff --git a/Userland/Libraries/LibWeb/Streams/UnderlyingSink.cpp b/Userland/Libraries/LibWeb/Streams/UnderlyingSink.cpp
index ab12c8c55b..553b7a9861 100644
--- a/Userland/Libraries/LibWeb/Streams/UnderlyingSink.cpp
+++ b/Userland/Libraries/LibWeb/Streams/UnderlyingSink.cpp
@@ -19,10 +19,10 @@ JS::ThrowCompletionOr<UnderlyingSink> UnderlyingSink::from_value(JS::VM& vm, JS:
     auto& object = value.as_object();
 
     UnderlyingSink underlying_sink {
-        .start = TRY(property_to_callback(vm, value, "start")),
-        .write = TRY(property_to_callback(vm, value, "write")),
-        .close = TRY(property_to_callback(vm, value, "close")),
-        .abort = TRY(property_to_callback(vm, value, "abort")),
+        .start = TRY(property_to_callback(vm, value, "start", WebIDL::OperationReturnsPromise::No)),
+        .write = TRY(property_to_callback(vm, value, "write", WebIDL::OperationReturnsPromise::Yes)),
+        .close = TRY(property_to_callback(vm, value, "close", WebIDL::OperationReturnsPromise::Yes)),
+        .abort = TRY(property_to_callback(vm, value, "abort", WebIDL::OperationReturnsPromise::Yes)),
         .type = {},
     };
 
diff --git a/Userland/Libraries/LibWeb/Streams/UnderlyingSource.cpp b/Userland/Libraries/LibWeb/Streams/UnderlyingSource.cpp
index 6beb2504de..8062be06c0 100644
--- a/Userland/Libraries/LibWeb/Streams/UnderlyingSource.cpp
+++ b/Userland/Libraries/LibWeb/Streams/UnderlyingSource.cpp
@@ -19,9 +19,9 @@ JS::ThrowCompletionOr<UnderlyingSource> UnderlyingSource::from_value(JS::VM& vm,
     auto& object = value.as_object();
 
     UnderlyingSource underlying_source {
-        .start = TRY(property_to_callback(vm, value, "start")),
-        .pull = TRY(property_to_callback(vm, value, "pull")),
-        .cancel = TRY(property_to_callback(vm, value, "cancel")),
+        .start = TRY(property_to_callback(vm, value, "start", WebIDL::OperationReturnsPromise::No)),
+        .pull = TRY(property_to_callback(vm, value, "pull", WebIDL::OperationReturnsPromise::Yes)),
+        .cancel = TRY(property_to_callback(vm, value, "cancel", WebIDL::OperationReturnsPromise::Yes)),
         .type = {},
         .auto_allocate_chunk_size = {},
     };
diff --git a/Userland/Libraries/LibWeb/WebIDL/AbstractOperations.cpp b/Userland/Libraries/LibWeb/WebIDL/AbstractOperations.cpp
index 1ccb0c5da9..e6da0b8df0 100644
--- a/Userland/Libraries/LibWeb/WebIDL/AbstractOperations.cpp
+++ b/Userland/Libraries/LibWeb/WebIDL/AbstractOperations.cpp
@@ -13,6 +13,7 @@
 #include <LibJS/Runtime/PropertyKey.h>
 #include <LibJS/Runtime/TypedArray.h>
 #include <LibWeb/WebIDL/AbstractOperations.h>
+#include <LibWeb/WebIDL/Promise.h>
 
 namespace Web::WebIDL {
 
@@ -82,8 +83,10 @@ ErrorOr<ByteBuffer> get_buffer_source_copy(JS::Object const& buffer_source)
 }
 
 // https://webidl.spec.whatwg.org/#call-user-object-operation-return
-inline JS::Completion clean_up_on_return(HTML::EnvironmentSettingsObject& stored_settings, HTML::EnvironmentSettingsObject& relevant_settings, JS::Completion& completion)
+inline JS::Completion clean_up_on_return(HTML::EnvironmentSettingsObject& stored_settings, HTML::EnvironmentSettingsObject& relevant_settings, JS::Completion& completion, OperationReturnsPromise operation_returns_promise)
 {
+    auto& realm = stored_settings.realm();
+
     // Return: at this point completion will be set to an ECMAScript completion value.
 
     // 1. Clean up after running a callback with stored settings.
@@ -97,12 +100,15 @@ inline JS::Completion clean_up_on_return(HTML::EnvironmentSettingsObject& stored
         return completion;
 
     // 4. If completion is an abrupt completion and the operation has a return type that is not a promise type, return completion.
-    // FIXME: This does not handle promises and thus always returns completion at this point.
-    return completion;
+    if (completion.is_abrupt() && operation_returns_promise == OperationReturnsPromise::No)
+        return completion;
 
-    // FIXME: 5. Let rejectedPromise be ! Call(%Promise.reject%, %Promise%, «completion.[[Value]]»).
+    // 5. Let rejectedPromise be ! Call(%Promise.reject%, %Promise%, «completion.[[Value]]»).
+    auto rejected_promise = create_rejected_promise(realm, *completion.release_value());
 
-    // FIXME: 6. Return the result of converting rejectedPromise to the operation’s return type.
+    // 6. Return the result of converting rejectedPromise to the operation’s return type.
+    // Note: The operation must return a promise, so no conversion is necessary
+    return JS::Value { rejected_promise->promise() };
 }
 
 JS::Completion call_user_object_operation(WebIDL::CallbackType& callback, DeprecatedString const& operation_name, Optional<JS::Value> this_argument, JS::MarkedVector<JS::Value> args)
@@ -143,13 +149,13 @@ JS::Completion call_user_object_operation(WebIDL::CallbackType& callback, Deprec
         // 2. If getResult is an abrupt completion, set completion to getResult and jump to the step labeled return.
         if (get_result.is_throw_completion()) {
             completion = get_result.throw_completion();
-            return clean_up_on_return(stored_settings, relevant_settings, completion);
+            return clean_up_on_return(stored_settings, relevant_settings, completion, callback.operation_returns_promise);
         }
 
         // 4. If ! IsCallable(X) is false, then set completion to a new Completion{[[Type]]: throw, [[Value]]: a newly created TypeError object, [[Target]]: empty}, and jump to the step labeled return.
         if (!get_result.value().is_function()) {
             completion = realm.vm().template throw_completion<JS::TypeError>(JS::ErrorType::NotAFunction, TRY_OR_THROW_OOM(realm.vm(), get_result.value().to_string_without_side_effects()));
-            return clean_up_on_return(stored_settings, relevant_settings, completion);
+            return clean_up_on_return(stored_settings, relevant_settings, completion, callback.operation_returns_promise);
         }
 
         // 3. Set X to getResult.[[Value]].
@@ -171,14 +177,14 @@ JS::Completion call_user_object_operation(WebIDL::CallbackType& callback, Deprec
     // 13. If callResult is an abrupt completion, set completion to callResult and jump to the step labeled return.
     if (call_result.is_throw_completion()) {
         completion = call_result.throw_completion();
-        return clean_up_on_return(stored_settings, relevant_settings, completion);
+        return clean_up_on_return(stored_settings, relevant_settings, completion, callback.operation_returns_promise);
     }
 
     // 14. Set completion to the result of converting callResult.[[Value]] to an IDL value of the same type as the operation’s return type.
     // FIXME: This does no conversion.
     completion = call_result.value();
 
-    return clean_up_on_return(stored_settings, relevant_settings, completion);
+    return clean_up_on_return(stored_settings, relevant_settings, completion, callback.operation_returns_promise);
 }
 
 // https://webidl.spec.whatwg.org/#invoke-a-callback-function
@@ -229,14 +235,14 @@ JS::Completion invoke_callback(WebIDL::CallbackType& callback, Optional<JS::Valu
     // 12. If callResult is an abrupt completion, set completion to callResult and jump to the step labeled return.
     if (call_result.is_throw_completion()) {
         completion = call_result.throw_completion();
-        return clean_up_on_return(stored_settings, relevant_settings, completion);
+        return clean_up_on_return(stored_settings, relevant_settings, completion, callback.operation_returns_promise);
     }
 
     // 13. Set completion to the result of converting callResult.[[Value]] to an IDL value of the same type as the operation’s return type.
     // FIXME: This does no conversion.
     completion = call_result.value();
 
-    return clean_up_on_return(stored_settings, relevant_settings, completion);
+    return clean_up_on_return(stored_settings, relevant_settings, completion, callback.operation_returns_promise);
 }
 
 JS::Completion construct(WebIDL::CallbackType& callback, JS::MarkedVector<JS::Value> args)
diff --git a/Userland/Libraries/LibWeb/WebIDL/CallbackType.cpp b/Userland/Libraries/LibWeb/WebIDL/CallbackType.cpp
index 0493b55993..fd69121543 100644
--- a/Userland/Libraries/LibWeb/WebIDL/CallbackType.cpp
+++ b/Userland/Libraries/LibWeb/WebIDL/CallbackType.cpp
@@ -10,9 +10,10 @@
 
 namespace Web::WebIDL {
 
-CallbackType::CallbackType(JS::Object& callback, HTML::EnvironmentSettingsObject& callback_context)
+CallbackType::CallbackType(JS::Object& callback, HTML::EnvironmentSettingsObject& callback_context, OperationReturnsPromise operation_returns_promise)
     : callback(callback)
     , callback_context(callback_context)
+    , operation_returns_promise(operation_returns_promise)
 {
 }
 
diff --git a/Userland/Libraries/LibWeb/WebIDL/CallbackType.h b/Userland/Libraries/LibWeb/WebIDL/CallbackType.h
index fa93e41680..ff1421ff09 100644
--- a/Userland/Libraries/LibWeb/WebIDL/CallbackType.h
+++ b/Userland/Libraries/LibWeb/WebIDL/CallbackType.h
@@ -12,16 +12,24 @@
 
 namespace Web::WebIDL {
 
+enum class OperationReturnsPromise {
+    Yes,
+    No,
+};
+
 // https://webidl.spec.whatwg.org/#idl-callback-interface
 class CallbackType final : public JS::Cell {
 public:
-    CallbackType(JS::Object& callback, HTML::EnvironmentSettingsObject& callback_context);
+    CallbackType(JS::Object& callback, HTML::EnvironmentSettingsObject& callback_context, OperationReturnsPromise = OperationReturnsPromise::No);
 
     JS::NonnullGCPtr<JS::Object> callback;
 
     // https://webidl.spec.whatwg.org/#dfn-callback-context
     JS::NonnullGCPtr<HTML::EnvironmentSettingsObject> callback_context;
 
+    // Non-standard property used to distinguish Promise-returning callbacks in callback-related AOs
+    OperationReturnsPromise operation_returns_promise;
+
 private:
     virtual StringView class_name() const override;
     virtual void visit_edges(Cell::Visitor&) override;