LibJS: Convert Promise::create() to NonnullGCPtr

Author: https://github.com/linusg
Commit: https://github.com/SerenityOS/serenity/commit/a4d85cd522
Pull-request: https://github.com/SerenityOS/serenity/pull/16479
Reviewed-by: https://github.com/davidot ✅

Meta/Lagom/Tools/CodeGenerators/LibWeb/BindingsGenerator/IDLGenerators.cpp

@@ -520,7 +520,7 @@ static void generate_to_cpp(SourceGenerator& generator, ParameterType& parameter
         // anything of this sort. Both Gecko and Blink do it, however, so I'm sure it's correct.
         scoped_generator.append(R"~~~(
     if (!@js_name@@js_suffix@.is_object() || !is<JS::Promise>(@js_name@@js_suffix@.as_object())) {
-        auto* new_promise = JS::Promise::create(realm);
+        auto new_promise = JS::Promise::create(realm);
         new_promise->fulfill(@js_name@@js_suffix@);
         @js_name@@js_suffix@ = new_promise;
     }

Userland/Libraries/LibJS/Module.cpp

@@ -47,7 +47,7 @@ ThrowCompletionOr<u32> Module::inner_module_evaluation(VM& vm, Vector<Module*>&,
 {
     // 1. If module is not a Cyclic Module Record, then
     // a. Let promise be ! module.Evaluate().
-    auto* promise = TRY(evaluate(vm));
+    auto promise = TRY(evaluate(vm));
 
     // b. Assert: promise.[[PromiseState]] is not pending.
     VERIFY(promise->state() != Promise::State::Pending);

Userland/Libraries/LibJS/Runtime/AsyncFunctionDriverWrapper.cpp

@@ -56,7 +56,7 @@ ThrowCompletionOr<Value> AsyncFunctionDriverWrapper::react_to_async_task_complet
         return promise;
     }
 
-    auto* promise = static_cast<Promise*>(&promise_value.as_object());
+    auto promise = static_cast<Promise*>(&promise_value.as_object());
     if (TRY(result.get(vm, vm.names.done)).to_boolean())
         return promise;
 

Userland/Libraries/LibJS/Runtime/Completion.cpp

@@ -89,7 +89,7 @@ ThrowCompletionOr<Value> await(VM& vm, Value value)
     auto on_rejected = NativeFunction::create(realm, move(rejected_closure), 1, "");
 
     // 7. Perform PerformPromiseThen(promise, onFulfilled, onRejected).
-    auto* promise = verify_cast<Promise>(promise_object);
+    auto promise = verify_cast<Promise>(promise_object);
     promise->perform_then(on_fulfilled, on_rejected, {});
 
     // FIXME: Since we don't support context suspension, we attempt to "wait" for the promise to resolve

Userland/Libraries/LibJS/Runtime/Promise.cpp

@@ -42,9 +42,9 @@ ThrowCompletionOr<Object*> promise_resolve(VM& vm, Object& constructor, Value va
     return promise_capability->promise().ptr();
 }
 
-Promise* Promise::create(Realm& realm)
+NonnullGCPtr<Promise> Promise::create(Realm& realm)
 {
-    return realm.heap().allocate<Promise>(realm, *realm.intrinsics().promise_prototype());
+    return *realm.heap().allocate<Promise>(realm, *realm.intrinsics().promise_prototype());
 }
 
 // 27.2 Promise Objects, https://tc39.es/ecma262/#sec-promise-objects

Userland/Libraries/LibJS/Runtime/Promise.h

@@ -27,7 +27,7 @@ public:
         Handle,
     };
 
-    static Promise* create(Realm&);
+    static NonnullGCPtr<Promise> create(Realm&);
 
     virtual ~Promise() = default;
 

Userland/Libraries/LibJS/Runtime/PromiseCapability.cpp

@@ -78,7 +78,7 @@ ThrowCompletionOr<NonnullGCPtr<PromiseCapability>> new_promise_capability(VM& vm
     auto executor = NativeFunction::create(realm, move(executor_closure), 2, "");
 
     // 6. Let promise be ? Construct(C, « executor »).
-    auto* promise = TRY(construct(vm, constructor.as_function(), executor));
+    auto promise = TRY(construct(vm, constructor.as_function(), executor));
 
     // 7. If IsCallable(promiseCapability.[[Resolve]]) is false, throw a TypeError exception.
     // NOTE: We only assign a value in the executor closure if it is a function.

Userland/Libraries/LibJS/Runtime/PromiseConstructor.cpp

@@ -290,7 +290,7 @@ ThrowCompletionOr<Object*> PromiseConstructor::construct(FunctionObject& new_tar
     // 5. Set promise.[[PromiseFulfillReactions]] to a new empty List.
     // 6. Set promise.[[PromiseRejectReactions]] to a new empty List.
     // 7. Set promise.[[PromiseIsHandled]] to false.
-    auto* promise = TRY(ordinary_create_from_constructor<Promise>(vm, new_target, &Intrinsics::promise_prototype));
+    auto promise = TRY(ordinary_create_from_constructor<Promise>(vm, new_target, &Intrinsics::promise_prototype));
 
     // 8. Let resolvingFunctions be CreateResolvingFunctions(promise).
     auto [resolve_function, reject_function] = promise->create_resolving_functions();

Userland/Libraries/LibJS/Runtime/PromisePrototype.cpp

@@ -45,7 +45,7 @@ JS_DEFINE_NATIVE_FUNCTION(PromisePrototype::then)
 
     // 1. Let promise be the this value.
     // 2. If IsPromise(promise) is false, throw a TypeError exception.
-    auto* promise = TRY(typed_this_object(vm));
+    auto promise = TRY(typed_this_object(vm));
 
     // 3. Let C be ? SpeciesConstructor(promise, %Promise%).
     auto* constructor = TRY(species_constructor(vm, *promise, *realm.intrinsics().promise_constructor()));
@@ -113,7 +113,7 @@ JS_DEFINE_NATIVE_FUNCTION(PromisePrototype::finally)
             auto result = TRY(call(vm, on_finally, js_undefined()));
 
             // ii. Let promise be ? PromiseResolve(C, result).
-            auto* promise = TRY(promise_resolve(vm, constructor, result));
+            auto promise = TRY(promise_resolve(vm, constructor, result));
 
             // iii. Let returnValue be a new Abstract Closure with no parameters that captures value and performs the following steps when called:
             auto return_value = [value_handle = make_handle(value)](auto&) -> ThrowCompletionOr<Value> {
@@ -142,7 +142,7 @@ JS_DEFINE_NATIVE_FUNCTION(PromisePrototype::finally)
             auto result = TRY(call(vm, on_finally, js_undefined()));
 
             // ii. Let promise be ? PromiseResolve(C, result).
-            auto* promise = TRY(promise_resolve(vm, constructor, result));
+            auto promise = TRY(promise_resolve(vm, constructor, result));
 
             // iii. Let throwReason be a new Abstract Closure with no parameters that captures reason and performs the following steps when called:
             auto throw_reason = [reason_handle = make_handle(reason)](auto&) -> ThrowCompletionOr<Value> {

Userland/Libraries/LibJS/Runtime/VM.cpp

@@ -75,7 +75,7 @@ VM::VM(OwnPtr<CustomData> custom_data)
         // By default, we throw on dynamic imports this is to prevent arbitrary file access by scripts.
         VERIFY(current_realm());
         auto& realm = *current_realm();
-        auto* promise = Promise::create(realm);
+        auto promise = Promise::create(realm);
 
         // If you are here because you want to enable dynamic module importing make sure it won't be a security problem
         // by checking the default implementation of HostImportModuleDynamically and creating your own hook or calling
@@ -965,7 +965,7 @@ void VM::import_module_dynamically(ScriptOrModule referencing_script_or_module,
     //    FinishDynamicImport(referencingScriptOrModule, moduleRequest, promiseCapability, promise),
     //    where promise is a Promise rejected with an error representing the cause of failure.
 
-    auto* promise = Promise::create(realm);
+    auto promise = Promise::create(realm);
 
     ScopeGuard finish_dynamic_import = [&] {
         host_finish_dynamic_import(referencing_script_or_module, module_request, promise_capability, promise);

Userland/Libraries/LibJS/SyntheticModule.cpp

@@ -107,7 +107,7 @@ ThrowCompletionOr<Promise*> SyntheticModule::evaluate(VM& vm)
 
     // 12. Return Completion(result).
     // Note: Because we expect it to return a promise we convert this here.
-    auto* promise = Promise::create(realm());
+    auto promise = Promise::create(realm());
     if (result.is_error()) {
         VERIFY(result.throw_completion().value().has_value());
         promise->reject(*result.throw_completion().value());
@@ -115,7 +115,7 @@ ThrowCompletionOr<Promise*> SyntheticModule::evaluate(VM& vm)
         // Note: This value probably isn't visible to JS code? But undefined is fine anyway.
         promise->fulfill(js_undefined());
     }
-    return promise;
+    return promise.ptr();
 }
 
 // 1.2.2 SetSyntheticModuleExport ( module, exportName, exportValue ), https://tc39.es/proposal-json-modules/#sec-setsyntheticmoduleexport

Userland/Libraries/LibWeb/Crypto/SubtleCrypto.cpp

@@ -38,7 +38,7 @@ JS::Promise* SubtleCrypto::digest(DeprecatedString const& algorithm, JS::Handle<
     auto data_buffer_or_error = WebIDL::get_buffer_source_copy(*data.cell());
     if (data_buffer_or_error.is_error()) {
         auto error = WebIDL::OperationError::create(realm, "Failed to copy bytes from ArrayBuffer");
-        auto* promise = JS::Promise::create(realm);
+        auto promise = JS::Promise::create(realm);
         promise->reject(error.ptr());
         return promise;
     }
@@ -59,13 +59,13 @@ JS::Promise* SubtleCrypto::digest(DeprecatedString const& algorithm, JS::Handle<
     // 4. If an error occurred, return a Promise rejected with normalizedAlgorithm.
     else {
         auto error = WebIDL::NotSupportedError::create(realm, DeprecatedString::formatted("Invalid hash function '{}'", algorithm));
-        auto* promise = JS::Promise::create(realm);
+        auto promise = JS::Promise::create(realm);
         promise->reject(error.ptr());
         return promise;
     }
 
     // 5. Let promise be a new Promise.
-    auto* promise = JS::Promise::create(realm);
+    auto promise = JS::Promise::create(realm);
 
     // 6. Return promise and perform the remaining steps in parallel.
     // FIXME: We don't have a good abstraction for this yet, so we do it in sync.

Userland/Libraries/LibWeb/FileAPI/Blob.cpp

@@ -244,7 +244,7 @@ JS::Promise* Blob::text()
 
     // FIXME: We still need to implement ReadableStream for this step to be fully valid.
     // 3. Let promise be the result of reading all bytes from stream with reader
-    auto* promise = JS::Promise::create(realm());
+    auto promise = JS::Promise::create(realm());
     auto result = JS::PrimitiveString::create(vm(), DeprecatedString { m_byte_buffer.bytes() });
 
     // 4. Return the result of transforming promise by a fulfillment handler that returns the result of running UTF-8 decode on its first argument.
@@ -260,7 +260,7 @@ JS::Promise* Blob::array_buffer()
 
     // FIXME: We still need to implement ReadableStream for this step to be fully valid.
     // 3. Let promise be the result of reading all bytes from stream with reader.
-    auto* promise = JS::Promise::create(realm());
+    auto promise = JS::Promise::create(realm());
     auto buffer_result = JS::ArrayBuffer::create(realm(), m_byte_buffer.size());
     if (buffer_result.is_error()) {
         promise->reject(buffer_result.release_error().value().release_value());

Userland/Libraries/LibWeb/HTML/Scripting/ModuleScript.cpp

@@ -112,7 +112,7 @@ JS::Promise* JavaScriptModuleScript::run(PreventErrorReporting)
 
     // 2. Check if we can run script with settings. If this returns "do not run", then return a promise resolved with undefined.
     if (settings.can_run_script() == RunScriptDecision::DoNotRun) {
-        auto* promise = JS::Promise::create(settings.realm());
+        auto promise = JS::Promise::create(settings.realm());
         promise->fulfill(JS::js_undefined());
         return promise;
     }
@@ -140,7 +140,7 @@ JS::Promise* JavaScriptModuleScript::run(PreventErrorReporting)
         // If Evaluate fails to complete as a result of the user agent aborting the running script,
         // then set evaluationPromise to a promise rejected with a new "QuotaExceededError" DOMException.
         if (elevation_promise_or_error.is_error()) {
-            auto* promise = JS::Promise::create(settings_object().realm());
+            auto promise = JS::Promise::create(settings_object().realm());
             promise->reject(WebIDL::QuotaExceededError::create(settings_object().realm(), "Failed to evaluate module script").ptr());
 
             evaluation_promise = promise;

Userland/Libraries/LibWeb/WebDriver/ExecuteScript.cpp

@@ -315,7 +315,7 @@ ExecuteScriptResultSerialized execute_async_script(Web::Page& page, DeprecatedSt
     auto start = Time::now_monotonic();
 
     // 4. Let promise be a new Promise.
-    auto* promise = JS::Promise::create(realm);
+    auto promise = JS::Promise::create(realm);
 
     // FIXME: 5 Run the following substeps in parallel:
     auto result = [&] {

Userland/Libraries/LibWeb/WebIDL/Promise.cpp

@@ -135,7 +135,7 @@ JS::NonnullGCPtr<JS::Promise> react_to_promise(JS::PromiseCapability const& prom
     auto new_capability = MUST(JS::new_promise_capability(vm, constructor));
 
     // 7. Return PerformPromiseThen(promise.[[Promise]], onFulfilled, onRejected, newCapability).
-    auto* promise = verify_cast<JS::Promise>(promise_capability.promise().ptr());
+    auto promise = verify_cast<JS::Promise>(promise_capability.promise().ptr());
     auto value = promise->perform_then(on_fulfilled, on_rejected, new_capability);
     return verify_cast<JS::Promise>(value.as_object());
 }
@@ -173,7 +173,7 @@ JS::NonnullGCPtr<JS::Promise> upon_rejection(JS::PromiseCapability const& promis
 void mark_promise_as_handled(JS::PromiseCapability const& promise_capability)
 {
     // To mark as handled a Promise<T> promise, set promise.[[Promise]].[[PromiseIsHandled]] to true.
-    auto* promise = verify_cast<JS::Promise>(promise_capability.promise().ptr());
+    auto promise = verify_cast<JS::Promise>(promise_capability.promise().ptr());
     promise->set_is_handled();
 }