Object.observe: generate change records for named properties.

src/objects.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 1659 matching lines @@
 
 MaybeObject* JSObject::AddProperty(String* name,
                                    Object* value,
                                    PropertyAttributes attributes,
                                    StrictModeFlag strict_mode,
                                    JSReceiver::StoreFromKeyed store_mode,
                                    ExtensibilityCheck extensibility_check) {
   ASSERT(!IsJSGlobalProxy());
   Map* map_of_this = map();
   Heap* heap = GetHeap();
+  MaybeObject* ret;

[Toon Verwaest, 2012/11/05 13:33:22]

Can we call this maybe_result for consistency?

[rossberg, 2012/11/05 17:11:08]

Done.

   if (extensibility_check == PERFORM_EXTENSIBILITY_CHECK &&
       !map_of_this->is_extensible()) {
     if (strict_mode == kNonStrictMode) {
       return value;
     } else {
       Handle<Object> args[1] = {Handle<String>(name)};
       return heap->isolate()->Throw(
           *FACTORY->NewTypeError("object_not_extensible",
                                  HandleVector(args, 1)));
     }
   }
+
   if (HasFastProperties()) {
     // Ensure the descriptor array does not get too big.
     if (map_of_this->NumberOfOwnDescriptors() <
         DescriptorArray::kMaxNumberOfDescriptors) {
       if (value->IsJSFunction()) {
-        return AddConstantFunctionProperty(name,
-                                           JSFunction::cast(value),
-                                           attributes);
+        ret = AddConstantFunctionProperty(name,
+                                             JSFunction::cast(value),

[rafaelw, 2012/10/31 14:44:31]

whitespace

[rossberg, 2012/11/05 17:11:08]

Done.

+                                             attributes);
       } else {
-        return AddFastProperty(name, value, attributes, store_mode);
+        ret = AddFastProperty(name, value, attributes, store_mode);
       }
     } else {
       // Normalize the object to prevent very large instance descriptors.
       // This eliminates unwanted N^2 allocation and lookup behavior.
       Object* obj;
-      { MaybeObject* maybe_obj =
-            NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0);
-        if (!maybe_obj->ToObject(&obj)) return maybe_obj;
-      }
+      ret = NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0);
+      if (!ret->ToObject(&obj)) return ret;

[Toon Verwaest, 2012/11/05 13:33:22]

if (ret->IsFailure()) return ret;

I actually prefer:
MaybeObject* maybe_failure = NormalizeProperties(...;
if (maybe_failure->IsFailure()) return maybe_failure;

since ret will be overwritten in the next line anyway. The values seem
independent.

[rossberg, 2012/11/05 17:11:08]

Done.

+      ret = AddSlowProperty(name, value, attributes);
     }
+  } else {
+    ret = AddSlowProperty(name, value, attributes);
   }
-  return AddSlowProperty(name, value, attributes);
+
+  if (FLAG_harmony_observation && map()->is_observed() && !ret->IsFailure()) {
+    this->NotifyObservers("new", name, heap->the_hole_value());
+  }
+
+  return ret;

[rafaelw, 2012/10/31 14:44:31]

Isn't this unsafe? The notifyObservers could have called GC.  Don't you need to:

if (ret->IsFailure()) return ret;
Handle<Object> return_handle(return_value->ToObjectUnchecked());
// call NotifyObservers
return *return_handle;

[Toon Verwaest, 2012/11/05 13:33:22]

Yes. We should not jump into handlified code from non-handlified code. This is
very prone to errors, especially since the caller of the function does not know
that handlified code may cause GC without returning failure, potentially
invalidating all raw pointers in the client code.

Either we have to ensure that no code is handlified for implementing
NotifyObservers, or we'll have to pull it up to the handlified client of this
functionality.

On 2012/10/31 14:44:31, rafaelw wrote:

[rossberg, 2012/11/05 17:11:08]

Indeed.

[rossberg, 2012/11/05 17:11:08]

Most of the methods in question are already documented as "can cause GC", and I
added respective comments to the remaining ones (plus, checked all their call
sites).

