Bug 1225396 part 2 - Fix ES6 iterator prototype chains. r=jorendorff

js/public/Class.h

@@ -719,7 +719,7 @@ struct JSClass {
 // application.
 #define JSCLASS_GLOBAL_APPLICATION_SLOTS 5
 #define JSCLASS_GLOBAL_SLOT_COUNT                                             \
-    (JSCLASS_GLOBAL_APPLICATION_SLOTS + JSProto_LIMIT * 3 + 35)
+    (JSCLASS_GLOBAL_APPLICATION_SLOTS + JSProto_LIMIT * 3 + 36)
 #define JSCLASS_GLOBAL_FLAGS_WITH_SLOTS(n)                                    \
     (JSCLASS_IS_GLOBAL | JSCLASS_HAS_RESERVED_SLOTS(JSCLASS_GLOBAL_SLOT_COUNT + (n)))
 #define JSCLASS_GLOBAL_FLAGS                                                  \

js/src/builtin/Array.js

@@ -685,9 +685,6 @@ function CreateArrayIterator(obj, kind) {
     return CreateArrayIteratorAt(obj, kind, 0);
 }
 
-function ArrayIteratorIdentity() {
-    return this;
-}
 
 function ArrayIteratorNext() {
     if (!IsObject(this) || !IsArrayIterator(this)) {

js/src/builtin/MapObject.cpp

@@ -118,7 +118,6 @@ const Class MapIteratorObject::class_ = {
 };
 
 const JSFunctionSpec MapIteratorObject::methods[] = {
-    JS_SELF_HOSTED_SYM_FN(iterator, "IteratorIdentity", 0, 0),
     JS_SELF_HOSTED_FN("next", "MapIteratorNext", 0, 0),
     JS_FS_END
 };
@@ -141,14 +140,12 @@ MapIteratorObject::kind() const
 bool
 GlobalObject::initMapIteratorProto(JSContext* cx, Handle<GlobalObject*> global)
 {
-    Rooted<JSObject*> base(cx, GlobalObject::getOrCreateLegacyIteratorPrototype(cx, global));
+    Rooted<JSObject*> base(cx, GlobalObject::getOrCreateIteratorPrototype(cx, global));
     if (!base)
         return false;
-    Rooted<MapIteratorObject*> proto(cx,
-        NewObjectWithGivenProto<MapIteratorObject>(cx, base));
+    RootedPlainObject proto(cx, NewObjectWithGivenProto<PlainObject>(cx, base));
     if (!proto)
         return false;
-    proto->setSlot(MapIteratorObject::RangeSlot, PrivateValue(nullptr));
     if (!JS_DefineFunctions(cx, proto, MapIteratorObject::methods))
         return false;
     global->setReservedSlot(MAP_ITERATOR_PROTO, ObjectValue(*proto));
@@ -844,7 +841,6 @@ const Class SetIteratorObject::class_ = {
 };
 
 const JSFunctionSpec SetIteratorObject::methods[] = {
-    JS_SELF_HOSTED_SYM_FN(iterator, "IteratorIdentity", 0, 0),
     JS_FN("next", next, 0, 0),
     JS_FS_END
 };
@@ -866,14 +862,12 @@ SetIteratorObject::kind() const
 bool
 GlobalObject::initSetIteratorProto(JSContext* cx, Handle<GlobalObject*> global)
 {
-    Rooted<JSObject*> base(cx, GlobalObject::getOrCreateLegacyIteratorPrototype(cx, global));
+    Rooted<JSObject*> base(cx, GlobalObject::getOrCreateIteratorPrototype(cx, global));
     if (!base)
         return false;
-    Rooted<SetIteratorObject*> proto(cx,
-        NewObjectWithGivenProto<SetIteratorObject>(cx, base));
+    RootedPlainObject proto(cx, NewObjectWithGivenProto<PlainObject>(cx, base));
     if (!proto)
         return false;
-    proto->setSlot(SetIteratorObject::RangeSlot, PrivateValue(nullptr));
     if (!JS_DefineFunctions(cx, proto, SetIteratorObject::methods))
         return false;
     global->setReservedSlot(SET_ITERATOR_PROTO, ObjectValue(*proto));

js/src/builtin/String.js

@@ -199,10 +199,6 @@ function String_iterator() {
     return iterator;
 }
 
-function StringIteratorIdentity() {
-    return this;
-}
-
 function StringIteratorNext() {
     if (!IsObject(this) || !IsStringIterator(this)) {
         return callFunction(CallStringIteratorMethodIfWrapped, this,

js/src/jit-test/tests/collections/iterator-1.js

@@ -1,11 +1,11 @@
-// collection.iterator() returns an Iterator object.
+// collection.iterator() returns an iterator object.
 
 load(libdir + "iteration.js");
 
 function test(obj, name) {
     var iter = obj[Symbol.iterator]();
     assertEq(typeof iter, "object");
-    assertEq(iter instanceof Iterator, true);
+    assertEq(iter instanceof Iterator, false); // Not a legacy Iterator.
     assertEq(iter.toString(), "[object " + obj.constructor.name + " Iterator]");
 }
 

js/src/jit-test/tests/collections/iterator-proto-1.js

@@ -1,13 +1,16 @@
 // All iterators of the same collection type share their immediate prototype.
-// Those prototype objects in turn inherit directly from Iterator.prototype.
+// Those prototype objects in turn inherit directly from %IteratorPrototype%.
 
 load(libdir + "iteration.js");
 
+// Get %IteratorPrototype%.
+var iterProto = Object.getPrototypeOf(Object.getPrototypeOf([][Symbol.iterator]()));
+
 function test(obj0, obj1) {
     var iter0 = obj0[Symbol.iterator](), iter1 = obj1[Symbol.iterator]();
     var proto = Object.getPrototypeOf(iter0);
     assertEq(Object.getPrototypeOf(iter1), proto);
-    assertEq(Object.getPrototypeOf(proto), Iterator.prototype);
+    assertEq(Object.getPrototypeOf(proto), iterProto);
 }
 
 test([], [1]);

js/src/jit-test/tests/collections/iterator-proto-surfaces.js

@@ -3,22 +3,41 @@
 load(libdir + "asserts.js");
 load(libdir + "iteration.js");
 
-function test(constructor) {
-    var proto = Object.getPrototypeOf(new constructor()[Symbol.iterator]());
-    var names = Object.getOwnPropertyNames(proto);
-    names.sort();
-    assertDeepEq(names, ['next']);
-    assertEq(proto.hasOwnProperty(Symbol.iterator), true);
+var iterProto = null;
 
-    var desc = Object.getOwnPropertyDescriptor(proto, 'next');
+function test(constructor) {
+    var iter = new constructor()[Symbol.iterator]();
+    assertDeepEq(Reflect.ownKeys(iter), []);
+
+    // Iterator prototypes only have a .next property.
+    // At least until we support @@toStringTag.
+    var proto1 = Object.getPrototypeOf(iter);
+
+    var names = Reflect.ownKeys(proto1);
+    names.sort();
+    assertDeepEq(Reflect.ownKeys(proto1), ['next']);
+
+    var desc = Object.getOwnPropertyDescriptor(proto1, 'next');
     assertEq(desc.configurable, true);
     assertEq(desc.enumerable, false);
     assertEq(desc.writable, true);
 
-    assertEq(proto[Symbol.iterator](), proto);
-    assertIteratorDone(proto, undefined);
+    // %IteratorPrototype%
+    var proto2 = Object.getPrototypeOf(proto1);
+    assertEq(Object.getPrototypeOf(proto2), Object.prototype);
+    assertEq(Object.prototype.toString.call(proto2), "[object Object]");
+
+    assertDeepEq(Reflect.ownKeys(proto2), [Symbol.iterator]);
+    assertEq(proto2[Symbol.iterator](), proto2);
+
+    // Check there's a single %IteratorPrototype% object.
+    if (iterProto === null)
+	iterProto = proto2;
+    else
+	assertEq(iterProto, proto2);
 }
 
-//test(Array);
+test(Array);
+test(String);
 test(Map);
 test(Set);

js/src/jit-test/tests/for-of/array-iterator-surfaces-2.js

@@ -3,11 +3,11 @@
 load(libdir + "iteration.js");
 
 var proto = Object.getPrototypeOf([][Symbol.iterator]());
-assertEq(Object.getPrototypeOf(proto), Iterator.prototype);
+var iterProto = Object.getPrototypeOf(proto);
 proto = Object.getPrototypeOf([].keys());
-assertEq(Object.getPrototypeOf(proto), Iterator.prototype);
+assertEq(Object.getPrototypeOf(proto), iterProto);
 proto = Object.getPrototypeOf([].entries());
-assertEq(Object.getPrototypeOf(proto), Iterator.prototype);
+assertEq(Object.getPrototypeOf(proto), iterProto);
 
 function check(it) {
     assertEq(typeof it, 'object');

js/src/jit-test/tests/for-of/string-iterator-surfaces.js

@@ -53,15 +53,13 @@ assertBuiltinFunction(String.prototype, Symbol.iterator, 0);
 var iter = ""[Symbol.iterator]();
 var iterProto = Object.getPrototypeOf(iter);
 
-// StringIterator.prototype inherits from Object.prototype
-assertEq(Object.getPrototypeOf(iterProto), Object.prototype);
+// StringIterator.prototype inherits from %IteratorPrototype%. Check it's the
+// same object as %ArrayIteratorPrototype%'s proto.
+assertEq(Object.getPrototypeOf(iterProto),
+         Object.getPrototypeOf(Object.getPrototypeOf([][Symbol.iterator]())));
 
 // Own properties for StringIterator.prototype: "next"
 arraysEqual(Object.getOwnPropertyNames(iterProto).sort(), ["next"]);
-assertEq(iterProto.hasOwnProperty(Symbol.iterator), true);
-
-// StringIterator.prototype[@@iterator] is a built-in function
-assertBuiltinFunction(iterProto, Symbol.iterator, 0);
 
 // StringIterator.prototype.next is a built-in function
 assertBuiltinFunction(iterProto, "next", 0);

js/src/jsapi.cpp

@@ -1225,7 +1225,7 @@ JS_GetIteratorPrototype(JSContext* cx)
 {
     CHECK_REQUEST(cx);
     Rooted<GlobalObject*> global(cx, cx->global());
-    return GlobalObject::getOrCreateLegacyIteratorPrototype(cx, global);
+    return GlobalObject::getOrCreateIteratorPrototype(cx, global);
 }
 
 JS_PUBLIC_API(JSObject*)

js/src/jsiter.cpp

@@ -1109,7 +1109,6 @@ const Class ArrayIteratorObject::class_ = {
 };
 
 static const JSFunctionSpec array_iterator_methods[] = {
-    JS_SELF_HOSTED_SYM_FN(iterator, "ArrayIteratorIdentity", 0, 0),
     JS_SELF_HOSTED_FN("next", "ArrayIteratorNext", 0, 0),
     JS_FS_END
 };
@@ -1131,7 +1130,6 @@ const Class StringIteratorObject::class_ = {
 };
 
 static const JSFunctionSpec string_iterator_methods[] = {
-    JS_SELF_HOSTED_SYM_FN(iterator, "StringIteratorIdentity", 0, 0),
     JS_SELF_HOSTED_FN("next", "StringIteratorNext", 0, 0),
     JS_FS_END
 };
@@ -1462,13 +1460,32 @@ const Class StopIterationObject::class_ = {
     stopiter_hasInstance
 };
 
+static const JSFunctionSpec iterator_proto_methods[] = {
+    JS_SELF_HOSTED_SYM_FN(iterator, "IteratorIdentity", 0, 0),
+    JS_FS_END
+};
+
+/* static */ bool
+GlobalObject::initIteratorProto(JSContext* cx, Handle<GlobalObject*> global)
+{
+    if (global->getReservedSlot(ITERATOR_PROTO).isObject())
+        return true;
+
+    RootedObject proto(cx, global->createBlankPrototype<PlainObject>(cx));
+    if (!proto || !DefinePropertiesAndFunctions(cx, proto, nullptr, iterator_proto_methods))
+        return false;
+
+    global->setReservedSlot(ITERATOR_PROTO, ObjectValue(*proto));
+    return true;
+}
+
 /* static */ bool
 GlobalObject::initArrayIteratorProto(JSContext* cx, Handle<GlobalObject*> global)
 {
     if (global->getReservedSlot(ARRAY_ITERATOR_PROTO).isObject())
         return true;
 
-    RootedObject iteratorProto(cx, GlobalObject::getOrCreateLegacyIteratorPrototype(cx, global));
+    RootedObject iteratorProto(cx, GlobalObject::getOrCreateIteratorPrototype(cx, global));
     if (!iteratorProto)
         return false;
 
@@ -1487,8 +1504,12 @@ GlobalObject::initStringIteratorProto(JSContext* cx, Handle<GlobalObject*> globa
     if (global->getReservedSlot(STRING_ITERATOR_PROTO).isObject())
         return true;
 
+    RootedObject iteratorProto(cx, GlobalObject::getOrCreateIteratorPrototype(cx, global));
+    if (!iteratorProto)
+        return false;
+
     const Class* cls = &StringIteratorPrototypeClass;
-    RootedObject proto(cx, global->createBlankPrototype(cx, cls));
+    RootedObject proto(cx, global->createBlankPrototypeInheriting(cx, cls, iteratorProto));
     if (!proto || !DefinePropertiesAndFunctions(cx, proto, nullptr, string_iterator_methods))
         return false;
 

js/src/vm/GlobalObject.h

@@ -96,6 +96,7 @@ class GlobalObject : public NativeObject
 
         /* One-off properties stored after slots for built-ins. */
         LEXICAL_SCOPE,
+        ITERATOR_PROTO,
         ARRAY_ITERATOR_PROTO,
         STRING_ITERATOR_PROTO,
         LEGACY_GENERATOR_OBJECT_PROTO,
@@ -509,13 +510,9 @@ class GlobalObject : public NativeObject
     }
 
   public:
-    static NativeObject* getOrCreateLegacyIteratorPrototype(JSContext* cx,
-                                                            Handle<GlobalObject*> global)
+    static NativeObject* getOrCreateIteratorPrototype(JSContext* cx, Handle<GlobalObject*> global)
     {
-        if (!ensureConstructor(cx, global, JSProto_Iterator))
-            return nullptr;
-        size_t slot = APPLICATION_SLOTS + JSProto_LIMIT + JSProto_Iterator;
-        return &global->getSlot(slot).toObject().as<NativeObject>();
+        return MaybeNativeObject(global->getOrCreateObject(cx, ITERATOR_PROTO, initIteratorProto));
     }
 
     static NativeObject* getOrCreateArrayIteratorPrototype(JSContext* cx, Handle<GlobalObject*> global)
@@ -678,6 +675,7 @@ class GlobalObject : public NativeObject
     bool valueIsEval(Value val);
 
     // Implemented in jsiter.cpp.
+    static bool initIteratorProto(JSContext* cx, Handle<GlobalObject*> global);
     static bool initArrayIteratorProto(JSContext* cx, Handle<GlobalObject*> global);
     static bool initStringIteratorProto(JSContext* cx, Handle<GlobalObject*> global);
 

js/src/vm/SelfHosting.cpp

@@ -448,7 +448,7 @@ intrinsic_GetIteratorPrototype(JSContext* cx, unsigned argc, Value* vp)
     CallArgs args = CallArgsFromVp(argc, vp);
     MOZ_ASSERT(args.length() == 0);
 
-    JSObject* obj = GlobalObject::getOrCreateLegacyIteratorPrototype(cx, cx->global());
+    JSObject* obj = GlobalObject::getOrCreateIteratorPrototype(cx, cx->global());
     if (!obj)
         return false;
 
@@ -513,7 +513,7 @@ intrinsic_NewListIterator(JSContext* cx, unsigned argc, Value* vp)
     CallArgs args = CallArgsFromVp(argc, vp);
     MOZ_ASSERT(args.length() == 0);
 
-    RootedObject proto(cx, GlobalObject::getOrCreateLegacyIteratorPrototype(cx, cx->global()));
+    RootedObject proto(cx, GlobalObject::getOrCreateIteratorPrototype(cx, cx->global()));
     if (!proto)
         return false;