Issue #618 - Simplify module resolve hook to be a function pointer

This is an ahead-of time port to try and address #1624.
This is based on BZ 1461751 and Jon Coppeard's work in it.

7 files changed, 56 insertions, 42 deletions

diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp
index 77124355cf..69a3ba2ac2 100644
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -4667,21 +4667,16 @@ JS::Evaluate(JSContext* cx, const ReadOnlyCompileOptions& optionsArg,
     return ::Evaluate(cx, optionsArg, filename, rval);
 }
 
-JS_PUBLIC_API(JSFunction*)
-JS::GetModuleResolveHook(JSContext* cx)
+JS_PUBLIC_API(JS::ModuleResolveHook)
+JS::GetModuleResolveHook(JSRuntime* rt)
 {
-    AssertHeapIsIdle(cx);
-    CHECK_REQUEST(cx);
-    return cx->global()->moduleResolveHook();
+    return rt->moduleResolveHook;
 }
 
 JS_PUBLIC_API(void)
-JS::SetModuleResolveHook(JSContext* cx, HandleFunction func)
+JS::SetModuleResolveHook(JSRuntime* rt, JS::ModuleResolveHook func)
 {
-    AssertHeapIsIdle(cx);
-    CHECK_REQUEST(cx);
-    assertSameCompartment(cx, func);
-    cx->global()->setModuleResolveHook(func);
+    rt->moduleResolveHook = func;
 }
 
 JS_PUBLIC_API(bool)
diff --git a/js/src/jsapi.h b/js/src/jsapi.h
index c6299e3f57..5cdfd958e7 100644
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -4325,17 +4325,19 @@ extern JS_PUBLIC_API(bool)
 Evaluate(JSContext* cx, const ReadOnlyCompileOptions& options,
          const char* filename, JS::MutableHandleValue rval);
 
+using ModuleResolveHook = JSObject* (*)(JSContext*, HandleObject, HandleString);
+
 /**
- * Get the HostResolveImportedModule hook for a global.
+ * Get the HostResolveImportedModule hook for the runtime.
  */
-extern JS_PUBLIC_API(JSFunction*)
-GetModuleResolveHook(JSContext* cx);
+extern JS_PUBLIC_API(ModuleResolveHook)
+GetModuleResolveHook(JSRuntime* rt);
 
 /**
- * Set the HostResolveImportedModule hook for a global to the given function.
+ * Set the HostResolveImportedModule hook for the runtime to the given function.
  */
 extern JS_PUBLIC_API(void)
