Commit 4fe8a40e authored by Hiroshige Hayashizaki's avatar Hiroshige Hayashizaki Committed by Commit Bot

[Import Maps] Always consider "importmap" valid in IsValidScriptTypeAndLanguage

Previously, ScriptLoader::IsValidScriptTypeAndLanguage() returns false
for "importmap" type if import maps are not enabled.
However, when import maps can be enabled via origin trials, checking whether
import maps are enabled requires ExecutionContext, which is not trivial
in IsValidScriptTypeAndLanguage(), especially because it is called
from HTMLTreeBuilderSimulator used by background parsers.

Therefore, this CL removes the check and makes "importmap"
always valid in IsValidScriptTypeAndLanguage().
Instead, import maps are ignored subsequently in PrepareScript()
(in ParseAndRegisterImportMap()).

There are slight behavior changes that are probably acceptable as
these affects only pages with <script type="importmap"> on browsers
where import maps are not enabled.
- UseCounter in HTMLScriptElement::InsertedInto()
- HTMLTreeBuilderSimulator considers <script type="importmap"> always
  as kValidScriptStart.
- In PrepareScript(), <script type="importmap"> is rejected slightly
  later.

Bug: 829084
Change-Id: Iebdfe158ecebee4c310ffca88fd77f3133856035
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1502000
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: default avatarKouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#637913}
parent 68635148
...@@ -165,8 +165,12 @@ bool IsValidClassicScriptTypeAndLanguage( ...@@ -165,8 +165,12 @@ bool IsValidClassicScriptTypeAndLanguage(
return false; return false;
} }
// Returns true on success. enum class ShouldFireErrorEvent {
bool ParseAndRegisterImportMap(ScriptElementBase& element) { kDoNotFire,
kShouldFire,
};
ShouldFireErrorEvent ParseAndRegisterImportMap(ScriptElementBase& element) {
Document& element_document = element.GetDocument(); Document& element_document = element.GetDocument();
Document* context_document = element_document.ContextDocument(); Document* context_document = element_document.ContextDocument();
DCHECK(context_document); DCHECK(context_document);
...@@ -174,11 +178,16 @@ bool ParseAndRegisterImportMap(ScriptElementBase& element) { ...@@ -174,11 +178,16 @@ bool ParseAndRegisterImportMap(ScriptElementBase& element) {
Modulator::From(ToScriptStateForMainWorld(context_document->GetFrame())); Modulator::From(ToScriptStateForMainWorld(context_document->GetFrame()));
DCHECK(modulator); DCHECK(modulator);
// If import maps are not enabled, we do nothing and return here, and also
// do not fire error events.
if (!RuntimeEnabledFeatures::BuiltInModuleInfraEnabled())
return ShouldFireErrorEvent::kDoNotFire;
if (!modulator->IsAcquiringImportMaps()) { if (!modulator->IsAcquiringImportMaps()) {
element_document.AddConsoleMessage(ConsoleMessage::Create( element_document.AddConsoleMessage(ConsoleMessage::Create(
kJSMessageSource, kErrorMessageLevel, kJSMessageSource, kErrorMessageLevel,
"An import map is added after module script load was triggered.")); "An import map is added after module script load was triggered."));
return false; return ShouldFireErrorEvent::kShouldFire;
} }
// TODO(crbug.com/922212): Implemenet external import maps. // TODO(crbug.com/922212): Implemenet external import maps.
...@@ -186,7 +195,7 @@ bool ParseAndRegisterImportMap(ScriptElementBase& element) { ...@@ -186,7 +195,7 @@ bool ParseAndRegisterImportMap(ScriptElementBase& element) {
element_document.AddConsoleMessage( element_document.AddConsoleMessage(
ConsoleMessage::Create(kJSMessageSource, kErrorMessageLevel, ConsoleMessage::Create(kJSMessageSource, kErrorMessageLevel,
"External import maps are not yet supported.")); "External import maps are not yet supported."));
return false; return ShouldFireErrorEvent::kShouldFire;
} }
KURL base_url = element_document.BaseURL(); KURL base_url = element_document.BaseURL();
...@@ -194,10 +203,10 @@ bool ParseAndRegisterImportMap(ScriptElementBase& element) { ...@@ -194,10 +203,10 @@ bool ParseAndRegisterImportMap(ScriptElementBase& element) {
ImportMap::Create(element.TextFromChildren(), base_url, element_document); ImportMap::Create(element.TextFromChildren(), base_url, element_document);
if (!import_map) if (!import_map)
return false; return ShouldFireErrorEvent::kShouldFire;
modulator->RegisterImportMap(import_map); modulator->RegisterImportMap(import_map);
return true; return ShouldFireErrorEvent::kDoNotFire;
} }
} // namespace } // namespace
...@@ -233,8 +242,7 @@ bool ScriptLoader::IsValidScriptTypeAndLanguage( ...@@ -233,8 +242,7 @@ bool ScriptLoader::IsValidScriptTypeAndLanguage(
return true; return true;
} }
if (RuntimeEnabledFeatures::BuiltInModuleInfraEnabled() && if (type == "importmap") {
type == "importmap") {
if (out_is_import_map) if (out_is_import_map)
*out_is_import_map = true; *out_is_import_map = true;
return true; return true;
...@@ -386,7 +394,8 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position, ...@@ -386,7 +394,8 @@ bool ScriptLoader::PrepareScript(const TextPosition& script_start_position,
// Process the import map. // Process the import map.
if (is_import_map) { if (is_import_map) {
if (!ParseAndRegisterImportMap(*element_)) { if (ParseAndRegisterImportMap(*element_) ==
ShouldFireErrorEvent::kShouldFire) {
element_document.GetTaskRunner(TaskType::kDOMManipulation) element_document.GetTaskRunner(TaskType::kDOMManipulation)
->PostTask(FROM_HERE, ->PostTask(FROM_HERE,
WTF::Bind(&ScriptElementBase::DispatchErrorEvent, WTF::Bind(&ScriptElementBase::DispatchErrorEvent,
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment