Revert of Make ui gn check pass. Misc GN build improvements. (patchset #3...

Revert of Make ui gn check pass. Misc GN build improvements. (patchset #3 id:40001 of https://codereview.chromium.org/545313002/) Reason for revert: Broke http://build.chromium.org/p/chromium.linux/builders/Linux%20GN%20%28dbg%29/builds/11835 [783/786 | 258.324] LINK ./chrome FAILED: /mnt/data/b/build/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -Wl,--fatal-warnings -m64 -fPIC -Wl,-z,noexecstack -Wl,-z,now -Wl,-z,relro -pthread -B../../third_party/binutils/Linux_x64/Release/bin -fuse-ld=gold -Wl,--icf=none -Wl,-rpath=\$ORIGIN/ -Wl,-rpath-link= -Wl,--disable-new-dtags -o ./keyboard_unittests -Wl,--start-group @./keyboard_unittests.rsp -Wl,--end-group -ldl -lgmodule-2.0 -lgobject-2.0 -lgthread-2.0 -lrt -lglib-2.0 -ludev -lfontconfig -lX11 -lXcomposite -lXcursor -lXdamage -lXext -lXfixes -lXi -lXrender -lXss -lXtst -lpangocairo-1.0 -lpango-1.0 -lcairo -lnss3 -lnssutil3 -lsmime3 -lplds4 -lplc4 -lnspr4 -lgconf-2 -lresolv -lfreetype -lasound -lXrandr -lcups -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lgcrypt -lz -lpthread -lcrypt -lm -lcap -ldbus-1 -lcrypto -lexpat ../../content/utility/utility_thread_impl.cc:38: error: undefined reference to 'content::UtilityThread::UtilityThread()' ../../content/utility/utility_thread_impl.cc:44: error: undefined reference to 'content::UtilityThread::UtilityThread()' ../../content/utility/utility_thread_impl.cc:49: error: undefined reference to 'content::UtilityThread::~UtilityThread()' clang: error: linker command failed with exit code 1 (use -v to see invocation) ninja: build stopped: subcommand failed. Original issue's description: > Make ui gn check pass. Misc GN build improvements. > > Content/public/app was messed up and didn't link everything properly, so this separates it out into browser and child like content/app is. > > R=jamesr@chromium.org > > Committed: https://chromium.googlesource.com/chromium/src/+/b41a8d6e789ac5df9a6d314cd0ad59f584f53dea TBR=jamesr@chromium.org,brettw@chromium.org NOTREECHECKS=true NOTRY=true Review URL: https://codereview.chromium.org/546943003 Cr-Commit-Position: refs/heads/master@{#293595}

Revert of Make ui gn check pass. Misc GN build improvements. (patchset #3...
Revert of Make ui gn check pass. Misc GN build improvements. (patchset #3 id:40001 of https://codereview.chromium.org/545313002/) Reason for revert: Broke http://build.chromium.org/p/chromium.linux/builders/Linux%20GN%20%28dbg%29/builds/11835 [783/786 | 258.324] LINK ./chrome FAILED: /mnt/data/b/build/goma/gomacc ../../third_party/llvm-build/Release+Asserts/bin/clang++ -Wl,--fatal-warnings -m64 -fPIC -Wl,-z,noexecstack -Wl,-z,now -Wl,-z,relro -pthread -B../../third_party/binutils/Linux_x64/Release/bin -fuse-ld=gold -Wl,--icf=none -Wl,-rpath=\$ORIGIN/ -Wl,-rpath-link= -Wl,--disable-new-dtags -o ./keyboard_unittests -Wl,--start-group @./keyboard_unittests.rsp -Wl,--end-group -ldl -lgmodule-2.0 -lgobject-2.0 -lgthread-2.0 -lrt -lglib-2.0 -ludev -lfontconfig -lX11 -lXcomposite -lXcursor -lXdamage -lXext -lXfixes -lXi -lXrender -lXss -lXtst -lpangocairo-1.0 -lpango-1.0 -lcairo -lnss3 -lnssutil3 -lsmime3 -lplds4 -lplc4 -lnspr4 -lgconf-2 -lresolv -lfreetype -lasound -lXrandr -lcups -lgssapi_krb5 -lkrb5 -lk5crypto -lcom_err -lgcrypt -lz -lpthread -lcrypt -lm -lcap -ldbus-1 -lcrypto -lexpat ../../content/utility/utility_thread_impl.cc:38: error: undefined reference to 'content::UtilityThread::UtilityThread()' ../../content/utility/utility_thread_impl.cc:44: error: undefined reference to 'content::UtilityThread::UtilityThread()' ../../content/utility/utility_thread_impl.cc:49: error: undefined reference to 'content::UtilityThread::~UtilityThread()' clang: error: linker command failed with exit code 1 (use -v to see invocation) ninja: build stopped: subcommand failed. Original issue's description: > Make ui gn check pass. Misc GN build improvements. > > Content/public/app was messed up and didn't link everything properly, so this separates it out into browser and child like content/app is. > > R=jamesr@chromium.org > > Committed: https://chromium.googlesource.com/chromium/src/+/b41a8d6e789ac5df9a6d314cd0ad59f584f53dea TBR=jamesr@chromium.org,brettw@chromium.org NOTREECHECKS=true NOTRY=true Review URL: https://codereview.chromium.org/546943003 Cr-Commit-Position: refs/heads/master@{#293595}
3f1281e5 · thakis · Commit bot · bd54da79 · 3f1281e5 · 3f1281e5
Commit 3f1281e5 authored Sep 05, 2014 by thakis Committed by Commit bot Sep 06, 2014
19 changed files
--- a/build/config/BUILDCONFIG.gn
+++ b/build/config/BUILDCONFIG.gn
@@ -511,13 +511,11 @@ template("component") {
      set_sources_assignment_filter([])
      if (defined(invoker.all_dependent_configs)) { all_dependent_configs = invoker.all_dependent_configs }
-      if (defined(invoker.allow_circular_includes_from)) { allow_circular_includes_from = invoker.allow_circular_includes_from }
      if (defined(invoker.cflags)) { cflags = invoker.cflags }
      if (defined(invoker.cflags_c)) { cflags_c = invoker.cflags_c }
      if (defined(invoker.cflags_cc)) { cflags_cc = invoker.cflags_cc }
      if (defined(invoker.cflags_objc)) { cflags_objc = invoker.cflags_objc }
      if (defined(invoker.cflags_objcc)) { cflags_objcc = invoker.cflags_objcc }
-      if (defined(invoker.check_includes)) { check_includes = invoker.check_includes }
      if (defined(invoker.data)) { data = invoker.data }
      if (defined(invoker.datadeps)) { datadeps = invoker.datadeps }
      if (defined(invoker.defines)) { defines = invoker.defines }
@@ -532,7 +530,6 @@ template("component") {
      if (defined(invoker.output_name)) { output_name = invoker.output_name }
      if (defined(invoker.public)) { public = invoker.public }
      if (defined(invoker.sources)) { sources = invoker.sources }
-      if (defined(invoker.testonly)) { testonly = invoker.testonly }
      if (defined(invoker.visibility)) { visibility = invoker.visibility }
    }
  } else {
@@ -545,13 +542,11 @@ template("component") {
      set_sources_assignment_filter([])
      if (defined(invoker.all_dependent_configs)) { all_dependent_configs = invoker.all_dependent_configs }
-      if (defined(invoker.allow_circular_includes_from)) { allow_circular_includes_from = invoker.allow_circular_includes_from }
      if (defined(invoker.cflags)) { cflags = invoker.cflags }
      if (defined(invoker.cflags_c)) { cflags_c = invoker.cflags_c }
      if (defined(invoker.cflags_cc)) { cflags_cc = invoker.cflags_cc }
      if (defined(invoker.cflags_objc)) { cflags_objc = invoker.cflags_objc }
      if (defined(invoker.cflags_objcc)) { cflags_objcc = invoker.cflags_objcc }
-      if (defined(invoker.check_includes)) { check_includes = invoker.check_includes }
      if (defined(invoker.data)) { data = invoker.data }
      if (defined(invoker.datadeps)) { datadeps = invoker.datadeps }
      if (defined(invoker.defines)) { defines = invoker.defines }
@@ -566,7 +561,6 @@ template("component") {
      if (defined(invoker.output_name)) { output_name = invoker.output_name }
      if (defined(invoker.public)) { public = invoker.public }
      if (defined(invoker.sources)) { sources = invoker.sources }
-      if (defined(invoker.testonly)) { testonly = invoker.testonly }
      if (defined(invoker.visibility)) { visibility = invoker.visibility }
    }
  }
@@ -592,13 +586,11 @@ template("test") {
    set_sources_assignment_filter([])
    if (defined(invoker.all_dependent_configs)) { all_dependent_configs = invoker.all_dependent_configs }
-    if (defined(invoker.allow_circular_includes_from)) { allow_circular_includes_from = invoker.allow_circular_includes_from }
    if (defined(invoker.cflags)) { cflags = invoker.cflags }
    if (defined(invoker.cflags_c)) { cflags_c = invoker.cflags_c }
    if (defined(invoker.cflags_cc)) { cflags_cc = invoker.cflags_cc }
    if (defined(invoker.cflags_objc)) { cflags_objc = invoker.cflags_objc }
    if (defined(invoker.cflags_objcc)) { cflags_objcc = invoker.cflags_objcc }
-    if (defined(invoker.check_includes)) { check_includes = invoker.check_includes }
    if (defined(invoker.data)) { data = invoker.data }
    if (defined(invoker.datadeps)) { datadeps = invoker.datadeps }
    if (defined(invoker.defines)) { defines = invoker.defines }

--- a/build/secondary/tools/grit/grit_rule.gni
+++ b/build/secondary/tools/grit/grit_rule.gni
@@ -258,7 +258,7 @@ template("grit") {
  # The config and the action below get this visibility son only the generated
  # source set can depend on them. The variable "target_name" will get
  # overwritten inside the inner classes so we need to compute it here.
-  target_visibility = [ ":$target_name" ]
+  target_visibility = ":$target_name"
  # The current grit setup makes an file in $output_dir/grit/foo.h that
  # the source code expects to include via "grit/foo.h". It would be nice to
@@ -301,20 +301,7 @@ template("grit") {
    args += grit_flags + assert_files_flags
-    if (defined(invoker.visibility)) {
+    visibility = target_visibility
-      # This needs to include both what the invoker specified (since they
-      # probably include generated headers from this target), as well as the
-      # generated source set (since there's no guarantee that the visibility
-      # specified by the invoker includes our target).
-      #
-      # Only define visibility at all if the invoker specified it. Otherwise,
-      # we want to keep the public "no visibility specified" default.
-      visibility = target_visibility
-      # This won't work if the invoker specifies a list of targets (an uncommon
-      # case). We may need to enhance the language to make this possible.
-      visibility += [ invoker.visibility ]
-    }
    deps = [ "//tools/grit:grit_sources" ]
    if (defined(invoker.deps)) {

--- a/chrome/BUILD.gn
+++ b/chrome/BUILD.gn
@@ -58,7 +58,7 @@ executable("chrome") {
      "//base/allocator",
      # Needed to use the master_preferences functions
      "//chrome/installer/util",
-      "//content/public/app:both",
+      "//content/app:both",
    ]
    # Needed for chrome_main.cc initialization of libraries.

--- a/content/BUILD.gn
+++ b/content/BUILD.gn
@@ -13,6 +13,7 @@ content_shared_components = [
  "//content/gpu",
  "//content/plugin",
  "//content/ppapi_plugin",
+  "//content/public/app",
  "//content/public/browser:sources",
  "//content/public/child",
  "//content/public/common",
@@ -26,7 +27,6 @@ if (is_component_build) {
  shared_library("content") {
    deps = content_shared_components + [
      "//content/app",
-      "//content/public/app",
    ]
    forward_dependent_configs_from = deps
  }

--- a/content/public/app/BUILD.gn
+++ b/content/public/app/BUILD.gn
@@ -2,8 +2,7 @@
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
-# Used internally to hold the sources shared between the various targets below.
+source_set("app") {
-source_set("app_sources") {
  sources = [
    "android_library_loader_hooks.h",
    "content_main.h",
@@ -24,45 +23,3 @@ source_set("app_sources") {
    "//content/public/utility",
  ]
 }
-# The structure of this is like the private content/app implementation.
-if (is_component_build) {
-  group("app") {
-    deps = [
-      ":app_sources",
-      "//content/app",
-    ]
-  }
-  group("browser") {
-    deps = [ "//content" ]
-  }
-  group("child") {
-    deps = [ "//content" ]
-  }
-  group("both") {
-    deps = [ "//content" ]
-  }
-} else {
-  group("browser") {
-    deps = [
-      ":app_sources",
-      "//content/app:browser"
-    ]
-  }
-  group("child") {
-    deps = [
-      ":app_sources",
-      "//content/app:child"
-    ]
-  }
-  group("both") {
-    deps = [
-      ":app_sources",
-      "//content/app:both"
-    ]
-  }
-}
--- a/content/test/BUILD.gn
+++ b/content/test/BUILD.gn
@@ -15,7 +15,7 @@ content_tests_gypi_values = exec_script(
 # GYP version //content/content_tests.gypi:test_support_content
 static_library("test_support") {
  deps = [
-    "//content/public/app:both",
+    "//content/public/app",
    "//content/public/browser",
    "//content/public/common",
    "//net:test_support",

--- a/extensions/BUILD.gn
+++ b/extensions/BUILD.gn
@@ -58,14 +58,10 @@ source_set("test_support") {
  sources = [
    "browser/api/dns/mock_host_resolver_creator.cc",
    "browser/api/dns/mock_host_resolver_creator.h",
-    "browser/api/storage/settings_test_util.cc",
-    "browser/api/storage/settings_test_util.h",
    "browser/api_test_utils.cc",
    "browser/api_test_utils.h",
    "browser/extensions_test.cc",
    "browser/extensions_test.h",
-    "browser/mock_extension_system.cc",
-    "browser/mock_extension_system.h",
    "browser/test_extensions_browser_client.cc",
    "browser/test_extensions_browser_client.h",
    "browser/test_management_policy.cc",
@@ -149,10 +145,6 @@ repack("shell_and_test_pak") {
 }
 # TODO(GYP): Enable this link errors are fixed.
-# This gives a link error in web_modal that is very mysterious. The GYP build
-# doesn't seem to pull in web_content_modal_dialog_manager.o since the build
-# does not have a reference to CreateNativeWebModalManager but it still links.
-# The GN build fails with this symbol being undefined.
 if (false) {
 # TODO(tfarina): Many extension unit tests run as part of Chrome"s
 # unit_tests target. They should be moved here, which may require some
@@ -162,41 +154,8 @@ test("unittests") {
  output_name = "extensions_unittests"
  sources = [
-    "browser/admin_policy_unittest.cc",
-    "browser/api/api_resource_manager_unittest.cc",
-    "browser/api/declarative/deduping_factory_unittest.cc",
-    "browser/api/sockets_tcp/sockets_tcp_api_unittest.cc",
-    "browser/api/storage/settings_quota_unittest.cc",
-    "browser/api/storage/storage_api_unittest.cc",
-    "browser/api/storage/storage_frontend_unittest.cc",
-    "browser/computed_hashes_unittest.cc",
-    "browser/content_hash_tree_unittest.cc",
-    "browser/event_listener_map_unittest.cc",
-    "browser/event_router_unittest.cc",
-    "browser/api_unittest.cc",
-    "browser/api_unittest.h",
-    "browser/extension_pref_value_map_unittest.cc",
-    "browser/extension_registry_unittest.cc",
-    "browser/file_highlighter_unittest.cc",
-    "browser/file_reader_unittest.cc",
-    "browser/guest_view/guest_view_manager_unittest.cc",
-    "browser/image_loader_unittest.cc",
-    "browser/image_util_unittest.cc",
    "browser/info_map_unittest.cc",
-    "browser/lazy_background_task_queue_unittest.cc",
-    "browser/management_policy_unittest.cc",
    "browser/process_manager_unittest.cc",
-    "browser/process_map_unittest.cc",
-    "browser/quota_service_unittest.cc",
-    "browser/runtime_data_unittest.cc",
-    "browser/value_store/leveldb_value_store_unittest.cc",
-    "browser/value_store/testing_value_store_unittest.cc",
-    "browser/value_store/value_store_change_unittest.cc",
-    "browser/value_store/value_store_frontend_unittest.cc",
-    "browser/value_store/value_store_unittest.cc",
-    "browser/value_store/value_store_unittest.h",
-    "browser/verified_contents_unittest.cc",
-    "browser/warning_service_unittest.cc",
    "common/api/sockets/sockets_manifest_permission_unittest.cc",
    "common/csp_validator_unittest.cc",
    "common/event_filter_unittest.cc",
@@ -226,16 +185,14 @@ test("unittests") {
    "renderer/script_context_set_unittest.cc",
    "renderer/utils_unittest.cc",
    "test/extensions_unittests_main.cc",
-  ]
+ ]
  deps = [
    ":extensions_resources",
    ":shell_and_test_pak",
    ":test_support",
    "//base",
-    "//base:prefs_test_support",
    "//base/test:test_support",
-    "//components/keyed_service/content",
    "//content/test:test_support",
    "//device/serial",
    "//device/serial:test_util",
@@ -248,7 +205,6 @@ test("unittests") {
    "//mojo/system",
    "//testing/gmock",
    "//testing/gtest",
-    "//third_party/leveldatabase",
  ]
  if (is_win) {

--- a/extensions/browser/BUILD.gn
+++ b/extensions/browser/BUILD.gn
@@ -377,7 +377,6 @@ source_set("browser") {
      "//components/storage_monitor",
      "//crypto:platform",
      "//device/bluetooth",
-      "//device/core",
      "//device/hid",
      "//device/serial",
      "//device/usb",

--- a/extensions/extensions.gyp
+++ b/extensions/extensions.gyp
@@ -871,7 +871,6 @@
        '<(SHARED_INTERMEDIATE_DIR)',
      ],
      'sources': [
-        # Note: sources list duplicated in GN build.
        'browser/api/dns/mock_host_resolver_creator.cc',
        'browser/api/dns/mock_host_resolver_creator.h',
        'browser/api/storage/settings_test_util.cc',

--- a/extensions/generated_extensions_api.gni
+++ b/extensions/generated_extensions_api.gni
@@ -109,11 +109,7 @@ template("generated_extensions_api") {
        "--namespace=$root_namespace",
        "--generator=cpp",
        "--include-rules=$schema_include_rules" ]
+      visibility = target_visibility
-      if (defined(invoker.visibility)) {
-        # If visibility is restricted, add our own target to it.
-        visibility = [ invoker.visibility, target_visibility ]
-      }
    }
  }

--- a/mojo/public/tools/bindings/mojom.gni
+++ b/mojo/public/tools/bindings/mojom.gni
@@ -12,10 +12,8 @@
 #       List of source .mojom files to compile.
 #
 #   deps (optional)
-#
 #   visibility (optional)
-#       Normal meaning. However, this must be a list (normal visibility
+#       Normal meaning.
-#       allows a single string).
 template("mojom") {
  assert(defined(invoker.sources),
         "\"sources\" must be defined for the $target_name template.")
@@ -77,18 +75,11 @@ template("mojom") {
    "{{source_gen_dir}}/{{source_name_part}}_mojom.py",
  ]
-  if (defined(invoker.visibility)) {
+  target_visibility = ":$target_name"
-    # Need to save this because the the target_name is overwritten inside the
-    # action to be that of the action itself. Only define this in the case the
-    # var is used to avoid unused var error.
-    target_visibility = ":$target_name"
-  }
  generator_target_name = target_name + "_generator"
  action_foreach(generator_target_name) {
-    if (defined(invoker.visibility)) {
+    visibility = target_visibility
-      visibility = [ target_visibility ] + invoker.visibility  # If you get an error here, make your visibility be a list.
-    }
    script = generator_script
    inputs = generator_sources
    sources = invoker.sources

--- a/tools/gn/input_file_manager.cc
+++ b/tools/gn/input_file_manager.cc
@@ -215,20 +215,11 @@ const ParseNode* InputFileManager::SyncLoadFile(
    }
  }
-  // The other load could have failed. In this case that error was probably
+  // The other load could have failed. In this case that error will be printed
-  // printed to the console, but we need to return something here, so make up a
+  // to the console, but we need to return something here, so make up a
  // dummy error.
-  //
+  if (!data->parsed_root)
-  // There is a race condition. The other load could have failed, but if the
+    *err = Err(origin, "File parse failed");
-  // other thread is delayed for some reason, this thread could end up
-  // reporting the error to the scheduler first (since first error report
-  // wins). The user will see this one and the "real" one will be discarded.
-  if (!data->parsed_root) {
-    *err = Err(origin, "File parse failed.",
-        "If you see this, I'm really sorry, but a race condition has caused\n"
-        "me to eat your error message. It was crunchy. If the parse error\n"
-        "in your imported file isn't obvious, try re-running GN.");
-  }
  return data->parsed_root.get();
 }

--- a/ui/app_list/BUILD.gn
+++ b/ui/app_list/BUILD.gn
@@ -58,7 +58,6 @@ component("app_list") {
    "//base:i18n",
    "//base/third_party/dynamic_annotations",
    "//skia",
-    "//sync",
    "//third_party/icu",
    "//ui/accessibility",
    "//ui/base",

--- a/ui/aura/BUILD.gn
+++ b/ui/aura/BUILD.gn
@@ -183,7 +183,6 @@ source_set("test_support") {
    "//ui/events:test_support",
    "//ui/gfx",
    "//ui/gfx/geometry",
-    "//ui/wm",
  ]
  if (is_win) {

--- a/ui/base/BUILD.gn
+++ b/ui/base/BUILD.gn
@@ -308,10 +308,6 @@ component("base") {
    ]
  } else {
    deps += [ "//ui/base/ime" ]
-    # http://crbug.com/408648 Circular includes from the ime sub-target to this
-    # one.
-    allow_circular_includes_from = [ "//ui/base/ime" ]
  }
  if (toolkit_views) {

--- a/ui/keyboard/BUILD.gn
+++ b/ui/keyboard/BUILD.gn
@@ -41,10 +41,6 @@ component("keyboard") {
    "//ui/keyboard/webui",
    "//ui/wm",
  ]
-  # http://crbug.com/408651 Include cycle between this target and the "webui"
-  # sub-target.
-  allow_circular_includes_from = [ "//ui/keyboard/webui" ]
 }
 grit("resources_grit") {

--- a/ui/resources/BUILD.gn
+++ b/ui/resources/BUILD.gn
@@ -15,6 +15,7 @@ group("resources") {
 }
 grit("ui_resources_grd") {
+  visibility = ":resources"
  source = "ui_resources.grd"
  outputs = [
    "grit/ui_resources.h",
@@ -26,6 +27,7 @@ grit("ui_resources_grd") {
 }
 grit("ui_unscaled_resources_grd") {
+  visibility = ":resources"
  source = "ui_unscaled_resources.grd"
  outputs = [
    "grit/ui_unscaled_resources.h",
@@ -34,6 +36,7 @@ grit("ui_unscaled_resources_grd") {
 }
 grit("webui_resources_grd") {
+  visibility = ":resources"
  source = "../webui/resources/webui_resources.grd"
  outputs = [
    "grit/webui_resources.h",

--- a/ui/shell_dialogs/BUILD.gn
+++ b/ui/shell_dialogs/BUILD.gn
@@ -66,6 +66,9 @@ component("shell_dialogs") {
  }
 }
+# TODO(GYP) enable shell_dialogs_unittests when media is converted.
+if (false) {
 test("shell_dialogs_unittests") {
  sources = [
    "select_file_dialog_win_unittest.cc",
@@ -79,3 +82,5 @@ test("shell_dialogs_unittests") {
    "//testing/gtest",
  ]
 }
+}  # if false
--- a/ui/wm/BUILD.gn
+++ b/ui/wm/BUILD.gn
@@ -87,9 +87,6 @@ component("wm") {
    "//ui/gfx/geometry",
    "//ui/resources",
  ]
-  # http://crbug.com/408650 Circular dependency between this target and Aura.
-  allow_circular_includes_from = [ "//ui/aura" ]
 }
 static_library("test_support") {