Improve GN header checker, make "check" on //ui/* pass.

I went through and made "gn check //ui/*" pass, and did a lot of enhancements for the bugs I found.

Fixes a big bug in the header checker where it did not consider direct private dependencies to be OK from a header include perspective. However, private deps only change things when there are intermediate targets. This patch marks direct deps OK, and changes the variable names from is_public to is_permitted (since it may not actually be public).

Allow includes to be permitted if any target allows the include, rather than all of them. This happens if multiple targets have the same file as sources.

Strip the generated file directory from the beginning of sources. Previously, this was only for outputs (since typically they're included assuming the root gen dir is on the path). This comes up when an action generates a file, and then it's put into a source set (which we do a lot). This change allows a public dependency on the source set to count for header include purposes.

Track public/private deps in the dependency path finder so the error message can show which deps are private that break the chain. This was really helpful when tracking down errors.

Add deps and public deps to the build to make check pass.

R=scottmg@chromium.org

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

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

base/test/BUILD.gn

@@ -101,11 +101,14 @@ source_set("test_support") {
     "values_test_util.cc",
     "values_test_util.h",
   ]
-  deps = [
+
+  public_deps = [
     "//base",
-    "//base/third_party/dynamic_annotations",
     "//base:i18n",
     "//base:base_static",
+  ]
+  deps = [
+    "//base/third_party/dynamic_annotations",
     "//testing/gmock",
     "//testing/gtest",
     "//third_party/libxml"

build/config/BUILDCONFIG.gn

@@ -550,6 +550,8 @@ template("component") {
       if (defined(invoker.output_extension)) { output_extension = invoker.output_extension }
       if (defined(invoker.output_name)) { output_name = invoker.output_name }
       if (defined(invoker.public)) { public = invoker.public }
+      if (defined(invoker.public_configs)) { public_configs = invoker.public_configs }
+      if (defined(invoker.public_deps)) { public_deps = invoker.public_deps }
       if (defined(invoker.sources)) { sources = invoker.sources }
       if (defined(invoker.testonly)) { testonly = invoker.testonly }
       if (defined(invoker.visibility)) { visibility = invoker.visibility }
@@ -584,6 +586,8 @@ template("component") {
       if (defined(invoker.output_extension)) { output_extension = invoker.output_extension }
       if (defined(invoker.output_name)) { output_name = invoker.output_name }
       if (defined(invoker.public)) { public = invoker.public }
+      if (defined(invoker.public_configs)) { public_configs = invoker.public_configs }
+      if (defined(invoker.public_deps)) { public_deps = invoker.public_deps }
       if (defined(invoker.sources)) { sources = invoker.sources }
       if (defined(invoker.testonly)) { testonly = invoker.testonly }
       if (defined(invoker.visibility)) { visibility = invoker.visibility }
@@ -632,6 +636,8 @@ template("test") {
       if (defined(invoker.output_extension)) { output_extension = invoker.output_extension }
       if (defined(invoker.output_name)) { output_name = invoker.output_name }
       if (defined(invoker.public)) { public = invoker.public }
+      if (defined(invoker.public_configs)) { public_configs = invoker.public_configs }
+      if (defined(invoker.public_deps)) { public_deps = invoker.public_deps }
       if (defined(invoker.sources)) { sources = invoker.sources }
       if (defined(invoker.visibility)) { visibility = invoker.visibility }
     }
@@ -667,6 +673,8 @@ template("test") {
       if (defined(invoker.output_extension)) { output_extension = invoker.output_extension }
       if (defined(invoker.output_name)) { output_name = invoker.output_name }
       if (defined(invoker.public)) { public = invoker.public }
+      if (defined(invoker.public_configs)) { public_configs = invoker.public_configs }
+      if (defined(invoker.public_deps)) { public_deps = invoker.public_deps }
       if (defined(invoker.sources)) { sources = invoker.sources }
       if (defined(invoker.visibility)) { visibility = invoker.visibility }
     }

cc/BUILD.gn

@@ -624,6 +624,9 @@ source_set("test_support") {
     "test",
   ]
 
+  public_deps = [
+    ":cc",
+  ]
   deps = [
     "//base",
     "//base/third_party/dynamic_annotations",

content/shell/BUILD.gn

@@ -203,6 +203,15 @@ static_library("content_shell_lib") {
 
   defines = [ "CONTENT_SHELL_VERSION=\"$content_shell_version\"" ]
 
+  public_deps = [
+    # This exposes all public content APIs.
+    "//content/public/app:both",
+    "//content/public/browser",
+    "//content/public/common",
+    "//content/public/plugin",
+    "//content/public/renderer",
+    "//content/public/utility",
+  ]
   deps = [
     ":resources",
     "//base",
@@ -216,12 +225,6 @@ static_library("content_shell_lib") {
     "//content/app/resources",
     "//content/app/strings",
     "//content/gpu",
-    "//content/public/app:both",
-    "//content/public/browser",
-    "//content/public/common",
-    "//content/public/plugin",
-    "//content/public/renderer",
-    "//content/public/utility",
     "//content/test:layouttest_support",
     "//gin",
     "//gpu",

extensions/generated_extensions_api.gni

@@ -176,13 +176,12 @@ template("generated_extensions_api") {
   source_set(target_name) {
     sources = []
     deps = []
-    
+    public_deps = []
+
     if (schemas) {
       sources += get_target_outputs(":$schema_generator_name")
-      deps += [
-        ":$schema_generator_name",
-        "//tools/json_schema_compiler:generated_api_util",
-      ]
+      public_deps += [ ":$schema_generator_name" ]
+      deps += [ "//tools/json_schema_compiler:generated_api_util" ]
     }
 
     if (bundle) {

sync/BUILD.gn

@@ -5,11 +5,10 @@
 import("//build/config/features.gni")
 
 component("sync") {
-  deps = [
+  public_deps = [
     ":sync_core",
     "//sync/protocol",
   ]
-  forward_dependent_configs_from = [ "//sync/protocol" ]
 }
 
 # GYP version: sync/sync.gyp:sync_core

tools/gn/header_checker.h

@@ -30,6 +30,23 @@ class MessageLoop;
 
 class HeaderChecker : public base::RefCountedThreadSafe<HeaderChecker> {
  public:
+  // Represents a dependency chain.
+  struct ChainLink {
+    ChainLink() : target(NULL), is_public(false) {}
+    ChainLink(const Target* t, bool p) : target(t), is_public(p) {}
+
+    const Target* target;
+
+    // True when the dependency on this target is public.
+    bool is_public;
+
+    // Used for testing.
+    bool operator==(const ChainLink& other) const {
+      return target == other.target && is_public == other.is_public;
+    }
+  };
+  typedef std::vector<ChainLink> Chain;
+
   HeaderChecker(const BuildSettings* build_settings,
                 const std::vector<const Target*>& targets);
 
@@ -49,12 +66,9 @@ class HeaderChecker : public base::RefCountedThreadSafe<HeaderChecker> {
  private:
   friend class base::RefCountedThreadSafe<HeaderChecker>;
   FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest, IsDependencyOf);
-  FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest,
-                           IsDependencyOf_ForwardsDirectDependentConfigs);
   FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest, CheckInclude);
+  FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest, PublicFirst);
   FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest, CheckIncludeAllowCircular);
-  FRIEND_TEST_ALL_PREFIXES(HeaderCheckerTest,
-                           GetDependentConfigChainProblemIndex);
   ~HeaderChecker();
 
   struct TargetInfo {
@@ -115,21 +129,25 @@ class HeaderChecker : public base::RefCountedThreadSafe<HeaderChecker> {
   // dependency chain from the dest target (chain[0] = search_for) to the src
   // target (chain[chain.size() - 1] = search_from).
   //
-  // Chains with public dependencies will be considered first. If a public
-  // match is found, *is_public will be set to true. A chain with non-public
-  // dependencies will only be considered if there are no public chains. In
-  // this case, *is_public will be false.
+  // Chains with permitted dependencies will be considered first. If a
+  // permitted match is found, *is_permitted will be set to true. A chain with
+  // indirect, non-public dependencies will only be considered if there are no
+  // public or direct chains. In this case, *is_permitted will be false.
+  //
+  // A permitted dependency is a sequence of public dependencies. The first
+  // one may be private, since a direct dependency always allows headers to be
+  // included.
   bool IsDependencyOf(const Target* search_for,
                       const Target* search_from,
-                      std::vector<const Target*>* chain,
-                      bool* is_public) const;
+                      Chain* chain,
+                      bool* is_permitted) const;
 
   // For internal use by the previous override of IsDependencyOf.  If
   // require_public is true, only public dependency chains are searched.
   bool IsDependencyOf(const Target* search_for,
                       const Target* search_from,
-                      bool require_public,
-                      std::vector<const Target*>* chain) const;
+                      bool require_permitted,
+                      Chain* chain) const;
 
   // Non-locked variables ------------------------------------------------------
   //

tools/gn/header_checker_unittest.cc

@@ -56,8 +56,8 @@ class HeaderCheckerTest : public testing::Test {
 
   TestWithScope setup_;
 
-  // Some headers that are automatically set up with a dependency chain.
-  // a -> b -> c
+  // Some headers that are automatically set up with a public dependency chain.
+  // a -> b -> c. D is unconnected.
   Target a_;
   Target b_;
   Target c_;
@@ -86,57 +86,58 @@ TEST_F(HeaderCheckerTest, IsDependencyOf) {
   a_.public_deps().push_back(LabelTargetPair(&p));
 
   // A does not depend on itself.
-  bool is_public = false;
-  std::vector<const Target*> chain;
-  EXPECT_FALSE(checker->IsDependencyOf(&a_, &a_, &chain, &is_public));
+  bool is_permitted = false;
+  HeaderChecker::Chain chain;
+  EXPECT_FALSE(checker->IsDependencyOf(&a_, &a_, &chain, &is_permitted));
 
-  // A depends on B.
+  // A depends publicly on B.
   chain.clear();
-  is_public = false;
-  EXPECT_TRUE(checker->IsDependencyOf(&b_, &a_, &chain, &is_public));
+  is_permitted = false;
+  EXPECT_TRUE(checker->IsDependencyOf(&b_, &a_, &chain, &is_permitted));
   ASSERT_EQ(2u, chain.size());
-  EXPECT_EQ(&b_, chain[0]);
-  EXPECT_EQ(&a_, chain[1]);
-  EXPECT_TRUE(is_public);
+  EXPECT_EQ(HeaderChecker::ChainLink(&b_, true), chain[0]);
+  EXPECT_EQ(HeaderChecker::ChainLink(&a_, true), chain[1]);
+  EXPECT_TRUE(is_permitted);
 
   // A indirectly depends on C. The "public" dependency path through B should
   // be identified.
   chain.clear();
-  is_public = false;
-  EXPECT_TRUE(checker->IsDependencyOf(&c_, &a_, &chain, &is_public));
+  is_permitted = false;
+  EXPECT_TRUE(checker->IsDependencyOf(&c_, &a_, &chain, &is_permitted));
   ASSERT_EQ(3u, chain.size());
-  EXPECT_EQ(&c_, chain[0]);
-  EXPECT_EQ(&b_, chain[1]);
-  EXPECT_EQ(&a_, chain[2]);
-  EXPECT_TRUE(is_public);
+  EXPECT_EQ(HeaderChecker::ChainLink(&c_, true), chain[0]);
+  EXPECT_EQ(HeaderChecker::ChainLink(&b_, true), chain[1]);
+  EXPECT_EQ(HeaderChecker::ChainLink(&a_, true), chain[2]);
+  EXPECT_TRUE(is_permitted);
 
   // C does not depend on A.
   chain.clear();
-  is_public = false;
-  EXPECT_FALSE(checker->IsDependencyOf(&a_, &c_, &chain, &is_public));
+  is_permitted = false;
+  EXPECT_FALSE(checker->IsDependencyOf(&a_, &c_, &chain, &is_permitted));
   EXPECT_TRUE(chain.empty());
-  EXPECT_FALSE(is_public);
+  EXPECT_FALSE(is_permitted);
 
-  // Add a private A -> C dependency. This should not be detected since it's
-  // private, even though it's shorter than the A -> B -> C one.
+  // Remove the B -> C public dependency, leaving P's private dep on C the only
+  // path from A to C. This should now be found.
   chain.clear();
-  is_public = false;
-  a_.private_deps().push_back(LabelTargetPair(&c_));
-  EXPECT_TRUE(checker->IsDependencyOf(&c_, &a_, &chain, &is_public));
-  EXPECT_EQ(&c_, chain[0]);
-  EXPECT_EQ(&b_, chain[1]);
-  EXPECT_EQ(&a_, chain[2]);
-  EXPECT_TRUE(is_public);
-
-  // Remove the B -> C public dependency, leaving A's private dep on C the only
-  // path. This should now be found.
-  chain.clear();
-  EXPECT_EQ(&c_, b_.public_deps()[0].ptr); // Validate it's the right one.
+  EXPECT_EQ(&c_, b_.public_deps()[0].ptr);  // Validate it's the right one.
   b_.public_deps().erase(b_.public_deps().begin());
-  EXPECT_TRUE(checker->IsDependencyOf(&c_, &a_, &chain, &is_public));
-  EXPECT_EQ(&c_, chain[0]);
-  EXPECT_EQ(&a_, chain[1]);
-  EXPECT_FALSE(is_public);
+  EXPECT_TRUE(checker->IsDependencyOf(&c_, &a_, &chain, &is_permitted));
+  EXPECT_EQ(3u, chain.size());
+  EXPECT_EQ(HeaderChecker::ChainLink(&c_, false), chain[0]);
+  EXPECT_EQ(HeaderChecker::ChainLink(&p, true), chain[1]);
+  EXPECT_EQ(HeaderChecker::ChainLink(&a_, true), chain[2]);
+  EXPECT_FALSE(is_permitted);
+
+  // P privately depends on C. That dependency should be OK since it's only
+  // one hop.
+  chain.clear();
+  is_permitted = false;
+  EXPECT_TRUE(checker->IsDependencyOf(&c_, &p, &chain, &is_permitted));
+  ASSERT_EQ(2u, chain.size());
+  EXPECT_EQ(HeaderChecker::ChainLink(&c_, false), chain[0]);
+  EXPECT_EQ(HeaderChecker::ChainLink(&p, true), chain[1]);
+  EXPECT_TRUE(is_permitted);
 }
 
 TEST_F(HeaderCheckerTest, CheckInclude) {
@@ -161,7 +162,6 @@ TEST_F(HeaderCheckerTest, CheckInclude) {
   c_.public_headers().push_back(c_public);
   c_.set_all_headers_public(false);
 
-  targets_.push_back(&d_);
   scoped_refptr<HeaderChecker> checker(
       new HeaderChecker(setup_.build_settings(), targets_));
 
@@ -190,6 +190,51 @@ TEST_F(HeaderCheckerTest, CheckInclude) {
   EXPECT_FALSE(err.has_error());
 }
 
+// A public chain of dependencies should always be identified first, even if
+// it is longer than a private one.
+TEST_F(HeaderCheckerTest, PublicFirst) {
+  // Now make a A -> Z -> D private dependency chain (one shorter than the
+  // public one to get to D).
+  Target z(setup_.settings(), Label(SourceDir("//a/"), "a"));
+  z.set_output_type(Target::SOURCE_SET);
+  Err err;
+  EXPECT_TRUE(z.SetToolchain(setup_.toolchain(), &err));
+  z.private_deps().push_back(LabelTargetPair(&d_));
+  EXPECT_TRUE(z.OnResolved(&err));
+  targets_.push_back(&z);
+
+  a_.private_deps().push_back(LabelTargetPair(&z));
+
+  // Check that D can be found from A, but since it's private, it will be
+  // marked as not permitted.
+  bool is_permitted = false;
+  HeaderChecker::Chain chain;
+  scoped_refptr<HeaderChecker> checker(
+      new HeaderChecker(setup_.build_settings(), targets_));
+  EXPECT_TRUE(checker->IsDependencyOf(&d_, &a_, &chain, &is_permitted));
+
+  EXPECT_FALSE(is_permitted);
+  ASSERT_EQ(3u, chain.size());
+  EXPECT_EQ(HeaderChecker::ChainLink(&d_, false), chain[0]);
+  EXPECT_EQ(HeaderChecker::ChainLink(&z, false), chain[1]);
+  EXPECT_EQ(HeaderChecker::ChainLink(&a_, true), chain[2]);
+
+  // Hook up D to the existing public A -> B -> C chain to make a long one, and
+  // search for D again.
+  c_.public_deps().push_back(LabelTargetPair(&d_));
+  checker = new HeaderChecker(setup_.build_settings(), targets_);
+  chain.clear();
+  EXPECT_TRUE(checker->IsDependencyOf(&d_, &a_, &chain, &is_permitted));
+
+  // This should have found the long public one.
+  EXPECT_TRUE(is_permitted);
+  ASSERT_EQ(4u, chain.size());
+  EXPECT_EQ(HeaderChecker::ChainLink(&d_, true), chain[0]);
+  EXPECT_EQ(HeaderChecker::ChainLink(&c_, true), chain[1]);
+  EXPECT_EQ(HeaderChecker::ChainLink(&b_, true), chain[2]);
+  EXPECT_EQ(HeaderChecker::ChainLink(&a_, true), chain[3]);
+}
+
 // Checks that the allow_circular_includes_from list works.
 TEST_F(HeaderCheckerTest, CheckIncludeAllowCircular) {
   InputFile input_file(SourceFile("//some_file.cc"));

ui/accelerometer/BUILD.gn

@@ -15,6 +15,7 @@ component("accelerometer") {
   defines = [ "UI_ACCELEROMETER_IMPLEMENTATION" ]
 
   deps = [
+    "//base",
     "//ui/gfx/geometry",
   ]
 }

ui/accessibility/BUILD.gn

@@ -34,9 +34,7 @@ component("accessibility") {
 
   defines = [ "ACCESSIBILITY_IMPLEMENTATION" ]
 
-  forward_dependent_configs_from = [ ":ax_gen" ]
-
-  deps = [
+  public_deps = [
     ":ax_gen",
     "//base",
     "//ui/gfx",

ui/aura/BUILD.gn

@@ -172,11 +172,12 @@ source_set("test_support") {
     "test/window_test_api.h",
   ]
 
-  deps = [
+  public_deps = [
     ":aura",
+  ]
+  deps = [
     "//skia",
     "//testing/gtest",
-    "//ui/base",
     "//ui/base:test_support",
     "//ui/compositor:test_support",
     "//ui/events",
@@ -248,7 +249,6 @@ executable("bench") {
   ]
 
   deps = [
-    ":aura",
     ":test_support",
     "//base",
     "//base:i18n",
@@ -283,18 +283,14 @@ test("unittests") {
   ]
 
   deps = [
-    ":aura",
     ":test_support",
     "//base/allocator",
     "//base/test:test_support",
     "//skia",
     "//testing/gtest",
-    "//ui/base",
     "//ui/base:test_support",
-    "//ui/compositor",
     "//ui/compositor:test_support",
-    "//ui/events",
-    "//ui/events:events_base",
+    "//ui/events:test_support",
     "//ui/events:gesture_detection",
     "//ui/gfx",
     "//ui/gfx/geometry",

ui/base/BUILD.gn

@@ -261,13 +261,6 @@ component("base") {
     "window_open_disposition.cc",
     "window_open_disposition.h",
     "work_area_watcher_observer.h",
-    "x/x11_foreign_window_manager.cc",
-    "x/x11_foreign_window_manager.h",
-    "x/x11_menu_list.cc",
-    "x/x11_menu_list.h",
-    "x/x11_util.cc",
-    "x/x11_util.h",
-    "x/x11_util_internal.h",
   ]
 
   if (is_win) {
@@ -285,26 +278,24 @@ component("base") {
 
   defines = [ "UI_BASE_IMPLEMENTATION" ]
 
-  deps = [
+  public_deps = [
     "//base",
-    "//base/third_party/dynamic_annotations",
-    "//base:base_static",
-    "//base:i18n",
-    "//net",
     "//skia",
-    "//third_party/icu",
     "//ui/events/platform",
     "//ui/events:events_base",
     "//ui/gfx",
     "//ui/gfx/geometry",
+  ]
+  deps = [
+    "//base/third_party/dynamic_annotations",
+    "//base:base_static",
+    "//base:i18n",
+    "//net",
+    "//third_party/icu",
     "//ui/resources",
     "//ui/strings",
     "//url",
   ]
-  # We expose these things in our headers:
-  forward_dependent_configs_from = [
-    "//ui/gfx",
-  ]
 
   if (build_ime) {
     if (!is_android) {
@@ -446,8 +437,8 @@ component("base") {
       "dragdrop/drag_utils_aura.cc",
     ]
   }
-  if (!use_x11) {
-    sources -= [
+  if (use_x11) {
+    sources += [
       "x/x11_foreign_window_manager.cc",
       "x/x11_foreign_window_manager.h",
       "x/x11_menu_list.cc",
@@ -456,6 +447,7 @@ component("base") {
       "x/x11_util.h",
       "x/x11_util_internal.h",
     ]
+    deps += [ "//ui/gfx/x" ]
   }
 
   if (!use_aura || !is_linux) {
@@ -666,8 +658,10 @@ source_set("test_support") {
     "test/ui_controls_win.cc",
   ]
 
-  deps = [
+  public_deps = [
     ":base",
+  ]
+  deps = [
     "//base",
     "//skia",
     "//testing/gtest",

ui/compositor/BUILD.gn

@@ -103,9 +103,11 @@ source_set("test_support") {
     "test/test_utils.h",
   ]
 
-  deps = [
+  public_deps = [
     ":compositor",
-    "//base",
+  ]
+  deps = [
+    "//base/test:test_support",
     "//cc",
     "//cc:test_support",
     "//skia",

ui/display/BUILD.gn

@@ -64,6 +64,9 @@ component("test_util") {
 
   defines = [ "DISPLAY_IMPLEMENTATION" ]
 
+  public_deps = [
+    ":display",
+  ]
   deps = [
     "//base",
     "//ui/gfx",
@@ -85,6 +88,8 @@ test("display_unittests") {
   ]
 
   deps = [
+    ":test_util",
+    "//base",
     "//base/test:run_all_unittests",
     "//testing/gtest",
     "//ui/display/util",
@@ -94,7 +99,6 @@ test("display_unittests") {
   if (is_chromeos) {
     deps += [
       ":display",
-      ":test_util",
       "//ui/display/types",
     ]
   }

ui/events/BUILD.gn

@@ -46,16 +46,17 @@ component("events_base") {
 
   deps = [
     ":dom4_keycode_converter",
-    "//base",
     "//base/third_party/dynamic_annotations",
     "//skia",
+  ]
+
+  public_deps = [
+    "//base",
     "//ui/events/platform",
     "//ui/gfx",
     "//ui/gfx/geometry",
   ]
 
-  forward_dependent_configs_from = [ "//ui/gfx" ]
-
   if (use_x11) {
     configs += [ "//build/config/linux:x11" ]
 
@@ -73,6 +74,8 @@ component("events_base") {
       "x/touch_factory_x11.cc",
       "x/touch_factory_x11.h",
     ]
+
+    deps += [ "//ui/gfx/x" ]
   }
 }
 
@@ -104,14 +107,15 @@ component("events") {
     "gestures/gesture_recognizer_impl_mac.cc",
     "gestures/gesture_types.h",
     "win/events_win.cc",
-    "x/events_x.cc",
   ]
 
   defines = [ "EVENTS_IMPLEMENTATION" ]
 
+  public_deps = [
+    ":events_base",
+  ]
   deps = [
     ":dom4_keycode_converter",
-    ":events_base",
     ":gesture_detection",
     "//base/third_party/dynamic_annotations",
     "//skia",
@@ -122,14 +126,12 @@ component("events") {
   forward_dependent_configs_from = [ ":events_base" ]
 
   if (use_x11) {
+    sources += [ "x/events_x.cc" ]
     configs += [
       "//build/config/linux:glib",
       "//build/config/linux:x11",
     ]
-  } else {
-    sources -= [
-      "x/events_x.cc",
-    ]
+    deps += [ "//ui/gfx/x" ]
   }
 
   if (!is_chromeos && is_linux) {
@@ -225,8 +227,6 @@ source_set("test_support") {
     "test/event_generator.h",
     "test/events_test_utils.cc",
     "test/events_test_utils.h",
-    "test/events_test_utils_x11.cc",
-    "test/events_test_utils_x11.h",
     "test/mock_motion_event.cc",
     "test/mock_motion_event.h",
     "test/platform_event_waiter.cc",
@@ -239,10 +239,12 @@ source_set("test_support") {
     "test/test_event_target.h",
   ]
 
-  deps = [
+  public_deps = [
     ":events",
     ":events_base",
     ":gesture_detection",
+  ]
+  deps = [
     "//base",
     "//skia",
     "//ui/events/platform",
@@ -256,11 +258,12 @@ source_set("test_support") {
     ]
   }
 
-  if (!use_x11) {
-    sources -= [
+  if (use_x11) {
+    sources += [
       "test/events_test_utils_x11.cc",
       "test/events_test_utils_x11.h",
     ]
+    deps += [ "//ui/gfx/x" ]
   }
 }
 

ui/events/platform/x11/BUILD.gn

@@ -12,15 +12,11 @@ component("x11") {
     "x11_event_source_libevent.cc",
   ]
 
-  defines = [
-    "EVENTS_IMPLEMENTATION",
-  ]
+  defines = [ "EVENTS_IMPLEMENTATION" ]
 
-  configs += [
-    "//build/config/linux:x11",
-  ]
+  configs += [ "//build/config/linux:x11" ]
 
-  deps = [
+  public_deps = [
     "//ui/events",
     "//ui/events:events_base",
     "//ui/events/platform",
@@ -28,16 +24,10 @@ component("x11") {
   ]
 
   if (is_linux) {
-    sources -= [
-      "x11_event_source_libevent.cc",
-    ]
+    sources -= [ "x11_event_source_libevent.cc" ]
 
-    configs += [
-      "//build/config/linux:glib",
-    ]
+    configs += [ "//build/config/linux:glib" ]
   } else {
-    sources -= [
-      "x11_event_source_glib.cc",
-    ]
+    sources -= [ "x11_event_source_glib.cc" ]
   }
 }

ui/gfx/BUILD.gn

@@ -221,19 +221,20 @@ component("gfx") {
 
   deps = [
     ":gfx_export",
-    "//base",
     "//base:i18n",
     "//base:base_static",
     "//base/third_party/dynamic_annotations",
     "//skia",
     "//third_party/harfbuzz-ng",
-    "//third_party/icu:icui18n",
-    "//third_party/icu:icuuc",
     "//third_party/libpng",
     "//third_party/zlib",
     "//ui/gfx/geometry",
   ]
-  forward_dependent_configs_from = [ "//skia" ]
+  public_deps = [
+    "//base",
+    "//skia",
+    "//third_party/icu",
+  ]
 
   # Text rendering conditions (complicated so separated out).
   if (is_android || is_ios) {
@@ -378,8 +379,11 @@ source_set("test_support") {
     "test/ui_cocoa_test_helper.mm",
   ]
 
-  deps = [
+  public_deps = [
     ":gfx",
+  ]
+
+  deps = [
     "//base",
     "//base/test:test_support",
     "//skia",
@@ -464,6 +468,7 @@ test("gfx_unittests") {
     "//skia",
     "//third_party/icu:icuuc",
     "//third_party/libpng",
+    "//third_party/zlib",
     "//testing/gtest",
     "//ui/base",
     "//ui/gfx/geometry",

ui/gl/BUILD.gn

@@ -118,19 +118,18 @@ component("gl") {
 
   deps = [
     ":generate_gl_bindings",
-    "//base",
     "//base/third_party/dynamic_annotations",
     "//gpu/command_buffer/common",
-    "//third_party/mesa:mesa_headers",
     "//skia",
+  ]
+  public_deps = [
+    "//base",
+    "//third_party/mesa:mesa_headers",
     "//ui/events/platform",
     "//ui/gfx",
     "//ui/gfx/geometry",
   ]
 
-  # TODO(GYP) hook up once this is converted.
-  forward_dependent_configs_from = [ "//third_party/mesa:mesa_headers" ]
-
   if (is_win || is_android || is_linux) {
     sources += [
       "egl_util.cc",
@@ -342,7 +341,6 @@ if (is_android) {
 
   generate_jni("gl_jni_headers") {
     deps = [ ":surface_jni_headers" ]
-    forward_dependent_configs_from = [ ":surface_jni_headers" ]
     sources = [
       "../android/java/src/org/chromium/ui/gl/SurfaceTextureListener.java",
       "../android/java/src/org/chromium/ui/gl/SurfaceTexturePlatformWrapper.java",

ui/keyboard/BUILD.gn

@@ -83,8 +83,7 @@ grit("resources_grit") {
 copy("resources") {
   sources = [ "$target_gen_dir/keyboard_resources.pak" ]
   outputs = [ "$root_out_dir/keyboard_resources.pak" ]
-  deps = [ ":resources_grit" ]
-  forward_dependent_configs_from = deps
+  public_deps = [ ":resources_grit" ]
 }
 
 test("keyboard_unittests") {
@@ -101,11 +100,10 @@ test("keyboard_unittests") {
     "//content",
     "//skia",
     "//testing/gtest",
-    "//ui/aura",
     "//ui/aura:test_support",
-    "//ui/base",
-    "//ui/compositor",
+    "//ui/base:test_support",
     "//ui/compositor:test_support",
+    "//ui/events:test_support",
     "//ui/gfx",
     "//ui/gfx/geometry",
     "//ui/gl",

ui/message_center/BUILD.gn

@@ -6,8 +6,6 @@ import("//build/config/features.gni")
 import("//build/config/ui.gni")
 
 component("message_center") {
-  defines = [ "MESSAGE_CENTER_IMPLEMENTATION" ]
-
   deps = [
     "//base",
     "//base:i18n",
@@ -18,10 +16,14 @@ component("message_center") {
     "//ui/events",
     "//ui/gfx",
     "//ui/gfx/geometry",
+    "//ui/native_theme",
     "//ui/resources",
     "//ui/strings",
+    "//url",
   ]
 
+  defines = [ "MESSAGE_CENTER_IMPLEMENTATION" ]
+
   if (enable_notifications) {
     sources = [
       "cocoa/notification_controller.h",
@@ -159,8 +161,11 @@ static_library("test_support") {
     "fake_notifier_settings_provider.cc",
   ]
 
-  deps = [
+  public_deps = [
     ":message_center",
+  ]
+
+  deps = [
     "//base",
     "//base/test:test_support",
     "//skia",

ui/views/BUILD.gn

@@ -17,15 +17,23 @@ component("views") {
   defines = [ "VIEWS_IMPLEMENTATION" ]
 
   deps = [
-    "//base",
     "//base:i18n",
     "//base/third_party/dynamic_annotations",
     "//skia",
     "//third_party/icu",
     "//third_party/wtl",
     "//ui/accessibility",
-    "//ui/accessibility:ax_gen",
     "//ui/aura",
+    "//ui/native_theme",
+    "//ui/resources",
+    "//ui/strings",
+    "//ui/wm",
+    "//url",
+  ]
+
+  public_deps = [
+    "//base",
+    "//ui/accessibility:ax_gen",
     "//ui/base",
     "//ui/compositor",
     "//ui/events",
@@ -33,15 +41,8 @@ component("views") {
     "//ui/events/platform",
     "//ui/gfx",
     "//ui/gfx/geometry",
-    "//ui/native_theme",
-    "//ui/resources",
-    "//ui/strings",
-    "//ui/wm",
-    "//url",
   ]
 
-  forward_dependent_configs_from = [ "//ui/accessibility:ax_gen" ]
-
   if (use_x11 && !is_chromeos) {
     deps += [ "//ui/display/util" ]
   }
@@ -85,6 +86,7 @@ component("views") {
     ]
     deps += [
       "//ui/events/platform/x11",
+      "//ui/gfx/x",
     ]
   }
 
@@ -107,8 +109,10 @@ static_library("test_support") {
   testonly = true
   sources = gypi_values.views_test_support_sources
 
-  deps = [
+  public_deps = [
     ":views",
+  ]
+  deps = [
     "//base",
     "//ipc:test_support",
     "//skia",
@@ -136,6 +140,9 @@ static_library("test_support") {
     sources += gypi_values.views_test_support_aura_sources
     deps += [ "//ui/aura:test_support" ]
   }
+  if (use_x11) {
+    deps += [ "//ui/gfx/x" ]
+  }
 }
 
 test("views_unittests") {
@@ -143,7 +150,6 @@ test("views_unittests") {
 
   deps = [
     ":test_support",
-    ":views",
     "//base",
     "//base:i18n",
     "//base/allocator",
@@ -155,13 +161,15 @@ test("views_unittests") {
     "//ui/aura",
     "//ui/base",
     "//ui/base:test_support",
-    "//ui/compositor",
-    "//ui/events",
+    "//ui/compositor:test_support",
+    "//ui/events:test_support",
     "//ui/events:events_base",
     "//ui/events/platform",
     "//ui/gfx",
     "//ui/gfx/geometry",
     "//ui/gl",
+    "//ui/resources",
+    "//ui/strings",
     "//ui/wm",
     "//url",
   ]
@@ -199,6 +207,7 @@ test("views_unittests") {
     ]
     deps += [
       "//ui/events/platform/x11",
+      "//ui/gfx/x",
     ]
   }
 
@@ -210,7 +219,10 @@ test("views_unittests") {
     }
   }
   if (use_x11) {
-    deps += [ "//ui/events/platform/x11" ]
+    deps += [
+      "//ui/events/platform/x11",
+      "//ui/gfx/x",
+    ]
   }
 
   if (is_mac) {

ui/views/controls/webview/BUILD.gn

@@ -18,23 +18,24 @@ component("webview") {
   defines = [ "WEBVIEW_IMPLEMENTATION" ]
 
   deps = [
-    "//base",
     "//base:i18n",
     "//base/third_party/dynamic_annotations",
-    "//content/public/browser",
+    "//ipc",
     "//skia",
     "//ui/accessibility",
     "//ui/base",
     "//ui/events",
     "//ui/events:events_base",
-    "//ui/gfx",
-    "//ui/gfx/geometry",
     "//ui/web_dialogs",
-    "//ui/views",
     "//url",
   ]
 
-  forward_dependent_configs_from = [
+  public_deps = [
+    "//base",
+    "//content/public/browser",
+    "//ui/aura",
+    "//ui/gfx",
+    "//ui/gfx/geometry",
     "//ui/views",
   ]
 }
@@ -46,8 +47,10 @@ source_set("test_support") {
     "../../test/webview_test_helper.h",
   ]
 
-  deps = [
+  public_deps = [
     ":webview",
+  ]
+  deps = [
     "//base",
     "//content",
     "//content/test:test_support",

ui/views/examples/BUILD.gn

@@ -73,6 +73,7 @@ component("views_examples_lib") {
     "//ui/gfx/geometry",
     "//ui/resources",
     "//ui/resources:ui_test_pak",
+    "//ui/views",
   ]
 
   if (is_win) {
@@ -102,14 +103,19 @@ executable("views_examples_exe") {
     "//ui/compositor",
     "//ui/compositor:test_support",
     "//ui/gfx",
+    "//ui/gl",
     "//ui/resources:ui_test_pak",
     "//ui/views",
     "//ui/views:test_support",
+    "//ui/wm",
   ]
 
   if (use_aura) {
     deps += [ "//ui/aura" ]
   }
+  if (use_x11) {
+    deps += [ "//ui/gfx/x" ]
+  }
 }
 
 component("views_examples_with_content_lib") {

ui/views_content_client/BUILD.gn

@@ -35,6 +35,7 @@ component("views_content_client") {
     "//ui/resources:ui_test_pak",
     "//ui/views",
     "//ui/views:test_support",
+    "//ui/wm",
   ]
 
   if (use_aura) {

ui/web_dialogs/BUILD.gn

@@ -22,6 +22,7 @@ component("web_dialogs") {
     "//content/public/common",
     "//skia",
     "//ui/base",
+    "//url",
   ]
 }
 
@@ -33,10 +34,13 @@ static_library("test_support") {
     "test/test_web_dialog_delegate.h",
   ]
 
-  deps = [
+  public_deps = [
     ":web_dialogs",
+  ]
+  deps = [
     "//base",
     "//content/public/browser",
     "//skia",
+    "//url",
   ]
 }

ui/wm/BUILD.gn

@@ -134,12 +134,10 @@ test("wm_unittests") {
     "//base/test:test_support",
     "//skia",
     "//testing/gtest",
-    "//ui/aura",
     "//ui/aura:test_support",
-    "//ui/base",
-    "//ui/compositor",
-    "//ui/events",
-    "//ui/events:events_base",
+    "//ui/base:test_support",
+    "//ui/compositor:test_support",
+    "//ui/events:test_support",
     "//ui/events/platform",
     "//ui/gfx",
     "//ui/gfx/geometry",