As discussed offline, there is no real alternative short of "handlifying all the
code" -- which I agree we should do at some point.

 }
 
 
+void JSObject::NotifyObservers(
+    const char* type_raw, String* name_raw, Object* oldValue_raw) {
+  Isolate* isolate = GetIsolate();
+  HandleScope scope;
+  Handle<String> type = isolate->factory()->LookupAsciiSymbol(type_raw);
+  Handle<JSObject> object(this);
+  Handle<String> name(name_raw);
+  bool threw;
+  if (oldValue_raw->IsTheHole()) {

[rafaelw, 2012/10/31 14:44:31]

nit: This can be cleaner if you make argv a ScopedVector<Handle<Object> >, which
contains either 3 or 4 values based on whether oldValue_raw->IsTheHole().

[rossberg, 2012/11/05 17:11:08]

Well, that would make the code pretty verbose as well. I removed the duplication
using a simpler trick.

+    Handle<Object> args[] = { type, object, name };
+    Execution::Call(Handle<JSFunction>(isolate->observers_notify_change()),
+                    Handle<Object>(isolate->heap()->undefined_value()),
+                    ARRAY_SIZE(args), args,
+                    &threw);
+  } else {
+    Handle<Object> oldValue(oldValue_raw);
+    Handle<Object> args[] = { type, object, name, oldValue };
+    Execution::Call(Handle<JSFunction>(isolate->observers_notify_change()),
+                    Handle<Object>(isolate->heap()->undefined_value()),
+                    ARRAY_SIZE(args), args,
+                    &threw);
+  }
+  ASSERT(!threw);
+}
+
+
 MaybeObject* JSObject::SetPropertyPostInterceptor(
     String* name,
     Object* value,
     PropertyAttributes attributes,
     StrictModeFlag strict_mode,
     ExtensibilityCheck extensibility_check) {
   // Check local property, ignore interceptor.
   LookupResult result(GetIsolate());
   LocalLookupRealNamedProperty(name, &result);
   if (!result.IsFound()) map()->LookupTransition(this, name, &result);
@@ skipping 1127 matching lines @@
     MaybeObject* result_object = self->SetPropertyViaPrototypes(
         *name, *value, attributes, strict_mode, &done);
     if (done) return result_object;
   }
 
   if (!result->IsFound()) {
     // Neither properties nor transitions found.
     return self->AddProperty(
         *name, *value, attributes, strict_mode, store_mode);
   }
+
   if (result->IsProperty() && result->IsReadOnly()) {
     if (strict_mode == kStrictMode) {
       Handle<Object> args[] = { name, self };
       return heap->isolate()->Throw(*heap->isolate()->factory()->NewTypeError(
           "strict_read_only_property", HandleVector(args, ARRAY_SIZE(args))));
     } else {
       return *value;
     }
   }
 
+  Object* oldValue = heap->the_hole_value();
+  if (FLAG_harmony_observation && map()->is_observed()) {
+    // TODO(observe): save oldValue
+  }
+
   // This is a real property that is not read-only, or it is a
   // transition or null descriptor and there are no setters in the prototypes.
