From 43e0632cd474265ef0660bf881f4472996c8ad5a Mon Sep 17 00:00:00 2001 From: Moonchild Date: Wed, 8 Jul 2020 12:58:30 +0000 Subject: Issue #618 - Align error handling for module scripts with the spec (again) This updates module implementation to match spec regarding handling of instantiation errors, after it was changed yet again, this time to not remember instantiation errors, but instead immediately rethrow applicable ones. Ref: BZ 1420420 --- dom/script/ModuleLoadRequest.cpp | 4 +-- dom/script/ModuleScript.cpp | 43 +++++++++------------- dom/script/ModuleScript.h | 12 ++++--- dom/script/ScriptLoader.cpp | 77 ++++++++++++++++++++++++++-------------- dom/script/ScriptLoader.h | 1 + 5 files changed, 79 insertions(+), 58 deletions(-) (limited to 'dom') diff --git a/dom/script/ModuleLoadRequest.cpp b/dom/script/ModuleLoadRequest.cpp index d62214304..a12fcb162 100644 --- a/dom/script/ModuleLoadRequest.cpp +++ b/dom/script/ModuleLoadRequest.cpp @@ -83,7 +83,7 @@ ModuleLoadRequest::ModuleLoaded() // been loaded. mModuleScript = mLoader->GetFetchedModule(mURI); - if (!mModuleScript || mModuleScript->IsErrored()) { + if (!mModuleScript || mModuleScript->HasParseError()) { ModuleErrored(); return; } @@ -95,7 +95,7 @@ void ModuleLoadRequest::ModuleErrored() { mLoader->CheckModuleDependenciesLoaded(this); - MOZ_ASSERT(!mModuleScript || mModuleScript->IsErrored()); + MOZ_ASSERT(!mModuleScript || mModuleScript->HasParseError()); CancelImports(); SetReady(); diff --git a/dom/script/ModuleScript.cpp b/dom/script/ModuleScript.cpp index 28b97a3cb..1bf9d0b0f 100644 --- a/dom/script/ModuleScript.cpp +++ b/dom/script/ModuleScript.cpp @@ -26,7 +26,8 @@ NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN(ModuleScript) NS_IMPL_CYCLE_COLLECTION_UNLINK(mLoader) NS_IMPL_CYCLE_COLLECTION_UNLINK(mBaseURL) tmp->UnlinkModuleRecord(); - tmp->mError.setUndefined(); + tmp->mParseError.setUndefined(); + tmp->mErrorToRethrow.setUndefined(); NS_IMPL_CYCLE_COLLECTION_UNLINK_END NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN(ModuleScript) @@ -35,7 +36,8 @@ 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_JS_MEMBER_CALLBACK(mParseError) + NS_IMPL_CYCLE_COLLECTION_TRACE_JS_MEMBER_CALLBACK(mErrorToRethrow) NS_IMPL_CYCLE_COLLECTION_TRACE_END NS_IMPL_CYCLE_COLLECTING_ADDREF(ModuleScript) @@ -48,7 +50,8 @@ ModuleScript::ModuleScript(ScriptLoader *aLoader, nsIURI* aBaseURL) MOZ_ASSERT(mLoader); MOZ_ASSERT(mBaseURL); MOZ_ASSERT(!mModuleRecord); - MOZ_ASSERT(mError.isUndefined()); + MOZ_ASSERT(!HasParseError()); + MOZ_ASSERT(!HasErrorToRethrow()); } void @@ -74,7 +77,8 @@ void ModuleScript::SetModuleRecord(JS::Handle aModuleRecord) { MOZ_ASSERT(!mModuleRecord); - MOZ_ASSERT(mError.isUndefined()); + MOZ_ASSERT(!HasParseError()); + MOZ_ASSERT(!HasErrorToRethrow()); mModuleRecord = aModuleRecord; @@ -85,37 +89,24 @@ ModuleScript::SetModuleRecord(JS::Handle aModuleRecord) } void -ModuleScript::SetPreInstantiationError(const JS::Value& aError) +ModuleScript::SetParseError(const JS::Value& aError) { MOZ_ASSERT(!aError.isUndefined()); + MOZ_ASSERT(!HasParseError()); + MOZ_ASSERT(!HasErrorToRethrow()); UnlinkModuleRecord(); - mError = aError; - + mParseError = aError; HoldJSObjects(this); } -bool -ModuleScript::IsErrored() const -{ - if (!mModuleRecord) { - MOZ_ASSERT(!mError.isUndefined()); - return true; - } - - return JS::IsModuleErrored(mModuleRecord); -} - -JS::Value -ModuleScript::Error() const +void +ModuleScript::SetErrorToRethrow(const JS::Value& aError) { - MOZ_ASSERT(IsErrored()); - - if (!mModuleRecord) { - return mError; - } + MOZ_ASSERT(!aError.isUndefined()); + MOZ_ASSERT(!HasErrorToRethrow()); - return JS::GetModuleError(mModuleRecord); + mErrorToRethrow = aError; } } // dom namespace diff --git a/dom/script/ModuleScript.h b/dom/script/ModuleScript.h index 571359859..f765aa0fa 100644 --- a/dom/script/ModuleScript.h +++ b/dom/script/ModuleScript.h @@ -23,7 +23,8 @@ class ModuleScript final : public nsISupports RefPtr mLoader; nsCOMPtr mBaseURL; JS::Heap mModuleRecord; - JS::Heap mError; + JS::Heap mParseError; + JS::Heap mErrorToRethrow; ~ModuleScript(); @@ -35,14 +36,17 @@ public: nsIURI* aBaseURL); void SetModuleRecord(JS::Handle aModuleRecord); - void SetPreInstantiationError(const JS::Value& aError); + void SetParseError(const JS::Value& aError); + void SetErrorToRethrow(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; + JS::Value ParseError() const { return mParseError; } + JS::Value ErrorToRethrow() const { return mErrorToRethrow; } + bool HasParseError() const { return !mParseError.isUndefined(); } + bool HasErrorToRethrow() const { return !mErrorToRethrow.isUndefined(); } void UnlinkModuleRecord(); }; diff --git a/dom/script/ScriptLoader.cpp b/dom/script/ScriptLoader.cpp index 1426c30c9..3dbbcacea 100644 --- a/dom/script/ScriptLoader.cpp +++ b/dom/script/ScriptLoader.cpp @@ -493,7 +493,7 @@ ScriptLoader::ProcessFetchedModuleSource(ModuleLoadRequest* aRequest) return rv; } - if (!aRequest->mModuleScript->IsErrored()) { + if (!aRequest->mModuleScript->HasParseError()) { StartFetchingModuleDependencies(aRequest); } @@ -568,7 +568,7 @@ ScriptLoader::CreateModuleScript(ModuleLoadRequest* aRequest) return NS_ERROR_FAILURE; } - moduleScript->SetPreInstantiationError(error); + moduleScript->SetParseError(error); aRequest->ModuleErrored(); return NS_OK; } @@ -580,8 +580,6 @@ ScriptLoader::CreateModuleScript(ModuleLoadRequest* aRequest) 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; } @@ -627,7 +625,7 @@ HandleResolveFailure(JSContext* aCx, ModuleScript* aScript, return NS_ERROR_OUT_OF_MEMORY; } - aScript->SetPreInstantiationError(error); + aScript->SetParseError(error); return NS_OK; } @@ -723,7 +721,6 @@ ResolveRequestedModules(ModuleLoadRequest* aRequest, nsCOMArray &aUrls) } // Let url be the result of resolving a module specifier given module script and requested. - ModuleScript* ms = aRequest->mModuleScript; nsCOMPtr uri = ResolveModuleSpecifier(ms, specifier); if (!uri) { nsresult rv = HandleResolveFailure(cx, ms, specifier); @@ -746,7 +743,7 @@ void ScriptLoader::StartFetchingModuleDependencies(ModuleLoadRequest* aRequest) { MOZ_ASSERT(aRequest->mModuleScript); - MOZ_ASSERT(!aRequest->mModuleScript->IsErrored()); + MOZ_ASSERT(!aRequest->mModuleScript->HasParseError()); MOZ_ASSERT(!aRequest->IsReadyToRun()); aRequest->mProgress = ModuleLoadRequest::Progress::FetchingImports; @@ -843,7 +840,7 @@ HostResolveImportedModule(JSContext* aCx, unsigned argc, JS::Value* vp) ModuleScript* ms = script->Loader()->GetFetchedModule(uri); MOZ_ASSERT(ms, "Resolved module not found in module map"); - MOZ_ASSERT(!ms->IsErrored()); + MOZ_ASSERT(!ms->HasParseError()); *vp = JS::ObjectValue(*ms->ModuleRecord()); return true; @@ -871,18 +868,16 @@ 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; - } + if (!moduleScript || moduleScript->HasParseError()) { + return; + } + + for (auto childRequest : aRequest->mImports) { + ModuleScript* childScript = childRequest->mModuleScript; + if (!childScript) { + aRequest->mModuleScript = nullptr; + // Load error on script load request; bail. + return; } } } @@ -892,7 +887,7 @@ ScriptLoader::ProcessLoadedModuleTree(ModuleLoadRequest* aRequest) { if (aRequest->IsTopLevel()) { ModuleScript* moduleScript = aRequest->mModuleScript; - if (moduleScript && !moduleScript->IsErrored()) { + if (moduleScript && !moduleScript->HasErrorToRethrow()) { if (!InstantiateModuleTree(aRequest)) { aRequest->mModuleScript = nullptr; } @@ -906,6 +901,28 @@ ScriptLoader::ProcessLoadedModuleTree(ModuleLoadRequest* aRequest) } } +JS::Value +ScriptLoader::FindFirstParseError(ModuleLoadRequest* aRequest) +{ + MOZ_ASSERT(aRequest); + + ModuleScript* moduleScript = aRequest->mModuleScript; + MOZ_ASSERT(moduleScript); + + if (moduleScript->HasParseError()) { + return moduleScript->ParseError(); + } + + for (ModuleLoadRequest* childRequest : aRequest->mImports) { + JS::Value error = FindFirstParseError(childRequest); + if (!error.isUndefined()) { + return error; + } + } + + return JS::UndefinedValue(); +} + bool ScriptLoader::InstantiateModuleTree(ModuleLoadRequest* aRequest) { @@ -916,6 +933,14 @@ ScriptLoader::InstantiateModuleTree(ModuleLoadRequest* aRequest) ModuleScript* moduleScript = aRequest->mModuleScript; MOZ_ASSERT(moduleScript); + + JS::Value parseError = FindFirstParseError(aRequest); + if (!parseError.isUndefined()) { + // Parse error found in the requested script + moduleScript->SetErrorToRethrow(parseError); + return true; + } + MOZ_ASSERT(moduleScript->ModuleRecord()); nsAutoMicroTask mt; @@ -937,7 +962,7 @@ ScriptLoader::InstantiateModuleTree(ModuleLoadRequest* aRequest) return false; } MOZ_ASSERT(!exception.isUndefined()); - // Ignore the exception. It will be recorded in the module record. + moduleScript->SetErrorToRethrow(exception); } return true; @@ -1462,7 +1487,7 @@ ScriptLoader::ProcessScriptElement(nsIScriptElement *aElement) } else { AddDeferRequest(request); } - if (!modReq->mModuleScript->IsErrored()) { + if (!modReq->mModuleScript->HasParseError()) { StartFetchingModuleDependencies(modReq); } return false; @@ -1966,9 +1991,9 @@ ScriptLoader::EvaluateScript(ScriptLoadRequest* aRequest) MOZ_ASSERT(!request->mOffThreadToken); ModuleScript* moduleScript = request->mModuleScript; - if (moduleScript->IsErrored()) { - // Module has an error status - JS::Rooted error(cx, moduleScript->Error()); + if (moduleScript->HasErrorToRethrow()) { + // Module has an error status to be rethrown + JS::Rooted error(cx, moduleScript->ErrorToRethrow()); JS_SetPendingException(cx, error); return NS_OK; // An error is reported by AutoEntryScript. } diff --git a/dom/script/ScriptLoader.h b/dom/script/ScriptLoader.h index b07dd4ec6..db2eeed31 100644 --- a/dom/script/ScriptLoader.h +++ b/dom/script/ScriptLoader.h @@ -583,6 +583,7 @@ private: nsresult ProcessFetchedModuleSource(ModuleLoadRequest* aRequest); void CheckModuleDependenciesLoaded(ModuleLoadRequest* aRequest); void ProcessLoadedModuleTree(ModuleLoadRequest* aRequest); + JS::Value FindFirstParseError(ModuleLoadRequest* aRequest); bool InstantiateModuleTree(ModuleLoadRequest* aRequest); void StartFetchingModuleDependencies(ModuleLoadRequest* aRequest); -- cgit v1.2.3