Display proper error message at couple of places during installation.

* If Chrome is used to download and install the same version of Chrome, it should display that the current version is in use.
* When Chrome gets updated while it is running, there is no reason to launch Chrome (as it would still be old version), so do not write the launch command to registry.
* Remove the case of HIGHER_VERSION_EXISTS as a successful install. Not sure why it was marked as successful install.
* Remove the code to remove ChromeExt registry keys as this has already been pushed to the channels that had it in the first place.
* Move the code to convert install_status into boolean (1 or 0) in Chromium distribution class.

BUG=19459, 25534
TEST= 1) Install Chrome, and keep it running. 2) Try to install the same version again and check the error message in the registry. 3) Try to install a higher version and make launch command doesn't get written to the registry.

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

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

10 files changed, 83 insertions, 90 deletions

diff --git a/chrome/installer/setup/install.cc b/chrome/installer/setup/install.cc
index 9b36b20..088ba65 100644
--- a/chrome/installer/setup/install.cc
+++ b/chrome/installer/setup/install.cc
@@ -444,7 +444,6 @@ void RegisterChromeOnMachine(const std::wstring& install_path,
 }
 
 // This function installs a new version of Chrome to the specified location.
-// It returns true if install was successful and false in case of an error.
 //
 // exe_path: Path to the executable (setup.exe) as it will be copied
 //           to Chrome install folder after install is complete
