Split GN installer_util into one that has no strings.

chrome/installer/util/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/config/chrome_build.gni")
 import("//testing/test.gni")
 
-static_library("util") {
+# 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" ]
+  }
+}
+
+# chrome/installer/util has generated strings on Windows. These appear as

[grt (UTC plus 2), 2015/12/18 19:02:15]

I think this comment is incorrect. Chrome calls many functions in installer_util
that use strings, and does not require that it be linked with the string
resources. ChromeBrowserMainPartsWin::PreMainMessageLoopStart takes care of
installing a TranslationDelegate for Chrome so that installer_util string
lookups are routed to l10n_util::GetStringUTF16 rather than
AtlGetStringResourceImage. In the GYP world, targets that don't install a custom
TranslationDelegate link with installer_util_strings.rc to get the string
resources. At the moment, this is installer_util_unittests.exe, setup.exe,
setup_unittests.exe, and unit_tests.exe. Hope this helps clear things up a bit.

[brettw, 2015/12/21 17:33:00]

This comment is referring specifically to chrome.exe, not "Chrome" (which is
mostly chrome.dll). This all happens long before ChromeBrowserMain, and there is
no TranslationDelegate in this module until chrome.dll is loaded. Since the code
looked clearly wrong, I actually audited all the calls from chrome.exe to
installer_util and none result in getting any strings.

[grt (UTC plus 2), 2015/12/21 19:45:06]

Ah, now I see what you mean. :-(

[brettw, 2015/12/21 22:56:22]

I made this distinction more clear in the comment here.

+# Windows resources and are fairly large (~200KB). They are generated by the
+# ":generate_strings" target and compiled by the ":strings" target.
+#
+# Some code, like the Windows chrome.exe code uses installer_util functions
+# without calling any functions that use the strings. And historitally in the

[Dirk Pranke, 2015/12/20 19:53:46]

nit: s/historitally/historically

+# GYP build the strings 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.
+#
+# 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") {
   deps = [
     "//base",
     "//chrome:strings",
     "//chrome/common:constants",
     "//components/variations",
     "//content/public/common:result_codes",
   ]
 
   # The constants target checks the Chrome distribution from this target. Both
   # targets have to be linked together in practice.
@@ skipping 40 matching lines @@
       "self_cleaning_temp_dir.cc",
       "self_cleaning_temp_dir.h",
       "shell_util.cc",
       "shell_util.h",
       "uninstall_metrics.cc",
       "uninstall_metrics.h",
       "user_experiment.cc",
       "user_experiment.h",
     ]
 
-    public_deps = [
-      ":strings",
-    ]
     deps += [
+      # Need to depend on the generated strings target since files here
+      # depend on the generated header, but only depend on the ":strings"
+      # target (which actually compiles and causes the generated code to be
+      # linked) from the ":util" target.
+      ":generate_strings",
       "//base/third_party/dynamic_annotations",
       "//components/metrics",
       "//courgette:courgette_lib",
       "//crypto",
       "//third_party/bspatch",
       "//third_party/icu",
       "//third_party/lzma_sdk",
     ]
 
     configs += [
@@ skipping 97 matching lines @@
     sources = [
       "master_preferences.cc",
       "master_preferences.h",
       "master_preferences_constants.cc",
       "master_preferences_constants.h",
     ]
   }
 }
 
 action("generate_strings") {
-  visibility = [ ":strings" ]
+  visibility = [
+    ":strings",
+    ":with_no_strings_some_things_wont_work",
+  ]
   script = "prebuild/create_string_rc.py"
 
   if (is_chrome_branded) {
     grdfile = "//chrome/app/google_chrome_strings.grd"
   } else {
     grdfile = "//chrome/app/chromium_strings.grd"
   }
 
   inputs = [
     grdfile,
@@ skipping 83 matching lines @@
       "//chrome/installer/test:alternate_version_generator_lib",
       "//components/variations",
       "//content/public/common",
       "//testing/gmock",
       "//testing/gtest",
     ]
 
     # TODO(GYP) manifest: $(ProjectDir)\\installer\\mini_installer\\mini_installer.exe.manifest
   }
 }  # is_win