C++ reability review for gab.

chrome/installer/setup/install_worker.cc

 // Copyright (c) 2012 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.
 //
 // This file contains the definitions of the installer functions that build
 // the WorkItemList used to install the application.
 
 #include "chrome/installer/setup/install_worker.h"
 
 #include <oaidl.h>
@@ skipping 70 matching lines @@
                               arraysize(cf_clsid));
     key_path->append(&cf_clsid[0], len - 1);
   } else {
     key_path->append(kIELowRightsPolicyOldGuid,
                      arraysize(kIELowRightsPolicyOldGuid)- 1);
   }
 }
 
 // Local helper to call AddRegisterComDllWorkItems for all DLLs in a set of
 // products managed by a given package.
+// |old_version| can be NULL to indicate no Chrome is currently installed.
 void AddRegisterComDllWorkItemsForPackage(const InstallerState& installer_state,
                                           const Version* old_version,
                                           const Version& new_version,
                                           WorkItemList* work_item_list) {
   // First collect the list of DLLs to be registered from each product.
   std::vector<FilePath> com_dll_list;
   installer_state.AddComDllList(&com_dll_list);
 
   // Then, if we got some, attempt to unregister the DLLs from the old
   // version directory and then re-register them in the new one.
@@ skipping 62 matching lines @@
     // setup.exe cannot be run again without regenerating the archive, so copy
     // it instead in this case to save developer time.
     install_list->AddCopyTreeWorkItem(archive_path.value(), archive_dst.value(),
                                       temp_path.value(), WorkItem::ALWAYS);
 #endif  // COMPONENT_BUILD
   }
 }
 
 void AddInstallAppCommandWorkItems(const InstallerState& installer_state,
                                    const InstallationState& machine_state,
-                                   const FilePath* setup_path,
-                                   const Version* new_version,
+                                   const FilePath& setup_path,
+                                   const Version& new_version,
                                    const Product& product,
                                    WorkItemList* work_item_list) {
   DCHECK(product.is_chrome_app_host());
 
   string16 cmd_key(product.distribution()->GetVersionKey());
   cmd_key.append(1, L'\\').append(google_update::kRegCommandsKey)
       .append(1, L'\\').append(kCmdInstallApp);
 
-  if (installer_state.operation() != InstallerState::UNINSTALL) {
-    FilePath target_path(installer_state.target_path());
-    CommandLine cmd_line(target_path.Append(installer::kChromeAppHostExe));
+  if (installer_state.operation() == InstallerState::UNINSTALL) {
+    work_item_list->AddDeleteRegKeyWorkItem(
+        installer_state.root_key(), cmd_key)->set_log_message(
+            "removing install-application command");
+  } else {
+    CommandLine cmd_line(
+        installer_state.target_path().Append(installer::kChromeAppHostExe));
     cmd_line.AppendSwitchASCII(::switches::kAppsInstallFromManifestURL, "%1");
 
     AppCommand cmd(cmd_line.GetCommandLineString());
     cmd.set_sends_pings(true);
     cmd.set_is_web_accessible(true);
     cmd.AddWorkItems(installer_state.root_key(), cmd_key, work_item_list);
-  } else {
-    work_item_list->AddDeleteRegKeyWorkItem(installer_state.root_key(),
-                                            cmd_key)->set_log_message(
-        "removing install-application command");
   }
 }
 
 void AddProductSpecificWorkItems(const InstallationState& original_state,
                                  const InstallerState& installer_state,
                                  const FilePath& setup_path,
                                  const Version& new_version,
                                  WorkItemList* list) {
   const Products& products = installer_state.products();
-  for (size_t i = 0; i < products.size(); ++i) {
-    const Product& p = *products[i];
+  for (Products::const_iterator it = products.begin(); it < products.end();
+       ++it) {
+    const Product& p = **it;
     if (p.is_chrome_frame()) {
       AddChromeFrameWorkItems(original_state, installer_state, setup_path,
                               new_version, p, list);
     }
     if (p.is_chrome_app_host()) {
       AddInstallAppCommandWorkItems(installer_state, original_state,
-                                    &setup_path, &new_version, p, list);
+                                    setup_path, new_version, p, list);
     }
     if (p.is_chrome()) {
-      AddOsUpgradeWorkItems(installer_state, &setup_path, &new_version, p,
+      AddOsUpgradeWorkItems(installer_state, setup_path, new_version, p,
                             list);
     }
   }
 }
 
 // This is called when an MSI installation is run. It may be that a user is
 // attempting to install the MSI on top of a non-MSI managed installation.
 // If so, try and remove any existing uninstallation shortcuts, as we want the
 // uninstall to be managed entirely by the MSI machinery (accessible via the
 // Add/Remove programs dialog).
@@ skipping 31 matching lines @@
     VLOG(1) << "Deleting old uninstall shortcut (if present): "
             << uninstall_link.value();
     WorkItem* delete_link = work_item_list->AddDeleteTreeWorkItem(
         uninstall_link, temp_path);
     delete_link->set_ignore_failure(true);
     delete_link->set_log_message(
         "Failed to delete old uninstall shortcut.");
   }
 }
 
