Rework the native module importing schemewip/imports-cleanup

Make modules always supply their own module object when imported, and
simply define the properties that we want to on it afterwards. This
simplifies the structure of how native modules work and allows us to
remove a significant amount of support code for it.

18 files changed, 78 insertions, 224 deletions

diff --git a/gi/object.c b/gi/object.c
index 0549254d..039b65f9 100644
--- a/gi/object.c
+++ b/gi/object.c
@@ -2676,10 +2676,16 @@ static JSFunctionSpec module_funcs[] = {
 
 JSBool
 gjs_define_private_gi_stuff(JSContext *context,
-                            JSObject  *module_obj)
+                            JSObject **module_out)
 {
-    if (!JS_DefineFunctions(context, module_obj, &module_funcs[0]))
+    JSObject *module;
+
+    module = JS_NewObject (context, NULL, NULL, NULL);
+
+    if (!JS_DefineFunctions(context, module, &module_funcs[0]))
         return JS_FALSE;
 
+    *module_out = module;
+
     return JS_TRUE;
 }
diff --git a/gi/repo.c b/gi/repo.c
index 3b10d902..993290a5 100644
--- a/gi/repo.c
+++ b/gi/repo.c
@@ -345,18 +345,13 @@ repo_new(JSContext *context)
 
 JSBool
 gjs_define_repo(JSContext  *context,
-                JSObject   *module_obj,
+                JSObject  **module_out,
                 const char *name)
 {
     JSObject *repo;
 
     repo = repo_new(context);
-
-    if (!JS_DefineProperty(context, module_obj,
-                           name, OBJECT_TO_JSVAL(repo),
-                           NULL, NULL,
-                           GJS_MODULE_PROP_FLAGS))
-        return JS_FALSE;
+    *module_out = repo;
 
     return JS_TRUE;
 }
diff --git a/gi/repo.h b/gi/repo.h
index 940a6e72..60d5be45 100644
--- a/gi/repo.h
+++ b/gi/repo.h
@@ -33,7 +33,7 @@
 G_BEGIN_DECLS
 
 JSBool      gjs_define_repo                     (JSContext      *context,
-                                                 JSObject       *module_obj,
+                                                 JSObject      **module_out,
                                                  const char     *name);
 const char* gjs_info_type_name                  (GIInfoType      type);
 JSObject*   gjs_lookup_private_namespace        (JSContext      *context);
diff --git a/gjs/byteArray.c b/gjs/byteArray.c
index 4597d687..1145b9e5 100644
--- a/gjs/byteArray.c
+++ b/gjs/byteArray.c
@@ -910,10 +910,14 @@ static JSFunctionSpec gjs_byte_array_module_funcs[] = {
 };
 
 JSBool
-gjs_define_byte_array_stuff(JSContext      *context,
-                            JSObject       *in_object)
+gjs_define_byte_array_stuff(JSContext  *context,
+                            JSObject  **module_out)
 {
-    gjs_byte_array_prototype = JS_InitClass(context, in_object,
+    JSObject *module;
+
+    module = JS_NewObject (context, NULL, NULL, NULL);
+
+    gjs_byte_array_prototype = JS_InitClass(context, module,
                              NULL,
                              &gjs_byte_array_class,
                              gjs_byte_array_constructor,
@@ -926,8 +930,9 @@ gjs_define_byte_array_stuff(JSContext      *context,
     if (gjs_byte_array_prototype == NULL)
         return JS_FALSE;
 
-    if (!JS_DefineFunctions(context, in_object, &gjs_byte_array_module_funcs[0]))
+    if (!JS_DefineFunctions(context, module, &gjs_byte_array_module_funcs[0]))
         return JS_FALSE;
 
+    *module_out = module;
     return JS_TRUE;
 }
diff --git a/gjs/byteArray.h b/gjs/byteArray.h
index 2d0dadcd..c3b42349 100644
--- a/gjs/byteArray.h
+++ b/gjs/byteArray.h
@@ -38,7 +38,7 @@ JSBool    gjs_typecheck_bytearray        (JSContext     *context,
                                           JSBool         throw);
 
 JSBool        gjs_define_byte_array_stuff    (JSContext  *context,
-                                              JSObject   *in_object);
+                                              JSObject  **module_out);
 
 JSObject *    gjs_byte_array_from_byte_array (JSContext  *context,
                                               GByteArray *array);
diff --git a/gjs/context.c b/gjs/context.c
index c9353d76..cb2fe1b4 100644
--- a/gjs/context.c
+++ b/gjs/context.c
@@ -354,9 +354,9 @@ gjs_context_class_init(GjsContextClass *klass)
         g_free (priv_typelib_dir);
     }
 
-    gjs_register_native_module("byteArray", gjs_define_byte_array_stuff, 0);
-    gjs_register_native_module("_gi", gjs_define_private_gi_stuff, 0);
-    gjs_register_native_module("gi", gjs_define_gi_stuff, GJS_NATIVE_SUPPLIES_MODULE_OBJ);
+    gjs_register_native_module("byteArray", gjs_define_byte_array_stuff);
+    gjs_register_native_module("_gi", gjs_define_private_gi_stuff);
+    gjs_register_native_module("gi", gjs_define_gi_stuff);
 
     gjs_register_static_modules();
 }
