Support --delete-old-versions in setup.exe.

chrome/installer/setup/setup_main.cc

Index: chrome/installer/setup/setup_main.cc
diff --git a/chrome/installer/setup/setup_main.cc b/chrome/installer/setup/setup_main.cc
index 1f4e2688c09bee1445f6fe79163c96ab193587d3..6508dbfb54e180d52cd7ce7c1c815f6cdbeb18ee 100644
--- a/chrome/installer/setup/setup_main.cc
+++ b/chrome/installer/setup/setup_main.cc
@@ -56,6 +56,7 @@
 #include "chrome/installer/setup/uninstall.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/delete_after_reboot_helper.h"
+#include "chrome/installer/util/delete_old_versions.h"
 #include "chrome/installer/util/delete_tree_work_item.h"
 #include "chrome/installer/util/google_update_constants.h"
 #include "chrome/installer/util/google_update_settings.h"
@@ -386,6 +387,37 @@ void AddExistingMultiInstalls(const InstallationState& original_state,
   }
 }
 
+// Repetitively attempts to delete all files that belong to old versions of
+// Chrome from |install_dir|. Waits 15 seconds before the first attempt and 5
+// minutes after each unsuccessful attempt. Returns when no files that belong to
+// an old version of Chrome remain or when another process tries to acquire the
+// SetupSingleton.
+installer::InstallStatus RepeatDeleteOldVersionsUntilSuccess(

[grt (UTC plus 2), 2016/09/13 08:10:40]

it looks like we'll be blind to how this performs. how difficult would it be to
have persistent histograms written for this case so that you can use UMA in here
to log things like the # of long sleeps needed before termination, and whether
termination was due to success or interrupt (see the call to
EndPersistentHistogramStorage at the end of wWinMain)?

[fdoray, 2016/09/13 21:34:54]

Done.

+    const base::FilePath& install_dir,
+    const installer::SetupSingleton& setup_singleton) {
+  // Wait 15 seconds because trying to delete old files right away is likely to
+  // fail. Indeed, this is called in 2 occasions:
+  // - When the installer fails to delete old files after a not-in-use update:
+  //   retrying immediately is likely to fail again.
+  // - When executables are successfully renamed on Chrome startup or shutdown:
+  //   old files can't be deleted because Chrome is still in use.
+  if (setup_singleton.WaitForInterrupt(base::TimeDelta::FromSeconds(15)))
+    return installer::SETUP_SINGLETON_RELEASED;
+
+  for (;;) {

[grt (UTC plus 2), 2016/09/13 08:10:40]

why not?
  while (true) {

[fdoray, 2016/09/13 21:34:54]

Done.

+    if (installer::DeleteOldVersions(install_dir)) {
+      LOG(INFO) << "Successfully deleted old files from --delete-old-versions "

[grt (UTC plus 2), 2016/09/13 08:10:40]

please use VLOG(1) here and below

[fdoray, 2016/09/13 21:34:54]

Done.

+                   "process.";
+      return installer::DELETE_OLD_VERSIONS_SUCCESS;
+    }
+
+    LOG(INFO) << "Failed to delete old files from --delete-old-versions "

[grt (UTC plus 2), 2016/09/13 08:10:40]

does this case mean that no old files were deleted, or that not all old files
were deleted? if the latter, please change to "Failed to delete all old..."

[fdoray, 2016/09/13 21:34:54]

Done. It means that not *all* old files were deleted. Some files may have been
deleted.

+                 "process. Will try again in 5 minutes.";
+    if (setup_singleton.WaitForInterrupt(base::TimeDelta::FromMinutes(5)))
+      return installer::SETUP_SINGLETON_RELEASED;
+  }
+}
+
 // This function is called when --rename-chrome-exe option is specified on
 // setup.exe command line. This function assumes an in-use update has happened
 // for Chrome so there should be a file called new_chrome.exe on the file
@@ -454,7 +486,9 @@ installer::InstallStatus RenameChromeExecutables(
       ->set_best_effort(true);
 
   installer::InstallStatus ret = installer::RENAME_SUCCESSFUL;
-  if (!install_list->Do()) {
+  if (install_list->Do()) {
+    installer::LaunchDeleteOldVersionsProcess(*installer_state);

[grt (UTC plus 2), 2016/09/13 08:10:40]

in this specific case, the current setup.exe (PathService::Get(base::FILE_EXE))
is the right one. so perhaps fetch that here and pass it in, or have
HandleNonInstallCmdLineOptions pass it into this fn to be passed along.

[fdoray, 2016/09/13 21:34:54]

Done.

+  } else {
     LOG(ERROR) << "Renaming of executables failed. Rolling back any changes.";
     install_list->Rollback();
     ret = installer::RENAME_FAILED;
@@ -1236,17 +1270,21 @@ bool HandleNonInstallCmdLineOptions(const base::FilePath& setup_exe,
       LOG(DFATAL) << "Can't register browser - Chrome distribution not found";
     }
     *exit_code = InstallUtil::GetInstallReturnCode(status);
-  } else if (cmd_line.HasSwitch(installer::switches::kRenameChromeExe)) {
-    // If --rename-chrome-exe is specified, we want to rename the executables
-    // and exit.
+  } else if (cmd_line.HasSwitch(installer::switches::kDeleteOldVersions) ||
+             cmd_line.HasSwitch(installer::switches::kRenameChromeExe)) {
     std::unique_ptr<installer::SetupSingleton> setup_singleton(
         installer::SetupSingleton::Acquire(
             cmd_line, MasterPreferences::ForCurrentProcess(), original_state,
             installer_state));
-    if (!setup_singleton)
+    if (!setup_singleton) {
       *exit_code = installer::SETUP_SINGLETON_ACQUISITION_FAILED;
-    else
+    } else if (cmd_line.HasSwitch(installer::switches::kDeleteOldVersions)) {
+      *exit_code = RepeatDeleteOldVersionsUntilSuccess(
+          installer_state->target_path(), *setup_singleton);
+    } else {
+      DCHECK(cmd_line.HasSwitch(installer::switches::kRenameChromeExe));
       *exit_code = RenameChromeExecutables(*original_state, installer_state);
+    }
   } else if (cmd_line.HasSwitch(
                  installer::switches::kRemoveChromeRegistration)) {
     // This is almost reverse of --register-chrome-browser option above.