[gjs: 13/16] jsapi-util: Fix error handling in gjs_get_string_id()

[Philip Chimento <pchimento src gnome org>]

commit ed523c6d8acef93413bb51ea0c26077e4cf79692
Author: Philip Chimento <philip chimento gmail com>
Date:   Sun Oct 28 20:14:49 2018 -0400

    jsapi-util: Fix error handling in gjs_get_string_id()
    
    This makes gjs_get_string_id() conform to the JSAPI return convention, so
    that it is clear when an exception is pending.

 gi/boxed.cpp              |  4 +++-
 gi/fundamental.cpp        |  4 +++-
 gi/interface.cpp          |  4 +++-
 gi/ns.cpp                 |  4 +++-
 gi/object.cpp             |  4 +++-
 gi/param.cpp              |  4 +++-
 gi/repo.cpp               |  4 ++++
 gi/union.cpp              |  4 +++-
 gjs/importer.cpp          |  4 ++++
 gjs/jsapi-util-string.cpp | 26 +++++++++++---------------
 10 files changed, 40 insertions(+), 22 deletions(-)
---
diff --git a/gi/boxed.cpp b/gi/boxed.cpp
index 86d08ffc..5fd9f639 100644
--- a/gi/boxed.cpp
+++ b/gi/boxed.cpp
@@ -142,7 +142,9 @@ boxed_resolve(JSContext       *context,
     }
 
     JS::UniqueChars name;
