Product-specific flags are no longer required, used, or validated in --rename-chrome-exe command lines.

BUG=79935
TEST=Install version N-1 w/ --verbose-logging --chrome --chrome-frame --multi-install --ready-mode --system-level, navigate to gcf:about:version in IE, then install version N with same args.  Inspect chrome_frame_installer.log and note the lack of validation errors.  Close IE and note that the update completes and all "opv" and "cmd" values are removed from the various ClientState keys.

Review URL: http://codereview.chromium.org/6951016

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@84641 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 63 insertions, 46 deletions

diff --git a/chrome/installer/setup/install_worker.cc b/chrome/installer/setup/install_worker.cc
index b14880f..4bf0802 100644
--- a/chrome/installer/setup/install_worker.cc
+++ b/chrome/installer/setup/install_worker.cc
@@ -423,6 +423,8 @@ bool AppendPostInstallTasks(const InstallerState& installer_state,
     rename.AppendSwitch(switches::kRenameChromeExe);
     if (installer_state.system_install())
       rename.AppendSwitch(switches::kSystemLevel);
+    if (installer_state.is_multi_install())
+      rename.AppendSwitch(switches::kMultiInstall);
 
     if (installer_state.verbose_logging())
       rename.AppendSwitch(switches::kVerboseLogging);
@@ -440,16 +442,14 @@ bool AppendPostInstallTasks(const InstallerState& installer_state,
 
       // Adding this registry entry for all products is overkill.
       // However, as it stands, we don't have a way to know which distribution
-      // will check the key and run the command, so we add it for all.
-      // After the first run, the subsequent runs should just be noops.
-      // (see upgrade_utils::SwapNewChromeExeIfPresent).
-      CommandLine product_rename_cmd(rename);
-      products[i]->AppendProductFlags(&product_rename_cmd);
+      // will check the key and run the command, so we add it for all.  The
+      // first to run it will perform the operation and clean up the other
+      // values.
       in_use_update_work_items->AddSetRegValueWorkItem(
           root,
           version_key,
           google_update::kRegRenameCmdField,
-          product_rename_cmd.command_line_string(),
+          rename.command_line_string(),
           true);
     }
 
diff --git a/chrome/installer/setup/setup_main.cc b/chrome/installer/setup/setup_main.cc
index 15d04e1..60a570d 100644
--- a/chrome/installer/setup/setup_main.cc
+++ b/chrome/installer/setup/setup_main.cc
@@ -141,14 +141,48 @@ DWORD UnPackArchive(const FilePath& archive,
       output_directory.value(), &unpacked_file);
 }
 
