Revert "LibJS+LibWeb: Return Vector<PropertyKey> from…

internal_own_property_keys"

This reverts commit 5ee810f772.

Libraries/LibJS/Bytecode/Interpreter.cpp

@@ -1863,8 +1863,8 @@ inline ThrowCompletionOr<Value> get_object_property_iterator(Interpreter& interp
         seen_objects.set(*object_to_check);
         auto keys = TRY(object_to_check->internal_own_property_keys());
         properties.ensure_capacity(properties.size() + keys.size());
-        for (auto& property_key : keys) {
-            if (property_key.is_symbol())
+        for (auto& key : keys) {
+            if (key.is_symbol())
                 continue;
 
             // NOTE: If there is a non-enumerable property higher up the prototype chain with the same key,
@@ -1872,7 +1872,7 @@ inline ThrowCompletionOr<Value> get_object_property_iterator(Interpreter& interp
             //       This is achieved with the PropertyKeyAndEnumerableFlag struct, which doesn't consider
             //       the enumerable flag when comparing keys.
             PropertyKeyAndEnumerableFlag new_entry {
-                .key = property_key,
+                .key = TRY(PropertyKey::from_value(vm, key)),
                 .enumerable = false,
             };
 
@@ -1884,7 +1884,7 @@ inline ThrowCompletionOr<Value> get_object_property_iterator(Interpreter& interp
                 continue;
 
             new_entry.enumerable = *descriptor->enumerable;
-            properties.set(move(new_entry), property_key.to_value(vm), AK::HashSetExistingEntryBehavior::Keep);
+            properties.set(move(new_entry), key, AK::HashSetExistingEntryBehavior::Keep);
         }
     }
 

Libraries/LibJS/Runtime/Array.cpp

@@ -330,12 +330,12 @@ ThrowCompletionOr<bool> Array::internal_delete(PropertyKey const& property_key)
 }
 
 // NON-STANDARD: Used to inject the ephemeral length property's key
-ThrowCompletionOr<Vector<PropertyKey>> Array::internal_own_property_keys() const
+ThrowCompletionOr<GC::RootVector<Value>> Array::internal_own_property_keys() const
 {
     auto& vm = this->vm();
     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(), PropertyKey { vm.names.length.as_string() });
+    keys.insert(indexed_properties().real_size(), PrimitiveString::create(vm, vm.names.length.as_string()));
     return { move(keys) };
 }
 

Libraries/LibJS/Runtime/Array.h

@@ -50,7 +50,7 @@ public:
     virtual ThrowCompletionOr<Optional<PropertyDescriptor>> internal_get_own_property(PropertyKey const&) const override final;
     virtual ThrowCompletionOr<bool> internal_define_own_property(PropertyKey const&, PropertyDescriptor const&, Optional<PropertyDescriptor>* precomputed_get_own_property = nullptr) override final;
     virtual ThrowCompletionOr<bool> internal_delete(PropertyKey const&) override;
-    virtual ThrowCompletionOr<Vector<PropertyKey>> internal_own_property_keys() const override final;
+    virtual ThrowCompletionOr<GC::RootVector<Value>> internal_own_property_keys() const override final;
 
     [[nodiscard]] bool length_is_writable() const { return m_length_writable; }
 

Libraries/LibJS/Runtime/ModuleNamespaceObject.cpp

@@ -211,11 +211,11 @@ ThrowCompletionOr<bool> ModuleNamespaceObject::internal_delete(PropertyKey const
 }
 
 // 10.4.6.11 [[OwnPropertyKeys]] ( ), https://tc39.es/ecma262/#sec-module-namespace-exotic-objects-ownpropertykeys
-ThrowCompletionOr<Vector<PropertyKey>> ModuleNamespaceObject::internal_own_property_keys() const
+ThrowCompletionOr<GC::RootVector<Value>> ModuleNamespaceObject::internal_own_property_keys() const
 {
     // 1. Let exports be O.[[Exports]].
     // NOTE: We only add the exports after we know the size of symbolKeys
-    Vector<PropertyKey> exports;
+    GC::RootVector<Value> exports { vm().heap() };
 
     // 2. Let symbolKeys be OrdinaryOwnPropertyKeys(O).
     auto symbol_keys = MUST(Object::internal_own_property_keys());
@@ -223,7 +223,7 @@ ThrowCompletionOr<Vector<PropertyKey>> ModuleNamespaceObject::internal_own_prope
     // 3. Return the list-concatenation of exports and symbolKeys.
     exports.ensure_capacity(m_exports.size() + symbol_keys.size());
     for (auto const& export_name : m_exports)
-        exports.unchecked_append(export_name);
+        exports.unchecked_append(PrimitiveString::create(vm(), export_name));
     exports.extend(symbol_keys);
 
     return exports;

Libraries/LibJS/Runtime/ModuleNamespaceObject.h

@@ -29,7 +29,7 @@ public:
     virtual ThrowCompletionOr<Value> internal_get(PropertyKey const&, Value receiver, CacheablePropertyMetadata* = nullptr, PropertyLookupPhase = PropertyLookupPhase::OwnProperty) const override;
     virtual ThrowCompletionOr<bool> internal_set(PropertyKey const&, Value value, Value receiver, CacheablePropertyMetadata*, PropertyLookupPhase) override;
     virtual ThrowCompletionOr<bool> internal_delete(PropertyKey const&) override;
-    virtual ThrowCompletionOr<Vector<PropertyKey>> internal_own_property_keys() const override;
+    virtual ThrowCompletionOr<GC::RootVector<Value>> internal_own_property_keys() const override;
     virtual void initialize(Realm&) override;
 
 private:

Libraries/LibJS/Runtime/Object.cpp

@@ -279,6 +279,8 @@ ThrowCompletionOr<bool> Object::has_own_property(PropertyKey const& property_key
 // 7.3.16 SetIntegrityLevel ( O, level ), https://tc39.es/ecma262/#sec-setintegritylevel
 ThrowCompletionOr<bool> Object::set_integrity_level(IntegrityLevel level)
 {
+    auto& vm = this->vm();
+
     // 1. Let status be ? O.[[PreventExtensions]]().
     auto status = TRY(internal_prevent_extensions());
 
@@ -292,7 +294,9 @@ ThrowCompletionOr<bool> Object::set_integrity_level(IntegrityLevel level)
     // 4. If level is sealed, then
     if (level == IntegrityLevel::Sealed) {
         // a. For each element k of keys, do
-        for (auto& property_key : keys) {
+        for (auto& key : keys) {
+            auto property_key = MUST(PropertyKey::from_value(vm, key));
+
             // i. Perform ? DefinePropertyOrThrow(O, k, PropertyDescriptor { [[Configurable]]: false }).
             TRY(define_property_or_throw(property_key, { .configurable = false }));
         }
@@ -302,7 +306,9 @@ ThrowCompletionOr<bool> Object::set_integrity_level(IntegrityLevel level)
         // a. Assert: level is frozen.
 
         // b. For each element k of keys, do
-        for (auto& property_key : keys) {
+        for (auto& key : keys) {
+            auto property_key = MUST(PropertyKey::from_value(vm, key));
+
             // i. Let currentDesc be ? O.[[GetOwnProperty]](k).
             auto current_descriptor = TRY(internal_get_own_property(property_key));
 
@@ -335,6 +341,8 @@ ThrowCompletionOr<bool> Object::set_integrity_level(IntegrityLevel level)
 // 7.3.17 TestIntegrityLevel ( O, level ), https://tc39.es/ecma262/#sec-testintegritylevel
 ThrowCompletionOr<bool> Object::test_integrity_level(IntegrityLevel level) const
 {
+    auto& vm = this->vm();
+
     // 1. Let extensible be ? IsExtensible(O).
     auto extensible = TRY(is_extensible());
 
@@ -347,7 +355,9 @@ ThrowCompletionOr<bool> Object::test_integrity_level(IntegrityLevel level) const
     auto keys = TRY(internal_own_property_keys());
 
     // 5. For each element k of keys, do
-    for (auto& property_key : keys) {
+    for (auto& key : keys) {
+        auto property_key = MUST(PropertyKey::from_value(vm, key));
+
         // a. Let currentDesc be ? O.[[GetOwnProperty]](k).
         auto current_descriptor = TRY(internal_get_own_property(property_key));
 
@@ -386,11 +396,11 @@ ThrowCompletionOr<GC::RootVector<Value>> Object::enumerable_own_property_names(P
     auto properties = GC::RootVector<Value> { heap() };
 
     // 3. For each element key of ownKeys, do
-    for (auto& property_key : own_keys) {
+    for (auto& key : own_keys) {
         // a. If Type(key) is String, then
-        if (!property_key.is_string() && !property_key.is_number()) {
+        if (!key.is_string())
             continue;
-        }
+        auto property_key = MUST(PropertyKey::from_value(vm, key));
 
         // i. Let desc be ? O.[[GetOwnProperty]](key).
         auto descriptor = TRY(internal_get_own_property(property_key));
@@ -399,7 +409,7 @@ ThrowCompletionOr<GC::RootVector<Value>> Object::enumerable_own_property_names(P
         if (descriptor.has_value() && *descriptor->enumerable) {
             // 1. If kind is key, append key to properties.
             if (kind == PropertyKind::Key) {
-                properties.append(property_key.to_value(vm));
+                properties.append(key);
                 continue;
             }
             // 2. Else,
@@ -418,7 +428,7 @@ ThrowCompletionOr<GC::RootVector<Value>> Object::enumerable_own_property_names(P
             VERIFY(kind == PropertyKind::KeyAndValue);
 
             // ii. Let entry be CreateArrayFromList(« key, value »).
-            auto entry = Array::create_from(realm, { property_key.to_value(vm), value });
+            auto entry = Array::create_from(realm, { key, value });
 
             // iii. Append entry to properties.
             properties.append(entry);
@@ -444,23 +454,25 @@ ThrowCompletionOr<void> Object::copy_data_properties(VM& vm, Value source, HashT
     auto keys = TRY(from->internal_own_property_keys());
 
     // 4. For each element nextKey of keys, do
-    for (auto& next_property_key : keys) {
+    for (auto& next_key_value : keys) {
+        auto next_key = MUST(PropertyKey::from_value(vm, next_key_value));
+
         // a. Let excluded be false.
         // b. For each element e of excludedKeys, do
         //    i. If SameValue(e, nextKey) is true, then
         //        1. Set excluded to true.
-        if (excluded_keys.contains(next_property_key))
+        if (excluded_keys.contains(next_key))
             continue;
 
         // c. If excluded is false, then
 
         // i. Let desc be ? from.[[GetOwnProperty]](nextKey).
-        auto desc = TRY(from->internal_get_own_property(next_property_key));
+        auto desc = TRY(from->internal_get_own_property(next_key));
 
         // ii. If desc is not undefined and desc.[[Enumerable]] is true, then
         if (desc.has_value() && desc->attributes().is_enumerable()) {
             // 1. Let propValue be ? Get(from, nextKey).
-            auto prop_value = TRY(from->get(next_property_key));
+            auto prop_value = TRY(from->get(next_key));
 
             // 2. If excludedValues is present, then
             //     a. For each element e of excludedValues, do
@@ -469,7 +481,7 @@ ThrowCompletionOr<void> Object::copy_data_properties(VM& vm, Value source, HashT
             // 3. If excluded is false, Perform ! CreateDataPropertyOrThrow(target, nextKey, propValue).
             // NOTE: HashTable traits for JS::Value uses SameValue.
             if (!excluded_values.contains(prop_value))
-                MUST(create_data_property_or_throw(next_property_key, prop_value));
+                MUST(create_data_property_or_throw(next_key, prop_value));
         }
     }
 
@@ -1108,30 +1120,33 @@ ThrowCompletionOr<bool> Object::internal_delete(PropertyKey const& property_key)
     return false;
 }
 
-ThrowCompletionOr<Vector<PropertyKey>> Object::internal_own_property_keys() const
+// 10.1.11 [[OwnPropertyKeys]] ( ), https://tc39.es/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots-ownpropertykeys
+ThrowCompletionOr<GC::RootVector<Value>> Object::internal_own_property_keys() const
 {
+    auto& vm = this->vm();
+
     // 1. Let keys be a new empty List.
-    Vector<PropertyKey> keys;
+    GC::RootVector<Value> keys { heap() };
 
     // 2. For each own property key P of O such that P is an array index, in ascending numeric index order, do
-    for (auto const& entry : m_indexed_properties) {
+    for (auto& entry : m_indexed_properties) {
         // a. Add P as the last element of keys.
-        keys.append(String::number(entry.index()));
+        keys.append(PrimitiveString::create(vm, String::number(entry.index())));
     }
 
     // 3. For each own property key P of O such that Type(P) is String and P is not an array index, in ascending chronological order of property creation, do
-    for (auto const& it : shape().property_table()) {
+    for (auto& it : shape().property_table()) {
         if (it.key.is_string()) {
             // a. Add P as the last element of keys.
-            keys.append(it.key);
+            keys.append(it.key.to_value(vm));
         }
     }
 
     // 4. For each own property key P of O such that Type(P) is Symbol, in ascending chronological order of property creation, do
-    for (auto const& it : shape().property_table()) {
+    for (auto& it : shape().property_table()) {
         if (it.key.is_symbol()) {
             // a. Add P as the last element of keys.
-            keys.append(it.key);
+            keys.append(it.key.to_value(vm));
         }
     }
 
@@ -1361,20 +1376,22 @@ ThrowCompletionOr<Object*> Object::define_properties(Value properties)
     Vector<NameAndDescriptor> descriptors;
 
     // 4. For each element nextKey of keys, do
-    for (auto& next_property_key : keys) {
+    for (auto& next_key : keys) {
+        auto property_key = MUST(PropertyKey::from_value(vm, next_key));
+
         // a. Let propDesc be ? props.[[GetOwnProperty]](nextKey).
-        auto property_descriptor = TRY(props->internal_get_own_property(next_property_key));
+        auto property_descriptor = TRY(props->internal_get_own_property(property_key));
 
         // b. If propDesc is not undefined and propDesc.[[Enumerable]] is true, then
         if (property_descriptor.has_value() && *property_descriptor->enumerable) {
             // i. Let descObj be ? Get(props, nextKey).
-            auto descriptor_object = TRY(props->get(next_property_key));
+            auto descriptor_object = TRY(props->get(property_key));
 
             // ii. Let desc be ? ToPropertyDescriptor(descObj).
             auto descriptor = TRY(to_property_descriptor(vm, descriptor_object));
 
             // iii. Append the pair (a two element List) consisting of nextKey and desc to the end of descriptors.
-            descriptors.append({ next_property_key, descriptor });
+            descriptors.append({ property_key, descriptor });
         }
     }
 
@@ -1403,24 +1420,24 @@ Optional<Completion> Object::enumerate_object_properties(Function<Optional<Compl
     //    * Enumerating the properties of the target object includes enumerating properties of its prototype, and the prototype of the prototype, and so on, recursively.
     //    * A property of a prototype is not processed if it has the same name as a property that has already been processed.
 
-    auto& vm = this->vm();
     HashTable<FlyString> visited;
 
     auto const* target = this;
     while (target) {
         auto own_keys = TRY(target->internal_own_property_keys());
-        for (auto& property_key : own_keys) {
-            if (!property_key.is_string())
+        for (auto& key : own_keys) {
+            if (!key.is_string())
                 continue;
-            if (visited.contains(property_key.as_string()))
+            FlyString property_key = key.as_string().utf8_string();
+            if (visited.contains(property_key))
                 continue;
             auto descriptor = TRY(target->internal_get_own_property(property_key));
             if (!descriptor.has_value())
                 continue;
-            visited.set(property_key.as_string());
+            visited.set(property_key);
             if (!*descriptor->enumerable)
                 continue;
-            if (auto completion = callback(property_key.to_value(vm)); completion.has_value())
+            if (auto completion = callback(key); completion.has_value())
                 return completion.release_value();
         }
 

Libraries/LibJS/Runtime/Object.h

@@ -147,7 +147,7 @@ public:
     virtual ThrowCompletionOr<Value> internal_get(PropertyKey const&, Value receiver, CacheablePropertyMetadata* = nullptr, PropertyLookupPhase = PropertyLookupPhase::OwnProperty) const;
     virtual ThrowCompletionOr<bool> internal_set(PropertyKey const&, Value value, Value receiver, CacheablePropertyMetadata* = nullptr, PropertyLookupPhase = PropertyLookupPhase::OwnProperty);
     virtual ThrowCompletionOr<bool> internal_delete(PropertyKey const&);
-    virtual ThrowCompletionOr<Vector<PropertyKey>> internal_own_property_keys() const;
+    virtual ThrowCompletionOr<GC::RootVector<Value>> internal_own_property_keys() const;
 
     // NOTE: Any subclass of Object that overrides property access slots ([[Get]], [[Set]] etc)
     //       to customize access to indexed properties (properties where the name is a positive integer)

Libraries/LibJS/Runtime/ObjectConstructor.cpp

@@ -105,11 +105,11 @@ static ThrowCompletionOr<GC::RootVector<Value>> get_own_property_keys(VM& vm, Va
     auto name_list = GC::RootVector<Value> { vm.heap() };
 
     // 4. For each element nextKey of keys, do
-    for (auto& next_property_key : keys) {
+    for (auto& next_key : keys) {
         // a. If Type(nextKey) is Symbol and type is symbol or Type(nextKey) is String and type is string, then
-        if ((next_property_key.is_symbol() && type == GetOwnPropertyKeysType::Symbol) || ((next_property_key.is_string() || next_property_key.is_number()) && type == GetOwnPropertyKeysType::String)) {
+        if ((next_key.is_symbol() && type == GetOwnPropertyKeysType::Symbol) || (next_key.is_string() && type == GetOwnPropertyKeysType::String)) {
             // i. Append nextKey as the last element of nameList.
-            name_list.append(next_property_key.to_value(vm));
+            name_list.append(next_key);
         }
     }
 
@@ -142,19 +142,21 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::assign)
         auto keys = TRY(from->internal_own_property_keys());
 
         // iii. For each element nextKey of keys, do
-        for (auto& next_property_key : keys) {
+        for (auto& next_key : keys) {
+            auto property_key = MUST(PropertyKey::from_value(vm, next_key));
+
             // 1. Let desc be ? from.[[GetOwnProperty]](nextKey).
-            auto desc = TRY(from->internal_get_own_property(next_property_key));
+            auto desc = TRY(from->internal_get_own_property(property_key));
 
             // 2. If desc is not undefined and desc.[[Enumerable]] is true, then
             if (!desc.has_value() || !*desc->enumerable)
                 continue;
 
             // a. Let propValue be ? Get(from, nextKey).
-            auto prop_value = TRY(from->get(next_property_key));
+            auto prop_value = TRY(from->get(property_key));
 
             // b. Perform ? Set(to, nextKey, propValue, true).
-            TRY(to->set(next_property_key, prop_value, Object::ShouldThrowExceptions::Yes));
+            TRY(to->set(property_key, prop_value, Object::ShouldThrowExceptions::Yes));
         }
     }
 
@@ -325,7 +327,9 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::get_own_property_descriptors)
     auto descriptors = Object::create(realm, realm.intrinsics().object_prototype());
 
     // 4. For each element key of ownKeys, do
-    for (auto& property_key : own_keys) {
+    for (auto& key : own_keys) {
+        auto property_key = MUST(PropertyKey::from_value(vm, key));
+
         // a. Let desc be ? obj.[[GetOwnProperty]](key).
         auto desc = TRY(object->internal_get_own_property(property_key));
 

Libraries/LibJS/Runtime/PropertyKey.h

@@ -84,8 +84,7 @@ public:
 
     String to_string() const
     {
-        if (is_symbol())
-            return as_symbol()->descriptive_string().release_value();
+        VERIFY(!is_symbol());
         if (is_string())
             return as_string().to_string();
         return String::number(as_number());
@@ -99,15 +98,6 @@ public:
         return StringOrSymbol(as_symbol());
     }
 
-    Value to_value(VM& vm) const
-    {
-        if (is_string())
-            return Value { PrimitiveString::create(vm, as_string()) };
-        if (is_symbol())
-            return Value { as_symbol() };
-        return Value { PrimitiveString::create(vm, String::number(as_number())) };
-    }
-
     bool operator==(PropertyKey const&) const = default;
 
 private:

Libraries/LibJS/Runtime/ProxyObject.cpp

@@ -659,7 +659,7 @@ ThrowCompletionOr<bool> ProxyObject::internal_delete(PropertyKey const& property
 }
 
 // 10.5.11 [[OwnPropertyKeys]] ( ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-ownpropertykeys
-ThrowCompletionOr<Vector<PropertyKey>> ProxyObject::internal_own_property_keys() const
+ThrowCompletionOr<GC::RootVector<Value>> ProxyObject::internal_own_property_keys() const
 {
     LIMIT_PROXY_RECURSION_DEPTH();
 
@@ -693,10 +693,6 @@ ThrowCompletionOr<Vector<PropertyKey>> ProxyObject::internal_own_property_keys()
         unique_keys.set(property_key, AK::HashSetExistingEntryBehavior::Keep);
         return {};
     }));
-    Vector<PropertyKey> trap_result_property_keys;
-    for (auto const& key : trap_result) {
-        trap_result_property_keys.append(MUST(PropertyKey::from_value(vm, key)));
-    }
 
     // 9. If trapResult contains any duplicate entries, throw a TypeError exception.
     if (unique_keys.size() != trap_result.size())
@@ -712,72 +708,74 @@ ThrowCompletionOr<Vector<PropertyKey>> ProxyObject::internal_own_property_keys()
     // 13. Assert: targetKeys contains no duplicate entries.
 
     // 14. Let targetConfigurableKeys be a new empty List.
-    Vector<PropertyKey> target_configurable_keys;
+    auto target_configurable_keys = GC::RootVector<Value> { heap() };
 
     // 15. Let targetNonconfigurableKeys be a new empty List.
-    Vector<PropertyKey> target_nonconfigurable_keys;
+    auto target_nonconfigurable_keys = GC::RootVector<Value> { heap() };
 
     // 16. For each element key of targetKeys, do
-    for (auto& property_key : target_keys) {
+    for (auto& key : target_keys) {
+        auto property_key = MUST(PropertyKey::from_value(vm, key));
+
         // a. Let desc be ? target.[[GetOwnProperty]](key).
         auto descriptor = TRY(m_target->internal_get_own_property(property_key));
 
         // b. If desc is not undefined and desc.[[Configurable]] is false, then
         if (descriptor.has_value() && !*descriptor->configurable) {
             // i. Append key as an element of targetNonconfigurableKeys.
-            target_nonconfigurable_keys.append(property_key);
+            target_nonconfigurable_keys.append(key);
         }
         // c. Else,
         else {
             // i. Append key as an element of targetConfigurableKeys.
-            target_configurable_keys.append(property_key);
+            target_configurable_keys.append(key);
         }
     }
 
     // 17. If extensibleTarget is true and targetNonconfigurableKeys is empty, then
     if (extensible_target && target_nonconfigurable_keys.is_empty()) {
         // a. Return trapResult.
-        return { move(trap_result_property_keys) };
+        return { move(trap_result) };
     }
 
     // 18. Let uncheckedResultKeys be a List whose elements are the elements of trapResult.
-    Vector<PropertyKey> unchecked_result_keys;
-    unchecked_result_keys.extend(trap_result_property_keys);
+    auto unchecked_result_keys = GC::RootVector<Value> { heap() };
+    unchecked_result_keys.extend(trap_result);
 
     // 19. For each element key of targetNonconfigurableKeys, do
-    for (auto& property_key : target_nonconfigurable_keys) {
+    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(property_key))
-            return vm.throw_completion<TypeError>(ErrorType::ProxyOwnPropertyKeysSkippedNonconfigurableProperty, property_key.to_string());
+        if (!unchecked_result_keys.contains_slow(key))
+            return vm.throw_completion<TypeError>(ErrorType::ProxyOwnPropertyKeysSkippedNonconfigurableProperty, key.to_string_without_side_effects());
 
         // b. Remove key from uncheckedResultKeys.
         unchecked_result_keys.remove_first_matching([&](auto& value) {
-            return value == property_key;
+            return same_value(value, key);
         });
     }
 
     // 20. If extensibleTarget is true, return trapResult.
     if (extensible_target)
-        return { move(trap_result_property_keys) };
+        return { move(trap_result) };
 
     // 21. For each element key of targetConfigurableKeys, do
-    for (auto& property_key : target_configurable_keys) {
+    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(property_key))
-            return vm.throw_completion<TypeError>(ErrorType::ProxyOwnPropertyKeysNonExtensibleSkippedProperty, property_key.to_string());
+        if (!unchecked_result_keys.contains_slow(key))
+            return vm.throw_completion<TypeError>(ErrorType::ProxyOwnPropertyKeysNonExtensibleSkippedProperty, key.to_string_without_side_effects());
 
         // b. Remove key from uncheckedResultKeys.
         unchecked_result_keys.remove_first_matching([&](auto& value) {
-            return value == property_key;
+            return same_value(value, key);
         });
     }
 
     // 22. If uncheckedResultKeys is not empty, throw a TypeError exception.
     if (!unchecked_result_keys.is_empty())
-        return vm.throw_completion<TypeError>(ErrorType::ProxyOwnPropertyKeysNonExtensibleNewProperty, unchecked_result_keys[0].to_string());
+        return vm.throw_completion<TypeError>(ErrorType::ProxyOwnPropertyKeysNonExtensibleNewProperty, unchecked_result_keys[0].to_string_without_side_effects());
 
     // 23. Return trapResult.
-    return { move(trap_result_property_keys) };
+    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

Libraries/LibJS/Runtime/ProxyObject.h

@@ -41,7 +41,7 @@ public:
     virtual ThrowCompletionOr<Value> internal_get(PropertyKey const&, Value receiver, CacheablePropertyMetadata*, PropertyLookupPhase) const override;
     virtual ThrowCompletionOr<bool> internal_set(PropertyKey const&, Value value, Value receiver, CacheablePropertyMetadata*, PropertyLookupPhase) override;
     virtual ThrowCompletionOr<bool> internal_delete(PropertyKey const&) override;
-    virtual ThrowCompletionOr<Vector<PropertyKey>> internal_own_property_keys() const override;
+    virtual ThrowCompletionOr<GC::RootVector<Value>> internal_own_property_keys() const override;
     virtual ThrowCompletionOr<Value> internal_call(ExecutionContext&, Value this_argument) override;
     virtual ThrowCompletionOr<GC::Ref<Object>> internal_construct(ReadonlySpan<Value> arguments_list, FunctionObject& new_target) override;
     ThrowCompletionOr<void> validate_non_revoked_proxy() const;

Libraries/LibJS/Runtime/ReflectObject.cpp

@@ -226,11 +226,7 @@ JS_DEFINE_NATIVE_FUNCTION(ReflectObject::own_keys)
         return vm.throw_completion<TypeError>(ErrorType::NotAnObject, target.to_string_without_side_effects());
 
     // 2. Let keys be ? target.[[OwnPropertyKeys]]().
-    auto property_keys = TRY(target.as_object().internal_own_property_keys());
-    GC::RootVector<Value> keys { vm.heap() };
-    keys.ensure_capacity(property_keys.size());
-    for (auto& property_key : property_keys)
-        keys.append(property_key.to_value(vm));
+    auto keys = TRY(target.as_object().internal_own_property_keys());
 
     // 3. Return CreateArrayFromList(keys).
     return Array::create_from(realm, keys);

Libraries/LibJS/Runtime/StringObject.cpp

@@ -128,10 +128,12 @@ ThrowCompletionOr<bool> StringObject::internal_define_own_property(PropertyKey c
 }
 
 // 10.4.3.3 [[OwnPropertyKeys]] ( ), https://tc39.es/ecma262/#sec-string-exotic-objects-ownpropertykeys
-ThrowCompletionOr<Vector<PropertyKey>> StringObject::internal_own_property_keys() const
+ThrowCompletionOr<GC::RootVector<Value>> StringObject::internal_own_property_keys() const
 {
+    auto& vm = this->vm();
+
     // 1. Let keys be a new empty List.
-    Vector<PropertyKey> keys;
+    auto keys = GC::RootVector<Value> { heap() };
 
     // 2. Let str be O.[[StringData]].
     // 3. Assert: str is a String.
@@ -141,30 +143,30 @@ ThrowCompletionOr<Vector<PropertyKey>> StringObject::internal_own_property_keys(
     // 5. For each integer i starting with 0 such that i < len, in ascending order, do
     for (size_t i = 0; i < length; ++i) {
         // a. Add ! ToString(𝔽(i)) as the last element of keys.
-        keys.append(String::number(i));
+        keys.append(PrimitiveString::create(vm, String::number(i)));
     }
 
     // 6. For each own property key P of O such that P is an array index and ! ToIntegerOrInfinity(P) ≥ len, in ascending numeric index order, do
-    for (auto const& entry : indexed_properties()) {
+    for (auto& entry : indexed_properties()) {
         if (entry.index() >= length) {
             // a. Add P as the last element of keys.
-            keys.append(String::number(entry.index()));
+            keys.append(PrimitiveString::create(vm, String::number(entry.index())));
         }
     }
 
     // 7. For each own property key P of O such that P is a String and P is not an array index, in ascending chronological order of property creation, do
-    for (auto const& it : shape().property_table()) {
+    for (auto& it : shape().property_table()) {
         if (it.key.is_string()) {
             // a. Add P as the last element of keys.
-            keys.append(it.key);
+            keys.append(it.key.to_value(vm));
         }
     }
 
     // 8. For each own property key P of O such that P is a Symbol, in ascending chronological order of property creation, do
-    for (auto const& it : shape().property_table()) {
+    for (auto& it : shape().property_table()) {
         if (it.key.is_symbol()) {
             // a. Add P as the last element of keys.
-            keys.append(it.key);
+            keys.append(it.key.to_value(vm));
         }
     }
 

Libraries/LibJS/Runtime/StringObject.h

@@ -29,7 +29,7 @@ protected:
 private:
     virtual ThrowCompletionOr<Optional<PropertyDescriptor>> internal_get_own_property(PropertyKey const&) const override;
     virtual ThrowCompletionOr<bool> internal_define_own_property(PropertyKey const&, PropertyDescriptor const&, Optional<PropertyDescriptor>* precomputed_get_own_property = nullptr) override;
-    virtual ThrowCompletionOr<Vector<PropertyKey>> internal_own_property_keys() const override;
+    virtual ThrowCompletionOr<GC::RootVector<Value>> internal_own_property_keys() const override;
 
     virtual bool is_string_object() const final { return true; }
     virtual void visit_edges(Visitor&) override;

Libraries/LibJS/Runtime/TypedArray.h

@@ -413,13 +413,15 @@ public:
     }
 
     // 10.4.5.8 [[OwnPropertyKeys]] ( ), https://tc39.es/ecma262/#sec-integer-indexed-exotic-objects-ownpropertykeys
-    virtual ThrowCompletionOr<Vector<PropertyKey>> internal_own_property_keys() const override
+    virtual ThrowCompletionOr<GC::RootVector<Value>> internal_own_property_keys() const override
     {
+        auto& vm = this->vm();
+
         // 1. Let taRecord be MakeTypedArrayWithBufferWitnessRecord(O, seq-cst).
         auto typed_array_record = make_typed_array_with_buffer_witness_record(*this, ArrayBuffer::Order::SeqCst);
 
         // 2. Let keys be a new empty List.
-        Vector<PropertyKey> keys;
+        auto keys = GC::RootVector<Value> { heap() };
 
         // 3. If IsTypedArrayOutOfBounds(taRecord) is false, then
         if (!is_typed_array_out_of_bounds(typed_array_record)) {
@@ -429,7 +431,7 @@ public:
             // b. For each integer i such that 0 ≤ i < length, in ascending order, do
             for (size_t i = 0; i < length; ++i) {
                 // i. Append ! ToString(𝔽(i)) to keys.
-                keys.append(String::number(i));
+                keys.append(PrimitiveString::create(vm, String::number(i)));
             }
         }
 
@@ -437,7 +439,7 @@ public:
         for (auto& it : shape().property_table()) {
             if (it.key.is_string()) {
                 // a. Append P to keys.
-                keys.append(it.key);
+                keys.append(it.key.to_value(vm));
             }
         }
 
@@ -445,7 +447,7 @@ public:
         for (auto& it : shape().property_table()) {
             if (it.key.is_symbol()) {
                 // a. Append P to keys.
-                keys.append(it.key);
+                keys.append(it.key.to_value(vm));
             }
         }
 

Libraries/LibWeb/Bindings/PlatformObject.cpp

@@ -400,19 +400,21 @@ JS::ThrowCompletionOr<bool> PlatformObject::internal_prevent_extensions()
 }
 
 // https://webidl.spec.whatwg.org/#legacy-platform-object-ownpropertykeys
-JS::ThrowCompletionOr<Vector<JS::PropertyKey>> PlatformObject::internal_own_property_keys() const
+JS::ThrowCompletionOr<GC::RootVector<JS::Value>> PlatformObject::internal_own_property_keys() const
 {
     if (!m_legacy_platform_object_flags.has_value() || m_legacy_platform_object_flags->has_global_interface_extended_attribute)
         return Base::internal_own_property_keys();
 
+    auto& vm = this->vm();
+
     // 1. Let keys be a new empty list of ECMAScript String and Symbol values.
-    Vector<JS::PropertyKey> keys;
+    GC::RootVector<JS::Value> keys { heap() };
 
     // 2. If O supports indexed properties, then for each index of O’s supported property indices, in ascending numerical order, append ! ToString(index) to keys.
     if (m_legacy_platform_object_flags->supports_indexed_properties) {
         for (u64 index = 0; index <= NumericLimits<u32>::max(); ++index) {
             if (is_supported_property_index(index))
-                keys.append(String::number(index));
+                keys.append(JS::PrimitiveString::create(vm, String::number(index)));
             else
                 break;
         }
@@ -420,22 +422,22 @@ JS::ThrowCompletionOr<Vector<JS::PropertyKey>> PlatformObject::internal_own_prop
 
     // 3. If O supports named properties, then for each P of O’s supported property names that is visible according to the named property visibility algorithm, append P to keys.
     if (m_legacy_platform_object_flags->supports_named_properties) {
-        for (auto const& named_property : supported_property_names()) {
+        for (auto& named_property : supported_property_names()) {
             if (TRY(is_named_property_exposed_on_object(named_property)))
-                keys.append(named_property);
+                keys.append(JS::PrimitiveString::create(vm, named_property));
         }
     }
 
     // 4. For each P of O’s own property keys that is a String, in ascending chronological order of property creation, append P to keys.
-    for (auto const& it : shape().property_table()) {
+    for (auto& it : shape().property_table()) {
         if (it.key.is_string())
-            keys.append(it.key);
+            keys.append(it.key.to_value(vm));
     }
 
     // 5. For each P of O’s own property keys that is a Symbol, in ascending chronological order of property creation, append P to keys.
-    for (auto const& it : shape().property_table()) {
+    for (auto& it : shape().property_table()) {
         if (it.key.is_symbol())
-            keys.append(it.key);
+            keys.append(it.key.to_value(vm));
     }
 
     // FIXME: 6. Assert: keys has no duplicate items.

Libraries/LibWeb/Bindings/PlatformObject.h

@@ -40,7 +40,7 @@ public:
     virtual JS::ThrowCompletionOr<bool> internal_define_own_property(JS::PropertyKey const&, JS::PropertyDescriptor const&, Optional<JS::PropertyDescriptor>* precomputed_get_own_property = nullptr) override;
     virtual JS::ThrowCompletionOr<bool> internal_delete(JS::PropertyKey const&) override;
     virtual JS::ThrowCompletionOr<bool> internal_prevent_extensions() override;
-    virtual JS::ThrowCompletionOr<Vector<JS::PropertyKey>> internal_own_property_keys() const override;
+    virtual JS::ThrowCompletionOr<GC::RootVector<JS::Value>> internal_own_property_keys() const override;
 
     JS::ThrowCompletionOr<bool> is_named_property_exposed_on_object(JS::PropertyKey const&) const;
 

Libraries/LibWeb/HTML/CrossOrigin/AbstractOperations.cpp

@@ -236,20 +236,20 @@ JS::ThrowCompletionOr<bool> cross_origin_set(JS::VM& vm, JS::Object& object, JS:
 }
 
 // 7.2.3.7 CrossOriginOwnPropertyKeys ( O ), https://html.spec.whatwg.org/multipage/browsers.html#crossoriginownpropertykeys-(-o-)
-Vector<JS::PropertyKey> cross_origin_own_property_keys(Variant<HTML::Location const*, HTML::Window const*> const& object)
+GC::RootVector<JS::Value> cross_origin_own_property_keys(Variant<HTML::Location const*, HTML::Window const*> const& object)
 {
     auto& event_loop = HTML::main_thread_event_loop();
     auto& vm = event_loop.vm();
 
     // 1. Let keys be a new empty List.
-    Vector<JS::PropertyKey> keys;
+    auto keys = GC::RootVector<JS::Value> { vm.heap() };
 
     // 2. For each e of CrossOriginProperties(O), append e.[[Property]] to keys.
     for (auto& entry : cross_origin_properties(object))
-        keys.append(entry.property);
+        keys.append(JS::PrimitiveString::create(vm, move(entry.property)));
 
     // 3. Return the concatenation of keys and « "then", @@toStringTag, @@hasInstance, @@isConcatSpreadable ».
-    keys.append(vm.names.then.as_string());
+    keys.append(JS::PrimitiveString::create(vm, vm.names.then.as_string()));
     keys.append(vm.well_known_symbol_to_string_tag());
     keys.append(vm.well_known_symbol_has_instance());
     keys.append(vm.well_known_symbol_is_concat_spreadable());

Libraries/LibWeb/HTML/CrossOrigin/AbstractOperations.h

@@ -21,6 +21,6 @@ bool is_platform_object_same_origin(JS::Object const&);
 Optional<JS::PropertyDescriptor> cross_origin_get_own_property_helper(Variant<HTML::Location*, HTML::Window*> const&, JS::PropertyKey const&);
 JS::ThrowCompletionOr<JS::Value> cross_origin_get(JS::VM&, JS::Object const&, JS::PropertyKey const&, JS::Value receiver);
 JS::ThrowCompletionOr<bool> cross_origin_set(JS::VM&, JS::Object&, JS::PropertyKey const&, JS::Value, JS::Value receiver);
-Vector<JS::PropertyKey> cross_origin_own_property_keys(Variant<HTML::Location const*, HTML::Window const*> const&);
+GC::RootVector<JS::Value> cross_origin_own_property_keys(Variant<HTML::Location const*, HTML::Window const*> const&);
 
 }

Libraries/LibWeb/HTML/Location.cpp

@@ -36,6 +36,7 @@ Location::~Location() = default;
 void Location::visit_edges(Cell::Visitor& visitor)
 {
     Base::visit_edges(visitor);
+    visitor.visit(m_default_properties);
 }
 
 // https://html.spec.whatwg.org/multipage/nav-history-apis.html#the-location-interface
@@ -622,7 +623,10 @@ JS::ThrowCompletionOr<Optional<JS::PropertyDescriptor>> Location::internal_get_o
 
         // 2. If the value of the [[DefaultProperties]] internal slot of this contains P, then set desc.[[Configurable]] to true.
         // FIXME: This doesn't align with what the other browsers do. Spec issue: https://github.com/whatwg/html/issues/4157
-        if (m_default_properties.contains_slow(property_key))
+        auto property_key_value = property_key.is_symbol()
+            ? JS::Value { property_key.as_symbol() }
+            : JS::PrimitiveString::create(vm, property_key.to_string());
+        if (m_default_properties.contains_slow(property_key_value))
             descriptor->configurable = true;
 
         // 3. Return desc.
@@ -692,7 +696,7 @@ JS::ThrowCompletionOr<bool> Location::internal_delete(JS::PropertyKey const& pro
 }
 
 // 7.10.5.10 [[OwnPropertyKeys]] ( ), https://html.spec.whatwg.org/multipage/history.html#location-ownpropertykeys
-JS::ThrowCompletionOr<Vector<JS::PropertyKey>> Location::internal_own_property_keys() const
+JS::ThrowCompletionOr<GC::RootVector<JS::Value>> Location::internal_own_property_keys() const
 {
     // 1. If IsPlatformObjectSameOrigin(this) is true, then return OrdinaryOwnPropertyKeys(this).
     if (HTML::is_platform_object_same_origin(*this))

Libraries/LibWeb/HTML/Location.h

@@ -63,7 +63,7 @@ public:
     virtual JS::ThrowCompletionOr<JS::Value> internal_get(JS::PropertyKey const&, JS::Value receiver, JS::CacheablePropertyMetadata*, PropertyLookupPhase) const override;
     virtual JS::ThrowCompletionOr<bool> internal_set(JS::PropertyKey const&, JS::Value value, JS::Value receiver, JS::CacheablePropertyMetadata*, PropertyLookupPhase) override;
     virtual JS::ThrowCompletionOr<bool> internal_delete(JS::PropertyKey const&) override;
-    virtual JS::ThrowCompletionOr<Vector<JS::PropertyKey>> internal_own_property_keys() const override;
+    virtual JS::ThrowCompletionOr<GC::RootVector<JS::Value>> internal_own_property_keys() const override;
 
     HTML::CrossOriginPropertyDescriptorMap const& cross_origin_property_descriptor_map() const { return m_cross_origin_property_descriptor_map; }
     HTML::CrossOriginPropertyDescriptorMap& cross_origin_property_descriptor_map() { return m_cross_origin_property_descriptor_map; }
@@ -84,7 +84,7 @@ private:
     HTML::CrossOriginPropertyDescriptorMap m_cross_origin_property_descriptor_map;
 
     // [[DefaultProperties]], https://html.spec.whatwg.org/multipage/history.html#defaultproperties
-    Vector<JS::PropertyKey> m_default_properties;
+    Vector<JS::Value> m_default_properties;
 };
 
 }

Libraries/LibWeb/HTML/WindowProxy.cpp

@@ -228,12 +228,15 @@ JS::ThrowCompletionOr<bool> WindowProxy::internal_delete(JS::PropertyKey const&
 }
 
 // 7.4.10 [[OwnPropertyKeys]] ( ), https://html.spec.whatwg.org/multipage/window-object.html#windowproxy-ownpropertykeys
-JS::ThrowCompletionOr<Vector<JS::PropertyKey>> WindowProxy::internal_own_property_keys() const
+JS::ThrowCompletionOr<GC::RootVector<JS::Value>> WindowProxy::internal_own_property_keys() const
 {
+    auto& event_loop = main_thread_event_loop();
+    auto& vm = event_loop.vm();
+
     // 1. Let W be the value of the [[Window]] internal slot of this.
 
     // 2. Let keys be a new empty List.
-    Vector<JS::PropertyKey> keys;
+    auto keys = GC::RootVector<JS::Value> { vm.heap() };
 
     // 3. Let maxProperties be W's associated Document's document-tree child navigables's size.
     auto max_properties = m_window->associated_document().document_tree_child_navigables().size();
@@ -242,7 +245,7 @@ JS::ThrowCompletionOr<Vector<JS::PropertyKey>> WindowProxy::internal_own_propert
     // 5. Repeat while index < maxProperties,
     for (size_t i = 0; i < max_properties; ++i) {
         // 1. Add ! ToString(index) as the last element of keys.
-        keys.append(String::number(i));
+        keys.append(JS::PrimitiveString::create(vm, ByteString::number(i)));
 
         // 2. Increment index by 1.
     }

Libraries/LibWeb/HTML/WindowProxy.h

@@ -30,7 +30,7 @@ public:
     virtual JS::ThrowCompletionOr<JS::Value> internal_get(JS::PropertyKey const&, JS::Value receiver, JS::CacheablePropertyMetadata*, PropertyLookupPhase) const override;
     virtual JS::ThrowCompletionOr<bool> internal_set(JS::PropertyKey const&, JS::Value value, JS::Value receiver, JS::CacheablePropertyMetadata*, PropertyLookupPhase) override;
     virtual JS::ThrowCompletionOr<bool> internal_delete(JS::PropertyKey const&) override;
-    virtual JS::ThrowCompletionOr<Vector<JS::PropertyKey>> internal_own_property_keys() const override;
+    virtual JS::ThrowCompletionOr<GC::RootVector<JS::Value>> internal_own_property_keys() const override;
 
     GC::Ptr<Window> window() const { return m_window; }
     void set_window(GC::Ref<Window>);

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

@@ -1139,15 +1139,18 @@ static void generate_to_cpp(SourceGenerator& generator, ParameterType& parameter
 
     auto record_keys@recursion_depth@ = TRY(@js_name@@js_suffix@_object.internal_own_property_keys());
 
-    for (auto& property_key@recursion_depth@ : record_keys@recursion_depth@) {
+    for (auto& key@recursion_depth@ : record_keys@recursion_depth@) {
+        auto property_key@recursion_depth@ = MUST(JS::PropertyKey::from_value(vm, key@recursion_depth@));
+
         auto descriptor@recursion_depth@ = TRY(@js_name@@js_suffix@_object.internal_get_own_property(property_key@recursion_depth@));
 
         if (!descriptor@recursion_depth@.has_value() || !descriptor@recursion_depth@->enumerable.has_value() || !descriptor@recursion_depth@->enumerable.value())
             continue;
-
-        auto key_string = TRY(WebIDL::to_byte_string(vm, property_key@recursion_depth@.to_value(vm)));
 )~~~");
 
+        IDL::Parameter key_parameter { .type = parameterized_type.parameters()[0], .name = acceptable_cpp_name, .optional_default_value = {}, .extended_attributes = {} };
+        generate_to_cpp(record_generator, key_parameter, "key", ByteString::number(recursion_depth), ByteString::formatted("typed_key{}", recursion_depth), interface, false, false, {}, false, recursion_depth + 1);
+
         record_generator.append(R"~~~(
         auto value@recursion_depth@ = TRY(@js_name@@js_suffix@_object.get(property_key@recursion_depth@));
 )~~~");
@@ -1157,7 +1160,7 @@ static void generate_to_cpp(SourceGenerator& generator, ParameterType& parameter
         generate_to_cpp(record_generator, value_parameter, "value", ByteString::number(recursion_depth), ByteString::formatted("typed_value{}", recursion_depth), interface, false, false, {}, false, recursion_depth + 1);
 
         record_generator.append(R"~~~(
-        @cpp_name@.set(key_string, typed_value@recursion_depth@);
+        @cpp_name@.set(typed_key@recursion_depth@, typed_value@recursion_depth@);
     }
 )~~~");
     } else if (is<IDL::UnionType>(*parameter.type)) {