Bug 1097267 - Change to the simpler enumerate hook in the js engine. r=jorendorff
authorTom Schuster <evilpies@gmail.com>
Thu, 11 Dec 2014 19:31:10 +0100
changeset 219311 906c45183f30120a0dec85f47e018474e56b7a2e
parent 219310 339befde1a62c1b13fcd4ce9bac37bf285d49b93
child 219312 5a8e7d35dd45e58c129a352390bcff981fa255bc
push id10368
push userkwierso@gmail.com
push dateFri, 12 Dec 2014 01:38:39 +0000
treeherderfx-team@5288b15d22de [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjorendorff
bugs1097267
milestone37.0a1
Bug 1097267 - Change to the simpler enumerate hook in the js engine. r=jorendorff
js/public/Class.h
js/public/Value.h
js/src/builtin/TypedObject.cpp
js/src/builtin/TypedObject.h
js/src/jsfriendapi.h
js/src/jsiter.cpp
js/src/jsobj.cpp
js/src/jsobj.h
js/src/jspubtd.h
--- a/js/public/Class.h
+++ b/js/public/Class.h
@@ -36,16 +36,22 @@ class PropertyName;
 class Shape;
 
 // This is equal to JSFunction::class_.  Use it in places where you don't want
 // to #include jsfun.h.
 extern JS_FRIEND_DATA(const js::Class* const) FunctionClassPtr;
 
 } // namespace js
 
+namespace JS {
+
+class AutoIdVector;
+
+}
+
 // JSClass operation signatures.
 
 // Add or get a property named by id in obj.  Note the jsid id type -- id may
 // be a string (Unicode property identifier) or an int (element index).  The
 // *vp out parameter, on success, is the new property value after the action.
 typedef bool
 (* JSPropertyOp)(JSContext *cx, JS::HandleObject obj, JS::HandleId id,
                  JS::MutableHandleValue vp);
@@ -71,49 +77,29 @@ typedef bool
 // If no error occurred and the deletion wasn't disallowed (this is *not* the
 // same as saying that a deletion actually occurred -- deleting a non-existent
 // property, or an inherited property, is allowed -- it's just pointless),
 // set *succeeded to true and return true.
 typedef bool
 (* JSDeletePropertyOp)(JSContext *cx, JS::HandleObject obj, JS::HandleId id,
                        bool *succeeded);
 
-// This function type is used for callbacks that enumerate the properties of
-// a JSObject.  The behavior depends on the value of enum_op:
-//
-//  JSENUMERATE_INIT
-//    A new, opaque iterator state should be allocated and stored in *statep.
-//    (You can use PRIVATE_TO_JSVAL() to tag the pointer to be stored).
-//
-//    The number of properties that will be enumerated should be returned as
-//    an integer jsval in *idp, if idp is non-null, and provided the number of
-//    enumerable properties is known.  If idp is non-null and the number of
-//    enumerable properties can't be computed in advance, *idp should be set
-//    to JSVAL_ZERO.
-//
-//  JSENUMERATE_INIT_ALL
-//    Used identically to JSENUMERATE_INIT, but exposes all properties of the
-//    object regardless of enumerability.
+// The type of ObjectOps::enumerate. This callback overrides a portion of SpiderMonkey's default
+// [[Enumerate]] internal method. When an ordinary object is enumerated, that object and each object
+// on its prototype chain is tested for an enumerate op, and those ops are called in order.
+// The properties each op adds to the 'properties' vector are added to the set of values the
+// for-in loop will iterate over. All of this is nonstandard.
 //
-//  JSENUMERATE_NEXT
-//    A previously allocated opaque iterator state is passed in via statep.
-//    Return the next jsid in the iteration using *idp.  The opaque iterator
-//    state pointed at by statep is destroyed and *statep is set to JSVAL_NULL
-//    if there are no properties left to enumerate.
+// An object is "enumerated" when it's the target of a for-in loop or JS_Enumerate().
+// All other property inspection, including Object.keys(obj), goes through [[OwnKeys]].
 //
