[gjs/wip/imports-cleanup: 13/13] Rework the native module importing scheme



commit fcad2b95a8369aad5b81d1d727e80747d8b42b3b
Author: Jasper St. Pierre <jstpierre mecheye net>
Date:   Fri Sep 27 19:30:06 2013 -0400

    Rework the native module importing scheme
    
    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.

 gi/object.c            |   10 ++++-
 gi/repo.c              |    9 +---
 gi/repo.h              |    2 +-
 gjs/byteArray.c        |   13 ++++--
 gjs/byteArray.h        |    2 +-
 gjs/context.c          |    6 +-
 gjs/gi.c               |    4 +-
 gjs/gi.h               |    4 +-
 gjs/importer.c         |  109 ++---------------------------------------------
 gjs/native.c           |   75 ++++++---------------------------
 gjs/native.h           |   30 +++-----------
 modules/cairo-module.h |    2 +-
 modules/cairo.c        |    7 +++-
 modules/console.c      |   11 ++++-
 modules/console.h      |    2 +-
 modules/modules.c      |    6 +-
 modules/system.c       |    8 +++-
 modules/system.h       |    2 +-
 18 files changed, 78 insertions(+), 224 deletions(-)
---
diff --git a/gi/object.c b/gi/object.c
index 0549254..039b65f 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 3b10d90..993290a 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 940a6e7..60d5be4 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 4597d68..1145b9e 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 2d0dadc..c3b4234 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 c9353d7..cb2fe1b 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 0e43122..6ff4847 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 c8da00f..837d03d 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 4d3c208..62185a9 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 05d5936..5041dbe 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 902f7bc..d2f4b4a 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 beb6a56..9b5e2d5 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 d72e39c..0565169 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 a1805b3..1424b68 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 39fae94..bb1e2da 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 479fb4c..aae3569 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 004beb4..87350d1 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 ca093a6..608de0e 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
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]