[Import Maps] Add spec comments and TODOs

As a preparation for updating the behavior according to the
draft spec, this CL adds spec comments and TODOs,
with some renaming and
aligning method boundaries with the spec concepts.

This CL doesn't change the behavior.

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

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

@@ -15,6 +15,7 @@
 namespace blink {
 
 class ConsoleLogger;
+class JSONObject;
 class Modulator;
 class ParsedSpecifier;
 
@@ -30,23 +31,43 @@ class ImportMap final : public GarbageCollectedFinalized<ImportMap> {
 
   ImportMap(const Modulator&, const HashMap<String, Vector<KURL>>& imports);
 
+  // https://wicg.github.io/import-maps/#resolve-an-imports-match
   // Returns nullopt when not mapped by |this| import map (i.e. the import map
   // doesn't have corresponding keys).
   // Returns a null URL when resolution fails.
-  base::Optional<KURL> Resolve(const ParsedSpecifier&,
-                               String* debug_message) const;
+  base::Optional<KURL> ResolveImportsMatch(const ParsedSpecifier&,
+                                           String* debug_message) const;
 
   String ToString() const;
 
   void Trace(Visitor*);
 
  private:
-  using MatchResult = HashMap<String, Vector<KURL>>::const_iterator;
-  base::Optional<MatchResult> Match(const ParsedSpecifier&) const;
-  base::Optional<MatchResult> MatchExact(const ParsedSpecifier&) const;
+  // <spec href="https://wicg.github.io/import-maps/#specifier-map">A specifier
+  // map is an ordered map from strings to lists of URLs.</spec>
+  //
+  // In Blink, we actually use an unordered map here, and related algorithms
+  // are implemented differently from the spec.
+  using SpecifierMap = HashMap<String, Vector<KURL>>;
+
+  using MatchResult = SpecifierMap::const_iterator;
+
   base::Optional<MatchResult> MatchPrefix(const ParsedSpecifier&) const;
+  static SpecifierMap SortAndNormalizeSpecifierMap(const JSONObject* imports,
+                                                   const KURL& base_url,
+                                                   ConsoleLogger&);
+
+  base::Optional<KURL> ResolveImportsMatchInternal(
+      const String& normalizedSpecifier,
+      const MatchResult&,
+      String* debug_message) const;
+
+  // https://wicg.github.io/import-maps/#import-map-imports
+  SpecifierMap imports_;
+
+  // TODO(crbug.com/927181): Implement
+  // https://wicg.github.io/import-maps/#import-map-scopes.
 
-  HashMap<String, Vector<KURL>> imports_;
   Member<const Modulator> modulator_for_built_in_modules_;
 };
 

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

@@ -164,8 +164,8 @@ KURL ModulatorImplBase::ResolveModuleSpecifier(const String& specifier,
   base::Optional<KURL> mapped_url;
   if (import_map_) {
     String import_map_debug_message;
-    mapped_url =
-        import_map_->Resolve(parsed_specifier, &import_map_debug_message);
+    mapped_url = import_map_->ResolveImportsMatch(parsed_specifier,
+                                                  &import_map_debug_message);
 
     // Output the resolution log. This is too verbose to be always shown, but
     // will be helpful for Web developers (and also Chromium developers) for

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

@@ -7,34 +7,63 @@
 namespace blink {
 
 // <specdef href="https://html.spec.whatwg.org/#resolve-a-module-specifier">
+// <specdef label="import-specifier"
+// href="https://wicg.github.io/import-maps/#parse-a-url-like-import-specifier">
+// This can return a kBare ParsedSpecifier for cases where the spec concepts
+// listed above should return failure/null. The users of ParsedSpecifier should
+// handle kBare cases properly, depending on contexts and whether import maps
+// are enabled.
 ParsedSpecifier ParsedSpecifier::Create(const String& specifier,
                                         const KURL& base_url) {
   // <spec step="1">Apply the URL parser to specifier. If the result is not
   // failure, return the result.</spec>
+  //
+  // <spec label="import-specifier" step="2">Let url be the result of parsing
+  // specifier (with no base URL).</spec>
   KURL url(NullURL(), specifier);
-  if (url.IsValid())
+  if (url.IsValid()) {
+    // <spec label="import-specifier" step="4">If url’s scheme is either a fetch
+    // scheme or "std", then return url.</spec>
+    //
+    // TODO(hiroshige): This check is done in the callers of ParsedSpecifier.
     return ParsedSpecifier(url);
+  }
 
   // <spec step="2">If specifier does not start with the character U+002F
   // SOLIDUS (/), the two-character sequence U+002E FULL STOP, U+002F SOLIDUS
   // (./), or the three-character sequence U+002E FULL STOP, U+002E FULL STOP,
   // U+002F SOLIDUS (../), return failure.</spec>
+  //
+  // <spec label="import-specifier" step="1">If specifier starts with "/", "./",
+  // or "../", then:</spec>
   if (!specifier.StartsWith("/") && !specifier.StartsWith("./") &&
       !specifier.StartsWith("../")) {
     // Do not consider an empty specifier as a valid bare specifier.
+    //
+    // <spec
+    // href="https://wicg.github.io/import-maps/#normalize-a-specifier-key"
+    // step="1">If specifierKey is the empty string, then:</spec>
     if (specifier.IsEmpty())
       return ParsedSpecifier();
 
+    // <spec label="import-specifier" step="3">If url is failure, then return
+    // null.</spec>
     return ParsedSpecifier(specifier);
   }
 
   // <spec step="3">Return the result of applying the URL parser to specifier
   // with base URL as the base URL.</spec>
+  //
+  // <spec label="import-specifier" step="1.1">Let url be the result of parsing
+  // specifier with baseURL as the base URL.</spec>
   DCHECK(base_url.IsValid());
   KURL absolute_url(base_url, specifier);
+  // <spec label="import-specifier" step="1.3">Return url.</spec>
   if (absolute_url.IsValid())
     return ParsedSpecifier(absolute_url);
 
+  // <spec label="import-specifier" step="1.2">If url is failure, then return
+  // null.</spec>
   return ParsedSpecifier();
 }