GNOME.org
 

[gjs/esm/static-imports: 4/5] SQUASH: Move module metadata hook into C++


commit a457f6e1b31129f079ad73ba3d82167f7f511768
Author: Philip Chimento <philip chimento gmail com>
Date:   Fri Feb 5 15:48:43 2021 -0800

    SQUASH: Move module metadata hook into C++
    
    I found this a convoluted path:
    - Call a C++ function exposed as a global to put a JS function in a global
      slot
    - SpiderMonkey calls a C++ function to populate the metadata
    - Lookup the JS function from the global slot and call it
    - Define a property to a C++ function exposed as a global
    
    This unfortunately moves a small amount of code from JS into C++ but I
    think it's worth it because of the smaller number of steps that you have
    to follow across several files to figure out how it works.

 gjs/atoms.h                        |  4 ++
 gjs/global.cpp                     |  3 --
 gjs/global.h                       |  2 -
 gjs/internal.cpp                   | 79 ---------------------------------
 gjs/internal.h                     |  8 ----
 gjs/module.cpp                     | 91 +++++++++++++++++++++++++++++++++-----
 modules/internal/.eslintrc.yml     |  1 -
 modules/internal/internalLoader.js |  4 --
 modules/internal/loader.js         | 11 -----
 9 files changed, 83 insertions(+), 120 deletions(-)
---
diff --git a/gjs/atoms.h b/gjs/atoms.h
index 966999c8..7fe061f1 100644
--- a/gjs/atoms.h
+++ b/gjs/atoms.h
@@ -35,9 +35,11 @@ class JSTracer;
     macro(gtype, "$gtype") \
     macro(height, "height") \
     macro(imports, "imports") \
+    macro(importSync, "importSync") \
     macro(init, "_init") \
     macro(instance_init, "_instance_init") \
     macro(interact, "interact") \
+    macro(internal, "internal") \
     macro(length, "length") \
     macro(line_number, "lineNumber") \
     macro(message, "message") \
@@ -56,6 +58,8 @@ class JSTracer;
     macro(signal_id, "signalId") \
     macro(stack, "stack") \
     macro(to_string, "toString") \
+    macro(uri, "uri") \
+    macro(url, "url") \
     macro(value_of, "valueOf") \
     macro(version, "version") \
     macro(versions, "versions") \
