[Fixit-Dec-2012] Auto-launch system-level Chrome post user-level Chrome self-destruction.

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.
 
 #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"
 #include "base/files/scoped_temp_dir.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/path_service.h"
 #include "base/process_util.h"
 #include "base/string16.h"
 #include "base/string_number_conversions.h"
 #include "base/string_util.h"
 #include "base/stringprintf.h"
 #include "base/utf_string_conversions.h"
 #include "base/values.h"
 #include "base/win/registry.h"
 #include "base/win/scoped_com_initializer.h"
 #include "base/win/scoped_comptr.h"
 #include "base/win/scoped_handle.h"
 #include "base/win/win_util.h"
 #include "base/win/windows_version.h"
 #include "breakpad/src/client/windows/handler/exception_handler.h"
+#include "chrome/common/chrome_constants.h"
 #include "chrome/common/chrome_switches.h"
 #include "chrome/installer/setup/chrome_frame_quick_enable.h"
 #include "chrome/installer/setup/chrome_frame_ready_mode.h"
 #include "chrome/installer/setup/install.h"
 #include "chrome/installer/setup/install_worker.h"
 #include "chrome/installer/setup/setup_constants.h"
 #include "chrome/installer/setup/setup_util.h"
 #include "chrome/installer/setup/uninstall.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/channel_info.h"
@@ skipping 956 matching lines @@
       original_state, installer_state, cmd_line.GetProgram(), product,
       remove_all, force_uninstall, cmd_line);
 }
 
 installer::InstallStatus UninstallProducts(
     const InstallationState& original_state,
     const InstallerState& installer_state,
     const CommandLine& cmd_line) {
   const Products& products = installer_state.products();
 
+  // Decide whether Active Setup should be triggered and/or system-level Chrome
+  // should be launched post-uninstall. This needs to be done outside the
+  // UninstallProduct calls as some of them might terminate the processes
+  // launched by a previous one otherwise...
+  bool trigger_active_setup = false;
+  // System-level Chrome will be launched via this command if it gets set below.
+  scoped_ptr<CommandLine> system_level_cmd;

[grt (UTC plus 2), 2013/01/02 17:59:33]

It seems more straightforward to use:
  CommandLine system_level_cmd(CommandLine::NO_PROGRAM);
here,
       SetProgram(system_exe_path);
on line 1030, and
     if (!system_level_cmd.GetProgram().empty())
on line 1073, in which case scoped_ptr.h isn't needed.

[gab, 2013/01/02 21:15:48]

Done.

+
   if (installer_state.FindProduct(BrowserDistribution::CHROME_BROWSER)) {
     // InstallerState::Initialize always puts Chrome first, and we rely on that
     // here for this reason: if Chrome is in-use, the user will be prompted to
     // confirm uninstallation.  Upon cancel, we should not continue with the
     // other products.
     DCHECK(products[0]->is_chrome());
+
+    if (cmd_line.HasSwitch(installer::switches::kSelfDestruct)) {

[grt (UTC plus 2), 2013/01/02 17:59:33]

&& !system_level() since this shouldn't be done if someone accidentally puts
--self-destruct on a system-level uninstall command line?

[gab, 2013/01/02 21:15:48]

Sure.

+      BrowserDistribution* dist = BrowserDistribution::GetSpecificDistribution(
+          BrowserDistribution::CHROME_BROWSER);
+      const FilePath system_exe_path(
+          installer::GetChromeInstallPath(true, dist)
+              .Append(installer::kChromeExe));
+      system_level_cmd.reset(new CommandLine(system_exe_path));
+
+      FilePath first_run_sentinel;
+      InstallUtil::GetSentinelFilePath(
+          chrome::kFirstRunSentinel, false, products[0]->distribution(),

[grt (UTC plus 2), 2013/01/02 17:59:33]

isn't products[0]->distribution() the same as |dist|?

[gab, 2013/01/02 21:15:48]

Indeed :)!

+          &first_run_sentinel);
+      if (file_util::PathExists(first_run_sentinel)) {
+        // If the Chrome being self-destructed has already undergone First Run,
+        // trigger Active Setup and make sure the system-level Chrome doesn't go
+        // through first run.

[grt (UTC plus 2), 2013/01/02 17:59:33]

is there a difference between using --no-first-run and simply creating the
system-level beacon here?

[gab, 2013/01/02 21:15:48]

Hmmm, well arguably dropping the beacon here could fail (i.e. write error) where
as passing --no-first-run can't really fail...

+        trigger_active_setup = true;
+        system_level_cmd->AppendSwitch(::switches::kNoFirstRun);

[grt (UTC plus 2), 2013/01/02 17:59:33]

nit: remove the :: in front of "switches" unless it's actually necessary.

[gab, 2013/01/02 21:15:48]

It is necessary since installer::switches also exists and this code lives in the
installer:: namespace.
kNoFirstRun is in switches:: not installer::switches::.

+      }
+    }
   }
   if (installer_state.FindProduct(BrowserDistribution::CHROME_BINARIES)) {
     // Chrome Binaries should be last; if something else is cancelled, they
     // 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 (Products::const_iterator it = products.begin();
        install_status != installer::UNINSTALL_CANCELLED && it < products.end();
        ++it) {
     prod_status = UninstallProduct(original_state, installer_state,
         cmd_line, remove_all, force, **it);
     if (prod_status != installer::UNINSTALL_SUCCESSFUL)
       install_status = prod_status;
   }
 
   installer::CleanUpInstallationDirectoryAfterUninstall(
       original_state, installer_state, cmd_line, &install_status);
 
