[ES6 modules] Cosmetic changes to ModuleTreeLinker

third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp

 // Copyright 2017 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "core/loader/modulescript/ModuleTreeLinker.h"
 
 #include "bindings/core/v8/ScriptModule.h"
 #include "core/dom/AncestorList.h"
 #include "core/dom/ModuleScript.h"
 #include "core/loader/modulescript/ModuleScriptFetchRequest.h"
 #include "core/loader/modulescript/ModuleTreeLinkerRegistry.h"
 #include "platform/loader/fetch/ResourceLoadingLog.h"
 #include "platform/wtf/HashSet.h"

[nhiroki, 2017/04/20 09:06:25]

"HashSet.h" may be unused (HeapHashSet is defined in HeapAllocator.h)

Can you add Vector.h?

 
 namespace blink {
 
 ModuleTreeLinker* ModuleTreeLinker::Fetch(
     const ModuleScriptFetchRequest& request,
     const AncestorList& ancestor_list,
     ModuleGraphLevel level,
     Modulator* modulator,
     ModuleTreeLinkerRegistry* registry,
     ModuleTreeClient* client) {
@@ skipping 100 matching lines @@
   // Step 1. Fetch a single module script given url, fetch client settings
   // object, destination, cryptographic nonce, parser state, credentials mode,
   // module map settings object, referrer, and the top-level module fetch flag.
   // If the caller of this algorithm specified custom perform the fetch steps,
   // pass those along while fetching a single module script.
   AdvanceState(State::kFetchingSelf);
   modulator_->FetchSingle(request, level, this);
 
   // Step 2. Return from this algorithm, and run the following steps when
   // fetching a single module script asynchronously completes with result.
-  // Note: Modulator::fetchSingle asynchronously notifies result to
+  // Note: Modulator::FetchSingle asynchronously notifies result to
   // ModuleTreeLinker::notifyModuleLoadFinished().
 }
 
 void ModuleTreeLinker::NotifyModuleLoadFinished(ModuleScript* module_script) {
   // https://html.spec.whatwg.org/multipage/webappapis.html#internal-module-script-graph-fetching-procedure
 
   // Step 3. "If result is null, ..."
   if (!module_script) {
     // "asynchronously complete this algorithm with null and abort these steps."
-    // Note: The return variable for "internal module script graph fetching
-    // procedure" is descendants_module_script_ per Step 8.
+    // Note: We return null by calling AdvanceState(), which calls
+    // NotifyModuleTreeLoadFinished(descendants_module_script_), and in this
+    // case |descendants_module_script_| is always null)

[hiroshige, 2017/04/20 09:38:47]

small nit: extra ')' at the end of the line.

     DCHECK(!descendants_module_script_);
     AdvanceState(State::kFinished);
     return;
   }
 
   // Step 4. Otherwise, result is a module script. Fetch the descendants of
   // result given destination and an ancestor list obtained by appending url to
   // ancestor list. Wait for fetching the descendants of a module script to
   // asynchronously complete with descendants result before proceeding to the
   // next step.
   module_script_ = module_script;
 
   FetchDescendants();
 
-  // Note: Step 5- continues in instantiate() method, after
+  // Note: Step 5- continues in Instantiate() method, after
   // "fetch the descendants of a module script" procedure completes.
 }
 
 class ModuleTreeLinker::DependencyModuleClient
     : public GarbageCollectedFinalized<
           ModuleTreeLinker::DependencyModuleClient>,
       public ModuleTreeClient {
   USING_GARBAGE_COLLECTED_MIXIN(ModuleTreeLinker::DependencyModuleClient);
 
  public:
   static DependencyModuleClient* Create(ModuleTreeLinker* module_tree_linker) {
     return new DependencyModuleClient(module_tree_linker);
   }
   virtual ~DependencyModuleClient() = default;
 
   DEFINE_INLINE_TRACE() {
     visitor->Trace(module_tree_linker_);
     ModuleTreeClient::Trace(visitor);
   }
 
  private:
-  DependencyModuleClient(ModuleTreeLinker* module_tree_linker)
+  explicit DependencyModuleClient(ModuleTreeLinker* module_tree_linker)
       : module_tree_linker_(module_tree_linker) {
     CHECK(module_tree_linker);
   }
 
   // Implements ModuleTreeClient
   void NotifyModuleTreeLoadFinished(ModuleScript*) override;
 
   Member<ModuleTreeLinker> module_tree_linker_;
 };
 
 void ModuleTreeLinker::FetchDescendants() {
   CHECK(module_script_);
   AdvanceState(State::kFetchingDependencies);
 
   // https://html.spec.whatwg.org/multipage/webappapis.html#fetch-the-descendants-of-a-module-script
 
   // Step 1. Let record be module script's module record.
   ScriptModule record = module_script_->Record();
 
   // Step 2. If record.[[RequestedModules]] is empty, asynchronously complete
   // this algorithm with module script.
   Vector<String> module_requests =
       modulator_->ModuleRequestsFromScriptModule(record);
+  if (module_requests.IsEmpty()) {
+    // Continue to Instantiate() to process "internal module script graph
+    // fetching procedure" Step 5-.
+    descendants_module_script_ = module_script_;
+    Instantiate();
+    return;
+  }
 
   // Step 3. Let urls be a new empty list.
-  HashSet<KURL> urls;
+  Vector<KURL> urls;
 
   // Step 4. For each string requested of record.[[RequestedModules]],
   for (const auto& module_request : module_requests) {
     // Step 4.1. Let url be the result of resolving a module specifier given
     // module script and requested.
     KURL url = Modulator::ResolveModuleSpecifier(module_request,
                                                  module_script_->BaseURL());
 
     // Step 4.2. If the result is error: ...
     if (url.IsNull()) {
@@ skipping 12 matching lines @@
       return;
     }
 
     // Step 4.3. Otherwise, if ancestor list does not contain url, append url to
     // urls.
 
     // Modulator::resolveModuleSpecifier() impl must return either a valid url
     // or null.
     CHECK(url.IsValid());
     if (!ancestor_list_with_url_.Contains(url))
-      urls.insert(url);
+      urls.push_back(url);
   }
 
   // Step 5. For each url in urls, perform the internal module script graph
   // fetching procedure given url, module script's credentials mode, module
   // script's cryptographic nonce, module script's parser state, destination,
   // module script's settings object, module script's settings object, ancestor
   // list, module script's base URL, and with the top-level module fetch flag
   // unset. If the caller of this algorithm specified custom perform the fetch
   // steps, pass those along while performing the internal module script graph
   // fetching procedure.
   // TODO(kouhei): handle "destination".
+  DCHECK(!urls.IsEmpty());
   CHECK_EQ(num_incomplete_descendants_, 0u);
-  if (urls.IsEmpty()) {
-    // Continue to instantiate() to process "internal module script graph
-    // fetching procedure" Step 5-.
-    descendants_module_script_ = module_script_;
-    Instantiate();
-    return;
-  }
   num_incomplete_descendants_ = urls.size();
   for (const KURL& url : urls) {
     DependencyModuleClient* dependency_client =
         DependencyModuleClient::Create(this);
     dependency_clients_.insert(dependency_client);
 
     ModuleScriptFetchRequest request(url, module_script_->Nonce(),
                                      module_script_->ParserState(),
                                      module_script_->CredentialsMode(),
                                      module_script_->BaseURL().GetString());
     modulator_->FetchTreeInternal(request, ancestor_list_with_url_,
                                   ModuleGraphLevel::kDependentModuleFetch,
                                   dependency_client);
   }
 
   // Asynchronously continue processing after notifyOneDescendantFinished() is
   // called m_numIncompleteDescendants times.
   CHECK_GT(num_incomplete_descendants_, 0u);
 }
 
 void ModuleTreeLinker::DependencyModuleClient::NotifyModuleTreeLoadFinished(
     ModuleScript* module_script) {
   DescendantLoad was_success =
-      !!module_script ? DescendantLoad::kSuccess : DescendantLoad::kFailed;
+      module_script ? DescendantLoad::kSuccess : DescendantLoad::kFailed;
   module_tree_linker_->NotifyOneDescendantFinished(was_success);
 }
 
 void ModuleTreeLinker::NotifyOneDescendantFinished(DescendantLoad was_success) {
   CHECK(!descendants_module_script_);
 
   if (state_ == State::kFinished) {
     // We may reach here if one of the descendant failed to load, and the other
     // descendants fetches were in flight.
     return;
   }
   CHECK_EQ(state_, State::kFetchingDependencies);
 
   CHECK_GT(num_incomplete_descendants_, 0u);
   --num_incomplete_descendants_;
 
   // TODO(kouhei): Potential room for optimization. Cancel inflight descendants
   // fetch if a descendant load failed.
 
   CHECK(module_script_);
   RESOURCE_LOADING_DVLOG(1)
       << "ModuleTreeLinker[" << this << "]::NotifyOneDescendantFinished with "
       << (was_success == DescendantLoad::kSuccess ? "success. " : "failure. ")
       << num_incomplete_descendants_ << " remaining descendants.";
 
   // https://html.spec.whatwg.org/multipage/webappapis.html#fetch-the-descendants-of-a-module-script
   // Step 5. "... If any of them asynchronously complete with null, then
-  // asynchronously complete this algorithm with null"
+  // asynchronously complete this algorithm with null ..."
   if (was_success == DescendantLoad::kFailed) {
     DCHECK(!descendants_module_script_);
     // Note: while we complete "fetch the descendants of a module script"
     //       algorithm here, we still need to continue to the rest of the steps
     //       in "internal module script graph fetching procedure"
     Instantiate();
     return;
   }
 
   // Step 5. "Wait for all of the internal module script graph fetching
-  // procedure invocations to asynchronously complete..."
+  // procedure invocations to asynchronously complete. ... Otherwise,
+  // asynchronously complete this algorithm with module script."
   if (!num_incomplete_descendants_) {
     descendants_module_script_ = module_script_;
     Instantiate();
-    return;
   }
 }
 
 void ModuleTreeLinker::Instantiate() {
   CHECK(module_script_);
   AdvanceState(State::kInstantiating);
 
   // https://html.spec.whatwg.org/multipage/webappapis.html#internal-module-script-graph-fetching-procedure
 
   // Step 5. Let record be result's module record.
@@ skipping 26 matching lines @@
   AdvanceState(State::kFinished);
 }
 
 HeapHashSet<Member<ModuleScript>>
 ModuleTreeLinker::UninstantiatedInclusiveDescendants() {
   // https://html.spec.whatwg.org/multipage/webappapis.html#uninstantiated-inclusive-descendant-module-scripts
   // Step 1. Let moduleMap be script's settings object's module map.
   // Note: Modulator is our "settings object".
   // Note: We won't reference the ModuleMap directly here to aid testing.
 
-  // Step 2. Let stack be the stack « script ».
+  // Step 2. Let stack be the stack << script >>.
   // TODO(kouhei): Make stack a HeapLinkedHashSet for O(1) lookups.
   HeapDeque<Member<ModuleScript>> stack;
   stack.push_front(module_script_);
 
   // Step 3. Let inclusive descendants be an empty set.
   // Note: We use unordered set here as the order is not observable from web
   // platform.
   // This is allowed per spec: https://infra.spec.whatwg.org/#sets
   HeapHashSet<Member<ModuleScript>> inclusive_descendants;
 
@@ skipping 39 matching lines @@
     for (const auto& child_url : child_urls) {
       ModuleScript* module_script =
           modulator_->GetFetchedModuleScript(child_url);
 
       child_modules.push_back(module_script);
     }
     // Step 4.3.5. For each s of child modules:
     for (const auto& s : child_modules) {
       // Step 4.3.5.2. If s is null, continue.
       // Note: We do null check first, as Blink HashSet can't contain nullptr.
+
       // Step 4.3.5.3. Assert: s is a module script (i.e., it is not "fetching",
       // since by this point all child modules must have been fetched). Note:
       // GetFetchedModuleScript returns nullptr if "fetching"
       if (!s)
         continue;
 
       // Step 4.3.5.1. If inclusive descendants already contains s, continue.
       if (inclusive_descendants.Contains(s))
         continue;
 
-      // Step 4.3.5.4. Push s onto satck.
+      // Step 4.3.5.4. Push s onto stack.
       stack.push_front(s);
     }
   }
 
   // Step 5. Return a set containing all items of inclusive descendants whose
   // instantiation state is "uninstantiated".
   HeapHashSet<Member<ModuleScript>> uninstantiated_set;
   for (const auto& script : inclusive_descendants) {
     if (script->InstantiationState() ==
         ModuleInstantiationState::kUninstantiated)
       uninstantiated_set.insert(script);
   }
   return uninstantiated_set;
 }
 
 }  // namespace blink