Fix corner-case behavior of JSObject::SetPrototype.

src/objects.cc

Index: src/objects.cc
diff --git a/src/objects.cc b/src/objects.cc
index 5382b411e6bb563a58139215f85900b7446f34f1..14e5d3e69f876f2616b60f2ac28af73a1c4d1d79 100644
--- a/src/objects.cc
+++ b/src/objects.cc
@@ -13939,6 +13939,26 @@ Maybe<bool> JSObject::SetPrototypeUnobserved(Handle<JSObject> object,
   // SpiderMonkey behaves this way.
   if (!value->IsJSReceiver() && !value->IsNull()) return Just(true);
 
+  bool dictionary_elements_in_chain =
+      object->map()->DictionaryElementsInPrototypeChainOnly();
+
+  bool all_extensible = object->map()->is_extensible();
+  Handle<JSObject> real_receiver = object;
+  if (from_javascript) {
+    // Find the first object in the chain whose prototype object is not
+    // hidden.
+    PrototypeIterator iter(isolate, real_receiver);
+    while (!iter.IsAtEnd(PrototypeIterator::END_AT_NON_HIDDEN)) {
+      real_receiver = PrototypeIterator::GetCurrent<JSObject>(iter);

[Toon Verwaest, 2015/10/22 13:21:07]

This code is a bit overkill, since this is only relevant for JSGlobalObjects
behind JSGlobalProxies, but I guess it's ok for now.
Real receiver is quite a bad name though...

+      iter.Advance();
+      all_extensible = all_extensible && real_receiver->map()->is_extensible();
+    }
+  }
+  Handle<Map> map(real_receiver->map());

[Toon Verwaest, 2015/10/22 13:21:07]

Slight overkill to allocate a handle to just dereference it once below...

[neis, 2015/10/23 14:26:54]

It's used again further down.

+
+  // Nothing to do if prototype is already set.
+  if (map->prototype() == *value) return Just(true);
+
   // From 8.6.2 Object Internal Methods
   // ...
   // In addition, if [[Extensible]] is false the value of the [[Class]] and
@@ -13947,16 +13967,14 @@ Maybe<bool> JSObject::SetPrototypeUnobserved(Handle<JSObject> object,
   // Implementation specific extensions that modify [[Class]], [[Prototype]]
   // or [[Extensible]] must not violate the invariants defined in the preceding
   // paragraph.
-  if (!object->map()->is_extensible()) {
+  if (!all_extensible) {
     RETURN_FAILURE(isolate, should_throw,
                    NewTypeError(MessageTemplate::kNonExtensibleProto, object));
-    // TODO(neis): Don't fail if new and old prototype happen to be the same.
   }
 
-  // Before we can set the prototype we need to be sure
-  // prototype cycles are prevented.
-  // It is sufficient to validate that the receiver is not in the new prototype
-  // chain.
+  // Before we can set the prototype we need to be sure prototype cycles are
+  // prevented.  It is sufficient to validate that the receiver is not in the
+  // new prototype chain.
   for (PrototypeIterator iter(isolate, *value,
                               PrototypeIterator::START_AT_RECEIVER);
        !iter.IsAtEnd(); iter.Advance()) {
@@ -13967,30 +13985,7 @@ Maybe<bool> JSObject::SetPrototypeUnobserved(Handle<JSObject> object,
     }
   }
 
-  bool dictionary_elements_in_chain =
-      object->map()->DictionaryElementsInPrototypeChainOnly();
-  Handle<JSObject> real_receiver = object;
-
-  if (from_javascript) {
-    // Find the first object in the chain whose prototype object is not
-    // hidden and set the new prototype on that object.
-    PrototypeIterator iter(isolate, real_receiver);
-    while (!iter.IsAtEnd(PrototypeIterator::END_AT_NON_HIDDEN)) {
-      real_receiver = PrototypeIterator::GetCurrent<JSObject>(iter);
-      iter.Advance();
-      if (!real_receiver->map()->is_extensible()) {
-        RETURN_FAILURE(
-            isolate, should_throw,
-            NewTypeError(MessageTemplate::kNonExtensibleProto, object));
-      }
-    }
-  }
-
   // Set the new prototype of the object.
-  Handle<Map> map(real_receiver->map());
-
-  // Nothing to do if prototype is already set.
-  if (map->prototype() == *value) return Just(true);
 
   isolate->UpdateArrayProtectorOnSetPrototype(real_receiver);