Break up the chrome/common:common target

chrome/common/BUILD.gn

 # Copyright 2014 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 import("//build/buildflag_header.gni")
 import("//build/util/process_version.gni")
 import("//chrome/common/features.gni")
 import("//chrome/process_version_rc_template.gni")  # For branding_file_path.
 import("//extensions/features/features.gni")
 import("//mojo/public/tools/bindings/mojom.gni")
@@ skipping 30 matching lines @@
     "ENABLE_PACKAGE_MASH_SERVICES=$enable_package_mash_services",
     "ENABLE_SERVICE_DISCOVERY=$enable_service_discovery",
     "ENABLE_SESSION_SERVICE=$enable_session_service",
     "ENABLE_SUPERVISED_USERS=$enable_supervised_users",
     "ENABLE_WAYLAND_SERVER=$enable_wayland_server",
     "PGO_BUILD=$pgo_build",
     "USE_VULCANIZE=$use_vulcanize",
   ]
 }
 
-# Use a static library here because many test binaries depend on this but don't
-# require many files from it. This makes linking more efficient.
-static_library("common") {
+source_set("channel_info") {
   sources = [
-    "all_messages.h",
-    "attrition_experiments.h",
-    "auto_start_linux.cc",
-    "auto_start_linux.h",
     "channel_info.cc",
     "channel_info.h",
     "channel_info_android.cc",
     "channel_info_chromeos.cc",
     "channel_info_mac.mm",
     "channel_info_posix.cc",
     "channel_info_win.cc",
+  ]
+
+  # DO NOT SUBMIT. Preexisting logic. Validate substractive approach.

[agrieve, 2017/06/15 13:52:00]

Maybe address this DO NOT SUBMIT if the change is ready :P

[manzagop (departed), 2017/06/15 14:54:59]

Yup! I meant to ask if there's any reason for the "substractive" approach -
seems not!

Done.

+  if (is_android || is_chromeos || is_mac) {
+    sources -= [ "channel_info_posix.cc" ]

[agrieve, 2017/06/15 13:52:00]

Rather than adding and then removing channel_info_posix.cc, it's a bit nicer to
leave it out of the original sources list, and only added it for is_linux

[manzagop (departed), 2017/06/15 14:54:59]

Totally! Done.

+  }
+  public_deps = [
+    "//base",
+    "//components/version_info",
+    "//components/version_info:version_string",
+  ]
+  if (is_win) {
+    public_deps += [ "//chrome/install_static:install_static_util" ]
+  }
+}
+
+# Use a static library here because many test binaries depend on this but don't
+# require many files from it. This makes linking more efficient.
+static_library("common") {
+  sources = [
+    "all_messages.h",
+    "attrition_experiments.h",
+    "auto_start_linux.cc",
+    "auto_start_linux.h",
     "child_process_logging.h",
     "child_process_logging_win.cc",
     "chrome_content_client.cc",
     "chrome_content_client.h",
     "chrome_content_client_constants.cc",
     "chrome_isolated_world_ids.h",
     "chrome_result_codes.h",
     "chrome_utility_messages.h",
     "common_message_generator.cc",
     "common_message_generator.h",
@@ skipping 83 matching lines @@
     "web_application_info.h",
   ]
   defines = []
 
   configs += [
     "//build/config:precompiled_headers",
     "//build/config/compiler:wexit_time_destructors",
   ]
 
   public_deps = [
+    ":channel_info",
     ":features",
     ":mojo_bindings",
     ":page_load_metrics_mojom",
     "//base:base",
     "//base:base_static",
     "//base:i18n",
     "//chrome:resources",
     "//chrome:strings",
     "//chrome/app/theme:theme_resources",
     "//chrome/common:constants",
@@ skipping 28 matching lines @@
     "//components/policy:generated",
     "//components/policy/core/common",
     "//components/prefs",
     "//components/safe_browsing:csd_proto",
     "//components/signin/core/common",
     "//components/strings",
     "//components/translate/content/common",
     "//components/translate/core/common",
     "//components/url_formatter",
     "//components/variations",
-    "//components/version_info",
-    "//components/version_info:version_string",
     "//components/visitedlink/common",
     "//content/public/common",
     "//crypto",
     "//extensions/common:common_constants",
     "//extensions/features",
     "//google_apis",
     "//gpu/command_buffer/service",
     "//gpu/config",
     "//gpu/config:crash_keys",
     "//ipc",
     "//media",
     "//mojo/edk/system",
     "//mojo/public/cpp/bindings",
-    "//net",
     "//pdf:features",
     "//ppapi/features",
     "//printing/features",
     "//skia",
     "//third_party/icu",
     "//third_party/re2",
     "//third_party/widevine/cdm:headers",
     "//third_party/zlib/google:zip",
     "//ui/accessibility",
     "//ui/base",
@@ skipping 129 matching lines @@
   if (is_chromeos) {
     public_deps += [ "//chromeos" ]
   }
 
   if (enable_nacl) {
     public_deps += [ "//components/nacl/common" ]
   }
 
   # Printing.
   if (enable_basic_printing || enable_print_preview) {
-    sources += [
-      "chrome_utility_printing_messages.h",
-      "cloud_print/cloud_print_cdd_conversion.cc",
-      "cloud_print/cloud_print_cdd_conversion.h",
-      "cloud_print/cloud_print_class_mac.h",
-      "cloud_print/cloud_print_class_mac.mm",
-      "cloud_print/cloud_print_constants.cc",
-      "cloud_print/cloud_print_constants.h",
-      "cloud_print/cloud_print_helpers.cc",
-      "cloud_print/cloud_print_helpers.h",
-      "cloud_print/cloud_print_proxy_info.cc",
-      "cloud_print/cloud_print_proxy_info.h",
-    ]
+    sources += [ "chrome_utility_printing_messages.h" ]
 
     public_deps += [
+      "//chrome/common/cloud_print",
       "//components/printing/common",
       "//printing",
     ]
     if (enable_print_preview) {
       # Full printing support.
       sources += [
         "service_messages.h",
         "service_process_util.cc",
         "service_process_util.h",
         "service_process_util_linux.cc",
         "service_process_util_mac.mm",
         "service_process_util_posix.cc",
         "service_process_util_posix.h",
         "service_process_util_win.cc",
       ]
     }
   }
 
   if (is_android) {
-    sources -= [
-      "channel_info_posix.cc",
-      "media_galleries/metadata_types.h",
-    ]
+    sources -= [ "media_galleries/metadata_types.h" ]
     sources += [
       "media/chrome_media_drm_bridge_client.cc",
       "media/chrome_media_drm_bridge_client.h",
     ]
   } else {
     # Non-Android.
     sources += [
       "importer/edge_importer_utils_win.cc",
       "importer/edge_importer_utils_win.h",
       "importer/firefox_importer_utils.cc",
@@ skipping 18 matching lines @@
       "importer/importer_url_row.h",
       "importer/profile_import_process_param_traits.cc",
       "importer/profile_import_process_param_traits.h",
       "importer/profile_import_process_param_traits_macros.h",
       "importer/pstore_declarations.h",
       "importer/safari_importer_utils.h",
       "importer/safari_importer_utils.mm",
     ]
   }
 
-  if (is_chromeos) {
-    sources -= [ "channel_info_posix.cc" ]
-  }
-
   if (is_win) {
     public_deps += [
-      "//chrome/install_static:install_static_util",

[agrieve, 2017/06/15 13:52:00]

Do you know what this might have been for?

[manzagop (departed), 2017/06/15 14:54:59]

Hm.

channel_info_win.cc uses it so I'd moved it above to the channel_info
source_set.

But it looks like logging_chrome.cc in this target uses it too. I guess it works
because this file uses public_deps. Should I add it back here? 

More generally, how do you feel about deps vs public_deps?

[agrieve, 2017/06/15 15:06:17]

Ah, just missed that it was added above.

Unless you consider the dep a part of your public api (e.g. aka, your header
files use types defined by the other library), then non-public is the way to go.

       "//components/dom_distiller/core",  # Needed by chrome_content_client.cc.
       "//third_party/wtl",
     ]
 
     libs = [ "wintrust.lib" ]
   }
 
   if (is_mac) {
-    sources -= [ "channel_info_posix.cc" ]
     public_deps += [
       "//third_party/google_toolbox_for_mac",
       "//third_party/mach_override",
     ]
   }
 
   if (enable_plugins) {
     sources += [
       "pepper_flash.cc",
       "pepper_flash.h",
@@ skipping 286 matching lines @@
 
 mojom("page_load_metrics_mojom") {
   sources = [
     "page_load_metrics/page_load_metrics.mojom",
   ]
 
   public_deps = [
     "//mojo/common:common_custom_types",
   ]
 }