@@ -468,16 +467,17 @@ void RegisterChromeOnMachine(const std::wstring& install_path,
 // install_path. If install_path does not exist before calling the function
 // (typical new install), the function creates install_path during install
 // and removes the whole directory during rollback.
-bool InstallNewVersion(const std::wstring& exe_path,
-                       const std::wstring& archive_path,
-                       const std::wstring& src_path,
-                       const std::wstring& install_path,
-                       const std::wstring& temp_dir,
-                       const HKEY reg_root,
-                       const installer::Version& new_version,
-                       std::wstring* current_version) {
+installer_util::InstallStatus InstallNewVersion(
+    const std::wstring& exe_path,
+    const std::wstring& archive_path,
+    const std::wstring& src_path,
+    const std::wstring& install_path,
+    const std::wstring& temp_dir,
+    const HKEY reg_root,
+    const installer::Version& new_version,
+    std::wstring* current_version) {
   if (reg_root != HKEY_LOCAL_MACHINE && reg_root != HKEY_CURRENT_USER)
-    return false;
+    return installer_util::INSTALL_FAILED;
 
   if (InstallUtil::IsChromeFrameProcess()) {
     // Make sure that we don't end up deleting installed files on next reboot.
@@ -491,20 +491,6 @@ bool InstallNewVersion(const std::wstring& exe_path,
   install_list->AddCreateDirWorkItem(FilePath::FromWStringHack(temp_dir));
   install_list->AddCreateDirWorkItem(FilePath::FromWStringHack(install_path));
 
-  // If it is system level install copy the version folder (since we want to
-  // take the permissions of %ProgramFiles% folder) otherwise just move it.
-  if (reg_root == HKEY_LOCAL_MACHINE) {
-    install_list->AddCopyTreeWorkItem(
-        AppendPath(src_path, new_version.GetString()),
-        AppendPath(install_path, new_version.GetString()),
-        temp_dir, WorkItem::ALWAYS);
-  } else {
-    install_list->AddMoveTreeWorkItem(
-        AppendPath(src_path, new_version.GetString()),
-        AppendPath(install_path, new_version.GetString()),
-        temp_dir);
-  }
-
   // Delete any new_chrome.exe if present (we will end up creating a new one
   // if required) and then copy chrome.exe
   std::wstring new_chrome_exe = AppendPath(install_path,
@@ -531,6 +517,20 @@ bool InstallNewVersion(const std::wstring& exe_path,
         temp_dir, WorkItem::ALWAYS);
   }
 
+  // If it is system level install copy the version folder (since we want to
+  // take the permissions of %ProgramFiles% folder) otherwise just move it.
+  if (reg_root == HKEY_LOCAL_MACHINE) {
+    install_list->AddCopyTreeWorkItem(
+        AppendPath(src_path, new_version.GetString()),
+        AppendPath(install_path, new_version.GetString()),
+        temp_dir, WorkItem::ALWAYS);
+  } else {
+    install_list->AddMoveTreeWorkItem(
+        AppendPath(src_path, new_version.GetString()),
+        AppendPath(install_path, new_version.GetString()),
+        temp_dir);
+  }
+
   // Copy the default Dictionaries only if the folder doesnt exist already
   install_list->AddCopyTreeWorkItem(
       AppendPath(src_path, installer::kDictionaries),
@@ -563,16 +563,44 @@ bool InstallNewVersion(const std::wstring& exe_path,
                                        new_version.GetString(),
                                        true);    // overwrite version
 
+  installer_util::InstallStatus result = installer_util::INSTALL_FAILED;
   if (!install_list->Do() ||
       !DoPostInstallTasks(reg_root, exe_path, install_path,
                           new_chrome_exe, *current_version, new_version)) {
+    if (file_util::PathExists(FilePath::FromWStringHack(new_chrome_exe)) &&
+        !current_version->empty() &&
+        (new_version.GetString() == *current_version))
+      result = installer_util::SAME_VERSION_REPAIR_FAILED;
     LOG(ERROR) << "Install failed, rolling back... ";
     install_list->Rollback();
     LOG(ERROR) << "Rollback complete. ";
-    return false;
+  } else {
+    scoped_ptr<installer::Version> installed_version;
+    if (!current_version->empty())
+      installed_version.reset(
+          installer::Version::GetVersionFromString(*current_version));
+    if (!installed_version.get()) {
+      LOG(INFO) << "First install of version " << new_version.GetString();
+      result = installer_util::FIRST_INSTALL_SUCCESS;
+    } else if (new_version.GetString() == installed_version->GetString()) {
+      LOG(INFO) << "Install repaired of version " << new_version.GetString();
+      result = installer_util::INSTALL_REPAIRED;
+    } else if (new_version.IsHigherThan(installed_version.get())) {
+      if (file_util::PathExists(FilePath::FromWStringHack(new_chrome_exe))) {
+        LOG(INFO) << "Version updated to " << new_version.GetString()
+                  << " while running " << installed_version->GetString();
+        result = installer_util::IN_USE_UPDATED;
+      } else {
+        LOG(INFO) << "Version updated to " << new_version.GetString();
+        result = installer_util::NEW_VERSION_UPDATED;
+      }
+    } else {
+      LOG(ERROR) << "Not sure how we got here while updating"
+                 << ", new version: " << new_version.GetString()
+                 << ", old version: " << installed_version->GetString();
+    }
   }
-
-  return true;
+  return result;
 }
 
 }  // namespace
@@ -607,33 +635,14 @@ installer_util::InstallStatus installer::InstallOrUpdateChrome(
 
   HKEY reg_root = (system_install) ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
   std::wstring current_version;
-  bool install_success = InstallNewVersion(exe_path, archive_path, src_path,
-      install_path, install_temp_path, reg_root, new_version, &current_version);
+  installer_util::InstallStatus result = InstallNewVersion(exe_path,
+      archive_path, src_path, install_path, install_temp_path, reg_root,
+      new_version, &current_version);
 
-  installer_util::InstallStatus result;
-  if (!install_success) {
-    LOG(ERROR) << "Install failed.";
-    result = installer_util::INSTALL_FAILED;
-  } else {
-    if (!installed_version) {
-      LOG(INFO) << "First install of version " << new_version.GetString();
-      result = installer_util::FIRST_INSTALL_SUCCESS;
+  BrowserDistribution* dist = BrowserDistribution::GetDistribution();
+  if (!dist->GetInstallReturnCode(result)) {
+    if (result == installer_util::FIRST_INSTALL_SUCCESS)
       CopyPreferenceFileForFirstRun(system_install, prefs_path);
-    } else if (new_version.GetString() == installed_version->GetString()) {
-      LOG(INFO) << "Install repaired of version " << new_version.GetString();
-      result = installer_util::INSTALL_REPAIRED;
-    } else if (new_version.IsHigherThan(installed_version)) {
-      LOG(INFO) << "Version updated to " << new_version.GetString();
-      result = installer_util::NEW_VERSION_UPDATED;
-    } else {
-      LOG(ERROR) << "Not sure how we got here."
-                 << " New version: " << new_version.GetString()
-                 << ", installed version: " << installed_version->GetString();
-      // This should never happen but we are seeing some inconsistent exit
-      // code reports in Omaha logs. We will treat this case as update to
-      // see if the inconsistency goes away.
-      result = installer_util::NEW_VERSION_UPDATED;
-    }
 
     bool value = false;
     if (!installer_util::GetDistroBooleanPreference(prefs,
diff --git a/chrome/installer/setup/setup_main.cc b/chrome/installer/setup/setup_main.cc
index d1bb152..c23b8db 100644
--- a/chrome/installer/setup/setup_main.cc
+++ b/chrome/installer/setup/setup_main.cc
@@ -260,7 +260,9 @@ installer_util::InstallStatus InstallChrome(const CommandLine& cmd_line,
 
         int install_msg_base = IDS_INSTALL_FAILED_BASE;
         std::wstring chrome_exe;
-        if (install_status != installer_util::INSTALL_FAILED) {
+        if (install_status == installer_util::SAME_VERSION_REPAIR_FAILED) {
+            install_msg_base = IDS_SAME_VERSION_REPAIR_FAILED_BASE;
+        } else if (install_status != installer_util::INSTALL_FAILED) {
           chrome_exe = installer::GetChromeInstallPath(system_level);
           if (chrome_exe.empty()) {
             // If we failed to construct install path, it means the OS call to
@@ -278,7 +280,8 @@ installer_util::InstallStatus InstallChrome(const CommandLine& cmd_line,
         installer_util::GetDistroBooleanPreference(prefs,
             installer_util::master_preferences::kDoNotRegisterForUpdateLaunch,
             &value);
-        bool write_chrome_launch_string = !value;
+        bool write_chrome_launch_string = (!value) &&
+            (install_status != installer_util::IN_USE_UPDATED);
 
         InstallUtil::WriteInstallerResult(system_level, install_status,
             install_msg_base, write_chrome_launch_string ? &chrome_exe : NULL);
@@ -292,7 +295,8 @@ installer_util::InstallStatus InstallChrome(const CommandLine& cmd_line,
               &do_not_launch_chrome);
           if (!system_level && !do_not_launch_chrome)
             installer::LaunchChrome(system_level);
-        } else if (install_status == installer_util::NEW_VERSION_UPDATED) {
+        } else if ((install_status == installer_util::NEW_VERSION_UPDATED) || 
+                   (install_status == installer_util::IN_USE_UPDATED)) {
           installer_setup::RemoveLegacyRegistryKeys();
         }
       }
diff --git a/chrome/installer/util/browser_distribution.cc b/chrome/installer/util/browser_distribution.cc
index b3d0cdd..74c735b 100644
--- a/chrome/installer/util/browser_distribution.cc
+++ b/chrome/installer/util/browser_distribution.cc
@@ -78,8 +78,16 @@ std::wstring BrowserDistribution::GetLongAppDescription() {
 }
 
 int BrowserDistribution::GetInstallReturnCode(
-    installer_util::InstallStatus install_status) {
-  return install_status;
+    installer_util::InstallStatus status) {
+  switch (status) {
+    case installer_util::FIRST_INSTALL_SUCCESS:
+    case installer_util::INSTALL_REPAIRED:
+    case installer_util::NEW_VERSION_UPDATED:
+    case installer_util::IN_USE_UPDATED:
+      return 0;
+    default:
+      return status;
+  }
 }
 
 std::string BrowserDistribution::GetSafeBrowsingName() {
diff --git a/chrome/installer/util/chrome_frame_distribution.cc b/chrome/installer/util/chrome_frame_distribution.cc
index 20ba51b..2b336fe 100644
--- a/chrome/installer/util/chrome_frame_distribution.cc
+++ b/chrome/installer/util/chrome_frame_distribution.cc
@@ -96,19 +96,6 @@ std::wstring ChromeFrameDistribution::GetVersionKey() {
   return key;
 }
 
-int ChromeFrameDistribution::GetInstallReturnCode(
-    installer_util::InstallStatus status) {
-  switch (status) {
-    case installer_util::FIRST_INSTALL_SUCCESS:
-    case installer_util::INSTALL_REPAIRED:
-    case installer_util::NEW_VERSION_UPDATED:
-    case installer_util::HIGHER_VERSION_EXISTS:
-      return 0;  // For Google Update's benefit we need to return 0 for success
-    default:
-      return status;
-  }
-}
-
 void ChromeFrameDistribution::UpdateDiffInstallStatus(bool system_install,
     bool incremental_install, installer_util::InstallStatus install_status) {
   HKEY reg_root = (system_install) ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
diff --git a/chrome/installer/util/chrome_frame_distribution.h b/chrome/installer/util/chrome_frame_distribution.h
index c58cbbe..f7bcce3 100644
--- a/chrome/installer/util/chrome_frame_distribution.h
+++ b/chrome/installer/util/chrome_frame_distribution.h
@@ -43,8 +43,6 @@ class ChromeFrameDistribution : public BrowserDistribution {
 
   virtual std::wstring GetVersionKey();
 
-  virtual int GetInstallReturnCode(installer_util::InstallStatus status);
-
   // This is the point at which the Google Chrome installer removes the Google
   // Update ap value. We implement this simply to have the same behaviour re.
   // the ap value.
diff --git a/chrome/installer/util/google_chrome_distribution.cc b/chrome/installer/util/google_chrome_distribution.cc
index 64816ee..3222501 100644
--- a/chrome/installer/util/google_chrome_distribution.cc
+++ b/chrome/installer/util/google_chrome_distribution.cc
@@ -299,19 +299,6 @@ std::wstring GoogleChromeDistribution::GetAppDescription() {
   return app_description;
 }
 
-int GoogleChromeDistribution::GetInstallReturnCode(
-    installer_util::InstallStatus status) {
-  switch (status) {
-    case installer_util::FIRST_INSTALL_SUCCESS:
-    case installer_util::INSTALL_REPAIRED:
-    case installer_util::NEW_VERSION_UPDATED:
-    case installer_util::HIGHER_VERSION_EXISTS:
-      return 0;  // For Google Update's benefit we need to return 0 for success
-    default:
-      return status;
-  }
-}
-
 std::string GoogleChromeDistribution::GetSafeBrowsingName() {
   return "googlechrome";
 }
diff --git a/chrome/installer/util/google_chrome_distribution.h b/chrome/installer/util/google_chrome_distribution.h
index 7aa265f..7490a05 100644
--- a/chrome/installer/util/google_chrome_distribution.h
+++ b/chrome/installer/util/google_chrome_distribution.h
@@ -57,9 +57,6 @@ class GoogleChromeDistribution : public BrowserDistribution {
 
   virtual std::wstring GetAppDescription();
 
-  virtual int GetInstallReturnCode(
-      installer_util::InstallStatus install_status);
-
   virtual std::string GetSafeBrowsingName();
 
   virtual std::wstring GetStateKey();
diff --git a/chrome/installer/util/install_util.cc b/chrome/installer/util/install_util.cc
index 25f3046b..a6114e5 100644
--- a/chrome/installer/util/install_util.cc
+++ b/chrome/installer/util/install_util.cc
@@ -110,7 +110,7 @@ void InstallUtil::WriteInstallerResult(bool system_install,
     install_list->AddSetRegValueWorkItem(root, key, L"InstallerResultUIString",
                                          msg, true);
   }
-  if (launch_cmd != NULL) {
+  if (launch_cmd != NULL && !launch_cmd->empty()) {
     install_list->AddSetRegValueWorkItem(root, key,
                                          L"InstallerSuccessLaunchCmdLine",
                                          *launch_cmd, true);
diff --git a/chrome/installer/util/prebuild/create_string_rc.py b/chrome/installer/util/prebuild/create_string_rc.py
index e3ccdb5..5433029 100755
--- a/chrome/installer/util/prebuild/create_string_rc.py
+++ b/chrome/installer/util/prebuild/create_string_rc.py
@@ -49,6 +49,7 @@ kStringIds = [
   'IDS_INSTALL_USER_LEVEL_EXISTS',
   'IDS_INSTALL_SYSTEM_LEVEL_EXISTS',
   'IDS_INSTALL_FAILED',
+  'IDS_SAME_VERSION_REPAIR_FAILED',
   'IDS_SETUP_PATCH_FAILED',
   'IDS_INSTALL_OS_NOT_SUPPORTED',
   'IDS_INSTALL_OS_ERROR',
diff --git a/chrome/installer/util/util_constants.h b/chrome/installer/util/util_constants.h
index d781be2..f510cdc 100644
--- a/chrome/installer/util/util_constants.h
+++ b/chrome/installer/util/util_constants.h
@@ -40,7 +40,9 @@ enum InstallStatus {
   EULA_ACCEPTED,         // EULA dialog was accepted by user.
   EULA_ACCEPTED_OPT_IN,  // EULA accepted wtih the crash optin selected.
   INSTALL_DIR_IN_USE,    // Installation directory is in use by another process
-  UNINSTALL_REQUIRES_REBOOT // Uninstallation required a reboot.
+  UNINSTALL_REQUIRES_REBOOT, // Uninstallation required a reboot.
+  IN_USE_UPDATED,        // Chrome successfully updated but old version running
+  SAME_VERSION_REPAIR_FAILED // Chrome repair failed as Chrome was running
 };
 
 namespace switches {