Regkey functions return error code instead of bool

Change the Regkey helper to consistently use and return LONG
instead of bool.  Fix RegKey usage all over the code base and
get rid of workarounds due to lack of return value.

Reviewers:

brettw: everything (skip parts for other reviewers if you wish)
robertshield,grt: chrome_frame, installer
siggi: ceee


BUG=none
TEST=covered by existing tests



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

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

26 files changed, 388 insertions, 447 deletions

diff --git a/chrome/installer/util/channel_info.cc b/chrome/installer/util/channel_info.cc
index fd8c6b5..4abcb70 100644
--- a/chrome/installer/util/channel_info.cc
+++ b/chrome/installer/util/channel_info.cc
@@ -116,11 +116,12 @@ bool SetModifier(ModifierIndex index, bool set, std::wstring* ap_value) {
 namespace installer {
 
 bool ChannelInfo::Initialize(const RegKey& key) {
-  return key.ReadValue(google_update::kRegApField, &value_);
+  return (key.ReadValue(google_update::kRegApField, &value_) == ERROR_SUCCESS);
 }
 
 bool ChannelInfo::Write(RegKey* key) const {
-  return key->WriteValue(google_update::kRegApField, value_.c_str());
+  return (key->WriteValue(google_update::kRegApField, value_.c_str()) ==
+      ERROR_SUCCESS);
 }
 
 bool ChannelInfo::GetChannelName(std::wstring* channel_name) const {
diff --git a/chrome/installer/util/create_reg_key_work_item.cc b/chrome/installer/util/create_reg_key_work_item.cc
index 4fa08c6..e634c9c 100644
--- a/chrome/installer/util/create_reg_key_work_item.cc
+++ b/chrome/installer/util/create_reg_key_work_item.cc
@@ -55,7 +55,7 @@ bool CreateRegKeyWorkItem::Do() {
     key_path.assign(key_list_[i - 1]);
 
     if (key.CreateWithDisposition(predefined_root_, key_path.c_str(),
-                                  &disposition, KEY_READ)) {
+                                  &disposition, KEY_READ) == ERROR_SUCCESS) {
       if (disposition == REG_OPENED_EXISTING_KEY) {
         if (key_created_) {
           // This should not happen. Someone created a subkey under the key
diff --git a/chrome/installer/util/create_reg_key_work_item_unittest.cc b/chrome/installer/util/create_reg_key_work_item_unittest.cc
index c4f9d88..47aa13e 100644
--- a/chrome/installer/util/create_reg_key_work_item_unittest.cc
+++ b/chrome/installer/util/create_reg_key_work_item_unittest.cc
@@ -24,14 +24,15 @@ class CreateRegKeyWorkItemTest : public testing::Test {
     // Create a temporary key for testing
     RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS);
     key.DeleteKey(test_root);
-    ASSERT_FALSE(key.Open(HKEY_CURRENT_USER, test_root, KEY_READ));
-    ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, test_root, KEY_READ));
+    ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER, test_root, KEY_READ));
+    ASSERT_EQ(ERROR_SUCCESS, key.Create(HKEY_CURRENT_USER, test_root,
+                                        KEY_READ));
   }
   virtual void TearDown() {
     logging::CloseLogFile();
     // Clean up the temporary key
     RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS);
-    ASSERT_TRUE(key.DeleteKey(test_root));
+    ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(test_root));
   }
 };
 
@@ -42,8 +43,8 @@ TEST_F(CreateRegKeyWorkItemTest, CreateKey) {
 
   FilePath parent_key(test_root);
   parent_key = parent_key.AppendASCII("a");
-  ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, parent_key.value().c_str(),
-                         KEY_READ));
+  ASSERT_EQ(ERROR_SUCCESS,
+      key.Create(HKEY_CURRENT_USER, parent_key.value().c_str(), KEY_READ));
 
   FilePath top_key_to_create(parent_key);
   top_key_to_create = top_key_to_create.AppendASCII("b");
@@ -58,16 +59,16 @@ TEST_F(CreateRegKeyWorkItemTest, CreateKey) {
 
   EXPECT_TRUE(work_item->Do());
 
-  EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, key_to_create.value().c_str(),
-                       KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create.value().c_str(), KEY_READ));
 
   work_item->Rollback();
 
   // Rollback should delete all the keys up to top_key_to_create.
-  EXPECT_FALSE(key.Open(HKEY_CURRENT_USER, top_key_to_create.value().c_str(),
-                        KEY_READ));
-  EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, parent_key.value().c_str(),
-                       KEY_READ));
+  EXPECT_NE(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, top_key_to_create.value().c_str(), KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, parent_key.value().c_str(), KEY_READ));
 }
 
 TEST_F(CreateRegKeyWorkItemTest, CreateExistingKey) {
@@ -75,8 +76,8 @@ TEST_F(CreateRegKeyWorkItemTest, CreateExistingKey) {
 
   FilePath key_to_create(test_root);
   key_to_create = key_to_create.AppendASCII("aa");
-  ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, key_to_create.value().c_str(),
-                         KEY_READ));
+  ASSERT_EQ(ERROR_SUCCESS,
+      key.Create(HKEY_CURRENT_USER, key_to_create.value().c_str(), KEY_READ));
 
   scoped_ptr<CreateRegKeyWorkItem> work_item(
       WorkItem::CreateCreateRegKeyWorkItem(HKEY_CURRENT_USER,
@@ -84,15 +85,15 @@ TEST_F(CreateRegKeyWorkItemTest, CreateExistingKey) {
 
   EXPECT_TRUE(work_item->Do());
 
-  EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, key_to_create.value().c_str(),
-                       KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create.value().c_str(), KEY_READ));
 
   work_item->Rollback();
 
   // Rollback should not remove the key since it exists before
   // the CreateRegKeyWorkItem is called.
-  EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, key_to_create.value().c_str(),
-                       KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create.value().c_str(), KEY_READ));
 }
 
 TEST_F(CreateRegKeyWorkItemTest, CreateSharedKey) {
@@ -112,26 +113,26 @@ TEST_F(CreateRegKeyWorkItemTest, CreateSharedKey) {
 
   EXPECT_TRUE(work_item->Do());
 
-  EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, key_to_create_3.value().c_str(),
-                       KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create_3.value().c_str(), KEY_READ));
 
   // Create another key under key_to_create_2
   FilePath key_to_create_4(key_to_create_2);
   key_to_create_4 = key_to_create_4.AppendASCII("ddd");
-  ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, key_to_create_4.value().c_str(),
-                         KEY_READ));
+  ASSERT_EQ(ERROR_SUCCESS,
+      key.Create(HKEY_CURRENT_USER, key_to_create_4.value().c_str(), KEY_READ));
 
   work_item->Rollback();
 
   // Rollback should delete key_to_create_3.
-  EXPECT_FALSE(key.Open(HKEY_CURRENT_USER, key_to_create_3.value().c_str(),
-                        KEY_READ));
+  EXPECT_NE(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create_3.value().c_str(), KEY_READ));
 
   // Rollback should not delete key_to_create_2 as it is shared.
-  EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, key_to_create_2.value().c_str(),
-                       KEY_READ));
-  EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, key_to_create_4.value().c_str(),
-                       KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create_2.value().c_str(), KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create_4.value().c_str(), KEY_READ));
 }
 
 TEST_F(CreateRegKeyWorkItemTest, RollbackWithMissingKey) {
@@ -151,22 +152,22 @@ TEST_F(CreateRegKeyWorkItemTest, RollbackWithMissingKey) {
 
   EXPECT_TRUE(work_item->Do());
 
-  EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, key_to_create_3.value().c_str(),
-                       KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create_3.value().c_str(), KEY_READ));
   key.Close();
 
   // now delete key_to_create_3
-  ASSERT_TRUE(RegDeleteKey(HKEY_CURRENT_USER,
-                           key_to_create_3.value().c_str()) == ERROR_SUCCESS);
-  ASSERT_FALSE(key.Open(HKEY_CURRENT_USER, key_to_create_3.value().c_str(),
-                        KEY_READ));
+  ASSERT_EQ(ERROR_SUCCESS,
+      RegDeleteKey(HKEY_CURRENT_USER, key_to_create_3.value().c_str()));
+  ASSERT_NE(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create_3.value().c_str(), KEY_READ));
 
   work_item->Rollback();
 
   // key_to_create_3 has already been deleted, Rollback should delete
   // the rest.
-  ASSERT_FALSE(key.Open(HKEY_CURRENT_USER, key_to_create_1.value().c_str(),
-                        KEY_READ));
+  ASSERT_NE(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create_1.value().c_str(), KEY_READ));
 }
 
 TEST_F(CreateRegKeyWorkItemTest, RollbackWithSetValue) {
@@ -182,14 +183,14 @@ TEST_F(CreateRegKeyWorkItemTest, RollbackWithSetValue) {
   EXPECT_TRUE(work_item->Do());
 
   // Write a value under the key we just created.
-  EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, key_to_create.value().c_str(),
-                       KEY_READ | KEY_SET_VALUE));
-  EXPECT_TRUE(key.WriteValue(L"name", L"value"));
+  EXPECT_EQ(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER,
+      key_to_create.value().c_str(), KEY_READ | KEY_SET_VALUE));
+  EXPECT_EQ(ERROR_SUCCESS, key.WriteValue(L"name", L"value"));
   key.Close();
 
   work_item->Rollback();
 
   // Rollback should not remove the key.
-  EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, key_to_create.value().c_str(),
-                       KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create.value().c_str(), KEY_READ));
 }
diff --git a/chrome/installer/util/delete_after_reboot_helper.cc b/chrome/installer/util/delete_after_reboot_helper.cc
index a678c73..4a50df5 100644
--- a/chrome/installer/util/delete_after_reboot_helper.cc
+++ b/chrome/installer/util/delete_after_reboot_helper.cc
@@ -379,13 +379,14 @@ bool RemoveFromMovesPendingReboot(const wchar_t* directory) {
 
   if (strings_to_keep.size() <= 1) {
     // We have only the trailing NULL string. Don't bother writing that.
-    return session_manager_key.DeleteValue(kPendingFileRenameOps);
+    return (session_manager_key.DeleteValue(kPendingFileRenameOps) ==
+        ERROR_SUCCESS);
   }
   std::vector<char> buffer;
   StringArrayToMultiSZBytes(strings_to_keep, &buffer);
   DCHECK(buffer.size() > 0);
   if (buffer.empty())
     return false;
-  return session_manager_key.WriteValue(kPendingFileRenameOps, &buffer[0],
-                                        buffer.size(), REG_MULTI_SZ);
+  return (session_manager_key.WriteValue(kPendingFileRenameOps, &buffer[0],
+      buffer.size(), REG_MULTI_SZ) == ERROR_SUCCESS);
 }
