From 27f4d0b5a50e6808ae7de14e1812d775d0754258 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Wed, 1 Jul 2020 21:12:16 +0000 Subject: [PATCH 01/20] Issue #618 - Check for failed instantiation when starting to fetch dependencies If instantiation has failed, then also fail the load and don't fetch imports. Ref BZ: 1358882 --- dom/script/ScriptLoader.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index adc046b7c4..ffbe37f281 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -721,6 +721,12 @@ ScriptLoader::StartFetchingModuleDependencies(ModuleLoadRequest* aRequest) { MOZ_ASSERT(aRequest->mModuleScript); MOZ_ASSERT(!aRequest->IsReadyToRun()); + + if (aRequest->mModuleScript->InstantiationFailed()) { + aRequest->LoadFailed(); + return; + } + aRequest->mProgress = ModuleLoadRequest::Progress::FetchingImports; nsCOMArray urls; From 336347212b0a8f255c48cccaa0279918a8d34309 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Wed, 1 Jul 2020 21:12:49 +0000 Subject: [PATCH 02/20] Issue #618 - Add clarifying code comments. --- dom/script/ModuleLoadRequest.cpp | 8 ++++++++ dom/script/ScriptLoader.cpp | 4 ++++ 2 files changed, 12 insertions(+) diff --git a/dom/script/ModuleLoadRequest.cpp b/dom/script/ModuleLoadRequest.cpp index e72edca2e8..98106df1b9 100644 --- a/dom/script/ModuleLoadRequest.cpp +++ b/dom/script/ModuleLoadRequest.cpp @@ -52,6 +52,14 @@ void ModuleLoadRequest::Cancel() void ModuleLoadRequest::SetReady() { + // Mark a module as ready to execute. This means that this module and all it + // dependencies have had their source loaded, parsed as a module and the + // modules instantiated. + // + // The mReady promise is used to ensure that when all dependencies of a module + // have become ready, DependenciesLoaded is called on that module + // request. This is set up in StartFetchingModuleDependencies. + #ifdef DEBUG for (size_t i = 0; i < mImports.Length(); i++) { MOZ_ASSERT(mImports[i]->IsReadyToRun()); diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index ffbe37f281..7aca04da5c 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -420,6 +420,10 @@ ScriptLoader::SetModuleFetchFinishedAndResumeWaitingRequests(ModuleLoadRequest * { // Update module map with the result of fetching a single module script. The // module script pointer is nullptr on error. + // + // If any requests for the same URL are waiting on this one to complete, they + // will have ModuleLoaded or LoadFailed on them when the promise is + // resolved/rejected. This is set up in StartLoad. MOZ_ASSERT(!aRequest->IsReadyToRun()); From c7d195e633b4adff8bb8c86db17c6e0fce20b130 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Fri, 3 Jul 2020 13:56:49 +0000 Subject: [PATCH 03/20] Issue #618 - Align module instantiation/errors with the updated spec. Store and re-throw module instantiation and evaluation errors. Ref: BZ 1374239, 1394492 --- js/src/builtin/Module.js | 563 ++++++++++++++---- js/src/builtin/ModuleObject.cpp | 69 ++- js/src/builtin/ModuleObject.h | 30 +- js/src/builtin/SelfHostingDefines.h | 16 +- js/src/builtin/Utilities.js | 19 +- .../tests/modules/ambiguous-star-export.js | 15 +- .../tests/modules/bad-namespace-created.js | 17 + js/src/jit-test/tests/modules/bug-1284486.js | 39 +- js/src/jit-test/tests/modules/bug-1287410.js | 2 + js/src/jit-test/tests/modules/bug-1394492.js | 6 + js/src/jit-test/tests/modules/global-scope.js | 18 +- .../tests/modules/module-evaluation.js | 33 +- js/src/js.msg | 2 +- js/src/jsapi.cpp | 4 +- js/src/vm/CommonPropertyNames.h | 4 +- js/src/vm/SelfHosting.cpp | 19 +- 16 files changed, 625 insertions(+), 231 deletions(-) create mode 100644 js/src/jit-test/tests/modules/bad-namespace-created.js create mode 100644 js/src/jit-test/tests/modules/bug-1394492.js diff --git a/js/src/builtin/Module.js b/js/src/builtin/Module.js index 5c3d5e1479..0642936701 100644 --- a/js/src/builtin/Module.js +++ b/js/src/builtin/Module.js @@ -2,11 +2,11 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ -function CallModuleResolveHook(module, specifier, expectedMinimumState) +function CallModuleResolveHook(module, specifier, expectedMinimumStatus) { let requestedModule = HostResolveImportedModule(module, specifier); - if (requestedModule.state < expectedMinimumState) - ThrowInternalError(JSMSG_BAD_MODULE_STATE); + if (requestedModule.state < expectedMinimumStatus) + ThrowInternalError(JSMSG_BAD_MODULE_STATUS); return requestedModule; } @@ -65,7 +65,36 @@ function ModuleGetExportedNames(exportStarSet = []) return exportedNames; } +function ModuleSetStatus(module, newStatus) +{ + assert(newStatus >= MODULE_STATUS_ERRORED && newStatus <= MODULE_STATUS_EVALUATED, + "Bad new module status in ModuleSetStatus"); + if (newStatus !== MODULE_STATUS_ERRORED) + assert(newStatus > module.status, "New module status inconsistent with current status"); + + UnsafeSetReservedSlot(module, MODULE_OBJECT_STATUS_SLOT, newStatus); +} + // 15.2.1.16.3 ResolveExport(exportName, resolveSet) +// +// Returns an object describing the location of the resolved export or +// indicating a failure. +// +// On success this returns: { resolved: true, module, bindingName } +// +// There are three failure cases: +// +// - The resolution failure can be blamed on a particular module. +// Returns: { resolved: false, module, ambiguous: false } +// +// - No culprit can be determined and the resolution failure was due to star +// export ambiguity. +// Returns: { resolved: false, module: null, ambiguous: true } +// +// - No culprit can be determined and the resolution failure was not due to +// star export ambiguity. +// Returns: { resolved: false, module: null, ambiguous: false } +// function ModuleResolveExport(exportName, resolveSet = []) { if (!IsObject(this) || !IsModule(this)) { @@ -77,88 +106,104 @@ function ModuleResolveExport(exportName, resolveSet = []) let module = this; // Step 2 + assert(module.status !== MODULE_STATUS_ERRORED, "Bad module status in ResolveExport"); + + // Step 3 for (let i = 0; i < resolveSet.length; i++) { let r = resolveSet[i]; - if (r.module === module && r.exportName === exportName) - return null; + if (r.module === module && r.exportName === exportName) { + // This is a circular import request. + return {resolved: false, module: null, ambiguous: false}; + } } - // Step 3 + // Step 4 _DefineDataProperty(resolveSet, resolveSet.length, {module: module, exportName: exportName}); - // Step 4 + // Step 5 let localExportEntries = module.localExportEntries; for (let i = 0; i < localExportEntries.length; i++) { let e = localExportEntries[i]; if (exportName === e.exportName) - return {module: module, bindingName: e.localName}; + return {resolved: true, module, bindingName: e.localName}; } - // Step 5 + // Step 6 let indirectExportEntries = module.indirectExportEntries; for (let i = 0; i < indirectExportEntries.length; i++) { let e = indirectExportEntries[i]; if (exportName === e.exportName) { let importedModule = CallModuleResolveHook(module, e.moduleRequest, - MODULE_STATE_PARSED); - return callFunction(importedModule.resolveExport, importedModule, e.importName, - resolveSet); + MODULE_STATUS_UNINSTANTIATED); + let resolution = callFunction(importedModule.resolveExport, importedModule, e.importName, + resolveSet); + if (!resolution.resolved && !resolution.module) + resolution.module = module; + return resolution; } } - // Step 6 + // Step 7 if (exportName === "default") { // A default export cannot be provided by an export *. - return null; + return {resolved: false, module: null, ambiguous: false}; } - // Step 7 + // Step 8 let starResolution = null; - // Step 8 + // Step 9 let starExportEntries = module.starExportEntries; for (let i = 0; i < starExportEntries.length; i++) { let e = starExportEntries[i]; let importedModule = CallModuleResolveHook(module, e.moduleRequest, - MODULE_STATE_PARSED); - let resolution = callFunction(importedModule.resolveExport, importedModule, - exportName, resolveSet); - if (resolution === "ambiguous") + MODULE_STATUS_UNINSTANTIATED); + let resolution = callFunction(importedModule.resolveExport, importedModule, exportName, + resolveSet); + if (!resolution.resolved && (resolution.module || resolution.ambiguous)) return resolution; - if (resolution !== null) { + if (resolution.resolved) { if (starResolution === null) { starResolution = resolution; } else { if (resolution.module !== starResolution.module || - resolution.exportName !== starResolution.exportName) + resolution.bindingName !== starResolution.bindingName) { - return "ambiguous"; + return {resolved: false, module: null, ambiguous: true}; } } } } - // Step 9 - return starResolution; + // Step 10 + if (starResolution !== null) + return starResolution; + + return {resolved: false, module: null, ambiguous: false}; } // 15.2.1.18 GetModuleNamespace(module) function GetModuleNamespace(module) { + // Step 1 + assert(IsModule(module), "GetModuleNamespace called with non-module"); + // Step 2 - let namespace = module.namespace; + assert(module.status !== MODULE_STATUS_UNINSTANTIATED && + module.status !== MODULE_STATUS_ERRORED, + "Bad module status in GetModuleNamespace"); // Step 3 + let namespace = module.namespace; + if (typeof namespace === "undefined") { let exportedNames = callFunction(module.getExportedNames, module); let unambiguousNames = []; for (let i = 0; i < exportedNames.length; i++) { let name = exportedNames[i]; let resolution = callFunction(module.resolveExport, module, name); - if (resolution === null) - ThrowSyntaxError(JSMSG_MISSING_NAMESPACE_EXPORT); - if (resolution !== "ambiguous") + if (resolution.resolved) _DefineDataProperty(unambiguousNames, unambiguousNames.length, name); } namespace = ModuleNamespaceCreate(module, unambiguousNames); @@ -180,7 +225,7 @@ function ModuleNamespaceCreate(module, exports) for (let i = 0; i < exports.length; i++) { let name = exports[i]; let binding = callFunction(module.resolveExport, module, name); - assert(binding !== null && binding !== "ambiguous", "Failed to resolve binding"); + assert(binding.resolved, "Failed to resolve binding"); AddModuleNamespaceBinding(ns, name, binding.module, binding.bindingName); } @@ -193,8 +238,8 @@ function GetModuleEnvironment(module) // Check for a previous failed attempt to instantiate this module. This can // only happen due to a bug in the module loader. - if (module.state == MODULE_STATE_FAILED) - ThrowInternalError(JSMSG_MODULE_INSTANTIATE_FAILED); + if (module.status === MODULE_STATUS_ERRORED) + ThrowInternalError(JSMSG_MODULE_INSTANTIATE_FAILED, module.status); let env = UnsafeGetReservedSlot(module, MODULE_OBJECT_ENVIRONMENT_SLOT); assert(env === undefined || IsModuleEnvironment(env), @@ -203,112 +248,396 @@ function GetModuleEnvironment(module) return env; } -function RecordInstantationFailure(module) +function RecordModuleError(module, error) { - // Set the module's state to 'failed' to indicate a failed module - // instantiation and reset the environment slot to 'undefined'. - assert(IsModule(module), "Non-module passed to RecordInstantationFailure"); - SetModuleState(module, MODULE_STATE_FAILED); + // Set the module's status to 'errored' to indicate a failed module + // instantiation and record the exception. The environment slot is also + // reset to 'undefined'. + + assert(IsObject(module) && IsModule(module), "Non-module passed to RecordModuleError"); + + ModuleSetStatus(module, MODULE_STATUS_ERRORED); + UnsafeSetReservedSlot(module, MODULE_OBJECT_ERROR_SLOT, error); UnsafeSetReservedSlot(module, MODULE_OBJECT_ENVIRONMENT_SLOT, undefined); } -// 15.2.1.16.4 ModuleDeclarationInstantiation() -function ModuleDeclarationInstantiation() +function CountArrayValues(array, value) { - if (!IsObject(this) || !IsModule(this)) - return callFunction(CallModuleMethodIfWrapped, this, "ModuleDeclarationInstantiation"); - - // Step 1 - let module = this; - - // Step 5 - if (GetModuleEnvironment(module) !== undefined) - return; - - // Step 7 - CreateModuleEnvironment(module); - let env = GetModuleEnvironment(module); - - SetModuleState(this, MODULE_STATE_INSTANTIATED); - - try { - // Step 8 - let requestedModules = module.requestedModules; - for (let i = 0; i < requestedModules.length; i++) { - let required = requestedModules[i]; - let requiredModule = CallModuleResolveHook(module, required, MODULE_STATE_PARSED); - callFunction(requiredModule.declarationInstantiation, requiredModule); - } - - // Step 9 - let indirectExportEntries = module.indirectExportEntries; - for (let i = 0; i < indirectExportEntries.length; i++) { - let e = indirectExportEntries[i]; - let resolution = callFunction(module.resolveExport, module, e.exportName); - if (resolution === null) - ThrowSyntaxError(JSMSG_MISSING_INDIRECT_EXPORT, e.exportName); - if (resolution === "ambiguous") - ThrowSyntaxError(JSMSG_AMBIGUOUS_INDIRECT_EXPORT, e.exportName); - } - - // Step 12 - let importEntries = module.importEntries; - for (let i = 0; i < importEntries.length; i++) { - let imp = importEntries[i]; - let importedModule = CallModuleResolveHook(module, imp.moduleRequest, - MODULE_STATE_INSTANTIATED); - if (imp.importName === "*") { - let namespace = GetModuleNamespace(importedModule); - CreateNamespaceBinding(env, imp.localName, namespace); - } else { - let resolution = callFunction(importedModule.resolveExport, importedModule, - imp.importName); - if (resolution === null) - ThrowSyntaxError(JSMSG_MISSING_IMPORT, imp.importName); - if (resolution === "ambiguous") - ThrowSyntaxError(JSMSG_AMBIGUOUS_IMPORT, imp.importName); - if (resolution.module.state < MODULE_STATE_INSTANTIATED) - ThrowInternalError(JSMSG_BAD_MODULE_STATE); - CreateImportBinding(env, imp.localName, resolution.module, resolution.bindingName); - } - } - - // Step 17.a.iii - InstantiateModuleFunctionDeclarations(module); - } catch (e) { - RecordInstantationFailure(module); - throw e; + let count = 0; + for (let i = 0; i < array.length; i++) { + if (array[i] === value) + count++; } + return count; } -_SetCanonicalName(ModuleDeclarationInstantiation, "ModuleDeclarationInstantiation"); -// 15.2.1.16.5 ModuleEvaluation() -function ModuleEvaluation() +function ArrayContains(array, value) +{ + for (let i = 0; i < array.length; i++) { + if (array[i] === value) + return true; + } + return false; +} + +// 15.2.1.16.4 ModuleInstantiate() +function ModuleInstantiate() { if (!IsObject(this) || !IsModule(this)) - return callFunction(CallModuleMethodIfWrapped, this, "ModuleEvaluation"); + return callFunction(CallModuleMethodIfWrapped, this, "ModuleInstantiate"); // Step 1 let module = this; - if (module.state < MODULE_STATE_INSTANTIATED) - ThrowInternalError(JSMSG_BAD_MODULE_STATE); + // Step 2 + if (module.status === MODULE_STATUS_INSTANTIATING || + module.status === MODULE_STATUS_EVALUATING) + { + ThrowInternalError(JSMSG_BAD_MODULE_STATUS); + } - // Step 4 - if (module.state == MODULE_STATE_EVALUATED) - return undefined; + // Step 3 + let stack = []; - // Step 5 - SetModuleState(this, MODULE_STATE_EVALUATED); + // Steps 4-5 + try { + InnerModuleDeclarationInstantiation(module, stack, 0); + } catch (error) { + for (let i = 0; i < stack.length; i++) { + let m = stack[i]; + + assert(m.status === MODULE_STATUS_INSTANTIATING || + m.status === MODULE_STATUS_ERRORED, + "Bad module status after failed instantiation"); + + RecordModuleError(m, error); + } + + if (stack.length === 0 && + typeof(UnsafeGetReservedSlot(module, MODULE_OBJECT_ERROR_SLOT)) === "undefined") + { + // This can happen due to OOM when appending to the stack. + assert(error === "out of memory", + "Stack must contain module unless we hit OOM"); + RecordModuleError(module, error); + } + + assert(module.status === MODULE_STATUS_ERRORED, + "Bad module status after failed instantiation"); + assert(SameValue(UnsafeGetReservedSlot(module, MODULE_OBJECT_ERROR_SLOT), error), + "Module has different error set after failed instantiation"); + + throw error; + } // Step 6 + assert(module.status == MODULE_STATUS_INSTANTIATED || + module.status == MODULE_STATUS_EVALUATED, + "Bad module status after successful instantiation"); + + // Step 7 + assert(stack.length === 0, + "Stack should be empty after successful instantiation"); + + // Step 8 + return undefined; +} +_SetCanonicalName(ModuleInstantiate, "ModuleInstantiate"); + +// 15.2.1.16.4.1 InnerModuleDeclarationInstantiation(module, stack, index) +function InnerModuleDeclarationInstantiation(module, stack, index) +{ + // Step 1 + // TODO: Support module records other than source text module records. + + // Step 2 + if (module.status === MODULE_STATUS_INSTANTIATING || + module.status === MODULE_STATUS_INSTANTIATED || + module.status === MODULE_STATUS_EVALUATED) + { + return index; + } + + // Step 3 + if (module.status === MODULE_STATUS_ERRORED) + throw module.error; + + // Step 4 + assert(module.status === MODULE_STATUS_UNINSTANTIATED, + "Bad module status in ModuleDeclarationInstantiation"); + + // Steps 5 + ModuleSetStatus(module, MODULE_STATUS_INSTANTIATING); + + // Step 6-8 + UnsafeSetReservedSlot(module, MODULE_OBJECT_DFS_INDEX_SLOT, index); + UnsafeSetReservedSlot(module, MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT, index); + index++; + + // Step 9 + _DefineDataProperty(stack, stack.length, module); + + // Step 10 let requestedModules = module.requestedModules; for (let i = 0; i < requestedModules.length; i++) { let required = requestedModules[i]; - let requiredModule = CallModuleResolveHook(module, required, MODULE_STATE_INSTANTIATED); - callFunction(requiredModule.evaluation, requiredModule); + let requiredModule = CallModuleResolveHook(module, required, MODULE_STATUS_ERRORED); + + index = InnerModuleDeclarationInstantiation(requiredModule, stack, index); + + assert(requiredModule.status === MODULE_STATUS_INSTANTIATING || + requiredModule.status === MODULE_STATUS_INSTANTIATED || + requiredModule.status === MODULE_STATUS_EVALUATED, + "Bad required module status after InnerModuleDeclarationInstantiation"); + + assert((requiredModule.status === MODULE_STATUS_INSTANTIATING) === + ArrayContains(stack, requiredModule), + "Required module should be in the stack iff it is currently being instantiated"); + + assert(typeof requiredModule.dfsIndex === "number", "Bad dfsIndex"); + assert(typeof requiredModule.dfsAncestorIndex === "number", "Bad dfsAncestorIndex"); + + if (requiredModule.status === MODULE_STATUS_INSTANTIATING) { + UnsafeSetReservedSlot(module, MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT, + std_Math_min(module.dfsAncestorIndex, + requiredModule.dfsAncestorIndex)); + } } - return EvaluateModule(module); + // Step 11 + ModuleDeclarationEnvironmentSetup(module); + + // Steps 12-13 + assert(CountArrayValues(stack, module) === 1, + "Current module should appear exactly once in the stack"); + assert(module.dfsAncestorIndex <= module.dfsIndex, + "Bad DFS ancestor index"); + + // Step 14 + if (module.dfsAncestorIndex === module.dfsIndex) { + let requiredModule; + do { + requiredModule = callFunction(std_Array_pop, stack); + ModuleSetStatus(requiredModule, MODULE_STATUS_INSTANTIATED); + } while (requiredModule !== module); + } + + // Step 15 + return index; +} + +// 15.2.1.16.4.2 ModuleDeclarationEnvironmentSetup(module) +function ModuleDeclarationEnvironmentSetup(module) +{ + // Step 1 + let indirectExportEntries = module.indirectExportEntries; + for (let i = 0; i < indirectExportEntries.length; i++) { + let e = indirectExportEntries[i]; + let resolution = callFunction(module.resolveExport, module, e.exportName); + assert(resolution.resolved || resolution.module, + "Unexpected failure to resolve export in ModuleDeclarationEnvironmentSetup"); + if (!resolution.resolved) + return ResolutionError(resolution, "indirectExport", e.exportName) + } + + // Steps 5-6 + CreateModuleEnvironment(module); + let env = GetModuleEnvironment(module); + + // Step 8 + let importEntries = module.importEntries; + for (let i = 0; i < importEntries.length; i++) { + let imp = importEntries[i]; + let importedModule = CallModuleResolveHook(module, imp.moduleRequest, + MODULE_STATUS_INSTANTIATING); + if (imp.importName === "*") { + let namespace = GetModuleNamespace(importedModule); + CreateNamespaceBinding(env, imp.localName, namespace); + } else { + let resolution = callFunction(importedModule.resolveExport, importedModule, + imp.importName); + if (!resolution.resolved && !resolution.module) + resolution.module = module; + + if (!resolution.resolved) + return ResolutionError(resolution, "import", imp.importName); + + CreateImportBinding(env, imp.localName, resolution.module, resolution.bindingName); + } + } + + InstantiateModuleFunctionDeclarations(module); +} + +// 15.2.1.16.4.3 ResolutionError(module) +function ResolutionError(resolution, kind, name) +{ + let module = resolution.module; + assert(module !== null, + "Null module passed to ResolutionError"); + + assert(module.status === MODULE_STATUS_UNINSTANTIATED || + module.status === MODULE_STATUS_INSTANTIATING, + "Unexpected module status in ResolutionError"); + + assert(kind === "import" || kind === "indirectExport", + "Unexpected kind in ResolutionError"); + + let message; + if (kind === "import") { + message = resolution.ambiguous ? JSMSG_AMBIGUOUS_IMPORT + : JSMSG_MISSING_IMPORT; + } else { + message = resolution.ambiguous ? JSMSG_AMBIGUOUS_INDIRECT_EXPORT + : JSMSG_MISSING_INDIRECT_EXPORT; + } + + try { + ThrowSyntaxError(message, name); + } catch (error) { + RecordModuleError(module, error); + throw error; + } +} + +// 15.2.1.16.5 ModuleEvaluate() +function ModuleEvaluate() +{ + if (!IsObject(this) || !IsModule(this)) + return callFunction(CallModuleMethodIfWrapped, this, "ModuleEvaluatie"); + + // Step 1 + let module = this; + + // Step 2 + if (module.status !== MODULE_STATUS_ERRORED && + module.status !== MODULE_STATUS_INSTANTIATED && + module.status !== MODULE_STATUS_EVALUATED) + { + ThrowInternalError(JSMSG_BAD_MODULE_STATUS); + } + + // Step 3 + let stack = []; + + // Steps 4-5 + try { + InnerModuleEvaluation(module, stack, 0); + } catch (error) { + for (let i = 0; i < stack.length; i++) { + let m = stack[i]; + + assert(m.status === MODULE_STATUS_EVALUATING, + "Bad module status after failed evaluation"); + + RecordModuleError(m, error); + } + + if (stack.length === 0 && + typeof(UnsafeGetReservedSlot(module, MODULE_OBJECT_ERROR_SLOT)) === "undefined") + { + // This can happen due to OOM when appending to the stack. + assert(error === "out of memory", + "Stack must contain module unless we hit OOM"); + RecordModuleError(module, error); + } + + assert(module.status === MODULE_STATUS_ERRORED, + "Bad module status after failed evaluation"); + assert(SameValue(UnsafeGetReservedSlot(module, MODULE_OBJECT_ERROR_SLOT), error), + "Module has different error set after failed evaluation"); + + throw error; + } + + assert(module.status == MODULE_STATUS_EVALUATED, + "Bad module status after successful evaluation"); + assert(stack.length === 0, + "Stack should be empty after successful evaluation"); + + return undefined; +} +_SetCanonicalName(ModuleEvaluate, "ModuleEvaluate"); + +// 15.2.1.16.5.1 InnerModuleEvaluation(module, stack, index) +function InnerModuleEvaluation(module, stack, index) +{ + // Step 1 + // TODO: Support module records other than source text module records. + + // Step 2 + if (module.status === MODULE_STATUS_EVALUATING || + module.status === MODULE_STATUS_EVALUATED) + { + return index; + } + + // Step 3 + if (module.status === MODULE_STATUS_ERRORED) + throw module.error; + + // Step 4 + assert(module.status === MODULE_STATUS_INSTANTIATED, + "Bad module status in ModuleEvaluation"); + + // Step 5 + ModuleSetStatus(module, MODULE_STATUS_EVALUATING); + + // Steps 6-8 + UnsafeSetReservedSlot(module, MODULE_OBJECT_DFS_INDEX_SLOT, index); + UnsafeSetReservedSlot(module, MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT, index); + index++; + + // Step 9 + _DefineDataProperty(stack, stack.length, module); + + // Step 10 + let requestedModules = module.requestedModules; + for (let i = 0; i < requestedModules.length; i++) { + let required = requestedModules[i]; + let requiredModule = + CallModuleResolveHook(module, required, MODULE_STATUS_INSTANTIATED); + + index = InnerModuleEvaluation(requiredModule, stack, index); + + assert(requiredModule.status == MODULE_STATUS_EVALUATING || + requiredModule.status == MODULE_STATUS_EVALUATED, + "Bad module status after InnerModuleEvaluation"); + + assert((requiredModule.status === MODULE_STATUS_EVALUATING) === + ArrayContains(stack, requiredModule), + "Required module should be in the stack iff it is currently being evaluated"); + + assert(typeof requiredModule.dfsIndex === "number", "Bad dfsIndex"); + assert(typeof requiredModule.dfsAncestorIndex === "number", "Bad dfsAncestorIndex"); + + if (requiredModule.status === MODULE_STATUS_EVALUATING) { + UnsafeSetReservedSlot(module, MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT, + std_Math_min(module.dfsAncestorIndex, + requiredModule.dfsAncestorIndex)); + } + } + + // Step 11 + ExecuteModule(module); + + // Step 12 + assert(CountArrayValues(stack, module) === 1, + "Current module should appear exactly once in the stack"); + + // Step 13 + assert(module.dfsAncestorIndex <= module.dfsIndex, + "Bad DFS ancestor index"); + + // Step 14 + if (module.dfsAncestorIndex === module.dfsIndex) { + let requiredModule; + do { + requiredModule = callFunction(std_Array_pop, stack); + ModuleSetStatus(requiredModule, MODULE_STATUS_EVALUATED); + } while (requiredModule !== module); + } + + // Step 15 + return index; } -_SetCanonicalName(ModuleEvaluation, "ModuleEvaluation"); diff --git a/js/src/builtin/ModuleObject.cpp b/js/src/builtin/ModuleObject.cpp index b275cb9681..0a836c75bf 100644 --- a/js/src/builtin/ModuleObject.cpp +++ b/js/src/builtin/ModuleObject.cpp @@ -18,10 +18,11 @@ using namespace js; using namespace js::frontend; -static_assert(MODULE_STATE_FAILED < MODULE_STATE_PARSED && - MODULE_STATE_PARSED < MODULE_STATE_INSTANTIATED && - MODULE_STATE_INSTANTIATED < MODULE_STATE_EVALUATED, - "Module states are ordered incorrectly"); +static_assert(MODULE_STATUS_ERRORED < MODULE_STATUS_UNINSTANTIATED && + MODULE_STATUS_UNINSTANTIATED < MODULE_STATUS_INSTANTIATING && + MODULE_STATUS_INSTANTIATING < MODULE_STATUS_INSTANTIATED && + MODULE_STATUS_INSTANTIATED < MODULE_STATUS_EVALUATED, + "Module statuses are ordered incorrectly"); template static bool @@ -42,7 +43,7 @@ ModuleValueGetter(JSContext* cx, unsigned argc, Value* vp) #define DEFINE_GETTER_FUNCTIONS(cls, name, slot) \ static Value \ cls##_##name##Value(const cls* obj) { \ - return obj->getFixedSlot(cls::slot); \ + return obj->getReservedSlot(cls::slot); \ } \ \ static bool \ @@ -564,7 +565,7 @@ ModuleObject::class_ = { ArrayObject& \ cls::name() const \ { \ - return getFixedSlot(cls::slot).toObject().as(); \ + return getReservedSlot(cls::slot).toObject().as(); \ } DEFINE_ARRAY_SLOT_ACCESSOR(ModuleObject, requestedModules, RequestedModulesSlot) @@ -688,7 +689,7 @@ void ModuleObject::init(HandleScript script) { initReservedSlot(ScriptSlot, PrivateValue(script)); - initReservedSlot(StateSlot, Int32Value(MODULE_STATE_FAILED)); + initReservedSlot(StatusSlot, Int32Value(MODULE_STATUS_ERRORED)); } void @@ -709,7 +710,7 @@ ModuleObject::initImportExportData(HandleArrayObject requestedModules, initReservedSlot(LocalExportEntriesSlot, ObjectValue(*localExportEntries)); initReservedSlot(IndirectExportEntriesSlot, ObjectValue(*indirectExportEntries)); initReservedSlot(StarExportEntriesSlot, ObjectValue(*starExportEntries)); - setReservedSlot(StateSlot, Int32Value(MODULE_STATE_PARSED)); + setReservedSlot(StatusSlot, Int32Value(MODULE_STATUS_UNINSTANTIATED)); } static bool @@ -790,17 +791,24 @@ ModuleObject::script() const } static inline void -AssertValidModuleState(ModuleState state) +AssertValidModuleStatus(ModuleStatus status) { - MOZ_ASSERT(state >= MODULE_STATE_FAILED && state <= MODULE_STATE_EVALUATED); + MOZ_ASSERT(status >= MODULE_STATUS_ERRORED && status <= MODULE_STATUS_EVALUATED); } -ModuleState -ModuleObject::state() const +ModuleStatus +ModuleObject::status() const { - ModuleState state = getReservedSlot(StateSlot).toInt32(); - AssertValidModuleState(state); - return state; + ModuleStatus status = getReservedSlot(StatusSlot).toInt32(); + AssertValidModuleStatus(status); + return status; +} + +Value +ModuleObject::error() const +{ + MOZ_ASSERT(status() == MODULE_STATUS_ERRORED); + return getReservedSlot(ErrorSlot); } Value @@ -899,17 +907,8 @@ ModuleObject::instantiateFunctionDeclarations(JSContext* cx, HandleModuleObject return true; } -void -ModuleObject::setState(int32_t newState) -{ - AssertValidModuleState(newState); - MOZ_ASSERT(state() != MODULE_STATE_FAILED); - MOZ_ASSERT(newState == MODULE_STATE_FAILED || newState > state()); - setReservedSlot(StateSlot, Int32Value(newState)); -} - /* static */ bool -ModuleObject::evaluate(JSContext* cx, HandleModuleObject self, MutableHandleValue rval) +ModuleObject::execute(JSContext* cx, HandleModuleObject self, MutableHandleValue rval) { MOZ_ASSERT(IsFrozen(cx, self)); @@ -959,36 +958,42 @@ InvokeSelfHostedMethod(JSContext* cx, HandleModuleObject self, HandlePropertyNam } /* static */ bool -ModuleObject::DeclarationInstantiation(JSContext* cx, HandleModuleObject self) +ModuleObject::Instantiate(JSContext* cx, HandleModuleObject self) { - return InvokeSelfHostedMethod(cx, self, cx->names().ModuleDeclarationInstantiation); + return InvokeSelfHostedMethod(cx, self, cx->names().ModuleInstantiate); } /* static */ bool -ModuleObject::Evaluation(JSContext* cx, HandleModuleObject self) +ModuleObject::Evaluate(JSContext* cx, HandleModuleObject self) { - return InvokeSelfHostedMethod(cx, self, cx->names().ModuleEvaluation); + return InvokeSelfHostedMethod(cx, self, cx->names().ModuleEvaluate); } DEFINE_GETTER_FUNCTIONS(ModuleObject, namespace_, NamespaceSlot) -DEFINE_GETTER_FUNCTIONS(ModuleObject, state, StateSlot) +DEFINE_GETTER_FUNCTIONS(ModuleObject, status, StatusSlot) +DEFINE_GETTER_FUNCTIONS(ModuleObject, error, ErrorSlot) DEFINE_GETTER_FUNCTIONS(ModuleObject, requestedModules, RequestedModulesSlot) DEFINE_GETTER_FUNCTIONS(ModuleObject, importEntries, ImportEntriesSlot) DEFINE_GETTER_FUNCTIONS(ModuleObject, localExportEntries, LocalExportEntriesSlot) DEFINE_GETTER_FUNCTIONS(ModuleObject, indirectExportEntries, IndirectExportEntriesSlot) DEFINE_GETTER_FUNCTIONS(ModuleObject, starExportEntries, StarExportEntriesSlot) +DEFINE_GETTER_FUNCTIONS(ModuleObject, dfsIndex, DFSIndexSlot) +DEFINE_GETTER_FUNCTIONS(ModuleObject, dfsAncestorIndex, DFSAncestorIndexSlot) /* static */ bool GlobalObject::initModuleProto(JSContext* cx, Handle global) { static const JSPropertySpec protoAccessors[] = { JS_PSG("namespace", ModuleObject_namespace_Getter, 0), - JS_PSG("state", ModuleObject_stateGetter, 0), + JS_PSG("status", ModuleObject_statusGetter, 0), + JS_PSG("error", ModuleObject_errorGetter, 0), JS_PSG("requestedModules", ModuleObject_requestedModulesGetter, 0), JS_PSG("importEntries", ModuleObject_importEntriesGetter, 0), JS_PSG("localExportEntries", ModuleObject_localExportEntriesGetter, 0), JS_PSG("indirectExportEntries", ModuleObject_indirectExportEntriesGetter, 0), JS_PSG("starExportEntries", ModuleObject_starExportEntriesGetter, 0), + JS_PSG("dfsIndex", ModuleObject_dfsIndexGetter, 0), + JS_PSG("dfsAncestorIndex", ModuleObject_dfsAncestorIndexGetter, 0), JS_PS_END }; @@ -997,6 +1002,8 @@ GlobalObject::initModuleProto(JSContext* cx, Handle global) JS_SELF_HOSTED_FN("resolveExport", "ModuleResolveExport", 2, 0), JS_SELF_HOSTED_FN("declarationInstantiation", "ModuleDeclarationInstantiation", 0, 0), JS_SELF_HOSTED_FN("evaluation", "ModuleEvaluation", 0, 0), + JS_SELF_HOSTED_FN("declarationInstantiation", "ModuleInstantiate", 0, 0), + JS_SELF_HOSTED_FN("evaluation", "ModuleEvaluate", 0, 0), JS_FS_END }; diff --git a/js/src/builtin/ModuleObject.h b/js/src/builtin/ModuleObject.h index 22db762ac6..365fcd4bf3 100644 --- a/js/src/builtin/ModuleObject.h +++ b/js/src/builtin/ModuleObject.h @@ -192,8 +192,8 @@ struct FunctionDeclaration using FunctionDeclarationVector = GCVector; -// Possible values for ModuleState are defined in SelfHostingDefines.h. -using ModuleState = int32_t; +// Possible values for ModuleStatus are defined in SelfHostingDefines.h. +using ModuleStatus = int32_t; class ModuleObject : public NativeObject { @@ -204,7 +204,8 @@ class ModuleObject : public NativeObject InitialEnvironmentSlot, EnvironmentSlot, NamespaceSlot, - StateSlot, + StatusSlot, + ErrorSlot, HostDefinedSlot, RequestedModulesSlot, ImportEntriesSlot, @@ -215,11 +216,21 @@ class ModuleObject : public NativeObject NamespaceExportsSlot, NamespaceBindingsSlot, FunctionDeclarationsSlot, + DFSIndexSlot, + DFSAncestorIndexSlot, SlotCount }; static_assert(EnvironmentSlot == MODULE_OBJECT_ENVIRONMENT_SLOT, "EnvironmentSlot must match self-hosting define"); + static_assert(StatusSlot == MODULE_OBJECT_STATUS_SLOT, + "StatusSlot must match self-hosting define"); + static_assert(ErrorSlot == MODULE_OBJECT_ERROR_SLOT, + "ErrorSlot must match self-hosting define"); + static_assert(DFSIndexSlot == MODULE_OBJECT_DFS_INDEX_SLOT, + "DFSIndexSlot must match self-hosting define"); + static_assert(DFSAncestorIndexSlot == MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT, + "DFSAncestorIndexSlot must match self-hosting define"); static const Class class_; @@ -244,7 +255,8 @@ class ModuleObject : public NativeObject ModuleEnvironmentObject& initialEnvironment() const; ModuleEnvironmentObject* environment() const; ModuleNamespaceObject* namespace_(); - ModuleState state() const; + ModuleStatus status() const; + Value error() const; Value hostDefinedField() const; ArrayObject& requestedModules() const; ArrayObject& importEntries() const; @@ -255,8 +267,8 @@ class ModuleObject : public NativeObject JSObject* namespaceExports(); IndirectBindingMap* namespaceBindings(); - static bool DeclarationInstantiation(JSContext* cx, HandleModuleObject self); - static bool Evaluation(JSContext* cx, HandleModuleObject self); + static bool Instantiate(JSContext* cx, HandleModuleObject self); + static bool Evaluate(JSContext* cx, HandleModuleObject self); void setHostDefinedField(const JS::Value& value); @@ -269,10 +281,8 @@ class ModuleObject : public NativeObject // For intrinsic_InstantiateModuleFunctionDeclarations. static bool instantiateFunctionDeclarations(JSContext* cx, HandleModuleObject self); - void setState(ModuleState newState); - - // For intrinsic_EvaluateModule. - static bool evaluate(JSContext* cx, HandleModuleObject self, MutableHandleValue rval); + // For intrinsic_ExecuteModule. + static bool execute(JSContext* cx, HandleModuleObject self, MutableHandleValue rval); // For intrinsic_NewModuleNamespace. static ModuleNamespaceObject* createNamespace(JSContext* cx, HandleModuleObject self, diff --git a/js/src/builtin/SelfHostingDefines.h b/js/src/builtin/SelfHostingDefines.h index 117ac7ffd3..a06c2aa62c 100644 --- a/js/src/builtin/SelfHostingDefines.h +++ b/js/src/builtin/SelfHostingDefines.h @@ -97,11 +97,17 @@ #define REGEXP_STRING_ITERATOR_FLAGS_SLOT 2 #define REGEXP_STRING_ITERATOR_DONE_SLOT 3 -#define MODULE_OBJECT_ENVIRONMENT_SLOT 2 +#define MODULE_OBJECT_ENVIRONMENT_SLOT 2 +#define MODULE_OBJECT_STATUS_SLOT 4 +#define MODULE_OBJECT_ERROR_SLOT 5 +#define MODULE_OBJECT_DFS_INDEX_SLOT 16 +#define MODULE_OBJECT_DFS_ANCESTOR_INDEX_SLOT 17 -#define MODULE_STATE_FAILED 0 -#define MODULE_STATE_PARSED 1 -#define MODULE_STATE_INSTANTIATED 2 -#define MODULE_STATE_EVALUATED 3 +#define MODULE_STATUS_ERRORED 0 +#define MODULE_STATUS_UNINSTANTIATED 1 +#define MODULE_STATUS_INSTANTIATING 2 +#define MODULE_STATUS_INSTANTIATED 3 +#define MODULE_STATUS_EVALUATING 4 +#define MODULE_STATUS_EVALUATED 5 #endif diff --git a/js/src/builtin/Utilities.js b/js/src/builtin/Utilities.js index d5f233d050..3916311db3 100644 --- a/js/src/builtin/Utilities.js +++ b/js/src/builtin/Utilities.js @@ -27,13 +27,20 @@ // Assertions and debug printing, defined here instead of in the header above // to make `assert` invisible to C++. #ifdef DEBUG -#define assert(b, info) if (!(b)) AssertionFailed(__FILE__ + ":" + __LINE__ + ": " + info) -#define dbg(msg) DumpMessage(callFunction(std_Array_pop, \ - StringSplitString(__FILE__, '/')) \ - + '#' + __LINE__ + ': ' + msg) +#define assert(b, info) \ + do { \ + if (!(b)) \ + AssertionFailed(__FILE__ + ":" + __LINE__ + ": " + info) \ + } while (false) +#define dbg(msg) \ + do { \ + DumpMessage(callFunction(std_Array_pop, \ + StringSplitString(__FILE__, '/')) + \ + '#' + __LINE__ + ': ' + msg) \ + } while (false) #else -#define assert(b, info) // Elided assertion. -#define dbg(msg) // Elided debugging output. +#define assert(b, info) do {} while (false) // Elided assertion. +#define dbg(msg) do {} while (false) // Elided debugging output. #endif // All C++-implemented standard builtins library functions used in self-hosted diff --git a/js/src/jit-test/tests/modules/ambiguous-star-export.js b/js/src/jit-test/tests/modules/ambiguous-star-export.js index b8c91445c3..94aa7ac4af 100644 --- a/js/src/jit-test/tests/modules/ambiguous-star-export.js +++ b/js/src/jit-test/tests/modules/ambiguous-star-export.js @@ -5,10 +5,11 @@ load(libdir + "asserts.js"); load(libdir + "dummyModuleResolveHook.js"); -function checkModuleEval(source, result) { +function checkModuleEval(source) { let m = parseModule(source); m.declarationInstantiation(); - assertEq(m.evaluation(), result); + m.evaluation(); + return m; } function checkModuleSyntaxError(source) { @@ -23,17 +24,19 @@ c.declarationInstantiation(); c.evaluation(); // Check importing/exporting non-ambiguous name works. -checkModuleEval("import { a } from 'c'; a;", 1); -checkModuleEval("export { a } from 'c';", undefined); +let d = checkModuleEval("import { a } from 'c';"); +assertEq(getModuleEnvironmentValue(d, "a"), 1); +checkModuleEval("export { a } from 'c';"); // Check importing/exporting ambiguous name is a syntax error. checkModuleSyntaxError("import { b } from 'c';"); checkModuleSyntaxError("export { b } from 'c';"); // Check that namespace objects include only non-ambiguous names. -let m = parseModule("import * as ns from 'c'; ns;"); +let m = parseModule("import * as ns from 'c';"); m.declarationInstantiation(); -let ns = m.evaluation(); +m.evaluation(); +let ns = c.namespace; let names = Object.keys(ns); assertEq(names.length, 2); assertEq('a' in ns, true); diff --git a/js/src/jit-test/tests/modules/bad-namespace-created.js b/js/src/jit-test/tests/modules/bad-namespace-created.js new file mode 100644 index 0000000000..127892d6e7 --- /dev/null +++ b/js/src/jit-test/tests/modules/bad-namespace-created.js @@ -0,0 +1,17 @@ +// Prior to https://github.com/tc39/ecma262/pull/916 it was possible for a +// module namespace object to be successfully created that was later found to be +// erroneous. Test that this is no longer the case. + +"use strict"; + +load(libdir + "asserts.js"); +load(libdir + "dummyModuleResolveHook.js"); + +moduleRepo['A'] = parseModule('import "B"; export {x} from "C"'); +moduleRepo['B'] = parseModule('import * as a from "A"'); +moduleRepo['C'] = parseModule('export * from "D"; export * from "E"'); +moduleRepo['D'] = parseModule('export let x'); +moduleRepo['E'] = parseModule('export let x'); + +let m = moduleRepo['A']; +assertThrowsInstanceOf(() => m.declarationInstantiation(), SyntaxError); diff --git a/js/src/jit-test/tests/modules/bug-1284486.js b/js/src/jit-test/tests/modules/bug-1284486.js index 9a3244ec31..08286393a3 100644 --- a/js/src/jit-test/tests/modules/bug-1284486.js +++ b/js/src/jit-test/tests/modules/bug-1284486.js @@ -1,23 +1,36 @@ -// |jit-test| error: InternalError - // This tests that attempting to perform ModuleDeclarationInstantation a second -// time after a failure throws an error. Doing this would be a bug in the module -// loader, which is expected to throw away modules if there is an error -// instantiating them. +// time after a failure re-throws the same error. // // The first attempt fails becuase module 'a' is not available. The second // attempt fails because of the previous failure (it would otherwise succeed as // 'a' is now available). -let moduleRepo = {}; -setModuleResolveHook(function(module, specifier) { - return moduleRepo[specifier]; -}); +load(libdir + "dummyModuleResolveHook.js"); + +let b = moduleRepo['b'] = parseModule("export var b = 3; export var c = 4;"); +let c = moduleRepo['c'] = parseModule("export * from 'a'; export * from 'b';"); + +let e1; +let threw = false; try { - let b = moduleRepo['b'] = parseModule("export var b = 3; export var c = 4;"); - let c = moduleRepo['c'] = parseModule("export * from 'a'; export * from 'b';"); c.declarationInstantiation(); -} catch (exc) {} +} catch (exc) { + threw = true; + e1 = exc; +} +assertEq(threw, true); +assertEq(typeof e1 === "undefined", false); + let a = moduleRepo['a'] = parseModule("export var a = 1; export var b = 2;"); let d = moduleRepo['d'] = parseModule("import { a } from 'c'; a;"); -d.declarationInstantiation(); + +threw = false; +let e2; +try { + d.declarationInstantiation(); +} catch (exc) { + threw = true; + e2 = exc; +} +assertEq(threw, true); +assertEq(e1, e2); diff --git a/js/src/jit-test/tests/modules/bug-1287410.js b/js/src/jit-test/tests/modules/bug-1287410.js index 8a891372a5..7df5621a5f 100644 --- a/js/src/jit-test/tests/modules/bug-1287410.js +++ b/js/src/jit-test/tests/modules/bug-1287410.js @@ -20,3 +20,5 @@ let d = moduleRepo['d'] = parseModule("import { a } from 'c'; a;"); // Attempting to instantiate 'd' throws an error because depdency 'a' of // instantiated module 'c' is not instantiated. d.declarationInstantiation(); +d.evaluation(); + diff --git a/js/src/jit-test/tests/modules/bug-1394492.js b/js/src/jit-test/tests/modules/bug-1394492.js new file mode 100644 index 0000000000..a0e5d2ac39 --- /dev/null +++ b/js/src/jit-test/tests/modules/bug-1394492.js @@ -0,0 +1,6 @@ +// |jit-test| error: NaN +let m = parseModule(` + throw i => { return 5; }, m-1; +`); +m.declarationInstantiation(); +m.evaluation(); diff --git a/js/src/jit-test/tests/modules/global-scope.js b/js/src/jit-test/tests/modules/global-scope.js index 90a9f7026b..b99019fa86 100644 --- a/js/src/jit-test/tests/modules/global-scope.js +++ b/js/src/jit-test/tests/modules/global-scope.js @@ -1,32 +1,34 @@ // Test interaction with global object and global lexical scope. -function parseAndEvaluate(source) { +function evalModuleAndCheck(source, expected) { let m = parseModule(source); m.declarationInstantiation(); - return m.evaluation(); + m.evaluation(); + assertEq(getModuleEnvironmentValue(m, "r"), expected); } var x = 1; -assertEq(parseAndEvaluate("let r = x; x = 2; r"), 1); +evalModuleAndCheck("export let r = x; x = 2;", 1); assertEq(x, 2); let y = 3; -assertEq(parseAndEvaluate("let r = y; y = 4; r"), 3); +evalModuleAndCheck("export let r = y; y = 4;", 3); assertEq(y, 4); if (helperThreadCount() == 0) quit(); -function offThreadParseAndEvaluate(source) { +function offThreadEvalModuleAndCheck(source, expected) { offThreadCompileModule(source); let m = finishOffThreadModule(); print("compiled"); m.declarationInstantiation(); - return m.evaluation(); + m.evaluation(); + assertEq(getModuleEnvironmentValue(m, "r"), expected); } -assertEq(offThreadParseAndEvaluate("let r = x; x = 5; r"), 2); +offThreadEvalModuleAndCheck("export let r = x; x = 5;", 2); assertEq(x, 5); -assertEq(offThreadParseAndEvaluate("let r = y; y = 6; r"), 4); +offThreadEvalModuleAndCheck("export let r = y; y = 6;", 4); assertEq(y, 6); diff --git a/js/src/jit-test/tests/modules/module-evaluation.js b/js/src/jit-test/tests/modules/module-evaluation.js index eec13c040e..84d88f19c7 100644 --- a/js/src/jit-test/tests/modules/module-evaluation.js +++ b/js/src/jit-test/tests/modules/module-evaluation.js @@ -6,16 +6,17 @@ load(libdir + "dummyModuleResolveHook.js"); function parseAndEvaluate(source) { let m = parseModule(source); m.declarationInstantiation(); - return m.evaluation(); + m.evaluation(); + return m; } // Check the evaluation of an empty module succeeds. -assertEq(typeof parseAndEvaluate(""), "undefined"); +parseAndEvaluate(""); // Check evaluation returns evaluation result the first time, then undefined. let m = parseModule("1"); m.declarationInstantiation(); -assertEq(m.evaluation(), 1); +assertEq(m.evaluation(), undefined); assertEq(typeof m.evaluation(), "undefined"); // Check top level variables are initialized by evaluation. @@ -60,31 +61,35 @@ parseAndEvaluate("export default class { constructor() {} };"); parseAndEvaluate("export default class foo { constructor() {} };"); // Test default import -m = parseModule("import a from 'a'; a;") +m = parseModule("import a from 'a'; export { a };") m.declarationInstantiation(); -assertEq(m.evaluation(), 2); +m.evaluation(); +assertEq(getModuleEnvironmentValue(m, "a"), 2); // Test named import -m = parseModule("import { x as y } from 'a'; y;") +m = parseModule("import { x as y } from 'a'; export { y };") m.declarationInstantiation(); -assertEq(m.evaluation(), 1); +m.evaluation(); +assertEq(getModuleEnvironmentValue(m, "y"), 1); // Call exported function -m = parseModule("import { f } from 'a'; f(3);") +m = parseModule("import { f } from 'a'; export let x = f(3);") m.declarationInstantiation(); -assertEq(m.evaluation(), 4); +m.evaluation(); +assertEq(getModuleEnvironmentValue(m, "x"), 4); // Test importing an indirect export moduleRepo['b'] = parseModule("export { x as z } from 'a';"); -assertEq(parseAndEvaluate("import { z } from 'b'; z"), 1); +m = parseAndEvaluate("import { z } from 'b'; export { z }"); +assertEq(getModuleEnvironmentValue(m, "z"), 1); // Test cyclic dependencies moduleRepo['c1'] = parseModule("export var x = 1; export {y} from 'c2'"); moduleRepo['c2'] = parseModule("export var y = 2; export {x} from 'c1'"); -assertDeepEq(parseAndEvaluate(`import { x as x1, y as y1 } from 'c1'; - import { x as x2, y as y2 } from 'c2'; - [x1, y1, x2, y2]`), - [1, 2, 1, 2]); +m = parseAndEvaluate(`import { x as x1, y as y1 } from 'c1'; + import { x as x2, y as y2 } from 'c2'; + export let z = [x1, y1, x2, y2]`), +assertDeepEq(getModuleEnvironmentValue(m, "z"), [1, 2, 1, 2]); // Import access in functions m = parseModule("import { x } from 'a'; function f() { return x; }") diff --git a/js/src/js.msg b/js/src/js.msg index 9dc5f4e9f9..ee74c8dd55 100644 --- a/js/src/js.msg +++ b/js/src/js.msg @@ -580,7 +580,7 @@ MSG_DEF(JSMSG_AMBIGUOUS_IMPORT, 1, JSEXN_SYNTAXERR, "ambiguous import ' MSG_DEF(JSMSG_MISSING_NAMESPACE_EXPORT, 0, JSEXN_SYNTAXERR, "export not found for namespace") MSG_DEF(JSMSG_MISSING_EXPORT, 1, JSEXN_SYNTAXERR, "local binding for export '{0}' not found") MSG_DEF(JSMSG_MODULE_INSTANTIATE_FAILED, 0, JSEXN_INTERNALERR, "attempt to re-instantiate module after failure") -MSG_DEF(JSMSG_BAD_MODULE_STATE, 0, JSEXN_INTERNALERR, "module record in unexpected state") +MSG_DEF(JSMSG_BAD_MODULE_STATUS, 0, JSEXN_INTERNALERR, "module record has unexpected status") // Promise MSG_DEF(JSMSG_CANNOT_RESOLVE_PROMISE_WITH_ITSELF, 0, JSEXN_TYPEERR, "A promise cannot be resolved with itself.") diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index d75a3c33a0..53ea4ebc60 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -4709,7 +4709,7 @@ JS::ModuleDeclarationInstantiation(JSContext* cx, JS::HandleObject moduleArg) AssertHeapIsIdle(cx); CHECK_REQUEST(cx); assertSameCompartment(cx, moduleArg); - return ModuleObject::DeclarationInstantiation(cx, moduleArg.as()); + return ModuleObject::Instantiate(cx, moduleArg.as()); } JS_PUBLIC_API(bool) @@ -4718,7 +4718,7 @@ JS::ModuleEvaluation(JSContext* cx, JS::HandleObject moduleArg) AssertHeapIsIdle(cx); CHECK_REQUEST(cx); assertSameCompartment(cx, moduleArg); - return ModuleObject::Evaluation(cx, moduleArg.as()); + return ModuleObject::Evaluate(cx, moduleArg.as()); } JS_PUBLIC_API(JSObject*) diff --git a/js/src/vm/CommonPropertyNames.h b/js/src/vm/CommonPropertyNames.h index b4a2de6f30..aa555886e6 100644 --- a/js/src/vm/CommonPropertyNames.h +++ b/js/src/vm/CommonPropertyNames.h @@ -227,8 +227,8 @@ macro(missingArguments, missingArguments, "missingArguments") \ macro(module, module, "module") \ macro(Module, Module, "Module") \ - macro(ModuleDeclarationInstantiation, ModuleDeclarationInstantiation, "ModuleDeclarationInstantiation") \ - macro(ModuleEvaluation, ModuleEvaluation, "ModuleEvaluation") \ + macro(ModuleInstantiate, ModuleInstantiate, "ModuleInstantiate") \ + macro(ModuleEvaluate, ModuleEvaluate, "ModuleEvaluate") \ macro(month, month, "month") \ macro(multiline, multiline, "multiline") \ macro(name, name, "name") \ diff --git a/js/src/vm/SelfHosting.cpp b/js/src/vm/SelfHosting.cpp index 2216bf91e9..89750d61a6 100644 --- a/js/src/vm/SelfHosting.cpp +++ b/js/src/vm/SelfHosting.cpp @@ -2060,24 +2060,12 @@ intrinsic_InstantiateModuleFunctionDeclarations(JSContext* cx, unsigned argc, Va } static bool -intrinsic_SetModuleState(JSContext* cx, unsigned argc, Value* vp) -{ - CallArgs args = CallArgsFromVp(argc, vp); - MOZ_ASSERT(args.length() == 2); - RootedModuleObject module(cx, &args[0].toObject().as()); - ModuleState newState = args[1].toInt32(); - module->setState(newState); - args.rval().setUndefined(); - return true; -} - -static bool -intrinsic_EvaluateModule(JSContext* cx, unsigned argc, Value* vp) +intrinsic_ExecuteModule(JSContext* cx, unsigned argc, Value* vp) { CallArgs args = CallArgsFromVp(argc, vp); MOZ_ASSERT(args.length() == 1); RootedModuleObject module(cx, &args[0].toObject().as()); - return ModuleObject::evaluate(cx, module, args.rval()); + return ModuleObject::execute(cx, module, args.rval()); } static bool @@ -2630,8 +2618,7 @@ static const JSFunctionSpec intrinsic_functions[] = { JS_FN("CreateNamespaceBinding", intrinsic_CreateNamespaceBinding, 3, 0), JS_FN("InstantiateModuleFunctionDeclarations", intrinsic_InstantiateModuleFunctionDeclarations, 1, 0), - JS_FN("SetModuleState", intrinsic_SetModuleState, 1, 0), - JS_FN("EvaluateModule", intrinsic_EvaluateModule, 1, 0), + JS_FN("ExecuteModule", intrinsic_ExecuteModule, 1, 0), JS_FN("NewModuleNamespace", intrinsic_NewModuleNamespace, 2, 0), JS_FN("AddModuleNamespaceBinding", intrinsic_AddModuleNamespaceBinding, 4, 0), JS_FN("ModuleNamespaceExports", intrinsic_ModuleNamespaceExports, 1, 0), From 7b30bfee4cff81a01ad82345f0ca1a40e81f7c97 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Fri, 3 Jul 2020 14:05:40 +0000 Subject: [PATCH 04/20] Issue #618 - Update code comments for ModuleInstantiate --- js/src/frontend/BytecodeEmitter.cpp | 2 +- js/src/frontend/Parser.cpp | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/js/src/frontend/BytecodeEmitter.cpp b/js/src/frontend/BytecodeEmitter.cpp index 18cc7d954e..654336a643 100644 --- a/js/src/frontend/BytecodeEmitter.cpp +++ b/js/src/frontend/BytecodeEmitter.cpp @@ -8261,7 +8261,7 @@ BytecodeEmitter::emitFunction(ParseNode* pn, bool needsProto) if (topLevelFunction) { if (sc->isModuleContext()) { // For modules, we record the function and instantiate the binding - // during ModuleDeclarationInstantiation(), before the script is run. + // during ModuleInstantiate(), before the script is run. RootedModuleObject module(cx, sc->asModuleContext()->module()); if (!module->noteFunctionDeclaration(cx, name, fun)) diff --git a/js/src/frontend/Parser.cpp b/js/src/frontend/Parser.cpp index 810d589be9..59783a759f 100644 --- a/js/src/frontend/Parser.cpp +++ b/js/src/frontend/Parser.cpp @@ -5038,7 +5038,7 @@ Parser::namedImportsOrNamespaceImport(TokenKind tt, Node impor // Namespace imports are are not indirect bindings but lexical // definitions that hold a module namespace object. They are treated // as const variables which are initialized during the - // ModuleDeclarationInstantiation step. + // ModuleInstantiate step. RootedPropertyName bindingName(context, importedBinding()); if (!bindingName) return false; From 5838eded5f4eac9fcba6277c0a646c64dcb0cb9b Mon Sep 17 00:00:00 2001 From: Moonchild Date: Fri, 3 Jul 2020 14:15:55 +0000 Subject: [PATCH 05/20] Issue #618 - Add APIs to query module record errors Ref: BZ 1388728 --- js/src/jsapi.cpp | 14 ++++++++++++++ js/src/jsapi.h | 6 ++++++ 2 files changed, 20 insertions(+) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 53ea4ebc60..f8c34885bb 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -4739,6 +4739,20 @@ JS::GetModuleScript(JSContext* cx, JS::HandleObject moduleArg) return moduleArg->as().script(); } +JS_PUBLIC_API(bool) +JS::IsModuleErrored(JSObject* moduleArg) +{ + AssertHeapIsIdle(); + return moduleArg->as().status() == MODULE_STATUS_ERRORED; +} + +JS_PUBLIC_API(JS::Value) +JS::GetModuleError(JSObject* moduleArg) +{ + AssertHeapIsIdle(); + return moduleArg->as().error(); +} + JS_PUBLIC_API(JSObject*) JS_New(JSContext* cx, HandleObject ctor, const JS::HandleValueArray& inputArgs) { diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 9138a4a92f..5e02595874 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -4396,6 +4396,12 @@ GetRequestedModules(JSContext* cx, JS::HandleObject moduleRecord); extern JS_PUBLIC_API(JSScript*) GetModuleScript(JSContext* cx, JS::HandleObject moduleRecord); +extern JS_PUBLIC_API(bool) +IsModuleErrored(JSObject* moduleRecord); + +extern JS_PUBLIC_API(JS::Value) +GetModuleError(JSObject* moduleRecord); + } /* namespace JS */ extern JS_PUBLIC_API(bool) From 91a79c0ddc1ca6382e66a2feed15baac538ebc41 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Fri, 3 Jul 2020 14:21:27 +0000 Subject: [PATCH 06/20] Issue #618 - Match JSAPI names with the changes in 9ca74147225eed305e28c7887f9b2251aeeb0f36 Ref: BZ 1388728 --- dom/base/nsJSUtils.cpp | 12 ++++++------ dom/base/nsJSUtils.h | 8 ++++---- dom/script/ScriptLoader.cpp | 4 ++-- js/src/jsapi.cpp | 4 ++-- js/src/jsapi.h | 13 ++++++------- 5 files changed, 20 insertions(+), 21 deletions(-) diff --git a/dom/base/nsJSUtils.cpp b/dom/base/nsJSUtils.cpp index c24adc7390..c294cfdbad 100644 --- a/dom/base/nsJSUtils.cpp +++ b/dom/base/nsJSUtils.cpp @@ -308,9 +308,9 @@ nsJSUtils::CompileModule(JSContext* aCx, } nsresult -nsJSUtils::ModuleDeclarationInstantiation(JSContext* aCx, JS::Handle aModule) +nsJSUtils::ModuleInstantiate(JSContext* aCx, JS::Handle aModule) { - PROFILER_LABEL("nsJSUtils", "ModuleDeclarationInstantiation", + PROFILER_LABEL("nsJSUtils", "ModuleInstantiate", js::ProfileEntry::Category::JS); MOZ_ASSERT(aCx == nsContentUtils::GetCurrentJSContext()); @@ -318,7 +318,7 @@ nsJSUtils::ModuleDeclarationInstantiation(JSContext* aCx, JS::Handle NS_ENSURE_TRUE(xpc::Scriptability::Get(aModule).Allowed(), NS_OK); - if (!JS::ModuleDeclarationInstantiation(aCx, aModule)) { + if (!JS::ModuleInstantiate(aCx, aModule)) { return NS_ERROR_FAILURE; } @@ -326,9 +326,9 @@ nsJSUtils::ModuleDeclarationInstantiation(JSContext* aCx, JS::Handle } nsresult -nsJSUtils::ModuleEvaluation(JSContext* aCx, JS::Handle aModule) +nsJSUtils::ModuleEvaluate(JSContext* aCx, JS::Handle aModule) { - PROFILER_LABEL("nsJSUtils", "ModuleEvaluation", + PROFILER_LABEL("nsJSUtils", "ModuleEvaluate", js::ProfileEntry::Category::JS); MOZ_ASSERT(aCx == nsContentUtils::GetCurrentJSContext()); @@ -338,7 +338,7 @@ nsJSUtils::ModuleEvaluation(JSContext* aCx, JS::Handle aModule) NS_ENSURE_TRUE(xpc::Scriptability::Get(aModule).Allowed(), NS_OK); - if (!JS::ModuleEvaluation(aCx, aModule)) { + if (!JS::ModuleEvaluate(aCx, aModule)) { return NS_ERROR_FAILURE; } diff --git a/dom/base/nsJSUtils.h b/dom/base/nsJSUtils.h index 4affab2d36..9eea6ae83d 100644 --- a/dom/base/nsJSUtils.h +++ b/dom/base/nsJSUtils.h @@ -116,11 +116,11 @@ public: JS::CompileOptions &aCompileOptions, JS::MutableHandle aModule); - static nsresult ModuleDeclarationInstantiation(JSContext* aCx, - JS::Handle aModule); + static nsresult ModuleInstantiate(JSContext* aCx, + JS::Handle aModule); - static nsresult ModuleEvaluation(JSContext* aCx, - JS::Handle aModule); + static nsresult ModuleEvaluate(JSContext* aCx, + JS::Handle aModule); // Returns false if an exception got thrown on aCx. Passing a null // aElement is allowed; that wil produce an empty aScopeChain. diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index 7aca04da5c..736bb43499 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -888,7 +888,7 @@ ScriptLoader::InstantiateModuleTree(ModuleLoadRequest* aRequest) NS_ENSURE_SUCCESS(rv, false); JS::Rooted module(jsapi.cx(), ms->ModuleRecord()); - bool ok = NS_SUCCEEDED(nsJSUtils::ModuleDeclarationInstantiation(jsapi.cx(), module)); + bool ok = NS_SUCCEEDED(nsJSUtils::ModuleInstantiate(jsapi.cx(), module)); JS::RootedValue exception(jsapi.cx()); if (!ok) { @@ -1953,7 +1953,7 @@ ScriptLoader::EvaluateScript(ScriptLoadRequest* aRequest) } else { JS::Rooted module(aes.cx(), ms->ModuleRecord()); MOZ_ASSERT(module); - rv = nsJSUtils::ModuleEvaluation(aes.cx(), module); + rv = nsJSUtils::ModuleEvaluate(aes.cx(), module); } } else { JS::CompileOptions options(aes.cx()); diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index f8c34885bb..9c24f1676c 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -4704,7 +4704,7 @@ JS::GetModuleHostDefinedField(JSObject* module) } JS_PUBLIC_API(bool) -JS::ModuleDeclarationInstantiation(JSContext* cx, JS::HandleObject moduleArg) +JS::ModuleInstantiate(JSContext* cx, JS::HandleObject moduleArg) { AssertHeapIsIdle(cx); CHECK_REQUEST(cx); @@ -4713,7 +4713,7 @@ JS::ModuleDeclarationInstantiation(JSContext* cx, JS::HandleObject moduleArg) } JS_PUBLIC_API(bool) -JS::ModuleEvaluation(JSContext* cx, JS::HandleObject moduleArg) +JS::ModuleEvaluate(JSContext* cx, JS::HandleObject moduleArg) { AssertHeapIsIdle(cx); CHECK_REQUEST(cx); diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 5e02595874..9c3bf81518 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -4356,28 +4356,27 @@ extern JS_PUBLIC_API(JS::Value) GetModuleHostDefinedField(JSObject* module); /* - * Perform the ModuleDeclarationInstantiation operation on on the give source - * text module record. + * Perform the ModuleInstantiate operation on the given source text module + * record. * * This transitively resolves all module dependencies (calling the * HostResolveImportedModule hook) and initializes the environment record for * the module. */ extern JS_PUBLIC_API(bool) -ModuleDeclarationInstantiation(JSContext* cx, JS::HandleObject moduleRecord); +ModuleInstantiate(JSContext* cx, JS::HandleObject moduleRecord); /* - * Perform the ModuleEvaluation operation on on the give source text module - * record. + * Perform the ModuleEvaluate operation on the given source text module record. * * This does nothing if this module has already been evaluated. Otherwise, it * transitively evaluates all dependences of this module and then evaluates this * module. * - * ModuleDeclarationInstantiation must have completed prior to calling this. + * ModuleInstantiate must have completed prior to calling this. */ extern JS_PUBLIC_API(bool) -ModuleEvaluation(JSContext* cx, JS::HandleObject moduleRecord); +ModuleEvaluate(JSContext* cx, JS::HandleObject moduleRecord); /* * Get a list of the module specifiers used by a source text module From b590ecea12df0ea59d4c427847c1253909edca48 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Fri, 3 Jul 2020 15:59:00 +0000 Subject: [PATCH 07/20] Issue #618 - Fix JSAPI additions to pass the JS context. --- js/src/jsapi.cpp | 10 ++++++---- js/src/jsapi.h | 4 ++-- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index 9c24f1676c..cb0851f80b 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -4740,16 +4740,18 @@ JS::GetModuleScript(JSContext* cx, JS::HandleObject moduleArg) } JS_PUBLIC_API(bool) -JS::IsModuleErrored(JSObject* moduleArg) +JS::IsModuleErrored(JSContext* cx, JSObject* moduleArg) { - AssertHeapIsIdle(); + AssertHeapIsIdle(cx); + CHECK_REQUEST(cx); return moduleArg->as().status() == MODULE_STATUS_ERRORED; } JS_PUBLIC_API(JS::Value) -JS::GetModuleError(JSObject* moduleArg) +JS::GetModuleError(JSContext* cx, JSObject* moduleArg) { - AssertHeapIsIdle(); + AssertHeapIsIdle(cx); + CHECK_REQUEST(cx); return moduleArg->as().error(); } diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 9c3bf81518..6d306edc56 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -4396,10 +4396,10 @@ extern JS_PUBLIC_API(JSScript*) GetModuleScript(JSContext* cx, JS::HandleObject moduleRecord); extern JS_PUBLIC_API(bool) -IsModuleErrored(JSObject* moduleRecord); +IsModuleErrored(JSContext* cx, JSObject* moduleRecord); extern JS_PUBLIC_API(JS::Value) -GetModuleError(JSObject* moduleRecord); +GetModuleError(JSContext* cx, JSObject* moduleRecord); } /* namespace JS */ From a754a7be266a1c92f8ce9bb94c83578dda3c1d2c Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sat, 4 Jul 2020 10:35:22 +0000 Subject: [PATCH 08/20] Issue #618 - Remove eager instantiation This backs out the stuff added in Bug 1295978. Ref: BZ 1295978, 1388728 --- dom/base/nsJSUtils.cpp | 1 + dom/script/ModuleLoadRequest.cpp | 5 -- dom/script/ModuleScript.cpp | 22 +------- dom/script/ModuleScript.h | 20 ------- dom/script/ScriptLoader.cpp | 90 +++----------------------------- dom/script/ScriptLoader.h | 1 - 6 files changed, 10 insertions(+), 129 deletions(-) diff --git a/dom/base/nsJSUtils.cpp b/dom/base/nsJSUtils.cpp index c294cfdbad..65965d74bf 100644 --- a/dom/base/nsJSUtils.cpp +++ b/dom/base/nsJSUtils.cpp @@ -315,6 +315,7 @@ nsJSUtils::ModuleInstantiate(JSContext* aCx, JS::Handle aModule) MOZ_ASSERT(aCx == nsContentUtils::GetCurrentJSContext()); MOZ_ASSERT(NS_IsMainThread()); + MOZ_ASSERT(nsContentUtils::IsInMicroTask()); NS_ENSURE_TRUE(xpc::Scriptability::Get(aModule).Allowed(), NS_OK); diff --git a/dom/script/ModuleLoadRequest.cpp b/dom/script/ModuleLoadRequest.cpp index 98106df1b9..8871dcdab2 100644 --- a/dom/script/ModuleLoadRequest.cpp +++ b/dom/script/ModuleLoadRequest.cpp @@ -86,11 +86,6 @@ ModuleLoadRequest::DependenciesLoaded() // The module and all of its dependencies have been successfully fetched and // compiled. - if (!mLoader->InstantiateModuleTree(this)) { - LoadFailed(); - return; - } - SetReady(); mLoader->ProcessLoadedModuleTree(this); mLoader = nullptr; diff --git a/dom/script/ModuleScript.cpp b/dom/script/ModuleScript.cpp index 34ef4dec46..bf02f522de 100644 --- a/dom/script/ModuleScript.cpp +++ b/dom/script/ModuleScript.cpp @@ -34,7 +34,6 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(ModuleScript) NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mModuleRecord) - NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mException) NS_IMPL_CYCLE_COLLECTION_TRACE_END NS_IMPL_CYCLE_COLLECTING_ADDREF(ModuleScript) @@ -44,13 +43,11 @@ ModuleScript::ModuleScript(ScriptLoader *aLoader, nsIURI* aBaseURL, JS::Handle aModuleRecord) : mLoader(aLoader), mBaseURL(aBaseURL), - mModuleRecord(aModuleRecord), - mInstantiationState(Uninstantiated) + mModuleRecord(aModuleRecord) { MOZ_ASSERT(mLoader); MOZ_ASSERT(mBaseURL); MOZ_ASSERT(mModuleRecord); - MOZ_ASSERT(mException.isUndefined()); // Make module's host defined field point to this module script object. // This is cleared in the UnlinkModuleRecord(). @@ -68,7 +65,6 @@ ModuleScript::UnlinkModuleRecord() JS::SetModuleHostDefinedField(mModuleRecord, JS::UndefinedValue()); } mModuleRecord = nullptr; - mException.setUndefined(); } ModuleScript::~ModuleScript() @@ -80,21 +76,5 @@ ModuleScript::~ModuleScript() DropJSObjects(this); } -void -ModuleScript::SetInstantiationResult(JS::Handle aMaybeException) -{ - MOZ_ASSERT(mInstantiationState == Uninstantiated); - MOZ_ASSERT(mModuleRecord); - MOZ_ASSERT(mException.isUndefined()); - - if (aMaybeException.isUndefined()) { - mInstantiationState = Instantiated; - } else { - mModuleRecord = nullptr; - mException = aMaybeException; - mInstantiationState = Errored; - } -} - } // dom namespace } // mozilla namespace diff --git a/dom/script/ModuleScript.h b/dom/script/ModuleScript.h index dd0d07e847..97fdb8ed19 100644 --- a/dom/script/ModuleScript.h +++ b/dom/script/ModuleScript.h @@ -20,17 +20,9 @@ class ScriptLoader; class ModuleScript final : public nsISupports { - enum InstantiationState { - Uninstantiated, - Instantiated, - Errored - }; - RefPtr mLoader; nsCOMPtr mBaseURL; JS::Heap mModuleRecord; - JS::Heap mException; - InstantiationState mInstantiationState; ~ModuleScript(); @@ -44,20 +36,8 @@ public: ScriptLoader* Loader() const { return mLoader; } JSObject* ModuleRecord() const { return mModuleRecord; } - JS::Value Exception() const { return mException; } nsIURI* BaseURL() const { return mBaseURL; } - void SetInstantiationResult(JS::Handle aMaybeException); - bool IsUninstantiated() const { - return mInstantiationState == Uninstantiated; - } - bool IsInstantiated() const { - return mInstantiationState == Instantiated; - } - bool InstantiationFailed() const { - return mInstantiationState == Errored; - } - void UnlinkModuleRecord(); }; diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index 736bb43499..cf7f06e713 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -726,11 +726,6 @@ ScriptLoader::StartFetchingModuleDependencies(ModuleLoadRequest* aRequest) MOZ_ASSERT(aRequest->mModuleScript); MOZ_ASSERT(!aRequest->IsReadyToRun()); - if (aRequest->mModuleScript->InstantiationFailed()) { - aRequest->LoadFailed(); - return; - } - aRequest->mProgress = ModuleLoadRequest::Progress::FetchingImports; nsCOMArray urls; @@ -825,12 +820,6 @@ HostResolveImportedModule(JSContext* aCx, unsigned argc, JS::Value* vp) return HandleModuleNotFound(aCx, script, string); } - if (ms->InstantiationFailed()) { - JS::Rooted exception(aCx, ms->Exception()); - JS_SetPendingException(aCx, exception); - return false; - } - *vp = JS::ObjectValue(*ms->ModuleRecord()); return true; } @@ -866,68 +855,6 @@ ScriptLoader::ProcessLoadedModuleTree(ModuleLoadRequest* aRequest) } } -bool -ScriptLoader::InstantiateModuleTree(ModuleLoadRequest* aRequest) -{ - // Perform eager instantiation of the loaded module tree. - - MOZ_ASSERT(aRequest); - - ModuleScript* ms = aRequest->mModuleScript; - MOZ_ASSERT(ms); - if (!ms || !ms->ModuleRecord()) { - return false; - } - - AutoJSAPI jsapi; - if (NS_WARN_IF(!jsapi.Init(ms->ModuleRecord()))) { - return false; - } - - nsresult rv = EnsureModuleResolveHook(jsapi.cx()); - NS_ENSURE_SUCCESS(rv, false); - - JS::Rooted module(jsapi.cx(), ms->ModuleRecord()); - bool ok = NS_SUCCEEDED(nsJSUtils::ModuleInstantiate(jsapi.cx(), module)); - - JS::RootedValue exception(jsapi.cx()); - if (!ok) { - MOZ_ASSERT(jsapi.HasException()); - if (!jsapi.StealException(&exception)) { - return false; - } - MOZ_ASSERT(!exception.isUndefined()); - } - - // Mark this module and any uninstantiated dependencies found via depth-first - // search as instantiated and record any error. - - mozilla::Vector requests; - if (!requests.append(aRequest)) { - return false; - } - - while (!requests.empty()) { - ModuleLoadRequest* request = requests.popCopy(); - ModuleScript* ms = request->mModuleScript; - if (!ms->IsUninstantiated()) { - continue; - } - - ms->SetInstantiationResult(exception); - - for (auto import : request->mImports) { - if (import->mModuleScript->IsUninstantiated() && - !requests.append(import)) - { - return false; - } - } - } - - return true; -} - nsresult ScriptLoader::StartLoad(ScriptLoadRequest *aRequest, const nsAString &aType, bool aScriptFromHead) @@ -1941,18 +1868,17 @@ ScriptLoader::EvaluateScript(ScriptLoadRequest* aRequest) } if (aRequest->IsModuleRequest()) { + rv = EnsureModuleResolveHook(aes.cx()); + NS_ENSURE_SUCCESS(rv, rv); + ModuleLoadRequest* request = aRequest->AsModuleRequest(); MOZ_ASSERT(request->mModuleScript); MOZ_ASSERT(!request->mOffThreadToken); - ModuleScript* ms = request->mModuleScript; - MOZ_ASSERT(!ms->IsUninstantiated()); - if (ms->InstantiationFailed()) { - JS::Rooted exception(aes.cx(), ms->Exception()); - JS_SetPendingException(aes.cx(), exception); - rv = NS_ERROR_FAILURE; - } else { - JS::Rooted module(aes.cx(), ms->ModuleRecord()); - MOZ_ASSERT(module); + JS::Rooted module(aes.cx(), + request->mModuleScript->ModuleRecord()); + MOZ_ASSERT(module); + rv = nsJSUtils::ModuleInstantiate(aes.cx(), module); + if (NS_SUCCEEDED(rv)) { rv = nsJSUtils::ModuleEvaluate(aes.cx(), module); } } else { diff --git a/dom/script/ScriptLoader.h b/dom/script/ScriptLoader.h index 3c428deea6..32401d1b6b 100644 --- a/dom/script/ScriptLoader.h +++ b/dom/script/ScriptLoader.h @@ -581,7 +581,6 @@ private: nsresult CreateModuleScript(ModuleLoadRequest* aRequest); nsresult ProcessFetchedModuleSource(ModuleLoadRequest* aRequest); void ProcessLoadedModuleTree(ModuleLoadRequest* aRequest); - bool InstantiateModuleTree(ModuleLoadRequest* aRequest); void StartFetchingModuleDependencies(ModuleLoadRequest* aRequest); RefPtr From c65ca2ca5861ac38fa4d384c1098743ce10343c2 Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sat, 4 Jul 2020 12:59:38 +0000 Subject: [PATCH 09/20] Issue #618 - Remove context and heap-idle check For checking if a module is in an error state and what the error is, it shouldn't matter if we are currently GC-ing or not. So we don't need to check for it, which removes the requirement to pass in the JS context (needed for AssertHeapIsIdle's runtime check); this unblocks progress where otherwise we'd have to figure out what the context is at the module level just to satisfy this check. --- js/src/jsapi.cpp | 8 ++------ js/src/jsapi.h | 4 ++-- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/js/src/jsapi.cpp b/js/src/jsapi.cpp index cb0851f80b..cf5880e038 100644 --- a/js/src/jsapi.cpp +++ b/js/src/jsapi.cpp @@ -4740,18 +4740,14 @@ JS::GetModuleScript(JSContext* cx, JS::HandleObject moduleArg) } JS_PUBLIC_API(bool) -JS::IsModuleErrored(JSContext* cx, JSObject* moduleArg) +JS::IsModuleErrored(JSObject* moduleArg) { - AssertHeapIsIdle(cx); - CHECK_REQUEST(cx); return moduleArg->as().status() == MODULE_STATUS_ERRORED; } JS_PUBLIC_API(JS::Value) -JS::GetModuleError(JSContext* cx, JSObject* moduleArg) +JS::GetModuleError(JSObject* moduleArg) { - AssertHeapIsIdle(cx); - CHECK_REQUEST(cx); return moduleArg->as().error(); } diff --git a/js/src/jsapi.h b/js/src/jsapi.h index 6d306edc56..9c3bf81518 100644 --- a/js/src/jsapi.h +++ b/js/src/jsapi.h @@ -4396,10 +4396,10 @@ extern JS_PUBLIC_API(JSScript*) GetModuleScript(JSContext* cx, JS::HandleObject moduleRecord); extern JS_PUBLIC_API(bool) -IsModuleErrored(JSContext* cx, JSObject* moduleRecord); +IsModuleErrored(JSObject* moduleRecord); extern JS_PUBLIC_API(JS::Value) -GetModuleError(JSContext* cx, JSObject* moduleRecord); +GetModuleError(JSObject* moduleRecord); } /* namespace JS */ From 743bdc04fbc441ab5fcf0c37a361dd15f8c7568d Mon Sep 17 00:00:00 2001 From: Moonchild Date: Sat, 4 Jul 2020 16:28:30 +0000 Subject: [PATCH 10/20] Issue #618 - Further align error handling for module scripts with the spec Ref: BZ 1388728 --- dom/script/ModuleLoadRequest.cpp | 42 +++++- dom/script/ModuleLoadRequest.h | 6 + dom/script/ModuleScript.cpp | 72 ++++++++-- dom/script/ModuleScript.h | 10 +- dom/script/ScriptLoader.cpp | 237 ++++++++++++++++++++++--------- dom/script/ScriptLoader.h | 2 + 6 files changed, 280 insertions(+), 89 deletions(-) diff --git a/dom/script/ModuleLoadRequest.cpp b/dom/script/ModuleLoadRequest.cpp index 8871dcdab2..d622143044 100644 --- a/dom/script/ModuleLoadRequest.cpp +++ b/dom/script/ModuleLoadRequest.cpp @@ -43,10 +43,16 @@ void ModuleLoadRequest::Cancel() ScriptLoadRequest::Cancel(); mModuleScript = nullptr; mProgress = ScriptLoadRequest::Progress::Ready; + CancelImports(); + mReady.RejectIfExists(NS_ERROR_DOM_ABORT_ERR, __func__); +} + +void +ModuleLoadRequest::CancelImports() +{ for (size_t i = 0; i < mImports.Length(); i++) { mImports[i]->Cancel(); } - mReady.RejectIfExists(NS_ERROR_FAILURE, __func__); } void @@ -77,25 +83,53 @@ ModuleLoadRequest::ModuleLoaded() // been loaded. mModuleScript = mLoader->GetFetchedModule(mURI); + if (!mModuleScript || mModuleScript->IsErrored()) { + ModuleErrored(); + return; + } + mLoader->StartFetchingModuleDependencies(this); } +void +ModuleLoadRequest::ModuleErrored() +{ + mLoader->CheckModuleDependenciesLoaded(this); + MOZ_ASSERT(!mModuleScript || mModuleScript->IsErrored()); + + CancelImports(); + SetReady(); + LoadFinished(); +} + void ModuleLoadRequest::DependenciesLoaded() { // The module and all of its dependencies have been successfully fetched and // compiled. + MOZ_ASSERT(mModuleScript); + + mLoader->CheckModuleDependenciesLoaded(this); SetReady(); - mLoader->ProcessLoadedModuleTree(this); - mLoader = nullptr; - mParent = nullptr; + LoadFinished(); } void ModuleLoadRequest::LoadFailed() { + // We failed to load the source text or an error occurred unrelated to the + // content of the module (e.g. OOM). + + MOZ_ASSERT(!mModuleScript); + Cancel(); + LoadFinished(); +} + +void +ModuleLoadRequest::LoadFinished() +{ mLoader->ProcessLoadedModuleTree(this); mLoader = nullptr; mParent = nullptr; diff --git a/dom/script/ModuleLoadRequest.h b/dom/script/ModuleLoadRequest.h index 0119fad385..7b06dd2cf3 100644 --- a/dom/script/ModuleLoadRequest.h +++ b/dom/script/ModuleLoadRequest.h @@ -45,9 +45,15 @@ public: void Cancel() override; void ModuleLoaded(); + void ModuleErrored(); void DependenciesLoaded(); void LoadFailed(); +private: + void LoadFinished(); + void CancelImports(); + +public: // Is this a request for a top level module script or an import? bool mIsTopLevel; diff --git a/dom/script/ModuleScript.cpp b/dom/script/ModuleScript.cpp index bf02f522de..28b97a3cb5 100644 --- a/dom/script/ModuleScript.cpp +++ b/dom/script/ModuleScript.cpp @@ -26,6 +26,7 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ModuleScript) NS_IMPL_CYCLE_COLLECTION_UNLINK(mLoader) NS_IMPL_CYCLE_COLLECTION_UNLINK(mBaseURL) tmp->UnlinkModuleRecord(); + tmp->mError.setUndefined(); NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(ModuleScript) @@ -34,25 +35,20 @@ NS_IMPL_CYCLE_COLLECTION_TRAVERSE_END NS_IMPL_CYCLE_COLLECTION_TRACE_BEGIN(ModuleScript) NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mModuleRecord) + NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mError) NS_IMPL_CYCLE_COLLECTION_TRACE_END NS_IMPL_CYCLE_COLLECTING_ADDREF(ModuleScript) NS_IMPL_CYCLE_COLLECTING_RELEASE(ModuleScript) -ModuleScript::ModuleScript(ScriptLoader *aLoader, nsIURI* aBaseURL, - JS::Handle aModuleRecord) +ModuleScript::ModuleScript(ScriptLoader *aLoader, nsIURI* aBaseURL) : mLoader(aLoader), - mBaseURL(aBaseURL), - mModuleRecord(aModuleRecord) + mBaseURL(aBaseURL) { MOZ_ASSERT(mLoader); MOZ_ASSERT(mBaseURL); - MOZ_ASSERT(mModuleRecord); - - // Make module's host defined field point to this module script object. - // This is cleared in the UnlinkModuleRecord(). - JS::SetModuleHostDefinedField(mModuleRecord, JS::PrivateValue(this)); - HoldJSObjects(this); + MOZ_ASSERT(!mModuleRecord); + MOZ_ASSERT(mError.isUndefined()); } void @@ -63,18 +59,64 @@ ModuleScript::UnlinkModuleRecord() MOZ_ASSERT(JS::GetModuleHostDefinedField(mModuleRecord).toPrivate() == this); JS::SetModuleHostDefinedField(mModuleRecord, JS::UndefinedValue()); + mModuleRecord = nullptr; } - mModuleRecord = nullptr; } ModuleScript::~ModuleScript() { - if (mModuleRecord) { - // The object may be destroyed without being unlinked first. - UnlinkModuleRecord(); - } + // The object may be destroyed without being unlinked first. + UnlinkModuleRecord(); DropJSObjects(this); } +void +ModuleScript::SetModuleRecord(JS::Handle aModuleRecord) +{ + MOZ_ASSERT(!mModuleRecord); + MOZ_ASSERT(mError.isUndefined()); + + mModuleRecord = aModuleRecord; + + // Make module's host defined field point to this module script object. + // This is cleared in the UnlinkModuleRecord(). + JS::SetModuleHostDefinedField(mModuleRecord, JS::PrivateValue(this)); + HoldJSObjects(this); +} + +void +ModuleScript::SetPreInstantiationError(const JS::Value& aError) +{ + MOZ_ASSERT(!aError.isUndefined()); + + UnlinkModuleRecord(); + mError = aError; + + HoldJSObjects(this); +} + +bool +ModuleScript::IsErrored() const +{ + if (!mModuleRecord) { + MOZ_ASSERT(!mError.isUndefined()); + return true; + } + + return JS::IsModuleErrored(mModuleRecord); +} + +JS::Value +ModuleScript::Error() const +{ + MOZ_ASSERT(IsErrored()); + + if (!mModuleRecord) { + return mError; + } + + return JS::GetModuleError(mModuleRecord); +} + } // dom namespace } // mozilla namespace diff --git a/dom/script/ModuleScript.h b/dom/script/ModuleScript.h index 97fdb8ed19..5713598592 100644 --- a/dom/script/ModuleScript.h +++ b/dom/script/ModuleScript.h @@ -23,6 +23,7 @@ class ModuleScript final : public nsISupports RefPtr mLoader; nsCOMPtr mBaseURL; JS::Heap mModuleRecord; + JS::Heap mError; ~ModuleScript(); @@ -31,13 +32,18 @@ public: NS_DECL_CYCLE_COLLECTION_SCRIPT_HOLDER_CLASS(ModuleScript) ModuleScript(ScriptLoader* aLoader, - nsIURI* aBaseURL, - JS::Handle aModuleRecord); + nsIURI* aBaseURL); + + void SetModuleRecord(JS::Handle aModuleRecord); + void SetPreInstantiationError(const JS::Value& aError); ScriptLoader* Loader() const { return mLoader; } JSObject* ModuleRecord() const { return mModuleRecord; } nsIURI* BaseURL() const { return mBaseURL; } + bool IsErrored() const; + JS::Value Error() const; + void UnlinkModuleRecord(); }; diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index cf7f06e713..a530989742 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -418,25 +418,23 @@ void ScriptLoader::SetModuleFetchFinishedAndResumeWaitingRequests(ModuleLoadRequest *aRequest, nsresult aResult) { - // Update module map with the result of fetching a single module script. The - // module script pointer is nullptr on error. + // Update module map with the result of fetching a single module script. // // If any requests for the same URL are waiting on this one to complete, they // will have ModuleLoaded or LoadFailed on them when the promise is // resolved/rejected. This is set up in StartLoad. - MOZ_ASSERT(!aRequest->IsReadyToRun()); - RefPtr promise; MOZ_ALWAYS_TRUE(mFetchingModules.Get(aRequest->mURI, getter_AddRefs(promise))); mFetchingModules.Remove(aRequest->mURI); - RefPtr ms(aRequest->mModuleScript); - MOZ_ASSERT(NS_SUCCEEDED(aResult) == (ms != nullptr)); - mFetchedModules.Put(aRequest->mURI, ms); + RefPtr moduleScript(aRequest->mModuleScript); + MOZ_ASSERT(NS_FAILED(aResult) == !moduleScript); + + mFetchedModules.Put(aRequest->mURI, moduleScript); if (promise) { - if (ms) { + if (moduleScript) { promise->Resolve(true, __func__); } else { promise->Reject(aResult, __func__); @@ -482,19 +480,29 @@ ScriptLoader::ProcessFetchedModuleSource(ModuleLoadRequest* aRequest) MOZ_ASSERT(!aRequest->mModuleScript); nsresult rv = CreateModuleScript(aRequest); + MOZ_ASSERT(NS_FAILED(rv) == !aRequest->mModuleScript); + SetModuleFetchFinishedAndResumeWaitingRequests(aRequest, rv); free(aRequest->mScriptTextBuf); aRequest->mScriptTextBuf = nullptr; aRequest->mScriptTextLength = 0; - if (NS_SUCCEEDED(rv)) { + if (NS_FAILED(rv)) { + aRequest->LoadFailed(); + return rv; + } + + if (!aRequest->mModuleScript->IsErrored()) { StartFetchingModuleDependencies(aRequest); } - return rv; + return NS_OK; } +static nsresult +ResolveRequestedModules(ModuleLoadRequest* aRequest, nsCOMArray& aUrls); + nsresult ScriptLoader::CreateModuleScript(ModuleLoadRequest* aRequest) { @@ -548,9 +556,34 @@ ScriptLoader::CreateModuleScript(ModuleLoadRequest* aRequest) } } MOZ_ASSERT(NS_SUCCEEDED(rv) == (module != nullptr)); - if (module) { - aRequest->mModuleScript = - new ModuleScript(this, aRequest->mBaseURL, module); + RefPtr moduleScript = new ModuleScript(this, aRequest->mBaseURL); + aRequest->mModuleScript = moduleScript; + + if (!module) { + // Compilation failed + MOZ_ASSERT(aes.HasException()); + JS::Rooted error(cx); + if (!aes.StealException(&error)) { + aRequest->mModuleScript = nullptr; + return NS_ERROR_FAILURE; + } + + moduleScript->SetPreInstantiationError(error); + aRequest->ModuleErrored(); + return NS_OK; + } + + moduleScript->SetModuleRecord(module); + + // Validate requested modules and treat failure to resolve module specifiers + // the same as a parse error. + nsCOMArray urls; + rv = ResolveRequestedModules(aRequest, urls); + if (NS_FAILED(rv)) { + // ResolveRequestedModules sets pre-instanitation error on failure. + MOZ_ASSERT(moduleScript->IsErrored()); + aRequest->ModuleErrored(); + return NS_OK; } } @@ -560,8 +593,8 @@ ScriptLoader::CreateModuleScript(ModuleLoadRequest* aRequest) } static bool -ThrowTypeError(JSContext* aCx, ModuleScript* aScript, - const nsString& aMessage) +CreateTypeError(JSContext* aCx, ModuleScript* aScript, const nsString& aMessage, + JS::MutableHandle aError) { JS::Rooted module(aCx, aScript->ModuleRecord()); JS::Rooted script(aCx, JS::GetModuleScript(aCx, module)); @@ -576,17 +609,11 @@ ThrowTypeError(JSContext* aCx, ModuleScript* aScript, return false; } - JS::Rooted error(aCx); - if (!JS::CreateError(aCx, JSEXN_TYPEERR, nullptr, filename, 0, 0, nullptr, - message, &error)) { - return false; - } - - JS_SetPendingException(aCx, error); - return false; + return JS::CreateError(aCx, JSEXN_TYPEERR, nullptr, filename, 0, 0, nullptr, + message, aError); } -static bool +static nsresult HandleResolveFailure(JSContext* aCx, ModuleScript* aScript, const nsAString& aSpecifier) { @@ -595,19 +622,13 @@ HandleResolveFailure(JSContext* aCx, ModuleScript* aScript, nsAutoString message(NS_LITERAL_STRING("Error resolving module specifier: ")); message.Append(aSpecifier); - return ThrowTypeError(aCx, aScript, message); -} + JS::Rooted error(aCx); + if (!CreateTypeError(aCx, aScript, message, &error)) { + return NS_ERROR_OUT_OF_MEMORY; + } -static bool -HandleModuleNotFound(JSContext* aCx, ModuleScript* aScript, - const nsAString& aSpecifier) -{ - // TODO: How can we get the line number of the failed import? - - nsAutoString message(NS_LITERAL_STRING("Resolved module not found in map: ")); - message.Append(aSpecifier); - - return ThrowTypeError(aCx, aScript, message); + aScript->SetPreInstantiationError(error); + return NS_OK; } static already_AddRefed @@ -705,7 +726,8 @@ ResolveRequestedModules(ModuleLoadRequest* aRequest, nsCOMArray &aUrls) ModuleScript* ms = aRequest->mModuleScript; nsCOMPtr uri = ResolveModuleSpecifier(ms, specifier); if (!uri) { - HandleResolveFailure(cx, ms, specifier); + nsresult rv = HandleResolveFailure(cx, ms, specifier); + NS_ENSURE_SUCCESS(rv, rv); return NS_ERROR_FAILURE; } @@ -724,6 +746,7 @@ void ScriptLoader::StartFetchingModuleDependencies(ModuleLoadRequest* aRequest) { MOZ_ASSERT(aRequest->mModuleScript); + MOZ_ASSERT(!aRequest->mModuleScript->IsErrored()); MOZ_ASSERT(!aRequest->IsReadyToRun()); aRequest->mProgress = ModuleLoadRequest::Progress::FetchingImports; @@ -731,7 +754,7 @@ ScriptLoader::StartFetchingModuleDependencies(ModuleLoadRequest* aRequest) nsCOMArray urls; nsresult rv = ResolveRequestedModules(aRequest, urls); if (NS_FAILED(rv)) { - aRequest->LoadFailed(); + aRequest->ModuleErrored(); return; } @@ -755,7 +778,7 @@ ScriptLoader::StartFetchingModuleDependencies(ModuleLoadRequest* aRequest) GenericPromise::All(AbstractThread::GetCurrent(), importsReady); allReady->Then(AbstractThread::GetCurrent(), __func__, aRequest, &ModuleLoadRequest::DependenciesLoaded, - &ModuleLoadRequest::LoadFailed); + &ModuleLoadRequest::ModuleErrored); } RefPtr @@ -778,6 +801,7 @@ ScriptLoader::StartFetchingModuleAndDependencies(ModuleLoadRequest* aRequest, nsresult rv = StartLoad(childRequest, NS_LITERAL_STRING("module"), false); if (NS_FAILED(rv)) { + MOZ_ASSERT(!childRequest->mModuleScript); childRequest->mReady.Reject(rv, __func__); return ready; } @@ -786,6 +810,7 @@ ScriptLoader::StartFetchingModuleAndDependencies(ModuleLoadRequest* aRequest, return ready; } +// 8.1.3.8.1 HostResolveImportedModule(referencingModule, specifier) bool HostResolveImportedModule(JSContext* aCx, unsigned argc, JS::Value* vp) { @@ -800,25 +825,25 @@ HostResolveImportedModule(JSContext* aCx, unsigned argc, JS::Value* vp) MOZ_ASSERT(script->ModuleRecord() == module); // Let url be the result of resolving a module specifier given referencing - // module script and specifier. If the result is failure, throw a TypeError - // exception and abort these steps. + // module script and specifier. nsAutoJSString string; if (!string.init(aCx, specifier)) { return false; } nsCOMPtr uri = ResolveModuleSpecifier(script, string); - if (!uri) { - return HandleResolveFailure(aCx, script, string); - } - // Let resolved module script be the value of the entry in module map whose - // key is url. If no such entry exists, throw a TypeError exception and abort - // these steps. + // 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); - if (!ms) { - return HandleModuleNotFound(aCx, script, string); - } + MOZ_ASSERT(ms, "Resolved module not found in module map"); + + MOZ_ASSERT(!ms->IsErrored()); *vp = JS::ObjectValue(*ms->ModuleRecord()); return true; @@ -842,10 +867,36 @@ EnsureModuleResolveHook(JSContext* aCx) return NS_OK; } +void +ScriptLoader::CheckModuleDependenciesLoaded(ModuleLoadRequest* aRequest) +{ + RefPtr moduleScript = aRequest->mModuleScript; + if (moduleScript && !moduleScript->IsErrored()) { + for (auto childRequest : aRequest->mImports) { + ModuleScript* childScript = childRequest->mModuleScript; + if (!childScript) { + // Load error + aRequest->mModuleScript = nullptr; + return; + } else if (childScript->IsErrored()) { + // Script error + moduleScript->SetPreInstantiationError(childScript->Error()); + return; + } + } + } +} + void ScriptLoader::ProcessLoadedModuleTree(ModuleLoadRequest* aRequest) { if (aRequest->IsTopLevel()) { + ModuleScript* moduleScript = aRequest->mModuleScript; + if (moduleScript && !moduleScript->IsErrored()) { + if (!InstantiateModuleTree(aRequest)) { + aRequest->mModuleScript = nullptr; + } + } MaybeMoveToLoadedList(aRequest); ProcessPendingRequests(); } @@ -855,6 +906,43 @@ ScriptLoader::ProcessLoadedModuleTree(ModuleLoadRequest* aRequest) } } +bool +ScriptLoader::InstantiateModuleTree(ModuleLoadRequest* aRequest) +{ + // Instantiate a top-level module and record any error. + + MOZ_ASSERT(aRequest); + MOZ_ASSERT(aRequest->IsTopLevel()); + + ModuleScript* moduleScript = aRequest->mModuleScript; + MOZ_ASSERT(moduleScript); + 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); + + JS::Rooted module(jsapi.cx(), moduleScript->ModuleRecord()); + bool ok = NS_SUCCEEDED(nsJSUtils::ModuleInstantiate(jsapi.cx(), module)); + + if (!ok) { + MOZ_ASSERT(jsapi.HasException()); + JS::RootedValue exception(jsapi.cx()); + if (!jsapi.StealException(&exception)) { + return false; + } + MOZ_ASSERT(!exception.isUndefined()); + // Ignore the exception. It will be recorded in the module record. + } + + return true; +} + nsresult ScriptLoader::StartLoad(ScriptLoadRequest *aRequest, const nsAString &aType, bool aScriptFromHead) @@ -1364,13 +1452,19 @@ ScriptLoader::ProcessScriptElement(nsIScriptElement *aElement) ModuleLoadRequest* modReq = request->AsModuleRequest(); modReq->mBaseURL = mDocument->GetDocBaseURI(); rv = CreateModuleScript(modReq); - NS_ENSURE_SUCCESS(rv, false); - StartFetchingModuleDependencies(modReq); + MOZ_ASSERT(NS_FAILED(rv) == !modReq->mModuleScript); + if (NS_FAILED(rv)) { + modReq->LoadFailed(); + return false; + } if (aElement->GetScriptAsync()) { mLoadingAsyncRequests.AppendElement(request); } else { AddDeferRequest(request); } + if (!modReq->mModuleScript->IsErrored()) { + StartFetchingModuleDependencies(modReq); + } return false; } request->mProgress = ScriptLoadRequest::Progress::Ready; @@ -1448,11 +1542,7 @@ ScriptLoader::ProcessOffThreadRequest(ScriptLoadRequest* aRequest) if (aRequest->IsModuleRequest()) { MOZ_ASSERT(aRequest->mOffThreadToken); ModuleLoadRequest* request = aRequest->AsModuleRequest(); - nsresult rv = ProcessFetchedModuleSource(request); - if (NS_FAILED(rv)) { - request->LoadFailed(); - } - return rv; + return ProcessFetchedModuleSource(request); } aRequest->SetReady(); @@ -1624,7 +1714,7 @@ ScriptLoader::ProcessRequest(ScriptLoadRequest* aRequest) if (aRequest->IsModuleRequest() && !aRequest->AsModuleRequest()->mModuleScript) { - // There was an error parsing a module script. Nothing to do here. + // There was an error fetching a module script. Nothing to do here. FireScriptAvailable(NS_ERROR_FAILURE, aRequest); return NS_OK; } @@ -1846,8 +1936,8 @@ ScriptLoader::EvaluateScript(ScriptLoadRequest* aRequest) // http://www.whatwg.org/specs/web-apps/current-work/#execute-the-script-block nsAutoMicroTask mt; AutoEntryScript aes(globalObject, " - + diff --git a/dom/base/test/jsmodules/test_syntaxErrorAsync.html b/dom/base/test/jsmodules/test_syntaxErrorAsync.html index 35d9237553..3593d9dd73 100644 --- a/dom/base/test/jsmodules/test_syntaxErrorAsync.html +++ b/dom/base/test/jsmodules/test_syntaxErrorAsync.html @@ -4,20 +4,27 @@ - + diff --git a/dom/base/test/jsmodules/test_syntaxErrorInline.html b/dom/base/test/jsmodules/test_syntaxErrorInline.html index 705bc5902e..b85b954ece 100644 --- a/dom/base/test/jsmodules/test_syntaxErrorInline.html +++ b/dom/base/test/jsmodules/test_syntaxErrorInline.html @@ -4,22 +4,29 @@ - -