+  MaybeObject* ret = *value;
   switch (result->type()) {
     case NORMAL:
-      return self->SetNormalizedProperty(result, *value);
+      ret = self->SetNormalizedProperty(result, *value);
+      break;
     case FIELD:
-      return self->FastPropertyAtPut(result->GetFieldIndex(), *value);
+      ret = self->FastPropertyAtPut(result->GetFieldIndex(), *value);
+      break;
     case CONSTANT_FUNCTION:
       // Only replace the function if necessary.
       if (*value == result->GetConstantFunction()) return *value;
       // Preserve the attributes of this existing property.
       attributes = result->GetAttributes();
-      return self->ConvertDescriptorToField(*name, *value, attributes);
+      ret = self->ConvertDescriptorToField(*name, *value, attributes);
+      break;
     case CALLBACKS: {
       Object* callback_object = result->GetCallbackObject();
       return self->SetPropertyWithCallback(callback_object,
                                            *name,
                                            *value,
                                            result->holder(),
                                            strict_mode);
     }
     case INTERCEPTOR:
-      return self->SetPropertyWithInterceptor(*name,
-                                              *value,
-                                              attributes,
-                                              strict_mode);
+      ret = self->SetPropertyWithInterceptor(*name,
+                                             *value,
+                                             attributes,
+                                             strict_mode);
+      break;
     case TRANSITION: {
       Map* transition_map = result->GetTransitionTarget();
       int descriptor = transition_map->LastAdded();
 
       DescriptorArray* descriptors = transition_map->instance_descriptors();
       PropertyDetails details = descriptors->GetDetails(descriptor);
 
       if (details.type() == FIELD) {
         if (attributes == details.attributes()) {
           int field_index = descriptors->GetFieldIndex(descriptor);
-          return self->AddFastPropertyUsingMap(transition_map,
-                                               *name,
-                                               *value,
-                                               field_index);
+          ret = self->AddFastPropertyUsingMap(transition_map,
+                                              *name,
+                                              *value,
+                                              field_index);
+        } else {
+          ret = self->ConvertDescriptorToField(*name, *value, attributes);
         }
-        return self->ConvertDescriptorToField(*name, *value, attributes);
       } else if (details.type() == CALLBACKS) {
-        return ConvertDescriptorToField(*name, *value, attributes);
+        ret = ConvertDescriptorToField(*name, *value, attributes);
+      } else {
+        ASSERT(details.type() == CONSTANT_FUNCTION);
+
+        Object* constant_function = descriptors->GetValue(descriptor);
+        if (constant_function == *value) {
+          // If the same constant function is being added we can simply
+          // transition to the target map.
+          self->set_map(transition_map);
+          ret = constant_function;
+        } else {
+          // Otherwise, replace with a map transition to a new map with a FIELD,
+          // even if the value is a constant function.
+          ret = ConvertTransitionToMapTransition(
+              result->GetTransitionIndex(), *name, *value, attributes);
+        }
       }
-
-      ASSERT(details.type() == CONSTANT_FUNCTION);
-
-      Object* constant_function = descriptors->GetValue(descriptor);
-      // If the same constant function is being added we can simply
-      // transition to the target map.
-      if (constant_function == *value) {
-        self->set_map(transition_map);
-        return constant_function;
-      }
-      // Otherwise, replace with a map transition to a new map with a FIELD,
-      // even if the value is a constant function.
-      return ConvertTransitionToMapTransition(
-          result->GetTransitionIndex(), *name, *value, attributes);
+      break;
     }
     case HANDLER:
     case NONEXISTENT:
       UNREACHABLE();
-      return *value;
   }
-  UNREACHABLE();  // keep the compiler happy
-  return *value;
+
+  if (FLAG_harmony_observation && map()->is_observed() && !ret->IsFailure()) {
+    this->NotifyObservers("updated", *name, oldValue);
+  }
+
+  return ret;

[rafaelw, 2012/10/31 14:44:31]

same question about returning MaybeObject after invoking script.

[rossberg, 2012/11/05 17:11:08]

Done.

 }
 
 
 // Set a real local property, even if it is READ_ONLY.  If the property is not
 // present, add it with attributes NONE.  This code is an exact clone of
 // SetProperty, with the check for IsReadOnly and the check for a
 // callback setter removed.  The two lines looking up the LookupResult
 // result are also added.  If one of the functions is changed, the other
 // should be.
 // Note that this method cannot be used to set the prototype of a function
@@ skipping 42 matching lines @@
         value,
         attributes);
   }
 
   // Check for accessor in prototype chain removed here in clone.
   if (!result.IsFound()) {
     // Neither properties nor transitions found.
     return AddProperty(name, value, attributes, kNonStrictMode);
   }
 
+  Object* oldValue = isolate->heap()->the_hole_value();
+  if (FLAG_harmony_observation && map()->is_observed()) {
+    // TODO(observe): save oldValue
+  }
+
   // Check of IsReadOnly removed from here in clone.
