[gjs/esm/dynamic-imports: 5/5] SQUASH: fixes

[Philip Chimento <pchimento src gnome org>]

commit 320f11acd21853b1ed51f8599673eaa8114b2e08
Author: Philip Chimento <philip chimento gmail com>
Date:   Sun Feb 7 17:46:25 2021 -0800

    SQUASH: fixes

 gjs/module.cpp | 74 ++++++++++++++++++++++++++++++++--------------------------
 1 file changed, 41 insertions(+), 33 deletions(-)
---
diff --git a/gjs/module.cpp b/gjs/module.cpp
index 03deec25..49ad5da6 100644
--- a/gjs/module.cpp
+++ b/gjs/module.cpp
@@ -523,15 +523,15 @@ JSObject* gjs_module_resolve(JSContext* cx, JS::HandleValue importingModulePriv,
     return &result.toObject();
 }
 
-bool gjs_dynamic_module_get_meta(JSContext* cx, JS::HandleObject meta,
-                                 JS::MutableHandleValue importing_module_priv,
-                                 JS::MutableHandleString specifier,
-                                 JS::MutableHandleObject internal_promise) {
+static bool read_callback_data(JSContext* cx, JS::HandleObject callback_data,
+                               JS::MutableHandleValue importing_module_priv,
+                               JS::MutableHandleString specifier,
+                               JS::MutableHandleObject internal_promise) {
     JS::RootedValue v_specifier(cx);
     JS::RootedValue v_internal_promise(cx);
-    if (!JS_GetProperty(cx, meta, "priv", importing_module_priv) ||
-        !JS_GetProperty(cx, meta, "promise", &v_internal_promise) ||
-        !JS_GetProperty(cx, meta, "specifier", &v_specifier))
+    if (!JS_GetProperty(cx, callback_data, "priv", importing_module_priv) ||
+        !JS_GetProperty(cx, callback_data, "promise", &v_internal_promise) ||
+        !JS_GetProperty(cx, callback_data, "specifier", &v_specifier))
         return false;
 
     g_assert(v_specifier.isString());
@@ -543,40 +543,36 @@ bool gjs_dynamic_module_get_meta(JSContext* cx, JS::HandleObject meta,
     return true;
 }
 
-bool gjs_dynamic_module_rejected(JSContext* cx, unsigned argc, JS::Value* vp) {
+static bool import_rejected(JSContext* cx, unsigned argc, JS::Value* vp) {
     JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
 
     JS::Value priv_value = js::GetFunctionNativeReserved(&args.callee(), 0);
-    g_assert(!priv_value.isNull() && "Dynamic module rejection called twice");
-    JS::RootedObject meta(cx, &priv_value.toObject());
-    g_assert(meta && "Dynamic module rejection called twice");
-    js::SetFunctionNativeReserved(&args.callee(), 0, JS::NullValue());
+    g_assert(priv_value.isObject() && "Wrong private value");
+    JS::RootedObject callback_data(cx, &priv_value.toObject());
 
     JS::RootedValue importing_module_priv(cx);
     JS::RootedString specifier(cx);
     JS::RootedObject internal_promise(cx);
-    if (!gjs_dynamic_module_get_meta(cx, meta, &importing_module_priv,
-                                     &specifier, &internal_promise))
+    if (!read_callback_data(cx, callback_data, &importing_module_priv,
+                            &specifier, &internal_promise))
         return false;
 
     return JS::FinishDynamicModuleImport(cx, importing_module_priv, specifier,
                                          internal_promise);
 }
 
-bool gjs_dynamic_module_resolved(JSContext* cx, unsigned argc, JS::Value* vp) {
+static bool import_resolved(JSContext* cx, unsigned argc, JS::Value* vp) {
     JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
 
     JS::Value priv_value = js::GetFunctionNativeReserved(&args.callee(), 0);
-    g_assert(!priv_value.isNull() && "Dynamic module resolution called twice");
-    JS::RootedObject meta(cx, &priv_value.toObject());
-    g_assert(meta && "Dynamic module resolution called twice");
-    js::SetFunctionNativeReserved(&args.callee(), 0, JS::NullValue());
+    g_assert(priv_value.isObject() && "Wrong private value");
+    JS::RootedObject callback_data(cx, &priv_value.toObject());
 
     JS::RootedValue importing_module_priv(cx);
     JS::RootedString specifier(cx);
     JS::RootedObject internal_promise(cx);
-    if (!gjs_dynamic_module_get_meta(cx, meta, &importing_module_priv,
-                                     &specifier, &internal_promise))
+    if (!read_callback_data(cx, callback_data, &importing_module_priv,
+                            &specifier, &internal_promise))
         return false;
 
     JS::RootedObject global(cx, gjs_get_import_global(cx));
@@ -609,15 +605,22 @@ bool gjs_dynamic_module_resolve(JSContext* cx,
     g_assert(v_loader.isObject());
     JS::RootedObject loader(cx, &v_loader.toObject());
 
-    JS::RootedObject meta(cx, JS_NewPlainObject(cx));
-    if (!JS_DefineProperty(cx, meta, "specifier", specifier,
+    JS::RootedObject callback_data(cx, JS_NewPlainObject(cx));
+    if (!callback_data ||
+        !JS_DefineProperty(cx, callback_data, "specifier", specifier,
                            JSPROP_PERMANENT) ||
-        !JS_DefineProperty(cx, meta, "promise", internal_promise,
+        !JS_DefineProperty(cx, callback_data, "promise", internal_promise,
                            JSPROP_PERMANENT) ||
-        !JS_DefineProperty(cx, meta, "priv", importing_module_priv,
+        !JS_DefineProperty(cx, callback_data, "priv", importing_module_priv,
                            JSPROP_PERMANENT))
         return false;
 
+    gjs_debug(GJS_DEBUG_IMPORTER,
+              "Dynamic module resolve hook for module '%s' (relative to %p), "
+              "global %p",
+              gjs_debug_string(specifier).c_str(),
+              &importing_module_priv.toObject(), global.get());
+
     JS::RootedValueArray<2> args(cx);
     args[0].set(importing_module_priv);
     args[1].setString(specifier);
@@ -629,17 +632,22 @@ bool gjs_dynamic_module_resolve(JSContext* cx,
 
     JS::RootedObject resolved(
         cx, JS_GetFunctionObject(js::NewFunctionWithReserved(
-                cx, gjs_dynamic_module_resolved, 2, 0, "import resolved")));
+                cx, import_resolved, 2, 0, "import resolved")));
+    if (!resolved)
+        return false;
     JS::RootedObject rejected(
         cx, JS_GetFunctionObject(js::NewFunctionWithReserved(
-                cx, gjs_dynamic_module_rejected, 2, 0, "import rejected")));
-    js::SetFunctionNativeReserved(resolved, 0, JS::ObjectValue(*meta));
-    js::SetFunctionNativeReserved(rejected, 0, JS::ObjectValue(*meta));
+                cx, import_rejected, 2, 0, "import rejected")));
+    if (!rejected)
+        return false;
+    js::SetFunctionNativeReserved(resolved, 0, JS::ObjectValue(*callback_data));
+    js::SetFunctionNativeReserved(rejected, 0, JS::ObjectValue(*callback_data));
 
     JS::RootedObject promise(cx, &result.toObject());
 
-    if (!JS::AddPromiseReactions(cx, promise, resolved, rejected))
-        return false;
-
-    return true;
+    // Calling JS::FinishDynamicModuleImport() at the end of the resolve and
+    // reject handlers will also call the module resolve hook. The module will
+    // already have been resolved, but that is how SpiderMonkey obtains the
+    // module object.
+    return JS::AddPromiseReactions(cx, promise, resolved, rejected);
 }