+// Adds Chrome specific install work items to |install_list|.
+// |current_version| can be NULL to indicate no Chrome is currently installed.
 void AddChromeWorkItems(const InstallationState& original_state,
                         const InstallerState& installer_state,
                         const FilePath& setup_path,
                         const FilePath& archive_path,
                         const FilePath& src_path,
                         const FilePath& temp_path,
+                        const Version* current_version,
                         const Version& new_version,
-                        scoped_ptr<Version>* current_version,
                         WorkItemList* install_list) {
   const FilePath& target_path = installer_state.target_path();
 
-  if (current_version != NULL && current_version->get() != NULL) {
+  if (current_version) {
     // Delete the archive from an existing install to save some disk space.  We
     // make this an unconditional work item since there's no need to roll this
     // back; if installation fails we'll be moved to the "-full" channel anyway.
     FilePath old_installer_dir(
-        installer_state.GetInstallerDirectory(**current_version));
+        installer_state.GetInstallerDirectory(*current_version));
     FilePath old_archive(old_installer_dir.Append(installer::kChromeArchive));
     // Don't delete the archive that we are actually installing from.
     if (archive_path != old_archive) {
-      install_list->AddDeleteTreeWorkItem(old_archive, temp_path)
-          ->set_ignore_failure(true);
+      install_list->AddDeleteTreeWorkItem(old_archive, temp_path)->
+          set_ignore_failure(true);
     }
   }
 
   // Delete any new_chrome.exe if present (we will end up creating a new one
   // if required) and then copy chrome.exe
   FilePath new_chrome_exe(target_path.Append(installer::kChromeNewExe));
 
   install_list->AddDeleteTreeWorkItem(new_chrome_exe, temp_path);
 
   if (installer_state.IsChromeFrameRunning(original_state)) {
@@ skipping 63 matching lines @@
       temp_path.value(),
       WorkItem::ALWAYS_MOVE);
 #endif  // defined(COMPONENT_BUILD)
 
   // In the past, we copied rather than moved for system level installs so that
   // the permissions of %ProgramFiles% would be picked up.  Now that |temp_path|
   // is in %ProgramFiles% for system level installs (and in %LOCALAPPDATA%
   // otherwise), there is no need to do this.
   // Note that we pass true for check_duplicates to avoid failing on in-use
   // repair runs if the current_version is the same as the new_version.
-  bool check_for_duplicates =
-      (current_version != NULL && current_version->get() != NULL &&
-       current_version->get()->Equals(new_version));
+  bool check_for_duplicates = (current_version &&
+                               current_version->Equals(new_version));
   install_list->AddMoveTreeWorkItem(
       src_path.AppendASCII(new_version.GetString()).value(),
       target_path.AppendASCII(new_version.GetString()).value(),
       temp_path.value(),
       check_for_duplicates ? WorkItem::CHECK_DUPLICATES :
                              WorkItem::ALWAYS_MOVE);
 
   // Delete any old_chrome.exe if present (ignore failure if it's in use).
   install_list->AddDeleteTreeWorkItem(
-      target_path.Append(installer::kChromeOldExe), temp_path)
-      ->set_ignore_failure(true);
+      target_path.Append(installer::kChromeOldExe), temp_path)->
+          set_ignore_failure(true);
 
   // Copy installer in install directory and
   // add shortcut in Control Panel->Add/Remove Programs.
   AddInstallerCopyTasks(installer_state, setup_path, archive_path, temp_path,
                         new_version, install_list);
 }
 
 // Probes COM machinery to get an instance of delegate_execute.exe's
 // CommandExecuteImpl class.  This is required so that COM purges its cache of
 // the path to the binary, which changes on updates.  This callback