+// In multi-install, adds all products to |installer_state| that are
+// multi-installed and must be updated along with the products already present
+// in |installer_state|.
+void AddExistingMultiInstalls(const InstallationState& original_state,
+                              InstallerState* installer_state) {
+  if (installer_state->is_multi_install()) {
+    // TODO(grt): Find all occurrences of such arrays and generalize/centralize.
+    BrowserDistribution::Type product_checks[] = {
+      BrowserDistribution::CHROME_BROWSER,
+      BrowserDistribution::CHROME_FRAME
+    };
+
+    for (size_t i = 0; i < arraysize(product_checks); ++i) {
+      BrowserDistribution::Type type = product_checks[i];
+      if (!installer_state->FindProduct(type)) {
+        const ProductState* state =
+            original_state.GetProductState(installer_state->system_install(),
+                                           type);
+        if ((state != NULL) && state->is_multi_install()) {
+          installer_state->AddProductFromState(type, *state);
+          VLOG(1) << "Product already installed and must be included: "
+                  << BrowserDistribution::GetSpecificDistribution(
+                         type)->GetApplicationName();
+        }
+      }
+    }
+  }
+}
+
 // 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
 // system and a key called 'opv' in the registry. This function will move
 // new_chrome.exe to chrome.exe and delete 'opv' key in one atomic operation.
 installer::InstallStatus RenameChromeExecutables(
-    const InstallerState& installer_state) {
-  const FilePath &target_path = installer_state.target_path();
+    const InstallationState& original_state,
+    InstallerState* installer_state) {
+  // See what products are already installed in multi mode.  When we do the
+  // rename for multi installs, we must update all installations since they
+  // share the binaries.
+  AddExistingMultiInstalls(original_state, installer_state);
+  const FilePath &target_path = installer_state->target_path();
   FilePath chrome_exe(target_path.Append(installer::kChromeExe));
   FilePath chrome_new_exe(target_path.Append(installer::kChromeNewExe));
   FilePath chrome_old_exe(target_path.Append(installer::kChromeOldExe));
@@ -176,12 +210,26 @@ installer::InstallStatus RenameChromeExecutables(
   install_list->AddDeleteTreeWorkItem(chrome_old_exe, temp_path.path())
       ->set_ignore_failure(true);
 
-  HKEY reg_root = installer_state.root_key();
-  const Products& products = installer_state.products();
-  for (size_t i = 0; i < products.size(); ++i) {
-    const Product* product = products[i];
-    BrowserDistribution* browser_dist = product->distribution();
-    std::wstring version_key(browser_dist->GetVersionKey());
+  // Collect the set of distributions we need to update, which is the
+  // multi-install binaries (if this is a multi-install operation) and all
+  // products we're operating on.
+  BrowserDistribution* dists[BrowserDistribution::NUM_TYPES];
+  int num_dists = 0;
+  // First, add the multi-install binaries, if relevant.
+  if (installer_state->is_multi_install())
+    dists[num_dists++] = installer_state->multi_package_binaries_distribution();
+  // Next, add all products we're operating on.  std::transform can handily do
+  // this for us, but this is discouraged as being too tricky.
+  const Products& products = installer_state->products();
+  for (Products::size_type i = 0; i < products.size(); ++i) {
+    dists[num_dists++] = products[i]->distribution();
+  }
+
+  // Add work items to delete the "opv" and "cmd" values from all distributions.
+  HKEY reg_root = installer_state->root_key();
+  std::wstring version_key;
+  for (int i = 0; i < num_dists; ++i) {
+    version_key = dists[i]->GetVersionKey();
     install_list->AddDeleteRegValueWorkItem(reg_root,
                                             version_key,
                                             google_update::kRegOldVersionField);
@@ -329,34 +377,6 @@ bool CheckMultiInstallConditions(const InstallationState& original_state,
   return true;
 }
 
-// In multi-install, adds all products to |installer_state| that are
-// multi-installed and must be updated along with the products already present
-// in |installer_state|.
-void AddExistingMultiInstalls(const InstallationState& original_state,
-                              InstallerState* installer_state) {
-  if (installer_state->is_multi_install()) {
-    BrowserDistribution::Type product_checks[] = {
-      BrowserDistribution::CHROME_BROWSER,
-      BrowserDistribution::CHROME_FRAME
-    };
-
-    for (size_t i = 0; i < arraysize(product_checks); ++i) {
-      BrowserDistribution::Type type = product_checks[i];
-      if (!installer_state->FindProduct(type)) {
-        const ProductState* state =
-            original_state.GetProductState(installer_state->system_install(),
-                                           type);
-        if ((state != NULL) && state->is_multi_install()) {
-          installer_state->AddProductFromState(type, *state);
-          VLOG(1) << "Product already installed and must be included: "
-                  << BrowserDistribution::GetSpecificDistribution(
-                         type)->GetApplicationName();
-        }
-      }
-    }
-  }
-}
-
 // Checks for compatibility between the current state of the system and the
 // desired operation.  Also applies policy that mutates the desired operation;
 // specifically, the |installer_state| object.
@@ -854,7 +874,7 @@ bool HandleNonInstallCmdLineOptions(const InstallationState& original_state,
   } else if (cmd_line.HasSwitch(installer::switches::kRenameChromeExe)) {
     // If --rename-chrome-exe is specified, we want to rename the executables
     // and exit.
-    *exit_code = RenameChromeExecutables(*installer_state);
+    *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.
diff --git a/chrome/installer/util/installation_validator.cc b/chrome/installer/util/installation_validator.cc
index c9cd663..fdc9a2a 100644
--- a/chrome/installer/util/installation_validator.cc
+++ b/chrome/installer/util/installation_validator.cc
@@ -397,9 +397,6 @@ void InstallationValidator::ValidateRenameCommand(const ProductContext& ctx,
                                     ctx.system_install));
   expected.push_back(std::make_pair(std::string(switches::kMultiInstall),
                                     ctx.state.is_multi_install()));
-  ctx.rules.AddProductSwitchExpectations(ctx.machine_state,
-                                         ctx.system_install,
-                                         ctx.state, &expected);
 
   ValidateCommandExpectations(ctx, command, expected, "in-use renamer",
                               is_valid);