Handle Object.observe notifications for setting Array.length

Also handles notification of deleted properties when an array
is truncated by setting length.

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

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

src/accessors.cc

@@ -95,6 +95,47 @@ Object* Accessors::FlattenNumber(Object* value) {
 }
 
 
+static MaybeObject* ArraySetLengthObserved(Isolate* isolate,
+                                           Handle<JSArray> array,
+                                           Handle<Object> new_length_handle) {
+  List<Handle<String> > indices;
+  List<Handle<Object> > old_values;
+  Handle<Object> old_length_handle(array->length(), isolate);
+  uint32_t old_length = 0;
+  CHECK(old_length_handle->ToArrayIndex(&old_length));
+  uint32_t new_length = 0;
+  CHECK(new_length_handle->ToArrayIndex(&new_length));
+  // TODO(adamk): This loop can be very slow for arrays in dictionary mode.
+  // Find another way to iterate over arrays with dictionary elements.
+  for (uint32_t i = old_length - 1; i + 1 > new_length; --i) {
+    PropertyAttributes attributes = array->GetLocalElementAttribute(i);
+    if (attributes == ABSENT) continue;
+    // A non-configurable property will cause the truncation operation to
+    // stop at this index.
+    if (attributes == DONT_DELETE) break;
+    // TODO(adamk): Don't fetch the old value if it's an accessor.
+    old_values.Add(Object::GetElement(array, i));
+    indices.Add(isolate->factory()->Uint32ToString(i));
+  }
+
+  MaybeObject* result = array->SetElementsLength(*new_length_handle);
+  Handle<Object> hresult;
+  if (!result->ToHandle(&hresult)) return result;
+
+  CHECK(array->length()->ToArrayIndex(&new_length));
+  if (old_length != new_length) {
+    for (int i = 0; i < indices.length(); ++i) {
+      JSObject::EnqueueChangeRecord(
+          array, "deleted", indices[i], old_values[i]);
+    }
+    JSObject::EnqueueChangeRecord(
+        array, "updated", isolate->factory()->length_symbol(),
+        old_length_handle);
+  }
+  return *hresult;
+}
+
+
 MaybeObject* Accessors::ArraySetLength(JSObject* object, Object* value, void*) {
   Isolate* isolate = object->GetIsolate();
 
@@ -112,7 +153,7 @@ MaybeObject* Accessors::ArraySetLength(JSObject* object, Object* value, void*) {
   HandleScope scope(isolate);
 
   // Protect raw pointers.
-  Handle<JSObject> object_handle(object, isolate);
+  Handle<JSArray> array_handle(JSArray::cast(object), isolate);
   Handle<Object> value_handle(value, isolate);
 
   bool has_exception;
@@ -122,7 +163,11 @@ MaybeObject* Accessors::ArraySetLength(JSObject* object, Object* value, void*) {
   if (has_exception) return Failure::Exception();
 
   if (uint32_v->Number() == number_v->Number()) {
-    return Handle<JSArray>::cast(object_handle)->SetElementsLength(*uint32_v);
+    if (FLAG_harmony_observation && array_handle->map()->is_observed()) {
+      return ArraySetLengthObserved(isolate, array_handle, uint32_v);
+    } else {
+      return array_handle->SetElementsLength(*uint32_v);
+    }
   }
   return isolate->Throw(
       *isolate->factory()->NewRangeError("invalid_array_length",

src/objects.cc

@@ -1717,20 +1717,21 @@ MaybeObject* JSObject::AddProperty(String* name,
   if (!result->ToHandle(&hresult)) return result;
 
   if (FLAG_harmony_observation && map()->is_observed()) {
-    this->EnqueueChangeRecord(
-        "new", handle(name), handle(heap->the_hole_value()));
+    EnqueueChangeRecord(handle(this), "new", handle(name),
+                        handle(heap->the_hole_value()));
   }
 
   return *hresult;
 }
 
 
-void JSObject::EnqueueChangeRecord(
-    const char* type_str, Handle<String> name, Handle<Object> old_value) {
-  Isolate* isolate = GetIsolate();
+void JSObject::EnqueueChangeRecord(Handle<JSObject> object,
+                                   const char* type_str,
+                                   Handle<String> name,
+                                   Handle<Object> old_value) {
+  Isolate* isolate = object->GetIsolate();
   HandleScope scope;
   Handle<String> type = isolate->factory()->LookupAsciiSymbol(type_str);
-  Handle<JSObject> object(this);
   Handle<Object> args[] = { type, object, name, old_value };
   bool threw;
   Execution::Call(Handle<JSFunction>(isolate->observers_notify_change()),
@@ -2998,13 +2999,13 @@ MaybeObject* JSObject::SetPropertyForResult(LookupResult* lookup,
 
   if (FLAG_harmony_observation && map()->is_observed()) {
     if (lookup->IsTransition()) {
-      self->EnqueueChangeRecord("new", name, old_value);
+      EnqueueChangeRecord(self, "new", name, old_value);
     } else {
       LookupResult new_lookup(self->GetIsolate());
       self->LocalLookup(*name, &new_lookup);
       ASSERT(!new_lookup.GetLazyValue()->IsTheHole());
       if (!new_lookup.GetLazyValue()->SameValue(*old_value)) {
-        self->EnqueueChangeRecord("updated", name, old_value);
+        EnqueueChangeRecord(self, "updated", name, old_value);
       }
     }
   }
@@ -3146,16 +3147,16 @@ MaybeObject* JSObject::SetLocalPropertyIgnoreAttributes(
 
   if (FLAG_harmony_observation && map()->is_observed()) {
     if (lookup.IsTransition()) {
-      self->EnqueueChangeRecord("new", name, old_value);
+      EnqueueChangeRecord(self, "new", name, old_value);
     } else {
       LookupResult new_lookup(isolate);
       self->LocalLookup(*name, &new_lookup);
       ASSERT(!new_lookup.GetLazyValue()->IsTheHole());
       if (old_value->IsTheHole() ||
           new_lookup.GetAttributes() != old_attributes) {
-        self->EnqueueChangeRecord("reconfigured", name, old_value);
+        EnqueueChangeRecord(self, "reconfigured", name, old_value);
       } else if (!new_lookup.GetLazyValue()->SameValue(*old_value)) {
-        self->EnqueueChangeRecord("updated", name, old_value);
+        EnqueueChangeRecord(self, "updated", name, old_value);
       }
     }
   }
@@ -4152,7 +4153,7 @@ MaybeObject* JSObject::DeleteElement(uint32_t index, DeleteMode mode) {
 
   if (FLAG_harmony_observation && map()->is_observed()) {
     if (preexists && !self->HasLocalElement(index))
-      self->EnqueueChangeRecord("deleted", name, old_value);
+      EnqueueChangeRecord(self, "deleted", name, old_value);
   }
 
   return *hresult;
@@ -4239,7 +4240,7 @@ MaybeObject* JSObject::DeleteProperty(String* name, DeleteMode mode) {
 
   if (FLAG_harmony_observation && map()->is_observed()) {
     if (!self->HasLocalProperty(*hname))
-      self->EnqueueChangeRecord("deleted", hname, old_value);
+      EnqueueChangeRecord(self, "deleted", hname, old_value);
   }
 
   return *hresult;
@@ -4906,7 +4907,7 @@ MaybeObject* JSObject::DefineAccessor(String* name_raw,
 
   if (FLAG_harmony_observation && map()->is_observed()) {
     const char* type = preexists ? "reconfigured" : "new";
-    self->EnqueueChangeRecord(type, name, old_value);
+    EnqueueChangeRecord(self, type, name, old_value);
   }
 
   return *hresult;
@@ -10331,13 +10332,13 @@ MaybeObject* JSObject::SetElement(uint32_t index,
   if (FLAG_harmony_observation && map()->is_observed()) {
     PropertyAttributes new_attributes = self->GetLocalPropertyAttribute(*name);
     if (!preexists) {
-      self->EnqueueChangeRecord("new", name, old_value);
+      EnqueueChangeRecord(self, "new", name, old_value);
     } else if (new_attributes != old_attributes || old_value->IsTheHole()) {
-      self->EnqueueChangeRecord("reconfigured", name, old_value);
+      EnqueueChangeRecord(self, "reconfigured", name, old_value);
     } else {
       Handle<Object> newValue = Object::GetElement(self, index);
       if (!newValue->SameValue(*old_value))
-        self->EnqueueChangeRecord("updated", name, old_value);
+        EnqueueChangeRecord(self, "updated", name, old_value);
     }
   }
 

src/objects.h

@@ -2208,6 +2208,12 @@ class JSObject: public JSReceiver {
     static inline int SizeOf(Map* map, HeapObject* object);
   };
 
+  // Enqueue change record for Object.observe. May cause GC.
+  static void EnqueueChangeRecord(Handle<JSObject> object,
+                                  const char* type,
+                                  Handle<String> name,
+                                  Handle<Object> old_value);
+
   // Deliver change records to observers. May cause GC.
   static void DeliverChangeRecords(Isolate* isolate);
 
@@ -2316,11 +2322,6 @@ class JSObject: public JSReceiver {
   MUST_USE_RESULT MaybeObject* SetHiddenPropertiesHashTable(
       Object* value);
 
-  // Enqueue change record for Object.observe. May cause GC.
-  void EnqueueChangeRecord(const char* type,
-                           Handle<String> name,
-                           Handle<Object> old_value);
-
   DISALLOW_IMPLICIT_CONSTRUCTORS(JSObject);
 };
 

test/mjsunit/harmony/object-observe.js

@@ -333,6 +333,41 @@ observer.assertCallbackRecords([
   { object: obj, name: "1", type: "new" },
 ]);
 
+// Observing array length (including truncation)
+reset();
+var arr = ['a', 'b', 'c', 'd'];
+var arr2 = ['alpha', 'beta'];
+var arr3 = ['hello'];
+// TODO(adamk): Enable this test case when it can run in a reasonable
+// amount of time.
+//var slow_arr = new Array(1000000000);
+//slow_arr[500000000] = 'hello';
+Object.defineProperty(arr, '0', {configurable: false});
+Object.defineProperty(arr, '2', {get: function(){}});
+Object.defineProperty(arr2, '0', {get: function(){}, configurable: false});
+Object.observe(arr, observer.callback);
+Object.observe(arr2, observer.callback);
+Object.observe(arr3, observer.callback);
+arr.length = 2;
+arr.length = 0;
+arr.length = 10;
+arr2.length = 0;
+arr2.length = 1; // no change expected
+arr3.length = 0;
+Object.deliverChangeRecords(observer.callback);
+observer.assertCallbackRecords([
+  { object: arr, name: '3', type: 'deleted', oldValue: 'd' },
+  // TODO(adamk): oldValue should not be present below
+  { object: arr, name: '2', type: 'deleted', oldValue: undefined },
+  { object: arr, name: 'length', type: 'updated', oldValue: 4 },
+  { object: arr, name: '1', type: 'deleted', oldValue: 'b' },
+  { object: arr, name: 'length', type: 'updated', oldValue: 2 },
+  { object: arr, name: 'length', type: 'updated', oldValue: 1 },
+  { object: arr2, name: '1', type: 'deleted', oldValue: 'beta' },
+  { object: arr2, name: 'length', type: 'updated', oldValue: 2 },
+  { object: arr3, name: '0', type: 'deleted', oldValue: 'hello' },
+  { object: arr3, name: 'length', type: 'updated', oldValue: 1 },
+]);
 
 // Assignments in loops (checking different IC states).
 reset();