Bug 1298878 - Don't store the actual builtin constructor properties on the global in reserved slots. r=Waldo

js/public/Class.h

@@ -787,7 +787,7 @@ struct JSClass {
 // application.
 #define JSCLASS_GLOBAL_APPLICATION_SLOTS 5
 #define JSCLASS_GLOBAL_SLOT_COUNT                                             \
-    (JSCLASS_GLOBAL_APPLICATION_SLOTS + JSProto_LIMIT * 3 + 37)
+    (JSCLASS_GLOBAL_APPLICATION_SLOTS + JSProto_LIMIT * 2 + 37)
 #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/jsobj.cpp

@@ -1669,27 +1669,6 @@ DefineStandardSlot(JSContext* cx, HandleObject obj, JSProtoKey key, JSAtom* atom
                    HandleValue v, uint32_t attrs, bool& named)
 {
     RootedId id(cx, AtomToId(atom));
-
-    if (key != JSProto_Null) {
-        /*
-         * Initializing an actual standard class on a global object. If the
-         * property is not yet present, force it into a new one bound to a
-         * reserved slot. Otherwise, go through the normal property path.
-         */
-        Rooted<GlobalObject*> global(cx, &obj->as<GlobalObject>());
-
-        if (!global->lookup(cx, id)) {
-            global->setConstructorPropertySlot(key, v);
-
-            uint32_t slot = GlobalObject::constructorPropertySlot(key);
-            if (!NativeObject::addProperty(cx, global, id, nullptr, nullptr, slot, attrs, 0))
-                return false;
-
-            named = true;
-            return true;
-        }
-    }
-
     named = DefineProperty(cx, obj, id, v, nullptr, nullptr, attrs);
     return named;
 }

js/src/vm/GlobalObject.cpp

@@ -220,12 +220,12 @@ GlobalObject::resolveConstructor(JSContext* cx, Handle<GlobalObject*> global, JS
     RootedId id(cx, NameToId(ClassName(key, cx)));
     if (isObjectOrFunction) {
         if (clasp->specShouldDefineConstructor()) {
-            if (!global->addDataProperty(cx, id, constructorPropertySlot(key), 0))
+            RootedValue ctorValue(cx, ObjectValue(*ctor));
+            if (!DefineProperty(cx, global, id, ctorValue, nullptr, nullptr, JSPROP_RESOLVING))
                 return false;
         }
 
         global->setConstructor(key, ObjectValue(*ctor));
-        global->setConstructorPropertySlot(key, ObjectValue(*ctor));
     }
 
     // Define any specified functions and properties, unless we're a dependent
@@ -267,23 +267,17 @@ GlobalObject::resolveConstructor(JSContext* cx, Handle<GlobalObject*> global, JS
 
         // Fallible operation that modifies the global object.
         if (clasp->specShouldDefineConstructor()) {
-            if (!global->addDataProperty(cx, id, constructorPropertySlot(key), 0))
+            RootedValue ctorValue(cx, ObjectValue(*ctor));
+            if (!DefineProperty(cx, global, id, ctorValue, nullptr, nullptr, JSPROP_RESOLVING))
                 return false;
         }
 
         // Infallible operations that modify the global object.
         global->setConstructor(key, ObjectValue(*ctor));
-        global->setConstructorPropertySlot(key, ObjectValue(*ctor));
         if (proto)
             global->setPrototype(key, ObjectValue(*proto));
     }
 
-    if (clasp->specShouldDefineConstructor()) {
-        // Stash type information, so that what we do here is equivalent to
-        // initBuiltinConstructor.
-        AddTypePropertyId(cx, global, id, ObjectValue(*ctor));
-    }
-
     return true;
 }
 
@@ -299,14 +293,12 @@ GlobalObject::initBuiltinConstructor(JSContext* cx, Handle<GlobalObject*> global
     RootedId id(cx, NameToId(ClassName(key, cx)));
     MOZ_ASSERT(!global->lookup(cx, id));
 
-    if (!global->addDataProperty(cx, id, constructorPropertySlot(key), 0))
+    RootedValue ctorValue(cx, ObjectValue(*ctor));
+    if (!DefineProperty(cx, global, id, ctorValue, nullptr, nullptr, JSPROP_RESOLVING))
         return false;
 
     global->setConstructor(key, ObjectValue(*ctor));
     global->setPrototype(key, ObjectValue(*proto));
-    global->setConstructorPropertySlot(key, ObjectValue(*ctor));
-
-    AddTypePropertyId(cx, global, id, ObjectValue(*ctor));
     return true;
 }
 

js/src/vm/GlobalObject.h

@@ -46,22 +46,18 @@ enum class SimdType;
  * [APPLICATION_SLOTS + JSProto_LIMIT, APPLICATION_SLOTS + 2 * JSProto_LIMIT)
  *   Stores the prototype, if any, for the constructor for the corresponding
  *   JSProtoKey offset from JSProto_LIMIT.
- * [APPLICATION_SLOTS + 2 * JSProto_LIMIT, APPLICATION_SLOTS + 3 * JSProto_LIMIT)
- *   Stores the current value of the global property named for the JSProtoKey
- *   for the corresponding JSProtoKey offset from 2 * JSProto_LIMIT.
- * [APPLICATION_SLOTS + 3 * JSProto_LIMIT, RESERVED_SLOTS)
+ * [APPLICATION_SLOTS + 2 * JSProto_LIMIT, RESERVED_SLOTS)
  *   Various one-off values: ES5 13.2.3's [[ThrowTypeError]], RegExp statics,
  *   the original eval for this global object (implementing |var eval =
  *   otherWindow.eval; eval(...)| as an indirect eval), a bit indicating
  *   whether this object has been cleared (see JS_ClearScope), and a cache for
  *   whether eval is allowed (per the global's Content Security Policy).
  *
- * The first two JSProto_LIMIT-sized ranges are necessary to implement
+ * The two JSProto_LIMIT-sized ranges are necessary to implement
  * js::FindClassObject, and spec language speaking in terms of "the original
  * Array prototype object", or "as if by the expression new Array()" referring
- * to the original Array constructor. The third range stores the (writable and
- * even deletable) Object, Array, &c. properties (although a slot won't be used
- * again if its property is deleted and readded).
+ * to the original Array constructor. The actual (writable and even deletable)
+ * Object, Array, &c. properties are not stored in reserved slots.
  */
 class GlobalObject : public NativeObject
 {
@@ -69,10 +65,10 @@ class GlobalObject : public NativeObject
     static const unsigned APPLICATION_SLOTS = JSCLASS_GLOBAL_APPLICATION_SLOTS;
 
     /*
-     * Count of slots to store built-in constructors, prototypes, and initial
-     * visible properties for the constructors.
+     * Count of slots to store built-in prototypes and initial visible
+     * properties for the constructors.
      */
-    static const unsigned STANDARD_CLASS_SLOTS  = JSProto_LIMIT * 3;
+    static const unsigned STANDARD_CLASS_SLOTS = JSProto_LIMIT * 2;
 
     enum : unsigned {
         /* Various function values needed by the engine. */
@@ -184,19 +180,6 @@ class GlobalObject : public NativeObject
         setSlot(APPLICATION_SLOTS + JSProto_LIMIT + key, value);
     }
 
-    static uint32_t constructorPropertySlot(JSProtoKey key) {
-        MOZ_ASSERT(key <= JSProto_LIMIT);
-        return APPLICATION_SLOTS + JSProto_LIMIT * 2 + key;
-    }
-
-    Value getConstructorPropertySlot(JSProtoKey key) {
-        return getSlot(constructorPropertySlot(key));
-    }
-
-    void setConstructorPropertySlot(JSProtoKey key, const Value& ctor) {
-        setSlot(constructorPropertySlot(key), ctor);
-    }
-
     bool classIsInitialized(JSProtoKey key) const {
         bool inited = !getConstructor(key).isUndefined();
         MOZ_ASSERT(inited == !getPrototype(key).isUndefined());