+  MaybeObject* ret = value;
   switch (result.type()) {
     case NORMAL: {
       PropertyDetails details = PropertyDetails(attributes, NORMAL);
-      return SetNormalizedProperty(name, value, details);
+      ret = SetNormalizedProperty(name, value, details);
+      break;
     }
     case FIELD:
-      return FastPropertyAtPut(result.GetFieldIndex(), value);
+      ret = FastPropertyAtPut(result.GetFieldIndex(), value);
+      break;
     case CONSTANT_FUNCTION:
       // Only replace the function if necessary.
       if (value == result.GetConstantFunction()) return value;
       // Preserve the attributes of this existing property.
       attributes = result.GetAttributes();
-      return ConvertDescriptorToField(name, value, attributes);
+      ret = ConvertDescriptorToField(name, value, attributes);
+      break;
     case CALLBACKS:
     case INTERCEPTOR:
       // Override callback in clone
-      return ConvertDescriptorToField(name, value, attributes);
+      ret = ConvertDescriptorToField(name, value, attributes);
+      break;
     case TRANSITION: {
       Map* transition_map = result.GetTransitionTarget();
       int descriptor = transition_map->LastAdded();
 
       DescriptorArray* descriptors = transition_map->instance_descriptors();
       PropertyDetails details = descriptors->GetDetails(descriptor);
 
       if (details.type() == FIELD) {
         if (attributes == details.attributes()) {
           int field_index = descriptors->GetFieldIndex(descriptor);
-          return AddFastPropertyUsingMap(transition_map,
-                                         name,
-                                         value,
-                                         field_index);
+          ret = AddFastPropertyUsingMap(transition_map,
+                                        name,
+                                        value,
+                                        field_index);
+        } else {
+          ret = ConvertDescriptorToField(name, value, attributes);
         }
-        return ConvertDescriptorToField(name, value, attributes);
       } else if (details.type() == CALLBACKS) {
-        return ConvertDescriptorToField(name, value, attributes);
+        ret = ConvertDescriptorToField(name, value, attributes);
+      } else {
+        ASSERT(details.type() == CONSTANT_FUNCTION);
+
+        // Replace transition to CONSTANT FUNCTION with a map transition to a new
+        // map with a FIELD, even if the value is a function.
+        ret = ConvertTransitionToMapTransition(
+            result.GetTransitionIndex(), name, value, attributes);
       }
-
-      ASSERT(details.type() == CONSTANT_FUNCTION);
-
-      // Replace transition to CONSTANT FUNCTION with a map transition to a new
-      // map with a FIELD, even if the value is a function.
-      return ConvertTransitionToMapTransition(
-          result.GetTransitionIndex(), name, value, attributes);
+      break;
     }
     case HANDLER:
     case NONEXISTENT:
       UNREACHABLE();
   }
-  UNREACHABLE();  // keep the compiler happy
-  return value;
+
+  if (FLAG_harmony_observation && map()->is_observed() && !ret->IsFailure()) {
+    const char* type =
+        attributes == result.GetAttributes() ? "updated" : "reconfigured";
+    this->NotifyObservers(type, name, oldValue);
+  }
+
+  return ret;

[rafaelw, 2012/10/31 14:44:31]

same question about returning MaybeObject

[rossberg, 2012/11/05 17:11:08]

