Bug 1081660 - Remove property iterator from JSAPI. r=Waldo
authorTom Schuster <evilpies@gmail.com>
Thu, 16 Oct 2014 18:39:30 +0200
changeset 210795 8c7d2cd4fa067369938f25c582491cf546c6d198
parent 210794 d16adf32157638597da7dc95fa22a44dc56eec9f
child 210796 f5b05c63480d804fb91dd9a8a7ee54421695282f
push id1
push userroot
push dateMon, 20 Oct 2014 17:29:22 +0000
reviewersWaldo
bugs1081660
milestone36.0a1
Bug 1081660 - Remove property iterator from JSAPI. r=Waldo
js/src/ctypes/CTypes.cpp
js/src/jsapi.cpp
js/src/jsapi.h
--- a/js/src/ctypes/CTypes.cpp
+++ b/js/src/ctypes/CTypes.cpp
@@ -2620,35 +2620,37 @@ ImplicitConvert(JSContext* cx,
       return TypeError(cx, "array", val);
     }
     break;
   }
   case TYPE_struct: {
     if (val.isObject() && !sourceData) {
       // Enumerate the properties of the object; if they match the struct
       // specification, convert the fields.
-      RootedObject iter(cx, JS_NewPropertyIterator(cx, valObj));
-      if (!iter)
+      AutoIdArray props(cx, JS_Enumerate(cx, valObj));
+      if (!props)
         return false;
 
       // Convert into an intermediate, in case of failure.
       size_t structSize = CType::GetSize(targetType);
       AutoPtr<char> intermediate(cx->pod_malloc<char>(structSize));
       if (!intermediate) {
         JS_ReportAllocationOverflow(cx);
         return false;
       }
 
+      const FieldInfoHash* fields = StructType::GetFieldInfo(targetType);
+      if (props.length() != fields->count()) {
+        JS_ReportError(cx, "missing fields");
+        return false;
+      }
+
       RootedId id(cx);
-      size_t i = 0;
-      while (1) {
-        if (!JS_NextProperty(cx, iter, &id))
-          return false;
-        if (JSID_IS_VOID(id))
-          break;
+      for (size_t i = 0; i < props.length(); ++i) {
+        id = props[i];
 
         if (!JSID_IS_STRING(id)) {
           JS_ReportError(cx, "property name is not a string");
           return false;
         }
 
         JSFlatString *name = JSID_TO_FLAT_STRING(id);
         const FieldInfo* field = StructType::LookupField(cx, targetType, name);
@@ -2658,24 +2660,16 @@ ImplicitConvert(JSContext* cx,
         RootedValue prop(cx);
         if (!JS_GetPropertyById(cx, valObj, id, &prop))
           return false;
 
         // Convert the field via ImplicitConvert().
         char* fieldData = intermediate.get() + field->mOffset;
         if (!ImplicitConvert(cx, prop, field->mType, fieldData, false, nullptr))
           return false;
-
-        ++i;
-      }
-
-      const FieldInfoHash* fields = StructType::GetFieldInfo(targetType);
-      if (i != fields->count()) {
-        JS_ReportError(cx, "missing fields");
-        return false;
       }
 
       memcpy(buffer, intermediate.get(), structSize);
       break;
     }
 
     return TypeError(cx, "struct", val);
   }
@@ -4700,43 +4694,33 @@ ArrayType::AddressOfElement(JSContext* c
 static JSFlatString*
 ExtractStructField(JSContext* cx, jsval val, MutableHandleObject typeObj)
 {
   if (val.isPrimitive()) {
     JS_ReportError(cx, "struct field descriptors require a valid name and type");
     return nullptr;
   }
 
-  RootedObject obj(cx, val.toObjectOrNull());
-  RootedObject iter(cx, JS_NewPropertyIterator(cx, obj));
-  if (!iter)
+  RootedObject obj(cx, &val.toObject());
+  AutoIdArray props(cx, JS_Enumerate(cx, obj));
+  if (!props)
     return nullptr;
 
-  RootedId nameid(cx);
-  if (!JS_NextProperty(cx, iter, &nameid))
+  // make sure we have one, and only one, property
+  if (props.length() != 1) {
+    JS_ReportError(cx, "struct field descriptors must contain one property");
     return nullptr;
-  if (JSID_IS_VOID(nameid)) {
-    JS_ReportError(cx, "struct field descriptors require a valid name and type");
-    return nullptr;
-  }
-
+  }
+
+  RootedId nameid(cx, props[0]);
   if (!JSID_IS_STRING(nameid)) {
     JS_ReportError(cx, "struct field descriptors require a valid name and type");
     return nullptr;
   }
 
-  // make sure we have one, and only one, property
-  RootedId id(cx);
-  if (!JS_NextProperty(cx, iter, &id))
-    return nullptr;
-  if (!JSID_IS_VOID(id)) {
-    JS_ReportError(cx, "struct field descriptors must contain one property");
-    return nullptr;
-  }
-
   RootedValue propVal(cx);
   if (!JS_GetPropertyById(cx, obj, nameid, &propVal))
     return nullptr;
 
   if (propVal.isPrimitive() || !CType::IsCType(&propVal.toObject())) {
     JS_ReportError(cx, "struct field descriptors require a valid name and type");
     return nullptr;
   }
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -3615,147 +3615,16 @@ JS_Enumerate(JSContext *cx, HandleObject
 
     AutoIdVector props(cx);
     JSIdArray *ida;
     if (!GetPropertyKeys(cx, obj, JSITER_OWNONLY, &props) || !VectorToIdArray(cx, props, &ida))
         return nullptr;
     return ida;
 }
 
-/*
- * XXX reverse iterator for properties, unreverse and meld with jsinterp.c's
- *     prop_iterator_class somehow...
- * + preserve the obj->enumerate API while optimizing the native object case
- * + native case here uses a JSShape *, but that iterates in reverse!
- * + so we make non-native match, by reverse-iterating after JS_Enumerating
- */
-static const uint32_t JSSLOT_ITER_INDEX = 0;
-
-static void
-prop_iter_finalize(FreeOp *fop, JSObject *obj)
-{
-    void *pdata = obj->as<NativeObject>().getPrivate();
-    if (!pdata)
-        return;
-
-    if (obj->as<NativeObject>().getSlot(JSSLOT_ITER_INDEX).toInt32() >= 0) {
-        /* Non-native case: destroy the ida enumerated when obj was created. */
-        JSIdArray *ida = (JSIdArray *) pdata;
-        fop->free_(ida);
-    }
-}
-
-static void
-prop_iter_trace(JSTracer *trc, JSObject *obj)
-{
-    void *pdata = obj->as<NativeObject>().getPrivate();
-    if (!pdata)
-        return;
-
-    if (obj->as<NativeObject>().getSlot(JSSLOT_ITER_INDEX).toInt32() < 0) {
-        /*
-         * Native case: just mark the next property to visit. We don't need a
-         * barrier here because the pointer is updated via setPrivate, which
-         * always takes a barrier.
-         */
-        Shape *tmp = static_cast<Shape *>(pdata);
-        MarkShapeUnbarriered(trc, &tmp, "prop iter shape");
-        obj->as<NativeObject>().setPrivateUnbarriered(tmp);
-    } else {
-        /* Non-native case: mark each id in the JSIdArray private. */
-        JSIdArray *ida = (JSIdArray *) pdata;
-        MarkIdRange(trc, ida->length, ida->vector, "prop iter");
-    }
-}
-
-static const Class prop_iter_class = {
-    "PropertyIterator",
-    JSCLASS_HAS_PRIVATE | JSCLASS_IMPLEMENTS_BARRIERS | JSCLASS_HAS_RESERVED_SLOTS(1),
-    JS_PropertyStub,         /* addProperty */
-    JS_DeletePropertyStub,   /* delProperty */
-    JS_PropertyStub,         /* getProperty */
-    JS_StrictPropertyStub,   /* setProperty */
-    JS_EnumerateStub,
-    JS_ResolveStub,
-    JS_ConvertStub,
-    prop_iter_finalize,
-    nullptr,        /* call        */
-    nullptr,        /* hasInstance */
-    nullptr,        /* construct   */
-    prop_iter_trace
-};
-
-JS_PUBLIC_API(JSObject *)
-JS_NewPropertyIterator(JSContext *cx, HandleObject obj)
-{
-    AssertHeapIsIdle(cx);
-    CHECK_REQUEST(cx);
-    assertSameCompartment(cx, obj);
-
-    RootedNativeObject iterobj(cx, NewNativeObjectWithClassProto(cx, &prop_iter_class,
-                                                                 nullptr, obj));
-    if (!iterobj)
-        return nullptr;
-
-    int index;
-    if (obj->isNative()) {
-        /* Native case: start with the last property in obj. */
-        iterobj->setPrivateGCThing(obj->lastProperty());
-        index = -1;
-    } else {
-        /* Non-native case: enumerate a JSIdArray and keep it via private. */
-        JSIdArray *ida = JS_Enumerate(cx, obj);
-        if (!ida)
-            return nullptr;
-        iterobj->setPrivate((void *)ida);
-        index = ida->length;
-    }
-
-    /* iterobj cannot escape to other threads here. */
-    iterobj->setSlot(JSSLOT_ITER_INDEX, Int32Value(index));
-    return iterobj;
-}
-
-JS_PUBLIC_API(bool)
-JS_NextProperty(JSContext *cx, HandleObject iterobj, MutableHandleId idp)
-{
-    AssertHeapIsIdle(cx);
-    CHECK_REQUEST(cx);
-    assertSameCompartment(cx, iterobj);
-    int32_t i = iterobj->as<NativeObject>().getSlot(JSSLOT_ITER_INDEX).toInt32();
-    if (i < 0) {
-        /* Native case: private data is a property tree node pointer. */
-        MOZ_ASSERT(iterobj->getParent()->isNative());
-        Shape *shape = static_cast<Shape *>(iterobj->as<NativeObject>().getPrivate());
-
-        while (shape->previous() && !shape->enumerable())
-            shape = shape->previous();
-
-        if (!shape->previous()) {
-            MOZ_ASSERT(shape->isEmptyShape());
-            idp.set(JSID_VOID);
-        } else {
-            iterobj->as<NativeObject>().setPrivateGCThing(const_cast<Shape *>(shape->previous().get()));
-            idp.set(shape->propid());
-        }
-    } else {
-        /* Non-native case: use the ida enumerated when iterobj was created. */
-        JSIdArray *ida = (JSIdArray *) iterobj->as<NativeObject>().getPrivate();
-        MOZ_ASSERT(i <= ida->length);
-        STATIC_ASSUME(i <= ida->length);
-        if (i == 0) {
-            idp.set(JSID_VOID);
-        } else {
-            idp.set(ida->vector[--i]);
-            iterobj->as<NativeObject>().setSlot(JSSLOT_ITER_INDEX, Int32Value(i));
-        }
-    }
-    return true;
-}
-
 JS_PUBLIC_API(jsval)
 JS_GetReservedSlot(JSObject *obj, uint32_t index)
 {
     return obj->fakeNativeGetReservedSlot(index);
 }
 
 JS_PUBLIC_API(void)
 JS_SetReservedSlot(JSObject *obj, uint32_t index, Value value)
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -3300,32 +3300,16 @@ JS_CreateMappedArrayBufferContents(int f
  * release the resource used by the object.
  */
 extern JS_PUBLIC_API(void)
 JS_ReleaseMappedArrayBufferContents(void *contents, size_t length);
 
 extern JS_PUBLIC_API(JSIdArray *)
 JS_Enumerate(JSContext *cx, JS::HandleObject obj);
 
-/*
- * Create an object to iterate over enumerable properties of obj, in arbitrary
- * property definition order.  NB: This differs from longstanding for..in loop
- * order, which uses order of property definition in obj.
- */
-extern JS_PUBLIC_API(JSObject *)
-JS_NewPropertyIterator(JSContext *cx, JS::Handle<JSObject*> obj);
-
-/*
- * Return true on success with *idp containing the id of the next enumerable
- * property to visit using iterobj, or JSID_IS_VOID if there is no such property
- * left to visit.  Return false on error.
- */
-extern JS_PUBLIC_API(bool)
-JS_NextProperty(JSContext *cx, JS::HandleObject iterobj, JS::MutableHandleId idp);
-
 extern JS_PUBLIC_API(jsval)
 JS_GetReservedSlot(JSObject *obj, uint32_t index);
 
 extern JS_PUBLIC_API(void)
 JS_SetReservedSlot(JSObject *obj, uint32_t index, jsval v);
 
 /************************************************************************/