C++ reability review for gab.

chrome/installer/setup/setup_main.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.
 
+//Pickup file for readability review.
+
 #include <windows.h>
 #include <msi.h>
 #include <shellapi.h>
 #include <shlobj.h>
 
 #include "base/at_exit.h"
 #include "base/basictypes.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/file_version_info.h"
@@ skipping 137 matching lines @@
       BrowserDistribution::Type type =
           static_cast<BrowserDistribution::Type>(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)->GetAppShortCutName();
+                  << BrowserDistribution::GetSpecificDistribution(type)->
+                         GetAppShortCutName();
         }
       }
     }
   }
 }
 
 // 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
@@ skipping 33 matching lines @@
                                     temp_path.path().value(),
                                     WorkItem::ALWAYS_MOVE);
   install_list->AddDeleteTreeWorkItem(chrome_new_exe, temp_path.path());
   // Delete an elevation policy associated with the old version, should one
   // exist.
   if (installer_state->FindProduct(BrowserDistribution::CHROME_FRAME)) {
     installer::AddDeleteOldIELowRightsPolicyWorkItems(*installer_state,
                                                       install_list.get());
   }
   // old_chrome.exe is still in use in most cases, so ignore failures here.
-  install_list->AddDeleteTreeWorkItem(chrome_old_exe, temp_path.path())
-      ->set_ignore_failure(true);
+  install_list->AddDeleteTreeWorkItem(chrome_old_exe, temp_path.path())->
+      set_ignore_failure(true);
 
   // Add work items to delete the "opv", "cpv", and "cmd" values from all
   // products we're operating on (which including the multi-install binaries).
   const Products& products = installer_state->products();
   HKEY reg_root = installer_state->root_key();
   string16 version_key;
