Remove multi-install from chrome/installer/setup.

chrome/installer/setup/setup_util.cc

Index: chrome/installer/setup/setup_util.cc
diff --git a/chrome/installer/setup/setup_util.cc b/chrome/installer/setup/setup_util.cc
index 2d03e8c119e4a7f420b7b1ea09f8f8d9e019393d..12a35bf42ea17d9bb18c2a6c7a704e5bbcc7babe 100644
--- a/chrome/installer/setup/setup_util.cc
+++ b/chrome/installer/setup/setup_util.cc
@@ -42,6 +42,8 @@
 #include "chrome/installer/util/installation_state.h"
 #include "chrome/installer/util/master_preferences.h"
 #include "chrome/installer/util/master_preferences_constants.h"
+#include "chrome/installer/util/non_updating_app_registration_data.h"
+#include "chrome/installer/util/updating_app_registration_data.h"
 #include "chrome/installer/util/util_constants.h"
 #include "courgette/courgette.h"
 #include "courgette/third_party/bsdiff/bsdiff.h"
@@ -67,13 +69,6 @@ base::string16 InstallFullName() {
 #endif
 }
 
-// Returns true if product |type| cam be meaningfully installed without the
-// --multi-install flag.
-bool SupportsSingleInstall(BrowserDistribution::Type type) {
-  return (type == BrowserDistribution::CHROME_BROWSER ||
-          type == BrowserDistribution::CHROME_FRAME);
-}
-
 // Returns true if the "lastrun" value in |root|\|key_path| (a path to Chrome's
 // ClientState key for a user) indicates that Chrome has been used within the
 // last 28 days.
@@ -132,6 +127,136 @@ bool OnUserHive(const base::string16& client_state_path,
   return false;
 }
 
+// "The binaries" once referred to the on-disk footprint of Chrome and/or Chrome
+// Frame when the products were configured to share such on-disk bits. Support
+// for this mode of install was dropped from ToT in December 2016. Remove any
+// stray bits in the registry leftover from such installs.
+void RemoveBinariesVersionKey(const InstallerState& installer_state) {
+#if defined(GOOGLE_CHROME_BUILD)
+  UpdatingAppRegistrationData reg_data(
+      L"{4DC8B4CA-1BDA-483e-B5FA-D3C12E15B62D}");
+#else
+  NonUpdatingAppRegistrationData reg_data(L"Software\\Chromium Binaries");
+#endif
+
+  base::string16 path(reg_data.GetVersionKey());

[gab, 2017/01/05 22:08:01]

Why was this called the "version key" again? That confused me just now... it's
really the "product key" or something, nothing to do with version..

[grt (UTC plus 2), 2017/01/06 10:25:53]

Historical accident, I think. It's been that way for, like, ever. It is the
subkey of the Google Update "Clients" key that holds the app's Product Version
("pv") value. I am starting to call it "ClientsKey" in new APIs (I think :-) ),
since it's more clear to me that it be named after what it is rather than
calling it the "VersionKey" just because it holds the version value.