Done.

 }
 
 
 PropertyAttributes JSObject::GetPropertyAttributePostInterceptor(
       JSObject* receiver,
       String* name,
       bool continue_search) {
   // Check local property, ignore interceptor.
   LookupResult result(GetIsolate());
   LocalLookupRealNamedProperty(name, &result);
@@ skipping 883 matching lines @@
   if (IsJSGlobalProxy()) {
     Object* proto = GetPrototype();
     if (proto->IsNull()) return isolate->heap()->false_value();
     ASSERT(proto->IsJSGlobalObject());
     return JSGlobalObject::cast(proto)->DeleteProperty(name, mode);
   }
 
   uint32_t index = 0;
   if (name->AsArrayIndex(&index)) {
     return DeleteElement(index, mode);
+  }
+
+  LookupResult result(isolate);
+  LocalLookup(name, &result);
+  if (!result.IsFound()) return isolate->heap()->true_value();
+  // Ignore attributes if forcing a deletion.
+  if (result.IsDontDelete() && mode != FORCE_DELETION) {
+    if (mode == STRICT_DELETION) {
+      // Deleting a non-configurable property in strict mode.
+      HandleScope scope(isolate);
+      Handle<Object> args[2] = { Handle<Object>(name), Handle<Object>(this) };
+      return isolate->Throw(*isolate->factory()->NewTypeError(
+          "strict_delete_property", HandleVector(args, 2)));
+    }
+    return isolate->heap()->false_value();
+  }
+
+  Object* oldValue = isolate->heap()->the_hole_value();
+  if (FLAG_harmony_observation && map()->is_observed()) {
+    // TODO(observe): save oldValue
+  }
+  MaybeObject* ret;
+
+  // Check for interceptor.
+  if (result.IsInterceptor()) {
+    // Skip interceptor if forcing a deletion.
+    if (mode == FORCE_DELETION) {
+      ret = DeletePropertyPostInterceptor(name, mode);
+    } else {
+      ret = DeletePropertyWithInterceptor(name);
+    }
   } else {
-    LookupResult result(isolate);
-    LocalLookup(name, &result);
-    if (!result.IsFound()) return isolate->heap()->true_value();
-    // Ignore attributes if forcing a deletion.
-    if (result.IsDontDelete() && mode != FORCE_DELETION) {
-      if (mode == STRICT_DELETION) {
-        // Deleting a non-configurable property in strict mode.
-        HandleScope scope(isolate);
-        Handle<Object> args[2] = { Handle<Object>(name), Handle<Object>(this) };
-        return isolate->Throw(*isolate->factory()->NewTypeError(
-            "strict_delete_property", HandleVector(args, 2)));
-      }
-      return isolate->heap()->false_value();
-    }
-    // Check for interceptor.
-    if (result.IsInterceptor()) {
-      // Skip interceptor if forcing a deletion.
-      if (mode == FORCE_DELETION) {
-        return DeletePropertyPostInterceptor(name, mode);
-      }
-      return DeletePropertyWithInterceptor(name);
-    }
     // Normalize object if needed.
     Object* obj;
-    { MaybeObject* maybe_obj =
-          NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0);
-      if (!maybe_obj->ToObject(&obj)) return maybe_obj;
-    }
+    ret = NormalizeProperties(CLEAR_INOBJECT_PROPERTIES, 0);
+    if (!ret->ToObject(&obj)) return ret;
     // Make sure the properties are normalized before removing the entry.
-    return DeleteNormalizedProperty(name, mode);
+    ret = DeleteNormalizedProperty(name, mode);
   }
+
+  if (FLAG_harmony_observation && map()->is_observed() && !ret->IsFailure()) {
+    this->NotifyObservers("deleted", name, oldValue);
+  }
+
+  return ret;

[rafaelw, 2012/10/31 14:44:31]

ditto

[rossberg, 2012/11/05 17:11:08]

Done.

 }
 
 
 MaybeObject* JSReceiver::DeleteElement(uint32_t index, DeleteMode mode) {
   if (IsJSProxy()) {
     return JSProxy::cast(this)->DeleteElementWithHandler(index, mode);
   }
   return JSObject::cast(this)->DeleteElement(index, mode);
 }
 
@@ skipping 607 matching lines @@
 
   // Make sure that the top context does not change when doing callbacks or
   // interceptor calls.
   AssertNoContextChange ncc;
 
   // Try to flatten before operating on the string.
   name->TryFlatten();
 
   if (!CanSetCallback(name)) return isolate->heap()->undefined_value();
 
+  Object* oldValue = isolate->heap()->the_hole_value();
+  bool preexists;
+  if (FLAG_harmony_observation && map()->is_observed()) {
+    LookupResult result(isolate);
+    LocalLookup(name, &result);
+    preexists = result.IsFound();
+    // TODO(observe): save oldValue
+  }
+
   uint32_t index = 0;