diff --git a/gjs/gi.c b/gjs/gi.c
index 0e431221..6ff48479 100644
--- a/gjs/gi.c
+++ b/gjs/gi.c
@@ -33,7 +33,7 @@
 
 JSBool
 gjs_define_gi_stuff(JSContext      *context,
-                    JSObject       *module_obj)
+                    JSObject      **module_out)
 {
-    return gjs_define_repo(context, module_obj, "gi");
+    return gjs_define_repo(context, module_out, "gi");
 }
diff --git a/gjs/gi.h b/gjs/gi.h
index c8da00f9..837d03d0 100644
--- a/gjs/gi.h
+++ b/gjs/gi.h
@@ -31,9 +31,9 @@
 G_BEGIN_DECLS
 
 JSBool        gjs_define_gi_stuff     (JSContext      *context,
-                                       JSObject       *module_obj);
+                                       JSObject      **module_out);
 JSBool        gjs_define_private_gi_stuff   (JSContext     *context,
-                                             JSObject      *module_obj);
+                                             JSObject     **module_out);
 
 G_END_DECLS
 
diff --git a/gjs/importer.c b/gjs/importer.c
index 4d3c2085..62185a9f 100644
--- a/gjs/importer.c
+++ b/gjs/importer.c
@@ -130,91 +130,6 @@ import_directory(JSContext   *context,
 }
 
 static JSBool
-define_import(JSContext  *context,
-              JSObject   *obj,
-              JSObject   *module_obj,
-              const char *name)
-{
-    if (!JS_DefineProperty(context, obj,
-                           name, OBJECT_TO_JSVAL(module_obj),
-                           NULL, NULL,
-                           GJS_MODULE_PROP_FLAGS & ~JSPROP_PERMANENT)) {
-        gjs_debug(GJS_DEBUG_IMPORTER,
-                  "Failed to define '%s' in importer",
-                  name);
-        return JS_FALSE;
-    }
-
-    return JS_TRUE;
-}
-
-/* Make the property we set in define_import permament;
- * we do this after the import succesfully completes.
- */
-static JSBool
-seal_import(JSContext  *context,
-            JSObject   *obj,
-            const char *name)
-{
-    JSBool found;
-    unsigned attrs;
-
-    if (!JS_GetPropertyAttributes(context, obj, name,
-                                  &attrs, &found) || !found) {
-        gjs_debug(GJS_DEBUG_IMPORTER,
-                  "Failed to get attributes to seal '%s' in importer",
-                  name);
-        return JS_FALSE;
-    }
-
-    attrs |= JSPROP_PERMANENT;
-
-    if (!JS_SetPropertyAttributes(context, obj, name,
-                                  attrs, &found) || !found) {
-        gjs_debug(GJS_DEBUG_IMPORTER,
-                  "Failed to set attributes to seal '%s' in importer",
-                  name);
-        return JS_FALSE;
-    }
-
-    return JS_TRUE;
-}
-
-/* An import failed. Delete the property pointing to the import
- * from the parent namespace. In complicated situations this might
- * not be sufficient to get us fully back to a sane state. If:
- *
- *  - We import module A
- *  - module A imports module B
- *  - module B imports module A, storing a reference to the current
- *    module A module object
- *  - module A subsequently throws an exception
- *
- * Then module B is left imported, but the imported module B has
- * a reference to the failed module A module object. To handle this
- * we could could try to track the entire "import operation" and
- * roll back *all* modifications made to the namespace objects.
- * It's not clear that the complexity would be worth the small gain
- * in robustness. (You can still come up with ways of defeating
- * the attempt to clean up.)
- */
-static void
-cancel_import(JSContext  *context,
-              JSObject   *obj,
-              const char *name)
-{
-    gjs_debug(GJS_DEBUG_IMPORTER,
-              "Cleaning up from failed import of '%s'",
-              name);
-
-    if (!JS_DeleteProperty(context, obj, name)) {
-        gjs_debug(GJS_DEBUG_IMPORTER,
-                  "Failed to delete '%s' in importer",
-                  name);
-    }
-}
-
-static JSBool
 import_native_file(JSContext  *context,
                    JSObject   *obj,
                    const char *name)
