Improving Chrome Frame uninstall process:

1) Don't kill chrome.exe when uninstalling. This leads to immediate breakage of any running Chrome Frame windows and is also annoying if the user also happens to be using Google Chrome or some variant thereof.

2) If files are in use at uninstall time, schedule them for deletion and prompt the user to reboot.

3) Remember to remove any pending scheduled deletes at install time.


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

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

3 files changed, 104 insertions, 16 deletions

diff --git a/chrome/installer/setup/install.cc b/chrome/installer/setup/install.cc
index 24d2f80..1bc8164 100644
--- a/chrome/installer/setup/install.cc
+++ b/chrome/installer/setup/install.cc
@@ -15,6 +15,7 @@
 #include "chrome/installer/setup/setup_constants.h"
 #include "chrome/installer/util/browser_distribution.h"
 #include "chrome/installer/util/create_reg_key_work_item.h"
+#include "chrome/installer/util/delete_after_reboot_helper.h"
 #include "chrome/installer/util/google_update_constants.h"
 #include "chrome/installer/util/helper.h"
 #include "chrome/installer/util/install_util.h"
@@ -95,7 +96,7 @@ void AddUninstallShortcutWorkItems(HKEY reg_root,
   uninstall_cmd.append(L"\" --");
   uninstall_cmd.append(installer_util::switches::kUninstall);
 
-#ifdef CHROME_FRAME_BUILD
+#if defined(CHROME_FRAME_BUILD)
   uninstall_cmd.append(L" --");
   uninstall_cmd.append(installer_util::switches::kForceUninstall);
   uninstall_cmd.append(L" --");
@@ -246,7 +247,7 @@ bool CreateOrUpdateChromeShortcuts(const std::wstring& exe_path,
     std::wstring arguments(L" --");
     arguments.append(installer_util::switches::kUninstall);
 
-#ifdef CHROME_FRAME_BUILD
+#if defined(CHROME_FRAME_BUILD)
     arguments.append(L" --");
     arguments.append(installer_util::switches::kForceUninstall);
     arguments.append(L" --");
@@ -459,6 +460,13 @@ bool InstallNewVersion(const std::wstring& exe_path,
   if (reg_root != HKEY_LOCAL_MACHINE && reg_root != HKEY_CURRENT_USER)
     return false;
 
+#if defined(CHROME_FRAME_BUILD)
+  // Make sure that we don't end up deleting installed files on next reboot.
+  if (!RemoveFromMovesPendingReboot(install_path.c_str())) {
+    LOG(ERROR) << "Error accessing pending moves value.";
+  }
+#endif
+
   scoped_ptr<WorkItemList> install_list(WorkItem::CreateWorkItemList());
   // A temp directory that work items need and the actual install directory.
   install_list->AddCreateDirWorkItem(FilePath::FromWStringHack(temp_dir));
diff --git a/chrome/installer/setup/setup_main.cc b/chrome/installer/setup/setup_main.cc
index 8b70a1b..3b5377d 100644
--- a/chrome/installer/setup/setup_main.cc
+++ b/chrome/installer/setup/setup_main.cc
@@ -5,6 +5,7 @@
 #include <string>
 #include <windows.h>
 #include <msi.h>
+#include <shellapi.h>
 #include <shlobj.h>
 
 #include "base/at_exit.h"
@@ -13,6 +14,7 @@
 #include "base/file_util.h"
 #include "base/path_service.h"
 #include "base/registry.h"
+#include "base/scoped_handle_win.h"
 #include "base/string_util.h"
 #include "base/win_util.h"
 #include "chrome/installer/setup/install.h"
@@ -413,7 +415,7 @@ bool HandleNonInstallCmdLineOptions(const CommandLine& cmd_line,
     exit_code = ShowEULADialog(inner_frame);
     if (installer_util::EULA_REJECTED != exit_code)
       GoogleUpdateSettings::SetEULAConsent(true);
-    return true;;
+    return true;
   } else if (cmd_line.HasSwitch(
       installer_util::switches::kRegisterChromeBrowser)) {
     // If --register-chrome-browser option is specified, register all
@@ -464,6 +466,40 @@ bool HandleNonInstallCmdLineOptions(const CommandLine& cmd_line,
   return false;
 }
 
+bool ShowRebootDialog() {
+  // Get a token for this process.
+  HANDLE token;
+  if (!OpenProcessToken(GetCurrentProcess(),
+                        TOKEN_ADJUST_PRIVILEGES | TOKEN_QUERY,
+                        &token)) {
+    LOG(ERROR) << "Failed to open token.";
+    return false;
+  }
+
+  // Use a ScopedHandle to keep track of and eventually close our handle.
+  // TODO(robertshield): Add a Receive() method to base's ScopedHandle.
+  ScopedHandle scoped_handle(token);
+
+  // Get the LUID for the shutdown privilege.
+  TOKEN_PRIVILEGES tkp = {0};
+  LookupPrivilegeValue(NULL, SE_SHUTDOWN_NAME, &tkp.Privileges[0].Luid);
+  tkp.PrivilegeCount = 1;
+  tkp.Privileges[0].Attributes = SE_PRIVILEGE_ENABLED;
+
+  // Get the shutdown privilege for this process.
+  AdjustTokenPrivileges(token, FALSE, &tkp, 0,
+                        reinterpret_cast<PTOKEN_PRIVILEGES>(NULL), 0);
+  if (GetLastError() != ERROR_SUCCESS) {
+    LOG(ERROR) << "Unable to get shutdown privileges.";
+    return false;
+  }
+
+  // Popup a dialog that will prompt to reboot using the default system message.
+  // TODO(robertshield): Add a localized, more specific string to the prompt.
+  RestartDialog(NULL, NULL, EWX_REBOOT | EWX_FORCEIFHUNG);
+  return true;
+}
+
 }  // namespace
 
 int WINAPI wWinMain(HINSTANCE instance, HINSTANCE prev_instance,
@@ -550,7 +586,15 @@ int WINAPI wWinMain(HINSTANCE instance, HINSTANCE prev_instance,
                                    prefs.get());
   }
 
+  if (install_status == installer_util::UNINSTALL_REQUIRES_REBOOT) {
+    install_status = installer_util::UNINSTALL_SUCCESSFUL;
+#if defined(CHROME_FRAME_BUILD)
+    ShowRebootDialog();
+#endif
+  }
+
   CoUninitialize();
+
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   return dist->GetInstallReturnCode(install_status);
 }
diff --git a/chrome/installer/setup/uninstall.cc b/chrome/installer/setup/uninstall.cc
index 978c056..2ad8cf2 100644
--- a/chrome/installer/setup/uninstall.cc
+++ b/chrome/installer/setup/uninstall.cc
@@ -17,6 +17,7 @@
 #include "chrome/installer/setup/install.h"
 #include "chrome/installer/setup/setup_constants.h"
 #include "chrome/installer/util/browser_distribution.h"
+#include "chrome/installer/util/delete_after_reboot_helper.h"
 #include "chrome/installer/util/helper.h"
 #include "chrome/installer/util/install_util.h"
 #include "chrome/installer/util/logging_installer.h"
@@ -137,18 +138,27 @@ bool DeleteEmptyParentDir(const std::wstring& path) {
   return ret;
 }
 
-// Deletes all installed files of Chromium and Folders. Before deleting it
+enum DeleteResult {
+  DELETE_SUCCEEDED,
+  DELETE_FAILED,
+  DELETE_REQUIRES_REBOOT
+};
+
+// Deletes all installed files of Chromium and Folders or schedules them for
+// deletion on reboot if they are in use. Before deleting it
 // needs to move setup.exe in a temp folder because the current process
-// is using that file. It returns false when it can not get the path to
-// installation folder, in all other cases it returns true even in case
-// of error (only logs the error).
-bool DeleteFilesAndFolders(const std::wstring& exe_path, bool system_uninstall,
-    const installer::Version& installed_version,
+// is using that file.
+// Returns DELETE_SUCCEEDED if all files were successfully delete.
+// Returns DELETE_FAILED if it could not get the path to the install dir.
+// Returns DELETE_REQUIRES_REBOOT if the files were in use and so were
+// scheduled for deletion on next reboot.
+DeleteResult DeleteFilesAndFolders(const std::wstring& exe_path,
+    bool system_uninstall, const installer::Version& installed_version,
     std::wstring* local_state_path, bool delete_profile) {
   std::wstring install_path(installer::GetChromeInstallPath(system_uninstall));
   if (install_path.empty()) {
     LOG(ERROR) << "Could not get installation destination path.";
-    return false;  // Nothing else we can do for uninstall, so we return.
+    return DELETE_FAILED;  // Nothing else we can do to uninstall, so we return.
   } else {
     LOG(INFO) << "install destination path: " << install_path;
   }
@@ -179,27 +189,46 @@ bool DeleteFilesAndFolders(const std::wstring& exe_path, bool system_uninstall,
       file_util::CopyFile(user_local_file, path);
   }
 
+  DeleteResult result = DELETE_SUCCEEDED;
+
   LOG(INFO) << "Deleting install path " << install_path;
   if (!file_util::Delete(install_path, true)) {
     LOG(ERROR) << "Failed to delete folder (1st try): " << install_path;
+#if defined(CHROME_FRAME_BUILD)
+    // We don't try killing Chrome processes for Chrome Frame builds since
+    // that is unlikely to help. Instead, schedule files for deletion and
+    // return a value that will trigger a reboot prompt.
+    ScheduleDirectoryForDeletion(install_path.c_str());
+    result = DELETE_REQUIRES_REBOOT;
+#else
     // Try closing any running chrome processes and deleting files once again.
     CloseAllChromeProcesses();
-    if (!file_util::Delete(install_path, true))
+    if (!file_util::Delete(install_path, true)) {
       LOG(ERROR) << "Failed to delete folder (2nd try): " << install_path;
+      result = DELETE_FAILED;
+    }
+#endif
   }
 
   if (delete_profile) {
     LOG(INFO) << "Deleting user profile" << user_local_state.value();
-    if (!file_util::Delete(user_local_state, true))
-      LOG(ERROR) << "Failed to delete user profle dir: "
+    if (!file_util::Delete(user_local_state, true)) {
+      LOG(ERROR) << "Failed to delete user profile dir: "
                  << user_local_state.value();
+#if defined(CHROME_FRAME_BUILD)
+      ScheduleDirectoryForDeletion(user_local_state.value().c_str());
+      result = DELETE_REQUIRES_REBOOT;
+#else
+      result = DELETE_FAILED;
+#endif
+    }
     DeleteEmptyParentDir(user_local_state.value());
   }
 
   // Now check and delete if the parent directories are empty
   // For example Google\Chrome or Chromium
   DeleteEmptyParentDir(install_path);
-  return true;
+  return result;
 }
 
 // This method tries to delete a registry key and logs an error message
@@ -368,7 +397,9 @@ installer_util::InstallStatus installer_setup::UninstallChrome(
   if (force_uninstall) {
     // Since --force-uninstall command line option is used, we are going to
     // do silent uninstall. Try to close all running Chrome instances.
+#if !defined(CHROME_FRAME_BUILD)
     CloseAllChromeProcesses();
+#endif
   } else {  // no --force-uninstall so lets show some UI dialog boxes.
     status = IsChromeActiveOrUserCancelled(system_uninstall);
     if (status != installer_util::UNINSTALL_CONFIRMED &&
@@ -469,9 +500,14 @@ installer_util::InstallStatus installer_setup::UninstallChrome(
       (cmd_line.HasSwitch(installer_util::switches::kDeleteProfile));
   std::wstring local_state_path;
   ret = installer_util::UNINSTALL_SUCCESSFUL;
-  if (!DeleteFilesAndFolders(exe_path, system_uninstall, *installed_version,
-                             &local_state_path, delete_profile))
+
+  DeleteResult delete_result = DeleteFilesAndFolders(exe_path,
+      system_uninstall, *installed_version, &local_state_path, delete_profile);
+  if (delete_result == DELETE_FAILED) {
     ret = installer_util::UNINSTALL_FAILED;
+  } else if (delete_result == DELETE_REQUIRES_REBOOT) {
+    ret = installer_util::UNINSTALL_REQUIRES_REBOOT;
+  }
 
   if (!force_uninstall) {
     LOG(INFO) << "Uninstallation complete. Launching Uninstall survey.";