diff --git a/gjs/global.cpp b/gjs/global.cpp
index 441a2999..aeb11ad7 100644
--- a/gjs/global.cpp
+++ b/gjs/global.cpp
@@ -269,14 +269,11 @@ class GjsInternalGlobal : GjsBaseGlobal {
         JS_FN("compileInternalModule", gjs_internal_compile_internal_module, 2,
               0),
         JS_FN("getRegistry", gjs_internal_get_registry, 1, 0),
-        JS_FN("importSync", gjs_internal_global_import_sync, 1, 0),
         JS_FN("loadResourceOrFile", gjs_internal_load_resource_or_file, 1, 0),
         JS_FN("parseURI", gjs_internal_parse_uri, 1, 0),
         JS_FN("resolveRelativeResourceOrFile",
               gjs_internal_resolve_relative_resource_or_file, 2, 0),
         JS_FN("setModuleLoadHook", gjs_internal_global_set_module_hook, 3, 0),
-        JS_FN("setModuleMetaHook", gjs_internal_global_set_module_meta_hook, 2,
-              0),
         JS_FN("setModulePrivate", gjs_internal_set_module_private, 2, 0),
         JS_FN("setModuleResolveHook",
               gjs_internal_global_set_module_resolve_hook, 2, 0),
diff --git a/gjs/global.h b/gjs/global.h
index f39c4c8c..f7c39ad3 100644
--- a/gjs/global.h
+++ b/gjs/global.h
@@ -41,8 +41,6 @@ enum class GjsGlobalSlot : uint32_t {
     IMPORT_HOOK,
     // Stores a function which creates new module objects
     MODULE_HOOK,
-    // Stores a function which sets the metadata on import.meta
-    META_HOOK,
     // Stores the module registry (a Map object)
     MODULE_REGISTRY,
     NATIVE_REGISTRY,
diff --git a/gjs/internal.cpp b/gjs/internal.cpp
index b24056ad..388f2406 100644
--- a/gjs/internal.cpp
+++ b/gjs/internal.cpp
@@ -171,33 +171,6 @@ bool gjs_internal_global_set_module_resolve_hook([[maybe_unused]] JSContext* cx,
     return true;
 }
 
-/**
- * gjs_internal_global_set_module_meta_hook:
- *
- * @brief Sets the META_HOOK slot of the passed passed global object.
- * Asserts that the second argument must be callable (e.g. Function).
- * The passed callable is called by gjs_populate_module_meta.
- *
- * The META_HOOK is passed two parameters, a plain object for population with
- * meta properties and the module's private object.
- *
- * @example (in JavaScript)
- * setModuleMetaHook(globalThis, (module, meta) => {
- *   module // the module object
- *   meta // the meta object
- * });
- *
- * @returns guaranteed to return true or assert.
- */
-bool gjs_internal_global_set_module_meta_hook([[maybe_unused]] JSContext* cx,
-                                              unsigned argc, JS::Value* vp) {
-    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
-    g_assert(args.length() == 2 && "setModuleMetaHook takes 2 arguments");
-
-    set_module_hook(args, GjsGlobalSlot::META_HOOK);
-    return true;
-}
-
 /**
  * compile_module:
  *
@@ -313,58 +286,6 @@ bool gjs_internal_set_module_private(JSContext* cx, unsigned argc,
     return true;
 }
 
-/**
- * gjs_internal_global_import_sync:
- *
- * @brief Synchronously imports native "modules" from the import global's
- * native registry. This function does not do blocking I/O so it is
- * safe to call it synchronously for accessing native "modules" within
- * modules. This function is always called within the import global's
- * realm.
- *
- * @param cx the current JSContext
- * @param argc
- * @param vp
- *
- * @returns whether an error occurred while importing the native module.
- */
-bool gjs_internal_global_import_sync(JSContext* cx, unsigned argc,
-                                     JS::Value* vp) {
-    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
-    JS::UniqueChars id;
-    if (!gjs_parse_call_args(cx, "importSync", args, "s", "identifier", &id))
-        return false;
-
-    JS::RootedObject global(cx, gjs_get_import_global(cx));
-    JSAutoRealm ar(cx, global);
-
-    JS::AutoSaveExceptionState exc_state(cx);
-
-    JS::RootedObject native_registry(cx, gjs_get_native_registry(global));
-    JS::RootedObject v_module(cx);
-
-    JS::RootedId key(cx, gjs_intern_string_to_id(cx, id.get()));
-    if (!gjs_global_registry_get(cx, native_registry, key, &v_module))
-        return false;
-
-    if (v_module) {
-        args.rval().setObject(*v_module);
-        return true;
-    }
-
-    JS::RootedObject native_obj(cx);
-    if (!gjs_load_native_module(cx, id.get(), &native_obj)) {
-        gjs_throw(cx, "Failed to load native module: %s", id.get());
-        return false;
-    }
-
-    if (!gjs_global_registry_set(cx, native_registry, key, native_obj))
-        return false;
-
-    args.rval().setObject(*native_obj);
-    return true;
-}
-
 /**
  * gjs_internal_get_registry:
  *
diff --git a/gjs/internal.h b/gjs/internal.h
index 0e86856c..888b9033 100644
--- a/gjs/internal.h
+++ b/gjs/internal.h
@@ -24,18 +24,10 @@ bool gjs_internal_compile_internal_module(JSContext* cx, unsigned argc,
 GJS_JSAPI_RETURN_CONVENTION
 bool gjs_internal_get_registry(JSContext* cx, unsigned argc, JS::Value* vp);
 
-GJS_JSAPI_RETURN_CONVENTION
-bool gjs_internal_global_import_sync(JSContext* cx, unsigned argc,
-                                     JS::Value* vp);
-
 GJS_JSAPI_RETURN_CONVENTION
 bool gjs_internal_global_set_module_hook(JSContext* cx, unsigned argc,
                                          JS::Value* vp);
 
-GJS_JSAPI_RETURN_CONVENTION
-bool gjs_internal_global_set_module_meta_hook(JSContext* cx, unsigned argc,
-                                              JS::Value* vp);
-
 GJS_JSAPI_RETURN_CONVENTION
 bool gjs_internal_set_module_private(JSContext* cx, unsigned argc,
                                      JS::Value* vp);
diff --git a/gjs/module.cpp b/gjs/module.cpp
index 2b67ebca..37fdd3af 100644
--- a/gjs/module.cpp
+++ b/gjs/module.cpp
@@ -13,24 +13,31 @@
 #include <gio/gio.h>
 #include <glib.h>
 
+#include <js/CallArgs.h>
 #include <js/CharacterEncoding.h>  // for ConstUTF8CharsZ
 #include <js/Class.h>
 #include <js/CompilationAndEvaluation.h>
 #include <js/CompileOptions.h>
+#include <js/Conversions.h>
 #include <js/GCVector.h>  // for RootedVector
+#include <js/Id.h>
 #include <js/PropertyDescriptor.h>
 #include <js/RootingAPI.h>
 #include <js/SourceText.h>
 #include <js/TypeDecls.h>
+#include <js/Utility.h>  // for UniqueChars
 #include <js/Value.h>
 #include <js/ValueArray.h>
 #include <jsapi.h>  // for JS_DefinePropertyById, ...
 
+#include "gjs/atoms.h"
 #include "gjs/context-private.h"
 #include "gjs/global.h"
+#include "gjs/jsapi-util-args.h"
 #include "gjs/jsapi-util.h"
 #include "gjs/mem-private.h"
 #include "gjs/module.h"
+#include "gjs/native.h"
 #include "util/log.h"
 
 class GjsScriptModule {
@@ -336,10 +343,66 @@ JSObject* gjs_module_load(JSContext* cx, const char* identifier,
     return &result.toObject();
 }
 
+/**
+ * import_native_module_sync:
+ *
+ * @brief Synchronously imports native "modules" from the import global's
+ * native registry. This function does not do blocking I/O so it is
+ * safe to call it synchronously for accessing native "modules" within
+ * modules. This function is always called within the import global's
+ * realm.
+ *
+ * Compare gjs_import_native_module() for the legacy importer.
+ *
+ * @param cx the current JSContext
+ * @param argc
+ * @param vp
+ *
+ * @returns whether an error occurred while importing the native module.
+ */
+static bool import_native_module_sync(JSContext* cx, unsigned argc,
+                                      JS::Value* vp) {
+    JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+    JS::UniqueChars id;
+    if (!gjs_parse_call_args(cx, "importSync", args, "s", "identifier", &id))
+        return false;
+
+    JS::RootedObject global(cx, gjs_get_import_global(cx));
+    JSAutoRealm ar(cx, global);
+
+    JS::AutoSaveExceptionState exc_state(cx);
+
+    JS::RootedObject native_registry(cx, gjs_get_native_registry(global));
+    JS::RootedObject v_module(cx);
+
+    JS::RootedId key(cx, gjs_intern_string_to_id(cx, id.get()));
+    if (!gjs_global_registry_get(cx, native_registry, key, &v_module))
+        return false;
+
+    if (v_module) {
+        args.rval().setObject(*v_module);
+        return true;
+    }
+
+    JS::RootedObject native_obj(cx);
+    if (!gjs_load_native_module(cx, id.get(), &native_obj)) {
+        gjs_throw(cx, "Failed to load native module: %s", id.get());
+        return false;
+    }
+
+    if (!gjs_global_registry_set(cx, native_registry, key, native_obj))
+        return false;
+
+    args.rval().setObject(*native_obj);
+    return true;
+}
+
 /**
  * gjs_populate_module_meta:
  *
  * Hook SpiderMonkey calls to populate the import.meta object.
+ * Defines a property "import.meta.url", and additionally a method
+ * "import.meta.importSync" if this is an internal module.
  *
  * @param private_ref the private value for the #Module object
  * @param meta_object the import.meta object
@@ -347,23 +410,27 @@ JSObject* gjs_module_load(JSContext* cx, const char* identifier,
  * @returns whether an error occurred while populating the module meta.
  */
 bool gjs_populate_module_meta(JSContext* cx, JS::HandleValue private_ref,
-                              JS::HandleObject meta_object_handle) {
+                              JS::HandleObject meta) {
     g_assert(private_ref.isObject());
-
-    JS::RootedObject global(cx, JS::CurrentGlobalOrNull(cx));
-    JS::RootedValue hook(cx,
-                         gjs_get_global_slot(global, GjsGlobalSlot::META_HOOK));
-
-    JS::RootedObject meta(cx, meta_object_handle);
     JS::RootedObject module(cx, &private_ref.toObject());
-    JS::RootedValueArray<2> args(cx);
-    args[0].setObject(*module);
-    args[1].setObject(*meta);
 
-    JS::RootedValue ignore_result(cx);
-    if (!JS_CallFunctionValue(cx, nullptr, hook, args, &ignore_result))
+    const GjsAtoms& atoms = GjsContextPrivate::atoms(cx);
+    JS::RootedValue v_uri(cx);
+    if (!JS_GetPropertyById(cx, module, atoms.uri(), &v_uri) ||
+        !JS_DefinePropertyById(cx, meta, atoms.url(), v_uri,
+                               GJS_MODULE_PROP_FLAGS))
         return false;
 
+    JS::RootedValue v_internal(cx);
+    if (!JS_GetPropertyById(cx, module, atoms.internal(), &v_internal))
+        return false;
+    if (JS::ToBoolean(v_internal)) {
+        if (!JS_DefineFunctionById(cx, meta, atoms.importSync(),
+                                   import_native_module_sync, 1,
+                                   GJS_MODULE_PROP_FLAGS))
+            return false;
+    }
+
     return true;
 }
 
diff --git a/modules/internal/.eslintrc.yml b/modules/internal/.eslintrc.yml
index d2edebf8..6bcff33c 100644
--- a/modules/internal/.eslintrc.yml
+++ b/modules/internal/.eslintrc.yml
@@ -23,7 +23,6 @@ globals:
   uriExists: readonly
   resolveRelativeResourceOrFile: readonly
   setModuleResolveHook: readonly
-  setModuleMetaHook: readonly
   setModuleLoadHook: readonly
   setModulePrivate: readonly
   getRegistry: readonly
diff --git a/modules/internal/internalLoader.js b/modules/internal/internalLoader.js
index 7c870f4f..e59e5c5e 100644
--- a/modules/internal/internalLoader.js
+++ b/modules/internal/internalLoader.js
@@ -200,10 +200,6 @@ setModuleResolveHook(globalThis, (priv, specifier) => {
     return resolved;
 });
 
-setModuleMetaHook(globalThis, (module, meta) => {
-    meta.url = module.uri;
-});
-
 setModuleLoadHook(globalThis, (id, uri) => {
     const priv = new ModulePrivate(id, uri);
 
diff --git a/modules/internal/loader.js b/modules/internal/loader.js
index 8518c197..2c13d7a5 100644
--- a/modules/internal/loader.js
+++ b/modules/internal/loader.js
@@ -162,17 +162,6 @@ moduleLoader.registerScheme('gi', {
     },
 });
 
-/**
- * @param {ModulePrivate} module
- * @param {ImportMeta} meta
- */
-setModuleMetaHook(moduleGlobalThis, (module, meta) => {
-    meta.url = module.uri;
-
-    if (module.internal)
-        meta.importSync = globalThis.importSync;
-});
-
 /**
  * @param {string} id
  * @param {string} uri
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]