[Import Maps] Allow invalid or non-fetch-scheme URLs as keys (as bare specifiers)

Bug: 990561
Change-Id: Ie745f8a50c28cb19cdaf31c95b188b1db762d5d1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1761892
Commit-Queue: Hiroshige Hayashizaki <hiroshige@chromium.org>
Reviewed-by: Kouhei Ueno <kouhei@chromium.org>
Cr-Commit-Position: refs/heads/master@{#702251}

third_party/blink/renderer/core/script/import_map.cc

@@ -49,9 +49,15 @@ String NormalizeSpecifierKey(const String& key_string,
                              const KURL& base_url,
                              ConsoleLogger& logger) {
   // <spec step="1">If specifierKey is the empty string, then:</spec>
-  //
-  // TODO(hiroshige): Implement this step explicitly. Anyway currently empty
-  // strings are considered as invalid by ParsedSpecifier.
+  if (key_string.IsEmpty()) {
+    // <spec step="1.1">Report a warning to the console that specifier keys
+    // cannot be the empty string.</spec>
+    AddIgnoredKeyMessage(logger, key_string,
+                         "specifier keys cannot be the empty string.");
+
+    // <spec step="1.2">Return null.</spec>
+    return String();
+  }
 
   // <spec step="2">Let url be the result of parsing a URL-like import
   // specifier, given specifierKey and baseURL.</spec>
@@ -59,14 +65,9 @@ String NormalizeSpecifierKey(const String& key_string,
 
   switch (key.GetType()) {
     case ParsedSpecifier::Type::kInvalid:
-      // TODO(hiroshige): According to the spec, this should be considered as a
-      // bare specifier.
-      AddIgnoredKeyMessage(logger, key_string, "Invalid key (invalid URL)");
-      return String();
-
     case ParsedSpecifier::Type::kBare:
       // <spec step="4">Return specifierKey.</spec>
-      return key.GetImportMapKeyString();
+      return key_string;
 
     case ParsedSpecifier::Type::kURL:
       // <spec
@@ -77,9 +78,8 @@ String NormalizeSpecifierKey(const String& key_string,
       // TODO(hiroshige): Perhaps we should move this into ParsedSpecifier.
       if (!SchemeRegistry::IsFetchScheme(key.GetUrl().Protocol()) &&
           key.GetUrl().Protocol() != kStdScheme) {
-        AddIgnoredKeyMessage(logger, key_string,
-                             "Invalid key (non-fetch scheme)");
-        return String();
+        // <spec step="4">Return specifierKey.</spec>
+        return key_string;
       }
       // <spec step="3">If url is not null, then return the serialization of
       // url.</spec>

third_party/blink/web_tests/external/wpt/import-maps/imported/parsing-specifier-keys.tentative-expected.txt

@@ -3,7 +3,7 @@ PASS Relative URL-like specifier keys / should absolutize strings prefixed with
 FAIL Relative URL-like specifier keys / should not absolutize strings prefixed with ./, ../, or / with a data: URL base assert_equals: expected "{\"../foo\":[\"https://example.com/dotdotslash\"],\"./foo\":[\"https://example.com/dotslash\"],\"/foo\":[\"https://example.com/slash\"]}" but got "{\"http://web-platform.test:8001/foo\":[\"https://example.com/slash\"],\"http://web-platform.test:8001/import-maps/foo\":[\"https://example.com/dotdotslash\"],\"http://web-platform.test:8001/import-maps/imported/foo\":[\"https://example.com/dotslash\"]}"
 PASS Relative URL-like specifier keys / should absolutize the literal strings ./, ../, or / with no suffix
 PASS Relative URL-like specifier keys / should treat percent-encoded variants of ./, ../, or / as bare specifiers
-FAIL Absolute URL specifier keys / should only accept absolute URL specifier keys with fetch schemes, treating others as bare specifiers assert_equals: expected "{\"about:good\":[\"https://base.example/about\"],\"blob:good\":[\"https://base.example/blob\"],\"data:good\":[\"https://base.example/data\"],\"file:///good\":[\"https://base.example/file\"],\"filesystem:good\":[\"https://base.example/filesystem\"],\"ftp://good/\":[\"https://base.example/ftp/\"],\"http://good/\":[\"https://base.example/http/\"],\"https://good/\":[\"https://base.example/https/\"],\"import:bad\":[\"https://base.example/import\"],\"javascript:bad\":[\"https://base.example/javascript\"],\"mailto:bad\":[\"https://base.example/mailto\"],\"wss:bad\":[\"https://base.example/wss\"]}" but got "{\"about:good\":[\"https://base.example/about\"],\"blob:good\":[\"https://base.example/blob\"],\"data:good\":[\"https://base.example/data\"],\"file:///good\":[\"https://base.example/file\"],\"filesystem:good\":[\"https://base.example/filesystem\"],\"ftp://good/\":[\"https://base.example/ftp/\"],\"http://good/\":[\"https://base.example/http/\"],\"https://good/\":[\"https://base.example/https/\"]}"
+PASS Absolute URL specifier keys / should only accept absolute URL specifier keys with fetch schemes, treating others as bare specifiers
 FAIL Absolute URL specifier keys / should parse absolute URLs, treating unparseable ones as bare specifiers assert_equals: expected "{\"http://[www.example.com]/\":[\"https://base.example/unparseable3/\"],\"https://ex ample.org/\":[\"https://base.example/unparseable1/\"],\"https://example.com/\":[\"https://base.example/percentDecoding/\"],\"https://example.com/%41\":[\"https://base.example/noPercentDecoding\"],\"https://example.com///\":[\"https://base.example/invalidButParseable2/\"],\"https://example.com:demo\":[\"https://base.example/unparseable2\"],\"https://example.net/\":[\"https://base.example/prettyNormal/\"],\"https://example.org/\":[\"https://base.example/invalidButParseable1/\"]}" but got "{\"http://[www.example.com]/\":[\"https://base.example/unparseable3/\"],\"https://ex%20ample.org/\":[\"https://base.example/unparseable1/\"],\"https://example.com/\":[\"https://base.example/percentDecoding/\"],\"https://example.com///\":[\"https://base.example/invalidButParseable2/\"],\"https://example.com/A\":[\"https://base.example/noPercentDecoding\"],\"https://example.com:demo\":[\"https://base.example/unparseable2\"],\"https://example.net/\":[\"https://base.example/prettyNormal/\"],\"https://example.org/\":[\"https://base.example/invalidButParseable1/\"]}"
 FAIL Absolute URL specifier keys / should parse built-in module specifier keys, including with a "/" assert_equals: expected "{\"std:blank\":[\"https://base.example/blank\"],\"std:blank/\":[\"https://base.example/blank/\"],\"std:blank/foo\":[\"https://base.example/blank/foo\"],\"std:blank\\\foo\":[\"https://base.example/blank/backslashfoo\"]}" but got "{\"std:blank\":[\"https://base.example/blank\"],\"std:blank\\foo\":[\"https://base.example/blank/backslashfoo\"],\"std:blank/\":[\"https://base.example/blank/\"],\"std:blank/foo\":[\"https://base.example/blank/foo\"]}"
 Harness: the test ran to completion.