[gjs/wip/ptomato/mozjs38: 15/17] WIP - Switch to new JSClass.enumerate

[Philip Chimento <pchimento src gnome org>]

commit 97c0f92792725d2fe84fa05f54009b6b38a55c91
Author: Philip Chimento <philip chimento gmail com>
Date:   Wed Jan 11 23:38:42 2017 -0800

    WIP - Switch to new JSClass.enumerate
    
    JSNewEnumerateOp and JSEnumerateOp are merged into something completely
    different. In practice SpiderMonkey was not doing lazy enumeration, so
    they decided to change the enumerate callback from being called
    incrementally for each property, to just being called once the first time
    an object's properties are enumerated.
    
    This doesn't work properly yet. The JS::AutoIdVector is not passed into
    the callback anymore, so the callback should define all the properties on
    the object.

 gjs/importer.cpp |  249 ++++++++++++++++--------------------------------------
 1 files changed, 74 insertions(+), 175 deletions(-)
---
diff --git a/gjs/importer.cpp b/gjs/importer.cpp
index 5c9bc82..b03f713 100644
--- a/gjs/importer.cpp
+++ b/gjs/importer.cpp
@@ -392,10 +392,9 @@ load_module_init(JSContext       *context,
 static void
 load_module_elements(JSContext        *cx,
                      JS::HandleObject  in_object,
-                     ImporterIterator *iter,
+                     JS::AutoIdVector& prop_ids,
                      const char       *init_path)
 {
-    size_t ix, length;
     JS::RootedObject module_obj(cx, load_module_init(cx, in_object, init_path));
 
     if (module_obj == NULL)
@@ -405,14 +404,8 @@ load_module_elements(JSContext        *cx,
     if (!ids)
         return;
 
-    for (ix = 0, length = ids.length(); ix < length; ix++) {
-        char *name;
-        if (!gjs_get_string_id(cx, ids[ix], &name))
-            continue;
-
-        /* Pass ownership of name */
-        g_ptr_array_add(iter->elements, name);
-    }
+    for (size_t ix = 0; ix < ids.length(); ix++)
+        prop_ids.append(ids[ix]);
 }
 
 static bool
@@ -657,201 +650,108 @@ do_import(JSContext       *context,
     return result;
 }
 
-static ImporterIterator *
-importer_iterator_new(void)
-{
-    ImporterIterator *iter;
-
-    iter = g_slice_new0(ImporterIterator);
-
-    iter->elements = g_ptr_array_new();
-    iter->index = 0;
-
-    return iter;
-}
-
-static void
-importer_iterator_free(ImporterIterator *iter)
-{
-    g_ptr_array_foreach(iter->elements, (GFunc)g_free, NULL);
-    g_ptr_array_free(iter->elements, true);
-    g_slice_free(ImporterIterator, iter);
-}
-
-/*
- * Like JSEnumerateOp, but enum provides contextual information as follows:
- *
- * JSENUMERATE_INIT: allocate private enum struct in state_p, return number
- * of elements in *id_p
- * JSENUMERATE_NEXT: return next property id in *id_p, and if no new property
- * free state_p and set to JS::NullValue()
- * JSENUMERATE_DESTROY : destroy state_p
- *
- * Note that in a for ... in loop, this will be called first on the object,
+/* Note that in a for ... in loop, this will be called first on the object,
  * then on its prototype.
- *
  */
 static bool
-importer_new_enumerate(JSContext  *context,
-                       JS::HandleObject object,
-                       JSIterateOp enum_op,
-                       JS::MutableHandleValue statep,
-                       JS::MutableHandleId idp)
+importer_enumerate(JSContext        *context,
+                   JS::HandleObject  object,
+                   JS::AutoIdVector& properties)
 {
-    ImporterIterator *iter;
+    Importer *priv;
+    guint32 search_path_len;
+    guint32 i;
 
-    switch (enum_op) {
-    case JSENUMERATE_INIT_ALL:
-    case JSENUMERATE_INIT: {
-        Importer *priv;
-        JS::RootedObject search_path(context);
-        guint32 search_path_len;
-        guint32 i;
+    priv = priv_from_js(context, object);
 
-        statep.setNull();
+    if (!priv)
+        /* we are enumerating the prototype properties */
+        return true;
 
-        idp.set(INT_TO_JSID(0));
+    JS::RootedObject search_path(context);
+    JS::RootedId search_path_name(context,
+        gjs_context_get_const_string(context, GJS_STRING_SEARCH_PATH));
+    if (!gjs_object_require_property_value(context, object, "importer",
+                                           search_path_name, &search_path))
+        return false;
 
-        priv = priv_from_js(context, object);
+    if (!JS_IsArrayObject(context, search_path)) {
+        gjs_throw(context, "searchPath property on importer is not an array");
+        return false;
+    }
 
-        if (!priv)
-            /* we are enumerating the prototype properties */
-            return true;
+    if (!JS_GetArrayLength(context, search_path, &search_path_len)) {
+        gjs_throw(context, "searchPath array has no length");
+        return false;
+    }
 
-        JS::RootedId search_path_name(context,
-            gjs_context_get_const_string(context, GJS_STRING_SEARCH_PATH));
-        if (!gjs_object_require_property_value(context, object, "importer",
-                                               search_path_name, &search_path))
-            return false;
+    JS::RootedValue elem(context);
+    for (i = 0; i < search_path_len; ++i) {
+        char *dirname = NULL;
+        char *init_path;
+        const char *filename;
+        GDir *dir = NULL;
 
-        if (!JS_IsArrayObject(context, search_path)) {
-            gjs_throw(context, "searchPath property on importer is not an array");
+        elem.setUndefined();
+        if (!JS_GetElement(context, search_path, i, &elem)) {
+            /* this means there was an exception, while elem.isUndefined()
+             * means no element found
+             */
             return false;
         }
 
-        if (!JS_GetArrayLength(context, search_path, &search_path_len)) {
-            gjs_throw(context, "searchPath array has no length");
+        if (elem.isUndefined())
+            continue;
+
+        if (!elem.isString()) {
+            gjs_throw(context, "importer searchPath contains non-string");
             return false;
         }
 
-        iter = importer_iterator_new();
-
-        JS::RootedValue elem(context);
-        for (i = 0; i < search_path_len; ++i) {
-            char *dirname = NULL;
-            char *init_path;
-            const char *filename;
-            GDir *dir = NULL;
-
-            elem = JS::UndefinedValue();
-            if (!JS_GetElement(context, search_path, i, &elem)) {
-                /* this means there was an exception, while elem.isUndefined()
-                 * means no element found
-                 */
-                importer_iterator_free(iter);
-                return false;
-            }
-
-            if (elem.isUndefined())
-                continue;
-
-            if (!elem.isString()) {
-                gjs_throw(context, "importer searchPath contains non-string");
-                importer_iterator_free(iter);
-                return false;
-            }
-
-            if (!gjs_string_to_utf8(context, elem, &dirname)) {
-                importer_iterator_free(iter);
-                return false; /* Error message already set */
-            }
-
-            init_path = g_build_filename(dirname, MODULE_INIT_FILENAME,
-                                         NULL);
-
-            load_module_elements(context, object, iter, init_path);
-
-            g_free(init_path);
-
-            dir = g_dir_open(dirname, 0, NULL);
-
-            if (!dir) {
-                g_free(dirname);
-                continue;
-            }
-
-            while ((filename = g_dir_read_name(dir))) {
-                char *full_path;
-
-                /* skip hidden files and directories (.svn, .git, ...) */
-                if (filename[0] == '.')
-                    continue;
+        if (!gjs_string_to_utf8(context, elem, &dirname)) {
+            return false; /* Error message already set */
+        }
 
-                /* skip module init file */
-                if (strcmp(filename, MODULE_INIT_FILENAME) == 0)
-                    continue;
+        init_path = g_build_filename(dirname, MODULE_INIT_FILENAME, NULL);
 
-                full_path = g_build_filename(dirname, filename, NULL);
+        load_module_elements(context, object, properties, init_path);
 
-                if (g_file_test(full_path, G_FILE_TEST_IS_DIR)) {
-                    g_ptr_array_add(iter->elements, g_strdup(filename));
-                } else {
-                    if (g_str_has_suffix(filename, "." G_MODULE_SUFFIX) ||
-                        g_str_has_suffix(filename, ".js")) {
-                        g_ptr_array_add(iter->elements,
-                                        g_strndup(filename, strlen(filename) - 3));
-                    }
-                }
+        g_free(init_path);
 
-                g_free(full_path);
-            }
-            g_dir_close(dir);
+        dir = g_dir_open(dirname, 0, NULL);
 
+        if (!dir) {
             g_free(dirname);
+            continue;
         }
 
-        statep.set(JS::PrivateValue(context));
-
-        idp.set(INT_TO_JSID(iter->elements->len));
+        while ((filename = g_dir_read_name(dir))) {
+            char *full_path;
 
-        break;
-    }
-
-    case JSENUMERATE_NEXT: {
-        if (statep.isNull()) /* Iterating prototype */
-            return true;
-
-        iter = (ImporterIterator*) statep.get().toPrivate();
-
-        if (iter->index < iter->elements->len) {
-            JS::RootedValue element_val(context);
-            if (!gjs_string_from_utf8(context,
-                                         (const char*) g_ptr_array_index(iter->elements,
-                                                           iter->index++),
-                                         -1,
-                                         &element_val))
-                return false;
-
-            if (!JS_ValueToId(context, element_val, idp))
-                return false;
+            /* skip hidden files and directories (.svn, .git, ...) */
+            if (filename[0] == '.')
+                continue;
 
-            break;
-        }
-        /* else fall through to destroying the iterator */
-    }
+            /* skip module init file */
+            if (strcmp(filename, MODULE_INIT_FILENAME) == 0)
+                continue;
 
-    case JSENUMERATE_DESTROY: {
-        if (!statep.isNull()) {
-            iter = (ImporterIterator*) statep.get().toPrivate();
+            full_path = g_build_filename(dirname, filename, NULL);
 
-            importer_iterator_free(iter);
+            if (g_file_test(full_path, G_FILE_TEST_IS_DIR)) {
+                properties.append(gjs_intern_string_to_id(context, filename));
+            } else if (g_str_has_suffix(filename, "." G_MODULE_SUFFIX) ||
+                       g_str_has_suffix(filename, ".js")) {
+                g_autofree char *filename_noext =
+                    g_strndup(filename, strlen(filename) - 3);
+                properties.append(gjs_intern_string_to_id(context, filename_noext));
+            }
 
-            statep.setNull();
+            g_free(full_path);
         }
-    }
+        g_dir_close(dir);
 
-    default:
-        ;
+        g_free(dirname);
     }
 
     return true;
@@ -932,13 +832,12 @@ importer_finalize(JSFreeOp *fop,
 struct JSClass gjs_importer_class = {
     "GjsFileImporter",
     JSCLASS_HAS_PRIVATE |
-    JSCLASS_NEW_ENUMERATE |
     JSCLASS_IMPLEMENTS_BARRIERS,
     NULL,  /* addProperty */
     NULL,  /* deleteProperty */
     NULL,  /* getProperty */
     NULL,  /* setProperty */
-    (JSEnumerateOp) importer_new_enumerate, /* needs cast since it's the new enumerate signature */
+    NULL, // FIXME FIXME importer_enumerate,
     importer_resolve,
     NULL,  /* convert */
     importer_finalize