LibJS: Make Map iterators independent of the underlying hashmap

This implements ordered maps as a pair of an RBTree for key order, and
an underlying unordered hash map for storage.
Fixes (part of) #11004.

Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp

@@ -1815,7 +1815,7 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayPrototype::group_by_to_map)
 
         // b. Let entry be the Record { [[Key]]: g.[[Key]], [[Value]]: elements }.
         // c. Append entry as the last element of map.[[MapData]].
-        map->entries().set(group.key.value(), elements);
+        map->map_set(group.key.value(), elements);
     }
 
     // 9. Return map.

Userland/Libraries/LibJS/Runtime/Map.cpp

@@ -22,6 +22,65 @@ Map::~Map()
 {
 }
 
+// 24.1.3.1 Map.prototype.clear ( ), https://tc39.es/ecma262/#sec-map.prototype.clear
+void Map::map_clear()
+{
+    m_keys.clear();
+    m_entries.clear();
+}
+
+// 24.1.3.3 Map.prototype.delete ( key ), https://tc39.es/ecma262/#sec-map.prototype.delete
+bool Map::map_remove(Value const& key)
+{
+    Optional<size_t> index;
+
+    for (auto it = m_keys.begin(); it != m_keys.end(); ++it) {
+        if (ValueTraits::equals(*it, key)) {
+            index = it.key();
+            break;
+        }
+    }
+
+    if (!index.has_value())
+        return false;
+
+    m_keys.remove(*index);
+    m_entries.remove(key);
+    return true;
+}
+
+// 24.1.3.6 Map.prototype.get ( key ), https://tc39.es/ecma262/#sec-map.prototype.get
+Optional<Value> Map::map_get(Value const& key) const
+{
+    if (auto it = m_entries.find(key); it != m_entries.end())
+        return it->value;
+    return {};
+}
+
+// 24.1.3.7 Map.prototype.has ( key ), https://tc39.es/ecma262/#sec-map.prototype.has
+bool Map::map_has(Value const& key) const
+{
+    return m_entries.contains(key);
+}
+
+// 24.1.3.9 Map.prototype.set ( key, value ), https://tc39.es/ecma262/#sec-map.prototype.set
+void Map::map_set(Value const& key, Value value)
+{
+    auto it = m_entries.find(key);
+    if (it != m_entries.end()) {
+        it->value = value;
+    } else {
+        auto index = m_next_insertion_id++;
+        m_keys.insert(index, key);
+        m_entries.set(key, value);
+    }
+}
+
+size_t Map::map_size() const
+{
+    return m_keys.size();
+}
+
 void Map::visit_edges(Cell::Visitor& visitor)
 {
     Base::visit_edges(visitor);

Userland/Libraries/LibJS/Runtime/Map.h

@@ -6,7 +6,9 @@
 
 #pragma once
 
+#include <AK/Concepts.h>
 #include <AK/HashMap.h>
+#include <AK/RedBlackTree.h>
 #include <LibJS/Runtime/GlobalObject.h>
 #include <LibJS/Runtime/Object.h>
 #include <LibJS/Runtime/Value.h>
@@ -22,13 +24,89 @@ public:
     explicit Map(Object& prototype);
     virtual ~Map() override;
 
-    OrderedHashMap<Value, Value, ValueTraits> const& entries() const { return m_entries; };
+    void map_clear();
-    OrderedHashMap<Value, Value, ValueTraits>& entries() { return m_entries; };
+    bool map_remove(Value const&);
+    Optional<Value> map_get(Value const&) const;
+    bool map_has(Value const&) const;
+    void map_set(Value const&, Value);
+    size_t map_size() const;
+
+    struct EndIterator {
+    };
+
+    template<bool IsConst>
+    struct IteratorImpl {
+        bool is_end() const
+        {
+            if (m_index.has_value()) {
+                return m_map.m_keys.begin_from(*m_index).is_end()
+                    && m_map.m_keys.find_smallest_not_below_iterator(*m_index).is_end();
+            }
+
+            // First attempt and no iteration, ask the map if it has anything.
+            return m_map.m_keys.is_empty();
+        }
+
+        IteratorImpl& operator++()
+        {
+            if (auto it = m_map.m_keys.find_smallest_not_below_iterator(ensure_index() + 1); it.is_end())
+                m_index = m_map.m_next_insertion_id;
+            else
+                m_index = it.key();
+            return *this;
+        }
+
+        decltype(auto) operator*()
+        {
+            return *m_map.m_entries.find(*m_map.m_keys.begin_from(ensure_index()));
+        }
+
+        decltype(auto) operator*() const
+        {
+            return *m_map.m_entries.find(*m_map.m_keys.begin_from(ensure_index()));
+        }
+
+        bool operator==(IteratorImpl const& other) const { return m_index == other.m_index && &m_map == &other.m_map; }
+        bool operator==(EndIterator const&) const { return is_end(); }
+
+    private:
+        friend class Map;
+        IteratorImpl(Map const& map) requires(IsConst)
+            : m_map(map)
+        {
+        }
+
+        IteratorImpl(Map& map) requires(!IsConst)
+            : m_map(map)
+        {
+        }
+
+        size_t ensure_index()
+        {
+            if (!m_index.has_value()) {
+                VERIFY(!m_map.m_keys.is_empty());
+                m_index = m_map.m_keys.begin().key();
+            }
+            return *m_index;
+        }
+
+        Conditional<IsConst, Map const&, Map&> m_map;
+        mutable Optional<size_t> m_index;
+    };
+
+    using Iterator = IteratorImpl<false>;
+    using ConstIterator = IteratorImpl<true>;
+
+    ConstIterator begin() const { return { *this }; }
+    Iterator begin() { return { *this }; }
+    EndIterator end() const { return {}; }
 
 private:
     virtual void visit_edges(Visitor& visitor) override;
 
-    OrderedHashMap<Value, Value, ValueTraits> m_entries;
+    size_t m_next_insertion_id { 0 };
+    RedBlackTree<size_t, Value> m_keys;
+    HashMap<Value, Value, ValueTraits> m_entries;
 };
 
 }

