Use .rc strings in fewer places for installer util.

The resource strings are needed only for the setup app and unit tests. Previously they were also linked into Chrome.

This clarifies the usage and renames the targets to force you to pick (previously the naming encouraged you to link to the strings, which is probably not correct for most cases.

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

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

chrome/BUILD.gn

@@ -135,7 +135,7 @@ if (!is_android) {
         "//chrome/browser:chrome_process_finder",
         "//chrome/chrome_watcher",
         "//chrome/chrome_watcher:client",
-        "//chrome/installer/util:with_no_strings_some_things_wont_work",
+        "//chrome/installer/util:with_no_strings",
         "//chrome_elf",
         "//components/browser_watcher:browser_watcher_client",
         "//components/crash/content/app",
@@ -188,7 +188,7 @@ if (!is_android) {
         "//base/allocator",
 
         # Needed to use the master_preferences functions
-        "//chrome/installer/util",
+        "//chrome/installer/util:with_no_strings",
         "//content/public/app:both",
       ]
       if (enable_plugins && enable_pdf) {

chrome/browser/BUILD.gn

@@ -313,7 +313,7 @@ source_set("browser") {
       "//chrome/browser/metrics/variations:chrome_ui_string_overrider_factory",
       "//chrome/browser/resources:component_extension_resources",
       "//chrome/common/net",
-      "//chrome/installer/util",
+      "//chrome/installer/util:with_no_strings",
       "//components/about_handler",
       "//components/app_modal",
       "//components/autofill/content/browser",

chrome/browser/chromeos/BUILD.gn

@@ -50,7 +50,7 @@ source_set("chromeos") {
     "//chrome/common/extensions/api:api_registration",
     "//chrome/common/net",
     "//chrome/common/safe_browsing:proto",
-    "//chrome/installer/util",
+    "//chrome/installer/util:with_no_strings",
     "//chromeos",
     "//chromeos:cryptohome_proto",
     "//chromeos:cryptohome_signkey_proto",

chrome/browser/extensions/BUILD.gn

@@ -42,7 +42,7 @@ source_set("extensions") {
     "//chrome/common",
     "//chrome/common/extensions/api:api_registration",
     "//chrome/common/safe_browsing:proto",
-    "//chrome/installer/util",
+    "//chrome/installer/util:with_no_strings",
     "//components/copresence",
     "//components/data_reduction_proxy/proto:data_reduction_proxy_proto",
     "//components/dom_distiller/core",

chrome/browser/metro_utils/BUILD.gn

@@ -9,7 +9,7 @@ source_set("metro_utils") {
     # We use a few functions from here but don't depend on the resource
     # strings. This target is indirectly linked into chrome.exe, so
     # specifically doesn't want those strings.
-    "//chrome/installer/util:with_no_strings_some_things_wont_work",
+    "//chrome/installer/util:with_no_strings",
   ]
   sources = [
     "metro_chrome_win.cc",

chrome/browser/ui/BUILD.gn

@@ -99,7 +99,7 @@ source_set("ui") {
       "//chrome/browser/ui/webui/engagement:mojo_bindings",
       "//chrome/browser/ui/webui/omnibox:mojo_bindings",
       "//chrome/common/net",
-      "//chrome/installer/util",
+      "//chrome/installer/util:with_no_strings",
       "//components/autofill/content/browser:risk_proto",
       "//components/bubble:bubble",
       "//components/crash/core/browser",

chrome/chrome_watcher/BUILD.gn

@@ -35,7 +35,7 @@ shared_library("chrome_watcher") {
     "//base",
     "//base:base_static",
     "//build/config/sanitizers:deps",
-    "//chrome/installer/util:with_no_strings_some_things_wont_work",
+    "//chrome/installer/util:with_no_strings",
     "//components/browser_watcher",
   ]
   ldflags = [ "/DEF:" + rebase_path("chrome_watcher.def", root_build_dir) ]

chrome/common/BUILD.gn

@@ -68,7 +68,7 @@ static_library("common") {
     "//chrome/common:constants",
     "//chrome/common/safe_browsing:proto",
     "//chrome/common/variations:fieldtrial_testing_config",
-    "//chrome/installer/util",
+    "//chrome/installer/util:with_no_strings",
     "//components/cloud_devices/common",
     "//components/component_updater",
     "//components/content_settings/core/common",

chrome/installer/gcapi/BUILD.gn

@@ -33,7 +33,7 @@ source_set("lib") {
   deps = [
     "//base",
     "//chrome/installer/launcher_support",
-    "//chrome/installer/util",
+    "//chrome/installer/util:with_no_strings",
     "//components/variations",
     "//google_update",
   ]
@@ -54,7 +54,7 @@ test("gcapi_test") {
   deps = [
     ":lib",
     "//base/test:test_support",
-    "//chrome/installer/util",
+    "//chrome/installer/util:with_no_strings",
     "//components/variations",
     "//testing/gtest",
   ]

chrome/installer/setup/BUILD.gn

@@ -54,7 +54,7 @@ if (is_win) {
       "//base",
       "//chrome/common:constants",
       "//chrome/common:version_header",
-      "//chrome/installer/util",
+      "//chrome/installer/util:with_rc_strings",
       "//chrome_elf:constants",
       "//components/crash/content/app:app_breakpad_mac_win_to_be_deleted",
       "//components/crash/content/app:lib",
@@ -86,8 +86,6 @@ if (is_win) {
       "//base/allocator",
       "//base/test:test_support",
       "//chrome/installer/mini_installer:unit_tests",
-      "//chrome/installer/util:strings",
-      "//chrome/installer/util:util",
       "//testing/gmock",
       "//testing/gtest",
     ]

chrome/installer/test/BUILD.gn

@@ -18,7 +18,7 @@ executable("alternate_version_generator") {
     "//base/test:test_support",
     "//build/config/sanitizers:deps",
     "//chrome/common:constants",
-    "//chrome/installer/util",
+    "//chrome/installer/util:with_rc_strings",
     "//testing/gtest",
   ]
 }
@@ -39,7 +39,7 @@ source_set("alternate_version_generator_lib") {
     "//base",
     "//base:base_static",
     "//chrome/common:constants",
-    "//chrome/installer/util",
+    "//chrome/installer/util:with_rc_strings",
   ]
 }
 
@@ -53,7 +53,7 @@ test("upgrade_test") {
     "//base",
     "//base/test:test_support",
     "//chrome/common:constants",
-    "//chrome/installer/util",
+    "//chrome/installer/util:with_rc_strings",
     "//testing/gtest",
   ]
   data_deps = [

chrome/installer/util/BUILD.gn

@@ -5,16 +5,12 @@
 import("//build/config/chrome_build.gni")
 import("//testing/test.gni")
 
-# Generally you should depend on this target, see below for discussion.
-group("util") {
-  public_deps = [
-    ":with_no_strings_some_things_wont_work",
-  ]
-  if (is_win) {
-    public_deps += [ ":strings" ]
-  }
-}
-
+# This file deliberately has no default "util" target so dependants have to
+# specify with the ":with_no_strings" or ":with_rc_strings" variants. Random
+# code that ends up getting linked into chrome proper should depend on the
+# ":with_no_strings" variant. Other standalone apps will need to decide if they
+# need the resource strings or not.
+#
 # chrome/installer/util has generated strings on Windows. These appear as
 # Windows resources and are fairly large (~200KB). They are generated by the
 # ":generate_strings" target and compiled by the ":strings" target.
@@ -25,18 +21,24 @@ group("util") {
 # resource had to be manually linked, so the strings never ended up being in
 # chrome.exe and everything was fine.
 #
-# However, this is obviously a fragile and confusing situation. If you depend
-# on this target rather than going through the ":util" version that links the
-# strings above, anything that uses localized strings won't work. There is
-# no definition of what works and doesn't work, and this may also change over
-# time. As an example at the time of this writing, chrome.exe calls
-# BrowserDistribution::GetRegistryPath. This function doesn't use any strings,
-# but lots of other functions in BrowserDistribution do use localized strings.
+# Other code, like chrome.dll links to installer util, and hooks up a
+# TranslationDelegate which overrides the strings retrieved from the resources
+# with grit pak strings.
+#
+# In both of these cases, link to the ":with_no_strings" variant. However, this
+# is obviously a fragile and confusing situation. In the "I don't use strings
+# at all case", there is no definition of what works and doesn't work, and this
+# may also change over time. As an example at the time of this writing,
+# chrome.exe calls BrowserDistribution::GetRegistryPath. This function doesn't
+# use any strings, but lots of other functions in BrowserDistribution do use
+# localized strings.
 #
 # Ideally, this should be cleaved in two parts: the main "installer util" and
 # some kind of mini installer util that just contains the functions needed by
-# chrome.exe and any other clients that don't need the strings.
-static_library("with_no_strings_some_things_wont_work") {
+# chrome.exe and any other clients that don't need the strings. It's likely
+# we would still need the variant with no strings for when chrome.dll replaces
+# all strings with its own versions.
+static_library("with_no_strings") {
   deps = [
     "//base",
     "//chrome:strings",
@@ -221,10 +223,21 @@ static_library("with_no_strings_some_things_wont_work") {
   }
 }
 
+# Use this version of installer_util to link to the generated strings in .rc
+# format.
+group("with_rc_strings") {
+  public_deps = [
+    ":with_no_strings",
+  ]
+  if (is_win) {
+    public_deps += [ ":strings" ]
+  }
+}
+
 action("generate_strings") {
   visibility = [
     ":strings",
-    ":with_no_strings_some_things_wont_work",
+    ":with_no_strings",
   ]
   script = "prebuild/create_string_rc.py"
 
@@ -312,8 +325,7 @@ if (is_win) {
     ]
 
     deps = [
-      ":strings",
-      ":util",
+      ":with_rc_strings",
       "//base",
       "//base:i18n",
       "//base/test:test_support",

chrome/tools/crash_service/BUILD.gn

@@ -15,7 +15,7 @@ executable("crash_service") {
   deps = [
     "//base",
     "//chrome/common:constants",
-    "//chrome/installer/util",
+    "//chrome/installer/util:with_no_strings",
     "//components/crash/content/tools:crash_service",
     "//content/public/common:static_switches",
   ]

win8/delegate_execute/BUILD.gn

@@ -44,7 +44,7 @@ source_set("lib") {
     "//base",
     "//breakpad:breakpad_handler",
     "//chrome/common:constants",
-    "//chrome/installer/util:with_no_strings_some_things_wont_work",
+    "//chrome/installer/util:with_no_strings",
     "//ui/base",
     "//ui/gfx",
     "//ui/gfx/geometry",

win8/metro_driver/BUILD.gn

@@ -23,7 +23,7 @@ shared_library("metro_driver") {
     ":version_resources",
     "//base",
     "//chrome/common:constants",
-    "//chrome/installer/util:with_no_strings_some_things_wont_work",
+    "//chrome/installer/util:with_no_strings",
     "//crypto",
     "//ipc",
     "//sandbox",
@@ -122,7 +122,7 @@ test("metro_driver_unittests") {
   deps = [
     ":metro_driver",
     "//base",
-    "//chrome/installer/util",
+    "//chrome/installer/util:with_rc_strings",
     "//testing/gtest",
   ]
 }