@@ skipping 19 matching lines @@
       LOG(ERROR) << "Unexpected result creating CommandExecuteImpl; hr=0x"
                  << std::hex << hr;
     }
   }
 
   return true;
 }
 
 void AddGenericQuickEnableWorkItems(const InstallerState& installer_state,
                                     const InstallationState& machine_state,
-                                    const FilePath* setup_path,
-                                    const Version* new_version,
-                                    WorkItemList* work_item_list,
+                                    const FilePath& setup_path,
+                                    const Version& new_version,
                                     bool have_child_product,
                                     const CommandLine& child_product_switches,
-                                    const string16& command_id) {
-  DCHECK(setup_path ||
-         installer_state.operation() == InstallerState::UNINSTALL);
-  DCHECK(new_version ||
-         installer_state.operation() == InstallerState::UNINSTALL);
+                                    const string16& command_id,
+                                    WorkItemList* work_item_list) {
   DCHECK(work_item_list);
 
   const bool system_install = installer_state.system_install();
   bool have_chrome_binaries = false;
 
   // STEP 1: Figure out the state of the machine before the operation.
   const ProductState* product_state = NULL;
 
   // Are the Chrome Binaries already on the machine?
-  product_state =
-      machine_state.GetProductState(system_install,
-                                    BrowserDistribution::CHROME_BINARIES);
+  product_state = machine_state.GetProductState(
+      system_install, BrowserDistribution::CHROME_BINARIES);
   if (product_state != NULL && product_state->is_multi_install())
     have_chrome_binaries = true;
 
   // STEP 2: Now take into account the current operation.
   const Product* product = NULL;
 
   if (installer_state.operation() == InstallerState::UNINSTALL) {
     // Forget about multi-install Chrome if it is being uninstalled.
-    product =
-        installer_state.FindProduct(BrowserDistribution::CHROME_BINARIES);
+    product = installer_state.FindProduct(BrowserDistribution::CHROME_BINARIES);
     if (product != NULL && installer_state.is_multi_install())
       have_chrome_binaries = false;
   } else {
     // Check if we're installing Chrome Binaries
-    product =
-        installer_state.FindProduct(BrowserDistribution::CHROME_BINARIES);
+    product = installer_state.FindProduct(BrowserDistribution::CHROME_BINARIES);
     if (product != NULL && installer_state.is_multi_install())
       have_chrome_binaries = true;
   }
 
   // STEP 3: Decide what to do based on the final state of things.
   enum QuickEnableOperation {
     DO_NOTHING,
     ADD_COMMAND,
     REMOVE_COMMAND
   } operation = DO_NOTHING;
   FilePath binaries_setup_path;
 
   if (have_child_product) {
     // Child product is being uninstalled. Unconditionally remove the Quick
     // Enable command from the binaries. We do this even if multi-install Chrome
     // isn't installed since we don't want them left behind in any case.
     operation = REMOVE_COMMAND;
   } else if (have_chrome_binaries) {
     // Child product isn't (to be) installed while multi-install Chrome is (to
     // be) installed.  Add the Quick Enable command to the binaries.
     operation = ADD_COMMAND;
     // The path to setup.exe contains the version of the Chrome binaries, so it
     // takes a little work to get it right.
     if (installer_state.operation() == InstallerState::UNINSTALL) {
       // One or more products are being uninstalled, but not the binaries. Use
       // the path to the currently installed Chrome setup.exe.
-      product_state =
-          machine_state.GetProductState(system_install,
-                                        BrowserDistribution::CHROME_BINARIES);
+      product_state = machine_state.GetProductState(
+          system_install, BrowserDistribution::CHROME_BINARIES);
       DCHECK(product_state);
       binaries_setup_path = product_state->uninstall_command().GetProgram();
     } else {
       // Chrome Binaries are being installed, updated, or otherwise operated on.
       // Use the path to the given |setup_path| in the normal location of
       // multi-install Chrome Binaries of the given |version|.
       DCHECK(installer_state.is_multi_install());
       binaries_setup_path =
-          installer_state.GetInstallerDirectory(*new_version).Append(
-              setup_path->BaseName());
+          installer_state.GetInstallerDirectory(new_version).Append(
+              setup_path.BaseName());
     }
   }
 
   // STEP 4: Take action.
   if (operation != DO_NOTHING) {
     // Get the path to the quick-enable-cf command for the binaries.
     BrowserDistribution* binaries =
         BrowserDistribution::GetSpecificDistribution(
             BrowserDistribution::CHROME_BINARIES);
     string16 cmd_key(binaries->GetVersionKey());
     cmd_key.append(1, L'\\').append(google_update::kRegCommandsKey)
         .append(1, L'\\').append(command_id);
 
     if (operation == ADD_COMMAND) {
       DCHECK(!binaries_setup_path.empty());
       CommandLine cmd_line(binaries_setup_path);
       cmd_line.AppendArguments(child_product_switches,
                                false);  // include_program
       if (installer_state.verbose_logging())
         cmd_line.AppendSwitch(switches::kVerboseLogging);
       AppCommand cmd(cmd_line.GetCommandLineString());
       cmd.set_sends_pings(true);
       cmd.set_is_web_accessible(true);
       cmd.AddWorkItems(installer_state.root_key(), cmd_key, work_item_list);
     } else {
       DCHECK(operation == REMOVE_COMMAND);
-      work_item_list->AddDeleteRegKeyWorkItem(installer_state.root_key(),
-                                              cmd_key)->set_log_message(
-          "removing " + WideToASCII(command_id) + " command");
+      work_item_list->AddDeleteRegKeyWorkItem(
+          installer_state.root_key(), cmd_key)->set_log_message(
+              "removing " + WideToASCII(command_id) + " command");
     }
   }
 }
 
 }  // namespace
 
 // This method adds work items to create (or update) Chrome uninstall entry in
 // either the Control Panel->Add/Remove Programs list or in the Omaha client
 // state key if running under an MSI installer.
 void AddUninstallShortcutWorkItems(const InstallerState& installer_state,
                                    const FilePath& setup_path,
                                    const Version& new_version,
-                                   WorkItemList* install_list,
-                                   const Product& product) {
+                                   const Product& product,
+                                   WorkItemList* install_list) {
   HKEY reg_root = installer_state.root_key();
   BrowserDistribution* browser_dist = product.distribution();
   DCHECK(browser_dist);
 
   // When we are installed via an MSI, we need to store our uninstall strings
   // in the Google Update client state key. We do this even for non-MSI
   // managed installs to avoid breaking the edge case whereby an MSI-managed
   // install is updated by a non-msi installer (which would confuse the MSI
   // machinery if these strings were not also updated). The UninstallString
   // value placed in the client state key is also used by the mini_installer to
@@ skipping 152 matching lines @@
       // Nothing is being installed?  Entirely unexpected, so do no harm.
       LOG(ERROR) << "No products found in AddOemInstallWorkItems";
       return;
     }
     const ProductState* source_product =
         original_state.GetNonVersionedProductState(system_install, source_type);
 
     string16 oem_install;
     if (source_product->GetOemInstall(&oem_install)) {
       VLOG(1) << "Mirroring oeminstall=\"" << oem_install << "\" from "
-              << BrowserDistribution::GetSpecificDistribution(source_type)
-                     ->GetAppShortCutName();
+              << BrowserDistribution::GetSpecificDistribution(source_type)->
+                    GetAppShortCutName();
       install_list->AddCreateRegKeyWorkItem(root_key, multi_key);
       // Always overwrite an old value.
       install_list->AddSetRegValueWorkItem(root_key, multi_key,
                                            google_update::kRegOemInstallField,
                                            oem_install, true);
     } else {
       // Clear any old value.
       install_list->AddDeleteRegValueWorkItem(
           root_key, multi_key, google_update::kRegOemInstallField);
     }
