From 7310713d118deb667ed339c6cf4e08aeea7c67bb Mon Sep 17 00:00:00 2001
From: davidot <david.tuin@gmail.com>
Date: Wed, 7 Jul 2021 14:07:56 +0200
Subject: [PATCH] LibJS: Remove fast array paths in ArrayPrototype::{pop, push}

Unfortunately this fast path leads to problems if Array.prototype is
changed. We probably need to find out some way to optimize these methods
by detecting changes to the prototype or other mechanisms.
---
 .../Libraries/LibJS/Runtime/ArrayPrototype.cpp   | 16 ----------------
 .../Tests/builtins/Array/Array.prototype.pop.js  | 15 +++++++++++++++
 2 files changed, 15 insertions(+), 16 deletions(-)

diff --git a/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp b/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp
index b8e922fb10..c07c02f537 100644
--- a/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp
+++ b/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp
@@ -348,14 +348,6 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayPrototype::push)
     auto* this_object = vm.this_value(global_object).to_object(global_object);
     if (!this_object)
         return {};
-    if (is<Array>(this_object)) {
-        auto* array = static_cast<Array*>(this_object);
-        if (array->length_is_writable()) {
-            for (size_t i = 0; i < vm.argument_count(); ++i)
-                array->indexed_properties().append(vm.argument(i));
-            return Value(static_cast<i32>(array->indexed_properties().array_like_size()));
-        }
-    }
     auto length = length_of_array_like(global_object, *this_object);
     if (vm.exception())
         return {};
@@ -434,14 +426,6 @@ JS_DEFINE_NATIVE_FUNCTION(ArrayPrototype::pop)
     auto* this_object = vm.this_value(global_object).to_object(global_object);
     if (!this_object)
         return {};
-    if (is<Array>(this_object)) {
-        auto* array = static_cast<Array*>(this_object);
-        if (array->length_is_writable()) {
-            if (array->indexed_properties().is_empty())
-                return js_undefined();
-            return array->indexed_properties().take_last(array).value.value_or(js_undefined());
-        }
-    }
     auto length = length_of_array_like(global_object, *this_object);
     if (vm.exception())
         return {};
diff --git a/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.pop.js b/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.pop.js
index d1c34ad60e..a880106fc3 100644
--- a/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.pop.js
+++ b/Userland/Libraries/LibJS/Tests/builtins/Array/Array.prototype.pop.js
@@ -26,4 +26,19 @@ describe("normal behavior", () => {
         expect(a.pop()).toBeUndefined();
         expect(a).toEqual([]);
     });
+
+    test("array with prototype indexed value", () => {
+        Array.prototype[1] = 1;
+
+        var a = [0];
+        a.length = 2;
+        expect(a[1]).toEqual(1);
+        expect(a.pop()).toEqual(1);
+
+        expect(a.length).toEqual(1);
+        expect(a).toEqual([0]);
+        expect(a[1]).toEqual(1);
+
+        delete Array.prototype[1];
+    });
 });