+  if (base::win::RegKey(installer_state.root_key(), path.c_str(),
+                        KEY_QUERY_VALUE | KEY_WOW64_32KEY)
+          .Valid() &&
+      InstallUtil::DeleteRegistryKey(installer_state.root_key(), path,
+                                     KEY_WOW64_32KEY)) {
+    UMA_HISTOGRAM_BOOLEAN("Setup.Install.BinariesRemoved", true);
+  }
+}
+
+// Remove leftover traces of multi-install Chrome Frame, if present. Once upon a
+// time, Google Chrome Frame could be co-installed with Chrome such that they
+// shared the same binaries on disk. Support for new installs of GCF was dropped
+// from ToT in December 2013. Remove any stray bits in the registry leftover
+// from an old multi-install GCF.
+void RemoveMultiChromeFrame(const InstallerState& installer_state) {
+// There never was a "Chromium Frame".
+#if defined(GOOGLE_CHROME_BUILD)
+  // To maximize cleanup, unconditionally delete GCF's Clients and ClientState
+  // keys unless single-install GCF is present. This condition is satisfied if
+  // both keys exist, Clients\pv contains a value, and
+  // ClientState\UninstallString contains a path including "\Chrome
+  // Frame\". Multi-install GCF would have had "\Chrome\", and anything else is
+  // garbage.
+
+  UpdatingAppRegistrationData gcf_data(
+      L"{8BA986DA-5100-405E-AA35-86F34A02ACBF}");
+  base::win::RegKey clients_key;
+  base::win::RegKey client_state_key;
+
+  bool has_clients_key =
+      clients_key.Open(installer_state.root_key(),
+                       gcf_data.GetVersionKey().c_str(),
+                       KEY_QUERY_VALUE | KEY_WOW64_32KEY) == ERROR_SUCCESS;
+  bool has_client_state_key =
+      client_state_key.Open(installer_state.root_key(),
+                            gcf_data.GetStateKey().c_str(),
+                            KEY_QUERY_VALUE | KEY_WOW64_32KEY) == ERROR_SUCCESS;
+  if (!has_clients_key && !has_client_state_key)
+    return;  // Nothing to check or to clean.
+
+  base::string16 value;
+  if (has_clients_key && has_client_state_key &&
+      clients_key.ReadValue(google_update::kRegVersionField, &value) ==
+          ERROR_SUCCESS &&
+      !value.empty() &&
+      client_state_key.ReadValue(kUninstallStringField, &value) ==
+          ERROR_SUCCESS &&
+      value.find(L"\\Chrome Frame\\") != base::string16::npos) {

[gab, 2016/12/22 19:05:10]

Single Chrome Frame still exists? Frozen to some old version I guess? Was it
migrated off of multi back then (i.e. this is likely the vast majority of CFs
left and we're fine killing remaining multi CF?)

[grt (UTC plus 2), 2017/01/02 10:45:03]

Yes, and still has active users!
32.0.1700.107, to be exact.
Yes. Less than 0.4% of GCF installs (0.32% of actives) could be affected. I can
realistically count them between now and the end of my workday. :-)

[gab, 2017/01/05 22:08:01]

Ok but which code is this moving? I can't seem to find the old code that did
this already?

[grt (UTC plus 2), 2017/01/06 10:25:53]

UninstallMultiChromeFrameIfPresent in setup_main.cc used to more-or-less run an
Uninstall from the currently-running setup.exe. RemoveMultiChromeFrame is a
hack-and-slash to remove the most critical bits and allows for all other
CF-related code to be removed (BrowserDistribution, ProductOperations, etc).

For completeness:

The code to migrate multi-install Chrome Frame to single-install was added in
https://crrev.com/224600 (31.0.1639.0).
The code to uninstall multi-install Chrome Frame rather than migrate it was
added in https://crrev.com/238588 (33.0.1729.0).
The migration code went away when installation of Chrome Frame was removed in
https://crrev.com/242400 (34.0.1759.0).

+    return;  // Single-install Chrome Frame found.
+  }
+
+  // Remnants of multi-install GCF or of a malformed GCF are present. Remove the
+  // Clients and ClientState keys so that Google Update ceases to check for
+  // updates, and the Programs and Features control panel entry to reduce user
+  // confusion.
+  client_state_key.Close();
+  clients_key.Close();
+
+  InstallUtil::DeleteRegistryKey(installer_state.root_key(),
+                                 gcf_data.GetVersionKey(), KEY_WOW64_32KEY);
+  InstallUtil::DeleteRegistryKey(installer_state.root_key(),
+                                 gcf_data.GetStateKey(), KEY_WOW64_32KEY);
+  InstallUtil::DeleteRegistryKey(
+      installer_state.root_key(),
+      L"Software\\Microsoft\\Windows\\CurrentVersion\\Uninstall\\"
+      L"Google Chrome Frame",
+      KEY_WOW64_32KEY);
+
+  UMA_HISTOGRAM_BOOLEAN("Setup.Install.MultiChromeFrameRemoved", true);
+#endif
+}
+
+void RemoveAppLauncherVersionKey(const InstallerState& installer_state) {
+// The app launcher was only registered for Google Chrome.
+#if defined(GOOGLE_CHROME_BUILD)
+  UpdatingAppRegistrationData reg_data(
+      L"{FDA71E6F-AC4C-4a00-8B70-9958A68906BF}");
+
+  base::string16 path(reg_data.GetVersionKey());
+  if (base::win::RegKey(installer_state.root_key(), path.c_str(),
+                        KEY_QUERY_VALUE | KEY_WOW64_32KEY)
+          .Valid() &&
+      InstallUtil::DeleteRegistryKey(installer_state.root_key(), path,

[gab, 2016/12/22 19:05:10]

I'm not 100% sure about the destruction order here. Is the RegKey guaranteed to
be deleted (and thus closed) before InstallUtil::DeleteRegistryKey() or only
before the end of the whole conditional?

[grt (UTC plus 2), 2017/01/02 10:45:03]

"If the second expression is evaluated, every value computation and side effect
associated with the first expression is sequenced before every value computation
and side effect associated with the second expression." I'm pretty sure that's a
"yes".

[gab, 2017/01/05 22:08:01]

Cool beans :)

+                                     KEY_WOW64_32KEY)) {
+    UMA_HISTOGRAM_BOOLEAN("Setup.Install.AppLauncherRemoved", true);
+  }
+#endif  // GOOGLE_CHROME_BUILD
+}
+
+void RemoveAppHostExe(const InstallerState& installer_state) {
+// The app host was only installed for Google Chrome.
+#if defined(GOOGLE_CHROME_BUILD)
+  base::FilePath app_host(
+      installer_state.target_path().Append(FILE_PATH_LITERAL("app_host.exe")));
+
+  if (base::PathExists(app_host) && base::DeleteFile(app_host, false))
+    UMA_HISTOGRAM_BOOLEAN("Setup.Install.AppHostRemoved", true);
+#endif
+}
+
+void RemoveLegacyChromeAppCommands(const InstallerState& installer_state) {
+// These app commands were only registered for Google Chrome.
+#if defined(GOOGLE_CHROME_BUILD)
+  base::string16 path(GetRegistrationDataCommandKey(
+      installer_state.product().distribution()->GetAppRegistrationData(),
+      L"install-extension"));
+
+  if (base::win::RegKey(installer_state.root_key(), path.c_str(),
+                        KEY_QUERY_VALUE | KEY_WOW64_32KEY)
+          .Valid() &&
+      InstallUtil::DeleteRegistryKey(installer_state.root_key(), path,
+                                     KEY_WOW64_32KEY)) {
+    UMA_HISTOGRAM_BOOLEAN("Setup.Install.InstallExtensionCommandRemoved", true);

[gab, 2016/12/22 19:05:10]

Is UMA live until the very end of setup.exe? (DoLegacyCleanups() is called very
late)

[grt (UTC plus 2), 2017/01/02 10:45:03]

It's alive until the end of wWinMain (torn down in the dtor of the
|persistent_histogram_storage| instance on the stack).

+  }
+#endif
+}
+
 }  // namespace
 
 const char kUnPackStatusMetricsName[] = "Setup.Install.LzmaUnPackStatus";
