[ES6 modules] ModuleTreeLinker::FetchDescendants should not assume urls isn't empty.

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

Index: third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp
diff --git a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp
index e032e1043e7083354420fdd0d8f10eeabe6c255d..982b572bbb0a7cddfa30ca457a75abf023e4b31f 100644
--- a/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp
+++ b/third_party/WebKit/Source/core/loader/modulescript/ModuleTreeLinker.cpp
@@ -233,20 +233,15 @@ void ModuleTreeLinker::FetchDescendants() {
 
   // 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;
-  }
+  // Note: We defer this bail-out until Step 5. Step 4 will be no-op anyway if
+  //       record.[[RequestedModules]] is empty.
 
   // Step 3. Let urls be a new empty list.
   Vector<KURL> urls;
 
   // Step 4. For each string requested of record.[[RequestedModules]],
+  Vector<String> module_requests =
+      modulator_->ModuleRequestsFromScriptModule(record);
   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.
@@ -288,8 +283,20 @@ void ModuleTreeLinker::FetchDescendants() {
   // 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());
+
+  if (urls.IsEmpty()) {
+    // Step 5, when "urls" is empty, or...

[hiroshige, 2017/05/01 18:00:49]

Could you clarify this (Line 288, and Line 299) is not a text from the spec?

+    // Step 2. If record.[[RequestedModules]] is empty, asynchronously complete
+    // this algorithm with module script.

[kinuko, 2017/05/01 22:27:02]

If line 289 is true line 288 should be true too, so connecting these by 'or...' 
seems a bit strange to me.

'If "urls" is empty, that happens when record.[[RequestedModules]] is empty in
Step 2, or when resolved URL was already in the ancestor list, asynchronously
complete this algorithm with module script'

? (If these are not exact spec text)

[hiroshige, 2017/05/01 23:16:12]

[optional] My preference is e.g.:
  Step 2. If record.[[RequestedModules]] is empty, asynchronously
  complete this algorithm with module script. [spec text]
  Also, if record.[[RequestedModules]] is not empty but |urls| is
  empty here, we can immediately proceed to Instantiate() because
  we don't have to do or wait for anything in Step 5. [non-spec text]
(and perhaps placing this if block before Line 278).

[kouhei (in TOK), 2017/05/02 01:21:29]

Done.

+
+    // Continue to Instantiate() to process "internal module script graph
+    // fetching procedure" Step 5-.
+    descendants_module_script_ = module_script_;
+    Instantiate();
+    return;
+  }
+
+  // Step 5, when "urls" is non-empty.
   CHECK_EQ(num_incomplete_descendants_, 0u);
   num_incomplete_descendants_ = urls.size();
   for (const KURL& url : urls) {