GN: Schedule config loads for sub-configs.

Previously the Builder did not schedule file loads for configs of configs.

This meant that if a BUILD file contained only configs referenced by other
configs (rather than configs referenced by targets, or targets referenced by
other targets), it would never be loaded. The result will be a failed run with
an undefined object.

BUG=536844

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

Cr-Original-Commit-Position: refs/heads/master@{#351089}
Cr-Mirrored-From: https://chromium.googlesource.com/chromium/src
Cr-Mirrored-Commit: 1b1e7e8c8f3105b6c9811ff28947961eb96153c3

tools/gn/builder.cc

diff --git a/tools/gn/builder.cc b/tools/gn/builder.cc
index fbd4d57..3a0c1f5 100644
--- a/tools/gn/builder.cc
+++ b/tools/gn/builder.cc
@@ -240,6 +240,17 @@
   Config* config = record->item()->AsConfig();
   if (!AddDeps(record, config->configs(), err))
     return false;
+
+  // Make sure all deps of this config are scheduled to be loaded. For other
+  // item types like targets, the "should generate" flag is propogated around
+  // to mark whether this should happen. We could call
+  // RecursiveSetShouldGenerate to do this step here, but since configs nor
+  // anything they depend on is actually written, the "generate" flag isn't
+  // relevant and means extra book keeping. Just force load any deps of this
+  // config.
+  for (const auto& cur : record->all_deps())
+    ScheduleItemLoadIfNecessary(cur);
+
   return true;
 }
 

tools/gn/builder_unittest.cc

diff --git a/tools/gn/builder_unittest.cc b/tools/gn/builder_unittest.cc
index ffd1a9d..a5356a2 100644
--- a/tools/gn/builder_unittest.cc
+++ b/tools/gn/builder_unittest.cc
@@ -4,6 +4,7 @@
 
 #include "testing/gtest/include/gtest/gtest.h"
 #include "tools/gn/builder.h"
+#include "tools/gn/config.h"
 #include "tools/gn/loader.h"
 #include "tools/gn/target.h"
 #include "tools/gn/test_with_scope.h"
@@ -32,8 +33,17 @@
     return files_.empty();
   }
 
-  // Returns true if two loads have been requested and they match the given
-  // file. This will clear the records so it will be empty for the next call.
+  // Returns true if one/two loads have been requested and they match the given
+  // file(s). This will clear the records so it will be empty for the next call.
+  bool HasLoadedOne(const SourceFile& file) {
+    if (files_.size() != 1u) {
+      files_.clear();
+      return false;
+    }
+    bool match = (files_[0] == file);
+    files_.clear();
+    return match;
+  }
   bool HasLoadedTwo(const SourceFile& a, const SourceFile& b) {
     if (files_.size() != 2u) {
       files_.clear();
@@ -216,3 +226,24 @@
   // It should have gotten pushed to B.
   EXPECT_TRUE(b_record->should_generate());
 }
+
+// Tests that configs applied to a config get loaded (bug 536844).
+TEST_F(BuilderTest, ConfigLoad) {
+  SourceDir toolchain_dir = settings_.toolchain_label().dir();
+  std::string toolchain_name = settings_.toolchain_label().name();
+
+  // Construct a dependency chain: A -> B -> C. Define A first with a
+  // forward-reference to B, then C, then B to test the different orders that
+  // the dependencies are hooked up.
+  Label a_label(SourceDir("//a/"), "a", toolchain_dir, toolchain_name);
+  Label b_label(SourceDir("//b/"), "b", toolchain_dir, toolchain_name);
+  Label c_label(SourceDir("//c/"), "c", toolchain_dir, toolchain_name);
+
+  // The builder will take ownership of the pointers.
+  Config* a = new Config(&settings_, a_label);
+  a->configs().push_back(LabelConfigPair(b_label));
+  builder_->ItemDefined(scoped_ptr<Item>(a));
+
+  // Should have requested that B is loaded.
+  EXPECT_TRUE(loader_->HasLoadedOne(SourceFile("//b/BUILD.gn")));
+}