@@ -302,32 +427,6 @@ bool DeleteFileFromTempProcess(const base::FilePath& path,
   return ok != FALSE;
 }
 
-// There are 4 disjoint cases => return values {false,true}:
-// (1) Product is being uninstalled => false.
-// (2) Product is being installed => true.
-// (3) Current operation ignores product, product is absent => false.
-// (4) Current operation ignores product, product is present => true.
-bool WillProductBePresentAfterSetup(
-    const installer::InstallerState& installer_state,
-    const installer::InstallationState& machine_state,
-    BrowserDistribution::Type type) {
-  DCHECK(SupportsSingleInstall(type) || installer_state.is_multi_install());
-
-  const ProductState* product_state =
-      machine_state.GetProductState(installer_state.system_install(), type);
-
-  // Determine if the product is present prior to the current operation.
-  bool is_present = (product_state != NULL);
-  bool is_uninstall = installer_state.operation() == InstallerState::UNINSTALL;
-
-  // Determine if current operation affects the product.
-  const Product* product = installer_state.FindProduct(type);
-  bool is_affected = (product != NULL);
-
-  // Decide among {(1),(2),(3),(4)}.
-  return is_affected ? !is_uninstall : is_present;
-}
-
 bool AdjustProcessPriority() {
   if (base::win::GetVersion() >= base::win::VERSION_VISTA) {
     DWORD priority_class = ::GetPriorityClass(::GetCurrentProcess());
@@ -732,6 +831,23 @@ void DeRegisterEventLogProvider() {
                                  WorkItem::kWow64Default);
 }
 
+void DoLegacyCleanups(const InstallerState& installer_state,
+                      InstallStatus install_status) {
+  // Do no harm if the install didn't succeed.
+  if (InstallUtil::GetInstallReturnCode(install_status))
+    return;
+
+  // The cleanups below only apply to normal Chrome, not side-by-side (canary).
+  if (InstallUtil::IsChromeSxSProcess())
+    return;
+
+  RemoveBinariesVersionKey(installer_state);
+  RemoveMultiChromeFrame(installer_state);
+  RemoveAppLauncherVersionKey(installer_state);
+  RemoveAppHostExe(installer_state);
+  RemoveLegacyChromeAppCommands(installer_state);
+}
+
 ScopedTokenPrivilege::ScopedTokenPrivilege(const wchar_t* privilege_name)
     : is_enabled_(false) {
   HANDLE temp_handle;