Userland/Libraries/LibJS/Runtime/MapIterator.cpp

@@ -18,7 +18,7 @@ MapIterator::MapIterator(Map& map, Object::PropertyKind iteration_kind, Object&
     : Object(prototype)
     , m_map(map)
     , m_iteration_kind(iteration_kind)
-    , m_iterator(map.entries().begin())
+    , m_iterator(static_cast<Map const&>(map).begin())
 {
 }
 

Userland/Libraries/LibJS/Runtime/MapIterator.h

@@ -33,7 +33,7 @@ private:
     Map& m_map;
     bool m_done { false };
     Object::PropertyKind m_iteration_kind;
-    OrderedHashMap<Value, Value, ValueTraits>::IteratorType m_iterator;
+    Map::ConstIterator m_iterator;
 };
 
 }

Userland/Libraries/LibJS/Runtime/MapIteratorPrototype.cpp

@@ -38,8 +38,7 @@ JS_DEFINE_NATIVE_FUNCTION(MapIteratorPrototype::next)
     if (map_iterator->done())
         return create_iterator_result_object(global_object, js_undefined(), true);
 
-    auto& map = map_iterator->map();
+    if (map_iterator->m_iterator.is_end()) {
-    if (map_iterator->m_iterator == map.entries().end()) {
         map_iterator->m_done = true;
         return create_iterator_result_object(global_object, js_undefined(), true);
     }
@@ -50,7 +49,7 @@ JS_DEFINE_NATIVE_FUNCTION(MapIteratorPrototype::next)
     ++map_iterator->m_iterator;
     if (iteration_kind == Object::PropertyKind::Key)
         return create_iterator_result_object(global_object, entry.key, false);
-    else if (iteration_kind == Object::PropertyKind::Value)
+    if (iteration_kind == Object::PropertyKind::Value)
         return create_iterator_result_object(global_object, entry.value, false);
 
     return create_iterator_result_object(global_object, Array::create_from(global_object, { entry.key, entry.value }), false);

Userland/Libraries/LibJS/Runtime/MapPrototype.cpp

@@ -47,7 +47,7 @@ MapPrototype::~MapPrototype()
 JS_DEFINE_NATIVE_FUNCTION(MapPrototype::clear)
 {
     auto* map = TRY(typed_this_object(global_object));
-    map->entries().clear();
+    map->map_clear();
     return js_undefined();
 }
 
@@ -55,7 +55,7 @@ JS_DEFINE_NATIVE_FUNCTION(MapPrototype::clear)
 JS_DEFINE_NATIVE_FUNCTION(MapPrototype::delete_)
 {
     auto* map = TRY(typed_this_object(global_object));
-    return Value(map->entries().remove(vm.argument(0)));
+    return Value(map->map_remove(vm.argument(0)));
 }
 
 // 24.1.3.4 Map.prototype.entries ( ), https://tc39.es/ecma262/#sec-map.prototype.entries
@@ -73,7 +73,7 @@ JS_DEFINE_NATIVE_FUNCTION(MapPrototype::for_each)
     if (!vm.argument(0).is_function())
         return vm.throw_completion<TypeError>(global_object, ErrorType::NotAFunction, vm.argument(0).to_string_without_side_effects());
     auto this_value = vm.this_value(global_object);
-    for (auto& entry : map->entries())
+    for (auto& entry : *map)
         TRY(call(global_object, vm.argument(0).as_function(), vm.argument(1), entry.value, entry.key, this_value));
     return js_undefined();
 }