-//  JSENUMERATE_DESTROY
-//    Destroy the opaque iterator state previously allocated in *statep by a
-//    call to this function when enum_op was JSENUMERATE_INIT or
-//    JSENUMERATE_INIT_ALL.
-//
-// The return value is used to indicate success, with a value of false
-// indicating failure.
+// The callback's job is to populate 'properties' with all property keys that the for-in loop
+// should visit.
 typedef bool
-(* JSNewEnumerateOp)(JSContext *cx, JS::HandleObject obj, JSIterateOp enum_op,
-                     JS::MutableHandleValue statep, JS::MutableHandleId idp);
+(* JSNewEnumerateOp)(JSContext *cx, JS::HandleObject obj, JS::AutoIdVector &properties);
 
 // The old-style JSClass.enumerate op should define all lazy properties not
 // yet reflected in obj.
 typedef bool
 (* JSEnumerateOp)(JSContext *cx, JS::HandleObject obj);
 
 // Resolve a lazy property named by id in obj by defining it directly in obj.
 // Lazy properties are those reflected from some peer native property space
@@ -416,17 +402,16 @@ typedef void (*JSClassInternal)();
 
 struct JSClass {
     JS_CLASS_MEMBERS(JSFinalizeOp);
 
     void                *reserved[32];
 };
 
 #define JSCLASS_HAS_PRIVATE             (1<<0)  // objects have private slot
-#define JSCLASS_NEW_ENUMERATE           (1<<1)  // has JSNewEnumerateOp hook
 #define JSCLASS_PRIVATE_IS_NSISUPPORTS  (1<<3)  // private is (nsISupports *)
 #define JSCLASS_IS_DOMJSCLASS           (1<<4)  // objects are DOM
 #define JSCLASS_IMPLEMENTS_BARRIERS     (1<<5)  // Correctly implements GC read
                                                 // and write barriers
 #define JSCLASS_EMULATES_UNDEFINED      (1<<6)  // objects of this class act
                                                 // like the value undefined,
                                                 // in some contexts
 #define JSCLASS_USERBIT1                (1<<7)  // Reserved for embeddings.
--- a/js/public/Value.h
+++ b/js/public/Value.h
@@ -233,19 +233,16 @@ typedef uint64_t JSValueShiftedTag;
 #define JSVAL_UPPER_EXCL_SHIFTED_TAG_OF_NUMBER_SET       JSVAL_SHIFTED_TAG_UNDEFINED
 #define JSVAL_LOWER_INCL_SHIFTED_TAG_OF_GCTHING_SET      JSVAL_SHIFTED_TAG_STRING
 
 #endif /* JS_PUNBOX64 */
 
 typedef enum JSWhyMagic
 {
     JS_ELEMENTS_HOLE,            /* a hole in a native object's elements */
-    JS_NATIVE_ENUMERATE,         /* indicates that a custom enumerate hook forwarded
-                                  * to JS_EnumerateState, which really means the object can be
-                                  * enumerated like a native object. */
     JS_NO_ITER_VALUE,            /* there is not a pending iterator value */
     JS_GENERATOR_CLOSING,        /* exception value thrown when closing a generator */
     JS_NO_CONSTANT,              /* compiler sentinel value */
     JS_THIS_POISON,              /* used in debug builds to catch tracing errors */
     JS_ARG_POISON,               /* used in debug builds to catch tracing errors */
     JS_SERIALIZE_NO_NODE,        /* an empty subnode in the AST serializer */
     JS_LAZY_ARGUMENTS,           /* lazy arguments value on the stack */
     JS_OPTIMIZED_ARGUMENTS,      /* optimized-away 'arguments' value */
--- a/js/src/builtin/TypedObject.cpp
+++ b/js/src/builtin/TypedObject.cpp
@@ -2115,93 +2115,53 @@ TypedObject::obj_deleteGeneric(JSContext
         *succeeded = false;
         return true;
     }
 
     return JSObject::deleteGeneric(cx, proto, id, succeeded);
 }
 
 bool
