Bug 1461751 - Simplify module resolve hook to be a function pointer r=luke r=baku
authorJon Coppeard <jcoppeard@mozilla.com>
Wed, 16 May 2018 11:59:09 +0100
changeset 472732 e862899dca3f252a8fe3c7be10d83d79dad328c5
parent 472731 1076e77e7b5c6f5e443da46820d7571c291bb399
child 472733 2297c7b1314e7c725da4d8006df354d7821a7624
push id9374
push userjlund@mozilla.com
push dateMon, 18 Jun 2018 21:43:20 +0000
treeherdermozilla-beta@160e085dfb0b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersluke, baku
bugs1461751
milestone62.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1461751 - Simplify module resolve hook to be a function pointer r=luke r=baku
dom/script/ScriptLoader.cpp
dom/script/ScriptLoader.h
js/public/Class.h
js/src/jsapi.cpp
js/src/jsapi.h
js/src/shell/js.cpp
js/src/shell/jsshell.h
js/src/vm/GlobalObject.h
js/src/vm/Runtime.cpp
js/src/vm/Runtime.h
js/src/vm/SelfHosting.cpp
--- a/dom/script/ScriptLoader.cpp
+++ b/dom/script/ScriptLoader.cpp
@@ -764,70 +764,58 @@ ScriptLoader::StartFetchingModuleAndDepe
     childRequest->mReady.Reject(rv, __func__);
     return ready;
   }
 
   return ready;
 }
 
 // 8.1.3.8.1 HostResolveImportedModule(referencingModule, specifier)
