[gjs: 3/4] importer: Do JSString-to-UTF8 conversion as late as possible

[Cosimo Cecchi <cosimoc src gnome org>]

commit 0c0d30e9ffb5ff3e3806cee746915af93dc2a0af
Author: Philip Chimento <philip chimento gmail com>
Date:   Sat Oct 13 13:31:09 2018 -0700

    importer: Do JSString-to-UTF8 conversion as late as possible
    
    This does not matter much for performance, but it's possible to postpone
    the expensive string-to-UTF8 conversion in the resolve hook until after
    an error check. I'm just doing this because I'm trying to consistently
    move the conversion as late as possible everywhere.

 gjs/importer.cpp | 61 ++++++++++++++++++++++++++++----------------------------
 1 file changed, 30 insertions(+), 31 deletions(-)
---
diff --git a/gjs/importer.cpp b/gjs/importer.cpp
index c9f8400c..894ad135 100644
--- a/gjs/importer.cpp
+++ b/gjs/importer.cpp
@@ -442,13 +442,8 @@ import_file_on_module(JSContext       *context,
     return retval;
 }
 
-static bool
-do_import(JSContext       *context,
-          JS::HandleObject obj,
-          Importer        *priv,
-          JS::HandleId     id,
-          const char      *name)
-{
+static bool do_import(JSContext* context, JS::HandleObject obj, Importer* priv,
+                      JS::HandleId id) {
     JS::RootedObject search_path(context);
     guint32 search_path_len;
     guint32 i;
@@ -470,21 +465,26 @@ do_import(JSContext       *context,
         return false;
     }
 
-    GjsAutoChar filename = g_strdup_printf("%s.js", name);
-    std::vector<GjsAutoChar> directories;
-    JS::RootedValue elem(context);
-    JS::RootedString str(context);
+    JS::UniqueChars name;
+    if (!gjs_get_string_id(context, id, &name))
+        return false;
 
     /* First try importing an internal module like gi */
-    if (priv->is_root && gjs_is_registered_native_module(context, obj, name)) {
-        if (!gjs_import_native_module(context, obj, name))
+    if (priv->is_root &&
+        gjs_is_registered_native_module(context, obj, name.get())) {
+        if (!gjs_import_native_module(context, obj, name.get()))
             return false;
 
-        gjs_debug(GJS_DEBUG_IMPORTER,
-                  "successfully imported module '%s'", name);
+        gjs_debug(GJS_DEBUG_IMPORTER, "successfully imported module '%s'",
+                  name.get());
         return true;
     }
 
+    GjsAutoChar filename = g_strdup_printf("%s.js", name.get());
+    std::vector<GjsAutoChar> directories;
+    JS::RootedValue elem(context);
+    JS::RootedString str(context);
+
     for (i = 0; i < search_path_len; ++i) {
         elem.setUndefined();
         if (!JS_GetElement(context, search_path, i, &elem)) {
@@ -513,20 +513,21 @@ do_import(JSContext       *context,
 
         /* Try importing __init__.js and loading the symbol from it */
         bool found = false;
-        if (!import_symbol_from_init_js(context, obj, dirname.get(), name,
+        if (!import_symbol_from_init_js(context, obj, dirname.get(), name.get(),
                                         &found))
             return false;
         if (found)
             return true;
 
         /* Second try importing a directory (a sub-importer) */
-        GjsAutoChar full_path = g_build_filename(dirname.get(), name, nullptr);
+        GjsAutoChar full_path =
+            g_build_filename(dirname.get(), name.get(), nullptr);
         GjsAutoUnref<GFile> gfile = g_file_new_for_commandline_arg(full_path);
 
         if (g_file_query_file_type(gfile, (GFileQueryInfoFlags) 0, NULL) == G_FILE_TYPE_DIRECTORY) {
             gjs_debug(GJS_DEBUG_IMPORTER,
                       "Adding directory '%s' to child importer '%s'",
-                      full_path.get(), name);
+                      full_path.get(), name.get());
             directories.push_back(std::move(full_path));
         }
 
@@ -545,15 +546,14 @@ do_import(JSContext       *context,
         exists = g_file_query_exists(gfile, NULL);
 
         if (!exists) {
-            gjs_debug(GJS_DEBUG_IMPORTER,
-                      "JS import '%s' not found in %s",
-                      name, dirname.get());
+            gjs_debug(GJS_DEBUG_IMPORTER, "JS import '%s' not found in %s",
+                      name.get(), dirname.get());
             continue;
         }
 
-        if (import_file_on_module(context, obj, id, name, gfile)) {
-            gjs_debug(GJS_DEBUG_IMPORTER,
-                      "successfully imported module '%s'", name);
+        if (import_file_on_module(context, obj, id, name.get(), gfile)) {
+            gjs_debug(GJS_DEBUG_IMPORTER, "successfully imported module '%s'",
+                      name.get());
             return true;
         }
 
@@ -570,13 +570,13 @@ do_import(JSContext       *context,
         for (size_t ix = 0; ix < directories.size(); ix++)
             full_paths[ix] = directories[ix].get();
 
-        bool result = import_directory(context, obj, name, full_paths);
+        bool result = import_directory(context, obj, name.get(), full_paths);
         g_free(full_paths);
         if (!result)
             return false;
 
-        gjs_debug(GJS_DEBUG_IMPORTER,
-                  "successfully imported directory '%s'", name);
+        gjs_debug(GJS_DEBUG_IMPORTER, "successfully imported directory '%s'",
+                  name.get());
         return true;
     }
 
@@ -584,7 +584,7 @@ do_import(JSContext       *context,
      * end of the path. Be sure an exception is set. */
     g_assert(!JS_IsExceptionPending(context));
     gjs_throw_custom(context, JSProto_Error, "ImportError",
-                     "No JS module '%s' found in search path", name);
+                     "No JS module '%s' found in search path", name.get());
     return false;
 }
 
@@ -739,13 +739,12 @@ importer_resolve(JSContext        *context,
 
     JSAutoRequest ar(context);
 
-    JS::UniqueChars name;
-    if (!gjs_get_string_id(context, id, &name)) {
+    if (!JSID_IS_STRING(id)) {
         *resolved = false;
         return true;
     }
 
-    if (!do_import(context, obj, priv, id, name.get()))
+    if (!do_import(context, obj, priv, id))
         return false;
 
     *resolved = true;