In Array.of and Array.from, fall back to DefineOwnProperty

%AddElement is not intended for objects which are not arrays, and
its behavior may go away with future refactorings. This patch gets
rid of it if the receiver of from or of is not the intrinsic Array
object.

Array.of and Array.from previously papered over failures in calling
[[DefineOwnProperty]] when setting array elements. This patch
makes them lead to exceptions, and adds tests to assert that
the appropriate exceptions are thrown.

BUG=v8:4168
R=adamk
CC=rossberg,verwaest
LOG=Y

Review URL: https://codereview.chromium.org/1181623003

Cr-Commit-Position: refs/heads/master@{#28969}

src/harmony-array.js

@@ -19,6 +19,7 @@ var GetMethod;
 var MathMax;
 var MathMin;
 var ObjectIsFrozen;
+var ObjectDefineProperty;
 
 utils.Import(function(from) {
   GetIterator = from.GetIterator;
@@ -26,6 +27,7 @@ utils.Import(function(from) {
   MathMax = from.MathMax;
   MathMin = from.MathMin;
   ObjectIsFrozen = from.ObjectIsFrozen;
+  ObjectDefineProperty = from.ObjectDefineProperty;
 });
 
 // -------------------------------------------------------------------
@@ -191,6 +193,16 @@ function ArrayFill(value, start, end) {
   return InnerArrayFill(value, start, end, array, length);
 }
 
+function AddArrayElement(constructor, array, i, value) {
+  if (constructor === GlobalArray) {
+    %AddElement(array, i, value);
+  } else {
+    ObjectDefineProperty(array, i, {
+      value: value, writable: true, configurable: true, enumerable: true
+    });
+  }
+}
+
 // ES6, draft 10-14-14, section 22.1.2.1
 function ArrayFrom(arrayLike, mapfn, receiver) {
   var items = $toObject(arrayLike);
@@ -238,8 +250,8 @@ function ArrayFrom(arrayLike, mapfn, receiver) {
       } else {
         mappedValue = nextValue;
       }
-      // TODO(verwaest): This should redefine rather than adding.
-      %AddElement(result, k++, mappedValue);
+      AddArrayElement(this, result, k, mappedValue);
+      k++;
     }
   } else {
     var len = $toLength(items.length);
@@ -252,8 +264,7 @@ function ArrayFrom(arrayLike, mapfn, receiver) {
       } else {
         mappedValue = nextValue;
       }
-      // TODO(verwaest): This should redefine rather than adding.
-      %AddElement(result, k, mappedValue);
+      AddArrayElement(this, result, k, mappedValue);
     }
 
     result.length = k;
@@ -268,8 +279,7 @@ function ArrayOf() {
   // TODO: Implement IsConstructor (ES6 section 7.2.5)
   var array = %IsConstructor(constructor) ? new constructor(length) : [];
   for (var i = 0; i < length; i++) {
-    // TODO(verwaest): This should redefine rather than adding.
-    %AddElement(array, i, %_Arguments(i));
+    AddArrayElement(constructor, array, i, %_Arguments(i));
   }
   array.length = length;
   return array;

src/prologue.js

@@ -174,6 +174,7 @@ function PostNatives(utils) {
     "MathMax",
     "MathMin",
     "ObjectIsFrozen",
+    "ObjectDefineProperty",
     "OwnPropertyKeys",
     "ToNameArray",
   ];

test/mjsunit/harmony/array-from.js

@@ -148,4 +148,32 @@ testArrayFrom(Math.cos, Array);
 testArrayFrom(Math.cos.bind(Math), Array);
 testArrayFrom(boundFn, boundFn);
 
+// Assert that [[DefineOwnProperty]] is used in ArrayFrom, meaning a
+// setter isn't called, and a failed [[DefineOwnProperty]] will throw.
+var setterCalled = 0;
+function exotic() {
+  Object.defineProperty(this,  '0', {
+    get: function() { return 2; },
+    set: function() { setterCalled++; }
+  });
+}
+// Non-configurable properties can't be overwritten with DefineOwnProperty
+assertThrows(function () { Array.from.call(exotic, [1]); }, TypeError);
+// The setter wasn't called
+assertEquals(0, setterCalled);
+
+// Check that array properties defined are writable, enumerable, configurable
+function ordinary() { }
+var x = Array.from.call(ordinary, [2]);
+var xlength = Object.getOwnPropertyDescriptor(x, 'length');
+assertEquals(1, xlength.value);
+assertEquals(true, xlength.writable);
+assertEquals(true, xlength.enumerable);
+assertEquals(true, xlength.configurable);
+var x0 = Object.getOwnPropertyDescriptor(x, 0);
+assertEquals(2, x0.value);
+assertEquals(true, xlength.writable);
+assertEquals(true, xlength.enumerable);
+assertEquals(true, xlength.configurable);
+
 })();

test/mjsunit/harmony/array-of.js

@@ -182,3 +182,33 @@ assertThrows(function() { new Array.of() }, TypeError);  // not a constructor
   assertEquals(instance instanceof boundFn, true);
   assertEquals(Array.isArray(instance), false);
 })();
+
+(function testDefinesOwnProperty() {
+  // Assert that [[DefineOwnProperty]] is used in ArrayFrom, meaning a
+  // setter isn't called, and a failed [[DefineOwnProperty]] will throw.
+  var setterCalled = 0;
+  function exotic() {
+    Object.defineProperty(this,  '0', {
+      get: function() { return 2; },
+      set: function() { setterCalled++; }
+    });
+  }
+  // Non-configurable properties can't be overwritten with DefineOwnProperty
+  assertThrows(function () { Array.of.call(exotic, 1); }, TypeError);
+  // The setter wasn't called
+  assertEquals(0, setterCalled);
+
+  // Check that array properties defined are writable, enumerable, configurable
+  function ordinary() { }
+  var x = Array.of.call(ordinary, 2);
+  var xlength = Object.getOwnPropertyDescriptor(x, 'length');
+  assertEquals(1, xlength.value);
+  assertEquals(true, xlength.writable);
+  assertEquals(true, xlength.enumerable);
+  assertEquals(true, xlength.configurable);
+  var x0 = Object.getOwnPropertyDescriptor(x, 0);
+  assertEquals(2, x0.value);
+  assertEquals(true, xlength.writable);
+  assertEquals(true, xlength.enumerable);
+  assertEquals(true, xlength.configurable);
+})();