gin: ModuleRegistry::LoadModule() shouldn't extend the ModuleRegistry unsatisfied_dependencies set for a pending module.

BUG=

Review URL: https://codereview.chromium.org/759003002

Cr-Commit-Position: refs/heads/master@{#305700}

2 files changed, 34 insertions, 0 deletions

diff --git a/gin/modules/module_registry.cc b/gin/modules/module_registry.cc
index a92a5461..8341337 100644
--- a/gin/modules/module_registry.cc
+++ b/gin/modules/module_registry.cc
@@ -168,6 +168,12 @@ void ModuleRegistry::LoadModule(Isolate* isolate,
     return;
   }
   waiting_callbacks_.insert(std::make_pair(id, callback));
+
+  for (size_t i = 0; i < pending_modules_.size(); ++i) {
+    if (pending_modules_[i]->id == id)
+      return;
+  }
+
   unsatisfied_dependencies_.insert(id);
 }
 
diff --git a/gin/modules/module_registry_unittest.cc b/gin/modules/module_registry_unittest.cc
index 3d784dc..ed725fc 100644
--- a/gin/modules/module_registry_unittest.cc
+++ b/gin/modules/module_registry_unittest.cc
@@ -70,6 +70,10 @@ void OnModuleLoaded(TestHelper* helper,
       ->LoadModule(isolate, "two", base::Bind(NestedCallback));
 }
 
+void OnModuleLoadedNoOp(v8::Handle<v8::Value> value) {
+  ASSERT_TRUE(value->IsNumber());
+}
+
 }  // namespace
 
 typedef V8Test ModuleRegistryTest;
@@ -133,4 +137,28 @@ TEST_F(ModuleRegistryTest, LoadModuleTest) {
   EXPECT_EQ(3, counter);
 }
 
+// Verifies that explicitly loading a module that's already pending does
+// not cause the ModuleRegistry's unsatisfied_dependency set to grow.
+TEST_F(ModuleRegistryTest, UnsatisfiedDependenciesTest) {
+  TestHelper helper(instance_->isolate());
+  std::string source =
+      "define('one', ['no_such_module'], function(nsm) {"
+      "  return 1;"
+      "});";
+  ModuleRegistry* registry =
+    ModuleRegistry::From(helper.runner->GetContextHolder()->context());
+
+  std::set<std::string> no_such_module_set;
+  no_such_module_set.insert("no_such_module");
+
+  // Adds one unsatisfied dependency on "no-such-module".
+  helper.runner->Run(source, "script");
+  EXPECT_EQ(no_such_module_set, registry->unsatisfied_dependencies());
+
+  // Should have no effect on the unsatisfied_dependencies set.
+  ModuleRegistry::LoadModuleCallback callback = base::Bind(OnModuleLoadedNoOp);
+  registry->LoadModule(instance_->isolate(), "one", callback);
+  EXPECT_EQ(no_such_module_set, registry->unsatisfied_dependencies());
+}
+
 }  // namespace gin