[gjs: 9/16] gerror: Split gjs_throw_g_error()

[Cosimo Cecchi <cosimoc src gnome org>]

commit 61b1cedb1a503431138406ec1f32c26415b93fe2
Author: Philip Chimento <philip chimento gmail com>
Date:   Mon Sep 17 20:57:27 2018 -0700

    gerror: Split gjs_throw_g_error()
    
    This moves gjs_throw_g_error() to gi/gerror.cpp (and renames it
    gjs_throw_gerror() for consistency with the other functions in that
    file.)
    
    We also create gjs_throw_gerror_message() which we use in a situation
    where we don't have to marshal the GError into JavaScript. It simply
    copies the GError message into a regular JavaScript Exception object.
    This should be used inside internal functions, for example while
    converting strings to bytes, where the caller wouldn't necessarily expect
    to receive a GError.
    
    This allows more decoupling of the jsapi-util-* files from the
    gobject-introspection stuff.

 gi/function.cpp           | 11 ++++-------
 gi/gerror.cpp             | 25 +++++++++++++++++++++++++
 gi/gerror.h               |  2 ++
 gjs/byteArray.cpp         | 14 ++++----------
 gjs/global.cpp            |  6 ++----
 gjs/importer.cpp          |  2 +-
 gjs/jsapi-util-error.cpp  | 36 +++++++++++++++---------------------
 gjs/jsapi-util-string.cpp |  6 ++----
 gjs/jsapi-util.h          |  3 +--
 gjs/module.cpp            |  6 ++----
 10 files changed, 58 insertions(+), 53 deletions(-)