+  if (install_status != installer::UNINSTALL_CANCELLED) {
+    if (trigger_active_setup)

[grt (UTC plus 2), 2013/01/02 17:59:33]

install_status may be any of: (CHROME_NOT_INSTALLED, UNINSTALL_SUCCESSFUL,
UNINSTALL_FAILED, UNINSTALL_REQUIRES_REBOOT). should active setup and/or the
handoff happen for all of those cases?

[gab, 2013/01/02 21:15:48]

In fact I think it was wrong to base this off of "install_status !=
installer::UNINSTALL_CANCELLED" (had copied that logic from the for loop
condition -- and UNINSTALL_CANCELLED is only set by the user prompt which isn't
even shown with --force-uninstall anyways...).

I think that we should always launch Active Setup and the system-level Chrome no
matter the uninstall result.

i.e., I think it's a better user experience to keep getting the self-destruct
prompt in case of repetitive uninstall failures, but at least get system-level
Chrome to launch, then it is to only get the prompt with no Chrome launching...

I could wrap all of this in an if block that also checks for the presence of
kSelfDestruct; that would be redundant, but perhaps more readable..?

+      InstallUtil::TriggerActiveSetupCommand();
+
+    if (system_level_cmd.get())
+      base::LaunchProcess(*system_level_cmd, base::LaunchOptions(), NULL);
+  }
+
   // Tell Google Update that an uninstall has taken place.
   // Ignore the return value: success or failure of Google Update
   // has no bearing on the success or failure of Chrome's uninstallation.
   google_update::UninstallGoogleUpdate(installer_state.system_install());
 
   return install_status;
 }
 
 installer::InstallStatus ShowEULADialog(const string16& inner_frame) {
   VLOG(1) << "About to show EULA";
@@ skipping 13 matching lines @@
   if (installer::EulaHTMLDialog::ACCEPTED_OPT_IN == outcome) {
     VLOG(1) << "EULA accepted (opt-in)";
     return installer::EULA_ACCEPTED_OPT_IN;
   }
   VLOG(1) << "EULA accepted (no opt-in)";
   return installer::EULA_ACCEPTED;
 }
 
 // Creates the sentinel indicating that the EULA was required and has been
 // accepted.
-bool CreateEULASentinel(BrowserDistribution* dist) {
+bool CreateEULASentinel(bool system_install,

[grt (UTC plus 2), 2013/01/02 17:59:33]

The EULA only applies for system-level installs, so you can remove
|system_install| and pass |true| directly to GetSentinelFilePath.

[gab, 2013/01/02 21:15:48]

Done.

+                        BrowserDistribution* dist) {
   FilePath eula_sentinel;
-  if (!InstallUtil::GetSentinelFilePath(installer::kEULASentinelFile,
-                                        dist, &eula_sentinel)) {
+  if (!InstallUtil::GetSentinelFilePath(
+          installer::kEULASentinelFile, system_install, dist, &eula_sentinel)) {
     return false;
   }
 
   return (file_util::CreateDirectory(eula_sentinel.DirName()) &&
           file_util::WriteFile(eula_sentinel, "", 0) != -1);
 }
 
 void ActivateMetroChrome() {
   // Check to see if we're per-user or not. Need to do this since we may
   // not have been invoked with --system-level even for a machine install.
@@ skipping 149 matching lines @@
   } 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) {
       if (GoogleUpdateSettings::SetEULAConsent(
               original_state, BrowserDistribution::GetDistribution(), true)) {
-        CreateEULASentinel(BrowserDistribution::GetDistribution());
+        CreateEULASentinel(installer_state->system_install(),

[grt (UTC plus 2), 2013/01/02 17:59:33]

FYI: --system-level isn't passed to setup.exe in this case, so
installer_state->system_install() cannot be used. this isn't a problem, however;
see comment on line 1110.

[gab, 2013/01/02 21:15:48]

Done.

+                           BrowserDistribution::GetDistribution());
       }
       // For a metro-originated launch, we now need to launch back into metro.
       if (cmd_line.HasSwitch(installer::switches::kShowEulaForMetro))
         ActivateMetroChrome();
     }
   } else if (cmd_line.HasSwitch(installer::switches::kConfigureUserSettings)) {
     // NOTE: Should the work done here, on kConfigureUserSettings, change:
     // kActiveSetupVersion in install_worker.cc needs to be increased for Active
     // Setup to invoke this again for all users of this install.
     const Product* chrome_install =
@@ skipping 412 matching lines @@
   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;
 }