LibJS: Convert internal_own_property_keys() to ThrowCompletionOr

Author: https://github.com/linusg
Commit: https://github.com/SerenityOS/serenity/commit/ee8380edeab
Pull-request: https://github.com/SerenityOS/serenity/pull/10266
Reviewed-by: https://github.com/IdanHo ✅

Meta/Lagom/Tools/CodeGenerators/LibWeb/WrapperGenerator.cpp

@@ -1420,7 +1420,7 @@ public:
     virtual JS::ThrowCompletionOr<bool> internal_define_own_property(JS::PropertyName const&, JS::PropertyDescriptor const&) override;
     virtual JS::ThrowCompletionOr<bool> internal_delete(JS::PropertyName const&) override;
     virtual JS::ThrowCompletionOr<bool> internal_prevent_extensions() override;
-    virtual JS::MarkedValueList internal_own_property_keys() const override;
+    virtual JS::ThrowCompletionOr<JS::MarkedValueList> internal_own_property_keys() const override;
 )~~~");
     }
 
@@ -2301,7 +2301,7 @@ JS::ThrowCompletionOr<bool> @class_name@::internal_prevent_extensions()
 
         // 3.9.6. [[OwnPropertyKeys]], https://heycam.github.io/webidl/#legacy-platform-object-ownpropertykeys
         scoped_generator.append(R"~~~(
-JS::MarkedValueList @class_name@::internal_own_property_keys() const
+JS::ThrowCompletionOr<JS::MarkedValueList> @class_name@::internal_own_property_keys() const
 {
     auto& vm = this->vm();
 
@@ -2348,7 +2348,7 @@ JS::MarkedValueList @class_name@::internal_own_property_keys() const
     // FIXME: 6. Assert: keys has no duplicate items.
 
     // 7. Return keys.
-    return keys;
+    return { move(keys) };
 }
 )~~~");
     }

Userland/Libraries/LibJS/Bytecode/Op.cpp

@@ -192,9 +192,10 @@ void CopyObjectExcludingProperties::execute_impl(Bytecode::Interpreter& interpre
             return;
     }
 
-    auto own_keys = from_object->internal_own_property_keys();
-    if (interpreter.vm().exception())
+    auto own_keys_or_error = from_object->internal_own_property_keys();
+    if (own_keys_or_error.is_error())
         return;
+    auto own_keys = own_keys_or_error.release_value();
 
     for (auto& key : own_keys) {
         if (!excluded_names.contains(key)) {

Userland/Libraries/LibJS/Runtime/Array.cpp

@@ -226,15 +226,13 @@ ThrowCompletionOr<bool> Array::internal_delete(PropertyName const& property_name
 }
 
 // NON-STANDARD: Used to inject the ephemeral length property's key
-MarkedValueList Array::internal_own_property_keys() const
+ThrowCompletionOr<MarkedValueList> Array::internal_own_property_keys() const
 {
     auto& vm = this->vm();
-    auto keys = Object::internal_own_property_keys();
-    if (vm.exception())
-        return MarkedValueList { vm.heap() };
+    auto keys = TRY(Object::internal_own_property_keys());
     // FIXME: This is pretty expensive, find a better way to do this
     keys.insert(indexed_properties().real_size(), js_string(vm, vm.names.length.as_string()));
-    return keys;
+    return { move(keys) };
 }
 
 }

Userland/Libraries/LibJS/Runtime/Array.h

@@ -41,7 +41,7 @@ public:
     virtual ThrowCompletionOr<Optional<PropertyDescriptor>> internal_get_own_property(PropertyName const&) const override;
     virtual ThrowCompletionOr<bool> internal_define_own_property(PropertyName const&, PropertyDescriptor const&) override;
     virtual ThrowCompletionOr<bool> internal_delete(PropertyName const&) override;
-    virtual MarkedValueList internal_own_property_keys() const override;
+    virtual ThrowCompletionOr<MarkedValueList> internal_own_property_keys() const override;
 
     [[nodiscard]] bool length_is_writable() const { return m_length_writable; };
 

Userland/Libraries/LibJS/Runtime/Object.cpp

@@ -298,9 +298,7 @@ bool Object::set_integrity_level(IntegrityLevel level)
         return false;
 
     // 5. Let keys be ? O.[[OwnPropertyKeys]]().
-    auto keys = internal_own_property_keys();
-    if (vm.exception())
-        return {};
+    auto keys = TRY_OR_DISCARD(internal_own_property_keys());
 
     // 6. If level is sealed, then
     if (level == IntegrityLevel::Sealed) {
@@ -374,9 +372,7 @@ bool Object::test_integrity_level(IntegrityLevel level) const
         return false;
 
     // 6. Let keys be ? O.[[OwnPropertyKeys]]().
-    auto keys = internal_own_property_keys();
-    if (vm.exception())
-        return {};
+    auto keys = TRY_OR_DISCARD(internal_own_property_keys());
 
     // 7. For each element k of keys, do
     for (auto& key : keys) {
@@ -416,9 +412,10 @@ MarkedValueList Object::enumerable_own_property_names(PropertyKind kind) const
     // 1. Assert: Type(O) is Object.
 
     // 2. Let ownKeys be ? O.[[OwnPropertyKeys]]().
-    auto own_keys = internal_own_property_keys();
-    if (vm.exception())
+    auto own_keys_or_error = internal_own_property_keys();
+    if (own_keys_or_error.is_error())
         return MarkedValueList { heap() };
+    auto own_keys = own_keys_or_error.release_value();
 
     // 3. Let properties be a new empty List.
     auto properties = MarkedValueList { heap() };
@@ -820,7 +817,7 @@ ThrowCompletionOr<bool> Object::internal_delete(PropertyName const& property_nam
 }
 
 // 10.1.11 [[OwnPropertyKeys]] ( ), https://tc39.es/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-ownpropertykeys
-MarkedValueList Object::internal_own_property_keys() const
+ThrowCompletionOr<MarkedValueList> Object::internal_own_property_keys() const
 {
     auto& vm = this->vm();
 
@@ -850,7 +847,7 @@ MarkedValueList Object::internal_own_property_keys() const
     }
 
     // 5. Return keys.
-    return keys;
+    return { move(keys) };
 }
 
 // 10.4.7.2 SetImmutablePrototype ( O, V ), https://tc39.es/ecma262/#sec-set-immutable-prototype
@@ -1083,9 +1080,7 @@ Object* Object::define_properties(Value properties)
         return {};
 
     // 3. Let keys be ? props.[[OwnPropertyKeys]]().
-    auto keys = props->internal_own_property_keys();
-    if (vm.exception())
-        return {};
+    auto keys = TRY_OR_DISCARD(props->internal_own_property_keys());
 
     struct NameAndDescriptor {
         PropertyName name;

Userland/Libraries/LibJS/Runtime/Object.h

@@ -101,7 +101,7 @@ public:
     virtual ThrowCompletionOr<Value> internal_get(PropertyName const&, Value receiver) const;
     virtual ThrowCompletionOr<bool> internal_set(PropertyName const&, Value value, Value receiver);
     virtual ThrowCompletionOr<bool> internal_delete(PropertyName const&);
-    virtual MarkedValueList internal_own_property_keys() const;
+    virtual ThrowCompletionOr<MarkedValueList> internal_own_property_keys() const;
 
     bool ordinary_set_with_own_descriptor(PropertyName const&, Value, Value, Optional<PropertyDescriptor>);
 

Userland/Libraries/LibJS/Runtime/ObjectConstructor.cpp

@@ -97,9 +97,7 @@ static Array* get_own_property_keys(GlobalObject& global_object, Value value, Ge
         return {};
 
     // 2. Let keys be ? obj.[[OwnPropertyKeys]]().
-    auto keys = object->internal_own_property_keys();
-    if (vm.exception())
-        return {};
+    auto keys = TRY_OR_DISCARD(object->internal_own_property_keys());
 
     // 3. Let nameList be a new empty List.
     auto name_list = MarkedValueList { vm.heap() };
@@ -302,12 +300,13 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::get_own_property_descriptors)
     auto* object = vm.argument(0).to_object(global_object);
     if (vm.exception())
         return {};
+
     // 2. Let ownKeys be ? obj.[[OwnPropertyKeys]]().
-    auto own_keys = object->internal_own_property_keys();
-    if (vm.exception())
-        return {};
+    auto own_keys = TRY_OR_DISCARD(object->internal_own_property_keys());
+
     // 3. Let descriptors be ! OrdinaryObjectCreate(%Object.prototype%).
     auto* descriptors = Object::create(global_object, global_object.object_prototype());
+
     // 4. For each element key of ownKeys, do
     for (auto& key : own_keys) {
         auto property_name = PropertyName::from_value(global_object, key);
@@ -471,9 +470,7 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::assign)
         VERIFY(!vm.exception());
 
         // ii. Let keys be ? from.[[OwnPropertyKeys]]().
-        auto keys = from->internal_own_property_keys();
-        if (vm.exception())
-            return {};
+        auto keys = TRY_OR_DISCARD(from->internal_own_property_keys());
 
         // iii. For each element nextKey of keys, do
         for (auto& next_key : keys) {

Userland/Libraries/LibJS/Runtime/ProxyObject.cpp

@@ -661,7 +661,7 @@ ThrowCompletionOr<bool> ProxyObject::internal_delete(PropertyName const& propert
 }
 
 // 10.5.11 [[OwnPropertyKeys]] ( ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-ownpropertykeys
-MarkedValueList ProxyObject::internal_own_property_keys() const
+ThrowCompletionOr<MarkedValueList> ProxyObject::internal_own_property_keys() const
 {
     auto& vm = this->vm();
     auto& global_object = this->global_object();
@@ -669,19 +669,14 @@ MarkedValueList ProxyObject::internal_own_property_keys() const
     // 1. Let handler be O.[[ProxyHandler]].
 
     // 2. If handler is null, throw a TypeError exception.
-    if (m_is_revoked) {
-        vm.throw_exception<TypeError>(global_object, ErrorType::ProxyRevoked);
-        return MarkedValueList { heap() };
-    }
+    if (m_is_revoked)
+        return vm.throw_completion<TypeError>(global_object, ErrorType::ProxyRevoked);
 
     // 3. Assert: Type(handler) is Object.
     // 4. Let target be O.[[ProxyTarget]].
 
     // 5. Let trap be ? GetMethod(handler, "ownKeys").
-    auto trap_or_error = Value(&m_handler).get_method(global_object, vm.names.ownKeys);
-    if (trap_or_error.is_error())
-        return MarkedValueList { heap() };
-    auto trap = trap_or_error.release_value();
+    auto trap = TRY(Value(&m_handler).get_method(global_object, vm.names.ownKeys));
 
     // 6. If trap is undefined, then
     if (!trap) {
@@ -690,14 +685,11 @@ MarkedValueList ProxyObject::internal_own_property_keys() const
     }
 
     // 7. Let trapResultArray be ? Call(trap, handler, « target »).
-    auto trap_result_array_or_error = vm.call(*trap, &m_handler, &m_target);
-    if (trap_result_array_or_error.is_error())
-        return MarkedValueList { heap() };
-    auto trap_result_array = trap_result_array_or_error.release_value();
+    auto trap_result_array = TRY(vm.call(*trap, &m_handler, &m_target));
 
     // 8. Let trapResult be ? CreateListFromArrayLike(trapResultArray, « String, Symbol »).
     HashTable<StringOrSymbol> unique_keys;
-    auto throw_completion_or_trap_result = create_list_from_array_like(global_object, trap_result_array, [&](auto value) -> ThrowCompletionOr<void> {
+    auto trap_result = TRY(create_list_from_array_like(global_object, trap_result_array, [&](auto value) -> ThrowCompletionOr<void> {
         auto& vm = global_object.vm();
         if (!value.is_string() && !value.is_symbol())
             return vm.throw_completion<TypeError>(global_object, ErrorType::ProxyOwnPropertyKeysNotStringOrSymbol);
@@ -705,27 +697,19 @@ MarkedValueList ProxyObject::internal_own_property_keys() const
         VERIFY(!vm.exception());
         unique_keys.set(property_key, AK::HashSetExistingEntryBehavior::Keep);
         return {};
-    });
-    // TODO: This becomes a lot nicer once this function returns a ThrowCompletionOr as well.
-    if (throw_completion_or_trap_result.is_throw_completion())
-        return MarkedValueList { heap() };
-    auto trap_result = throw_completion_or_trap_result.release_value();
+    }));
 
     // 9. If trapResult contains any duplicate entries, throw a TypeError exception.
-    if (unique_keys.size() != trap_result.size()) {
-        vm.throw_exception<TypeError>(global_object, ErrorType::ProxyOwnPropertyKeysDuplicates);
-        return MarkedValueList { heap() };
-    }
+    if (unique_keys.size() != trap_result.size())
+        return vm.throw_completion<TypeError>(global_object, ErrorType::ProxyOwnPropertyKeysDuplicates);
 
     // 10. Let extensibleTarget be ? IsExtensible(target).
     auto extensible_target = m_target.is_extensible();
-    if (vm.exception())
-        return MarkedValueList { heap() };
+    if (auto* exception = vm.exception())
+        return throw_completion(exception->value());
 
     // 11. Let targetKeys be ? target.[[OwnPropertyKeys]]().
-    auto target_keys = m_target.internal_own_property_keys();
-    if (vm.exception())
-        return MarkedValueList { heap() };
+    auto target_keys = TRY(m_target.internal_own_property_keys());
 
     // 12. Assert: targetKeys is a List whose elements are only String and Symbol values.
     // 13. Assert: targetKeys contains no duplicate entries.
@@ -739,10 +723,7 @@ MarkedValueList ProxyObject::internal_own_property_keys() const
     // 16. For each element key of targetKeys, do
     for (auto& key : target_keys) {
         // a. Let desc be ? target.[[GetOwnProperty]](key).
-        auto descriptor_or_error = m_target.internal_get_own_property(PropertyName::from_value(global_object, key));
-        if (descriptor_or_error.is_error())
-            return MarkedValueList { heap() };
-        auto descriptor = descriptor_or_error.release_value();
+        auto descriptor = TRY(m_target.internal_get_own_property(PropertyName::from_value(global_object, key)));
 
         // b. If desc is not undefined and desc.[[Configurable]] is false, then
         if (descriptor.has_value() && !*descriptor->configurable) {
@@ -759,7 +740,7 @@ MarkedValueList ProxyObject::internal_own_property_keys() const
     // 17. If extensibleTarget is true and targetNonconfigurableKeys is empty, then
     if (extensible_target && target_nonconfigurable_keys.is_empty()) {
         // a. Return trapResult.
-        return trap_result;
+        return { move(trap_result) };
     }
 
     // 18. Let uncheckedResultKeys be a List whose elements are the elements of trapResult.
@@ -769,10 +750,8 @@ MarkedValueList ProxyObject::internal_own_property_keys() const
     // 19. For each element key of targetNonconfigurableKeys, do
     for (auto& key : target_nonconfigurable_keys) {
         // a. If key is not an element of uncheckedResultKeys, throw a TypeError exception.
-        if (!unchecked_result_keys.contains_slow(key)) {
-            vm.throw_exception<TypeError>(global_object, ErrorType::FixmeAddAnErrorString);
-            return MarkedValueList { heap() };
-        }
+        if (!unchecked_result_keys.contains_slow(key))
+            return vm.throw_completion<TypeError>(global_object, ErrorType::FixmeAddAnErrorString);
 
         // b. Remove key from uncheckedResultKeys.
         unchecked_result_keys.remove_first_matching([&](auto& value) {
@@ -782,15 +761,13 @@ MarkedValueList ProxyObject::internal_own_property_keys() const
 
     // 20. If extensibleTarget is true, return trapResult.
     if (extensible_target)
-        return trap_result;
+        return { move(trap_result) };
 
     // 21. For each element key of targetConfigurableKeys, do
     for (auto& key : target_configurable_keys) {
         // a. If key is not an element of uncheckedResultKeys, throw a TypeError exception.
-        if (!unchecked_result_keys.contains_slow(key)) {
-            vm.throw_exception<TypeError>(global_object, ErrorType::FixmeAddAnErrorString);
-            return MarkedValueList { heap() };
-        }
+        if (!unchecked_result_keys.contains_slow(key))
+            return vm.throw_completion<TypeError>(global_object, ErrorType::FixmeAddAnErrorString);
 
         // b. Remove key from uncheckedResultKeys.
         unchecked_result_keys.remove_first_matching([&](auto& value) {
@@ -799,13 +776,11 @@ MarkedValueList ProxyObject::internal_own_property_keys() const
     }
 
     // 22. If uncheckedResultKeys is not empty, throw a TypeError exception.
-    if (!unchecked_result_keys.is_empty()) {
-        vm.throw_exception<TypeError>(global_object, ErrorType::FixmeAddAnErrorString);
-        return MarkedValueList { heap() };
-    }
+    if (!unchecked_result_keys.is_empty())
+        return vm.throw_completion<TypeError>(global_object, ErrorType::FixmeAddAnErrorString);
 
     // 23. Return trapResult.
-    return trap_result;
+    return { move(trap_result) };
 }
 
 // 10.5.12 [[Call]] ( thisArgument, argumentsList ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-call-thisargument-argumentslist

Userland/Libraries/LibJS/Runtime/ProxyObject.h

@@ -45,7 +45,7 @@ public:
     virtual ThrowCompletionOr<Value> internal_get(PropertyName const&, Value receiver) const override;
     virtual ThrowCompletionOr<bool> internal_set(PropertyName const&, Value value, Value receiver) override;
     virtual ThrowCompletionOr<bool> internal_delete(PropertyName const&) override;
-    virtual MarkedValueList internal_own_property_keys() const override;
+    virtual ThrowCompletionOr<MarkedValueList> internal_own_property_keys() const override;
 
 private:
     virtual void visit_edges(Visitor&) override;

Userland/Libraries/LibJS/Runtime/ReflectObject.cpp

@@ -261,9 +261,7 @@ JS_DEFINE_NATIVE_FUNCTION(ReflectObject::own_keys)
     }
 
     // 2. Let keys be ? target.[[OwnPropertyKeys]]().
-    auto keys = target.as_object().internal_own_property_keys();
-    if (vm.exception())
-        return {};
+    auto keys = TRY_OR_DISCARD(target.as_object().internal_own_property_keys());
 
     // 3. Return CreateArrayFromList(keys).
     return Array::create_from(global_object, keys);

Userland/Libraries/LibJS/Runtime/StringObject.cpp

@@ -130,7 +130,7 @@ ThrowCompletionOr<bool> StringObject::internal_define_own_property(PropertyName
 }
 
 // 10.4.3.3 [[OwnPropertyKeys]] ( ), https://tc39.es/ecma262/#sec-string-exotic-objects-ownpropertykeys
-MarkedValueList StringObject::internal_own_property_keys() const
+ThrowCompletionOr<MarkedValueList> StringObject::internal_own_property_keys() const
 {
     auto& vm = this->vm();
 
@@ -176,7 +176,7 @@ MarkedValueList StringObject::internal_own_property_keys() const
     }
 
     // 9. Return keys.
-    return keys;
+    return { move(keys) };
 }
 
 }

Userland/Libraries/LibJS/Runtime/StringObject.h

@@ -29,7 +29,7 @@ public:
 private:
     virtual ThrowCompletionOr<Optional<PropertyDescriptor>> internal_get_own_property(PropertyName const&) const override;
     virtual ThrowCompletionOr<bool> internal_define_own_property(PropertyName const&, PropertyDescriptor const&) override;
-    virtual MarkedValueList internal_own_property_keys() const override;
+    virtual ThrowCompletionOr<MarkedValueList> internal_own_property_keys() const override;
 
     virtual bool is_string_object() const final { return true; }
     virtual void visit_edges(Visitor&) override;

Userland/Libraries/LibJS/Runtime/TypedArray.h

@@ -398,7 +398,7 @@ public:
     }
 
     // 10.4.5.7 [[OwnPropertyKeys]] ( ), https://tc39.es/ecma262/#sec-integer-indexed-exotic-objects-ownpropertykeys
-    virtual MarkedValueList internal_own_property_keys() const override
+    virtual ThrowCompletionOr<MarkedValueList> internal_own_property_keys() const override
     {
         auto& vm = this->vm();
 
@@ -433,7 +433,7 @@ public:
         }
 
         // 6. Return keys.
-        return keys;
+        return { move(keys) };
     }
 
     Span<const UnderlyingBufferDataType> data() const

Userland/Services/WebContent/ConsoleGlobalObject.cpp

@@ -93,7 +93,7 @@ JS::ThrowCompletionOr<bool> ConsoleGlobalObject::internal_delete(JS::PropertyNam
     return m_window_object->internal_delete(property_name);
 }
 
-JS::MarkedValueList ConsoleGlobalObject::internal_own_property_keys() const
+JS::ThrowCompletionOr<JS::MarkedValueList> ConsoleGlobalObject::internal_own_property_keys() const
 {
     return m_window_object->internal_own_property_keys();
 }

Userland/Services/WebContent/ConsoleGlobalObject.h

@@ -33,7 +33,7 @@ public:
     virtual JS::ThrowCompletionOr<JS::Value> internal_get(JS::PropertyName const&, JS::Value) const override;
     virtual JS::ThrowCompletionOr<bool> internal_set(JS::PropertyName const&, JS::Value value, JS::Value receiver) override;
     virtual JS::ThrowCompletionOr<bool> internal_delete(JS::PropertyName const& name) override;
-    virtual JS::MarkedValueList internal_own_property_keys() const override;
+    virtual JS::ThrowCompletionOr<JS::MarkedValueList> internal_own_property_keys() const override;
 
     virtual void initialize_global_object() override;