Delivery logic for Object.observe

This CL has two parts: the first is the logic itself, whereby each observer callback is assigned
a "priority" number the first time it's passed as an observer to Object.observe(), and that
priority is used to determine the order of delivery.

The second part invokes the above logic as part of the API, when the JS stack winds down to
zero.

Added several tests via the API, as the delivery logic isn't testable from a JS test
(it runs after such a test would exit).

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

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

src/bootstrapper.cc

@@ -1418,6 +1418,8 @@ void Genesis::InstallExperimentalNativeFunctions() {
   }
   if (FLAG_harmony_observation) {
     INSTALL_NATIVE(JSFunction, "NotifyChange", observers_notify_change);
+    INSTALL_NATIVE(JSFunction, "DeliverChangeRecords",
+                   observers_deliver_changes);
   }
 }
 

src/contexts.h

@@ -163,6 +163,7 @@ enum BindingFlags {
   V(DERIVED_SET_TRAP_INDEX, JSFunction, derived_set_trap) \
   V(PROXY_ENUMERATE_INDEX, JSFunction, proxy_enumerate) \
   V(OBSERVERS_NOTIFY_CHANGE_INDEX, JSFunction, observers_notify_change) \
+  V(OBSERVERS_DELIVER_CHANGES_INDEX, JSFunction, observers_deliver_changes) \
   V(RANDOM_SEED_INDEX, ByteArray, random_seed)
 
 // JSFunctions are pairs (context, function code), sometimes also called
@@ -291,6 +292,7 @@ class Context: public FixedArray {
     DERIVED_SET_TRAP_INDEX,
     PROXY_ENUMERATE_INDEX,
     OBSERVERS_NOTIFY_CHANGE_INDEX,
+    OBSERVERS_DELIVER_CHANGES_INDEX,
     RANDOM_SEED_INDEX,
 
     // Properties from here are treated as weak references by the full GC.

src/isolate.h

@@ -354,6 +354,7 @@ typedef List<HeapObject*, PreallocatedStorageAllocationPolicy> DebugObjectCache;
   V(uint64_t, enabled_cpu_features, 0)                                         \
   V(CpuProfiler*, cpu_profiler, NULL)                                          \
   V(HeapProfiler*, heap_profiler, NULL)                                        \
+  V(bool, observer_delivery_pending, false)                                    \
   ISOLATE_DEBUGGER_INIT_LIST(V)
 
 class Isolate {

src/object-observe.js

@@ -34,6 +34,8 @@ var observationState = %GetObservationState();
 if (IS_UNDEFINED(observationState.observerInfoMap)) {
   observationState.observerInfoMap = %CreateObjectHashTable();
   observationState.objectInfoMap = %CreateObjectHashTable();
+  observationState.activeObservers = new InternalArray;
+  observationState.observerPriority = 0;
 }
 
 function InternalObjectHashTable(table) {
@@ -65,9 +67,9 @@ function ObjectObserve(object, callback) {
     throw MakeTypeError("observe_callback_frozen");
 
   if (!observerInfoMap.has(callback)) {
-    // TODO: setup observerInfo.priority.
     observerInfoMap.set(callback, {
-      pendingChangeRecords: null
+      pendingChangeRecords: null,
+      priority: observationState.observerPriority++,
     });
   }
 
@@ -109,9 +111,8 @@ function EnqueueChangeRecord(changeRecord, observers) {
   for (var i = 0; i < observers.length; i++) {
     var observer = observers[i];
     var observerInfo = observerInfoMap.get(observer);
-
-    // TODO: "activate" the observer
-
+    observationState.activeObservers[observerInfo.priority] = observer;
+    %SetObserverDeliveryPending();
     if (IS_NULL(observerInfo.pendingChangeRecords)) {
       observerInfo.pendingChangeRecords = new InternalArray(changeRecord);
     } else {
@@ -151,11 +152,8 @@ function ObjectNotify(object, changeRecord) {
   EnqueueChangeRecord(newRecord, objectInfo.changeObservers);
 }
 
-function ObjectDeliverChangeRecords(callback) {
-  if (!IS_SPEC_FUNCTION(callback))
-    throw MakeTypeError("observe_non_function", ["deliverChangeRecords"]);
-
-  var observerInfo = observerInfoMap.get(callback);
+function DeliverChangeRecordsForObserver(observer) {
+  var observerInfo = observerInfoMap.get(observer);
   if (IS_UNDEFINED(observerInfo))
     return;
 
@@ -167,10 +165,27 @@ function ObjectDeliverChangeRecords(callback) {
   var delivered = [];
   %MoveArrayContents(pendingChangeRecords, delivered);
   try {
-    %Call(void 0, delivered, callback);
+    %Call(void 0, delivered, observer);
   } catch (ex) {}
 }
 
+function ObjectDeliverChangeRecords(callback) {
+  if (!IS_SPEC_FUNCTION(callback))
+    throw MakeTypeError("observe_non_function", ["deliverChangeRecords"]);
+
+  DeliverChangeRecordsForObserver(callback);
+}
+
+function DeliverChangeRecords() {
+  while (observationState.activeObservers.length) {
+    var activeObservers = observationState.activeObservers;
+    observationState.activeObservers = new InternalArray;
+    for (var i in activeObservers) {
+      DeliverChangeRecordsForObserver(activeObservers[i]);
+    }
+  }
+}
+
 function SetupObjectObserve() {
   %CheckIsBootstrapping();
   InstallFunctions($Object, DONT_ENUM, $Array(

src/objects.cc

@@ -1741,6 +1741,20 @@ void JSObject::EnqueueChangeRecord(
 }
 
 
+void JSObject::DeliverChangeRecords(Isolate* isolate) {
+  ASSERT(isolate->observer_delivery_pending());
+  bool threw = false;
+  Execution::Call(
+      isolate->observers_deliver_changes(),
+      isolate->factory()->undefined_value(),
+      0,
+      NULL,
+      &threw);
+  ASSERT(!threw);
+  isolate->set_observer_delivery_pending(false);
+}
+
+
 MaybeObject* JSObject::SetPropertyPostInterceptor(
     String* name,
     Object* value,

src/objects.h

@@ -2208,6 +2208,9 @@ class JSObject: public JSReceiver {
     static inline int SizeOf(Map* map, HeapObject* object);
   };
 
+  // Deliver change records to observers. May cause GC.
+  static void DeliverChangeRecords(Isolate* isolate);
+
  private:
   friend class DictionaryElementsAccessor;
 

src/runtime.cc

@@ -13263,6 +13263,13 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_SetIsObserved) {
 }
 
 
+RUNTIME_FUNCTION(MaybeObject*, Runtime_SetObserverDeliveryPending) {
+  ASSERT(args.length() == 0);
+  isolate->set_observer_delivery_pending(true);
+  return isolate->heap()->undefined_value();
+}
+
+
 RUNTIME_FUNCTION(MaybeObject*, Runtime_GetObservationState) {
   ASSERT(args.length() == 0);
   return isolate->heap()->observation_state();

src/runtime.h

@@ -322,6 +322,7 @@ namespace internal {
   /* Harmony observe */ \
   F(IsObserved, 1, 1) \
   F(SetIsObserved, 2, 1) \
+  F(SetObserverDeliveryPending, 0, 1) \
   F(GetObservationState, 0, 1) \
   F(CreateObjectHashTable, 0, 1) \
   F(ObjectHashTableGet, 2, 1) \

src/v8.cc

@@ -38,6 +38,7 @@
 #include "hydrogen.h"
 #include "lithium-allocator.h"
 #include "log.h"
+#include "objects.h"
 #include "once.h"
 #include "platform.h"
 #include "runtime-profiler.h"
@@ -216,14 +217,22 @@ void V8::RemoveCallCompletedCallback(CallCompletedCallback callback) {
 
 
 void V8::FireCallCompletedCallback(Isolate* isolate) {
-  if (call_completed_callbacks_ == NULL) return;
+  bool has_call_completed_callbacks = call_completed_callbacks_ != NULL;
+  bool observer_delivery_pending =
+      FLAG_harmony_observation && isolate->observer_delivery_pending();
+  if (!has_call_completed_callbacks && !observer_delivery_pending) return;
   HandleScopeImplementer* handle_scope_implementer =
       isolate->handle_scope_implementer();
   if (!handle_scope_implementer->CallDepthIsZero()) return;
   // Fire callbacks.  Increase call depth to prevent recursive callbacks.
   handle_scope_implementer->IncrementCallDepth();
-  for (int i = 0; i < call_completed_callbacks_->length(); i++) {
-    call_completed_callbacks_->at(i)();
+  if (observer_delivery_pending) {
+    JSObject::DeliverChangeRecords(isolate);
+  }
+  if (has_call_completed_callbacks) {
+    for (int i = 0; i < call_completed_callbacks_->length(); i++) {
+      call_completed_callbacks_->at(i)();
+    }
   }
   handle_scope_implementer->DecrementCallDepth();
 }

test/cctest/test-object-observe.cc

@@ -60,25 +60,108 @@ TEST(PerIsolateState) {
       "var calls = 0;"
       "var observer = function(records) { count = records.length; calls++ };"
       "var obj = {};"
-      "Object.observe(obj, observer);"
-      "Object.notify(obj, {type: 'a'});");
+      "Object.observe(obj, observer);");
   Handle<Value> observer = CompileRun("observer");
   Handle<Value> obj = CompileRun("obj");
+  Handle<Value> notify_fun1 = CompileRun(
+      "(function() { Object.notify(obj, {type: 'a'}); })");
+  Handle<Value> notify_fun2;
   {
     LocalContext context2;
     context2->Global()->Set(String::New("obj"), obj);
-    CompileRun("Object.notify(obj, {type: 'b'});");
+    notify_fun2 = CompileRun(
+        "(function() { Object.notify(obj, {type: 'b'}); })");
   }
+  Handle<Value> notify_fun3;
   {
     LocalContext context3;
     context3->Global()->Set(String::New("obj"), obj);
-    CompileRun("Object.notify(obj, {type: 'c'});");
+    notify_fun3 = CompileRun(
+        "(function() { Object.notify(obj, {type: 'c'}); })");
   }
   {
     LocalContext context4;
     context4->Global()->Set(String::New("observer"), observer);
-    CompileRun("Object.deliverChangeRecords(observer)");
+    context4->Global()->Set(String::New("fun1"), notify_fun1);
+    context4->Global()->Set(String::New("fun2"), notify_fun2);
+    context4->Global()->Set(String::New("fun3"), notify_fun3);
+    CompileRun("fun1(); fun2(); fun3(); Object.deliverChangeRecords(observer)");
   }
   CHECK_EQ(1, CompileRun("calls")->Int32Value());
   CHECK_EQ(3, CompileRun("count")->Int32Value());
 }
+
+TEST(EndOfMicrotaskDelivery) {
+  HarmonyIsolate isolate;
+  HandleScope scope;
+  LocalContext context;
+  CompileRun(
+      "var obj = {};"
+      "var count = 0;"
+      "var observer = function(records) { count = records.length };"
+      "Object.observe(obj, observer);"
+      "Object.notify(obj, {type: 'a'});");
+  CHECK_EQ(1, CompileRun("count")->Int32Value());
+}
+
+TEST(DeliveryOrdering) {
+  HarmonyIsolate isolate;
+  HandleScope scope;
+  LocalContext context;
+  CompileRun(
+      "var obj1 = {};"
+      "var obj2 = {};"
+      "var ordering = [];"
+      "function observer2() { ordering.push(2); };"
+      "function observer1() { ordering.push(1); };"
+      "function observer3() { ordering.push(3); };"
+      "Object.observe(obj1, observer1);"
+      "Object.observe(obj1, observer2);"
+      "Object.observe(obj1, observer3);"
+      "Object.notify(obj1, {type: 'a'});");
+  CHECK_EQ(3, CompileRun("ordering.length")->Int32Value());
+  CHECK_EQ(1, CompileRun("ordering[0]")->Int32Value());
+  CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());
+  CHECK_EQ(3, CompileRun("ordering[2]")->Int32Value());
+  CompileRun(
+      "ordering = [];"
+      "Object.observe(obj2, observer3);"
+      "Object.observe(obj2, observer2);"
+      "Object.observe(obj2, observer1);"
+      "Object.notify(obj2, {type: 'b'});");
+  CHECK_EQ(3, CompileRun("ordering.length")->Int32Value());
+  CHECK_EQ(1, CompileRun("ordering[0]")->Int32Value());
+  CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());
+  CHECK_EQ(3, CompileRun("ordering[2]")->Int32Value());
+}
+
+TEST(DeliveryOrderingReentrant) {
+  HarmonyIsolate isolate;
+  HandleScope scope;
+  LocalContext context;
+  CompileRun(
+      "var obj = {};"
+      "var reentered = false;"
+      "var ordering = [];"
+      "function observer1() { ordering.push(1); };"
+      "function observer2() {"
+      "  if (!reentered) {"
+      "    Object.notify(obj, {type: 'b'});"
+      "    reentered = true;"
+      "  }"
+      "  ordering.push(2);"
+      "};"
+      "function observer3() { ordering.push(3); };"
+      "Object.observe(obj, observer1);"
+      "Object.observe(obj, observer2);"
+      "Object.observe(obj, observer3);"
+      "Object.notify(obj, {type: 'a'});");
+  CHECK_EQ(5, CompileRun("ordering.length")->Int32Value());
+  CHECK_EQ(1, CompileRun("ordering[0]")->Int32Value());
+  CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());
+  CHECK_EQ(3, CompileRun("ordering[2]")->Int32Value());
+  // Note that we re-deliver to observers 1 and 2, while observer3
+  // already received the second record during the first round.
+  CHECK_EQ(1, CompileRun("ordering[3]")->Int32Value());
+  CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());
+}