Object.observe: Handle oldValue for elements with accessors properly.

src/elements.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 568 matching lines @@
         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 PropertyType GetType(
+      Object* receiver,
+      JSObject* holder,
+      uint32_t key,
+      FixedArrayBase* backing_store) {
+    if (backing_store == NULL) {
+      backing_store = holder->elements();
+    }
+    return ElementsAccessorSubclass::GetTypeImpl(
+        receiver, holder, key, BackingStore::cast(backing_store));
+  }
+
+  MUST_USE_RESULT static PropertyType GetTypeImpl(
+        Object* receiver,
+        JSObject* obj,
+        uint32_t key,
+        BackingStore* backing_store) {
+    if (key >= ElementsAccessorSubclass::GetCapacityImpl(backing_store)) {
+      return NONEXISTENT;
+    }
+    return backing_store->is_the_hole(key) ? NONEXISTENT : FIELD;
+  }
+
+  MUST_USE_RESULT virtual AccessorPair* GetAccessorPair(
+      Object* receiver,
+      JSObject* holder,
+      uint32_t key,
+      FixedArrayBase* backing_store) {
+    if (backing_store == NULL) {
+      backing_store = holder->elements();
+    }
+    return ElementsAccessorSubclass::GetAccessorPairImpl(
+        receiver, holder, key, BackingStore::cast(backing_store));
+  }
+
+  MUST_USE_RESULT static AccessorPair* GetAccessorPairImpl(
+        Object* receiver,
+        JSObject* obj,
+        uint32_t key,
+        BackingStore* backing_store) {
+    return NULL;
+  }
+
   MUST_USE_RESULT virtual MaybeObject* SetLength(JSArray* array,
                                                  Object* length) {
     return ElementsAccessorSubclass::SetLengthImpl(
         array, length, BackingStore::cast(array->elements()));
   }
 
   MUST_USE_RESULT static MaybeObject* SetLengthImpl(
       JSObject* obj,
       Object* length,
       BackingStore* backing_store);
@@ skipping 566 matching lines @@
         : 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;
+          ? NONE : ABSENT;
+  }
+
+  MUST_USE_RESULT static PropertyType GetTypeImpl(
+      Object* receiver,
+      JSObject* obj,
+      uint32_t key,
+      BackingStore* backing_store) {
+    return
+        key < ExternalElementsAccessorSubclass::GetCapacityImpl(backing_store)
+          ? FIELD : NONEXISTENT;
   }
 
   MUST_USE_RESULT static MaybeObject* SetLengthImpl(
       JSObject* obj,
       Object* length,
       BackingStore* backing_store) {
     // External arrays do not support changing their length.
     UNREACHABLE();
     return obj;
   }
@@ skipping 282 matching lines @@
       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;
   }
 
+  MUST_USE_RESULT static PropertyType GetTypeImpl(
+      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).type();
+    }
+    return NONEXISTENT;
+  }
+
+  MUST_USE_RESULT static AccessorPair* GetAccessorPairImpl(
+      Object* receiver,
+      JSObject* obj,
+      uint32_t key,
+      BackingStore* backing_store) {
+    int entry = backing_store->FindEntry(key);
+    if (entry != SeededNumberDictionary::kNotFound &&
+        backing_store->DetailsAt(entry).type() == CALLBACKS) {
+      return AccessorPair::cast(backing_store->ValueAt(entry));

[Michael Starzinger, 2012/11/14 17:30:39]

This could potentially also be a Foreign callback or a AccessorInfo callback. I
am unsure if that ever happens in any of our embedders, so we could just add a
CHECK (on top of the implicit assertion) here that the value we get out actually
is a AccessorPair. WDYT?

[Sven Panne, 2012/11/15 06:36:32]

The Foreign case is completely under our control and (very probably) doesn't
matter here, but the unhandled AccessorInfo case really makes me nervous: If I
understand things correctly, this would change our external API with unknown
consequences. I would very much prefer avoiding this. If there is really, really
no way around this, the restriction (which one exactly?) should be documented in
a very visible way in our external header.

[rossberg, 2012/11/15 11:25:43]

That was an oversight. Added IsAccessorPair condition, returning NULL otherwise
(analogous to JSObject::GetLocalPropertyAccessorPair).

+    }
+    return NULL;
+  }
+
   static bool HasElementImpl(Object* receiver,
                              JSObject* holder,
                              uint32_t key,
                              SeededNumberDictionary* backing_store) {
     return backing_store->FindEntry(key) !=
         SeededNumberDictionary::kNotFound;
   }
 
   static uint32_t GetKeyForIndexImpl(SeededNumberDictionary* dict,
                                      uint32_t index) {
@@ skipping 55 matching lines @@
     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 PropertyType GetTypeImpl(
+      Object* receiver,
+      JSObject* obj,
+      uint32_t key,
+      FixedArray* parameter_map) {
+    Object* probe = GetParameterMapArg(obj, parameter_map, key);
+    if (!probe->IsTheHole()) {
+      return FIELD;
+    } else {
+      // If not aliased, check the arguments.
+      FixedArray* arguments = FixedArray::cast(parameter_map->get(1));
+      return ElementsAccessor::ForArray(arguments)->GetType(
+          receiver, obj, key, arguments);
+    }
+  }
+
+  MUST_USE_RESULT static AccessorPair* GetAccessorPairImpl(
+      Object* receiver,
+      JSObject* obj,
+      uint32_t key,
+      FixedArray* parameter_map) {
+    Object* probe = GetParameterMapArg(obj, parameter_map, key);
+    if (!probe->IsTheHole()) {
+      return NULL;
+    } else {
+      // If not aliased, check the arguments.
+      FixedArray* arguments = FixedArray::cast(parameter_map->get(1));
+      return ElementsAccessor::ForArray(arguments)->GetAccessorPair(
+          receiver, obj, key, arguments);
+    }
+  }
+
   MUST_USE_RESULT static MaybeObject* SetLengthImpl(
       JSObject* obj,
       Object* length,
       FixedArray* parameter_map) {
     // TODO(mstarzinger): This was never implemented but will be used once we
     // correctly implement [[DefineOwnProperty]] on arrays.
     UNIMPLEMENTED();
     return obj;
   }
 
@@ skipping 184 matching lines @@
   if (!maybe_obj->To(&new_backing_store)) return maybe_obj;
   new_backing_store->set(0, length);
   { MaybeObject* result = array->SetContent(new_backing_store);
     if (result->IsFailure()) return result;
   }
   return array;
 }
 
 
 } }  // namespace v8::internal