-  return name->AsArrayIndex(&index) ?
+  MaybeObject* ret = name->AsArrayIndex(&index) ?
       DefineElementAccessor(index, getter, setter, attributes) :
       DefinePropertyAccessor(name, getter, setter, attributes);
+
+  if (FLAG_harmony_observation && map()->is_observed() && !ret->IsFailure()) {
+    const char* type = preexists ? "reconfigured" : "new";
+    this->NotifyObservers(type, name, oldValue);
+  }
+
+  return ret;

[rafaelw, 2012/10/31 14:44:31]

ditto

[rossberg, 2012/11/05 17:11:08]

Done.

 }
 
 
 static MaybeObject* TryAccessorTransition(JSObject* self,
                                           Map* transitioned_map,
                                           int target_descriptor,
                                           AccessorComponent component,
                                           Object* accessor,
                                           PropertyAttributes attributes) {
   DescriptorArray* descs = transitioned_map->instance_descriptors();
@@ skipping 260 matching lines @@
 MaybeObject* Map::RawCopy(int instance_size) {
   Map* result;
   MaybeObject* maybe_result =
       GetHeap()->AllocateMap(instance_type(), instance_size);
   if (!maybe_result->To(&result)) return maybe_result;
 
   result->set_prototype(prototype());
   result->set_constructor(constructor());
   result->set_bit_field(bit_field());
   result->set_bit_field2(bit_field2());
-  result->set_bit_field3(bit_field3());
   int new_bit_field3 = bit_field3();
   new_bit_field3 = OwnsDescriptors::update(new_bit_field3, true);
   new_bit_field3 = NumberOfOwnDescriptorsBits::update(new_bit_field3, 0);
   new_bit_field3 = EnumLengthBits::update(new_bit_field3, kInvalidEnumCache);
   result->set_bit_field3(new_bit_field3);
   return result;
 }
 
 
 MaybeObject* Map::CopyNormalized(PropertyNormalizationMode mode,
@@ skipping 2602 matching lines @@
                                        PropertyNormalizationMode mode) {
   return
     constructor() == other->constructor() &&
     prototype() == other->prototype() &&
     inobject_properties() == ((mode == CLEAR_INOBJECT_PROPERTIES) ?
                               0 :
                               other->inobject_properties()) &&
     instance_type() == other->instance_type() &&
     bit_field() == other->bit_field() &&
     bit_field2() == other->bit_field2() &&
+    (bit_field3() & IsObserved::kMask) ==
+        (other->bit_field3() & IsObserved::kMask) &&

[Toon Verwaest, 2012/11/05 13:33:22]

Can we just use is_observed() == other->is_observed()?

[rossberg, 2012/11/05 17:11:08]

Done.

     function_with_prototype() == other->function_with_prototype();
 }
 
 
 void JSFunction::JSFunctionIterateBody(int object_size, ObjectVisitor* v) {
   // Iterate over all fields in the body but take care in dealing with
   // the code entry.
   IteratePointers(v, kPropertiesOffset, kCodeEntryOffset);
   v->VisitCodeEntry(this->address() + kCodeEntryOffset);
   IteratePointers(v, kCodeEntryOffset + kPointerSize, object_size);
@@ skipping 5941 matching lines @@
   set_year(Smi::FromInt(year), SKIP_WRITE_BARRIER);
   set_month(Smi::FromInt(month), SKIP_WRITE_BARRIER);
   set_day(Smi::FromInt(day), SKIP_WRITE_BARRIER);
   set_weekday(Smi::FromInt(weekday), SKIP_WRITE_BARRIER);
   set_hour(Smi::FromInt(hour), SKIP_WRITE_BARRIER);
   set_min(Smi::FromInt(min), SKIP_WRITE_BARRIER);
   set_sec(Smi::FromInt(sec), SKIP_WRITE_BARRIER);
 }
 
 } }  // namespace v8::internal