@@ skipping 13 matching lines @@
                                       BrowserDistribution::CHROME_BINARIES)) {
     const HKEY root_key = installer_state.root_key();
     string16 multi_key(
         installer_state.multi_package_binaries_distribution()->GetStateKey());
 
     // Copy the value from the product with the greatest value.
     bool have_eula_accepted = false;
     BrowserDistribution::Type product_type;
     DWORD eula_accepted;
     const Products& products = installer_state.products();
-    for (size_t i = 0, count = products.size(); i != count; ++i) {
-      if (products[i]->is_chrome_binaries())
+    for (Products::const_iterator it = products.begin(); it < products.end();
+         ++it) {
+      const Product& product = **it;
+      if (product.is_chrome_binaries())
         continue;
       DWORD dword_value = 0;
-      BrowserDistribution::Type this_type =
-          products[i]->distribution()->GetType();
+      BrowserDistribution::Type this_type = product.distribution()->GetType();
       const ProductState* product_state =
           original_state.GetNonVersionedProductState(
               system_install, this_type);
       if (product_state->GetEulaAccepted(&dword_value) &&
           (!have_eula_accepted || dword_value > eula_accepted)) {
         have_eula_accepted = true;
         eula_accepted = dword_value;
         product_type = this_type;
       }
     }
 
     if (have_eula_accepted) {
       VLOG(1) << "Mirroring eulaaccepted=" << eula_accepted << " from "
-              << BrowserDistribution::GetSpecificDistribution(product_type)
-                     ->GetAppShortCutName();
+              << BrowserDistribution::GetSpecificDistribution(product_type)->
+                     GetAppShortCutName();
       install_list->AddCreateRegKeyWorkItem(root_key, multi_key);
       install_list->AddSetRegValueWorkItem(
           root_key, multi_key, google_update::kRegEULAAceptedField,
           eula_accepted, true);
     } else {
       // Clear any old value.
       install_list->AddDeleteRegValueWorkItem(
           root_key, multi_key, google_update::kRegEULAAceptedField);
     }
   }
@@ skipping 39 matching lines @@
       // if one is already present (although this shouldn't happen).
       install_list->AddSetRegValueWorkItem(root_key,
                                            multi_key,
                                            google_update::kRegBrandField,
                                            brand,
                                            false);
     }
   }
 
   AddOemInstallWorkItems(original_state, installer_state, install_list);
