Make uninstall slightly less prone to chrome.exe crashes by changing default to uninstall confirmation.

BUG=1215

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

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

3 files changed, 26 insertions, 45 deletions

diff --git a/chrome/installer/setup/uninstall.cc b/chrome/installer/setup/uninstall.cc
index 832b837..f4cb697 100644
--- a/chrome/installer/setup/uninstall.cc
+++ b/chrome/installer/setup/uninstall.cc
@@ -89,38 +89,31 @@ bool DeleteRegistryValue(HKEY reg_root, const std::wstring& key_path,
 installer_util::InstallStatus IsChromeActiveOrUserCancelled(
     bool system_uninstall) {
   static const std::wstring kCmdLineOptions(L" --uninstall");
-  static const int32 kTimeOutMs = 30000;
   int32 exit_code = ResultCodes::NORMAL_EXIT;
-  bool is_timeout = false;
-
-  // We want to continue with the uninstallation only when chrome.exe either
-  // returns NORMAL_EXIT (means Chrome is not running, user has confirmed
-  // uninstallation and sentinel file/desktop/ql shortcuts have been
-  // cleaned up) or UNINSTALL_DELETE_FILE_ERROR (means Chrome is not running,
-  // user has confirmed uninstallation but there was a problem with deleting
-  // sentinel file, desktop or ql shortcuts).
+
+  // Here we want to save user from frustration (in case of Chrome crashes)
+  // and continue with the uninstallation as long as chrome.exe process exit
+  // code is NOT one of the following:
+  // - UNINSTALL_CHROME_ALIVE - chrome.exe is currently running
+  // - UNINSTALL_USER_CANCEL - User cancelled uninstallation
+  // - HUNG - chrome.exe was killed by HuntForZombieProcesses() (until we can
+  //          give this method some brains and not kill chrome.exe launched
+  //          by us, we will not uninstall if we get this return code).
   LOG(INFO) << "Launching Chrome to do uninstall tasks.";
   if (installer::LaunchChromeAndWaitForResult(system_uninstall,
                                               kCmdLineOptions,
-                                              kTimeOutMs,
-                                              &exit_code,
-                                              &is_timeout)) {
-    if (is_timeout || exit_code == ResultCodes::UNINSTALL_CHROME_ALIVE) {
-      LOG(INFO) << "Can't uninstall when chrome is still running";
-      return installer_util::CHROME_RUNNING;
-    } else if (exit_code == ResultCodes::UNINSTALL_USER_CANCEL) {
-      LOG(INFO) << "User cancelled uninstall operation";
+                                              &exit_code)) {
+    LOG(INFO) << "chrome.exe launched for uninstall confirmation returned: "
+              << exit_code;
+    if ((exit_code == ResultCodes::UNINSTALL_CHROME_ALIVE) ||
+        (exit_code == ResultCodes::UNINSTALL_USER_CANCEL) ||
+        (exit_code == ResultCodes::HUNG))
       return installer_util::UNINSTALL_CANCELLED;
-    } else if (exit_code == ResultCodes::NORMAL_EXIT) {
-      LOG(INFO) << "chrome.exe confirmed uninstallation from user.";
-      return installer_util::UNINSTALL_CONFIRMED;
-    } else if (exit_code == ResultCodes::UNINSTALL_DELETE_FILE_ERROR) {
-      LOG(ERROR) << "chrome.exe returned delete file error.";
-      return installer_util::UNINSTALL_CONFIRMED;
-    }
+  } else {
+    LOG(ERROR) << "Failed to launch chrome.exe for uninstall confirmation.";
   }
 
-  return installer_util::UNINSTALL_FAILED;
+  return installer_util::UNINSTALL_CONFIRMED;
 }
 }  // namespace
 
diff --git a/chrome/installer/util/helper.cc b/chrome/installer/util/helper.cc
index 68fa646..3d86c8d 100644
--- a/chrome/installer/util/helper.cc
+++ b/chrome/installer/util/helper.cc
@@ -44,9 +44,7 @@ bool installer::LaunchChrome(bool system_install) {
 
 bool installer::LaunchChromeAndWaitForResult(bool system_install,
                                              const std::wstring& options,
-                                             int32 timeout_ms,
-                                             int32* exit_code,
-                                             bool* is_timeout) {
+                                             int32* exit_code) {
   std::wstring chrome_exe(installer::GetChromeInstallPath(system_install));
   if (chrome_exe.empty())
     return false;
@@ -63,16 +61,9 @@ bool installer::LaunchChromeAndWaitForResult(bool system_install,
     return false;
   }
 
-  DWORD wr = ::WaitForSingleObject(pi.hProcess, timeout_ms);
-  if (WAIT_TIMEOUT == wr) {
-    if (is_timeout)
-      *is_timeout = true;
-  } else {  // WAIT_OBJECT_0
-    if (is_timeout)
-      *is_timeout = false;
-    if (exit_code) {
-      ::GetExitCodeProcess(pi.hProcess, reinterpret_cast<DWORD*>(exit_code));
-    }
+  DWORD wr = ::WaitForSingleObject(pi.hProcess, INFINITE);
+  if (exit_code) {
+    ::GetExitCodeProcess(pi.hProcess, reinterpret_cast<DWORD*>(exit_code));
   }
 
   ::CloseHandle(pi.hProcess);
diff --git a/chrome/installer/util/helper.h b/chrome/installer/util/helper.h
index c530615..c4678b5 100644
--- a/chrome/installer/util/helper.h
+++ b/chrome/installer/util/helper.h
@@ -21,16 +21,13 @@ std::wstring GetChromeInstallPath(bool system_install);
 // Launches Chrome without waiting for its exit.
 bool LaunchChrome(bool system_install);
 
-// Launches Chrome with given command line, waits for Chrome to terminate
-// within timeout_ms, and gets the process exit code if available.
+// Launches Chrome with given command line, waits for Chrome indefinitely
+// (until it terminates), and gets the process exit code if available.
 // The function returns true as long as Chrome is successfully launched.
-// The status of Chrome at the return of the function is given by exit_code
-// and is_timeout.
+// The status of Chrome at the return of the function is given by exit_code.
 bool LaunchChromeAndWaitForResult(bool system_install,
                                   const std::wstring& options,
-                                  int32 timeout_ms,
-                                  int32* exit_code,
-                                  bool* is_timeout);
+                                  int32* exit_code);
 
 // This function tries to remove all previous version directories after a new
 // Chrome update. If an instance of Chrome with older version is still running