-bool
-HostResolveImportedModule(JSContext* aCx, unsigned argc, JS::Value* vp)
+JSObject*
+HostResolveImportedModule(JSContext* aCx, JS::Handle<JSObject*> aModule,
+                          JS::Handle<JSString*> aSpecifier)
 {
-
-  MOZ_ASSERT(argc == 2);
-  JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
-  JS::Rooted<JSObject*> module(aCx, &args[0].toObject());
-  JS::Rooted<JSString*> specifier(aCx, args[1].toString());
-
   // Let referencing module script be referencingModule.[[HostDefined]].
-  JS::Value value = JS::GetModuleHostDefinedField(module);
+  JS::Value value = JS::GetModuleHostDefinedField(aModule);
   auto script = static_cast<ModuleScript*>(value.toPrivate());
-  MOZ_ASSERT(script->ModuleRecord() == module);
+  MOZ_ASSERT(script->ModuleRecord() == aModule);
 
   // Let url be the result of resolving a module specifier given referencing
   // module script and specifier.
   nsAutoJSString string;
-  if (!string.init(aCx, specifier)) {
-    return false;
+  if (!string.init(aCx, aSpecifier)) {
+    return nullptr;
   }
 
   nsCOMPtr<nsIURI> uri = ResolveModuleSpecifier(script, string);
 
   // This cannot fail because resolving a module specifier must have been
   // previously successful with these same two arguments.
   MOZ_ASSERT(uri, "Failed to resolve previously-resolved module specifier");
 
   // Let resolved module script be moduleMap[url]. (This entry must exist for us
   // to have gotten to this point.)
   ModuleScript* ms = script->Loader()->GetFetchedModule(uri);
   MOZ_ASSERT(ms, "Resolved module not found in module map");
 
   MOZ_ASSERT(!ms->HasParseError());
-
-  *vp = JS::ObjectValue(*ms->ModuleRecord());
-  return true;
+  MOZ_ASSERT(ms->ModuleRecord());
+
+  return ms->ModuleRecord();
 }
 
-static nsresult
+static void
 EnsureModuleResolveHook(JSContext* aCx)
 {
-  if (JS::GetModuleResolveHook(aCx)) {
-    return NS_OK;
+  JSRuntime* rt = JS_GetRuntime(aCx);
+  if (JS::GetModuleResolveHook(rt)) {
+    return;
   }
 
-  JS::Rooted<JSFunction*> func(aCx);
-  func = JS_NewFunction(aCx, HostResolveImportedModule, 2, 0,
-                        "HostResolveImportedModule");
-  if (!func) {
-    return NS_ERROR_FAILURE;
-  }
-
-  JS::SetModuleResolveHook(aCx, func);
-  return NS_OK;
+  JS::SetModuleResolveHook(rt, HostResolveImportedModule);
 }
 
 void
 ScriptLoader::CheckModuleDependenciesLoaded(ModuleLoadRequest* aRequest)
 {
   LOG(("ScriptLoadRequest (%p): Check dependencies loaded", aRequest));
 
   RefPtr<ModuleScript> moduleScript = aRequest->mModuleScript;
@@ -939,18 +927,17 @@ ScriptLoader::InstantiateModuleTree(Modu
   MOZ_ASSERT(moduleScript->ModuleRecord());
 
   nsAutoMicroTask mt;
   AutoJSAPI jsapi;
   if (NS_WARN_IF(!jsapi.Init(moduleScript->ModuleRecord()))) {
     return false;
   }
 
-  nsresult rv = EnsureModuleResolveHook(jsapi.cx());
-  NS_ENSURE_SUCCESS(rv, false);
+  EnsureModuleResolveHook(jsapi.cx());
 
   JS::Rooted<JSObject*> module(jsapi.cx(), moduleScript->ModuleRecord());
   bool ok = NS_SUCCEEDED(nsJSUtils::ModuleInstantiate(jsapi.cx(), module));
 
   if (!ok) {
     LOG(("ScriptLoadRequest (%p): Instantiate failed", aRequest));
     MOZ_ASSERT(jsapi.HasException());
     JS::RootedValue exception(jsapi.cx());
@@ -2226,18 +2213,17 @@ ScriptLoader::EvaluateScript(ScriptLoadR
       // When a module is already loaded, it is not feched a second time and the
       // mDataType of the request might remain set to DataType::Unknown.
       MOZ_ASSERT(!aRequest->IsBytecode());
       LOG(("ScriptLoadRequest (%p): Evaluate Module", aRequest));
 
       // currentScript is set to null for modules.
       AutoCurrentScriptUpdater scriptUpdater(this, nullptr);
 
-      rv = EnsureModuleResolveHook(cx);
-      NS_ENSURE_SUCCESS(rv, rv);
+      EnsureModuleResolveHook(cx);
 
       ModuleLoadRequest* request = aRequest->AsModuleRequest();
       MOZ_ASSERT(request->mModuleScript);
       MOZ_ASSERT(!request->mOffThreadToken);
 
       ModuleScript* moduleScript = request->mModuleScript;
       if (moduleScript->HasErrorToRethrow()) {
         LOG(("ScriptLoadRequest (%p):   module has error to rethrow", aRequest));
--- a/dom/script/ScriptLoader.h
+++ b/dom/script/ScriptLoader.h
@@ -500,18 +500,19 @@ private:
                                                       nsresult aResult);
 
   bool IsFetchingModule(ModuleLoadRequest* aRequest) const;
 
   bool ModuleMapContainsURL(nsIURI* aURL) const;
   RefPtr<mozilla::GenericPromise> WaitForModuleFetch(nsIURI* aURL);
   ModuleScript* GetFetchedModule(nsIURI* aURL) const;
 
-  friend bool
-  HostResolveImportedModule(JSContext* aCx, unsigned argc, JS::Value* vp);
+  friend JSObject*
+  HostResolveImportedModule(JSContext* aCx, JS::Handle<JSObject*> aModule,
+                          JS::Handle<JSString*> aSpecifier);
 
   // Returns wether we should save the bytecode of this script after the
   // execution of the script.
   static bool
   ShouldCacheBytecode(ScriptLoadRequest* aRequest);
 
   nsresult CreateModuleScript(ModuleLoadRequest* aRequest);
   nsresult ProcessFetchedModuleSource(ModuleLoadRequest* aRequest);
--- a/js/public/Class.h
+++ b/js/public/Class.h
@@ -833,17 +833,17 @@ static const uint32_t JSCLASS_FOREGROUND
 // with the following flags. Failure to use JSCLASS_GLOBAL_FLAGS was
 // previously allowed, but is now an ES5 violation and thus unsupported.
 //
 // JSCLASS_GLOBAL_APPLICATION_SLOTS is the number of slots reserved at
 // the beginning of every global object's slots for use by the
 // application.
 static const uint32_t JSCLASS_GLOBAL_APPLICATION_SLOTS = 5;
 static const uint32_t JSCLASS_GLOBAL_SLOT_COUNT =
-    JSCLASS_GLOBAL_APPLICATION_SLOTS + JSProto_LIMIT * 2 + 37;
+    JSCLASS_GLOBAL_APPLICATION_SLOTS + JSProto_LIMIT * 2 + 36;
 
 #define JSCLASS_GLOBAL_FLAGS_WITH_SLOTS(n)                              \
     (JSCLASS_IS_GLOBAL | JSCLASS_HAS_RESERVED_SLOTS(JSCLASS_GLOBAL_SLOT_COUNT + (n)))
 #define JSCLASS_GLOBAL_FLAGS                                                  \
     JSCLASS_GLOBAL_FLAGS_WITH_SLOTS(0)
 #define JSCLASS_HAS_GLOBAL_FLAG_AND_SLOTS(clasp)                              \
   (((clasp)->flags & JSCLASS_IS_GLOBAL)                                       \
    && JSCLASS_RESERVED_SLOTS(clasp) >= JSCLASS_GLOBAL_SLOT_COUNT)
--- a/js/src/jsapi.cpp
+++ b/js/src/jsapi.cpp
@@ -4934,31 +4934,28 @@ JS::Evaluate(JSContext* cx, AutoObjectVe
 
 JS_PUBLIC_API(bool)
 JS::Evaluate(JSContext* cx, const ReadOnlyCompileOptions& optionsArg,
              const char* filename, MutableHandleValue rval)
 {
     return ::Evaluate(cx, optionsArg, filename, rval);
 }
 
-JS_PUBLIC_API(JSFunction*)
-JS::GetModuleResolveHook(JSContext* cx)
-{
-    AssertHeapIsIdle();
-    CHECK_REQUEST(cx);
-    return cx->global()->moduleResolveHook();
+JS_PUBLIC_API(JS::ModuleResolveHook)
+JS::GetModuleResolveHook(JSRuntime* rt)
+{
+    AssertHeapIsIdle();
+    return rt->moduleResolveHook;
 }
 
 JS_PUBLIC_API(void)
-JS::SetModuleResolveHook(JSContext* cx, HandleFunction func)
-{
-    AssertHeapIsIdle();
-    CHECK_REQUEST(cx);
-    assertSameCompartment(cx, func);
-    cx->global()->setModuleResolveHook(func);
+JS::SetModuleResolveHook(JSRuntime* rt, JS::ModuleResolveHook func)
+{
+    AssertHeapIsIdle();
+    rt->moduleResolveHook = func;
 }
 
 JS_PUBLIC_API(bool)
 JS::CompileModule(JSContext* cx, const ReadOnlyCompileOptions& options,
                   SourceBufferHolder& srcBuf, JS::MutableHandleObject module)
 {
     MOZ_ASSERT(!cx->runtime()->isAtomsCompartment(cx->compartment()));
     AssertHeapIsIdle();
--- a/js/src/jsapi.h
+++ b/js/src/jsapi.h
@@ -4136,27 +4136,29 @@ Evaluate(JSContext* cx, const ReadOnlyCo
 
 /**
  * Evaluate the given file in the scope of the current global of cx.
  */
 extern JS_PUBLIC_API(bool)
 Evaluate(JSContext* cx, const ReadOnlyCompileOptions& options,
          const char* filename, JS::MutableHandleValue rval);
 
-/**
- * Get the HostResolveImportedModule hook for a global.
- */
-extern JS_PUBLIC_API(JSFunction*)
-GetModuleResolveHook(JSContext* cx);
-
-/**
- * Set the HostResolveImportedModule hook for a global to the given function.
+using ModuleResolveHook = JSObject* (*)(JSContext*, HandleObject, HandleString);
+
+/**
+ * Get the HostResolveImportedModule hook for the runtime.
+ */
+extern JS_PUBLIC_API(ModuleResolveHook)
+GetModuleResolveHook(JSRuntime* rt);
+
+/**
+ * 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
  * of cx and return a source text module record.
  */
 extern JS_PUBLIC_API(bool)
 CompileModule(JSContext* cx, const ReadOnlyCompileOptions& options,
               SourceBufferHolder& srcBuf, JS::MutableHandleObject moduleRecord);
--- a/js/src/shell/js.cpp
+++ b/js/src/shell/js.cpp
@@ -605,17 +605,18 @@ ShellContext::ShellContext(JSContext* cx
     lastWarning(cx, NullValue()),
     promiseRejectionTrackerCallback(cx, NullValue()),
     watchdogLock(mutexid::ShellContextWatchdog),
     exitCode(0),
     quitting(false),
     readLineBufPos(0),
     errFilePtr(nullptr),
     outFilePtr(nullptr),
-    offThreadMonitor(mutexid::ShellOffThreadState)
+    offThreadMonitor(mutexid::ShellOffThreadState),
+    moduleResolveHook(cx)
 {}
 
 ShellContext::~ShellContext()
 {
     MOZ_ASSERT(offThreadJobs.empty());
 }
 
 ShellContext*
@@ -863,16 +864,17 @@ RunBinAST(JSContext* cx, const char* fil
     return JS_ExecuteScript(cx, script);
 }
 
 #endif // JS_BUILD_BINAST
 
 static bool
 InitModuleLoader(JSContext* cx)
 {
+
     // Decompress and evaluate the embedded module loader source to initialize
     // the module loader for the current compartment.
 
     uint32_t srcLen = moduleloader::GetRawScriptsSize();
     ScopedJSFreePtr<char> src(cx->pod_malloc<char>(srcLen));
     if (!src || !DecompressString(moduleloader::compressedSources, moduleloader::GetCompressedSize(),
                                   reinterpret_cast<unsigned char*>(src.get()), srcLen))
     {
@@ -4275,23 +4277,44 @@ SetModuleResolveHook(JSContext* cx, unsi
     }
 
     if (!args[0].isObject() || !args[0].toObject().is<JSFunction>()) {
         const char* typeName = InformalValueTypeName(args[0]);
         JS_ReportErrorASCII(cx, "expected hook function, got %s", typeName);
         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)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
 
     ShellContext* sc = GetShellContext(cx);
     if (sc->moduleLoadPath) {
         JSString* str = JS_NewStringCopyZ(cx, sc->moduleLoadPath.get());
@@ -9293,16 +9316,18 @@ main(int argc, char** argv, char** envp)
         JS_SetGCParameter(cx, JSGC_DYNAMIC_HEAP_GROWTH, 1);
         JS_SetGCParameter(cx, JSGC_DYNAMIC_MARK_SLICE, 1);
         JS_SetGCParameter(cx, JSGC_SLICE_TIME_BUDGET, 10);
     }
 #endif
 
     js::SetPreserveWrapperCallback(cx, DummyPreserveWrapperCallback);
 
+    JS::SetModuleResolveHook(cx->runtime(), CallModuleResolveHook);
+
     result = Shell(cx, &op, envp);
 
 #ifdef DEBUG
     if (OOM_printAllocationCount)
         printf("OOM max count: %" PRIu64 "\n", js::oom::counter);
 #endif
 
     JS_SetGrayGCRootsTracer(cx, nullptr, nullptr);
--- a/js/src/shell/jsshell.h
+++ b/js/src/shell/jsshell.h
@@ -176,16 +176,17 @@ struct ShellContext
 
     JS::UniqueChars moduleLoadPath;
     UniquePtr<MarkBitObservers> markObservers;
 
     // Off-thread parse state.
     js::Monitor offThreadMonitor;
     Vector<OffThreadJob*, 0, SystemAllocPolicy> offThreadJobs;
 
+    JS::PersistentRootedFunction moduleResolveHook;
 };
 
 extern ShellContext*
 GetShellContext(JSContext* cx);
 
 } /* namespace shell */
 } /* namespace js */
 
--- a/js/src/vm/GlobalObject.h
+++ b/js/src/vm/GlobalObject.h
@@ -100,17 +100,16 @@ class GlobalObject : public NativeObject
         IMPORT_ENTRY_PROTO,
         EXPORT_ENTRY_PROTO,
         REQUESTED_MODULE_PROTO,
         REGEXP_STATICS,
         RUNTIME_CODEGEN_ENABLED,
         DEBUGGERS,
         INTRINSICS,
         FOR_OF_PIC_CHAIN,
-        MODULE_RESOLVE_HOOK,
         WINDOW_PROXY,
 
         /* Total reserved-slot count for global objects. */
         RESERVED_SLOTS
     };
 
     /*
      * The slot count must be in the public API for JSCLASS_GLOBAL_FLAGS, and
@@ -816,29 +815,16 @@ class GlobalObject : public NativeObject
         Value v = getReservedSlot(WINDOW_PROXY);
         MOZ_ASSERT(v.isObject() || v.isUndefined());
         return v.isObject() ? &v.toObject() : nullptr;
     }
     void setWindowProxy(JSObject* windowProxy) {
         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>();
-    }
-
     // A class used in place of a prototype during off-thread parsing.
     struct OffThreadPlaceholderObject : public NativeObject
     {
         static const int32_t SlotIndexSlot = 0;
         static const Class class_;
         static OffThreadPlaceholderObject* New(JSContext* cx, unsigned slot);
         inline int32_t getSlotIndex() const;
     };
--- a/js/src/vm/Runtime.cpp
+++ b/js/src/vm/Runtime.cpp
@@ -169,17 +169,18 @@ JSRuntime::JSRuntime(JSRuntime* parentRu
     parallelParsingEnabled_(true),
     autoWritableJitCodeActive_(false),
     oomCallback(nullptr),
     debuggerMallocSizeOf(ReturnZeroSize),
     lastAnimationTime(0),
     performanceMonitoring_(),
     stackFormat_(parentRuntime ? js::StackFormat::Default
                                : js::StackFormat::SpiderMonkey),
-    wasmInstances(mutexid::WasmRuntimeInstances)
+    wasmInstances(mutexid::WasmRuntimeInstances),
+    moduleResolveHook()
 {
     JS_COUNT_CTOR(JSRuntime);
     liveRuntimesCount++;
 
     /* Initialize infallibly first, so we can goto bad and JS_DestroyRuntime. */
 
     PodZero(&asmJSCacheOps);
     lcovOutput().init();
--- a/js/src/vm/Runtime.h
+++ b/js/src/vm/Runtime.h
@@ -932,16 +932,19 @@ struct JSRuntime : public js::MallocProv
   public:
     js::RuntimeCaches& caches() { return caches_.ref(); }
 
     // List of all the live wasm::Instances in the runtime. Equal to the union
     // of all instances registered in all JSCompartments. Accessed from watchdog
     // threads for purposes of wasm::InterruptRunningCode().
     js::ExclusiveData<js::wasm::InstanceVector> wasmInstances;
 
+    // The implementation-defined abstract operation HostResolveImportedModule.
+    js::MainThreadData<JS::ModuleResolveHook> moduleResolveHook;
+
   public:
 #if defined(JS_BUILD_BINAST)
     js::BinaryASTSupport& binast() {
         return binast_;
     }
   private:
     js::BinaryASTSupport binast_;
 #endif // defined(JS_BUILD_BINAST)
--- a/js/src/vm/SelfHosting.cpp
+++ b/js/src/vm/SelfHosting.cpp
@@ -2141,35 +2141,36 @@ intrinsic_NameForTypedArray(JSContext* c
     return true;
 }
 
 static bool
 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());
-
-    RootedFunction moduleResolveHook(cx, cx->global()->moduleResolveHook());
+    RootedModuleObject module(cx, &args[0].toObject().as<ModuleObject>());
+    RootedString specifier(cx, args[1].toString());
+
+    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;
 }
 
 static bool
 intrinsic_CreateImportBinding(JSContext* cx, unsigned argc, Value* vp)
 {
     CallArgs args = CallArgsFromVp(argc, vp);
     MOZ_ASSERT(args.length() == 4);