diff --git a/chrome/installer/util/delete_reg_value_work_item.cc b/chrome/installer/util/delete_reg_value_work_item.cc
index 1db853a..143dae8 100644
--- a/chrome/installer/util/delete_reg_value_work_item.cc
+++ b/chrome/installer/util/delete_reg_value_work_item.cc
@@ -5,6 +5,7 @@
 #include "chrome/installer/util/delete_reg_value_work_item.h"
 
 #include "base/logging.h"
+#include "base/string_util.h"
 #include "base/win/registry.h"
 #include "chrome/installer/util/logging_installer.h"
 
@@ -12,16 +13,12 @@ using base::win::RegKey;
 
 DeleteRegValueWorkItem::DeleteRegValueWorkItem(HKEY predefined_root,
                                                const std::wstring& key_path,
-                                               const std::wstring& value_name,
-                                               DWORD type)
+                                               const std::wstring& value_name)
     : predefined_root_(predefined_root),
       key_path_(key_path),
       value_name_(value_name),
-      type_(type),
-      status_(DELETE_VALUE),
-      old_dw_(0),
-      old_qword_(0) {
-  DCHECK(type_ == REG_QWORD || type_ == REG_DWORD || type == REG_SZ);
+      previous_type_(0),
+      status_(DELETE_VALUE) {
 }
 
 DeleteRegValueWorkItem::~DeleteRegValueWorkItem() {
@@ -36,51 +33,39 @@ bool DeleteRegValueWorkItem::Do() {
 
   status_ = VALUE_UNCHANGED;
 
-  // A big flaw in the RegKey implementation is that all error information
-  // (besides success/failure) is lost in the translation from LSTATUS to bool.
-  // So, we resort to direct API calls here. :-/
-  HKEY raw_key = NULL;
-  LSTATUS err = RegOpenKeyEx(predefined_root_, key_path_.c_str(), 0,
-                             KEY_READ, &raw_key);
-  if (err != ERROR_SUCCESS) {
-    if (err == ERROR_FILE_NOT_FOUND) {
-      LOG(INFO) << "(delete value) can not open " << key_path_;
-      status_ = VALUE_NOT_FOUND;
-      return true;
-    }
-  } else {
-    ::RegCloseKey(raw_key);
-  }
-
   RegKey key;
-  bool result = false;
-
-  // Used in REG_QWORD case only
-  DWORD value_size = sizeof(old_qword_);
   DWORD type = 0;
+  DWORD size = 0;
+  LONG result = key.Open(predefined_root_, key_path_.c_str(),
+                         KEY_READ | KEY_WRITE);
+  if (result == ERROR_SUCCESS)
+    result = key.ReadValue(value_name_.c_str(), NULL, &size, &type);
 
-  if (!key.Open(predefined_root_, key_path_.c_str(), KEY_READ | KEY_WRITE)) {
-    LOG(ERROR) << "can not open " << key_path_;
-  } else if (!key.ValueExists(value_name_.c_str())) {
+  if (result == ERROR_FILE_NOT_FOUND) {
+    LOG(INFO) << "(delete value) Key: " << key_path_ << " or Value: "
+              << value_name_ << " does not exist.";
     status_ = VALUE_NOT_FOUND;
-    result = true;
-  // Read previous value for rollback and delete
-  } else if (((type_ == REG_SZ && key.ReadValue(value_name_.c_str(),
-                                                &old_str_)) ||
-              (type_ == REG_DWORD && key.ReadValueDW(value_name_.c_str(),
-                                                     &old_dw_)) ||
-              (type_ == REG_QWORD && key.ReadValue(value_name_.c_str(),
-                                                   &old_qword_,
-                                                   &value_size, &type) &&
-                  type == REG_QWORD && value_size == sizeof(old_qword_))) &&
-             (key.DeleteValue(value_name_.c_str()))) {
-    status_ = VALUE_DELETED;
-    result = true;
-  } else {
-    LOG(ERROR) << "failed to read/delete value " << value_name_;
+    return true;
+  }
+
+  if (result == ERROR_SUCCESS) {
+    previous_value_.resize(size);
+    result = key.ReadValue(value_name_.c_str(), &previous_value_[0], &size,
+                           &previous_type_);
+    if (result != ERROR_SUCCESS) {
+      previous_value_.erase();
+      VLOG(1) << "Failed to save original value. Error: " << result;
+    }
   }
 
-  return result;
+  result = key.DeleteValue(value_name_.c_str());
+  if (result != ERROR_SUCCESS) {
+    VLOG(1) << "Failed to delete value " << value_name_ << " error: " << result;
+    return false;
+  }
+
+  status_ = VALUE_DELETED;
+  return true;
 }
 
 void DeleteRegValueWorkItem::Rollback() {
@@ -94,23 +79,16 @@ void DeleteRegValueWorkItem::Rollback() {
 
   // At this point only possible state is VALUE_DELETED.
   RegKey key;
-  if (!key.Open(predefined_root_, key_path_.c_str(), KEY_READ | KEY_WRITE)) {
-    LOG(ERROR) << "rollback: can not open " << key_path_;
-  // try to restore the previous value
-  } else if ((type_ == REG_SZ && key.WriteValue(value_name_.c_str(),
-                                                old_str_.c_str())) ||
-             (type_ == REG_DWORD && key.WriteValue(value_name_.c_str(),
-                                                   old_dw_)) ||
-             (type_ == REG_QWORD && key.WriteValue(value_name_.c_str(),
-                                                   &old_qword_,
-                                                   sizeof(old_qword_),
-                                                   REG_QWORD))) {
-    status_ = VALUE_ROLLED_BACK;
-    VLOG(1) << "rollback: restored " << value_name_;
+  LONG result = key.Open(predefined_root_, key_path_.c_str(),
+                         KEY_READ | KEY_WRITE);
+  if (result == ERROR_SUCCESS) {
+    // try to restore the previous value
+    result = key.WriteValue(value_name_.c_str(), &previous_value_[0],
+                            static_cast<DWORD>(previous_value_.size()),
+                            previous_type_);
+    VLOG_IF(1, result != ERROR_SUCCESS) << "rollback: restoring "
+                                        << value_name_ << " error: " << result;
   } else {
-    LOG(ERROR) << "failed to restore value " << value_name_;
+    VLOG(1) << "can not open " << key_path_ << " error: " << result;
   }
-
-  key.Close();
-  return;
 }
diff --git a/chrome/installer/util/delete_reg_value_work_item.h b/chrome/installer/util/delete_reg_value_work_item.h
index 7b477fe..fc3d5a7 100644
--- a/chrome/installer/util/delete_reg_value_work_item.h
+++ b/chrome/installer/util/delete_reg_value_work_item.h
@@ -40,7 +40,7 @@ class DeleteRegValueWorkItem : public WorkItem {
   };
 
   DeleteRegValueWorkItem(HKEY predefined_root, const std::wstring& key_path,
-                         const std::wstring& value_name, DWORD type);
+                         const std::wstring& value_name);
 
   // Root key of the target key under which the value is set. The root key can
   // only be one of the predefined keys on Windows.
@@ -52,19 +52,11 @@ class DeleteRegValueWorkItem : public WorkItem {
   // Name of the value to be set.
   std::wstring value_name_;
 
-  // DWORD that tells whether data value is of type REG_SZ, REG_DWORD, or
-  // REG_QWORD
-  // Ideally we do not need this information from user of this class and can
-  // check the registry for the type. But to simpify implementation we are
-  // going to put the burden on the caller for now to provide us the type.
-  DWORD type_;
-
   DeletionStatus status_;
 
-  // Data of the previous value.
-  std::wstring old_str_;  // if data is of type REG_SZ
-  DWORD old_dw_;  // if data is of type REG_DWORD
-  int64 old_qword_;  // if data is of type REG_QWORD
+  // Previous value.
+  DWORD previous_type_;
+  std::string previous_value_;
 };
 
 #endif  // CHROME_INSTALLER_UTIL_DELETE_REG_VALUE_WORK_ITEM_H_
diff --git a/chrome/installer/util/delete_reg_value_work_item_unittest.cc b/chrome/installer/util/delete_reg_value_work_item_unittest.cc
index e8c37c0..98f37d15 100644
--- a/chrome/installer/util/delete_reg_value_work_item_unittest.cc
+++ b/chrome/installer/util/delete_reg_value_work_item_unittest.cc
@@ -24,14 +24,15 @@ class DeleteRegValueWorkItemTest : public testing::Test {
     // Create a temporary key for testing
     RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS);
     key.DeleteKey(test_root);
-    ASSERT_FALSE(key.Open(HKEY_CURRENT_USER, test_root, KEY_READ));
-    ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, test_root, KEY_READ));
+    ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER, test_root, KEY_READ));
+    ASSERT_EQ(ERROR_SUCCESS,
+        key.Create(HKEY_CURRENT_USER, test_root, KEY_READ));
   }
   virtual void TearDown() {
     logging::CloseLogFile();
     // Clean up the temporary key
     RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS);
-    ASSERT_TRUE(key.DeleteKey(test_root));
+    ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(test_root));
   }
 };
 
@@ -43,21 +44,21 @@ TEST_F(DeleteRegValueWorkItemTest, DeleteExistingValue) {
   RegKey key;
   std::wstring parent_key(test_root);
   file_util::AppendToPath(&parent_key, L"WriteNew");
-  ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, parent_key.c_str(),
-                         KEY_READ | KEY_WRITE));
+  ASSERT_EQ(ERROR_SUCCESS,
+      key.Create(HKEY_CURRENT_USER, parent_key.c_str(), KEY_READ | KEY_WRITE));
   std::wstring name_str(L"name_str");
   std::wstring data_str(L"data_111");
-  ASSERT_TRUE(key.WriteValue(name_str.c_str(), data_str.c_str()));
+  ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(name_str.c_str(), data_str.c_str()));
   std::wstring name_dword(L"name_dword");
   DWORD data_dword = 100;