---
diff --git a/gi/function.cpp b/gi/function.cpp
index 10645c7b..0491187b 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -1382,8 +1382,7 @@ release:
     }
 
     if (!failed && did_throw_gerror) {
-        gjs_throw_g_error(context, local_error);
-        return false;
+        return gjs_throw_gerror(context, local_error);
     } else if (failed) {
         return false;
     } else {
@@ -1602,8 +1601,7 @@ init_cached_function_data (JSContext      *context,
         if (!g_function_info_prep_invoker((GIFunctionInfo *)info,
                                           &(function->invoker),
                                           &error)) {
-            gjs_throw_g_error(context, error);
-            return false;
+            return gjs_throw_gerror(context, error);
         }
     } else if (info_type == GI_INFO_TYPE_VFUNC) {
         gpointer addr;
@@ -1611,7 +1609,7 @@ init_cached_function_data (JSContext      *context,
         addr = g_vfunc_info_get_address((GIVFuncInfo *)info, gtype, &error);
         if (error != NULL) {
             if (error->code != G_INVOKE_ERROR_SYMBOL_NOT_FOUND)
-                gjs_throw_g_error(context, error);
+                return gjs_throw_gerror(context, error);
 
             g_clear_error(&error);
             return false;
@@ -1620,8 +1618,7 @@ init_cached_function_data (JSContext      *context,
         if (!g_function_invoker_new_for_address(addr, info,
                                                 &(function->invoker),
                                                 &error)) {
-            gjs_throw_g_error(context, error);
-            return false;
+            return gjs_throw_gerror(context, error);
         }
     }
 
diff --git a/gi/gerror.cpp b/gi/gerror.cpp
index 7f04e8d4..703be0d3 100644
--- a/gi/gerror.cpp
+++ b/gi/gerror.cpp
@@ -585,3 +585,28 @@ gjs_gerror_make_from_error(JSContext       *cx,
 
     return g_error_new_literal(GJS_JS_ERROR, code, message);
 }
+
+/*
+ * gjs_throw_gerror:
+ *
+ * Converts a GError into a JavaScript exception, and frees the GError.
+ * Differently from gjs_throw(), it will overwrite an existing exception, as it
+ * is used to report errors from C functions.
+ *
+ * Returns: false, for convenience in returning from the calling function.
+ */
+bool gjs_throw_gerror(JSContext* cx, GError* error) {
+    // return false even if the GError is null, as presumably something failed
+    // in the calling code, and the caller expects to throw.
+    g_return_val_if_fail(error, false);
+
+    JSAutoRequest ar(cx);
+
+    JS::RootedValue err(
+        cx, JS::ObjectOrNullValue(gjs_error_from_gerror(cx, error, true)));
+    g_error_free(error);
+    if (!err.isNull())
+        JS_SetPendingException(cx, err);
+
+    return false;
+}
diff --git a/gi/gerror.h b/gi/gerror.h
index 8f7d549c..03e0c52d 100644
--- a/gi/gerror.h
+++ b/gi/gerror.h
@@ -47,6 +47,8 @@ bool      gjs_typecheck_gerror         (JSContext             *context,
 GError *gjs_gerror_make_from_error(JSContext       *cx,
                                    JS::HandleObject obj);
 
+bool gjs_throw_gerror(JSContext* cx, GError* error);
+
 G_END_DECLS
 
 #endif  /* __GJS_ERROR_H__ */
diff --git a/gjs/byteArray.cpp b/gjs/byteArray.cpp
index af238537..5d13ebed 100644
--- a/gjs/byteArray.cpp
+++ b/gjs/byteArray.cpp
@@ -93,11 +93,8 @@ static bool to_string_impl(JSContext* context, JS::HandleObject byte_array,
 #endif
                                         encoding, nullptr, /* bytes read */
                                         &bytes_written, &error);
-        if (u16_str == NULL) {
-            /* frees the GError */
-            gjs_throw_g_error(context, error);
-            return false;
-        }
+        if (!u16_str)
+            return gjs_throw_gerror_message(context, error);  // frees GError
 
         /* bytes_written should be bytes in a UTF-16 string so
          * should be a multiple of 2
@@ -240,11 +237,8 @@ from_string_func(JSContext *context,
             }
         }
 
-        if (encoded == NULL) {
-            /* frees the GError */
-            gjs_throw_g_error(context, error);
-            return false;
-        }
+        if (!encoded)
+            return gjs_throw_gerror_message(context, error);  // frees GError
 
         array_buffer =
             JS_NewExternalArrayBuffer(context, bytes_written, encoded, nullptr,
diff --git a/gjs/global.cpp b/gjs/global.cpp
index d18e4f90..02712e16 100644
--- a/gjs/global.cpp
+++ b/gjs/global.cpp
@@ -41,10 +41,8 @@ run_bootstrap(JSContext       *cx,
     std::unique_ptr<GBytes, decltype(&g_bytes_unref)> script_bytes(
         g_resources_lookup_data(path, G_RESOURCE_LOOKUP_FLAGS_NONE, &error),
         g_bytes_unref);
-    if (!script_bytes) {
-        gjs_throw_g_error(cx, error);
-        return false;
-    }
+    if (!script_bytes)
+        return gjs_throw_gerror_message(cx, error);
 
     JSAutoCompartment ac(cx, global);
 
diff --git a/gjs/importer.cpp b/gjs/importer.cpp
index 13dc7b15..ce4cd9d7 100644
--- a/gjs/importer.cpp
+++ b/gjs/importer.cpp
@@ -300,7 +300,7 @@ import_module_init(JSContext       *context,
         if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_IS_DIRECTORY) &&
             !g_error_matches(error, G_IO_ERROR, G_IO_ERROR_NOT_DIRECTORY) &&
             !g_error_matches(error, G_IO_ERROR, G_IO_ERROR_NOT_FOUND))
-            gjs_throw_g_error(context, error);
+            gjs_throw_gerror_message(context, error);
         else
             g_error_free(error);
 
diff --git a/gjs/jsapi-util-error.cpp b/gjs/jsapi-util-error.cpp
index 7657ffbb..b5e651dc 100644
--- a/gjs/jsapi-util-error.cpp
+++ b/gjs/jsapi-util-error.cpp
@@ -179,29 +179,23 @@ gjs_throw_literal(JSContext       *context,
 }
 
 /**
- * gjs_throw_g_error:
+ * gjs_throw_gerror_message:
  *
- * Convert a GError into a JavaScript Exception, and
- * frees the GError. Differently from gjs_throw(), it
- * will overwrite an existing exception, as it is used
- * to report errors from C functions.
+ * Similar to gjs_throw_gerror(), but does not marshal the GError structure into
+ * JavaScript. Instead, it creates a regular JavaScript Error object and copies
+ * the GError's message into it.
+ *
+ * Use this when handling a GError in an internal function, where the error code
+ * and domain don't matter. So, for example, don't use it to throw errors
+ * around calling from JS into C code.
+ *
+ * Frees the GError.
  */
-void
-gjs_throw_g_error (JSContext       *context,
-                   GError          *error)
-{
-    if (error == NULL)
-        return;
-
-    JS_BeginRequest(context);
-
-    JS::RootedValue err(context,
-        JS::ObjectOrNullValue(gjs_error_from_gerror(context, error, true)));
-    g_error_free (error);
-    if (!err.isNull())
-        JS_SetPendingException(context, err);
-
-    JS_EndRequest(context);
+bool gjs_throw_gerror_message(JSContext* cx, GError* error) {
+    g_return_val_if_fail(error, false);
+    gjs_throw_literal(cx, error->message);
+    g_error_free(error);
+    return false;
 }
 
 /**
diff --git a/gjs/jsapi-util-string.cpp b/gjs/jsapi-util-string.cpp
index f3e2c02a..6ec7c230 100644
--- a/gjs/jsapi-util-string.cpp
+++ b/gjs/jsapi-util-string.cpp
@@ -121,10 +121,8 @@ gjs_string_to_filename(JSContext      *context,
 
     error = NULL;
     *filename_string = g_filename_from_utf8(tmp, -1, NULL, NULL, &error);
-    if (!*filename_string) {
-        gjs_throw_g_error(context, error);
-        return false;
-    }
+    if (!*filename_string)
+        return gjs_throw_gerror_message(context, error);
 
     return true;
 }
diff --git a/gjs/jsapi-util.h b/gjs/jsapi-util.h
index f206ddfe..2391b2c3 100644
--- a/gjs/jsapi-util.h
+++ b/gjs/jsapi-util.h
@@ -206,8 +206,7 @@ void        gjs_throw_custom                 (JSContext       *context,
                                               ...)  G_GNUC_PRINTF (4, 5);
 void        gjs_throw_literal                (JSContext       *context,
                                               const char      *string);
-void        gjs_throw_g_error                (JSContext       *context,
-                                              GError          *error);
+bool gjs_throw_gerror_message(JSContext* cx, GError* error);
 
 bool        gjs_log_exception                (JSContext       *context);
 
diff --git a/gjs/module.cpp b/gjs/module.cpp
index ad6fb68f..417091a0 100644
--- a/gjs/module.cpp
+++ b/gjs/module.cpp
@@ -124,10 +124,8 @@ class GjsModule {
         int start_line_number = 1;
 
         if (!(g_file_load_contents(file, nullptr, &unowned_script, &script_len,
-                                   nullptr, &error))) {
-            gjs_throw_g_error(cx, error);
-            return false;
-        }
+                                   nullptr, &error)))
+            return gjs_throw_gerror_message(cx, error);
 
         GjsAutoChar script = unowned_script;  /* steals ownership */
         g_assert(script != nullptr);