Implement Object.getNotifier() and remove Object.notify()

Updated all tests to use getNotifier or actual object mutation instead of notify, and added tests for new behavior of getNotifier.

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

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

src/messages.js

@@ -96,7 +96,8 @@ var kMessages = {
   observe_non_object:            ["Object.", "%0", " cannot ", "%0", " non-object"],
   observe_non_function:          ["Object.", "%0", " cannot deliver to non-function"],
   observe_callback_frozen:       ["Object.observe cannot deliver to a frozen function object"],
-  observe_type_non_string:       ["Object.notify provided changeRecord with non-string 'type' property"],
+  observe_type_non_string:       ["Invalid changeRecord with non-string 'type' property"],
+  observe_notify_non_notifier:   ["notify called on non-notifier object"],
   // RangeError
   invalid_array_length:          ["Invalid array length"],
   stack_overflow:                ["Maximum call stack size exceeded"],

src/object-observe.js

@@ -34,6 +34,7 @@ var observationState = %GetObservationState();
 if (IS_UNDEFINED(observationState.observerInfoMap)) {
   observationState.observerInfoMap = %CreateObjectHashTable();
   observationState.objectInfoMap = %CreateObjectHashTable();
+  observationState.notifierTargetMap = %CreateObjectHashTable();
   observationState.activeObservers = new InternalArray;
   observationState.observerPriority = 0;
 }
@@ -57,6 +58,16 @@ InternalObjectHashTable.prototype = {
 
 var observerInfoMap = new InternalObjectHashTable('observerInfoMap');
 var objectInfoMap = new InternalObjectHashTable('objectInfoMap');
+var notifierTargetMap = new InternalObjectHashTable('notifierTargetMap');
+
+function CreateObjectInfo(object) {
+  var info = {
+    changeObservers: new InternalArray,
+    notifier: null,
+  };
+  objectInfoMap.set(object, info);
+  return info;
+}
 
 function ObjectObserve(object, callback) {
   if (!IS_SPEC_OBJECT(object))
@@ -75,13 +86,8 @@ function ObjectObserve(object, callback) {
 
   var objectInfo = objectInfoMap.get(object);
   if (IS_UNDEFINED(objectInfo)) {
-    // TODO: setup objectInfo.notifier
-    objectInfo = {
-      changeObservers: new InternalArray(callback)
-    };
-    objectInfoMap.set(object, objectInfo);
+    objectInfo = CreateObjectInfo(object);
     %SetIsObserved(object, true);
-    return;
   }
 
   var changeObservers = objectInfo.changeObservers;
@@ -130,17 +136,28 @@ function NotifyChange(type, object, name, oldValue) {
   EnqueueChangeRecord(changeRecord, objectInfo.changeObservers);
 }
 
-function ObjectNotify(object, changeRecord) {
-  // TODO: notifier needs to be [[THIS]]
+var notifierPrototype = {};
+
+function ObjectNotifierNotify(changeRecord) {
+  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(object);
+  var objectInfo = objectInfoMap.get(target);
   if (IS_UNDEFINED(objectInfo))
     return;
 
+  if (!objectInfo.changeObservers.length)
+    return;
+
   var newRecord = {
-    object: object  // TODO: Needs to be 'object' retrieved from notifier
+    object: target
   };
   for (var prop in changeRecord) {
     if (prop === 'object')
@@ -152,6 +169,27 @@ function ObjectNotify(object, changeRecord) {
   EnqueueChangeRecord(newRecord, objectInfo.changeObservers);
 }
 
+function ObjectGetNotifier(object) {
+  if (!IS_SPEC_OBJECT(object))
+    throw MakeTypeError("observe_non_object", ["getNotifier"]);
+
+  if (InternalObjectIsFrozen(object))
+    return null;
+
+  var objectInfo = objectInfoMap.get(object);
+  if (IS_UNDEFINED(objectInfo))
+    objectInfo = CreateObjectInfo(object);
+
+  if (IS_NULL(objectInfo.notifier)) {
+    objectInfo.notifier = {
+      __proto__: notifierPrototype
+    };
+    notifierTargetMap.set(objectInfo.notifier, object);
+  }
+
+  return objectInfo.notifier;
+}
+
 function DeliverChangeRecordsForObserver(observer) {
   var observerInfo = observerInfoMap.get(observer);
   if (IS_UNDEFINED(observerInfo))
@@ -190,10 +228,13 @@ function SetupObjectObserve() {
   %CheckIsBootstrapping();
   InstallFunctions($Object, DONT_ENUM, $Array(
     "deliverChangeRecords", ObjectDeliverChangeRecords,
-    "notify", ObjectNotify,  // TODO: Remove when getNotifier is implemented.
+    "getNotifier", ObjectGetNotifier,
     "observe", ObjectObserve,
     "unobserve", ObjectUnobserve
   ));
+  InstallFunctions(notifierPrototype, DONT_ENUM, $Array(
+    "notify", ObjectNotifierNotify
+  ));
 }
 
 SetupObjectObserve();

test/cctest/test-object-observe.cc

@@ -64,20 +64,20 @@ TEST(PerIsolateState) {
   Handle<Value> observer = CompileRun("observer");
   Handle<Value> obj = CompileRun("obj");
   Handle<Value> notify_fun1 = CompileRun(
-      "(function() { Object.notify(obj, {type: 'a'}); })");
+      "(function() { obj.foo = 'bar'; })");
   Handle<Value> notify_fun2;
   {
     LocalContext context2;
     context2->Global()->Set(String::New("obj"), obj);
     notify_fun2 = CompileRun(
-        "(function() { Object.notify(obj, {type: 'b'}); })");
+        "(function() { obj.foo = 'baz'; })");
   }
   Handle<Value> notify_fun3;
   {
     LocalContext context3;
     context3->Global()->Set(String::New("obj"), obj);
     notify_fun3 = CompileRun(
-        "(function() { Object.notify(obj, {type: 'c'}); })");
+        "(function() { obj.foo = 'bat'; })");
   }
   {
     LocalContext context4;
@@ -100,7 +100,7 @@ TEST(EndOfMicrotaskDelivery) {
       "var count = 0;"
       "var observer = function(records) { count = records.length };"
       "Object.observe(obj, observer);"
-      "Object.notify(obj, {type: 'a'});");
+      "obj.foo = 'bar';");
   CHECK_EQ(1, CompileRun("count")->Int32Value());
 }
 
@@ -118,7 +118,7 @@ TEST(DeliveryOrdering) {
       "Object.observe(obj1, observer1);"
       "Object.observe(obj1, observer2);"
       "Object.observe(obj1, observer3);"
-      "Object.notify(obj1, {type: 'a'});");
+      "obj1.foo = 'bar';");
   CHECK_EQ(3, CompileRun("ordering.length")->Int32Value());
   CHECK_EQ(1, CompileRun("ordering[0]")->Int32Value());
   CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());
@@ -128,7 +128,7 @@ TEST(DeliveryOrdering) {
       "Object.observe(obj2, observer3);"
       "Object.observe(obj2, observer2);"
       "Object.observe(obj2, observer1);"
-      "Object.notify(obj2, {type: 'b'});");
+      "obj2.foo = 'baz'");
   CHECK_EQ(3, CompileRun("ordering.length")->Int32Value());
   CHECK_EQ(1, CompileRun("ordering[0]")->Int32Value());
   CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());
@@ -146,7 +146,7 @@ TEST(DeliveryOrderingReentrant) {
       "function observer1() { ordering.push(1); };"
       "function observer2() {"
       "  if (!reentered) {"
-      "    Object.notify(obj, {type: 'b'});"
+      "    obj.foo = 'baz';"
       "    reentered = true;"
       "  }"
       "  ordering.push(2);"
@@ -155,7 +155,7 @@ TEST(DeliveryOrderingReentrant) {
       "Object.observe(obj, observer1);"
       "Object.observe(obj, observer2);"
       "Object.observe(obj, observer3);"
-      "Object.notify(obj, {type: 'a'});");
+      "obj.foo = 'bar';");
   CHECK_EQ(5, CompileRun("ordering.length")->Int32Value());
   CHECK_EQ(1, CompileRun("ordering[0]")->Int32Value());
   CHECK_EQ(2, CompileRun("ordering[1]")->Int32Value());

test/mjsunit/harmony/object-observe.js

@@ -101,27 +101,42 @@ assertThrows(function() { Object.observe(obj, frozenFunction); }, TypeError);
 // Object.unobserve
 assertThrows(function() { Object.unobserve(4, observer.callback); }, TypeError);
 
-// Object.notify
-assertThrows(function() { Object.notify(obj, {}); }, TypeError);
-assertThrows(function() { Object.notify(obj, { type: 4 }); }, TypeError);
-assertFalse(recordCreated);
-Object.notify(obj, changeRecordWithAccessor);
+// Object.getNotifier
+var notifier = Object.getNotifier(obj);
+assertSame(notifier, Object.getNotifier(obj));
+assertEquals(null, Object.getNotifier(Object.freeze({})));
+assertFalse(notifier.hasOwnProperty('notify'));
+assertEquals([], Object.keys(notifier));
+var notifyDesc = Object.getOwnPropertyDescriptor(notifier.__proto__, 'notify');
+assertTrue(notifyDesc.configurable);
+assertTrue(notifyDesc.writable);
+assertFalse(notifyDesc.enumerable);
+assertThrows(function() { notifier.notify({}); }, TypeError);
+assertThrows(function() { notifier.notify({ type: 4 }); }, TypeError);
+var notify = notifier.notify;
+assertThrows(function() { notify.call(undefined, { type: 'a' }); }, TypeError);
+assertThrows(function() { notify.call(null, { type: 'a' }); }, TypeError);
+assertThrows(function() { notify.call(5, { type: 'a' }); }, TypeError);
+assertThrows(function() { notify.call('hello', { type: 'a' }); }, TypeError);
+assertThrows(function() { notify.call(false, { type: 'a' }); }, TypeError);
+assertThrows(function() { notify.call({}, { type: 'a' }); }, TypeError);
 assertFalse(recordCreated);
+notifier.notify(changeRecordWithAccessor);
+assertFalse(recordCreated);  // not observed yet
 
 // Object.deliverChangeRecords
 assertThrows(function() { Object.deliverChangeRecords(nonFunction); }, TypeError);
 
 // Multiple records are delivered.
 Object.observe(obj, observer.callback);
-Object.notify(obj, {
-  object: obj,
+notifier.notify({
   type: 'updated',
   name: 'foo',
   expando: 1
 });
 
-Object.notify(obj, {
-  object: obj,
+notifier.notify({
+  object: notifier,  // object property is ignored
   type: 'deleted',
   name: 'bar',
   expando2: 'str'
@@ -141,7 +156,7 @@ observer.assertNotCalled();
 reset();
 Object.observe(obj, observer.callback);
 Object.observe(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
   type: 'foo',
 });
 Object.deliverChangeRecords(observer.callback);
@@ -150,7 +165,7 @@ observer.assertCalled();
 // Observation can be stopped.
 reset();
 Object.unobserve(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
   type: 'foo',
 });
 Object.deliverChangeRecords(observer.callback);
@@ -160,7 +175,7 @@ observer.assertNotCalled();
 reset();
 Object.unobserve(obj, observer.callback);
 Object.unobserve(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
   type: 'foo',
 });
 Object.deliverChangeRecords(observer.callback);
@@ -168,11 +183,11 @@ observer.assertNotCalled();
 
 // Re-observation works and only includes changeRecords after of call.
 reset();
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
   type: 'foo',
 });
 Object.observe(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
   type: 'foo',
 });
 records = undefined;
@@ -182,31 +197,31 @@ observer.assertRecordCount(1);
 // Observing a continuous stream of changes, while itermittantly unobserving.
 reset();
 Object.observe(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
   type: 'foo',
   val: 1
 });
 
 Object.unobserve(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
   type: 'foo',
   val: 2
 });
 
 Object.observe(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
   type: 'foo',
   val: 3
 });
 
 Object.unobserve(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
   type: 'foo',
   val: 4
 });
 
 Object.observe(obj, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
   type: 'foo',
   val: 5
 });
@@ -226,13 +241,13 @@ var obj3 = {}
 Object.observe(obj, observer.callback);
 Object.observe(obj3, observer.callback);
 Object.observe(obj2, observer.callback);
-Object.notify(obj, {
+Object.getNotifier(obj).notify({
   type: 'foo1',
 });
-Object.notify(obj2, {
+Object.getNotifier(obj2).notify({
   type: 'foo2',
 });
-Object.notify(obj3, {
+Object.getNotifier(obj3).notify({
   type: 'foo3',
 });
 Object.observe(obj3, observer.callback);