-  ASSERT_TRUE(key.WriteValue(name_dword.c_str(), data_dword));
+  ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(name_dword.c_str(), data_dword));
 
   scoped_ptr<DeleteRegValueWorkItem> work_item1(
       WorkItem::CreateDeleteRegValueWorkItem(HKEY_CURRENT_USER, parent_key,
-                                             name_str, REG_SZ));
+                                             name_str));
   scoped_ptr<DeleteRegValueWorkItem> work_item2(
       WorkItem::CreateDeleteRegValueWorkItem(HKEY_CURRENT_USER, parent_key,
-                                             name_dword, REG_DWORD));
+                                             name_dword));
 
   EXPECT_TRUE(work_item1->Do());
   EXPECT_TRUE(work_item2->Do());
@@ -70,8 +71,8 @@ TEST_F(DeleteRegValueWorkItemTest, DeleteExistingValue) {
 
   std::wstring read_str;
   DWORD read_dword;
-  EXPECT_TRUE(key.ReadValue(name_str.c_str(), &read_str));
-  EXPECT_TRUE(key.ReadValueDW(name_dword.c_str(), &read_dword));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name_str.c_str(), &read_str));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name_dword.c_str(), &read_dword));
   EXPECT_EQ(read_str, data_str);
   EXPECT_EQ(read_dword, data_dword);
 }
@@ -81,8 +82,8 @@ TEST_F(DeleteRegValueWorkItemTest, DeleteNonExistentValue) {
   RegKey key;
   std::wstring parent_key(test_root);
   file_util::AppendToPath(&parent_key, L"WriteNew");
-  ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, parent_key.c_str(),
-                         KEY_READ | KEY_WRITE));
+  ASSERT_EQ(ERROR_SUCCESS,
+      key.Create(HKEY_CURRENT_USER, parent_key.c_str(), KEY_READ | KEY_WRITE));
   std::wstring name_str(L"name_str");
   std::wstring name_dword(L"name_dword");
   EXPECT_FALSE(key.ValueExists(name_str.c_str()));
@@ -90,10 +91,10 @@ TEST_F(DeleteRegValueWorkItemTest, DeleteNonExistentValue) {
 
   scoped_ptr<DeleteRegValueWorkItem> work_item1(
       WorkItem::CreateDeleteRegValueWorkItem(HKEY_CURRENT_USER, parent_key,
-                                             name_str, REG_SZ));
+                                             name_str));
   scoped_ptr<DeleteRegValueWorkItem> work_item2(
       WorkItem::CreateDeleteRegValueWorkItem(HKEY_CURRENT_USER, parent_key,
-                                             name_dword, REG_DWORD));
+                                             name_dword));
 
   EXPECT_TRUE(work_item1->Do());
   EXPECT_TRUE(work_item2->Do());
