Add map transition for observed objects

src/runtime.cc

Index: src/runtime.cc
diff --git a/src/runtime.cc b/src/runtime.cc
index 2a473e437179b5518c6a9dc8627d5bf37e668aba..384c2c4d8963c2cd6612576f06443fcc07cba201 100644
--- a/src/runtime.cc
+++ b/src/runtime.cc
@@ -13743,9 +13743,8 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_IsObserved) {
 
 RUNTIME_FUNCTION(MaybeObject*, Runtime_SetIsObserved) {

[rossberg, 2013/07/12 06:58:02]

Drive-by comment: I think the (by now rather non-trivial) logic in this function
should be encapsulated in a method in objects.cc

[Toon Verwaest, 2013/07/12 09:02:56]

I agree.

Also, can we share this logic between SetIsObserved and JSObject::Freeze? It's
almost identical by now. The only differences (eg keeping the observed object
fast vs frozen becoming slow if adding a transition fails) are probably
irrelevant by now? Adding transitions will virtually never fail; so you can just
choose one uniform backup.

By briefly comparing this code with the JSObject::Freeze implementation, it
seems we are not doing any access checks for Object.observe? Shouldn't we do
that as well, to ensure we don't leak information?

On 2013/07/12 06:58:02, rossberg wrote:

[rafaelw, 2013/07/12 21:17:38]

Done.

   SealHandleScope shs(isolate);
-  ASSERT(args.length() == 2);
+  ASSERT(args.length() == 1);
   CONVERT_ARG_CHECKED(JSReceiver, obj, 0);
-  CONVERT_BOOLEAN_ARG_CHECKED(is_observed, 1);
   if (obj->IsJSGlobalProxy()) {
     Object* proto = obj->GetPrototype();
     if (proto->IsNull()) return isolate->heap()->undefined_value();
@@ -13754,21 +13753,37 @@ RUNTIME_FUNCTION(MaybeObject*, Runtime_SetIsObserved) {
   }
   ASSERT(!(obj->map()->is_observed() && obj->IsJSObject() &&
            JSObject::cast(obj)->HasFastElements()));
-  if (obj->map()->is_observed() != is_observed) {
-    if (is_observed && obj->IsJSObject() &&
-        !JSObject::cast(obj)->HasExternalArrayElements()) {
-      // Go to dictionary mode, so that we don't skip map checks.
-      MaybeObject* maybe = JSObject::cast(obj)->NormalizeElements();
-      if (maybe->IsFailure()) return maybe;
-      ASSERT(!JSObject::cast(obj)->HasFastElements());
-    }
-    MaybeObject* maybe = obj->map()->Copy();
-    Map* map;
-    if (!maybe->To(&map)) return maybe;
-    map->set_is_observed(is_observed);
-    obj->set_map(map);
+  if (obj->map()->is_observed())
+    return isolate->heap()->undefined_value();
+
+  JSObject* object = JSObject::cast(obj);
+  Heap* heap = isolate->heap();
+
+  if (!object->HasExternalArrayElements()) {
+    // Go to dictionary mode, so that we don't skip map checks.
+    MaybeObject* maybe = object->NormalizeElements();
+    if (maybe->IsFailure()) return maybe;
+    ASSERT(!object->HasFastElements());
   }
-  return isolate->heap()->undefined_value();
+
+  LookupResult result(isolate);
+  object->map()->LookupTransition(object, heap->observed_symbol(), &result);
+
+  Map* new_map;
+  if (result.IsTransition()) {
+    new_map = result.GetTransitionTarget();
+    ASSERT(new_map->is_observed());
+  } else if (object->map()->CanHaveMoreTransitions()) {
+    MaybeObject* maybe_new_map = object->map()->CopyForObserved();
+    if (!maybe_new_map->To(&new_map)) return maybe_new_map;
+  } else {
+    MaybeObject* maybe_copy = object->map()->Copy();
+    if (!maybe_copy->To(&new_map)) return maybe_copy;
+    new_map->set_is_observed(true);
+  }
+  object->set_map(new_map);
+
+  return heap->undefined_value();
 }