-
   AddEulaAcceptedWorkItems(original_state, installer_state, install_list);
-
   AddUsageStatsWorkItems(original_state, installer_state, install_list);
 
   // TODO(grt): check for other keys/values we should put in the package's
   // ClientState and/or Clients key.
 }
 
 void AddUsageStatsWorkItems(const InstallationState& original_state,
                             const InstallerState& installer_state,
                             WorkItemList* install_list) {
   DCHECK(installer_state.operation() == InstallerState::MULTI_INSTALL ||
@@ skipping 36 matching lines @@
       if (installer_state.system_install()) {
         install_list->AddDeleteRegValueWorkItem(
             root_key, dist->GetStateMediumKey(),
             google_update::kRegUsageStatsField);
         // Previous versions of Chrome also wrote a value in HKCU even for
         // system-level installs, so clean that up.
         install_list->AddDeleteRegValueWorkItem(
             HKEY_CURRENT_USER, dist->GetStateKey(),
             google_update::kRegUsageStatsField);
       }
-      install_list->AddDeleteRegValueWorkItem(root_key, dist->GetStateKey(),
-          google_update::kRegUsageStatsField);
+      install_list->AddDeleteRegValueWorkItem(
+          root_key, dist->GetStateKey(), google_update::kRegUsageStatsField);
     }
   }
 }
 
