Array "splice" changeRecords should be emitted after the performChange has completed...

R=rossberg@chromium.org
BUG=

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

Patch from Rafael Weinstein <rafaelw@chromium.org>.

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

diff --git a/src/array.js b/src/array.js
index 5f89ebb..4a7aea5 100644 (file)
--- a/src/array.js
+++ b/src/array.js
@@ -399,14 +399,13 @@ function ObservedArrayPop(n) {
   n--;
   var value = this[n];
 
-  EnqueueSpliceRecord(this, n, [value], 0);
-
   try {
     BeginPerformSplice(this);
     delete this[n];
     this.length = n;
   } finally {
     EndPerformSplice(this);
+    EnqueueSpliceRecord(this, n, [value], 0);
   }
 
   return value;
@@ -441,8 +440,6 @@ function ObservedArrayPush() {
   var n = TO_UINT32(this.length);
   var m = %_ArgumentsLength();
 
-  EnqueueSpliceRecord(this, n, [], m);
-
   try {
     BeginPerformSplice(this);
     for (var i = 0; i < m; i++) {
@@ -451,6 +448,7 @@ function ObservedArrayPush() {
     this.length = n + m;
   } finally {
     EndPerformSplice(this);
+    EnqueueSpliceRecord(this, n, [], m);
   }
 
   return this.length;
@@ -581,14 +579,13 @@ function ArrayReverse() {
 function ObservedArrayShift(len) {
   var first = this[0];
 
-  EnqueueSpliceRecord(this, 0, [first], 0);
-
   try {
     BeginPerformSplice(this);
     SimpleMove(this, 0, 1, len, 0);
     this.length = len - 1;
   } finally {
     EndPerformSplice(this);
+    EnqueueSpliceRecord(this, 0, [first], 0);
   }
 
   return first;
@@ -627,8 +624,6 @@ function ObservedArrayUnshift() {
   var len = TO_UINT32(this.length);
   var num_arguments = %_ArgumentsLength();
 
-  EnqueueSpliceRecord(this, 0, [], num_arguments);
-
   try {
     BeginPerformSplice(this);
     SimpleMove(this, 0, 0, len, num_arguments);
@@ -638,6 +633,7 @@ function ObservedArrayUnshift() {
     this.length = len + num_arguments;
   } finally {
     EndPerformSplice(this);
+    EnqueueSpliceRecord(this, 0, [], num_arguments);
   }
 
   return len + num_arguments;

diff --git a/test/mjsunit/harmony/object-observe.js b/test/mjsunit/harmony/object-observe.js
index e068742..f982a66 100644 (file)
--- a/test/mjsunit/harmony/object-observe.js
+++ b/test/mjsunit/harmony/object-observe.js
@@ -1254,6 +1254,75 @@ observer.assertCallbackRecords([
   { object: array, name: '0', type: 'updated', oldValue: 2 },
 ]);
 
+// Splice emitted after Array mutation methods
+function MockArray(initial, observer) {
+  for (var i = 0; i < initial.length; i++)
+    this[i] = initial[i];
+
+  this.length_ = initial.length;
+  this.observer = observer;
+}
+MockArray.prototype = {
+  set length(length) {
+    Object.getNotifier(this).notify({ type: 'lengthChange' });
+    this.length_ = length;
+    Object.observe(this, this.observer.callback, ['splice']);
+  },
+  get length() {
+    return this.length_;
+  }
+}
+
+reset();
+var array = new MockArray([], observer);
+Object.observe(array, observer.callback, ['lengthChange']);
+Array.prototype.push.call(array, 1);
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: array, type: 'lengthChange' },
+  { object: array, type: 'splice', index: 0, removed: [], addedCount: 1 },
+]);
+
+reset();
+var array = new MockArray([1], observer);
+Object.observe(array, observer.callback, ['lengthChange']);
+Array.prototype.pop.call(array);
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: array, type: 'lengthChange' },
+  { object: array, type: 'splice', index: 0, removed: [1], addedCount: 0 },
+]);
+
+reset();
+var array = new MockArray([1], observer);
+Object.observe(array, observer.callback, ['lengthChange']);
+Array.prototype.shift.call(array);
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: array, type: 'lengthChange' },
+  { object: array, type: 'splice', index: 0, removed: [1], addedCount: 0 },
+]);
+
+reset();
+var array = new MockArray([], observer);
+Object.observe(array, observer.callback, ['lengthChange']);
+Array.prototype.unshift.call(array, 1);
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: array, type: 'lengthChange' },
+  { object: array, type: 'splice', index: 0, removed: [], addedCount: 1 },
+]);
+
+reset();
+var array = new MockArray([0, 1, 2], observer);
+Object.observe(array, observer.callback, ['lengthChange']);
+Array.prototype.splice.call(array, 1, 1);
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: array, type: 'lengthChange' },
+  { object: array, type: 'splice', index: 1, removed: [1], addedCount: 0 },
+]);
+
 //
 // === PLAIN OBJECTS ===
 //