-    if (!gjs_get_string_id(context, id, &name)) {
+    if (!gjs_get_string_id(context, id, &name))
+        return false;
+    if (!name) {
         *resolved = false;
         return true;
     }
diff --git a/gi/fundamental.cpp b/gi/fundamental.cpp
index 30409140..85400ea1 100644
--- a/gi/fundamental.cpp
+++ b/gi/fundamental.cpp
@@ -317,7 +317,9 @@ fundamental_instance_resolve(JSContext       *context,
     }
 
     JS::UniqueChars name;
-    if (!gjs_get_string_id(context, id, &name)) {
+    if (!gjs_get_string_id(context, id, &name))
+        return false;
+    if (!name) {
         *resolved = false;
         return true; /* not resolved, but no error */
     }
diff --git a/gi/interface.cpp b/gi/interface.cpp
index 2d4c6bae..3f0372e5 100644
--- a/gi/interface.cpp
+++ b/gi/interface.cpp
@@ -125,7 +125,9 @@ interface_resolve(JSContext       *context,
     }
 
     JS::UniqueChars name;
-    if (!gjs_get_string_id(context, id, &name)) {
+    if (!gjs_get_string_id(context, id, &name))
+        return false;
+    if (!name) {
         *resolved = false;
         return true;
     }
diff --git a/gi/ns.cpp b/gi/ns.cpp
index 9ddac91c..dc309e5f 100644
--- a/gi/ns.cpp
+++ b/gi/ns.cpp
@@ -79,7 +79,9 @@ ns_resolve(JSContext       *context,
     }
 
     JS::UniqueChars name;
-    if (!gjs_get_string_id(context, id, &name)) {
+    if (!gjs_get_string_id(context, id, &name))
+        return false;
+    if (!name) {
         *resolved = false;
         return true;  /* not resolved, but no error */
     }
diff --git a/gi/object.cpp b/gi/object.cpp
index 30d57d24..9d19672e 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -840,7 +840,9 @@ bool ObjectPrototype::resolve_impl(JSContext* context, JS::HandleObject obj,
     debug_jsprop("Resolve hook", id, obj);
 
     JS::UniqueChars name;
-    if (!gjs_get_string_id(context, id, &name)) {
+    if (!gjs_get_string_id(context, id, &name))
+        return false;
+    if (!name) {
         *resolved = false;
         return true;  /* not resolved, but no error */
     }
diff --git a/gi/param.cpp b/gi/param.cpp
index 226bfc69..265e51f2 100644
--- a/gi/param.cpp
+++ b/gi/param.cpp
@@ -64,7 +64,9 @@ param_resolve(JSContext       *context,
     }
 
     JS::UniqueChars name;
-    if (!gjs_get_string_id(context, id, &name)) {
+    if (!gjs_get_string_id(context, id, &name))
+        return false;
+    if (!name) {
         *resolved = false;
         return true; /* not resolved, but no error */
     }
diff --git a/gi/repo.cpp b/gi/repo.cpp
index 0f34c84a..adefb8bf 100644
--- a/gi/repo.cpp
+++ b/gi/repo.cpp
@@ -93,6 +93,10 @@ static bool resolve_namespace_object(JSContext* context,
     JS::UniqueChars ns_name;
     if (!gjs_get_string_id(context, ns_id, &ns_name))
         return false;
+    if (!ns_name) {
+        gjs_throw(context, "Requiring invalid namespace on imports.gi");
+        return false;
+    }
 
     GList* versions = g_irepository_enumerate_versions(nullptr, ns_name.get());
     unsigned nversions = g_list_length(versions);
diff --git a/gi/union.cpp b/gi/union.cpp
index 9d148bdf..9852080b 100644
--- a/gi/union.cpp
+++ b/gi/union.cpp
@@ -81,7 +81,9 @@ union_resolve(JSContext       *context,
     }
 
     JS::UniqueChars name;
-    if (!gjs_get_string_id(context, id, &name)) {
+    if (!gjs_get_string_id(context, id, &name))
+        return false;
+    if (!name) {
         *resolved = false;
         return true; /* not resolved, but no error */
     }
diff --git a/gjs/importer.cpp b/gjs/importer.cpp
index 3deb2a99..fd178046 100644
--- a/gjs/importer.cpp
+++ b/gjs/importer.cpp
@@ -480,6 +480,10 @@ static bool do_import(JSContext* context, JS::HandleObject obj, Importer* priv,
     JS::UniqueChars name;
     if (!gjs_get_string_id(context, id, &name))
         return false;
+    if (!name) {
+        gjs_throw(context, "Importing invalid module name");
+        return false;
+    }
 
     /* First try importing an internal module like gi */
     if (priv->is_root &&
diff --git a/gjs/jsapi-util-string.cpp b/gjs/jsapi-util-string.cpp
index 950f7bf3..97f41768 100644
--- a/gjs/jsapi-util-string.cpp
+++ b/gjs/jsapi-util-string.cpp
@@ -315,28 +315,24 @@ gjs_string_from_ucs4(JSContext             *cx,
 
 /**
  * gjs_get_string_id:
- * @context: a #JSContext
+ * @cx: a #JSContext
  * @id: a jsid that is an object hash key (could be an int or string)
  * @name_p place to store ASCII string version of key
  *
- * If the id is not a string ID, return false and set *name_p to %NULL.
+ * If the id is not a string ID, return true and set *name_p to nullptr.
  * Otherwise, return true and fill in *name_p with ASCII name of id.
  *
- * Returns: true if *name_p is non-%NULL
+ * Returns: false on error, otherwise true
  **/
-bool gjs_get_string_id(JSContext* context, jsid id, JS::UniqueChars* name_p) {
-    JS::RootedValue id_val(context);
-
-    if (!JS_IdToValue(context, id, &id_val))
-        return false;
-
-    if (id_val.isString()) {
-        JS::RootedString str(context, id_val.toString());
-        name_p->reset(JS_EncodeStringToUTF8(context, str));
-        return !!*name_p;
-    } else {
-        return false;
+bool gjs_get_string_id(JSContext* cx, jsid id, JS::UniqueChars* name_p) {
+    if (!JSID_IS_STRING(id)) {
+        name_p->reset();
+        return true;
     }
+
+    JS::RootedString s(cx, JS_FORGET_STRING_FLATNESS(JSID_TO_FLAT_STRING(id)));
+    name_p->reset(JS_EncodeStringToUTF8(cx, s));
+    return !!*name_p;
 }
 
 /**