Minimal implementation and tests of observable array methods

Bail out of any special-casing in array methods.
Further optimization is possible, but can be left for later.

Review URL: https://codereview.chromium.org/11369151
Patch from Adam Klein <adamk@chromium.org>.

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@12917 ce2b1a6d-e550-0410-aec6-3dcde31c8c00

diff --git a/src/array.js b/src/array.js
index 155d565..37053ce 100644 (file)
--- a/src/array.js
+++ b/src/array.js
@@ -441,8 +441,8 @@ function ArrayPop() {
   }
   n--;
   var value = this[n];
-  this.length = n;
   delete this[n];
+  this.length = n;
   return value;
 }
 
@@ -581,7 +581,7 @@ function ArrayShift() {
 
   var first = this[0];
 
-  if (IS_ARRAY(this)) {
+  if (IS_ARRAY(this) && !%IsObserved(this)) {
     SmartMove(this, 0, 1, len, 0);
   } else {
     SimpleMove(this, 0, 1, len, 0);
@@ -602,7 +602,7 @@ function ArrayUnshift(arg1) {  // length == 1
   var len = TO_UINT32(this.length);
   var num_arguments = %_ArgumentsLength();
 
-  if (IS_ARRAY(this)) {
+  if (IS_ARRAY(this) && !%IsObserved(this)) {
     SmartMove(this, 0, 0, len, num_arguments);
   } else {
     SimpleMove(this, 0, 0, len, num_arguments);
@@ -649,6 +649,7 @@ function ArraySlice(start, end) {
   if (end_i < start_i) return result;
 
   if (IS_ARRAY(this) &&
+      !%IsObserved(this) &&
       (end_i > 1000) &&
       (%EstimateNumberOfElements(this) < end_i)) {
     SmartSlice(this, start_i, end_i - start_i, len, result);
@@ -705,7 +706,9 @@ function ArraySplice(start, delete_count) {
 
   var use_simple_splice = true;
 
-  if (IS_ARRAY(this) && num_additional_args !== del_count) {
+  if (IS_ARRAY(this) &&
+      !%IsObserved(this) &&
+      num_additional_args !== del_count) {
     // If we are only deleting/moving a few things near the end of the
     // array then the simple version is going to be faster, because it
     // doesn't touch most of the array.

diff --git a/src/builtins.cc b/src/builtins.cc
index df70cd4..620e4b3 100644 (file)
--- a/src/builtins.cc
+++ b/src/builtins.cc
@@ -510,6 +510,10 @@ BUILTIN(ArrayPush) {
   FixedArray* elms = FixedArray::cast(elms_obj);
   JSArray* array = JSArray::cast(receiver);
 
+  if (FLAG_harmony_observation && array->map()->is_observed()) {
+    return CallJsBuiltin(isolate, "ArrayPush", args);
+  }
+
   int len = Smi::cast(array->length())->value();
   int to_add = args.length() - 1;
   if (to_add == 0) {
@@ -566,11 +570,15 @@ BUILTIN(ArrayPop) {
   FixedArray* elms = FixedArray::cast(elms_obj);
   JSArray* array = JSArray::cast(receiver);
 
+  if (FLAG_harmony_observation && array->map()->is_observed()) {
+    return CallJsBuiltin(isolate, "ArrayPop", args);
+  }
+
   int len = Smi::cast(array->length())->value();
   if (len == 0) return heap->undefined_value();
 
   // Get top element
-  MaybeObject* top = elms->get(len - 1);
+  Object* top = elms->get(len - 1);
 
   // Set the length.
   array->set_length(Smi::FromInt(len - 1));
@@ -581,9 +589,7 @@ BUILTIN(ArrayPop) {
     return top;
   }
 
-  top = array->GetPrototype()->GetElement(len - 1);
-
-  return top;
+  return array->GetPrototype()->GetElement(len - 1);
 }
 
 
@@ -604,6 +610,10 @@ BUILTIN(ArrayShift) {
   JSArray* array = JSArray::cast(receiver);
   ASSERT(array->HasFastSmiOrObjectElements());
 
+  if (FLAG_harmony_observation && array->map()->is_observed()) {
+    return CallJsBuiltin(isolate, "ArrayShift", args);
+  }
+
   int len = Smi::cast(array->length())->value();
   if (len == 0) return heap->undefined_value();
 
@@ -646,6 +656,10 @@ BUILTIN(ArrayUnshift) {
   JSArray* array = JSArray::cast(receiver);
   ASSERT(array->HasFastSmiOrObjectElements());
 
+  if (FLAG_harmony_observation && array->map()->is_observed()) {
+    return CallJsBuiltin(isolate, "ArrayUnshift", args);
+  }
+
   int len = Smi::cast(array->length())->value();
   int to_add = args.length() - 1;
   int new_length = len + to_add;
@@ -802,6 +816,10 @@ BUILTIN(ArraySplice) {
   JSArray* array = JSArray::cast(receiver);
   ASSERT(array->HasFastSmiOrObjectElements());
 
+  if (FLAG_harmony_observation && array->map()->is_observed()) {
+    return CallJsBuiltin(isolate, "ArraySplice", args);
+  }
+
   int len = Smi::cast(array->length())->value();
 
   int n_arguments = args.length() - 1;

diff --git a/test/mjsunit/harmony/object-observe.js b/test/mjsunit/harmony/object-observe.js
index 962dadd..f05a9ad 100644 (file)
--- a/test/mjsunit/harmony/object-observe.js
+++ b/test/mjsunit/harmony/object-observe.js
@@ -359,7 +359,6 @@ arr3.length = 0;
 Object.defineProperty(arr3, 'length', {value: 5});
 Object.defineProperty(arr3, 'length', {value: 10, writable: false});
 Object.deliverChangeRecords(observer.callback);
-observer.records.forEach(function(r){print(JSON.stringify(r))});
 observer.assertCallbackRecords([
   { object: arr, name: '3', type: 'deleted', oldValue: 'd' },
   // TODO(adamk): oldValue should not be present below
@@ -431,3 +430,147 @@ observer.assertCallbackRecords([
   { object: arr, name: 'length', type: 'updated', oldValue: 201 },
   { object: arr, name: '50', type: 'new' },
 ]);
+
+// Tests for array methods, first on arrays and then on plain objects
+//
+// === ARRAYS ===
+//
+// Push
+reset();
+var array = [1, 2];
+Object.observe(array, observer.callback);
+array.push(3, 4);
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: array, name: '2', type: 'new' },
+  { object: array, name: 'length', type: 'updated', oldValue: 2 },
+  { object: array, name: '3', type: 'new' },
+  { object: array, name: 'length', type: 'updated', oldValue: 3 },
+]);
+
+// Pop
+reset();
+var array = [1, 2];
+Object.observe(array, observer.callback);
+array.pop();
+array.pop();
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: array, name: '1', type: 'deleted', oldValue: 2 },
+  { object: array, name: 'length', type: 'updated', oldValue: 2 },
+  { object: array, name: '0', type: 'deleted', oldValue: 1 },
+  { object: array, name: 'length', type: 'updated', oldValue: 1 },
+]);
+
+// Shift
+reset();
+var array = [1, 2];
+Object.observe(array, observer.callback);
+array.shift();
+array.shift();
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: array, name: '0', type: 'updated', oldValue: 1 },
+  { object: array, name: '1', type: 'deleted', oldValue: 2 },
+  { object: array, name: 'length', type: 'updated', oldValue: 2 },
+  { object: array, name: '0', type: 'deleted', oldValue: 2 },
+  { object: array, name: 'length', type: 'updated', oldValue: 1 },
+]);
+
+// Unshift
+reset();
+var array = [1, 2];
+Object.observe(array, observer.callback);
+array.unshift(3, 4);
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: array, name: '3', type: 'new' },
+  { object: array, name: 'length', type: 'updated', oldValue: 2 },
+  { object: array, name: '2', type: 'new' },
+  { object: array, name: '0', type: 'updated', oldValue: 1 },
+  { object: array, name: '1', type: 'updated', oldValue: 2 },
+]);
+
+// Splice
+reset();
+var array = [1, 2, 3];
+Object.observe(array, observer.callback);
+array.splice(1, 1, 4, 5);
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: array, name: '3', type: 'new' },
+  { object: array, name: 'length', type: 'updated', oldValue: 3 },
+  { object: array, name: '1', type: 'updated', oldValue: 2 },
+  { object: array, name: '2', type: 'updated', oldValue: 3 },
+]);
+
+//
+// === PLAIN OBJECTS ===
+//
+// Push
+reset()
+var array = {0: 1, 1: 2, length: 2}
+Object.observe(array, observer.callback);
+Array.prototype.push.call(array, 3, 4);
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: array, name: '2', type: 'new' },
+  { object: array, name: '3', type: 'new' },
+  { object: array, name: 'length', type: 'updated', oldValue: 2 },
+]);
+
+// Pop
+reset()
+var array = {0: 1, 1: 2, length: 2};
+Object.observe(array, observer.callback);
+Array.prototype.pop.call(array);
+Array.prototype.pop.call(array);
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: array, name: '1', type: 'deleted', oldValue: 2 },
+  { object: array, name: 'length', type: 'updated', oldValue: 2 },
+  { object: array, name: '0', type: 'deleted', oldValue: 1 },
+  { object: array, name: 'length', type: 'updated', oldValue: 1 },
+]);
+
+// Shift
+reset()
+var array = {0: 1, 1: 2, length: 2};
+Object.observe(array, observer.callback);
+Array.prototype.shift.call(array);
+Array.prototype.shift.call(array);
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: array, name: '0', type: 'updated', oldValue: 1 },
+  { object: array, name: '1', type: 'deleted', oldValue: 2 },
+  { object: array, name: 'length', type: 'updated', oldValue: 2 },
+  { object: array, name: '0', type: 'deleted', oldValue: 2 },
+  { object: array, name: 'length', type: 'updated', oldValue: 1 },
+]);
+
+// Unshift
+reset()
+var array = {0: 1, 1: 2, length: 2};
+Object.observe(array, observer.callback);
+Array.prototype.unshift.call(array, 3, 4);
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: array, name: '3', type: 'new' },
+  { object: array, name: '2', type: 'new' },
+  { object: array, name: '0', type: 'updated', oldValue: 1 },
+  { object: array, name: '1', type: 'updated', oldValue: 2 },
+  { object: array, name: 'length', type: 'updated', oldValue: 2 },
+]);
+
+// Splice
+reset()
+var array = {0: 1, 1: 2, 2: 3, length: 3};
+Object.observe(array, observer.callback);
+Array.prototype.splice.call(array, 1, 1, 4, 5);
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: array, name: '3', type: 'new' },
+  { object: array, name: '1', type: 'updated', oldValue: 2 },
+  { object: array, name: '2', type: 'updated', oldValue: 3 },
+  { object: array, name: 'length', type: 'updated', oldValue: 3 },
+]);