I found this working on

https://codereview.chromium.org/19541010/

The main problem is that if you called Object.getNotifier(obj) on an object, %SetObserved(object) would never get called on it, and thus it would be unobservable (new test added for this).

Additionally, Runtime::SetObserved was asserting obj->IsJSObject() which would fail if called on a proxy.

It just happens that our existing test always called getNotifier() before Object.observe on proxies, and thus we never previously attempted to transition the map of a proxy.

Both issues are now fixed and properly tested.

R=rossberg@chromium.org

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

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

diff --git a/src/object-observe.js b/src/object-observe.js
index a5c12bf..f5e0d9d 100644 (file)
--- a/src/object-observe.js
+++ b/src/object-observe.js
@@ -394,7 +394,10 @@ function ObjectGetNotifier(object) {
   if (ObjectIsFrozen(object)) return null;
 
   var objectInfo = objectInfoMap.get(object);
-  if (IS_UNDEFINED(objectInfo)) objectInfo = CreateObjectInfo(object);
+  if (IS_UNDEFINED(objectInfo)) {
+    objectInfo = CreateObjectInfo(object);
+    %SetIsObserved(object);
+  }
 
   if (IS_NULL(objectInfo.notifier)) {
     objectInfo.notifier = { __proto__: notifierPrototype };

diff --git a/src/runtime.cc b/src/runtime.cc
index 1778418..2fea169 100644 (file)
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -13931,6 +13931,9 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_SetIsObserved) {
     ASSERT(proto->IsJSGlobalObject());
     obj = JSReceiver::cast(proto);
   }
+  if (obj->IsJSProxy())
+    return isolate->heap()->undefined_value();
+
   ASSERT(!(obj->map()->is_observed() && obj->IsJSObject() &&
            JSObject::cast(obj)->HasFastElements()));
   ASSERT(obj->IsJSObject());

diff --git a/test/mjsunit/harmony/object-observe.js b/test/mjsunit/harmony/object-observe.js
index c0524e0..06254ee 100644 (file)
--- a/test/mjsunit/harmony/object-observe.js
+++ b/test/mjsunit/harmony/object-observe.js
@@ -259,6 +259,16 @@ records = undefined;
 Object.deliverChangeRecords(observer.callback);
 observer.assertRecordCount(1);
 
+// Get notifier prior to observing
+reset();
+var obj = {};
+Object.getNotifier(obj);
+Object.observe(obj, observer.callback);
+obj.id = 1;
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: obj, type: 'new', name: 'id' },
+]);
 
 // Observing a continuous stream of changes, while itermittantly unobserving.
 reset();
@@ -783,6 +793,8 @@ observer.assertNotCalled();
 // Test all kinds of objects generically.
 function TestObserveConfigurable(obj, prop) {
   reset();
+  Object.observe(obj, observer.callback);
+  Object.unobserve(obj, observer.callback);
   obj[prop] = 1;
   Object.observe(obj, observer.callback);
   obj[prop] = 2;
@@ -852,6 +864,8 @@ function TestObserveConfigurable(obj, prop) {
 
 function TestObserveNonConfigurable(obj, prop, desc) {
   reset();
+  Object.observe(obj, observer.callback);
+  Object.unobserve(obj, observer.callback);
   obj[prop] = 1;
   Object.observe(obj, observer.callback);
   obj[prop] = 4;