* Check the presence of new_chrome.exe before using opv key for version.

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

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

3 files changed, 47 insertions, 32 deletions

diff --git a/chrome/app/client_util.cc b/chrome/app/client_util.cc
index 9908cf6..42230cd 100644
--- a/chrome/app/client_util.cc
+++ b/chrome/app/client_util.cc
@@ -6,6 +6,29 @@
 #include "chrome/installer/util/install_util.h"
 
 #include "chrome/installer/util/google_update_constants.h"
+#include "chrome/installer/util/util_constants.h"
+
+namespace {
+bool ReadStrValueFromRegistry(const HKEY reg_key,
+                              const wchar_t* const value_name,
+                              wchar_t** value) {
+  DWORD size = 0;
+  if (::RegQueryValueEx(reg_key, value_name, NULL, NULL, NULL,
+                        &size) != ERROR_SUCCESS) {
+    return false;
+  }
+
+  *value = new wchar_t[1 + (size / sizeof(wchar_t))];
+  if (::RegQueryValueEx(reg_key, value_name, NULL, NULL,
+                        reinterpret_cast<BYTE*>(*value),
+                        &size) != ERROR_SUCCESS) {
+    delete[] *value;
+    return false;
+  }
+
+  return true;
+}
+}
 
 namespace client_util {
 bool FileExists(const wchar_t* const file_path) {
@@ -17,39 +40,25 @@ bool GetChromiumVersion(const wchar_t* const exe_path,
                         const wchar_t* const reg_key_path,
                         wchar_t** version) {
   HKEY reg_root = InstallUtil::IsPerUserInstall(exe_path) ? HKEY_CURRENT_USER :
-                                                HKEY_LOCAL_MACHINE;
+                                                            HKEY_LOCAL_MACHINE;
   HKEY reg_key;
   if (::RegOpenKeyEx(reg_root, reg_key_path, 0,
                      KEY_READ, &reg_key) != ERROR_SUCCESS) {
     return false;
   }
-  DWORD size = 0;
-  bool ret = false;
-  if (::RegQueryValueEx(reg_key, google_update::kRegOldVersionField, NULL, NULL,
-                        NULL, &size) == ERROR_SUCCESS) {
-    *version = new wchar_t[1 + (size / sizeof(wchar_t))];
-    if (::RegQueryValueEx(reg_key, google_update::kRegOldVersionField,
-                          NULL, NULL, reinterpret_cast<BYTE*>(*version),
-                          &size) == ERROR_SUCCESS) {
-      ret = true;
-    } else {
-      delete[] *version;
-    }
+
+  std::wstring new_chrome_exe(exe_path);
+  new_chrome_exe.append(installer_util::kChromeNewExe);
+  if (FileExists(new_chrome_exe.c_str()) &&
+      ReadStrValueFromRegistry(reg_key, google_update::kRegOldVersionField,
+                               version)) {
     ::RegCloseKey(reg_key);
-    return ret;
+    return true;
   }
 
-  if (::RegQueryValueEx(reg_key, google_update::kRegVersionField, NULL, NULL,
-                        NULL, &size) == ERROR_SUCCESS) {
-    *version = new wchar_t[1 + (size / sizeof(wchar_t))];
-    if (::RegQueryValueEx(reg_key, google_update::kRegVersionField,
-                          NULL, NULL, reinterpret_cast<BYTE*>(*version),
-                          &size) == ERROR_SUCCESS) {
-      ret = true;
-    } else {
-      delete[] *version;
-    }
-  }
+  bool ret = ReadStrValueFromRegistry(reg_key,
+                                      google_update::kRegVersionField,
+                                      version);
   ::RegCloseKey(reg_key);
   return ret;
 }
diff --git a/chrome/installer/setup/install.cc b/chrome/installer/setup/install.cc
index b051f82..7543212 100644
--- a/chrome/installer/setup/install.cc
+++ b/chrome/installer/setup/install.cc
@@ -160,13 +160,13 @@ bool installer::InstallNewVersion(const std::wstring& exe_path,
   std::wstring new_chrome_exe = AppendPath(install_path,
                                            installer_util::kChromeNewExe);
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
-  RegKey chrome_key(reg_root, dist->GetVersionKey().c_str(),
-                    KEY_READ | KEY_WRITE);
+  RegKey chrome_key(reg_root, dist->GetVersionKey().c_str(), KEY_READ);
   std::wstring current_version;
   if (file_util::PathExists(new_chrome_exe))
     chrome_key.ReadValue(google_update::kRegOldVersionField, &current_version);
   if (current_version.empty())
     chrome_key.ReadValue(google_update::kRegVersionField, &current_version);
+  chrome_key.Close();
 
   install_list->AddDeleteTreeWorkItem(new_chrome_exe, std::wstring());
   install_list->AddCopyTreeWorkItem(
@@ -250,11 +250,17 @@ bool installer::InstallNewVersion(const std::wstring& exe_path,
         }
       }
     } else {
-      if (chrome_key.ValueExists(google_update::kRegOldVersionField) &&
-          !chrome_key.DeleteValue(google_update::kRegOldVersionField)) {
-        LOG(ERROR) << "New chrome.exe doesn't exist but failed to delete "
-                   << "old version value in registry.";
+      scoped_ptr<WorkItemList> inuse_list(WorkItem::CreateWorkItemList());
+      inuse_list->AddDeleteRegValueWorkItem(reg_root, version_key, 
+                                            google_update::kRegOldVersionField,
+                                            true);
+      inuse_list->AddDeleteRegValueWorkItem(reg_root, version_key, 
+                                            google_update::kRegRenameCmdField,
+                                            true);
+      if (!inuse_list->Do()) {
+        LOG(ERROR) << "Couldn't write old version/rename value to registry.";
         success = false;
+        inuse_list->Rollback();
       }
     }
   }
diff --git a/chrome/installer/util/work_item_list.h b/chrome/installer/util/work_item_list.h
index 227533c..18cb186 100644
--- a/chrome/installer/util/work_item_list.h
+++ b/chrome/installer/util/work_item_list.h
@@ -47,7 +47,7 @@ class WorkItemList : public WorkItem {
   // Add a DeleteRegValueWorkItem that deletes registry value of type REG_SZ
   // or REG_DWORD.
   bool AddDeleteRegValueWorkItem(HKEY predefined_root, std::wstring key_path,
-                                 std::wstring value_name, bool it_str_type);
+                                 std::wstring value_name, bool is_str_type);
 
   // Add a DeleteTreeWorkItem that recursively deletes a file system
   // hierarchy at the given root path. A key file can be optionally specified