From 92db2105e4551edfdc9d13d372cee5c8504a4bb5 Mon Sep 17 00:00:00 2001
From: "rossberg@chromium.org"
 <rossberg@chromium.org@ce2b1a6d-e550-0410-aec6-3dcde31c8c00>
Date: Tue, 13 Nov 2012 15:53:28 +0000
Subject: [PATCH] Object.deliverChangeRecords should remove the observer from
 activeObservers

To preserve ordering guarantees during end-of-turn delivery, Object.deliverChangeRecords needs to remove the delivered-to observer from the list of active observers.

The added test demonstrates this behavior.

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

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@12951 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
---
 src/object-observe.js              | 15 ++++++++-------
 test/cctest/test-object-observe.cc | 24 ++++++++++++++++++++++++
 2 files changed, 32 insertions(+), 7 deletions(-)

diff --git a/src/object-observe.js b/src/object-observe.js
index 61f8aa3..1b0a491 100644
--- a/src/object-observe.js
+++ b/src/object-observe.js
@@ -35,7 +35,7 @@ if (IS_UNDEFINED(observationState.observerInfoMap)) {
   observationState.observerInfoMap = %CreateObjectHashTable();
   observationState.objectInfoMap = %CreateObjectHashTable();
   observationState.notifierTargetMap = %CreateObjectHashTable();
-  observationState.activeObservers = new InternalArray;
+  observationState.pendingObservers = new InternalArray;
   observationState.observerPriority = 0;
 }
 
@@ -119,7 +119,7 @@ function EnqueueChangeRecord(changeRecord, observers) {
   for (var i = 0; i < observers.length; i++) {
     var observer = observers[i];
     var observerInfo = observerInfoMap.get(observer);
-    observationState.activeObservers[observerInfo.priority] = observer;
+    observationState.pendingObservers[observerInfo.priority] = observer;
     %SetObserverDeliveryPending();
     if (IS_NULL(observerInfo.pendingChangeRecords)) {
       observerInfo.pendingChangeRecords = new InternalArray(changeRecord);
@@ -202,6 +202,7 @@ function DeliverChangeRecordsForObserver(observer) {
     return;
 
   observerInfo.pendingChangeRecords = null;
+  delete observationState.pendingObservers[observerInfo.priority];
   var delivered = [];
   %MoveArrayContents(pendingChangeRecords, delivered);
   try {
@@ -217,11 +218,11 @@ function ObjectDeliverChangeRecords(callback) {
 }
 
 function DeliverChangeRecords() {
-  while (observationState.activeObservers.length) {
-    var activeObservers = observationState.activeObservers;
-    observationState.activeObservers = new InternalArray;
-    for (var i in activeObservers) {
-      DeliverChangeRecordsForObserver(activeObservers[i]);
+  while (observationState.pendingObservers.length) {
+    var pendingObservers = observationState.pendingObservers;
+    observationState.pendingObservers = new InternalArray;
+    for (var i in pendingObservers) {
+      DeliverChangeRecordsForObserver(pendingObservers[i]);
     }
   }
 }
diff --git a/test/cctest/test-object-observe.cc b/test/cctest/test-object-observe.cc
index 374dca4..9decf17 100644
--- a/test/cctest/test-object-observe.cc
+++ b/test/cctest/test-object-observe.cc
@@ -166,6 +166,30 @@ TEST(DeliveryOrderingReentrant) {
   CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());
 }
 
+TEST(DeliveryOrderingDeliverChangeRecords) {
+  HarmonyIsolate isolate;
+  HandleScope scope;
+  LocalContext context;
+  CompileRun(
+      "var obj = {};"
+      "var ordering = [];"
+      "function observer1() { ordering.push(1); if (!obj.b) obj.b = true };"
+      "function observer2() { ordering.push(2); };"
+      "Object.observe(obj, observer1);"
+      "Object.observe(obj, observer2);"
+      "obj.a = 1;"
+      "Object.deliverChangeRecords(observer2);");
+  CHECK_EQ(4, CompileRun("ordering.length")->Int32Value());
+  // First, observer2 is called due to deliverChangeRecords
+  CHECK_EQ(2, CompileRun("ordering[0]")->Int32Value());
+  // Then, observer1 is called when the stack unwinds
+  CHECK_EQ(1, CompileRun("ordering[1]")->Int32Value());
+  // observer1's mutation causes both 1 and 2 to be reactivated,
+  // with 1 having priority.
+  CHECK_EQ(1, CompileRun("ordering[2]")->Int32Value());
+  CHECK_EQ(2, CompileRun("ordering[3]")->Int32Value());
+}
+
 TEST(ObjectHashTableGrowth) {
   HarmonyIsolate isolate;
   HandleScope scope;
-- 
2.7.4