@@ -82,7 +82,7 @@ JS_DEFINE_NATIVE_FUNCTION(MapPrototype::for_each)
 JS_DEFINE_NATIVE_FUNCTION(MapPrototype::get)
 {
     auto* map = TRY(typed_this_object(global_object));
-    auto result = map->entries().get(vm.argument(0));
+    auto result = map->map_get(vm.argument(0));
     if (!result.has_value())
         return js_undefined();
     return result.value();
@@ -92,8 +92,7 @@ JS_DEFINE_NATIVE_FUNCTION(MapPrototype::get)
 JS_DEFINE_NATIVE_FUNCTION(MapPrototype::has)
 {
     auto* map = TRY(typed_this_object(global_object));
-    auto& entries = map->entries();
+    return map->map_has(vm.argument(0));
-    return Value(entries.find(vm.argument(0)) != entries.end());
 }
 
 // 24.1.3.8 Map.prototype.keys ( ), https://tc39.es/ecma262/#sec-map.prototype.keys
@@ -111,7 +110,7 @@ JS_DEFINE_NATIVE_FUNCTION(MapPrototype::set)
     auto key = vm.argument(0);
     if (key.is_negative_zero())
         key = Value(0);
-    map->entries().set(key, vm.argument(1));
+    map->map_set(key, vm.argument(1));
     return map;
 }
 
@@ -127,7 +126,7 @@ JS_DEFINE_NATIVE_FUNCTION(MapPrototype::values)
 JS_DEFINE_NATIVE_FUNCTION(MapPrototype::size_getter)
 {
     auto* map = TRY(typed_this_object(global_object));
-    return Value(map->entries().size());
+    return Value(map->map_size());
 }
 
 }

Userland/Utilities/js.cpp

@@ -358,11 +358,10 @@ static void print_proxy_object(JS::Object const& object, HashTable<JS::Object*>&
 static void print_map(JS::Object const& object, HashTable<JS::Object*>& seen_objects)
 {
     auto& map = static_cast<JS::Map const&>(object);
-    auto& entries = map.entries();
     print_type("Map");
     js_out(" {{");
     bool first = true;
-    for (auto& entry : entries) {
+    for (auto& entry : map) {
         print_separator(first);
         print_value(entry.key, seen_objects);
         js_out(" => ");