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

[Philip Chimento <pchimento src gnome org>]

commit 35313c3de67dbefb2fa04326d05bbff9bf8835e9
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 0eec7b0..9720e03 100644
--- a/Makefile-insttest.am
+++ b/Makefile-insttest.am
@@ -24,7 +24,7 @@ gjsinsttest_PROGRAMS += jsunit
 gjsinsttest_DATA += $(TEST_INTROSPECTION_TYPELIBS)
 installedtestmeta_DATA += jsunit.test
 jstests_DATA += $(common_jstests_files)
-pkglib_LTLIBRARIES += libregress.la libwarnlib.la libgimarshallingtests.la
+pkglib_LTLIBRARIES += $(test_introspection_libraries)
 
 if ENABLE_CAIRO
 jstests_DATA += installed-tests/js/testCairo.js
diff --git a/Makefile-test.am b/Makefile-test.am
index e26d078..a79e3db 100644
--- a/Makefile-test.am
+++ b/Makefile-test.am
@@ -143,6 +143,12 @@ jsunit_SOURCES = \
 ### 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)
 
@@ -153,7 +159,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
@@ -222,6 +228,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 944626b..2ae66e1 100644
--- a/installed-tests/js/jsunit.gresources.xml
+++ b/installed-tests/js/jsunit.gresources.xml
@@ -8,6 +8,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 ee91160..ba5c88a 100644
--- a/installed-tests/js/testImporter.js
+++ b/installed-tests/js/testImporter.js
@@ -99,6 +99,10 @@ function testImporterUTF8() {
     JSUnit.assertEquals(ModUnicode.uval, "const \u2665 utf8");
 }
 
+function testImportOverrides() {
+    JSUnit.assertRaises(() => imports.gi.Fail);
+}
+
 let oldSearchPath;
 
 function setUp() {