From c1c03d0bd5df0d3b46a559c5b6ed750f22a00b87 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Andr=C3=A9=20Bargull?= <andre.bargull@gmail.com>
Date: Fri, 8 Nov 2019 15:56:45 +0000
Subject: [PATCH] Bug 1583528: Delete properties in ascending order in
 ArrayNativeSort. r=arai

This ensures our native Array.prototype.sort implementation matches the proposed
semantics in <https://github.com/tc39/ecma262/pull/1585>. (Our self-hosted
implementation was already deleting the properties in ascending order.)

Differential Revision: https://phabricator.services.mozilla.com/D52350

--HG--
extra : moz-landing-system : lando
---
 js/src/builtin/Array.cpp                      |  4 +-
 .../Array/sort-delete-ascending-order.js      | 37 +++++++++++++++++++
 2 files changed, 39 insertions(+), 2 deletions(-)
 create mode 100644 js/src/tests/non262/Array/sort-delete-ascending-order.js

diff --git a/js/src/builtin/Array.cpp b/js/src/builtin/Array.cpp
index 6344ee8a36db..70e3b57c7355 100644
--- a/js/src/builtin/Array.cpp
+++ b/js/src/builtin/Array.cpp
@@ -2354,8 +2354,8 @@ bool js::intrinsic_ArrayNativeSort(JSContext* cx, unsigned argc, Value* vp) {
   }
 
   /* Re-create any holes that sorted to the end of the array. */
-  while (len > n) {
-    if (!CheckForInterrupt(cx) || !DeletePropertyOrThrow(cx, obj, --len)) {
+  for (uint32_t i = n; i < len; i++) {
+    if (!CheckForInterrupt(cx) || !DeletePropertyOrThrow(cx, obj, i)) {
       return false;
     }
   }
diff --git a/js/src/tests/non262/Array/sort-delete-ascending-order.js b/js/src/tests/non262/Array/sort-delete-ascending-order.js
new file mode 100644
index 000000000000..99df536df72e
--- /dev/null
+++ b/js/src/tests/non262/Array/sort-delete-ascending-order.js
@@ -0,0 +1,37 @@
+// Calls Array.prototype.sort and tests that properties are deleted in the same order in the
+// native and the self-hosted implementation.
+
+function createProxy() {
+  var deleted = [];
+  var proxy = new Proxy([, , 0], {
+    deleteProperty(t, pk){
+      deleted.push(pk);
+      return delete t[pk];
+    }
+  });
+
+  return {proxy, deleted};
+}
+
+function compareFn(a, b) {
+  return a < b ? -1 : a > b ? 1 : 0;
+}
+
+// Sort an array without a comparator function. This calls the native sort implementation.
+
+var {proxy, deleted} = createProxy();
+
+assertEqArray(deleted, []);
+proxy.sort()
+assertEqArray(deleted, ["1", "2"]);
+
+// Now sort an array with a comparator function. This calls the self-hosted sort implementation.
+
+var {proxy, deleted} = createProxy();
+
+assertEqArray(deleted, []);
+proxy.sort(compareFn);
+assertEqArray(deleted, ["1", "2"]);
+
+if (typeof reportCompare === "function")
+  reportCompare(true, true);