Skip to content

Commit 80236c4

Browse files
allstarschhallstarschh
committed
Bug 1877703 - Don't add the URI into mFetchedModules if it has a parse error and is preloaded. r=jonco
Differential Revision: https://phabricator.services.mozilla.com/D200431
1 parent 195daa2 commit 80236c4

File tree

4 files changed

+110
-1
lines changed

4 files changed

+110
-1
lines changed

‎dom/base/test/jsmodules/importmaps/mochitest.toml

+2
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,6 @@ support-files = [
3030

3131
["test_bug_1873417.html"]
3232

33+
["test_dynamic_importMap_with_external_script.html"]
34+
3335
["test_importMap_with_external_script.html"]

‎dom/base/test/jsmodules/importmaps/test_dynamic_importMap_with_external_script.html

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
<!DOCTYPE html>
2+
<meta charset=utf-8>
3+
<head>
4+
<title>Test speculative preload of external script doesn't conflict with import map</title>
5+
<script src="/tests/SimpleTest/SimpleTest.js"></script>
6+
<link rel="stylesheet" type="text/css" href="/tests/SimpleTest/test.css"/>
7+
</head>
8+
9+
<!--
10+
These tests check that speculative preloading, which could happen before
11+
the import map is installed, doesn't load the wrong modules. This version
12+
dynamically inserts the import map script element after speculative
13+
preloading has started.
14+
-->
15+
16+
<script>
17+
const script = document.createElement('script');
18+
script.type = 'importmap';
19+
script.textContent =
20+
`{
21+
"imports": {
22+
"bare": "./good/module_0.mjs",
23+
"./bad/module_1.mjs": "./good/module_1.mjs",
24+
"./bad/module_2.mjs": "./good/module_2.mjs",
25+
"./bad/module_3.mjs": "./good/module_3.mjs",
26+
"./bad/module_4.mjs": "./good/module_4.mjs",
27+
"./bad/module_7.mjs": "./good/module_7.mjs"
28+
}
29+
}`;
30+
document.head.appendChild(script);
31+
</script>
32+
33+
<!--
34+
Test bareword import (not supported before import map installed).
35+
-->
36+
<script type="module" src="module_importMap_with_external_script_0.mjs"></script>
37+
38+
<!--
39+
Test mapping from missing resource to existing resource (not found before
40+
import map installed).
41+
-->
42+
<script type="module" src="module_importMap_with_external_script_1.mjs"></script>
43+
44+
<!--
45+
Test mapping from one existing resource to another (would load wrong resource before
46+
import map installed).
47+
-->
48+
<script type="module" src="module_importMap_with_external_script_2.mjs"></script>
49+
50+
<!--
51+
Test mapping from one existing resource to another with circular dependency.
52+
-->
53+
<script type="module" src="module_importMap_with_external_script_3.mjs"></script>
54+
55+
<!--
56+
Test with redirect, script_6.mjs -> script_5.mjs -> script_4.mjs.
57+
We redirect twice here, as sometimes one redirect can't reproduce the crash
58+
from bug 1835468.
59+
-->
60+
<script type="module" src="module_importMap_with_external_script_6.mjs"></script>
61+
62+
<!--
63+
Test with async attribute
64+
-->
65+
<script type="module" async src="module_importMap_with_external_script_7.mjs"></script>
66+
67+
<script>
68+
SimpleTest.waitForExplicitFinish();
69+
70+
let passCount = 0;
71+
const expectedCount = 6;
72+
73+
function success(name) {
74+
ok(true, "Test passed, loaded " + name);
75+
passCount++;
76+
if (passCount == expectedCount) {
77+
SimpleTest.finish();
78+
}
79+
}
80+
</script>
81+
<body></body>

‎js/loader/ModuleLoaderBase.cpp

+25-1
Original file line numberDiff line numberDiff line change
@@ -478,6 +478,23 @@ void ModuleLoaderBase::SetModuleFetchStarted(ModuleLoadRequest* aRequest) {
478478
mFetchingModules.InsertOrUpdate(aRequest->mURI, nullptr);
479479
}
480480

481+
bool ModuleLoaderBase::IsScriptPreloadedAndHasParseError(
482+
ModuleLoadRequest* aRequest) {
483+
RefPtr<ModuleScript> moduleScript(aRequest->mModuleScript);
484+
485+
bool hasParseError = false;
486+
if (moduleScript) {
487+
hasParseError = moduleScript->HasParseError();
488+
}
489+
490+
bool isPreload = false;
491+
if (aRequest->HasScriptLoadContext()) {
492+
isPreload = aRequest->GetScriptLoadContext()->IsPreload();
493+
}
494+
495+
return hasParseError && isPreload;
496+
}
497+
481498
void ModuleLoaderBase::SetModuleFetchFinishedAndResumeWaitingRequests(
482499
ModuleLoadRequest* aRequest, nsresult aResult) {
483500
// Update module map with the result of fetching a single module script.
@@ -505,7 +522,14 @@ void ModuleLoaderBase::SetModuleFetchFinishedAndResumeWaitingRequests(
505522
RefPtr<ModuleScript> moduleScript(aRequest->mModuleScript);
506523
MOZ_ASSERT(NS_FAILED(aResult) == !moduleScript);
507524

508-
mFetchedModules.InsertOrUpdate(aRequest->mURI, RefPtr{moduleScript});
525+
// In the case an import map is inserted dynamically, the preloaded module
526+
// scripts might fail to resolve the import specifiers and hence get parse
527+
// errors. In that case, we don't put the URI of the preloaded module script
528+
// into mFetchedModules map, and ScriptLoader will fetch the module script
529+
// again.
530+
if (!IsScriptPreloadedAndHasParseError(aRequest)) {
531+
mFetchedModules.InsertOrUpdate(aRequest->mURI, RefPtr{moduleScript});
532+
}
509533

510534
if (waitingRequests) {
511535
LOG(("ScriptLoadRequest (%p): Resuming waiting requests", aRequest));

‎js/loader/ModuleLoaderBase.h

+2
Original file line numberDiff line numberDiff line change
@@ -438,6 +438,8 @@ class ModuleLoaderBase : public nsISupports {
438438

439439
nsresult CreateModuleScript(ModuleLoadRequest* aRequest);
440440

441+
bool IsScriptPreloadedAndHasParseError(ModuleLoadRequest* aRequest);
442+
441443
// The slot stored in ImportMetaResolve function.
442444
enum { ModulePrivateSlot = 0, SlotCount };
443445

0 commit comments

Comments
 (0)