@@ -224,23 +139,10 @@ import_native_file(JSContext  *context,
 
     gjs_debug(GJS_DEBUG_IMPORTER, "Importing '%s'", name);
 
-    module_obj = JS_NewObject(context, NULL, NULL, NULL);
-    if (module_obj == NULL) {
-        return JS_FALSE;
-    }
-
-    /* We store the module object into the parent module before
-     * initializing the module. If the module has the
-     * GJS_NATIVE_SUPPLIES_MODULE_OBJ flag, it will just overwrite
-     * the reference we stored when it initializes.
-     */
-    if (!define_import(context, obj, module_obj, name))
-        return JS_FALSE;
-
-    if (!define_meta_properties(context, module_obj, NULL, name, obj))
+    if (!gjs_import_native_module(context, name, &module_obj))
         goto out;
 
-    if (!gjs_import_native_module(context, module_obj, name))
+    if (!define_meta_properties(context, module_obj, NULL, name, obj))
         goto out;
 
     if (JS_IsExceptionPending(context)) {
@@ -252,15 +154,14 @@ import_native_file(JSContext  *context,
         goto out;
     }
 
-    if (!seal_import(context, obj, name))
+    if (!JS_DefineProperty(context, obj,
+                           name, OBJECT_TO_JSVAL(module_obj),
+                           NULL, NULL, GJS_MODULE_PROP_FLAGS))
         goto out;
 
     retval = JS_TRUE;
 
  out:
-    if (!retval)
-        cancel_import(context, obj, name);
-
     return retval;
 }
 
diff --git a/gjs/native.c b/gjs/native.c
index 05d59362..5041dbea 100644
--- a/gjs/native.c
+++ b/gjs/native.c
@@ -32,30 +32,14 @@
 #include "jsapi-util.h"
 #include "runtime.h"
 
-typedef struct {
-    GjsDefineModuleFunc func;
-    GjsNativeFlags flags;
-} GjsNativeModule;
-
 static GHashTable *modules = NULL;
 
-static void
-native_module_free(void *data)
-{
-    g_slice_free(GjsNativeModule, data);
-}
-
 void
-gjs_register_native_module (const char            *module_id,
-                            GjsDefineModuleFunc  func,
-                            GjsNativeFlags       flags)
+gjs_register_native_module (const char          *module_id,
+                            GjsDefineModuleFunc  func)
 {
-    GjsNativeModule *module;
-
-    if (modules == NULL) {
-        modules = g_hash_table_new_full(g_str_hash, g_str_equal,
-                                        g_free, native_module_free);
-    }
+    if (modules == NULL)
+        modules = g_hash_table_new_full(g_str_hash, g_str_equal, g_free, NULL);
 
     if (g_hash_table_lookup(modules, module_id) != NULL) {
         g_warning("A second native module tried to register the same id '%s'",
@@ -63,34 +47,13 @@ gjs_register_native_module (const char            *module_id,
         return;
     }
 
-    module = g_slice_new(GjsNativeModule);
-    module->func = func;
-    module->flags = flags;
-
-    g_hash_table_replace(modules,
-                         g_strdup(module_id),
-                         module);
+    g_hash_table_replace(modules, g_strdup(module_id), func);
 
     gjs_debug(GJS_DEBUG_NATIVE,
               "Registered native JS module '%s'",
               module_id);
 }
 
-static JSObject*
-module_get_parent(JSContext *context,
-                  JSObject  *module_obj)
-{
-    jsval value;
-
-    if (gjs_object_get_property_const(context, module_obj, GJS_STRING_PARENT_MODULE, &value) &&
-        !JSVAL_IS_NULL(value) &&
-        JSVAL_IS_OBJECT(value)) {
-        return JSVAL_TO_OBJECT(value);
-    } else {
-        return NULL;
-    }
-}
-
 /**
  * gjs_is_registered_native_module:
  * @context:
@@ -120,39 +83,27 @@ gjs_is_registered_native_module(JSContext  *context,
  * Return a native module that's been preloaded.
  */
 JSBool
-gjs_import_native_module(JSContext *context,
-                         JSObject  *module_obj,
-                         const char *name)
+gjs_import_native_module(JSContext   *context,
+                         const char  *name,
+                         JSObject   **module_out)
 {
-    GjsNativeModule *native_module;
-    JSObject *parent;
+    GjsDefineModuleFunc func;
 
     gjs_debug(GJS_DEBUG_NATIVE,
               "Defining native module '%s'",
               name);
 
     if (modules != NULL)
-        native_module = g_hash_table_lookup(modules, name);
+        func = g_hash_table_lookup(modules, name);
     else
-        native_module = NULL;
+        func = NULL;
 
-    if (!native_module) {
+    if (!func) {
         gjs_throw(context,
                   "No native module '%s' has registered itself",
                   name);
         return JS_FALSE;
     }
 
-    if (native_module->flags & GJS_NATIVE_SUPPLIES_MODULE_OBJ) {
-
-        /* In this case we just throw away "module_obj" eventually,
-         * since the native module defines itself in the parent of
-         * module_obj directly.
-         */
-        parent = module_get_parent(context, module_obj);
-        return (* native_module->func) (context, parent);
-    } else {
-        return (* native_module->func) (context, module_obj);
-    }
+    return func (context, module_out);
 }
-
diff --git a/gjs/native.h b/gjs/native.h
index 902f7bca..d2f4b4a1 100644
--- a/gjs/native.h
+++ b/gjs/native.h
@@ -33,30 +33,12 @@
 
 G_BEGIN_DECLS
 
-typedef enum {
-    /* This means that the GjsDefineModuleFunc defines the module
-     * name in the parent module, as opposed to the normal process
-     * where the GjsDefineModuleFunc defines module contents. When
-     * importing imports.foo.bar, this flag means the native module is
-     * given foo and defines bar in it, while normally the native
-     * module is given bar and defines stuff in that.
-     *
-     * The purpose of this is to allow a module with lazy properties
-     * by allowing module objects to be custom classes. It's used for
-     * the gobject-introspection module for example.
-     */
-    GJS_NATIVE_SUPPLIES_MODULE_OBJ = 1 << 0
-
-} GjsNativeFlags;
-
-
-typedef JSBool (* GjsDefineModuleFunc) (JSContext *context,
-                                        JSObject  *module_obj);
+typedef JSBool (* GjsDefineModuleFunc) (JSContext  *context,
+                                        JSObject  **module_out);
 
 /* called on context init */
 void   gjs_register_native_module (const char            *module_id,
-                                   GjsDefineModuleFunc  func,
-                                   GjsNativeFlags       flags);
+                                   GjsDefineModuleFunc  func);
 
 /* called by importer.c to to check for already loaded modules */
 gboolean gjs_is_registered_native_module(JSContext  *context,
@@ -64,9 +46,9 @@ gboolean gjs_is_registered_native_module(JSContext  *context,
                                          const char *name);
 
 /* called by importer.c to load a statically linked native module */
-JSBool gjs_import_native_module (JSContext *context,
-                                 JSObject  *module_obj,
-                                 const char *name);
+JSBool gjs_import_native_module (JSContext  *context,
+                                 const char *name,
+                                 JSObject   **module_out);
 
 G_END_DECLS
 
diff --git a/modules/cairo-module.h b/modules/cairo-module.h
index beb6a562..9b5e2d59 100644
--- a/modules/cairo-module.h
+++ b/modules/cairo-module.h
@@ -24,6 +24,6 @@
 #define __CAIRO_MODULE_H__
 
 JSBool           gjs_js_define_cairo_stuff              (JSContext       *context,
-                                                         JSObject        *module);
+                                                         JSObject       **module_out);
 
 #endif /* __CAIRO_MODULE_H__ */
diff --git a/modules/cairo.c b/modules/cairo.c
index d72e39c4..05651692 100644
--- a/modules/cairo.c
+++ b/modules/cairo.c
@@ -45,11 +45,14 @@ gjs_cairo_check_status(JSContext      *context,
 
 JSBool
 gjs_js_define_cairo_stuff(JSContext *context,
-                          JSObject  *module)
+                          JSObject **module_out)
 {
     jsval obj;
+    JSObject *module;
     JSObject *surface_proto, *pattern_proto, *gradient_proto;
 
+    module = JS_NewObject (context, NULL, NULL, NULL);
+
     obj = gjs_cairo_context_create_proto(context, module,
                                          "Context", NULL);
     if (JSVAL_IS_NULL(obj))
@@ -122,5 +125,7 @@ gjs_js_define_cairo_stuff(JSContext *context,
     if (JSVAL_IS_NULL(obj))
         return JS_FALSE;
 
+    *module_out = module;
+
     return JS_TRUE;
 }
diff --git a/modules/console.c b/modules/console.c
index a1805b3f..1424b68d 100644
--- a/modules/console.c
+++ b/modules/console.c
@@ -226,14 +226,19 @@ gjs_console_interact(JSContext *context,
 }
 
 JSBool
-gjs_define_console_stuff(JSContext *context,
-                         JSObject  *module_obj)
+gjs_define_console_stuff(JSContext  *context,
+                         JSObject  **module_out)
 {
-    if (!JS_DefineFunction(context, module_obj,
+    JSObject *module;
+
+    module = JS_NewObject (context, NULL, NULL, NULL);
+
+    if (!JS_DefineFunction(context, module,
                            "interact",
                            (JSNative) gjs_console_interact,
                            1, GJS_MODULE_PROP_FLAGS))
         return JS_FALSE;
 
+    *module_out = module;
     return JS_TRUE;
 }
diff --git a/modules/console.h b/modules/console.h
index 39fae945..bb1e2da2 100644
--- a/modules/console.h
+++ b/modules/console.h
@@ -31,7 +31,7 @@
 G_BEGIN_DECLS
 
 JSBool        gjs_define_console_stuff     (JSContext      *context,
-                                            JSObject       *in_object);
+                                            JSObject      **module_out);
 JSBool        gjs_console_interact         (JSContext      *context,
                                             unsigned        argc,
                                             jsval          *vp);
diff --git a/modules/modules.c b/modules/modules.c
index 479fb4c9..aae35696 100644
--- a/modules/modules.c
+++ b/modules/modules.c
@@ -37,8 +37,8 @@ void
 gjs_register_static_modules (void)
 {
 #ifdef ENABLE_CAIRO
-    gjs_register_native_module("cairoNative", gjs_js_define_cairo_stuff, 0);
+    gjs_register_native_module("cairoNative", gjs_js_define_cairo_stuff);
 #endif
-    gjs_register_native_module("system", gjs_js_define_system_stuff, 0);
-    gjs_register_native_module("console", gjs_define_console_stuff, 0);
+    gjs_register_native_module("system", gjs_js_define_system_stuff);
+    gjs_register_native_module("console", gjs_define_console_stuff);
 }
diff --git a/modules/system.c b/modules/system.c
index 004beb47..87350d15 100644
--- a/modules/system.c
+++ b/modules/system.c
@@ -163,13 +163,16 @@ static JSFunctionSpec module_funcs[] = {
 };
 
 JSBool
-gjs_js_define_system_stuff(JSContext *context,
-                           JSObject  *module)
+gjs_js_define_system_stuff(JSContext  *context,
+                           JSObject  **module_out)
 {
     GjsContext *gjs_context;
     char *program_name;
     jsval value;
     JSBool retval;
+    JSObject *module;
+
+    module = JS_NewObject (context, NULL, NULL, NULL);
 
     if (!JS_DefineFunctions(context, module, &module_funcs[0]))
         return JS_FALSE;
@@ -207,6 +210,7 @@ gjs_js_define_system_stuff(JSContext *context,
 
  out:
     g_free(program_name);
+    *module_out = module;
 
     return retval;
 }
diff --git a/modules/system.h b/modules/system.h
index ca093a62..608de0e5 100644
--- a/modules/system.h
+++ b/modules/system.h
@@ -32,7 +32,7 @@
 G_BEGIN_DECLS
 
 JSBool        gjs_js_define_system_stuff     (JSContext      *context,
-                                              JSObject       *in_object);
+                                              JSObject      **module_out);
 
 G_END_DECLS