diff --git a/chrome/installer/util/google_chrome_distribution.cc b/chrome/installer/util/google_chrome_distribution.cc
index 17ded03..e0ab343 100644
--- a/chrome/installer/util/google_chrome_distribution.cc
+++ b/chrome/installer/util/google_chrome_distribution.cc
@@ -448,7 +448,7 @@ std::wstring GoogleChromeDistribution::GetDistributionData(HKEY root_key) {
   std::wstring result;
   std::wstring brand_value;
   if (client_state_key.ReadValue(google_update::kRegRLZBrandField,
-                                 &brand_value)) {
+                                 &brand_value) == ERROR_SUCCESS) {
     result = google_update::kRegRLZBrandField;
     result.append(L"=");
     result.append(brand_value);
@@ -457,7 +457,7 @@ std::wstring GoogleChromeDistribution::GetDistributionData(HKEY root_key) {
 
   std::wstring client_value;
   if (client_state_key.ReadValue(google_update::kRegClientField,
-                                 &client_value)) {
+                                 &client_value) == ERROR_SUCCESS) {
     result.append(google_update::kRegClientField);
     result.append(L"=");
     result.append(client_value);
diff --git a/chrome/installer/util/google_update_settings.cc b/chrome/installer/util/google_update_settings.cc
index 1e1d5e3..b93a6d6 100644
--- a/chrome/installer/util/google_update_settings.cc
+++ b/chrome/installer/util/google_update_settings.cc
@@ -40,9 +40,9 @@ bool ReadGoogleUpdateStrKey(const wchar_t* const name, std::wstring* value) {
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   std::wstring reg_path = dist->GetStateKey();
   RegKey key(HKEY_CURRENT_USER, reg_path.c_str(), KEY_READ);
-  if (!key.ReadValue(name, value)) {
+  if (key.ReadValue(name, value) != ERROR_SUCCESS) {
     RegKey hklm_key(HKEY_LOCAL_MACHINE, reg_path.c_str(), KEY_READ);
-    return hklm_key.ReadValue(name, value);
+    return (hklm_key.ReadValue(name, value) == ERROR_SUCCESS);
   }
   return true;
 }
@@ -52,7 +52,7 @@ bool WriteGoogleUpdateStrKey(const wchar_t* const name,
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   std::wstring reg_path = dist->GetStateKey();
   RegKey key(HKEY_CURRENT_USER, reg_path.c_str(), KEY_READ | KEY_WRITE);
-  return key.WriteValue(name, value.c_str());
+  return (key.WriteValue(name, value.c_str()) == ERROR_SUCCESS);
 }
 
 bool ClearGoogleUpdateStrKey(const wchar_t* const name) {
@@ -60,9 +60,9 @@ bool ClearGoogleUpdateStrKey(const wchar_t* const name) {
   std::wstring reg_path = dist->GetStateKey();
   RegKey key(HKEY_CURRENT_USER, reg_path.c_str(), KEY_READ | KEY_WRITE);
   std::wstring value;
-  if (!key.ReadValue(name, &value))
+  if (key.ReadValue(name, &value) != ERROR_SUCCESS)
     return false;
-  return key.WriteValue(name, L"");
+  return (key.WriteValue(name, L"") == ERROR_SUCCESS);
 }
 
 bool RemoveGoogleUpdateStrKey(const wchar_t* const name) {
@@ -71,18 +71,17 @@ bool RemoveGoogleUpdateStrKey(const wchar_t* const name) {
   RegKey key(HKEY_CURRENT_USER, reg_path.c_str(), KEY_READ | KEY_WRITE);
   if (!key.ValueExists(name))
     return true;
-  return key.DeleteValue(name);
+  return (key.DeleteValue(name) == ERROR_SUCCESS);
 }
 
 EulaSearchResult HasEULASetting(HKEY root, const std::wstring& state_key,
                                 bool setting) {
   RegKey key;
-  DWORD previous_value;
-
-  if (!key.Open(root, state_key.c_str(), KEY_QUERY_VALUE))
+  DWORD previous_value = setting ? 1 : 0;
+  if (key.Open(root, state_key.c_str(), KEY_QUERY_VALUE) != ERROR_SUCCESS)
     return NO_SETTING;
-
-  if (!key.ReadValueDW(google_update::kRegEULAAceptedField, &previous_value))
+  if (key.ReadValueDW(google_update::kRegEULAAceptedField,
+                      &previous_value) != ERROR_SUCCESS)
     return FOUND_CLIENT_STATE;
 
   return ((previous_value != 0) == setting) ?
@@ -95,11 +94,11 @@ bool GoogleUpdateSettings::GetCollectStatsConsent() {
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   std::wstring reg_path = dist->GetStateKey();
   RegKey key(HKEY_CURRENT_USER, reg_path.c_str(), KEY_READ);
-  DWORD value;
-  if (!key.ReadValueDW(google_update::kRegUsageStatsField, &value)) {
-    RegKey hklm_key(HKEY_LOCAL_MACHINE, reg_path.c_str(), KEY_READ);
-    if (!hklm_key.ReadValueDW(google_update::kRegUsageStatsField, &value))
-      return false;
+  DWORD value = 0;
+  if (key.ReadValueDW(google_update::kRegUsageStatsField, &value) !=
+      ERROR_SUCCESS) {
+    key.Open(HKEY_LOCAL_MACHINE, reg_path.c_str(), KEY_READ);
+    key.ReadValueDW(google_update::kRegUsageStatsField, &value);
   }
   return (1 == value);
 }
@@ -109,12 +108,13 @@ bool GoogleUpdateSettings::SetCollectStatsConsent(bool consented) {
   // Writing to HKLM is only a best effort deal.
   BrowserDistribution* dist = BrowserDistribution::GetDistribution();
   std::wstring reg_path = dist->GetStateMediumKey();
-  RegKey key_hklm(HKEY_LOCAL_MACHINE, reg_path.c_str(), KEY_READ | KEY_WRITE);
-  key_hklm.WriteValue(google_update::kRegUsageStatsField, value);
+  RegKey key(HKEY_LOCAL_MACHINE, reg_path.c_str(), KEY_READ | KEY_WRITE);
+  key.WriteValue(google_update::kRegUsageStatsField, value);
   // Writing to HKCU is used both by chrome and by the crash reporter.
   reg_path = dist->GetStateKey();
-  RegKey key_hkcu(HKEY_CURRENT_USER, reg_path.c_str(), KEY_READ | KEY_WRITE);
-  return key_hkcu.WriteValue(google_update::kRegUsageStatsField, value);
+  key.Open(HKEY_CURRENT_USER, reg_path.c_str(), KEY_READ | KEY_WRITE);
+  return (key.WriteValue(google_update::kRegUsageStatsField, value) ==
+      ERROR_SUCCESS);
 }
 
 bool GoogleUpdateSettings::GetMetricsId(std::wstring* metrics_id) {
@@ -156,7 +156,8 @@ bool GoogleUpdateSettings::SetEULAConsent(
     }
   }
   RegKey key(HKEY_LOCAL_MACHINE, reg_path.c_str(), KEY_SET_VALUE);
-  return key.WriteValue(google_update::kRegEULAAceptedField, consented ? 1 : 0);
+  return (key.WriteValue(google_update::kRegEULAAceptedField,
+                         consented ? 1 : 0) == ERROR_SUCCESS);
 }
 
 int GoogleUpdateSettings::GetLastRunTime() {
@@ -247,17 +248,21 @@ void GoogleUpdateSettings::UpdateInstallStatus(bool system_install,
   std::wstring reg_key(google_update::kRegPathClientState);
   reg_key.append(L"\\");
   reg_key.append(product_guid);
-  if (!key.Open(reg_root, reg_key.c_str(), KEY_QUERY_VALUE | KEY_SET_VALUE) ||
-      !channel_info.Initialize(key)) {
+  LONG result = key.Open(reg_root, reg_key.c_str(),
+                         KEY_QUERY_VALUE | KEY_SET_VALUE);
+  if (result != ERROR_SUCCESS || !channel_info.Initialize(key)) {
     VLOG(1) << "Application key not found.";
     if (!incremental_install && !multi_install || !install_return_code) {
       VLOG(1) << "Returning without changing application key.";
       return;
     } else if (!key.Valid()) {
       reg_key.assign(google_update::kRegPathClientState);
-      if (!key.Open(reg_root, reg_key.c_str(), KEY_CREATE_SUB_KEY) ||
-          !key.CreateKey(product_guid.c_str(), KEY_SET_VALUE)) {
-        LOG(ERROR) << "Failed to create application key.";
+      result = key.Open(reg_root, reg_key.c_str(), KEY_CREATE_SUB_KEY);
+      if (result == ERROR_SUCCESS)
+        result = key.CreateKey(product_guid.c_str(), KEY_SET_VALUE);
+
+      if (result != ERROR_SUCCESS) {
+        LOG(ERROR) << "Failed to create application key. Error: " << result;
         return;
       }
     }
diff --git a/chrome/installer/util/google_update_settings_unittest.cc b/chrome/installer/util/google_update_settings_unittest.cc
index d041d12..96a6dae 100644
--- a/chrome/installer/util/google_update_settings_unittest.cc
+++ b/chrome/installer/util/google_update_settings_unittest.cc
@@ -49,8 +49,10 @@ class GoogleUpdateSettingsTest: public testing::Test {
     EXPECT_TRUE(err == ERROR_SUCCESS || err == ERROR_FILE_NOT_FOUND);
 
     // Create the keys we're redirecting HKCU and HKLM to.
-    ASSERT_TRUE(hkcu_.Create(HKEY_CURRENT_USER, kHKCUReplacement, KEY_READ));
-    ASSERT_TRUE(hklm_.Create(HKEY_CURRENT_USER, kHKLMReplacement, KEY_READ));
+    ASSERT_EQ(ERROR_SUCCESS,
+        hkcu_.Create(HKEY_CURRENT_USER, kHKCUReplacement, KEY_READ));
+    ASSERT_EQ(ERROR_SUCCESS,
+        hklm_.Create(HKEY_CURRENT_USER, kHKLMReplacement, KEY_READ));
 
     // And do the switcharoo.
     ASSERT_EQ(ERROR_SUCCESS,
@@ -83,8 +85,8 @@ class GoogleUpdateSettingsTest: public testing::Test {
     RegKey update_key;
     BrowserDistribution* dist = BrowserDistribution::GetDistribution();
     std::wstring path = dist->GetStateKey();
-    ASSERT_TRUE(update_key.Create(root, path.c_str(), KEY_WRITE));
-    ASSERT_TRUE(update_key.WriteValue(L"ap", value));
+    ASSERT_EQ(ERROR_SUCCESS, update_key.Create(root, path.c_str(), KEY_WRITE));
+    ASSERT_EQ(ERROR_SUCCESS, update_key.WriteValue(L"ap", value));
   }
 
   // Tests setting the ap= value to various combinations of values with
@@ -165,11 +167,12 @@ class GoogleUpdateSettingsTest: public testing::Test {
     RegKey key;
     std::wstring ap_key_value;
     std::wstring reg_key = GetApKeyPath();
-    if (key.Open(HKEY_CURRENT_USER, reg_key.c_str(), KEY_ALL_ACCESS) &&
-        key.ReadValue(google_update::kRegApField, &ap_key_value)) {
-      return ap_key_value;
+    if (key.Open(HKEY_CURRENT_USER, reg_key.c_str(), KEY_ALL_ACCESS) ==
+        ERROR_SUCCESS) {
+      key.ReadValue(google_update::kRegApField, &ap_key_value);
     }
-    return std::wstring();
+
+    return ap_key_value;
   }
 
   RegKey hkcu_;
@@ -369,10 +372,11 @@ TEST_F(GoogleUpdateSettingsTest, UpdateInstallStatusTest) {
   HKEY reg_root = HKEY_CURRENT_USER;
   bool ap_key_deleted = false;
   RegKey key;
-  if (!key.Open(HKEY_CURRENT_USER, reg_key.c_str(), KEY_ALL_ACCESS)) {
+  if (key.Open(HKEY_CURRENT_USER, reg_key.c_str(), KEY_ALL_ACCESS) !=
+      ERROR_SUCCESS) {
     work_item_list->AddCreateRegKeyWorkItem(reg_root, reg_key);
     ASSERT_TRUE(work_item_list->Do()) << "Failed to create ClientState key.";
-  } else if (key.DeleteValue(google_update::kRegApField)) {
+  } else if (key.DeleteValue(google_update::kRegApField) == ERROR_SUCCESS) {
     ap_key_deleted = true;
   }
   // try differential installer
@@ -420,22 +424,27 @@ TEST_F(GoogleUpdateSettingsTest, SetEULAConsent) {
 
   // By default, eulaconsent ends up on the package.
   EXPECT_TRUE(GoogleUpdateSettings::SetEULAConsent(*package.get(), true));
-  EXPECT_TRUE(key.Open(HKEY_LOCAL_MACHINE,
-                       properties.GetStateMediumKey().c_str(),
-                       KEY_QUERY_VALUE | KEY_SET_VALUE));
-  EXPECT_TRUE(key.ReadValueDW(google_update::kRegEULAAceptedField, &value));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.Open(HKEY_LOCAL_MACHINE, properties.GetStateMediumKey().c_str(),
+               KEY_QUERY_VALUE | KEY_SET_VALUE));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.ReadValueDW(google_update::kRegEULAAceptedField, &value));
   EXPECT_EQ(1U, value);
-  EXPECT_TRUE(key.DeleteValue(google_update::kRegEULAAceptedField));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.DeleteValue(google_update::kRegEULAAceptedField));
 
   // But it will end up on the product if needed
-  EXPECT_TRUE(key.Create(HKEY_LOCAL_MACHINE,
-                         distribution->GetStateKey().c_str(), KEY_SET_VALUE));
-  EXPECT_TRUE(key.WriteValue(google_update::kRegEULAAceptedField,
-                             static_cast<DWORD>(0)));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.Create(HKEY_LOCAL_MACHINE, distribution->GetStateKey().c_str(),
+                 KEY_SET_VALUE));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.WriteValue(google_update::kRegEULAAceptedField,
+                     static_cast<DWORD>(0)));
   EXPECT_TRUE(GoogleUpdateSettings::SetEULAConsent(*package.get(), true));
-  EXPECT_TRUE(key.Open(HKEY_LOCAL_MACHINE,
-                       distribution->GetStateMediumKey().c_str(),
-                       KEY_QUERY_VALUE | KEY_SET_VALUE));
-  EXPECT_TRUE(key.ReadValueDW(google_update::kRegEULAAceptedField, &value));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.Open(HKEY_LOCAL_MACHINE, distribution->GetStateMediumKey().c_str(),
+               KEY_QUERY_VALUE | KEY_SET_VALUE));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.ReadValueDW(google_update::kRegEULAAceptedField, &value));
   EXPECT_EQ(1U, value);
 }
diff --git a/chrome/installer/util/install_util.cc b/chrome/installer/util/install_util.cc
index f241cdd..0e598dd 100644
--- a/chrome/installer/util/install_util.cc
+++ b/chrome/installer/util/install_util.cc
@@ -82,20 +82,25 @@ Version* InstallUtil::GetChromeVersion(BrowserDistribution* dist,
                                        bool system_install) {
   DCHECK(dist);
   RegKey key;
-  std::wstring version_str;
-
   HKEY reg_root = (system_install) ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
-  if (!key.Open(reg_root, dist->GetVersionKey().c_str(), KEY_READ) ||
-      !key.ReadValue(google_update::kRegVersionField, &version_str)) {
+  LONG result = key.Open(reg_root, dist->GetVersionKey().c_str(), KEY_READ);
+
+  std::wstring version_str;
+  if (result == ERROR_SUCCESS)
+    result = key.ReadValue(google_update::kRegVersionField, &version_str);
+
+  Version* ret = NULL;
+  if (result == ERROR_SUCCESS && !version_str.empty()) {
+    VLOG(1) << "Existing " << dist->GetApplicationName()
+            << " version found " << version_str;
+    ret = Version::GetVersionFromString(WideToASCII(version_str));
+  } else {
+    DCHECK_EQ(ERROR_FILE_NOT_FOUND, result);
     VLOG(1) << "No existing " << dist->GetApplicationName()
             << " install found.";
-    key.Close();
-    return NULL;
   }
-  key.Close();
-  VLOG(1) << "Existing " << dist->GetApplicationName()
-          << " version found " << version_str;
-  return Version::GetVersionFromString(WideToASCII(version_str));
+
+  return ret;
 }
 
 bool InstallUtil::IsOSSupported() {
@@ -195,9 +200,10 @@ bool InstallUtil::BuildDLLRegistrationList(const std::wstring& install_path,
 bool InstallUtil::DeleteRegistryKey(RegKey& root_key,
                                     const std::wstring& key_path) {
   VLOG(1) << "Deleting registry key " << key_path;
-  if (!root_key.DeleteKey(key_path.c_str()) &&
-      ::GetLastError() != ERROR_FILE_NOT_FOUND) {
-    PLOG(ERROR) << "Failed to delete registry key: " << key_path;
+  LONG result = root_key.DeleteKey(key_path.c_str());
+  if (result != ERROR_SUCCESS && result != ERROR_FILE_NOT_FOUND) {
+    PLOG(ERROR) << "Failed to delete registry key: " << key_path
+                << " error: " << result;
     return false;
   }
   return true;
@@ -211,10 +217,13 @@ bool InstallUtil::DeleteRegistryValue(HKEY reg_root,
                                       const std::wstring& value_name) {
   RegKey key(reg_root, key_path.c_str(), KEY_ALL_ACCESS);
   VLOG(1) << "Deleting registry value " << value_name;
-  if (key.ValueExists(value_name.c_str()) &&
-      !key.DeleteValue(value_name.c_str())) {
-    LOG(ERROR) << "Failed to delete registry value: " << value_name;
-    return false;
+  if (key.ValueExists(value_name.c_str())) {
+    LONG result = key.DeleteValue(value_name.c_str());
+    if (result != ERROR_SUCCESS) {
+      LOG(ERROR) << "Failed to delete registry value: " << value_name
+                 << " error: " << result;
+      return false;
+    }
   }
   return true;
 }
diff --git a/chrome/installer/util/install_util_unittest.cc b/chrome/installer/util/install_util_unittest.cc
index 95e0c3d..b0e05ba 100644
--- a/chrome/installer/util/install_util_unittest.cc
+++ b/chrome/installer/util/install_util_unittest.cc
@@ -45,9 +45,10 @@ TEST_F(InstallUtilTest, InstallerResult) {
     BrowserDistribution* distribution =
         BrowserDistribution::GetSpecificDistribution(
             BrowserDistribution::CHROME_BROWSER, prefs);
-    EXPECT_TRUE(key.Open(root, distribution->GetStateKey().c_str(), KEY_READ));
-    EXPECT_TRUE(key.ReadValue(installer::kInstallerSuccessLaunchCmdLine,
-                              &value));
+    EXPECT_EQ(ERROR_SUCCESS,
+        key.Open(root, distribution->GetStateKey().c_str(), KEY_READ));
+    EXPECT_EQ(ERROR_SUCCESS,
+        key.ReadValue(installer::kInstallerSuccessLaunchCmdLine, &value));
     EXPECT_EQ(launch_cmd, value);
   }
   TempRegKeyOverride::DeleteAllTempKeys();
@@ -67,9 +68,10 @@ TEST_F(InstallUtilTest, InstallerResult) {
     BrowserDistribution* distribution =
         BrowserDistribution::GetSpecificDistribution(
             BrowserDistribution::CHROME_BROWSER, prefs);
-    EXPECT_TRUE(key.Open(root, distribution->GetStateKey().c_str(), KEY_READ));
-    EXPECT_TRUE(key.ReadValue(installer::kInstallerSuccessLaunchCmdLine,
-                              &value));
+    EXPECT_EQ(ERROR_SUCCESS,
+        key.Open(root, distribution->GetStateKey().c_str(), KEY_READ));
+    EXPECT_EQ(ERROR_SUCCESS,
+        key.ReadValue(installer::kInstallerSuccessLaunchCmdLine, &value));
     EXPECT_EQ(launch_cmd, value);
     key.Close();
   }
diff --git a/chrome/installer/util/installation_state.cc b/chrome/installer/util/installation_state.cc
index 85023bf..c0ab82d 100644
--- a/chrome/installer/util/installation_state.cc
+++ b/chrome/installer/util/installation_state.cc
@@ -22,13 +22,14 @@ void ProductState::Initialize(bool system_install,
   const HKEY root_key = system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
   base::win::RegKey key(root_key, version_key.c_str(), KEY_QUERY_VALUE);
   std::wstring version_str;
-  if (key.ReadValue(google_update::kRegVersionField, &version_str)) {
+  if (key.ReadValue(google_update::kRegVersionField, &version_str)
+      == ERROR_SUCCESS) {
     version_.reset(Version::GetVersionFromString(WideToASCII(version_str)));
     if (version_.get() != NULL) {
       // The product is installed.  Check for the channel value (absent if not
       // installed/managed by Google Update).
-      if (!key.Open(root_key, state_key.c_str(), KEY_QUERY_VALUE) ||
-          !channel_.Initialize(key)) {
+      if ((key.Open(root_key, state_key.c_str(), KEY_QUERY_VALUE) !=
+          ERROR_SUCCESS) || !channel_.Initialize(key)) {
         channel_.set_value(std::wstring());
       }
     }
diff --git a/chrome/installer/util/package_properties_unittest.cc b/chrome/installer/util/package_properties_unittest.cc
index 6b6618d..000cc72 100644
--- a/chrome/installer/util/package_properties_unittest.cc
+++ b/chrome/installer/util/package_properties_unittest.cc
@@ -32,8 +32,8 @@ TEST_F(PackagePropertiesTest, Basic) {
     if (!props[i]->ReceivesUpdates()) {
       TempRegKeyOverride override(HKEY_CURRENT_USER, L"props");
       RegKey key;
-      EXPECT_TRUE(key.Create(HKEY_CURRENT_USER, state_key.c_str(),
-                             KEY_ALL_ACCESS));
+      EXPECT_EQ(ERROR_SUCCESS,
+          key.Create(HKEY_CURRENT_USER, state_key.c_str(), KEY_ALL_ACCESS));
     }
     TempRegKeyOverride::DeleteAllTempKeys();
   }
diff --git a/chrome/installer/util/package_unittest.cc b/chrome/installer/util/package_unittest.cc
index 89ed2cb..ea696a7 100644
--- a/chrome/installer/util/package_unittest.cc
+++ b/chrome/installer/util/package_unittest.cc
@@ -137,14 +137,15 @@ namespace {
 bool SetUninstallArguments(HKEY root, BrowserDistribution* dist,
                            const CommandLine& args) {
   RegKey key(root, dist->GetStateKey().c_str(), KEY_ALL_ACCESS);
-  return key.WriteValue(installer::kUninstallArgumentsField,
-                        args.command_line_string().c_str());
+  return (key.WriteValue(installer::kUninstallArgumentsField,
+                         args.command_line_string().c_str()) == ERROR_SUCCESS);
 }
 
 bool SetInstalledVersion(HKEY root, BrowserDistribution* dist,
                          const std::wstring& version) {
   RegKey key(root, dist->GetVersionKey().c_str(), KEY_ALL_ACCESS);
-  return key.WriteValue(google_update::kRegVersionField, version.c_str());
+  return (key.WriteValue(google_update::kRegVersionField, version.c_str()) ==
+      ERROR_SUCCESS);
 }
 }  // end namespace
 
diff --git a/chrome/installer/util/product.cc b/chrome/installer/util/product.cc
index bddd7d8..de56027 100644
--- a/chrome/installer/util/product.cc
+++ b/chrome/installer/util/product.cc
@@ -126,12 +126,11 @@ bool Product::IsMsi() const {
       // We didn't find it in the preferences, try looking in the registry.
       HKEY reg_root = system_level() ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
       RegKey key;
-      if (key.Open(reg_root, distribution_->GetStateKey().c_str(), KEY_READ)) {
-        DWORD msi_value;
-        if (key.ReadValueDW(google_update::kRegMSIField, &msi_value) &&
-            msi_value != 0) {
-          msi_ = true;
-        }
+      if (key.Open(reg_root, distribution_->GetStateKey().c_str(),
+                   KEY_READ) == ERROR_SUCCESS) {
+        DWORD msi_value = 0;
+        key.ReadValueDW(google_update::kRegMSIField, &msi_value);
+        msi_ = msi_value != 0;
       }
     } else {
       msi_ = true;
@@ -143,22 +142,19 @@ bool Product::IsMsi() const {
 }
 
 bool Product::SetMsiMarker(bool set) const {
-  bool success = false;
   HKEY reg_root = system_level() ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER;
   RegKey client_state_key;
-  if (client_state_key.Open(reg_root, distribution_->GetStateKey().c_str(),
-                            KEY_READ | KEY_WRITE)) {
-    DWORD msi_value = set ? 1 : 0;
-    if (client_state_key.WriteValue(google_update::kRegMSIField, msi_value)) {
-      success = true;
-    } else {
-      LOG(ERROR) << "Could not write MSI value to client state key.";
-    }
-  } else {
-    LOG(ERROR) << "SetMsiMarker: Could not open client state key!";
+  LONG result = client_state_key.Open(reg_root,
+      distribution_->GetStateKey().c_str(), KEY_READ | KEY_WRITE);
+  if (result == ERROR_SUCCESS) {
+    result = client_state_key.WriteValue(google_update::kRegMSIField,
+                                         set ? 1 : 0);
   }
 
-  return success;
+  LOG_IF(ERROR, result != ERROR_SUCCESS) << "Failed to Open or Write MSI value"
+      "to client state key. error: " << result;
+
+  return (result == ERROR_SUCCESS);
 }
 
 bool Product::ShouldCreateUninstallEntry() const {
diff --git a/chrome/installer/util/product_unittest.cc b/chrome/installer/util/product_unittest.cc
index c03b253..c072dc5 100644
--- a/chrome/installer/util/product_unittest.cc
+++ b/chrome/installer/util/product_unittest.cc
@@ -53,8 +53,8 @@ TempRegKeyOverride::TempRegKeyOverride(HKEY override, const wchar_t* temp_name)
   DCHECK(temp_name && lstrlenW(temp_name));
   std::wstring key_path(kTempTestKeyPath);
   key_path += L"\\" + temp_name_;
-  EXPECT_TRUE(temp_key_.Create(HKEY_CURRENT_USER, key_path.c_str(),
-                               KEY_ALL_ACCESS));
+  EXPECT_EQ(ERROR_SUCCESS,
+      temp_key_.Create(HKEY_CURRENT_USER, key_path.c_str(), KEY_ALL_ACCESS));
   EXPECT_EQ(ERROR_SUCCESS,
             ::RegOverridePredefKey(override_, temp_key_.Handle()));
 }
@@ -67,7 +67,7 @@ TempRegKeyOverride::~TempRegKeyOverride() {
 // static
 void TempRegKeyOverride::DeleteAllTempKeys() {
   RegKey key;
-  if (key.Open(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS)) {
+  if (key.Open(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS) == ERROR_SUCCESS) {
     key.DeleteKey(kTempTestKeyPath);
   }
 }
@@ -114,7 +114,8 @@ TEST_F(ProductTest, ProductInstallBasic) {
     // Create a make-believe client state key.
     RegKey key;
     std::wstring state_key_path(distribution->GetStateKey());
-    ASSERT_TRUE(key.Create(root, state_key_path.c_str(), KEY_ALL_ACCESS));
+    ASSERT_EQ(ERROR_SUCCESS,
+        key.Create(root, state_key_path.c_str(), KEY_ALL_ACCESS));
 
     // Set the MSI marker, delete the objects, create new ones and verify
     // that we now see the MSI marker.
diff --git a/chrome/installer/util/set_reg_value_work_item.cc b/chrome/installer/util/set_reg_value_work_item.cc
index 08d0097..9c4d1c0 100644
--- a/chrome/installer/util/set_reg_value_work_item.cc
+++ b/chrome/installer/util/set_reg_value_work_item.cc
@@ -5,6 +5,7 @@
 #include "chrome/installer/util/set_reg_value_work_item.h"
 
 #include "base/logging.h"
+#include "base/string_util.h"
 #include "base/win/registry.h"
 #include "chrome/installer/util/logging_installer.h"
 
@@ -19,14 +20,12 @@ SetRegValueWorkItem::SetRegValueWorkItem(HKEY predefined_root,
     : predefined_root_(predefined_root),
       key_path_(key_path),
       value_name_(value_name),
-      value_data_str_(value_data),
-      value_data_dword_(0),
-      value_data_qword_(0),
       overwrite_(overwrite),
       status_(SET_VALUE),
       type_(REG_SZ),
-      previous_value_dword_(0),
-      previous_value_qword_(0) {
+      previous_type_(0) {
+  const uint8* data = reinterpret_cast<const uint8*>(value_data.c_str());
+  value_.assign(data, data + (value_data.length() + 1) * sizeof(wchar_t));
 }
 
 SetRegValueWorkItem::SetRegValueWorkItem(HKEY predefined_root,
@@ -37,13 +36,12 @@ SetRegValueWorkItem::SetRegValueWorkItem(HKEY predefined_root,
     : predefined_root_(predefined_root),
       key_path_(key_path),
       value_name_(value_name),
-      value_data_dword_(value_data),
-      value_data_qword_(0),
       overwrite_(overwrite),
       status_(SET_VALUE),
       type_(REG_DWORD),
-      previous_value_dword_(0),
-      previous_value_qword_(0) {
+      previous_type_(0) {
+  const uint8* data = reinterpret_cast<const uint8*>(&value_data);
+  value_.assign(data, data + sizeof(value_data));
 }
 
 SetRegValueWorkItem::SetRegValueWorkItem(HKEY predefined_root,
@@ -54,151 +52,94 @@ SetRegValueWorkItem::SetRegValueWorkItem(HKEY predefined_root,
     : predefined_root_(predefined_root),
       key_path_(key_path),
       value_name_(value_name),
-      value_data_dword_(0),
-      value_data_qword_(value_data),
       overwrite_(overwrite),
       status_(SET_VALUE),
       type_(REG_QWORD),
-      previous_value_dword_(0),
-      previous_value_qword_(0) {
+      previous_type_(0) {
+  const uint8* data = reinterpret_cast<const uint8*>(&value_data);
+  value_.assign(data, data + sizeof(value_data));
 }
 
 bool SetRegValueWorkItem::Do() {
-  bool success = true;
-
+  LONG result = ERROR_SUCCESS;
+  base::win::RegKey key;
   if (status_ != SET_VALUE) {
     // we already did something.
-    LOG(ERROR) << "multiple calls to Do()";
-    success = false;
+    VLOG(1) << "multiple calls to Do()";
+    result = ERROR_CANTWRITE;
+    return ignore_failure_;
   }
 
-  base::win::RegKey key;
-  if (success && !key.Open(predefined_root_, key_path_.c_str(),
-                           KEY_READ | KEY_SET_VALUE)) {
-    LOG(ERROR) << "can not open " << key_path_;
-    status_ = VALUE_UNCHANGED;
-    success = false;
+  status_ = VALUE_UNCHANGED;
+  result = key.Open(predefined_root_, key_path_.c_str(),
+                    KEY_READ | KEY_SET_VALUE);
+  if (result != ERROR_SUCCESS) {
+    VLOG(1) << "can not open " << key_path_ << " error: " << result;
+    return ignore_failure_;
   }
 
-  if (success) {
-    if (key.ValueExists(value_name_.c_str())) {
-      if (overwrite_) {
-        // Read previous value for rollback and write new value
-        if (type_ == REG_SZ) {
-          std::wstring data;
-          if (key.ReadValue(value_name_.c_str(), &data)) {
-            previous_value_str_.assign(data);
-          }
-          success = key.WriteValue(value_name_.c_str(),
-                                   value_data_str_.c_str());
-        } else if (type_ == REG_DWORD) {
-          DWORD data;
-          if (key.ReadValueDW(value_name_.c_str(), &data)) {
-            previous_value_dword_ = data;
-          }
-          success = key.WriteValue(value_name_.c_str(), value_data_dword_);
-        } else if (type_ == REG_QWORD) {
-          int64 data;
-          DWORD data_size = sizeof(data);
-          DWORD data_type = NULL;
-
-          if (key.ReadValue(value_name_.c_str(), &data, &data_size,
-                            &data_type)) {
-            previous_value_qword_ = data;
-          }
-          success = key.WriteValue(value_name_.c_str(),
-                                   &value_data_qword_,
-                                   sizeof(value_data_qword_),
-                                   REG_QWORD);
-        }
-        if (success) {
-          VLOG(1) << "overwritten value for " << value_name_;
-          status_ = VALUE_OVERWRITTEN;
-        } else {
-          LOG(ERROR) << "failed to overwrite value for " << value_name_;
-          status_ = VALUE_UNCHANGED;
-        }
-      } else {
-        VLOG(1) << value_name_ << " exists. not changed ";
-        status_ = VALUE_UNCHANGED;
-      }
-    } else {
-      if (type_ == REG_SZ) {
-        success = key.WriteValue(value_name_.c_str(), value_data_str_.c_str());
-      } else if (type_ == REG_DWORD) {
-        success = key.WriteValue(value_name_.c_str(), value_data_dword_);
-      } else if (type_ == REG_QWORD) {
-        success = key.WriteValue(value_name_.c_str(),
-                                 &value_data_qword_,
-                                 sizeof(value_data_qword_),
-                                 REG_QWORD);
-      } else {
-        NOTREACHED() << "Unsupported value type.";
-      }
-      if (success) {
-        VLOG(1) << "created value for " << value_name_;
-        status_ = NEW_VALUE_CREATED;
-      } else {
-        LOG(ERROR) << "failed to create value for " << value_name_;
-        status_ = VALUE_UNCHANGED;
-      }
-    }
+  DWORD type = 0;
+  DWORD size = 0;
+  result = key.ReadValue(value_name_.c_str(), NULL, &size, &type);
+  // If the value exists but we don't want to overwrite then there's
+  // nothing more to do.
+  if ((result != ERROR_FILE_NOT_FOUND) && !overwrite_) {
+    return true;
   }
 
-  LOG_IF(ERROR, !success && !log_message_.empty()) << log_message_;
+  // If there's something to be saved, save it.
+  if (result == ERROR_SUCCESS) {
+    previous_value_.resize(size);
+    result = key.ReadValue(value_name_.c_str(), &previous_value_[0], &size,
+                           &previous_type_);
+    if (result != ERROR_SUCCESS) {
+      previous_value_.clear();
+      VLOG(1) << "Failed to save original value. Error: " << result;
+    }
+  }
 
-  if (ignore_failure_) {
-    success = true;
+  result = key.WriteValue(value_name_.c_str(), &value_[0],
+                          static_cast<DWORD>(value_.size()), type_);
+  if (result != ERROR_SUCCESS) {
+    VLOG(1) << "Failed to write value " << key_path_ << " error: " << result;
+    return ignore_failure_;
   }
 
-  return success;
+  status_ = previous_type_ ? VALUE_OVERWRITTEN : NEW_VALUE_CREATED;
+  return true;
 }
 
 void SetRegValueWorkItem::Rollback() {
-  if (!ignore_failure_) {
-    if (status_ == SET_VALUE || status_ == VALUE_ROLL_BACK)
-      return;
-
-    if (status_ == VALUE_UNCHANGED) {
-      status_ = VALUE_ROLL_BACK;
-      VLOG(1) << "rollback: setting unchanged, nothing to do";
-      return;
-    }
+  if (ignore_failure_)
+    return;
 
-    base::win::RegKey key;
-    if (!key.Open(predefined_root_, key_path_.c_str(), KEY_SET_VALUE)) {
-      status_ = VALUE_ROLL_BACK;
-      VLOG(1) << "rollback: can not open " << key_path_;
-      return;
-    }
+  if (status_ == SET_VALUE || status_ == VALUE_ROLL_BACK)
+    return;
 
-    std::wstring result_str(L" failed");
-    if (status_ == NEW_VALUE_CREATED) {
-      if (key.DeleteValue(value_name_.c_str()))
-        result_str.assign(L" succeeded");
-      VLOG(1) << "rollback: deleting " << value_name_ << result_str;
-    } else if (status_ == VALUE_OVERWRITTEN) {
-      // try restore the previous value
-      bool success = true;
-      if (type_ == REG_SZ) {
-        success = key.WriteValue(value_name_.c_str(),
-                                 previous_value_str_.c_str());
-      } else if (type_ == REG_DWORD) {
-        success = key.WriteValue(value_name_.c_str(), previous_value_dword_);
-      } else if (type_ == REG_QWORD) {
-        success = key.WriteValue(value_name_.c_str(),
-                                 &previous_value_qword_,
-                                 sizeof(previous_value_qword_),
-                                 REG_QWORD);
-      } else {
-        NOTREACHED();
-      }
-      if (success)
-        result_str.assign(L" succeeded");
-      VLOG(1) << "rollback: restoring " << value_name_ << result_str;
-
-      status_ = VALUE_ROLL_BACK;
-      return;
-    }
+  if (status_ == VALUE_UNCHANGED) {
+    status_ = VALUE_ROLL_BACK;
+    VLOG(1) << "rollback: setting unchanged, nothing to do";
+    return;
+  }
+
+  base::win::RegKey key;
+  LONG result = key.Open(predefined_root_, key_path_.c_str(), KEY_SET_VALUE);
+  if (result != ERROR_SUCCESS) {
+    VLOG(1) << "rollback: can not open " << key_path_ << " error: " << result;
+    return;
   }
+
+  if (status_ == NEW_VALUE_CREATED) {
+    result = key.DeleteValue(value_name_.c_str());
+    VLOG(1) << "rollback: deleting " << value_name_ << " error: " << result;
+  } else if (status_ == VALUE_OVERWRITTEN) {
+    result = key.WriteValue(value_name_.c_str(), &previous_value_[0],
+                            static_cast<DWORD>(previous_value_.size()),
+                            previous_type_);
+    VLOG(1) << "rollback: restoring " << value_name_ << " error: " << result;
+  } else {
+    NOTREACHED();
+  }
+
+  status_ = VALUE_ROLL_BACK;
 }
diff --git a/chrome/installer/util/set_reg_value_work_item.h b/chrome/installer/util/set_reg_value_work_item.h
index 1b51c5d..ec4ae1a 100644
--- a/chrome/installer/util/set_reg_value_work_item.h
+++ b/chrome/installer/util/set_reg_value_work_item.h
@@ -9,6 +9,7 @@
 #include <windows.h>
 
 #include <string>
+#include <vector>
 
 #include "chrome/installer/util/work_item.h"
 
@@ -66,23 +67,16 @@ class SetRegValueWorkItem : public WorkItem {
   // Name of the value to be set.
   std::wstring value_name_;
 
-  // Data of the value to be set.
-  std::wstring value_data_str_;  // if data is of type REG_SZ
-  DWORD value_data_dword_;  // if data is of type REG_DWORD
-  int64 value_data_qword_;  // if data is of type REG_QWORD
-
   // Whether to overwrite the existing value under the target key.
   bool overwrite_;
 
   // Type of data to store
   DWORD type_;
+  std::vector<uint8> value_;
+  DWORD previous_type_;
+  std::vector<uint8> previous_value_;
 
   SettingStatus status_;
-
-  // Data of the previous value.
-  std::wstring previous_value_str_;  // if data is of type REG_SZ
-  DWORD previous_value_dword_;  // if data is of type REG_DWORD
-  int64 previous_value_qword_;  // if data is of type REG_QWORD
 };
 
 #endif  // CHROME_INSTALLER_UTIL_SET_REG_VALUE_WORK_ITEM_H__
diff --git a/chrome/installer/util/set_reg_value_work_item_unittest.cc b/chrome/installer/util/set_reg_value_work_item_unittest.cc
index 13faba5..892f28d 100644
--- a/chrome/installer/util/set_reg_value_work_item_unittest.cc
+++ b/chrome/installer/util/set_reg_value_work_item_unittest.cc
@@ -27,14 +27,15 @@ class SetRegValueWorkItemTest : public testing::Test {
     // Create a temporary key for testing
     RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS);
     key.DeleteKey(test_root);
-    ASSERT_FALSE(key.Open(HKEY_CURRENT_USER, test_root, KEY_READ));
-    ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, test_root, KEY_READ));
+    ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER, test_root, KEY_READ));
+    ASSERT_EQ(ERROR_SUCCESS,
+        key.Create(HKEY_CURRENT_USER, test_root, KEY_READ));
   }
   virtual void TearDown() {
     logging::CloseLogFile();
     // Clean up the temporary key
     RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS);
-    ASSERT_TRUE(key.DeleteKey(test_root));
+    ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(test_root));
   }
 };
 
@@ -46,7 +47,8 @@ TEST_F(SetRegValueWorkItemTest, WriteNewNonOverwrite) {
 
   std::wstring parent_key(test_root);
   file_util::AppendToPath(&parent_key, L"WriteNewNonOverwrite");
-  ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, parent_key.c_str(), KEY_READ));
+  ASSERT_EQ(ERROR_SUCCESS,
+      key.Create(HKEY_CURRENT_USER, parent_key.c_str(), KEY_READ));
 
   std::wstring name_str(L"name_str");
   std::wstring data_str(data_str_1);
@@ -64,8 +66,8 @@ TEST_F(SetRegValueWorkItemTest, WriteNewNonOverwrite) {
 
   std::wstring read_out;
   DWORD read_dword;
-  EXPECT_TRUE(key.ReadValue(name_str.c_str(), &read_out));
-  EXPECT_TRUE(key.ReadValueDW(name_dword.c_str(), &read_dword));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name_str.c_str(), &read_out));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name_dword.c_str(), &read_dword));
   EXPECT_EQ(read_out, data_str_1);
   EXPECT_EQ(read_dword, dword1);
 
@@ -83,7 +85,8 @@ TEST_F(SetRegValueWorkItemTest, WriteNewOverwrite) {
 
   std::wstring parent_key(test_root);
   file_util::AppendToPath(&parent_key, L"WriteNewOverwrite");
-  ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, parent_key.c_str(), KEY_READ));
+  ASSERT_EQ(ERROR_SUCCESS,
+      key.Create(HKEY_CURRENT_USER, parent_key.c_str(), KEY_READ));
 
   std::wstring name_str(L"name_str");
   std::wstring data_str(data_str_1);
@@ -101,8 +104,8 @@ TEST_F(SetRegValueWorkItemTest, WriteNewOverwrite) {
 
   std::wstring read_out;
   DWORD read_dword;
-  EXPECT_TRUE(key.ReadValue(name_str.c_str(), &read_out));
-  EXPECT_TRUE(key.ReadValueDW(name_dword.c_str(), &read_dword));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name_str.c_str(), &read_out));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name_dword.c_str(), &read_dword));
   EXPECT_EQ(read_out, data_str_1);
   EXPECT_EQ(read_dword, dword1);
 
@@ -121,13 +124,14 @@ TEST_F(SetRegValueWorkItemTest, WriteExistingNonOverwrite) {
 
   std::wstring parent_key(test_root);
   file_util::AppendToPath(&parent_key, L"WriteExistingNonOverwrite");
-  ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, parent_key.c_str(),
-                         KEY_READ | KEY_SET_VALUE));
+  ASSERT_EQ(ERROR_SUCCESS,
+      key.Create(HKEY_CURRENT_USER, parent_key.c_str(),
+                 KEY_READ | KEY_SET_VALUE));
 
   // First test REG_SZ value.
   // Write data to the value we are going to set.
   std::wstring name(L"name_str");
-  ASSERT_TRUE(key.WriteValue(name.c_str(), data_str_1));
+  ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(name.c_str(), data_str_1));
 
   std::wstring data(data_str_2);
   scoped_ptr<SetRegValueWorkItem> work_item(
@@ -136,29 +140,29 @@ TEST_F(SetRegValueWorkItemTest, WriteExistingNonOverwrite) {
   EXPECT_TRUE(work_item->Do());
 
   std::wstring read_out;
-  EXPECT_TRUE(key.ReadValue(name.c_str(), &read_out));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
   EXPECT_EQ(0, read_out.compare(data_str_1));
 
   work_item->Rollback();
   EXPECT_TRUE(key.ValueExists(name.c_str()));
-  EXPECT_TRUE(key.ReadValue(name.c_str(), &read_out));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
   EXPECT_EQ(read_out, data_str_1);
 
   // Now test REG_DWORD value.
   // Write data to the value we are going to set.
   name.assign(L"name_dword");
-  ASSERT_TRUE(key.WriteValue(name.c_str(), dword1));
+  ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(name.c_str(), dword1));
   work_item.reset(WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
       parent_key, name, dword2, false));
   EXPECT_TRUE(work_item->Do());
 
   DWORD read_dword;
-  EXPECT_TRUE(key.ReadValueDW(name.c_str(), &read_dword));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name.c_str(), &read_dword));
   EXPECT_EQ(read_dword, dword1);
 
   work_item->Rollback();
   EXPECT_TRUE(key.ValueExists(name.c_str()));
-  EXPECT_TRUE(key.ReadValueDW(name.c_str(), &read_dword));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name.c_str(), &read_dword));
   EXPECT_EQ(read_dword, dword1);
 }
 
@@ -169,13 +173,14 @@ TEST_F(SetRegValueWorkItemTest, WriteExistingOverwrite) {
 
   std::wstring parent_key(test_root);
   file_util::AppendToPath(&parent_key, L"WriteExistingOverwrite");
-  ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, parent_key.c_str(),
-                         KEY_READ | KEY_SET_VALUE));
+  ASSERT_EQ(ERROR_SUCCESS,
+      key.Create(HKEY_CURRENT_USER, parent_key.c_str(),
+                 KEY_READ | KEY_SET_VALUE));
 
   // First test REG_SZ value.
   // Write data to the value we are going to set.
   std::wstring name(L"name_str");
-  ASSERT_TRUE(key.WriteValue(name.c_str(), data_str_1));
+  ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(name.c_str(), data_str_1));
 
   std::wstring data(data_str_2);
   scoped_ptr<SetRegValueWorkItem> work_item(
@@ -184,29 +189,29 @@ TEST_F(SetRegValueWorkItemTest, WriteExistingOverwrite) {
   EXPECT_TRUE(work_item->Do());
 
   std::wstring read_out;
-  EXPECT_TRUE(key.ReadValue(name.c_str(), &read_out));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
   EXPECT_EQ(0, read_out.compare(data_str_2));
 
   work_item->Rollback();
   EXPECT_TRUE(key.ValueExists(name.c_str()));
-  EXPECT_TRUE(key.ReadValue(name.c_str(), &read_out));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
   EXPECT_EQ(read_out, data_str_1);
 
   // Now test REG_DWORD value.
   // Write data to the value we are going to set.
   name.assign(L"name_dword");
-  ASSERT_TRUE(key.WriteValue(name.c_str(), dword1));
+  ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(name.c_str(), dword1));
   work_item.reset(WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER,
       parent_key, name, dword2, true));
   EXPECT_TRUE(work_item->Do());
 
   DWORD read_dword;
-  EXPECT_TRUE(key.ReadValueDW(name.c_str(), &read_dword));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name.c_str(), &read_dword));
   EXPECT_EQ(read_dword, dword2);
 
   work_item->Rollback();
   EXPECT_TRUE(key.ValueExists(name.c_str()));
-  EXPECT_TRUE(key.ReadValueDW(name.c_str(), &read_dword));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name.c_str(), &read_dword));
   EXPECT_EQ(read_dword, dword1);
 }
 
diff --git a/chrome/installer/util/shell_util.cc b/chrome/installer/util/shell_util.cc
index 8d4e643..d143ffb 100644
--- a/chrome/installer/util/shell_util.cc
+++ b/chrome/installer/util/shell_util.cc
@@ -200,14 +200,14 @@ class RegistryEntry {
     bool found = false;
     if (_is_string) {
       std::wstring read_value;
-      found = (key.ReadValue(_name.c_str(), &read_value)) &&
+      found = (key.ReadValue(_name.c_str(), &read_value) == ERROR_SUCCESS) &&
               (read_value.size() == _value.size()) &&
               (std::equal(_value.begin(), _value.end(), read_value.begin(),
                           base::CaseInsensitiveCompare<wchar_t>()));
     } else {
       DWORD read_value;
-      found = key.ReadValueDW(_name.c_str(), &read_value) &&
-              read_value == _int_value;
+      found = (key.ReadValueDW(_name.c_str(), &read_value) == ERROR_SUCCESS) &&
+              (read_value == _int_value);
     }
     key.Close();
     return found;
@@ -220,10 +220,10 @@ class RegistryEntry {
     bool found = false;
     if (_is_string) {
       std::wstring read_value;
-      found = key.ReadValue(_name.c_str(), &read_value);
+      found = key.ReadValue(_name.c_str(), &read_value) == ERROR_SUCCESS;
     } else {
       DWORD read_value;
-      found = key.ReadValueDW(_name.c_str(), &read_value);
+      found = key.ReadValueDW(_name.c_str(), &read_value) == ERROR_SUCCESS;
     }
     key.Close();
     return found;
@@ -356,7 +356,7 @@ bool AnotherUserHasDefaultBrowser(BrowserDistribution* dist,
   reg_key.append(L"\\" + dist->GetApplicationName() + ShellUtil::kRegShellOpen);
   RegKey key(HKEY_LOCAL_MACHINE, reg_key.c_str(), KEY_READ);
   std::wstring registry_chrome_exe;
-  if (!key.ReadValue(L"", &registry_chrome_exe) ||
+  if ((key.ReadValue(L"", &registry_chrome_exe) != ERROR_SUCCESS) ||
       registry_chrome_exe.length() < 2)
     return false;
 
@@ -583,15 +583,16 @@ void ShellUtil::GetRegisteredBrowsers(BrowserDistribution* dist,
     RegKey capabilities(root, (key + L"\\Capabilities").c_str(), KEY_READ);
     std::wstring name;
     if (!capabilities.Valid() ||
-        !capabilities.ReadValue(L"ApplicationName", &name)) {
+        capabilities.ReadValue(L"ApplicationName", &name) != ERROR_SUCCESS) {
       RegKey base_key(root, key.c_str(), KEY_READ);
-      if (!base_key.ReadValue(L"", &name))
+      if (base_key.ReadValue(L"", &name) != ERROR_SUCCESS)
         continue;
     }
     RegKey install_info(root, (key + L"\\InstallInfo").c_str(), KEY_READ);
     std::wstring command;
     if (!install_info.Valid() ||
-        !install_info.ReadValue(L"ReinstallCommand", &command))
+        (install_info.ReadValue(L"ReinstallCommand",
+                                &command) != ERROR_SUCCESS))
       continue;
     if (!name.empty() && !command.empty() &&
         name.find(dist->GetApplicationName()) == std::wstring::npos)
diff --git a/chrome/installer/util/work_item.cc b/chrome/installer/util/work_item.cc
index 5ed352b..546b9ebc 100644
--- a/chrome/installer/util/work_item.cc
+++ b/chrome/installer/util/work_item.cc
@@ -49,10 +49,8 @@ DeleteRegKeyWorkItem* WorkItem::CreateDeleteRegKeyWorkItem(
 DeleteRegValueWorkItem* WorkItem::CreateDeleteRegValueWorkItem(
     HKEY predefined_root,
     const std::wstring& key_path,
-    const std::wstring& value_name,
-    DWORD type) {
-  return new DeleteRegValueWorkItem(predefined_root, key_path,
-                                    value_name, type);
+    const std::wstring& value_name) {
+  return new DeleteRegValueWorkItem(predefined_root, key_path, value_name);
 }
 
 DeleteTreeWorkItem* WorkItem::CreateDeleteTreeWorkItem(
diff --git a/chrome/installer/util/work_item.h b/chrome/installer/util/work_item.h
index 12446be..7fd5db2 100644
--- a/chrome/installer/util/work_item.h
+++ b/chrome/installer/util/work_item.h
@@ -82,8 +82,7 @@ class WorkItem {
   static DeleteRegValueWorkItem* CreateDeleteRegValueWorkItem(
       HKEY predefined_root,
       const std::wstring& key_path,
-      const std::wstring& value_name,
-      DWORD type);
+      const std::wstring& value_name);
 
   // Create a DeleteTreeWorkItem that recursively deletes a file system
   // hierarchy at the given root path. A key file can be optionally specified
diff --git a/chrome/installer/util/work_item_list.cc b/chrome/installer/util/work_item_list.cc
index 41ee169..28e2829 100644
--- a/chrome/installer/util/work_item_list.cc
+++ b/chrome/installer/util/work_item_list.cc
@@ -110,12 +110,9 @@ WorkItem* WorkItemList::AddDeleteRegKeyWorkItem(HKEY predefined_root,
 WorkItem* WorkItemList::AddDeleteRegValueWorkItem(
     HKEY predefined_root,
     const std::wstring& key_path,
-    const std::wstring& value_name,
-    DWORD type) {
+    const std::wstring& value_name) {
   WorkItem* item = WorkItem::CreateDeleteRegValueWorkItem(predefined_root,
-                                                          key_path,
-                                                          value_name,
-                                                          type);
+                                                          key_path, value_name);
   AddWorkItem(item);
   return item;
 }
diff --git a/chrome/installer/util/work_item_list.h b/chrome/installer/util/work_item_list.h
index 5792d0f..23af15b 100644
--- a/chrome/installer/util/work_item_list.h
+++ b/chrome/installer/util/work_item_list.h
@@ -61,9 +61,8 @@ class WorkItemList : public WorkItem {
   // Add a DeleteRegValueWorkItem that deletes registry value of type REG_SZ
   // or REG_DWORD.
   virtual WorkItem* AddDeleteRegValueWorkItem(HKEY predefined_root,
-                                      const std::wstring& key_path,
-                                      const std::wstring& value_name,
-                                      DWORD type);
+                                              const std::wstring& key_path,
+                                              const std::wstring& value_name);
 
   // Add a DeleteTreeWorkItem that recursively deletes a file system
   // hierarchy at the given root path. A key file can be optionally specified
diff --git a/chrome/installer/util/work_item_list_unittest.cc b/chrome/installer/util/work_item_list_unittest.cc
index 97a6927..bd8f515 100644
--- a/chrome/installer/util/work_item_list_unittest.cc
+++ b/chrome/installer/util/work_item_list_unittest.cc
@@ -28,8 +28,9 @@ class WorkItemListTest : public testing::Test {
     // Create a temporary key for testing
     RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS);
     key.DeleteKey(test_root);
-    ASSERT_FALSE(key.Open(HKEY_CURRENT_USER, test_root, KEY_READ));
-    ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, test_root, KEY_READ));
+    ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER, test_root, KEY_READ));
+    ASSERT_EQ(ERROR_SUCCESS,
+        key.Create(HKEY_CURRENT_USER, test_root, KEY_READ));
 
     // Create a temp directory for test.
     ASSERT_TRUE(PathService::Get(base::DIR_TEMP, &test_dir_));
@@ -47,7 +48,7 @@ class WorkItemListTest : public testing::Test {
     ASSERT_FALSE(file_util::PathExists(test_dir_));
     // Clean up the temporary key
     RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS);
-    ASSERT_TRUE(key.DeleteKey(test_root));
+    ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(test_root));
   }
 
   FilePath test_dir_;
@@ -88,9 +89,10 @@ TEST_F(WorkItemListTest, ExecutionSuccess) {
 
   // Verify all WorkItems have been executed.
   RegKey key;
-  EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
   std::wstring read_out;
-  EXPECT_TRUE(key.ReadValue(name.c_str(), &read_out));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
   EXPECT_EQ(0, read_out.compare(data_str));
   key.Close();
   EXPECT_TRUE(file_util::PathExists(dir_to_create));
@@ -100,7 +102,8 @@ TEST_F(WorkItemListTest, ExecutionSuccess) {
   // Verify everything is rolled back.
   // The value must have been deleted first in roll back otherwise the key
   // can not be deleted.
-  EXPECT_FALSE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+  EXPECT_NE(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
   EXPECT_FALSE(file_util::PathExists(top_dir_to_create));
 }
 
@@ -145,17 +148,19 @@ TEST_F(WorkItemListTest, ExecutionFailAndRollback) {
 
   // Verify the first 2 WorkItems have been executed.
   RegKey key;
-  EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
   key.Close();
   EXPECT_TRUE(file_util::PathExists(dir_to_create));
   // The last one should not be there.
-  EXPECT_FALSE(key.Open(HKEY_CURRENT_USER, not_created_key.c_str(),
-               KEY_READ));
+  EXPECT_NE(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, not_created_key.c_str(), KEY_READ));
 
   work_item_list->Rollback();
 
   // Verify everything is rolled back.
-  EXPECT_FALSE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+  EXPECT_NE(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
   EXPECT_FALSE(file_util::PathExists(top_dir_to_create));
 }
 
@@ -196,9 +201,10 @@ TEST_F(WorkItemListTest, ConditionalExecutionSuccess) {
 
   // Verify all WorkItems have been executed.
   RegKey key;
-  EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+  EXPECT_EQ(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
   std::wstring read_out;
-  EXPECT_TRUE(key.ReadValue(name.c_str(), &read_out));
+  EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
   EXPECT_EQ(0, read_out.compare(data_str));
   key.Close();
   EXPECT_TRUE(file_util::PathExists(dir_to_create));
@@ -208,7 +214,8 @@ TEST_F(WorkItemListTest, ConditionalExecutionSuccess) {
   // Verify everything is rolled back.
   // The value must have been deleted first in roll back otherwise the key
   // can not be deleted.
-  EXPECT_FALSE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+  EXPECT_NE(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
   EXPECT_FALSE(file_util::PathExists(top_dir_to_create));
 }
 
@@ -250,9 +257,10 @@ TEST_F(WorkItemListTest, ConditionalExecutionConditionFailure) {
   // Verify that the WorkItems added as part of the conditional list have NOT
   // been executed.
   RegKey key;
-  EXPECT_FALSE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+  EXPECT_NE(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
   std::wstring read_out;
-  EXPECT_FALSE(key.ReadValue(name.c_str(), &read_out));
+  EXPECT_NE(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
   key.Close();
 
   // Verify that the other work item was executed.
@@ -263,7 +271,8 @@ TEST_F(WorkItemListTest, ConditionalExecutionConditionFailure) {
   // Verify everything is rolled back.
   // The value must have been deleted first in roll back otherwise the key
   // can not be deleted.
-  EXPECT_FALSE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+  EXPECT_NE(ERROR_SUCCESS,
+      key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
   EXPECT_FALSE(file_util::PathExists(top_dir_to_create));
 }