-  for (size_t i = 0; i < products.size(); ++i) {
-    version_key = products[i]->distribution()->GetVersionKey();
+  for (Products::const_iterator it = products.begin(); it < products.end();
+       ++it) {
+    const Product& product = **it;

[dominich, 2012/09/10 15:40:42]

nit - i wouldn't bother with this local. Just:

version_key = (*it)->distribution()->GetVersionKey();

2+ other places

[gab, 2012/09/10 19:14:05]

Ok, I felt |product| was clearer then |*it|, but I agree that the code snippets
are so short that the reader doesn't have to look far to understand what |*it|
is.

Done.

+    version_key = product.distribution()->GetVersionKey();
     install_list->AddDeleteRegValueWorkItem(
         reg_root, version_key, google_update::kRegOldVersionField);
     install_list->AddDeleteRegValueWorkItem(
         reg_root, version_key, google_update::kRegCriticalVersionField);
     install_list->AddDeleteRegValueWorkItem(
         reg_root, version_key, google_update::kRegRenameCmdField);
   }
   installer::InstallStatus ret = installer::RENAME_SUCCESSFUL;
   if (!install_list->Do()) {
     LOG(ERROR) << "Renaming of executables failed. Rolling back any changes.";
@@ skipping 158 matching lines @@
       return true;
     }
 
     if (chrome) {
       if (chrome_frame &&
           chrome_frame->HasOption(installer::kOptionReadyMode)) {
         // We're being asked to install Chrome with Chrome Frame in ready-mode.
         // This is an optimistic operation: if a SxS install of Chrome Frame
         // is already present, don't touch it; if a multi-install of Chrome
         // Frame is present, preserve its settings (ready-mode).
-        if (cf_state != NULL) {
+        if (cf_state) {
           installer_state->RemoveProduct(chrome_frame);
           chrome_frame = NULL;
           if (cf_state->is_multi_install()) {
             chrome_frame = installer_state->AddProductFromState(
                 BrowserDistribution::CHROME_FRAME, *cf_state);
             VLOG(1) << "Upgrading existing multi-install Chrome Frame rather "
                        "than installing in ready-mode.";
           } else {
             VLOG(1) << "Skipping upgrade of single-install Chrome Frame rather "
                        "than installing in ready-mode.";
@@ skipping 119 matching lines @@
         << "Not given any products to install and no products found to update.";
     *status = installer::CHROME_NOT_INSTALLED;
     installer_state->WriteInstallerResult(*status,
         IDS_INSTALL_NO_PRODUCTS_TO_UPDATE_BASE, NULL);
     return false;
   }
 
   if (!installer_state->system_install()) {
     // This is a user-level installation. Make sure that we are not installing
     // on top of an existing system-level installation.
-    for (size_t i = 0; i < products.size(); ++i) {
-      const Product* product = products[i];
-      BrowserDistribution* browser_dist = product->distribution();
+    for (Products::const_iterator it = products.begin(); it < products.end();
+         ++it) {
+      const Product& product = **it;
+      BrowserDistribution* browser_dist = product.distribution();
 
       // Skip over the binaries, as it's okay for them to be at both levels
       // for different products.
       if (browser_dist->GetType() == BrowserDistribution::CHROME_BINARIES)
         continue;
 
       const ProductState* user_level_product_state =
           original_state.GetProductState(false, browser_dist->GetType());
       const ProductState* system_level_product_state =
           original_state.GetProductState(true, browser_dist->GetType());
 
       // Allow upgrades to proceed so that out-of-date versions are not left
       // around.
       if (user_level_product_state)
         continue;
 
       // This is a new user-level install...
 
       if (system_level_product_state) {
         // ... and the product already exists at system-level.
         LOG(ERROR) << "Already installed version "
                    << system_level_product_state->version().GetString()
                    << " at system-level conflicts with this one at user-level.";
-        if (product->is_chrome()) {
+        if (product.is_chrome()) {
           // Instruct Google Update to launch the existing system-level Chrome.
           // There should be no error dialog.
           FilePath install_path(installer::GetChromeInstallPath(true,  // system
                                                                 browser_dist));
           if (install_path.empty()) {
             // Give up if we failed to construct the install path.
             *status = installer::OS_ERROR;
             installer_state->WriteInstallerResult(*status,
                                                   IDS_INSTALL_OS_ERROR_BASE,
                                                   NULL);
@@ skipping 107 matching lines @@
       install_status = installer::INVALID_ARCHIVE;
       installer_state.WriteInstallerResult(install_status,
           IDS_INSTALL_INVALID_ARCHIVE_BASE, NULL);
     } else {
       VLOG(1) << "version to install: " << installer_version->GetString();
       bool proceed_with_installation = true;
       uint32 higher_products = 0;
       COMPILE_ASSERT(
           sizeof(higher_products) * 8 > BrowserDistribution::NUM_TYPES,
           too_many_distribution_types_);
-      for (size_t i = 0; i < installer_state.products().size(); ++i) {
-        const Product* product = installer_state.products()[i];
+      const Products& products = installer_state.products();
+      for (Products::const_iterator it = products.begin(); it < products.end();
+           ++it) {
+        const Product& product = **it;
         const ProductState* product_state =
             original_state.GetProductState(system_install,
-                                           product->distribution()->GetType());
+                                           product.distribution()->GetType());
         if (product_state != NULL &&
             (product_state->version().CompareTo(*installer_version) > 0)) {
           LOG(ERROR) << "Higher version of "
-                     << product->distribution()->GetAppShortCutName()
+                     << product.distribution()->GetAppShortCutName()
                      << " is already installed.";
-          higher_products |= (1 << product->distribution()->GetType());
+          higher_products |= (1 << product.distribution()->GetType());
         }
       }
 
       if (higher_products != 0) {
         COMPILE_ASSERT(BrowserDistribution::NUM_TYPES == 4,
                        add_support_for_new_products_here_);
         const uint32 kBrowserBit = 1 << BrowserDistribution::CHROME_BROWSER;
         const uint32 kGCFBit = 1 << BrowserDistribution::CHROME_FRAME;
         const uint32 kAppHostBit = 1 << BrowserDistribution::CHROME_APP_HOST;
         int message_id = 0;
@@ skipping 19 matching lines @@
       }
 
       proceed_with_installation =
           proceed_with_installation &&
           CheckGroupPolicySettings(original_state, installer_state,
                                    *installer_version, &install_status);
 
       if (proceed_with_installation) {
         FilePath prefs_source_path(cmd_line.GetSwitchValueNative(
             installer::switches::kInstallerData));
-        install_status = installer::InstallOrUpdateProduct(original_state,
-            installer_state, cmd_line.GetProgram(), archive_to_copy,
-            temp_path.path(), prefs_source_path, prefs, *installer_version);
+        install_status = installer::InstallOrUpdateProduct(
+            original_state, installer_state, cmd_line.GetProgram(),
+            archive_to_copy, temp_path.path(), prefs_source_path, prefs,
+            *installer_version);
 
         int install_msg_base = IDS_INSTALL_FAILED_BASE;
         string16 chrome_exe;
         string16 quoted_chrome_exe;
         if (install_status == installer::SAME_VERSION_REPAIR_FAILED) {
           if (installer_state.FindProduct(BrowserDistribution::CHROME_FRAME)) {
             install_msg_base = IDS_SAME_VERSION_REPAIR_FAILED_CF_BASE;
           } else {
             install_msg_base = IDS_SAME_VERSION_REPAIR_FAILED_BASE;
           }
@@ skipping 68 matching lines @@
     // There is another way to reach this same function if this is a system
     // level install. See HandleNonInstallCmdLineOptions().
     {
       // If installation failed, use the path to the currently running setup.
       // If installation succeeded, use the path to setup in the installer dir.
       FilePath setup_path(cmd_line.GetProgram());
       if (InstallUtil::GetInstallReturnCode(install_status) == 0) {
         setup_path = installer_state.GetInstallerDirectory(*installer_version)
             .Append(setup_path.BaseName());
       }
-      for (size_t i = 0; i < products.size(); ++i) {
-        const Product* product = products[i];
-        product->distribution()->LaunchUserExperiment(setup_path,
-            install_status, *installer_version, *product, system_install);
+      for (Products::const_iterator it = products.begin(); it < products.end();
+           ++it) {
+        const Product& product = **it;
+        product.distribution()->LaunchUserExperiment(setup_path,
+            install_status, *installer_version, product, system_install);
       }
     }
   }
 
   // Delete the master profile file if present. Note that we do not care about
   // rollback here and we schedule for deletion on reboot if the delete fails.
   // As such, we do not use DeleteTreeWorkItem.
   if (cmd_line.HasSwitch(installer::switches::kInstallerData)) {
     FilePath prefs_path(cmd_line.GetSwitchValuePath(
         installer::switches::kInstallerData));
@@ skipping 18 matching lines @@
     const MasterPreferences& prefs,
     InstallerState* installer_state) {
   DCHECK(installer_state);
   const bool system_install = installer_state->system_install();
   installer::InstallStatus install_status = installer::UNKNOWN_STATUS;
   installer::ArchiveType archive_type = installer::UNKNOWN_ARCHIVE_TYPE;
   bool incremental_install = false;
   installer_state->UpdateStage(installer::PRECONDITIONS);
   // The stage provides more fine-grained information than -multifail, so remove
   // the -multifail suffix from the Google Update "ap" value.
-  BrowserDistribution::GetSpecificDistribution(installer_state->state_type())
-      ->UpdateInstallStatus(system_install, archive_type, install_status);
+  BrowserDistribution::GetSpecificDistribution(installer_state->state_type())->
+      UpdateInstallStatus(system_install, archive_type, install_status);
   if (CheckPreInstallConditions(original_state, installer_state,
                                 &install_status)) {
     VLOG(1) << "Installing to " << installer_state->target_path().value();
     install_status = InstallProductsHelper(
         original_state, cmd_line, prefs, *installer_state, &archive_type);
   } else {
     // CheckPreInstallConditions must set the status on failure.
     DCHECK_NE(install_status, installer::UNKNOWN_STATUS);
   }
 
   const Products& products = installer_state->products();
 
-  for (size_t i = 0; i < products.size(); ++i) {
-    const Product* product = products[i];
-    product->distribution()->UpdateInstallStatus(
+  for (Products::const_iterator it = products.begin(); it < products.end();
+       ++it) {
+    const Product& product = **it;
+    product.distribution()->UpdateInstallStatus(
         system_install, archive_type, install_status);
   }
 
   installer_state->UpdateStage(installer::NO_STAGE);
   return install_status;
 }
 
 installer::InstallStatus UninstallProduct(
     const InstallationState& original_state,
     const InstallerState& installer_state,
     const CommandLine& cmd_line,
     bool remove_all,
     bool force_uninstall,
     const Product& product) {
   const ProductState* product_state =
       original_state.GetProductState(installer_state.system_install(),
                                      product.distribution()->GetType());
   if (product_state != NULL) {
     VLOG(1) << "version on the system: "
             << product_state->version().GetString();
   } else if (!force_uninstall) {
     LOG(ERROR) << product.distribution()->GetAppShortCutName()
                << " not found for uninstall.";
     return installer::CHROME_NOT_INSTALLED;
   }
 
-  return installer::UninstallProduct(original_state, installer_state,
-      cmd_line.GetProgram(), product, remove_all, force_uninstall, cmd_line);
+  return installer::UninstallProduct(
+      original_state, installer_state, cmd_line.GetProgram(), product,
+      remove_all, force_uninstall, cmd_line);
 }
 
 // Tell Google Update that an uninstall has taken place.  This gives it a chance
 // to uninstall itself straight away if no more products are installed on the
 // system rather than waiting for the next time the scheduled task runs.
 // Success or failure of Google Update has no bearing on the success or failure
 // of Chrome's uninstallation.
 void UninstallGoogleUpdate(bool system_install) {
   string16 uninstall_cmd(
       GoogleUpdateSettings::GetUninstallCommandLine(system_install));
@@ skipping 43 matching lines @@
     // should stay.
     DCHECK(products[products.size() - 1]->is_chrome_binaries());
   }
 
   installer::InstallStatus install_status = installer::UNINSTALL_SUCCESSFUL;
   installer::InstallStatus prod_status = installer::UNKNOWN_STATUS;
   const bool force = cmd_line.HasSwitch(installer::switches::kForceUninstall);
   const bool remove_all = !cmd_line.HasSwitch(
       installer::switches::kDoNotRemoveSharedItems);
 
-  for (size_t i = 0;
-       install_status != installer::UNINSTALL_CANCELLED &&
-           i < products.size();
-       ++i) {
+  for (Products::const_iterator it = products.begin();
+       install_status != installer::UNINSTALL_CANCELLED && it < products.end();
+       ++it) {
+    const Product& product = **it;
     prod_status = UninstallProduct(original_state, installer_state,
-        cmd_line, remove_all, force, *products[i]);
+        cmd_line, remove_all, force, product);
     if (prod_status != installer::UNINSTALL_SUCCESSFUL)
       install_status = prod_status;
   }
 
   UninstallGoogleUpdate(installer_state.system_install());
 
   return install_status;
 }
 
 installer::InstallStatus ShowEULADialog(const string16& inner_frame) {
@@ skipping 60 matching lines @@
         // the Windows last-error code.
         LOG(WARNING) << "Scheduling temporary path " << temp_path.path().value()
                      << " for deletion at reboot.";
         ScheduleDirectoryForDeletion(temp_path.path().value().c_str());
       }
     }
 
     *exit_code = InstallUtil::GetInstallReturnCode(status);
     if (*exit_code) {
       LOG(WARNING) << "setup.exe patching failed.";
-      installer_state->WriteInstallerResult(status, IDS_SETUP_PATCH_FAILED_BASE,
-          NULL);
+      installer_state->WriteInstallerResult(
+          status, IDS_SETUP_PATCH_FAILED_BASE, NULL);
     }
     // We will be exiting normally, so clear the stage indicator.
     installer_state->UpdateStage(installer::NO_STAGE);
   } else if (cmd_line.HasSwitch(installer::switches::kShowEula)) {
     // Check if we need to show the EULA. If it is passed as a command line
     // then the dialog is shown and regardless of the outcome setup exits here.
     string16 inner_frame =
         cmd_line.GetSwitchValueNative(installer::switches::kShowEula);
     *exit_code = ShowEULADialog(inner_frame);
     if (installer::EULA_REJECTED != *exit_code) {
@@ skipping 38 matching lines @@
         suffix = cmd_line.GetSwitchValueNative(
             installer::switches::kRegisterChromeBrowserSuffix);
       }
       if (cmd_line.HasSwitch(
           installer::switches::kRegisterURLProtocol)) {
         string16 protocol = cmd_line.GetSwitchValueNative(
             installer::switches::kRegisterURLProtocol);
         // ShellUtil::RegisterChromeForProtocol performs all registration
         // done by ShellUtil::RegisterChromeBrowser, as well as registering
         // with Windows as capable of handling the supplied protocol.
-        if (ShellUtil::RegisterChromeForProtocol(chrome_install->distribution(),
-            chrome_exe, suffix, protocol, false))
+        if (ShellUtil::RegisterChromeForProtocol(
+                chrome_install->distribution(), chrome_exe, suffix, protocol,
+                false))
           status = installer::IN_USE_UPDATED;
       } else {
         if (ShellUtil::RegisterChromeBrowser(chrome_install->distribution(),
             chrome_exe, suffix, false))
           status = installer::IN_USE_UPDATED;
       }
     } else {
       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.
     *exit_code = RenameChromeExecutables(original_state, installer_state);
   } else if (cmd_line.HasSwitch(
-      installer::switches::kRemoveChromeRegistration)) {
+                 installer::switches::kRemoveChromeRegistration)) {
     // This is almost reverse of --register-chrome-browser option above.
     // Here we delete Chrome browser registration. This option should only
     // be used when setup.exe is launched with admin rights. We do not
     // make any user specific changes in this option.
     string16 suffix;
     if (cmd_line.HasSwitch(
-        installer::switches::kRegisterChromeBrowserSuffix)) {
+            installer::switches::kRegisterChromeBrowserSuffix)) {
       suffix = cmd_line.GetSwitchValueNative(
           installer::switches::kRegisterChromeBrowserSuffix);
     }
     installer::InstallStatus tmp = installer::UNKNOWN_STATUS;
     const Product* chrome_install =
         installer_state->FindProduct(BrowserDistribution::CHROME_BROWSER);
     DCHECK(chrome_install);
     if (chrome_install) {
       installer::DeleteChromeRegistrationKeys(chrome_install->distribution(),
           HKEY_LOCAL_MACHINE, suffix, installer_state->target_path(), &tmp);
@@ skipping 13 matching lines @@
     int flavor = -1;
     base::StringToInt(cmd_line.GetSwitchValueNative(
         installer::switches::kInactiveUserToast), &flavor);
     std::string experiment_group =
         cmd_line.GetSwitchValueASCII(installer::switches::kExperimentGroup);
     DCHECK_NE(-1, flavor);
     if (flavor == -1) {
       *exit_code = installer::UNKNOWN_STATUS;
     } else {
       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();
-        browser_dist->InactiveUserToastExperiment(flavor,
-            ASCIIToUTF16(experiment_group),
-            *product, installer_state->target_path());
+      for (Products::const_iterator it = products.begin(); it < products.end();
+           ++it) {
+        const Product& product = **it;
+        BrowserDistribution* browser_dist = product.distribution();
+        browser_dist->InactiveUserToastExperiment(
+            flavor, ASCIIToUTF16(experiment_group), product,
+            installer_state->target_path());
       }
     }
   } else if (cmd_line.HasSwitch(installer::switches::kSystemLevelToast)) {
     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();
+    for (Products::const_iterator it = products.begin(); it < products.end();
+         ++it) {
+      const Product& product = **it;
+      BrowserDistribution* browser_dist = product.distribution();
       // We started as system-level and have been re-launched as user level
       // to continue with the toast experiment.
       Version installed_version;
       InstallUtil::GetChromeVersion(browser_dist, true, &installed_version);
       if (!installed_version.IsValid()) {
         LOG(ERROR) << "No installation of "
                    << browser_dist->GetAppShortCutName()
                    << " found for system-level toast.";
       } else {
         browser_dist->LaunchUserExperiment(cmd_line.GetProgram(),
                                            installer::REENTRY_SYS_UPDATE,
-                                           installed_version, *product, true);
+                                           installed_version, product, true);
       }
     }
   } else if (cmd_line.HasSwitch(
                  installer::switches::kChromeFrameReadyModeOptIn)) {
     *exit_code = InstallUtil::GetInstallReturnCode(
         installer::ChromeFrameReadyModeOptIn(original_state, *installer_state));
   } else if (cmd_line.HasSwitch(
                  installer::switches::kChromeFrameReadyModeTempOptOut)) {
     *exit_code = InstallUtil::GetInstallReturnCode(
         installer::ChromeFrameReadyModeTempOptOut(original_state,
@@ skipping 65 matching lines @@
 
  private:
   bool initialized_;
 };
 
 // Returns the Custom information for the client identified by the exe path
 // passed in. This information is used for crash reporting.
 google_breakpad::CustomClientInfo* GetCustomInfo(const wchar_t* exe_path) {
   string16 product;
   string16 version;
-  scoped_ptr<FileVersionInfo>
-      version_info(FileVersionInfo::CreateFileVersionInfo(FilePath(exe_path)));
+  scoped_ptr<FileVersionInfo> version_info(
+      FileVersionInfo::CreateFileVersionInfo(FilePath(exe_path)));
   if (version_info.get()) {
     version = version_info->product_version();
     product = version_info->product_short_name();
   }
 
   if (version.empty())
     version = L"0.1.0.0";
 
   if (product.empty())
     product = L"Chrome Installer";
@@ skipping 74 matching lines @@
   original_state.Initialize();
 
   InstallerState installer_state;
   installer_state.Initialize(cmd_line, prefs, original_state);
   const bool is_uninstall = cmd_line.HasSwitch(installer::switches::kUninstall);
 
   // Check to make sure current system is WinXP or later. If not, log
   // error message and get out.
   if (!InstallUtil::IsOSSupported()) {
     LOG(ERROR) << "Chrome only supports Windows XP or later.";
-    installer_state.WriteInstallerResult(installer::OS_NOT_SUPPORTED,
-        IDS_INSTALL_OS_NOT_SUPPORTED_BASE, NULL);
+    installer_state.WriteInstallerResult(
+        installer::OS_NOT_SUPPORTED, IDS_INSTALL_OS_NOT_SUPPORTED_BASE, NULL);
     return installer::OS_NOT_SUPPORTED;
   }
 
   // Initialize COM for use later.
   AutoCom auto_com;
   if (!auto_com.Init(system_install)) {
-    installer_state.WriteInstallerResult(installer::OS_ERROR,
-        IDS_INSTALL_OS_ERROR_BASE, NULL);
+    installer_state.WriteInstallerResult(
+        installer::OS_ERROR, IDS_INSTALL_OS_ERROR_BASE, NULL);
     return installer::OS_ERROR;
   }
 
   // Some command line options don't work with SxS install/uninstall
   if (InstallUtil::IsChromeSxSProcess()) {
     if (system_install ||
         prefs.is_multi_install() ||
         cmd_line.HasSwitch(installer::switches::kForceUninstall) ||
         cmd_line.HasSwitch(installer::switches::kMakeChromeDefault) ||
         cmd_line.HasSwitch(installer::switches::kRegisterChromeBrowser) ||
-        cmd_line.HasSwitch(
-            installer::switches::kRemoveChromeRegistration) ||
+        cmd_line.HasSwitch(installer::switches::kRemoveChromeRegistration) ||
         cmd_line.HasSwitch(installer::switches::kInactiveUserToast) ||
         cmd_line.HasSwitch(installer::switches::kSystemLevelToast) ||
         cmd_line.HasSwitch(installer::switches::kChromeFrameQuickEnable)) {
       return installer::SXS_OPTION_NOT_SUPPORTED;
     }
   }
 
   int exit_code = 0;
-  if (HandleNonInstallCmdLineOptions(original_state, cmd_line, &installer_state,
-                                     &exit_code))
+  if (HandleNonInstallCmdLineOptions(
+          original_state, cmd_line, &installer_state, &exit_code)) {
     return exit_code;
+  }
 
   if (system_install && !IsUserAnAdmin()) {
     if (base::win::GetVersion() >= base::win::VERSION_VISTA &&
         !cmd_line.HasSwitch(installer::switches::kRunAsAdmin)) {
       CommandLine new_cmd(CommandLine::NO_PROGRAM);
       new_cmd.AppendArguments(cmd_line, true);
       // Append --run-as-admin flag to let the new instance of setup.exe know
       // that we already tried to launch ourselves as admin.
       new_cmd.AppendSwitch(installer::switches::kRunAsAdmin);
       // If system_install became true due to an environment variable, append
       // it to the command line here since env vars may not propagate past the
       // elevation.
       if (!new_cmd.HasSwitch(installer::switches::kSystemLevel))
         new_cmd.AppendSwitch(installer::switches::kSystemLevel);
+
       DWORD exit_code = installer::UNKNOWN_STATUS;
       InstallUtil::ExecuteExeAsAdmin(new_cmd, &exit_code);
       return exit_code;
     } else {
       LOG(ERROR) << "Non admin user can not install system level Chrome.";
       installer_state.WriteInstallerResult(installer::INSUFFICIENT_RIGHTS,
           IDS_INSTALL_INSUFFICIENT_RIGHTS_BASE, NULL);
       return installer::INSUFFICIENT_RIGHTS;
     }
   }
 
   installer::InstallStatus install_status = installer::UNKNOWN_STATUS;
   // If --uninstall option is given, uninstall the identified product(s)
   if (is_uninstall) {
-    install_status = UninstallProducts(original_state, installer_state,
-                                       cmd_line);
+    install_status =
+        UninstallProducts(original_state, installer_state, cmd_line);
   } else {
     // If --uninstall option is not specified, we assume it is install case.
-    install_status = InstallProducts(original_state, cmd_line, prefs,
-        &installer_state);
+    install_status =
+        InstallProducts(original_state, cmd_line, prefs, &installer_state);
   }
 
   // Validate that the machine is now in a good state following the operation.
   // TODO(grt): change this to log at DFATAL once we're convinced that the
   // validator handles all cases properly.
   InstallationValidator::InstallationType installation_type =
       InstallationValidator::NO_PRODUCTS;
   LOG_IF(ERROR,
          !InstallationValidator::ValidateInstallationType(system_install,
                                                           &installation_type));
 
   const Product* cf_install =
       installer_state.FindProduct(BrowserDistribution::CHROME_FRAME);
 
   if (cf_install &&
       !cmd_line.HasSwitch(installer::switches::kForceUninstall)) {
     if (install_status == installer::UNINSTALL_REQUIRES_REBOOT) {
       ShowRebootDialog();
     } else if (is_uninstall) {
       // Only show the message box if Chrome Frame was the only product being
       // uninstalled.
       const Products& products = installer_state.products();
       int num_products = 0;
-      for (size_t i = 0; i < products.size(); ++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())
           ++num_products;
       }
       if (num_products == 1U) {
         ::MessageBoxW(NULL,
                       installer::GetLocalizedString(
                           IDS_UNINSTALL_COMPLETE_BASE).c_str(),
                       cf_install->distribution()->GetAppShortCutName().c_str(),
                       MB_OK);
       }
     }
   }
 
   int return_code = 0;
   // MSI demands that custom actions always return 0 (ERROR_SUCCESS) or it will
   // rollback the action. If we're uninstalling we want to avoid this, so always
   // report success, squashing any more informative return codes.
   if (!(installer_state.is_msi() && is_uninstall))
     // Note that we allow the status installer::UNINSTALL_REQUIRES_REBOOT
     // to pass through, since this is only returned on uninstall which is
     // never invoked directly by Google Update.
     return_code = InstallUtil::GetInstallReturnCode(install_status);
 
   VLOG(1) << "Installation complete, returning: " << return_code;
 
   return return_code;
 }