Object.observe: generate change records for indexed properties.

Details:
- Extend ElementAccessors with GetAttributes method.
- Add HasLocalElement, Get[Local]ElementAttribute methods to JSReceiver/JSObject.
- Otherwise, mirror implementation for named properties.

Cannot correctly handle the cases yet where an accessor is redefined or deleted.

Also fixed handling of object info table.

(Based on CL https://codereview.chromium.org/11362115/)

R=verwaest@chromium.org,mstarzinger@chromium.org
BUG=

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

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

src/elements.cc

@@ -564,6 +564,29 @@ class ElementsAccessorBase : public ElementsAccessor {
            : backing_store->GetHeap()->the_hole_value();
   }
 
+  MUST_USE_RESULT virtual PropertyAttributes GetAttributes(
+      Object* receiver,
+      JSObject* holder,
+      uint32_t key,
+      FixedArrayBase* backing_store) {
+    if (backing_store == NULL) {
+      backing_store = holder->elements();
+    }
+    return ElementsAccessorSubclass::GetAttributesImpl(
+        receiver, holder, key, BackingStore::cast(backing_store));
+  }
+
+  MUST_USE_RESULT static PropertyAttributes GetAttributesImpl(
+        Object* receiver,
+        JSObject* obj,
+        uint32_t key,
+        BackingStore* backing_store) {
+    if (key >= ElementsAccessorSubclass::GetCapacityImpl(backing_store)) {
+      return ABSENT;
+    }
+    return backing_store->is_the_hole(key) ? ABSENT : NONE;
+  }
+
   MUST_USE_RESULT virtual MaybeObject* SetLength(JSArray* array,
                                                  Object* length) {
     return ElementsAccessorSubclass::SetLengthImpl(
@@ -1143,6 +1166,16 @@ class ExternalElementsAccessor
         : backing_store->GetHeap()->undefined_value();
   }
 
+  MUST_USE_RESULT static PropertyAttributes GetAttributesImpl(
+      Object* receiver,
+      JSObject* obj,
+      uint32_t key,
+      BackingStore* backing_store) {
+    return
+        key < ExternalElementsAccessorSubclass::GetCapacityImpl(backing_store)
+        ? NONE : ABSENT;
+  }
+
   MUST_USE_RESULT static MaybeObject* SetLengthImpl(
       JSObject* obj,
       Object* length,
@@ -1431,6 +1464,18 @@ class DictionaryElementsAccessor
     return obj->GetHeap()->the_hole_value();
   }
 
+  MUST_USE_RESULT static PropertyAttributes GetAttributesImpl(
+      Object* receiver,
+      JSObject* obj,
+      uint32_t key,
+      SeededNumberDictionary* backing_store) {
+    int entry = backing_store->FindEntry(key);
+    if (entry != SeededNumberDictionary::kNotFound) {
+      return backing_store->DetailsAt(entry).attributes();
+    }
+    return ABSENT;
+  }
+
   static bool HasElementImpl(Object* receiver,
                              JSObject* holder,
                              uint32_t key,
@@ -1490,6 +1535,22 @@ class NonStrictArgumentsElementsAccessor : public ElementsAccessorBase<
     }
   }
 
+  MUST_USE_RESULT static PropertyAttributes GetAttributesImpl(
+      Object* receiver,
+      JSObject* obj,
+      uint32_t key,
+      FixedArray* parameter_map) {
+    Object* probe = GetParameterMapArg(obj, parameter_map, key);
+    if (!probe->IsTheHole()) {
+      return NONE;
+    } else {
+      // If not aliased, check the arguments.
+      FixedArray* arguments = FixedArray::cast(parameter_map->get(1));
+      return ElementsAccessor::ForArray(arguments)->GetAttributes(
+          receiver, obj, key, arguments);
+    }
+  }
+
   MUST_USE_RESULT static MaybeObject* SetLengthImpl(
       JSObject* obj,
       Object* length,

src/elements.h

@@ -71,6 +71,17 @@ class ElementsAccessor {
       uint32_t key,
       FixedArrayBase* backing_store = NULL) = 0;
 
+  // Returns an element's attributes, or ABSENT if there is no such
+  // element. This method doesn't iterate up the prototype chain.  The caller
+  // can optionally pass in the backing store to use for the check, which must
+  // be compatible with the ElementsKind of the ElementsAccessor. If
+  // backing_store is NULL, the holder->elements() is used as the backing store.
+  MUST_USE_RESULT virtual PropertyAttributes GetAttributes(
+      Object* receiver,
+      JSObject* holder,
+      uint32_t key,
+      FixedArrayBase* backing_store = NULL) = 0;
+
   // Modifies the length data property as specified for JSArrays and resizes the
   // underlying backing store accordingly. The method honors the semantics of
   // changing array sizes as defined in EcmaScript 5.1 15.4.5.2, i.e. array that

src/object-observe.js

@@ -45,7 +45,7 @@ InternalObjectHashTable.prototype = {
     return %ObjectHashTableGet(this.table, key);
   },
   set: function(key, value) {
-    return %ObjectHashTableSet(this.table, key, value);
+    this.table = %ObjectHashTableSet(this.table, key, value);
   },
   has: function(key) {
     return %ObjectHashTableHas(this.table, key);

src/objects-inl.h

@@ -5053,6 +5053,11 @@ PropertyAttributes JSReceiver::GetPropertyAttribute(String* key) {
 }
 
 
+PropertyAttributes JSReceiver::GetElementAttribute(uint32_t index) {
+  return GetElementAttributeWithReceiver(this, index, true);
+}
+
+
 // TODO(504): this may be useful in other places too where JSGlobalProxy
 // is used.
 Object* JSObject::BypassGlobalProxy() {
@@ -5077,7 +5082,34 @@ bool JSReceiver::HasElement(uint32_t index) {
   if (IsJSProxy()) {
     return JSProxy::cast(this)->HasElementWithHandler(index);
   }
-  return JSObject::cast(this)->HasElementWithReceiver(this, index);
+  return JSObject::cast(this)->GetElementAttribute(index) != ABSENT;
+}
+
+
+bool JSReceiver::HasLocalElement(uint32_t index) {
+  if (IsJSProxy()) {
+    return JSProxy::cast(this)->HasElementWithHandler(index);
+  }
+  return JSObject::cast(this)->GetLocalElementAttribute(index) != ABSENT;
+}
+
+
+PropertyAttributes JSReceiver::GetElementAttributeWithReceiver(
+    JSReceiver* receiver, uint32_t index, bool continue_search) {
+  if (IsJSProxy()) {
+    return JSProxy::cast(this)->GetElementAttributeWithHandler(receiver, index);
+  }
+  return JSObject::cast(this)->GetElementAttributeWithReceiver(
+      receiver, index, continue_search);
+}
+
+
+PropertyAttributes JSReceiver::GetLocalElementAttribute(uint32_t index) {
+  if (IsJSProxy()) {
+    return JSProxy::cast(this)->GetElementAttributeWithHandler(this, index);
+  }
+  return JSObject::cast(this)->GetElementAttributeWithReceiver(
+      this, index, false);
 }
 
 

src/objects.h

@@ -1483,10 +1483,18 @@ class JSReceiver: public HeapObject {
                                                       String* name);
   PropertyAttributes GetLocalPropertyAttribute(String* name);
 
+  inline PropertyAttributes GetElementAttribute(uint32_t index);
+  inline PropertyAttributes GetElementAttributeWithReceiver(
+      JSReceiver* receiver,
+      uint32_t index,
+      bool continue_search);
+  inline PropertyAttributes GetLocalElementAttribute(uint32_t index);
+
   // Can cause a GC.
   inline bool HasProperty(String* name);
   inline bool HasLocalProperty(String* name);
   inline bool HasElement(uint32_t index);
+  inline bool HasLocalElement(uint32_t index);
 
   // Return the object's prototype (might be Heap::null_value()).
   inline Object* GetPrototype();
@@ -1701,6 +1709,9 @@ class JSObject: public JSReceiver {
       LookupResult* result,
       String* name,
       bool continue_search);
+  PropertyAttributes GetElementAttributeWithReceiver(JSReceiver* receiver,
+                                                     uint32_t index,
+                                                     bool continue_search);
 
   static void DefineAccessor(Handle<JSObject> object,
                              Handle<String> name,
@@ -1822,9 +1833,6 @@ class JSObject: public JSReceiver {
   // be represented as a double and not a Smi.
   bool ShouldConvertToFastDoubleElements(bool* has_smi_only_elements);
 
-  // Tells whether the index'th element is present.
-  bool HasElementWithReceiver(JSReceiver* receiver, uint32_t index);
-
   // Computes the new capacity when expanding the elements of a JSObject.
   static int NewElementsCapacity(int old_capacity) {
     // (old_capacity + 50%) + 16
@@ -1849,9 +1857,7 @@ class JSObject: public JSReceiver {
     DICTIONARY_ELEMENT
   };
 
-  LocalElementType HasLocalElement(uint32_t index);
-
-  bool HasElementWithInterceptor(JSReceiver* receiver, uint32_t index);
+  LocalElementType GetLocalElementType(uint32_t index);
 
   MUST_USE_RESULT MaybeObject* SetFastElement(uint32_t index,
                                               Object* value,
@@ -2209,6 +2215,14 @@ class JSObject: public JSReceiver {
                                                       Object* structure,
                                                       uint32_t index,
                                                       Object* holder);
+  MUST_USE_RESULT PropertyAttributes GetElementAttributeWithInterceptor(
+      JSReceiver* receiver,
+      uint32_t index,
+      bool continue_search);
+  MUST_USE_RESULT PropertyAttributes GetElementAttributeWithoutInterceptor(
+      JSReceiver* receiver,
+      uint32_t index,
+      bool continue_search);
   MUST_USE_RESULT MaybeObject* SetElementWithCallback(
       Object* structure,
       uint32_t index,

src/runtime.cc

@@ -1091,7 +1091,7 @@ static MaybeObject* GetOwnProperty(Isolate* isolate,
   // This could be an element.
   uint32_t index;
   if (name->AsArrayIndex(&index)) {
-    switch (obj->HasLocalElement(index)) {
+    switch (obj->GetLocalElementType(index)) {
       case JSObject::UNDEFINED_ELEMENT:
         return heap->undefined_value();
 
@@ -4699,7 +4699,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_IsPropertyEnumerable) {
 
   uint32_t index;
   if (key->AsArrayIndex(&index)) {
-    JSObject::LocalElementType type = object->HasLocalElement(index);
+    JSObject::LocalElementType type = object->GetLocalElementType(index);
     switch (type) {
       case JSObject::UNDEFINED_ELEMENT:
       case JSObject::STRING_CHARACTER_ELEMENT:
@@ -13291,8 +13291,7 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_ObjectHashTableSet) {
   CONVERT_ARG_HANDLE_CHECKED(ObjectHashTable, table, 0);
   Handle<Object> key = args.at<Object>(1);
   Handle<Object> value = args.at<Object>(2);
-  PutIntoObjectHashTable(table, key, value);
-  return isolate->heap()->undefined_value();
+  return *PutIntoObjectHashTable(table, key, value);
 }
 
 

test/mjsunit/harmony/object-observe.js

@@ -227,19 +227,20 @@ Object.observe(obj, observer.callback);
 Object.observe(obj3, observer.callback);
 Object.observe(obj2, observer.callback);
 Object.notify(obj, {
-  type: 'foo',
+  type: 'foo1',
 });
 Object.notify(obj2, {
-  type: 'foo',
+  type: 'foo2',
 });
 Object.notify(obj3, {
-  type: 'foo',
+  type: 'foo3',
 });
+Object.observe(obj3, observer.callback);
 Object.deliverChangeRecords(observer.callback);
 observer.assertCallbackRecords([
-  { object: obj, type: 'foo' },
-  { object: obj2, type: 'foo' },
-  { object: obj3, type: 'foo' }
+  { object: obj, type: 'foo1' },
+  { object: obj2, type: 'foo2' },
+  { object: obj3, type: 'foo3' }
 ]);
 
 // Observing named properties.
@@ -286,3 +287,48 @@ observer.assertCallbackRecords([
   { object: obj, name: "a", type: "deleted", oldValue: 10 },
   { object: obj, name: "a", type: "new" },
 ]);
+
+// Observing indexed properties.
+reset();
+var obj = {'1': 1}
+Object.observe(obj, observer.callback);
+obj[1] = 2;
+obj[1] = 3;
+delete obj[1];
+obj[1] = 4;
+obj[1] = 4;  // ignored
+obj[1] = 5;
+Object.defineProperty(obj, "1", {value: 6});
+Object.defineProperty(obj, "1", {writable: false});
+obj[1] = 7;  // ignored
+Object.defineProperty(obj, "1", {value: 8});
+Object.defineProperty(obj, "1", {value: 7, writable: true});
+Object.defineProperty(obj, "1", {get: function() {}});
+delete obj[1];
+delete obj[1];
+Object.defineProperty(obj, "1", {get: function() {}, configurable: true});
+Object.defineProperty(obj, "1", {value: 9, writable: true});
+obj[1] = 10;
+delete obj[1];
+Object.defineProperty(obj, "1", {value: 11, configurable: true});
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: obj, name: "1", type: "updated", oldValue: 1 },
+  { object: obj, name: "1", type: "updated", oldValue: 2 },
+  { object: obj, name: "1", type: "deleted", oldValue: 3 },
+  { object: obj, name: "1", type: "new" },
+  { object: obj, name: "1", type: "updated", oldValue: 4 },
+  { object: obj, name: "1", type: "updated", oldValue: 5 },
+  { object: obj, name: "1", type: "reconfigured", oldValue: 6 },
+  { object: obj, name: "1", type: "updated", oldValue: 6 },
+  { object: obj, name: "1", type: "reconfigured", oldValue: 8 },
+  { object: obj, name: "1", type: "reconfigured", oldValue: 7 },
+  // TODO(observe): oldValue should not be present below.
+  { object: obj, name: "1", type: "deleted", oldValue: undefined },
+  { object: obj, name: "1", type: "new" },
+  // TODO(observe): oldValue should be absent below, and type = "reconfigured".
+  { object: obj, name: "1", type: "updated", oldValue: undefined },
+  { object: obj, name: "1", type: "updated", oldValue: 9 },
+  { object: obj, name: "1", type: "deleted", oldValue: 10 },
+  { object: obj, name: "1", type: "new" },
+]);