-// After a successful copying of all the files, this function is called to
-// do a few post install tasks:
-// - Handle the case of in-use-update by updating "opv" (old version) key or
-//   deleting it if not required.
-// - Register any new dlls and unregister old dlls.
-// - If this is an MSI install, ensures that the MSI marker is set, and sets
-//   it if not.
-// If these operations are successful, the function returns true, otherwise
-// false.
 bool AppendPostInstallTasks(const InstallerState& installer_state,
                             const FilePath& setup_path,
                             const Version* current_version,
                             const Version& new_version,
                             const FilePath& temp_path,
                             WorkItemList* post_install_task_list) {
   DCHECK(post_install_task_list);
 
   HKEY root = installer_state.root_key();
   const Products& products = installer_state.products();
@@ skipping 23 matching lines @@
       rename.AppendSwitch(switches::kSystemLevel);
 
     if (installer_state.verbose_logging())
       rename.AppendSwitch(switches::kVerboseLogging);
 
     string16 version_key;
     for (size_t i = 0; i < products.size(); ++i) {
       BrowserDistribution* dist = products[i]->distribution();
       version_key = dist->GetVersionKey();
 
-      if (current_version != NULL) {
+      if (current_version) {
         in_use_update_work_items->AddSetRegValueWorkItem(root, version_key,
             google_update::kRegOldVersionField,
             ASCIIToWide(current_version->GetString()), true);
       }
       if (critical_version.IsValid()) {
         in_use_update_work_items->AddSetRegValueWorkItem(root, version_key,
             google_update::kRegCriticalVersionField,
             ASCIIToWide(critical_version.GetString()), true);
       } else {
         in_use_update_work_items->AddDeleteRegValueWorkItem(root, version_key,
@@ skipping 74 matching lines @@
 
   return true;
 }
 
 void AddInstallWorkItems(const InstallationState& original_state,
                          const InstallerState& installer_state,
                          const FilePath& setup_path,
                          const FilePath& archive_path,
                          const FilePath& src_path,
                          const FilePath& temp_path,
+                         const Version* current_version,
                          const Version& new_version,
-                         scoped_ptr<Version>* current_version,
                          WorkItemList* install_list) {
   DCHECK(install_list);
 
   const FilePath& target_path = installer_state.target_path();
 
   // A temp directory that work items need and the actual install directory.
   install_list->AddCreateDirWorkItem(temp_path);
   install_list->AddCreateDirWorkItem(target_path);
 
   if (installer_state.FindProduct(BrowserDistribution::CHROME_BROWSER) ||
       installer_state.FindProduct(BrowserDistribution::CHROME_FRAME) ||
       installer_state.FindProduct(BrowserDistribution::CHROME_BINARIES)) {
     AddChromeWorkItems(original_state,
                        installer_state,
                        setup_path,
                        archive_path,
                        src_path,
                        temp_path,
+                       current_version,
                        new_version,
-                       current_version,
                        install_list);
   }
 
   if (installer_state.FindProduct(BrowserDistribution::CHROME_APP_HOST)) {
     install_list->AddCopyTreeWorkItem(
         src_path.Append(installer::kChromeAppHostExe).value(),
         target_path.Append(installer::kChromeAppHostExe).value(),
         temp_path.value(),
         WorkItem::ALWAYS,
         L"");
   }
 
   const HKEY root = installer_state.root_key();
   // Only set "lang" for user-level installs since for system-level, the install
   // language may not be related to a given user's runtime language.
   const bool add_language_identifier = !installer_state.system_install();
 
   const Products& products = installer_state.products();
-  for (size_t i = 0; i < products.size(); ++i) {
-    const Product* product = products[i];
+  for (Products::const_iterator it = products.begin(); it < products.end();
+       ++it) {
+    const Product& product = **it;
 
     AddUninstallShortcutWorkItems(installer_state, setup_path, new_version,
-                                  install_list, *product);
+                                  product, install_list);
 
-    AddVersionKeyWorkItems(root, product->distribution(), new_version,
+    AddVersionKeyWorkItems(root, product.distribution(), new_version,
                            add_language_identifier, install_list);
 
     AddDelegateExecuteWorkItems(installer_state, src_path, new_version,
-                                *product, install_list);
+                                product, install_list);
 
 // TODO(gab): This is only disabled for M22 as the shortcut CL using Active
 // Setup will not make it in M22.
 #if 0
     AddActiveSetupWorkItems(installer_state, new_version, *product,
                             install_list);
 #endif
   }
 
   // Add any remaining work items that involve special settings for
   // each product.
   AddProductSpecificWorkItems(original_state, installer_state, setup_path,
                               new_version, install_list);
 
   // Copy over brand, usagestats, and other values.
   AddGoogleUpdateWorkItems(original_state, installer_state, install_list);
 
   AddQuickEnableApplicationHostWorkItems(installer_state, original_state,
-                                         &setup_path, &new_version,
+                                         setup_path, new_version,
                                          install_list);
 
   AddQuickEnableChromeFrameWorkItems(installer_state, original_state,
-                                     &setup_path, &new_version, install_list);
+                                     setup_path, new_version, install_list);
 
   // Append the tasks that run after the installation.
   AppendPostInstallTasks(installer_state,
                          setup_path,
-                         current_version->get(),
+                         current_version,
                          new_version,
                          temp_path,
                          install_list);
 }
 
 void AddRegisterComDllWorkItems(const FilePath& dll_folder,
                                 const std::vector<FilePath>& dll_list,
                                 bool system_level,
                                 bool do_register,
                                 bool ignore_failures,
@@ skipping 48 matching lines @@
   BrowserDistribution* dist =
       installer_state.multi_package_binaries_distribution();
   if (ready_mode) {
     // If GCF is being installed in ready mode, we write an entry to the
     // multi-install state key.  If the value already exists, we will not
     // overwrite it since the user might have opted out.
     list->AddCreateRegKeyWorkItem(root, dist->GetStateKey());
     list->AddSetRegValueWorkItem(root, dist->GetStateKey(),
         kChromeFrameReadyModeField,
         static_cast<int64>(is_install ? 1 : 0),  // The value we want to set.
-        is_install ? false : true);  // Overwrite existing value.
+        !is_install);  // Overwrite existing value.
     if (is_install) {
       FilePath installer_path(installer_state
           .GetInstallerDirectory(new_version).Append(setup_path.BaseName()));
 
       CommandLine basic_cl(installer_path);
       basic_cl.AppendSwitch(switches::kChromeFrame);
       basic_cl.AppendSwitch(switches::kMultiInstall);
 
       if (installer_state.system_install())
         basic_cl.AppendSwitch(switches::kSystemLevel);
@@ skipping 74 matching lines @@
                                     google_update::kRegCFOptOutCmdField);
     list->AddDeleteRegValueWorkItem(root, version_key,
                                     google_update::kRegCFOptInCmdField);
   }
 
   if (update_chrome_uninstall_command) {
     // Chrome is not a part of this installation run, so we have to explicitly
     // check if Chrome is installed, and if so, update its uninstallation
     // command lines.
     const ProductState* chrome_state = original_state.GetProductState(
-        installer_state.system_install(),
-        BrowserDistribution::CHROME_BROWSER);
+        installer_state.system_install(), BrowserDistribution::CHROME_BROWSER);
     if (chrome_state != NULL) {
       DCHECK(chrome_state->is_multi_install());
       Product chrome(BrowserDistribution::GetSpecificDistribution(
                          BrowserDistribution::CHROME_BROWSER));
       chrome.InitializeFromUninstallCommand(chrome_state->uninstall_command());
       AddUninstallShortcutWorkItems(installer_state, setup_path,
-                                    chrome_state->version(), list, chrome);
+                                    chrome_state->version(), chrome, list);
     } else {
       NOTREACHED() << "What happened to Chrome?";
     }
   }
 }
 
 void AddDelegateExecuteWorkItems(const InstallerState& installer_state,
                                  const FilePath& src_path,
                                  const Version& new_version,
                                  const Product& product,
@@ skipping 184 matching lines @@
     }
 
     FreeLibrary(ieframe);
   } else {
     VLOG(1) << "Cannot load " << kIEFrameDll;
   }
 }
 
 void AddQuickEnableChromeFrameWorkItems(const InstallerState& installer_state,
                                         const InstallationState& machine_state,
-                                        const FilePath* setup_path,
-                                        const Version* new_version,
+                                        const FilePath& setup_path,
+                                        const Version& new_version,
                                         WorkItemList* work_item_list) {
-  DCHECK(setup_path ||
-         installer_state.operation() == InstallerState::UNINSTALL);
-  DCHECK(new_version ||
-         installer_state.operation() == InstallerState::UNINSTALL);
   DCHECK(work_item_list);
 
   const bool system_install = installer_state.system_install();
   bool have_chrome_frame = false;
 
   // STEP 1: Figure out the state of the machine before the operation.
   const ProductState* product_state = NULL;
 
   // Is Chrome Frame !ready-mode already on the machine?
-  product_state =
-      machine_state.GetProductState(system_install,
-                                    BrowserDistribution::CHROME_FRAME);
+  product_state = machine_state.GetProductState(
+      system_install, BrowserDistribution::CHROME_FRAME);
   if (product_state != NULL &&
       !product_state->uninstall_command().HasSwitch(
           switches::kChromeFrameReadyMode))
     have_chrome_frame = true;
 
   // STEP 2: Now take into account the current operation.
   const Product* product = NULL;
 
   if (installer_state.operation() == InstallerState::UNINSTALL) {
     // Forget about Chrome Frame if it is being uninstalled.  Note that we don't
@@ skipping 14 matching lines @@
   CommandLine cmd_line(CommandLine::NO_PROGRAM);
   cmd_line.AppendSwitch(switches::kMultiInstall);
   if (installer_state.system_install())
     cmd_line.AppendSwitch(switches::kSystemLevel);
   cmd_line.AppendSwitch(switches::kChromeFrameQuickEnable);
 
   AddGenericQuickEnableWorkItems(installer_state,
                                  machine_state,
                                  setup_path,
                                  new_version,
-                                 work_item_list,
                                  have_chrome_frame,
                                  cmd_line,
-                                 kCmdQuickEnableCf);
+                                 kCmdQuickEnableCf,
+                                 work_item_list);
 }
 
 void AddQuickEnableApplicationHostWorkItems(
     const InstallerState& installer_state,
     const InstallationState& machine_state,
-    const FilePath* setup_path,
-    const Version* new_version,
+    const FilePath& setup_path,
+    const Version& new_version,
     WorkItemList* work_item_list) {
-  DCHECK(setup_path ||
-         installer_state.operation() == InstallerState::UNINSTALL);
-  DCHECK(new_version ||
-         installer_state.operation() == InstallerState::UNINSTALL);
   DCHECK(work_item_list);
 
   CommandLine cmd_line(CommandLine::NO_PROGRAM);
   cmd_line.AppendSwitch(switches::kMultiInstall);
   cmd_line.AppendSwitch(switches::kChromeAppHost);
 
   // For system-level binaries there is no way to keep the command state in sync
   // with the installation/uninstallation of the Application Host (which is
   // always at user-level).
   // So we pass false for 'have_child_product' to cause this command to always
   // be installed if the Chrome Binaries are installed.
   AddGenericQuickEnableWorkItems(installer_state,
                                  machine_state,
                                  setup_path,
                                  new_version,
-                                 work_item_list,
                                  false,  // have_child_product
                                  cmd_line,
-                                 kCmdQuickEnableApplicationHost);
+                                 kCmdQuickEnableApplicationHost,
+                                 work_item_list);
 }
 
 void AddOsUpgradeWorkItems(const InstallerState& installer_state,
-                           const FilePath* setup_path,
-                           const Version* new_version,
+                           const FilePath& setup_path,
+                           const Version& new_version,
                            const Product& product,
                            WorkItemList* install_list) {
   const HKEY root_key = installer_state.root_key();
   string16 cmd_key(product.distribution()->GetVersionKey());
   cmd_key.append(1, FilePath::kSeparators[0])
       .append(google_update::kRegCommandsKey)
       .append(1, FilePath::kSeparators[0])
       .append(kCmdOnOsUpgrade);
 
-  // This will make Google Update call setup.exe with --on-os-upgrade switch.
-  // For Chrome, this leads to HandleOsUpgradeForBrowser() being called.
-  if (installer_state.operation() != InstallerState::UNINSTALL) {
+  if (installer_state.operation() == InstallerState::UNINSTALL) {
+    install_list->AddDeleteRegKeyWorkItem(root_key, cmd_key)->
+        set_log_message("Removing OS upgrade command");
+  } else {
+    // Register with Google Update to have setup.exe --on-os-upgrade called on
+    // OS upgrade.
     CommandLine cmd_line(installer_state
-        .GetInstallerDirectory(*new_version)
-        .Append(setup_path->BaseName()));
+        .GetInstallerDirectory(new_version)
+        .Append(setup_path.BaseName()));
     // Add the main option to indicate OS upgrade flow.
     cmd_line.AppendSwitch(installer::switches::kOnOsUpgrade);
     // Add product-specific options.
     product.AppendProductFlags(&cmd_line);
     if (installer_state.system_install())
       cmd_line.AppendSwitch(installer::switches::kSystemLevel);
     // Log everything for now.
     cmd_line.AppendSwitch(installer::switches::kVerboseLogging);
 
     AppCommand cmd(cmd_line.GetCommandLineString());
     cmd.set_is_auto_run_on_os_upgrade(true);
     cmd.AddWorkItems(installer_state.root_key(), cmd_key, install_list);
-  } else {
-    install_list->AddDeleteRegKeyWorkItem(root_key, cmd_key)
-        ->set_log_message("Removing OS upgrade command");
   }
 }
 
 }  // namespace installer