Object.observe: Change semantics of deliverChangeRecords to iterate.

Added test for recursive change generation.

R=yangguo@chromium.org
BUG=v8:2409

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

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

src/object-observe.js

@@ -82,14 +82,11 @@ function ObjectObserve(object, callback) {
   }
 
   var objectInfo = objectInfoMap.get(object);
-  if (IS_UNDEFINED(objectInfo)) {
-    objectInfo = CreateObjectInfo(object);
-  }
+  if (IS_UNDEFINED(objectInfo)) objectInfo = CreateObjectInfo(object);
   %SetIsObserved(object, true);
 
   var changeObservers = objectInfo.changeObservers;
-  if (changeObservers.indexOf(callback) < 0)
-    changeObservers.push(callback);
+  if (changeObservers.indexOf(callback) < 0) changeObservers.push(callback);
 
   return object;
 }
@@ -108,8 +105,7 @@ function ObjectUnobserve(object, callback) {
   var index = changeObservers.indexOf(callback);
   if (index >= 0) {
     changeObservers.splice(index, 1);
-    if (changeObservers.length === 0)
-      %SetIsObserved(object, false);
+    if (changeObservers.length === 0) %SetIsObserved(object, false);
   }
 
   return object;
@@ -141,30 +137,21 @@ function NotifyChange(type, object, name, oldValue) {
 var notifierPrototype = {};
 
 function ObjectNotifierNotify(changeRecord) {
+  var target = notifierTargetMap.get(this);
   if (!IS_SPEC_OBJECT(this))
     throw MakeTypeError("called_on_non_object", ["notify"]);
-
-  var target = notifierTargetMap.get(this);
   if (IS_UNDEFINED(target))
     throw MakeTypeError("observe_notify_non_notifier");
-
   if (!IS_STRING(changeRecord.type))
     throw MakeTypeError("observe_type_non_string");
 
   var objectInfo = objectInfoMap.get(target);
-  if (IS_UNDEFINED(objectInfo))
+  if (IS_UNDEFINED(objectInfo) || objectInfo.changeObservers.length === 0)
     return;
 
-  if (!objectInfo.changeObservers.length)
-    return;
-
-  var newRecord = {
-    object: target
-  };
+  var newRecord = { object: target };
   for (var prop in changeRecord) {
-    if (prop === 'object')
-      continue;
-
+    if (prop === 'object') continue;
     %DefineOrRedefineDataProperty(newRecord, prop, changeRecord[prop],
         READ_ONLY + DONT_DELETE);
   }
@@ -177,17 +164,13 @@ function ObjectGetNotifier(object) {
   if (!IS_SPEC_OBJECT(object))
     throw MakeTypeError("observe_non_object", ["getNotifier"]);
 
-  if (ObjectIsFrozen(object))
-    return null;
+  if (ObjectIsFrozen(object)) return null;
 
   var objectInfo = objectInfoMap.get(object);
-  if (IS_UNDEFINED(objectInfo))
-    objectInfo = CreateObjectInfo(object);
+  if (IS_UNDEFINED(objectInfo)) objectInfo = CreateObjectInfo(object);
 
   if (IS_NULL(objectInfo.notifier)) {
-    objectInfo.notifier = {
-      __proto__: notifierPrototype
-    };
+    objectInfo.notifier = { __proto__: notifierPrototype };
     notifierTargetMap.set(objectInfo.notifier, object);
   }
 
@@ -197,11 +180,11 @@ function ObjectGetNotifier(object) {
 function DeliverChangeRecordsForObserver(observer) {
   var observerInfo = observerInfoMap.get(observer);
   if (IS_UNDEFINED(observerInfo))
-    return;
+    return false;
 
   var pendingChangeRecords = observerInfo.pendingChangeRecords;
   if (IS_NULL(pendingChangeRecords))
-    return;
+    return false;
 
   observerInfo.pendingChangeRecords = null;
   delete observationState.pendingObservers[observerInfo.priority];
@@ -210,13 +193,14 @@ function DeliverChangeRecordsForObserver(observer) {
   try {
     %Call(void 0, delivered, observer);
   } catch (ex) {}
+  return true;
 }
 
 function ObjectDeliverChangeRecords(callback) {
   if (!IS_SPEC_FUNCTION(callback))
     throw MakeTypeError("observe_non_function", ["deliverChangeRecords"]);
 
-  DeliverChangeRecordsForObserver(callback);
+  while (DeliverChangeRecordsForObserver(callback)) {}
 }
 
 function DeliverChangeRecords() {

test/mjsunit/harmony/object-observe.js

@@ -65,8 +65,7 @@ function createObserver() {
     assertCallbackRecords: function(recs) {
       this.assertRecordCount(recs.length);
       for (var i = 0; i < recs.length; i++) {
-        if ('name' in recs[i])
-          recs[i].name = String(recs[i].name);
+        if ('name' in recs[i]) recs[i].name = String(recs[i].name);
         print(i, stringifyNoThrow(this.records[i]), stringifyNoThrow(recs[i]));
         assertSame(this.records[i].object, recs[i].object);
         assertEquals('string', typeof recs[i].type);
@@ -105,17 +104,20 @@ Object.defineProperty(changeRecordWithAccessor, 'name', {
   enumerable: true
 })
 
+
 // Object.observe
 assertThrows(function() { Object.observe("non-object", observer.callback); }, TypeError);
 assertThrows(function() { Object.observe(obj, nonFunction); }, TypeError);
 assertThrows(function() { Object.observe(obj, frozenFunction); }, TypeError);
 assertEquals(obj, Object.observe(obj, observer.callback));
 
+
 // Object.unobserve
 assertThrows(function() { Object.unobserve(4, observer.callback); }, TypeError);
 assertThrows(function() { Object.unobserve(obj, nonFunction); }, TypeError);
 assertEquals(obj, Object.unobserve(obj, observer.callback));
 
+
 // Object.getNotifier
 var notifier = Object.getNotifier(obj);
 assertSame(notifier, Object.getNotifier(obj));
@@ -139,11 +141,13 @@ assertFalse(recordCreated);
 notifier.notify(changeRecordWithAccessor);
 assertFalse(recordCreated);  // not observed yet
 
+
 // Object.deliverChangeRecords
 assertThrows(function() { Object.deliverChangeRecords(nonFunction); }, TypeError);
 
 Object.observe(obj, observer.callback);
 
+
 // notify uses to [[CreateOwnProperty]] to create changeRecord;
 reset();
 var protoExpandoAccessed = false;
@@ -158,6 +162,7 @@ assertFalse(protoExpandoAccessed);
 delete Object.prototype.protoExpando;
 Object.deliverChangeRecords(observer.callback);
 
+
 // Multiple records are delivered.
 reset();
 notifier.notify({
@@ -178,11 +183,13 @@ observer.assertCallbackRecords([
   { object: obj, name: 'bar', type: 'deleted', expando2: 'str' }
 ]);
 
+
 // No delivery takes place if no records are pending
 reset();
 Object.deliverChangeRecords(observer.callback);
 observer.assertNotCalled();
 
+
 // Multiple observation has no effect.
 reset();
 Object.observe(obj, observer.callback);
@@ -193,6 +200,7 @@ Object.getNotifier(obj).notify({
 Object.deliverChangeRecords(observer.callback);
 observer.assertCalled();
 
+
 // Observation can be stopped.
 reset();
 Object.unobserve(obj, observer.callback);
@@ -202,6 +210,7 @@ Object.getNotifier(obj).notify({
 Object.deliverChangeRecords(observer.callback);
 observer.assertNotCalled();
 
+
 // Multiple unobservation has no effect
 reset();
 Object.unobserve(obj, observer.callback);
@@ -212,6 +221,7 @@ Object.getNotifier(obj).notify({
 Object.deliverChangeRecords(observer.callback);
 observer.assertNotCalled();
 
+
 // Re-observation works and only includes changeRecords after of call.
 reset();
 Object.getNotifier(obj).notify({
@@ -225,6 +235,7 @@ records = undefined;
 Object.deliverChangeRecords(observer.callback);
 observer.assertRecordCount(1);
 
+
 // Observing a continuous stream of changes, while itermittantly unobserving.
 reset();
 Object.observe(obj, observer.callback);
@@ -265,6 +276,7 @@ observer.assertCallbackRecords([
   { object: obj, type: 'foo', val: 5 }
 ]);
 
+
 // Observing multiple objects; records appear in order.
 reset();
 var obj2 = {};
@@ -289,6 +301,37 @@ observer.assertCallbackRecords([
   { object: obj3, type: 'foo3' }
 ]);
 
+
+// Recursive observation.
+var obj = {a: 1};
+var callbackCount = 0;
+function recursiveObserver(r) {
+  assertEquals(1, r.length);
+  ++callbackCount;
+  if (r[0].oldValue < 100) ++obj[r[0].name];
+}
+Object.observe(obj, recursiveObserver);
+++obj.a;
+Object.deliverChangeRecords(recursiveObserver);
+assertEquals(100, callbackCount);
+
+var obj1 = {a: 1};
+var obj2 = {a: 1};
+var recordCount = 0;
+function recursiveObserver2(r) {
+  recordCount += r.length;
+  if (r[0].oldValue < 100) {
+    ++obj1.a;
+    ++obj2.a;
+  }
+}
+Object.observe(obj1, recursiveObserver2);
+Object.observe(obj2, recursiveObserver2);
+++obj1.a;
+Object.deliverChangeRecords(recursiveObserver2);
+assertEquals(199, recordCount);
+
+
 // Observing named properties.
 reset();
 var obj = {a: 1}
@@ -340,6 +383,7 @@ observer.assertCallbackRecords([
   { object: obj, name: "a", type: "new" },
 ]);
 
+
 // Observing indexed properties.
 reset();
 var obj = {'1': 1}
@@ -466,8 +510,7 @@ function TestObserveNonConfigurable(obj, prop, desc) {
   Object.defineProperty(obj, prop, {value: 6});
   Object.defineProperty(obj, prop, {value: 6});  // ignored
   Object.defineProperty(obj, prop, {value: 7});
-  Object.defineProperty(obj, prop,
-                        {enumerable: desc.enumerable});  // ignored
+  Object.defineProperty(obj, prop, {enumerable: desc.enumerable});  // ignored
   Object.defineProperty(obj, prop, {writable: false});
   obj[prop] = 7;  // ignored
   Object.deliverChangeRecords(observer.callback);
@@ -541,8 +584,8 @@ var properties = ["a", "1", 1, "length", "prototype", "name", "caller"];
 // Cases that yield non-standard results.
 function blacklisted(obj, prop) {
   return (obj instanceof Int32Array && prop == 1) ||
-    (obj instanceof Int32Array && prop === "length") ||
-    (obj instanceof ArrayBuffer && prop == 1)
+         (obj instanceof Int32Array && prop === "length") ||
+         (obj instanceof ArrayBuffer && prop == 1)
 }
 
 for (var i in objects) for (var j in properties) {
@@ -604,6 +647,7 @@ observer.assertCallbackRecords([
   { object: arr3, name: 'length', type: 'reconfigured', oldValue: 5 },
 ]);
 
+
 // Assignments in loops (checking different IC states).
 reset();
 var obj = {};
@@ -635,6 +679,7 @@ observer.assertCallbackRecords([
   { object: obj, name: "4", type: "new" },
 ]);
 
+
 // Adding elements past the end of an array should notify on length
 reset();
 var arr = [1, 2, 3];
@@ -657,6 +702,7 @@ observer.assertCallbackRecords([
   { object: arr, name: '50', type: 'new' },
 ]);
 
+
 // Tests for array methods, first on arrays and then on plain objects
 //
 // === ARRAYS ===
@@ -730,6 +776,7 @@ observer.assertCallbackRecords([
   { object: array, name: '2', type: 'updated', oldValue: 3 },
 ]);
 
+
 //
 // === PLAIN OBJECTS ===
 //
@@ -836,6 +883,7 @@ observer.assertCallbackRecords([
   { object: obj, name: '__proto__', type: 'prototype', oldValue: null },
 ]);
 
+
 // Function.prototype
 reset();
 var fun = function(){};
@@ -904,7 +952,6 @@ for (var b1 = 0; b1 < 2; ++b1)
     for (var b3 = 0; b3 < 2; ++b3)
       TestFastElements(b1 != 0, b2 != 0, b3 != 0);
 
-
 function TestFastElementsLength(polymorphic, optimize, oldSize, newSize) {
   var setLength = eval(
     "(function setLength(a, n) { a.length = n " +