[gjs] js: Use JS_Enumerate instead of iterator

[Philip Chimento <pchimento src gnome org>]

commit 03f4e89995b8dc1ddbd5f514c678615bbce1e2c2
Author: Philip Chimento <philip chimento gmail com>
Date:   Sat Jan 7 18:23:07 2017 -0800

    js: Use JS_Enumerate instead of iterator
    
    JS_NewPropertyIterator() and its related functions are gone in mozjs38;
    apparently they were broken anyway for indexed properties (i.e. those
    with integer IDs rather than string IDs.)
    
    Instead, we use JS_Enumerate() which often makes the code cleaner anyway.
    Note that the JS::AutoIdArray does not root the jsids inside it, it just
    manages the array wrapper.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=776966

 gi/boxed.cpp     |   49 +++++++++++++++++++------------------------------
 gi/object.cpp    |   31 ++++++++++++++-----------------
 gjs/importer.cpp |   41 ++++++++++++++---------------------------
 3 files changed, 47 insertions(+), 74 deletions(-)
---
diff --git a/gi/boxed.cpp b/gi/boxed.cpp
index 4447ed7..da6eea1 100644
--- a/gi/boxed.cpp
+++ b/gi/boxed.cpp
@@ -251,7 +251,7 @@ boxed_init_from_props(JSContext   *context,
                       Boxed       *priv,
                       JS::Value    props_value)
 {
-    bool success = false;
+    size_t ix, length;
 
     if (!props_value.isObject()) {
         gjs_throw(context, "argument should be a hash with fields to set");
@@ -259,55 +259,44 @@ boxed_init_from_props(JSContext   *context,
     }
 
     JS::RootedObject props(context, &props_value.toObject());
-    JS::RootedObject iter(context, JS_NewPropertyIterator(context, props));
+    JS::AutoIdArray ids(context, JS_Enumerate(context, props));
 
-    if (iter == NULL) {
-        gjs_throw(context, "Failed to create property iterator for fields hash");
+    if (!ids) {
+        gjs_throw(context, "Failed to enumerate fields hash");
         return false;
     }
 
     if (!priv->field_map)
         priv->field_map = get_field_map(priv->info);
 
-    JS::RootedId prop_id(context, JSID_VOID);
-    if (!JS_NextProperty(context, iter, prop_id.address()))
-        goto out;
-
-    while (!JSID_IS_VOID(prop_id)) {
+    JS::RootedValue value(context);
+    JS::RootedId prop_id(context);
+    for (ix = 0, length = ids.length(); ix < length; ix++) {
         GIFieldInfo *field_info;
-        char *name;
-        JS::RootedValue value(context);
+        g_autofree char *name = NULL;
 
-        if (!gjs_get_string_id(context, prop_id, &name))
-            goto out;
+        if (!gjs_get_string_id(context, ids[ix], &name))
+            return false;
 
         field_info = (GIFieldInfo *) g_hash_table_lookup(priv->field_map, name);
         if (field_info == NULL) {
             gjs_throw(context, "No field %s on boxed type %s",
                       name, g_base_info_get_name((GIBaseInfo *)priv->info));
-            g_free(name);
-            goto out;
+            return false;
         }
 
-        if (!gjs_object_require_property(context, props, "property list", prop_id, &value)) {
-            g_free(name);
-            goto out;
-        }
-        g_free(name);
+        /* ids[ix] is reachable because props is rooted, but require_property
+         * doesn't know that */
+        prop_id = ids[ix];
+        if (!gjs_object_require_property(context, props, "property list",
+                                         prop_id, &value))
+            return false;
 
         if (!boxed_set_field_from_value(context, priv, field_info, value))
-            goto out;
-
-        prop_id = JSID_VOID;
-        if (!JS_NextProperty(context, iter, prop_id.address()))
-            goto out;
+            return false;
     }
 
-    success = true;
-
- out:
-
-    return success;
+    return true;
 }
 
 static bool
diff --git a/gi/object.cpp b/gi/object.cpp
index 37c6f93..28e74d6 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -825,33 +825,34 @@ object_instance_props_to_g_parameters(JSContext                  *context,
                                       GType                       gtype,
                                       std::vector<GParameter>&    gparams)
 {
+    size_t ix, length;
+
     if (args.length() == 0 || args[0].isUndefined())
         return true;
 
-    JS::RootedObject props(context), iter(context);
-    JS::RootedId prop_id(context);
-
     if (!args[0].isObject()) {
         gjs_throw(context, "argument should be a hash with props to set");
-        goto free_array_and_fail;
+        free_g_params(&gparams[0], gparams.size());
+        return false;
     }
 
-    props = &args[0].toObject();
-
-    iter = JS_NewPropertyIterator(context, props);
-    if (iter == NULL) {
+    JS::RootedObject props(context, &args[0].toObject());
+    JS::RootedId prop_id(context);
+    JS::AutoIdArray ids(context, JS_Enumerate(context, props));
+    JS::RootedValue value(context);
+    if (!ids) {
         gjs_throw(context, "Failed to create property iterator for object props hash");
         goto free_array_and_fail;
     }
 
-    if (!JS_NextProperty(context, iter, prop_id.address()))
-        goto free_array_and_fail;
-
-    while (!JSID_IS_VOID(prop_id)) {
+    for (ix = 0, length = ids.length(); ix < length; ix++) {
         char *name = NULL;
-        JS::RootedValue value(context);
         GParameter gparam = { NULL, { 0, }};
 
+        /* ids[ix] is reachable because props is rooted, but require_property
+         * doesn't know that */
+        prop_id = ids[ix];
+
         if (!gjs_object_require_property(context, props, "property list", prop_id, &value)) {
             goto free_array_and_fail;
         }
@@ -878,10 +879,6 @@ object_instance_props_to_g_parameters(JSContext                  *context,
         g_free(name);
 
         gparams.push_back(gparam);
-
-        prop_id = JSID_VOID;
-        if (!JS_NextProperty(context, iter, prop_id.address()))
-            goto free_array_and_fail;
     }
 
     return true;
diff --git a/gjs/importer.cpp b/gjs/importer.cpp
index f0f8877..9f96f5c 100644
--- a/gjs/importer.cpp
+++ b/gjs/importer.cpp
@@ -397,41 +397,28 @@ load_module_init(JSContext       *context,
 }
 
 static void
-load_module_elements(JSContext        *context,
+load_module_elements(JSContext        *cx,
                      JS::HandleObject  in_object,
                      ImporterIterator *iter,
                      const char       *init_path)
 {
-    JS::RootedObject jsiter(context),
-        module_obj(context, load_module_init(context, in_object, init_path));
+    size_t ix, length;
+    JS::RootedObject module_obj(cx, load_module_init(cx, in_object, init_path));
 
-    if (module_obj != NULL) {
-        jsid idp;
+    if (module_obj == NULL)
+        return;
 
-        jsiter = JS_NewPropertyIterator(context, module_obj);
+    JS::AutoIdArray ids(cx, JS_Enumerate(cx, module_obj));
+    if (!ids)
+        return;
 
-        if (jsiter == NULL) {
-            return;
-        }
-
-        if (!JS_NextProperty(context, jsiter, &idp)) {
-            return;
-        }
-
-        while (!JSID_IS_VOID(idp)) {
-            char *name;
-
-            if (!gjs_get_string_id(context, idp, &name)) {
-                continue;
-            }
-
-            /* Pass ownership of name */
-            g_ptr_array_add(iter->elements, name);
+    for (ix = 0, length = ids.length(); ix < length; ix++) {
+        char *name;
+        if (!gjs_get_string_id(cx, ids[ix], &name))
+            continue;
 
-            if (!JS_NextProperty(context, jsiter, &idp)) {
-                break;
-            }
-        }
+        /* Pass ownership of name */
+        g_ptr_array_add(iter->elements, name);
     }
 }