-TypedObject::obj_enumerate(JSContext *cx, HandleObject obj, JSIterateOp enum_op,
-                           MutableHandleValue statep, MutableHandleId idp)
+TypedObject::obj_enumerate(JSContext *cx, HandleObject obj, AutoIdVector &properties)
 {
-    int32_t index;
-
     MOZ_ASSERT(obj->is<TypedObject>());
     Rooted<TypedObject *> typedObj(cx, &obj->as<TypedObject>());
     Rooted<TypeDescr *> descr(cx, &typedObj->typeDescr());
+
+    RootedId id(cx);
     switch (descr->kind()) {
       case type::Scalar:
       case type::Reference:
-      case type::Simd:
-        switch (enum_op) {
-          case JSENUMERATE_INIT_ALL:
-          case JSENUMERATE_INIT:
-            statep.setInt32(0);
-            idp.set(INT_TO_JSID(0));
-
-          case JSENUMERATE_NEXT:
-          case JSENUMERATE_DESTROY:
-            statep.setNull();
-            break;
+      case type::Simd: {
+        // Nothing to enumerate.
+        break;
+      }
+
+      case type::Array: {
+        if (!properties.reserve(typedObj->length()))
+            return false;
+
+        for (int32_t index = 0; index < typedObj->length(); index++) {
+            id.set(INT_TO_JSID(index));
+            properties.infallibleAppend(id);
         }
         break;
-
-      case type::Array:
-        switch (enum_op) {
-          case JSENUMERATE_INIT_ALL:
-          case JSENUMERATE_INIT:
-            statep.setInt32(0);
-            idp.set(INT_TO_JSID(typedObj->length()));
-            break;
-
-          case JSENUMERATE_NEXT:
-            index = statep.toInt32();
-
-            if (index < typedObj->length()) {
-                idp.set(INT_TO_JSID(index));
-                statep.setInt32(index + 1);
-            } else {
-                MOZ_ASSERT(index == typedObj->length());
-                statep.setNull();
-            }
-
-            break;
-
-          case JSENUMERATE_DESTROY:
-            statep.setNull();
-            break;
+      }
+
+      case type::Struct: {
+        size_t fieldCount = descr->as<StructTypeDescr>().fieldCount();
+        if (!properties.reserve(fieldCount))
+            return false;
+
+        for (size_t index = 0; index < fieldCount; index++) {
+            id.set(AtomToId(&descr->as<StructTypeDescr>().fieldName(index)));
+            properties.infallibleAppend(id);
         }
         break;
-
-      case type::Struct:
-        switch (enum_op) {
-          case JSENUMERATE_INIT_ALL:
-          case JSENUMERATE_INIT:
-            statep.setInt32(0);
-            idp.set(INT_TO_JSID(descr->as<StructTypeDescr>().fieldCount()));
-            break;
-
-          case JSENUMERATE_NEXT:
-            index = static_cast<uint32_t>(statep.toInt32());
-
-            if ((size_t) index < descr->as<StructTypeDescr>().fieldCount()) {
-                idp.set(AtomToId(&descr->as<StructTypeDescr>().fieldName(index)));
-                statep.setInt32(index + 1);
-            } else {
-                statep.setNull();
-            }
-
-            break;
-
-          case JSENUMERATE_DESTROY:
-            statep.setNull();
-            break;
-        }
-        break;
+      }
     }
 
     return true;
 }
 
 void
 OutlineTypedObject::neuter(void *newData)
 {
--- a/js/src/builtin/TypedObject.h
+++ b/js/src/builtin/TypedObject.h
@@ -576,18 +576,17 @@ class TypedObject : public JSObject
 
     static bool obj_getGenericAttributes(JSContext *cx, HandleObject obj,
                                          HandleId id, unsigned *attrsp);
     static bool obj_setGenericAttributes(JSContext *cx, HandleObject obj,
                                          HandleId id, unsigned *attrsp);
 
     static bool obj_deleteGeneric(JSContext *cx, HandleObject obj, HandleId id, bool *succeeded);
 
-    static bool obj_enumerate(JSContext *cx, HandleObject obj, JSIterateOp enum_op,
-                              MutableHandleValue statep, MutableHandleId idp);
+    static bool obj_enumerate(JSContext *cx, HandleObject obj, AutoIdVector &properties);
 
   public:
     TypedProto &typedProto() const {
         return getProto()->as<TypedProto>();
     }
 
     TypedProto &maybeForwardedTypedProto() const {
         return MaybeForwarded(getProto())->as<TypedProto>();
--- a/js/src/jsfriendapi.h
+++ b/js/src/jsfriendapi.h
@@ -227,20 +227,16 @@ JS_CopyPropertiesFrom(JSContext *cx, JS:
  */
 extern JS_FRIEND_API(bool)
 JS_CopyPropertyFrom(JSContext *cx, JS::HandleId id, JS::HandleObject target,
                     JS::HandleObject obj);
 
 extern JS_FRIEND_API(bool)
 JS_WrapPropertyDescriptor(JSContext *cx, JS::MutableHandle<JSPropertyDescriptor> desc);
 
-extern JS_FRIEND_API(bool)
-JS_EnumerateState(JSContext *cx, JS::HandleObject obj, JSIterateOp enum_op,
-                  JS::MutableHandleValue statep, JS::MutableHandleId idp);
-
 struct JSFunctionSpecWithHelp {
     const char      *name;
     JSNative        call;
     uint16_t        nargs;
     uint16_t        flags;
     const char      *usage;
     const char      *help;
 };
--- a/js/src/jsiter.cpp
+++ b/js/src/jsiter.cpp
@@ -276,74 +276,67 @@ Snapshot(JSContext *cx, HandleObject pob
     // ~90% of the time this table ends up with 3 or fewer elements.
     IdSet ht(cx);
     if (!ht.init(3))
         return false;
 
     RootedObject pobj(cx, pobj_);
 
     do {
-        const Class *clasp = pobj->getClass();
-        if (pobj->isNative() &&
-            !pobj->getOps()->enumerate &&
-            !(clasp->flags & JSCLASS_NEW_ENUMERATE))
-        {
-            if (JSEnumerateOp enumerate = clasp->enumerate) {
+        if (JSNewEnumerateOp enumerate = pobj->getOps()->enumerate) {
+            // This hook has the full control over what gets enumerated.
+            AutoIdVector properties(cx);
+            if (!enumerate(cx, pobj, properties))
+                 return false;
+
+            for (size_t n = 0; n < properties.length(); n++) {
+                if (!Enumerate(cx, pobj, properties[n], true, flags, ht, props))
+                    return false;
+            }
+
+            if (pobj->isNative()) {
+                if (!EnumerateNativeProperties(cx, pobj.as<NativeObject>(), flags, ht, props))
+                    return false;
+            }
+        } else if (pobj->isNative()) {
+            // Give the object a chance to resolve all lazy properties
+            if (JSEnumerateOp enumerate = pobj->getClass()->enumerate) {
                 if (!enumerate(cx, pobj.as<NativeObject>()))
                     return false;
             }
             if (!EnumerateNativeProperties(cx, pobj.as<NativeObject>(), flags, ht, props))
                 return false;
-        } else {
-            if (pobj->is<ProxyObject>()) {
-                AutoIdVector proxyProps(cx);
-                if (flags & JSITER_OWNONLY) {
-                    if (flags & JSITER_HIDDEN) {
-                        // This gets all property keys, both strings and
-                        // symbols.  The call to Enumerate in the loop below
-                        // will filter out unwanted keys, per the flags.
-                        if (!Proxy::ownPropertyKeys(cx, pobj, proxyProps))
-                            return false;
-                    } else {
-                        if (!Proxy::getOwnEnumerablePropertyKeys(cx, pobj, proxyProps))
-                            return false;
-                    }
-                } else {
-                    if (!Proxy::getEnumerablePropertyKeys(cx, pobj, proxyProps))
+        } else if (pobj->is<ProxyObject>()) {
+            AutoIdVector proxyProps(cx);
+            if (flags & JSITER_OWNONLY) {
+                if (flags & JSITER_HIDDEN) {
+                    // This gets all property keys, both strings and
+                    // symbols.  The call to Enumerate in the loop below
+                    // will filter out unwanted keys, per the flags.
+                    if (!Proxy::ownPropertyKeys(cx, pobj, proxyProps))
                         return false;
+                 } else {
+                    if (!Proxy::getOwnEnumerablePropertyKeys(cx, pobj, proxyProps))
+                         return false;
                 }
-
-                for (size_t n = 0, len = proxyProps.length(); n < len; n++) {
-                    if (!Enumerate(cx, pobj, proxyProps[n], true, flags, ht, props))
-                        return false;
-                }
+            } else {
+                if (!Proxy::getEnumerablePropertyKeys(cx, pobj, proxyProps))
+                    return false;
+            }
 
-                // Proxy objects enumerate the prototype on their own, so we're
-                // done here.
-                break;
-            }
-            RootedValue state(cx);
-            RootedId id(cx);
-            JSIterateOp op = (flags & JSITER_HIDDEN) ? JSENUMERATE_INIT_ALL : JSENUMERATE_INIT;
-            if (!JSObject::enumerate(cx, pobj, op, &state, &id))
-                return false;
-            if (state.isMagic(JS_NATIVE_ENUMERATE)) {
-                if (!EnumerateNativeProperties(cx, pobj.as<NativeObject>(), flags, ht, props))
+            for (size_t n = 0, len = proxyProps.length(); n < len; n++) {
+                if (!Enumerate(cx, pobj, proxyProps[n], true, flags, ht, props))
                     return false;
-            } else {
-                while (true) {
-                    RootedId id(cx);
-                    if (!JSObject::enumerate(cx, pobj, JSENUMERATE_NEXT, &state, &id))
-                        return false;
-                    if (state.isNull())
-                        break;
-                    if (!Enumerate(cx, pobj, id, true, flags, ht, props))
-                        return false;
-                }
             }
+
+            // Proxy objects enumerate the prototype on their own, so we're
+            // done here.
+            break;
+        } else {
+            MOZ_CRASH("non-native objects must have an enumerate op");
         }
 
         if (flags & JSITER_OWNONLY)
             break;
 
     } while ((pobj = pobj->getProto()) != nullptr);
 
 #ifdef JS_MORE_DETERMINISTIC
--- a/js/src/jsobj.cpp
+++ b/js/src/jsobj.cpp
@@ -3518,37 +3518,16 @@ js::DefaultValue(JSContext *cx, HandleOb
     RootedValue val(cx, ObjectValue(*obj));
     js_ReportValueError2(cx, JSMSG_CANT_CONVERT_TO, JSDVG_SEARCH_STACK, val, str,
                          hint == JSTYPE_VOID
                          ? "primitive type"
                          : hint == JSTYPE_STRING ? "string" : "number");
     return false;
 }
 
-JS_FRIEND_API(bool)
-JS_EnumerateState(JSContext *cx, HandleObject obj, JSIterateOp enum_op,
-                  MutableHandleValue statep, JS::MutableHandleId idp)
-{
-    /* If the class has a custom JSCLASS_NEW_ENUMERATE hook, call it. */
-    const Class *clasp = obj->getClass();
-    JSEnumerateOp enumerate = clasp->enumerate;
-    if (enumerate) {
-        if (clasp->flags & JSCLASS_NEW_ENUMERATE)
-            return ((JSNewEnumerateOp) enumerate)(cx, obj, enum_op, statep, idp);
-
-        if (!enumerate(cx, obj))
-            return false;
-    }
-
-    /* Tell InitNativeIterator to treat us like a native object. */
-    MOZ_ASSERT(enum_op == JSENUMERATE_INIT || enum_op == JSENUMERATE_INIT_ALL);
-    statep.setMagic(JS_NATIVE_ENUMERATE);
-    return true;
-}
-
 bool
 js::IsDelegate(JSContext *cx, HandleObject obj, const js::Value &v, bool *result)
 {
     if (v.isPrimitive()) {
         *result = false;
         return true;
     }
     return IsDelegateOfObject(cx, obj, &v.toObject(), result);
@@ -3786,17 +3765,16 @@ dumpValue(const Value &v)
             fprintf(stderr, "true");
         else
             fprintf(stderr, "false");
     } else if (v.isMagic()) {
         fprintf(stderr, "<invalid");
 #ifdef DEBUG
         switch (v.whyMagic()) {
           case JS_ELEMENTS_HOLE:     fprintf(stderr, " elements hole");      break;
-          case JS_NATIVE_ENUMERATE:  fprintf(stderr, " native enumeration"); break;
           case JS_NO_ITER_VALUE:     fprintf(stderr, " no iter value");      break;
           case JS_GENERATOR_CLOSING: fprintf(stderr, " generator closing");  break;
           case JS_OPTIMIZED_OUT:     fprintf(stderr, " optimized out");      break;
           default:                   fprintf(stderr, " ?!");                 break;
         }
 #endif
         fprintf(stderr, ">");
     } else {
--- a/js/src/jsobj.h
+++ b/js/src/jsobj.h
@@ -669,23 +669,16 @@ class JSObject : public js::gc::Cell
                                      bool *succeeded);
     static inline bool deleteElement(JSContext *cx, js::HandleObject obj, uint32_t index,
                                      bool *succeeded);
 
     static inline bool watch(JSContext *cx, JS::HandleObject obj, JS::HandleId id,
                              JS::HandleObject callable);
     static inline bool unwatch(JSContext *cx, JS::HandleObject obj, JS::HandleId id);
 
-    static bool enumerate(JSContext *cx, JS::HandleObject obj, JSIterateOp iterop,
-                          JS::MutableHandleValue statep, JS::MutableHandleId idp)
-    {
-        JSNewEnumerateOp op = obj->getOps()->enumerate;
-        return (op ? op : JS_EnumerateState)(cx, obj, iterop, statep, idp);
-    }
-
     static bool defaultValue(JSContext *cx, js::HandleObject obj, JSType hint,
                              js::MutableHandleValue vp)
     {
         JSConvertOp op = obj->getClass()->convert;
         bool ok;
         if (!op)
             ok = js::DefaultValue(cx, obj, hint, vp);
         else
--- a/js/src/jspubtd.h
+++ b/js/src/jspubtd.h
@@ -79,34 +79,16 @@ enum JSType {
 /* Dense index into cached prototypes and class atoms for standard objects. */
 enum JSProtoKey {
 #define PROTOKEY_AND_INITIALIZER(name,code,init,clasp) JSProto_##name = code,
     JS_FOR_EACH_PROTOTYPE(PROTOKEY_AND_INITIALIZER)
 #undef PROTOKEY_AND_INITIALIZER
     JSProto_LIMIT
 };
 
-/*
- * This enum type is used to control the behavior of a JSObject property
- * iterator function that has type JSNewEnumerate.
- */
-enum JSIterateOp {
-    /* Create new iterator state over enumerable properties. */
-    JSENUMERATE_INIT,
-
-    /* Create new iterator state over all properties. */
-    JSENUMERATE_INIT_ALL,
-
-    /* Iterate once. */
-    JSENUMERATE_NEXT,
-
-    /* Destroy iterator state. */
-    JSENUMERATE_DESTROY
-};
-
 /* Struct forward declarations. */
 struct JSClass;
 struct JSCompartment;
 struct JSCrossCompartmentCall;
 class JSErrorReport;
 struct JSExceptionState;
 struct JSFunctionSpec;
 struct JSIdArray;