[gjs/wip/ptomato/730101: 19/19] WIP - overrides exceptions

[Philip Chimento <pchimento src gnome org>]

commit bca9a14395924e7871b1b3ebb86013d78f6c5374
Author: Philip Chimento <philip endlessm com>
Date:   Thu Dec 22 17:24:42 2016 -0700

    WIP - overrides exceptions

 Makefile-insttest.am                         |    2 +-
 Makefile-test.am                             |   15 +++++-
 gi/repo.cpp                                  |   75 ++++++++++++++++++++-----
 gjs/importer.cpp                             |    3 +-
 installed-tests/fail.c                       |    6 ++
 installed-tests/fail.h                       |    6 ++
 installed-tests/js/jsunit.gresources.xml     |    2 +
 installed-tests/js/modules/overrides/Fail.js |    2 +
 installed-tests/js/testImporter.js           |    4 ++
 9 files changed, 97 insertions(+), 18 deletions(-)
---
diff --git a/Makefile-insttest.am b/Makefile-insttest.am
index 1b1b5c5..3d84e9e 100644
--- a/Makefile-insttest.am
+++ b/Makefile-insttest.am
@@ -26,7 +26,7 @@ installedtestmeta_DATA +=             \
        $(NULL)
 jstests_DATA += $(jasmine_tests)
 jsscripttests_DATA += $(simple_tests)
-pkglib_LTLIBRARIES += libregress.la libwarnlib.la libgimarshallingtests.la
+pkglib_LTLIBRARIES += $(test_introspection_libraries)
 
 %.test: %.js installed-tests/minijasmine.test.in Makefile
        $(AM_V_GEN)$(MKDIR_P) $(@D) && \
diff --git a/Makefile-test.am b/Makefile-test.am
index b9ac608..6dc5e99 100644
--- a/Makefile-test.am
+++ b/Makefile-test.am
@@ -126,6 +126,12 @@ minijasmine_LDADD = $(GJS_LIBS) libgjs.la
 ### TEST GIRS ##########################################################
 
 TEST_INTROSPECTION_GIRS =
+test_introspection_libraries =         \
+       libfail.la                      \
+       libregress.la                   \
+       libwarnlib.la                   \
+       libgimarshallingtests.la        \
+       $(NULL)
 common_test_ldflags = -avoid-version
 common_test_libadd = $(GJS_LIBS)
 
@@ -136,7 +142,7 @@ if !BUILDOPT_INSTALL_TESTS
 common_test_ldflags += -rpath /nowhere
 # In the installed tests case, these libraries are built for pkglibdir; sadly we
 # can only have one destination at a time
-check_LTLIBRARIES += libregress.la libwarnlib.la libgimarshallingtests.la
+check_LTLIBRARIES += $(test_introspection_libraries)
 endif
 
 # These sources are installed as part of gobject-introspection, and symlinked
@@ -205,6 +211,13 @@ GIMarshallingTests_1_0_gir_SCANNERFLAGS =  \
        $(NULL)
 TEST_INTROSPECTION_GIRS += GIMarshallingTests-1.0.gir
 
+libfail_la_SOURCES = installed-tests/fail.c installed-tests/fail.h
+Fail-1.gir: libfail.la
+Fail_1_gir_LIBS = libfail.la
+Fail_1_gir_FILES = installed-tests/fail.h
+Fail_1_gir_SCANNERFLAGS = --symbol-prefix=fail $(WARN_SCANNERFLAGS)
+TEST_INTROSPECTION_GIRS += Fail-1.gir
+
 $(foreach gir,$(TEST_INTROSPECTION_GIRS),$(eval $(call introspection-scanner,$(gir))))
 TEST_INTROSPECTION_TYPELIBS = $(TEST_INTROSPECTION_GIRS:.gir=.typelib)
 noinst_DATA += $(TEST_INTROSPECTION_TYPELIBS)
diff --git a/gi/repo.cpp b/gi/repo.cpp
index 00c6e64..cabf192 100644
--- a/gi/repo.cpp
+++ b/gi/repo.cpp
@@ -54,7 +54,8 @@ extern struct JSClass gjs_repo_class;
 
 GJS_DEFINE_PRIV_FROM_JS(Repo, gjs_repo_class)
 
-static JSObject *lookup_override_function(JSContext *, JS::HandleId);
+static bool lookup_override_function(JSContext *, JS::HandleId,
+                                     JS::MutableHandleValue);
 
 static bool
 get_version_for_ns (JSContext       *context,
@@ -124,10 +125,12 @@ resolve_namespace_object(JSContext       *context,
                            GJS_MODULE_PROP_FLAGS))
         g_error("no memory to define ns property");
 
-    JS::RootedValue override(context,
-        JS::ObjectOrNullValue(lookup_override_function(context, ns_id)));
+    JS::RootedValue override(context);
+    if (!lookup_override_function(context, ns_id, &override))
+        return false;
+
     JS::RootedValue result(context);
