1777801 - Incomplete condition before calling GetScriptLoadContext()

Status: RESOLVED FIXED

(Core :: XPConnect, defect)

Description

[Tooru Fujisawa [:arai]]

https://searchfox.org/mozilla-central/rev/da3cc1b31be7a7d919d6ebde4e3c033a83446e05/js/loader/ScriptLoadRequest.cpp#114,116-117

void ScriptLoadRequest::Cancel() {
...
  if (HasLoadContext()) {
    GetScriptLoadContext()->MaybeCancelOffThreadScript();

https://searchfox.org/mozilla-central/rev/da3cc1b31be7a7d919d6ebde4e3c033a83446e05/js/loader/ScriptLoadRequest.cpp#126-133

bool ScriptLoadRequest::HasScriptLoadContext() const {
  return HasLoadContext() && mLoadContext->IsWindowContext();
}

mozilla::dom::ScriptLoadContext* ScriptLoadRequest::GetScriptLoadContext() {
  MOZ_ASSERT(mLoadContext);
  return mLoadContext->AsWindowContext();
}

GetScriptLoadContext() requires HasScriptLoadContext(), instead of HasLoadContext() alone.

Same situation in
https://searchfox.org/mozilla-central/rev/da3cc1b31be7a7d919d6ebde4e3c033a83446e05/js/loader/ScriptLoadRequest.cpp#177-180

nsresult ScriptLoadRequest::GetScriptSource(JSContext* aCx,
                                            MaybeSourceText* aMaybeSource) {
  // If there's no script text, we try to get it from the element
  if (HasLoadContext() && GetScriptLoadContext()->mIsInline) {

Comment 1

[Tooru Fujisawa [:arai]]

Attachment: testcase

Comment 2

[Tooru Fujisawa [:arai]]

Also, the file not found error isn't propagated to mozJSModuleLoader::ImportESModule and it fails with nullptr deref, even if the above HasLoadContext() is replaced to HasScriptLoadContext()

Comment 3

[Jon Coppeard (:jonco)]

Attachment: Bug 1777801 - Check for failure before attempting to instantiate module graph in mozJSModuleLoader::ImportESModule r?arai

This fixes the overly general context kind checks pointed out in the bug and
adds error checking in ImportESModule.

This also showed that JS::ClearModuleEnvironment could fail if the module's
environment had not been initialized yet.

This patch also adds a couple more tests.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/dee83eddd306
Check for failure before attempting to instantiate module graph in mozJSModuleLoader::ImportESModule r=arai

Comment 5

[Norisz Fay [:noriszfay]]

https://hg.mozilla.org/mozilla-central/rev/dee83eddd306