Array.prototype.sort: Unchecked calls to hasOwnProperty and push and sort

test/mjsunit/array-sort.js

 // Copyright 2010 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 386 matching lines @@
 }
 var arr = [o(1), o(2), o(4), o(8), o(16), o(32), o(64), o(128), o(256), o(-0)];
 var global = this;
 function cmpTest(a, b) {
   assertEquals(global, this);
   assertTrue(a instanceof o);
   assertTrue(b instanceof o);
   return a.val - b.val;
 }
 arr.sort(cmpTest);
+
+function TestSortDoesNotDependOnObjectPrototypeHasOwnProperty() {
+  Array.prototype.sort.call({
+    __proto__: { hasOwnProperty: null, 0: 1 },
+    length: 5
+  });
+
+  var arr = new Array(2);
+  Object.defineProperty(arr, 0, { get: function() {}, set: function() {} });
+  arr.hasOwnProperty = null;
+  arr.sort();
+}
+

[wingo, 2014/09/16 12:11:53]

I think the style is to join the invocation with the definition in tests, like

function foo() {
  ...
}
foo();

i.e. no empty line

[Diego Pino, 2014/09/16 13:23:20]

OK, this is actually two tests that both have to do with HasOwnProperty, that's
why I put an empty line. Should I separate each in its own test?

[arv (Not doing code reviews), 2014/09/16 13:41:09]

It is fine to keep them in one function.

I believe Andy was saying that the empty line before the call to
`TestSortDoesNotDependOnObjectPrototypeHasOwnProperty()` should be removed.
Since you're just following the style of this file I think either way is fine.

+TestSortDoesNotDependOnObjectPrototypeHasOwnProperty();
+
+function TestSortDoesNotDependOnArrayPrototypePush() {
+  // InsertionSort is used for arrays which length <= 22
+  var arr = [];
+  for (var i = 0; i < 22; i++) arr[i] = {};
+  Array.prototype.push = function() {
+    fail('Should not call push');
+  };
+  arr.sort();
+
+  // Quicksort is used for arrays which length > 22
+  // Arrays which length > 1000 guarantee GetThirdIndex is executed
+  arr = [];
+  for (var i = 0; i < 2000; ++i) arr[i] = {};
+  arr.sort();
+}
+
+TestSortDoesNotDependOnArrayPrototypePush();
+
+function TestSortDoesNotDependOnArrayPrototypeSort() {
+  var arr = [];
+  for (var i = 0; i < 2000; i++) arr[i] = {};
+  var sortfn = Array.prototype.sort;
+  Array.prototype.sort = function() {
+    fail('Should not call sort');
+  };
+  sortfn.call(arr);
+}
+
+TestSortDoesNotDependOnArrayPrototypeSort();