-    if (!override.isNull() &&
+    if (!override.isUndefined() &&
         !JS_CallFunctionValue (context, gi_namespace, /* thisp */
                                override, /* callee */
                                JS::HandleValueArray::empty(), &result))
@@ -562,16 +565,44 @@ gjs_lookup_namespace_object(JSContext  *context,
     return gjs_lookup_namespace_object_by_name(context, ns_name);
 }
 
-static JSObject*
-lookup_override_function(JSContext   *cx,
-                         JS::HandleId ns_name)
+/* Check if an exception's 'name' property is equal to compare_name. Ignores
+ * all errors that might arise. Requires request. */
+static bool
+error_has_name(JSContext       *cx,
+               JS::HandleObject exception,
+               JSString        *compare_name)
+{
+    JS::AutoSaveExceptionState saved_exc(cx);
+    JS::RootedId name_id(cx, gjs_context_get_const_string(cx, GJS_STRING_NAME));
+    JS::RootedValue exc_name(cx);
+    bool retval = false;
+
+    if (!JS_GetPropertyById(cx, exception, name_id, &exc_name))
+        goto out;
+
+    int32_t cmp_result;
+    if (!JS_CompareStrings(cx, exc_name.toString(), compare_name, &cmp_result))
+        goto out;
+
+    if (cmp_result == 0)
+        retval = true;
+
+out:
+    saved_exc.restore();
+    return retval;
+}
+
+static bool
+lookup_override_function(JSContext             *cx,
+                         JS::HandleId           ns_name,
+                         JS::MutableHandleValue function)
 {
     JSAutoRequest ar(cx);
+    JS::AutoSaveExceptionState saved_exc(cx);
 
     JS::RootedValue importer(cx, gjs_get_global_slot(cx, GJS_GLOBAL_SLOT_IMPORTS));
     g_assert(importer.isObject());
 
-    JS::RootedValue function(cx);
     JS::RootedId overrides_name(cx,
         gjs_context_get_const_string(cx, GJS_STRING_GI_OVERRIDES));
     JS::RootedId object_init_name(cx,
@@ -581,22 +612,36 @@ lookup_override_function(JSContext   *cx,
 
     if (!gjs_object_require_property_value(cx, importer_obj, "importer",
                                            overrides_name, &overridespkg))
-        goto fail;
+        goto pass;
 
     if (!gjs_object_require_property_value(cx, overridespkg, "GI repository object",
-                                           ns_name, &module))
+                                           ns_name, &module)) {
+        JS::RootedValue exc_val(cx);
+        JS_GetPendingException(cx, &exc_val);
+
+        JS::RootedObject exc(cx, &exc_val.toObject());
+        /* If the exception was an ImportError (i.e., module not found) then
+         * we simply didn't have an override, don't throw an exception */
+        if (error_has_name(cx, exc, JS_InternString(cx, "ImportError")))
+            goto pass;
+
         goto fail;
+    }
 
     if (!gjs_object_require_property(cx, module, "override module",
-                                     object_init_name, &function) ||
-        !function.isObjectOrNull())
+                                     object_init_name, function) ||
+        !function.isObjectOrNull()) {
+        gjs_throw(cx, "Unexpected value for _init in overrides module");
         goto fail;
+    }
 
-    return function.toObjectOrNull();
+pass:
+    saved_exc.restore();
+    return true;
 
  fail:
-    JS_ClearPendingException(cx);
-    return NULL;
+    saved_exc.drop();
+    return false;
 }
 
 JSObject*
diff --git a/gjs/importer.cpp b/gjs/importer.cpp
index bd883f3..ce6ee14 100644
--- a/gjs/importer.cpp
+++ b/gjs/importer.cpp
@@ -592,7 +592,8 @@ do_import(JSContext       *context,
         /* If no exception occurred, the problem is just that we got to the
          * end of the path. Be sure an exception is set.
          */
-        gjs_throw(context, "No JS module '%s' found in search path", name);
+        gjs_throw_custom(context, "Error", "ImportError",
+                         "No JS module '%s' found in search path", name);
     }
 
     return result;
diff --git a/installed-tests/fail.c b/installed-tests/fail.c
new file mode 100644
index 0000000..51f097d
--- /dev/null
+++ b/installed-tests/fail.c
@@ -0,0 +1,6 @@
+#include "fail.h"
+
+void
+fail_hello (void)
+{
+}
diff --git a/installed-tests/fail.h b/installed-tests/fail.h
new file mode 100644
index 0000000..08d3b08
--- /dev/null
+++ b/installed-tests/fail.h
@@ -0,0 +1,6 @@
+#ifndef FAIL_H
+#define FAIL_H
+
+void fail_hello (void);
+
+#endif
diff --git a/installed-tests/js/jsunit.gresources.xml b/installed-tests/js/jsunit.gresources.xml
index da983d4..f3f3434 100644
--- a/installed-tests/js/jsunit.gresources.xml
+++ b/installed-tests/js/jsunit.gresources.xml
@@ -10,6 +10,8 @@
     <file>modules/mutualImport/a.js</file>
     <file>modules/mutualImport/b.js</file>
     <file>modules/subA/subB/__init__.js</file>
+    <!-- <file>modules/overrides/Fail.js</file>
+    <file>modules/overrides/Success.js</file> -->
     <file>modules/subA/subB/baz.js</file>
     <file>modules/subA/subB/foobar.js</file>
   </gresource>
diff --git a/installed-tests/js/modules/overrides/Fail.js b/installed-tests/js/modules/overrides/Fail.js
new file mode 100644
index 0000000..80679c5
--- /dev/null
+++ b/installed-tests/js/modules/overrides/Fail.js
@@ -0,0 +1,2 @@
+// Sabotage the import of imports.gi.Fail!
+throw '☭';
diff --git a/installed-tests/js/testImporter.js b/installed-tests/js/testImporter.js
index f2c649a..3b49d6a 100644
--- a/installed-tests/js/testImporter.js
+++ b/installed-tests/js/testImporter.js
@@ -3,6 +3,10 @@ describe('GI importer', function () {
         var GLib = imports.gi.GLib;
         expect(GLib.MAJOR_VERSION).toEqual(2);
     });
+
+    it('throws an exception when the overrides file throws one', function () {
+        expect(() => imports.gi.Fail).toThrow();
+    });
 });
 
 describe('Importer', function () {