-SetModuleResolveHook(JSContext* cx, JS::HandleFunction func);
+SetModuleResolveHook(JSRuntime* rt, ModuleResolveHook func);
 
 /**
  * Parse the given source buffer as a module in the scope of the current global
diff --git a/js/src/shell/js.cpp b/js/src/shell/js.cpp
index 8cd821b311..bbf35459ff 100644
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -274,6 +274,7 @@ struct ShellContext
     JS::PersistentRooted<JobQueue> jobQueue;
     ExclusiveData<ShellAsyncTasks> asyncTasks;
     bool drainingJobQueue;
+    JS::PersistentRootedFunction moduleResolveHook;
 
     /*
      * Watchdog thread state.
@@ -439,7 +440,8 @@ ShellContext::ShellContext(JSContext* cx)
     exitCode(0),
     quitting(false),
     readLineBufPos(0),
-    spsProfilingStackSize(0)
+    spsProfilingStackSize(0),
+    moduleResolveHook(cx)
 {}
 
 static ShellContext*
@@ -4030,13 +4032,34 @@ SetModuleResolveHook(JSContext* cx, unsigned argc, Value* vp)
         return false;
     }
 
-    RootedFunction hook(cx, &args[0].toObject().as<JSFunction>());
-    Rooted<GlobalObject*> global(cx, cx->global());
-    global->setModuleResolveHook(hook);
+    ShellContext* sc = GetShellContext(cx);
+    sc->moduleResolveHook = &args[0].toObject().as<JSFunction>();
+
     args.rval().setUndefined();
     return true;
 }
 
+static JSObject*
+CallModuleResolveHook(JSContext* cx, HandleObject module, HandleString specifier)
+{
+    ShellContext* sc = GetShellContext(cx);
+
+    JS::AutoValueArray<2> args(cx);
+    args[0].setObject(*module);
+    args[1].setString(specifier);
+
+    RootedValue result(cx);
+    if (!JS_CallFunction(cx, nullptr, sc->moduleResolveHook, args, &result))
+        return nullptr;
+
+    if (!result.isObject() || !result.toObject().is<ModuleObject>()) {
+         JS_ReportErrorASCII(cx, "Module resolve hook did not return Module object");
+         return nullptr;
+    }
+
+    return &result.toObject();
+}
+
 static bool
 GetModuleLoadPath(JSContext* cx, unsigned argc, Value* vp)
 {
@@ -7962,6 +7985,8 @@ main(int argc, char** argv, char** envp)
 
     js::SetPreserveWrapperCallback(cx, DummyPreserveWrapperCallback);
 
+    JS::SetModuleResolveHook(cx->runtime(), CallModuleResolveHook);
+
     result = Shell(cx, &op, envp);
 
 #ifdef DEBUG
diff --git a/js/src/vm/GlobalObject.h b/js/src/vm/GlobalObject.h
index 720f63e698..a9f07d4956 100644
--- a/js/src/vm/GlobalObject.h
+++ b/js/src/vm/GlobalObject.h
@@ -119,7 +119,6 @@ class GlobalObject : public NativeObject
         DEBUGGERS,
         INTRINSICS,
         FOR_OF_PIC_CHAIN,
-        MODULE_RESOLVE_HOOK,
         WINDOW_PROXY,
         GLOBAL_THIS_RESOLVED,
 
@@ -879,19 +878,6 @@ class GlobalObject : public NativeObject
         setReservedSlot(WINDOW_PROXY, ObjectValue(*windowProxy));
     }
 
-    void setModuleResolveHook(HandleFunction hook) {
-        MOZ_ASSERT(hook);
-        setSlot(MODULE_RESOLVE_HOOK, ObjectValue(*hook));
-    }
-
-    JSFunction* moduleResolveHook() {
-        Value value = getSlotRef(MODULE_RESOLVE_HOOK);
-        if (value.isUndefined())
-            return nullptr;
-
-        return &value.toObject().as<JSFunction>();
-    }
-
     // Returns either this global's star-generator function prototype, or null
     // if that object was never created.  Dodgy; for use only in also-dodgy
     // GlobalHelperThreadState::mergeParseTaskCompartment().
diff --git a/js/src/vm/Runtime.cpp b/js/src/vm/Runtime.cpp
index 284a4f3d7c..5e0246b85a 100644
--- a/js/src/vm/Runtime.cpp
+++ b/js/src/vm/Runtime.cpp
@@ -241,8 +241,10 @@ JSRuntime::JSRuntime(JSRuntime* parentRuntime)
     lastAnimationTime(0),
     performanceMonitoring(thisFromCtor()),
     ionLazyLinkListSize_(0),
-    stackFormat_(parentRuntime ? js::StackFormat::Default
-                               : js::StackFormat::SpiderMonkey)
+    stackFormat_(parentRuntime ?
+                 js::StackFormat::Default :
+                 js::StackFormat::SpiderMonkey),
+    moduleResolveHook()
 {
     setGCStoreBufferPtr(&gc.storeBuffer);
 
diff --git a/js/src/vm/Runtime.h b/js/src/vm/Runtime.h
index e60371e38c..247a2dc9d7 100644
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -1294,6 +1294,9 @@ struct JSRuntime : public JS::shadow::Runtime,
     // For inherited heap state accessors.
     friend class js::gc::AutoTraceSession;
     friend class JS::AutoEnterCycleCollection;
+
+    // The implementation-defined abstract operation HostResolveImportedModule.
+    JS::ModuleResolveHook moduleResolveHook;
 };
 
 namespace js {
diff --git a/js/src/vm/SelfHosting.cpp b/js/src/vm/SelfHosting.cpp
index 50b0c01def..f324a0a67f 100644
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -2026,25 +2026,26 @@ intrinsic_HostResolveImportedModule(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     MOZ_ASSERT(args.length() == 2);
-    MOZ_ASSERT(args[0].toObject().is<ModuleObject>());
-    MOZ_ASSERT(args[1].isString());
+    RootedModuleObject module(cx, &args[0].toObject().as<ModuleObject>());
+    RootedString specifier(cx, args[1].toString());
 
-    RootedFunction moduleResolveHook(cx, cx->global()->moduleResolveHook());
+    JS::ModuleResolveHook moduleResolveHook = cx->runtime()->moduleResolveHook;
     if (!moduleResolveHook) {
         JS_ReportErrorASCII(cx, "Module resolve hook not set");
         return false;
     }
 
-    RootedValue result(cx);
-    if (!JS_CallFunction(cx, nullptr, moduleResolveHook, args, &result))
+    RootedObject result(cx);
+    result = moduleResolveHook(cx, module, specifier);
+    if (!result)
         return false;
 
-    if (!result.isObject() || !result.toObject().is<ModuleObject>()) {
+    if (!result->is<ModuleObject>()) {
         JS_ReportErrorASCII(cx, "Module resolve hook did not return Module object");
         return false;
     }
 
-    args.rval().set(result);
+    args.rval().setObject(*result);
     return true;
 }