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

4 files changed, 214 insertions, 148 deletions

diff --git a/base/win/registry.cc b/base/win/registry.cc
index 3c14a6c..cdcdc5c 100644
--- a/base/win/registry.cc
+++ b/base/win/registry.cc
@@ -39,48 +39,33 @@ RegKey::~RegKey() {
   Close();
 }
 
-bool RegKey::Create(HKEY rootkey, const wchar_t* subkey, REGSAM access) {
+GONG RegKey::Create(HKEY rootkey, const wchar_t* subkey, REGSAM access) {
   DWORD disposition_value;
   return CreateWithDisposition(rootkey, subkey, &disposition_value, access);
 }
 
-bool RegKey::CreateWithDisposition(HKEY rootkey, const wchar_t* subkey,
+GONG RegKey::CreateWithDisposition(HKEY rootkey, const wchar_t* subkey,
                                    DWORD* disposition, REGSAM access) {
   base::ThreadRestrictions::AssertIOAllowed();
   DCHECK(rootkey && subkey && access && disposition);
   Close();
 
-  LONG result = RegCreateKeyEx(rootkey,
-                               subkey,
-                               0,
-                               NULL,
-                               REG_OPTION_NON_VOLATILE,
-                               access,
-                               NULL,
-                               &key_,
+  LONG result = RegCreateKeyEx(rootkey, subkey, 0, NULL,
+                               REG_OPTION_NON_VOLATILE, access, NULL, &key_,
                                disposition);
-  if (result != ERROR_SUCCESS) {
-    key_ = NULL;
-    return false;
-  }
-
-  return true;
+  return result;
 }
 
-bool RegKey::Open(HKEY rootkey, const wchar_t* subkey, REGSAM access) {
+GONG RegKey::Open(HKEY rootkey, const wchar_t* subkey, REGSAM access) {
   base::ThreadRestrictions::AssertIOAllowed();
   DCHECK(rootkey && subkey && access);
   Close();
 
   LONG result = RegOpenKeyEx(rootkey, subkey, 0, access, &key_);
-  if (result != ERROR_SUCCESS) {
-    key_ = NULL;
-    return false;
-  }
-  return true;
+  return result;
 }
 
-bool RegKey::CreateKey(const wchar_t* name, REGSAM access) {
+GONG RegKey::CreateKey(const wchar_t* name, REGSAM access) {
   base::ThreadRestrictions::AssertIOAllowed();
   DCHECK(name && access);
 
@@ -90,10 +75,10 @@ bool RegKey::CreateKey(const wchar_t* name, REGSAM access) {
   Close();
 
   key_ = subkey;
-  return (result == ERROR_SUCCESS);
+  return result;
 }
 
-bool RegKey::OpenKey(const wchar_t* name, REGSAM access) {
+GONG RegKey::OpenKey(const wchar_t* name, REGSAM access) {
   base::ThreadRestrictions::AssertIOAllowed();
   DCHECK(name && access);
 
@@ -103,7 +88,7 @@ bool RegKey::OpenKey(const wchar_t* name, REGSAM access) {
   Close();
 
   key_ = subkey;
-  return (result == ERROR_SUCCESS);
+  return result;
 }
 
 void RegKey::Close() {
@@ -118,67 +103,61 @@ void RegKey::Close() {
 DWORD RegKey::ValueCount() const {
   base::ThreadRestrictions::AssertIOAllowed();
   DWORD count = 0;
-  HRESULT result = RegQueryInfoKey(key_, NULL, 0, NULL, NULL, NULL,
-                                   NULL, &count, NULL, NULL, NULL, NULL);
+  LONG result = RegQueryInfoKey(key_, NULL, 0, NULL, NULL, NULL, NULL, &count,
+                                NULL, NULL, NULL, NULL);
   return (result != ERROR_SUCCESS) ? 0 : count;
 }
 
-bool RegKey::ReadName(int index, std::wstring* name) const {
+GONG RegKey::ReadName(int index, std::wstring* name) const {
   base::ThreadRestrictions::AssertIOAllowed();
   wchar_t buf[256];
   DWORD bufsize = arraysize(buf);
-  LRESULT r = ::RegEnumValue(key_, index, buf, &bufsize, NULL, NULL,
-                             NULL, NULL);
-  if (r != ERROR_SUCCESS)
-    return false;
-  if (name)
+  LONG r = ::RegEnumValue(key_, index, buf, &bufsize, NULL, NULL, NULL, NULL);
+  if (r == ERROR_SUCCESS)
     *name = buf;
-  return true;
+
+  return r;
 }
 
-bool RegKey::DeleteKey(const wchar_t* name) {
+GONG RegKey::DeleteKey(const wchar_t* name) {
   base::ThreadRestrictions::AssertIOAllowed();
-  if (!key_)
-    return false;
-  LSTATUS ret = SHDeleteKey(key_, name);
-  if (ERROR_SUCCESS != ret)
-    SetLastError(ret);
-  return ERROR_SUCCESS == ret;
+  DCHECK(key_);
+  DCHECK(name);
+  LONG result = SHDeleteKey(key_, name);
+  return result;
 }
 
-bool RegKey::DeleteValue(const wchar_t* value_name) {
+GONG RegKey::DeleteValue(const wchar_t* value_name) {
   base::ThreadRestrictions::AssertIOAllowed();
+  DCHECK(key_);
   DCHECK(value_name);
-  HRESULT result = RegDeleteValue(key_, value_name);
-  return (result == ERROR_SUCCESS);
+  LONG result = RegDeleteValue(key_, value_name);
+  return result;
 }
 
-bool RegKey::ValueExists(const wchar_t* name) {
+bool RegKey::ValueExists(const wchar_t* name) const {
   base::ThreadRestrictions::AssertIOAllowed();
-  if (!key_)
-    return false;
-  HRESULT result = RegQueryValueEx(key_, name, 0, NULL, NULL, NULL);
-  return (result == ERROR_SUCCESS);
+  LONG result = RegQueryValueEx(key_, name, 0, NULL, NULL, NULL);
+  return result == ERROR_SUCCESS;
 }
 
-bool RegKey::ReadValue(const wchar_t* name, void* data,
-                       DWORD* dsize, DWORD* dtype) const {
+GONG RegKey::ReadValue(const wchar_t* name, void* data, DWORD* dsize,
+                       DWORD* dtype) const {
   base::ThreadRestrictions::AssertIOAllowed();
-  if (!key_)
-    return false;
-  HRESULT result = RegQueryValueEx(key_, name, 0, dtype,
-                                   reinterpret_cast<LPBYTE>(data), dsize);
-  return (result == ERROR_SUCCESS);
+  LONG result = RegQueryValueEx(key_, name, 0, dtype,
+                                reinterpret_cast<LPBYTE>(data), dsize);
+  return result;
 }
 
-bool RegKey::ReadValue(const wchar_t* name, std::wstring* value) const {
+GONG RegKey::ReadValue(const wchar_t* name, std::wstring* value) const {
   base::ThreadRestrictions::AssertIOAllowed();
   DCHECK(value);
   const size_t kMaxStringLength = 1024;  // This is after expansion.
   // Use the one of the other forms of ReadValue if 1024 is too small for you.
   wchar_t raw_value[kMaxStringLength];
   DWORD type = REG_SZ, size = sizeof(raw_value);
-  if (ReadValue(name, raw_value, &size, &type)) {
+  LONG result = ReadValue(name, raw_value, &size, &type);
+  if (result == ERROR_SUCCESS) {
     if (type == REG_SZ) {
       *value = raw_value;
     } else if (type == REG_EXPAND_SZ) {
@@ -187,63 +166,77 @@ bool RegKey::ReadValue(const wchar_t* name, std::wstring* value) const {
       // Success: returns the number of wchar_t's copied
       // Fail: buffer too small, returns the size required
       // Fail: other, returns 0
-      if (size == 0 || size > kMaxStringLength)
-        return false;
-      *value = expanded;
+      if (size == 0 || size > kMaxStringLength) {
+        result = ERROR_MORE_DATA;
+      } else {
+        *value = expanded;
+      }
     } else {
       // Not a string. Oops.
-      return false;
+      result = ERROR_CANTREAD;
     }
-    return true;
   }
 
-  return false;
+  return result;
 }
 
-bool RegKey::ReadValueDW(const wchar_t* name, DWORD* value) const {
+GONG RegKey::ReadValueDW(const wchar_t* name, DWORD* value) const {
   DCHECK(value);
   DWORD type = REG_DWORD;
   DWORD size = sizeof(DWORD);
-  DWORD result = 0;
-  if (ReadValue(name, &result, &size, &type) &&
-      (type == REG_DWORD || type == REG_BINARY) &&
-      size == sizeof(DWORD)) {
-    *value = result;
-    return true;
+  DWORD local_value = 0;
+  LONG result = ReadValue(name, &local_value, &size, &type);
+  if (result == ERROR_SUCCESS) {
+    if ((type == REG_DWORD || type == REG_BINARY) && size == sizeof(DWORD)) {
+      *value = local_value;
+    } else {
+      result = ERROR_CANTREAD;
+    }
   }
 
-  return false;
+  return result;
+}
+
+GONG RegKey::ReadInt64(const wchar_t* name, int64* value) const {
+  DCHECK(value);
+  DWORD type = REG_QWORD;
+  int64 local_value = 0;
+  DWORD size = sizeof(local_value);
+  LONG result = ReadValue(name, &local_value, &size, &type);
+  if (result == ERROR_SUCCESS) {
+    if ((type == REG_QWORD || type == REG_BINARY) &&
+        size == sizeof(local_value)) {
+      *value = local_value;
+    } else {
+      result = ERROR_CANTREAD;
+    }
+  }
+
+  return result;
 }
 
-bool RegKey::WriteValue(const wchar_t* name, const void * data,
+GONG RegKey::WriteValue(const wchar_t* name, const void * data,
                         DWORD dsize, DWORD dtype) {
   base::ThreadRestrictions::AssertIOAllowed();
   DCHECK(data);
+  DCHECK(key_);
 
-  if (!key_)
-    return false;
-
-  HRESULT result = RegSetValueEx(
-      key_,
-      name,
-      0,
-      dtype,
-      reinterpret_cast<LPBYTE>(const_cast<void*>(data)),
-      dsize);
-  return (result == ERROR_SUCCESS);
+  LONG result = RegSetValueEx(key_, name, 0, dtype,
+      reinterpret_cast<LPBYTE>(const_cast<void*>(data)), dsize);
+  return result;
 }
 
-bool RegKey::WriteValue(const wchar_t * name, const wchar_t* value) {
+GONG RegKey::WriteValue(const wchar_t * name, const wchar_t* value) {
   return WriteValue(name, value,
       static_cast<DWORD>(sizeof(*value) * (wcslen(value) + 1)), REG_SZ);
 }
 
-bool RegKey::WriteValue(const wchar_t* name, DWORD value) {
-  return WriteValue(name, &value,
-      static_cast<DWORD>(sizeof(value)), REG_DWORD);
+GONG RegKey::WriteValue(const wchar_t* name, DWORD value) {
+  return WriteValue(name, &value, static_cast<DWORD>(sizeof(value)), REG_DWORD);
 }
 
-bool RegKey::StartWatching() {
+GONG RegKey::StartWatching() {
+  DCHECK(key_);
   if (!watch_event_)
     watch_event_ = CreateEvent(NULL, TRUE, FALSE, NULL);
 
@@ -253,15 +246,13 @@ bool RegKey::StartWatching() {
                  REG_NOTIFY_CHANGE_SECURITY;
 
   // Watch the registry key for a change of value.
-  HRESULT result = RegNotifyChangeKeyValue(key_, TRUE, filter,
-                                           watch_event_, TRUE);
-  if (SUCCEEDED(result)) {
-    return true;
-  } else {
+  LONG result = RegNotifyChangeKeyValue(key_, TRUE, filter, watch_event_, TRUE);
+  if (result != ERROR_SUCCESS) {
     CloseHandle(watch_event_);
     watch_event_ = 0;
-    return false;
   }
+
+  return result;
 }
 
 bool RegKey::HasChanged() {
@@ -274,13 +265,14 @@ bool RegKey::HasChanged() {
   return false;
 }
 
-bool RegKey::StopWatching() {
+GONG RegKey::StopWatching() {
+  LONG result = ERROR_INVALID_HANDLE;
   if (watch_event_) {
     CloseHandle(watch_event_);
     watch_event_ = 0;
-    return true;
+    result = ERROR_SUCCESS;
   }
-  return false;
+  return result;
 }
 
 // RegistryValueIterator ------------------------------------------------------
@@ -317,9 +309,8 @@ RegistryValueIterator::~RegistryValueIterator() {
 DWORD RegistryValueIterator::ValueCount() const {
   base::ThreadRestrictions::AssertIOAllowed();
   DWORD count = 0;
-  HRESULT result = ::RegQueryInfoKey(key_, NULL, 0, NULL, NULL, NULL, NULL,
-                                     &count, NULL, NULL, NULL, NULL);
-
+  LONG result = ::RegQueryInfoKey(key_, NULL, 0, NULL, NULL, NULL, NULL,
+                                  &count, NULL, NULL, NULL, NULL);
   if (result != ERROR_SUCCESS)
     return 0;
 
@@ -340,8 +331,8 @@ bool RegistryValueIterator::Read() {
   if (Valid()) {
     DWORD ncount = arraysize(name_);
     value_size_ = sizeof(value_);
-    LRESULT r = ::RegEnumValue(key_, index_, name_, &ncount, NULL, &type_,
-                               reinterpret_cast<BYTE*>(value_), &value_size_);
+    LONG r = ::RegEnumValue(key_, index_, name_, &ncount, NULL, &type_,
+                            reinterpret_cast<BYTE*>(value_), &value_size_);
     if (ERROR_SUCCESS == r)
       return true;
   }
@@ -362,8 +353,8 @@ RegistryKeyIterator::RegistryKeyIterator(HKEY root_key,
     key_ = NULL;
   } else {
     DWORD count = 0;
-    HRESULT result = ::RegQueryInfoKey(key_, NULL, 0, NULL, &count, NULL, NULL,
-                                       NULL, NULL, NULL, NULL, NULL);
+    LONG result = ::RegQueryInfoKey(key_, NULL, 0, NULL, &count, NULL, NULL,
+                                    NULL, NULL, NULL, NULL, NULL);
 
     if (result != ERROR_SUCCESS) {
       ::RegCloseKey(key_);
@@ -385,9 +376,8 @@ RegistryKeyIterator::~RegistryKeyIterator() {
 DWORD RegistryKeyIterator::SubkeyCount() const {
   base::ThreadRestrictions::AssertIOAllowed();
   DWORD count = 0;
-  HRESULT result = ::RegQueryInfoKey(key_, NULL, 0, NULL, &count, NULL, NULL,
-                                     NULL, NULL, NULL, NULL, NULL);
-
+  LONG result = ::RegQueryInfoKey(key_, NULL, 0, NULL, &count, NULL, NULL,
+                                  NULL, NULL, NULL, NULL, NULL);
   if (result != ERROR_SUCCESS)
     return 0;
 
@@ -408,8 +398,8 @@ bool RegistryKeyIterator::Read() {
   if (Valid()) {
     DWORD ncount = arraysize(name_);
     FILETIME written;
-    LRESULT r = ::RegEnumKeyEx(key_, index_, name_, &ncount, NULL, NULL,
-                               NULL, &written);
+    LONG r = ::RegEnumKeyEx(key_, index_, name_, &ncount, NULL, NULL,
+                            NULL, &written);
     if (ERROR_SUCCESS == r)
       return true;
   }
diff --git a/base/win/registry.h b/base/win/registry.h
index 2f06641..790fe7d 100644
--- a/base/win/registry.h
+++ b/base/win/registry.h
@@ -11,63 +11,101 @@
 
 #include "base/basictypes.h"
 
+// Please ignore this part. This temporary hack exists
+// to detect if the return value is used as 'bool'.
+// Todo(amit): remove this before (or soon after) checkin.
+struct CatchBoolChecks {
+  CatchBoolChecks(LONG l) : l_(l) {}
+  LONG l_;
+  operator LONG() { return l_; }
+  LONG value() const { return l_; }
+  bool operator == (LONG l) const { return l == l_; }
+  bool operator != (LONG l) const { return l != l_; }
+ private:
+  // If you hit a compile error here, you most likely attempting to use the
+  // return value of a RegKey helper as a bool. Please note that RegKey
+  // methods return LONG now instead of bool.
+  operator bool () { return false; }
+};
+
+inline bool operator == (const LONG& l, const CatchBoolChecks& g) {
+  return g.value() == l;
+}
+
+inline bool operator != (const LONG& l, const CatchBoolChecks& g) {
+  return g.value() != l;
+}
+
+using std::ostream;
+inline ostream& operator <<(ostream &os, const CatchBoolChecks& g) {
+  os << g.value();
+  return os;
+}
+
+typedef CatchBoolChecks GONG;
+
 namespace base {
 namespace win {
 
 // Utility class to read, write and manipulate the Windows Registry.
 // Registry vocabulary primer: a "key" is like a folder, in which there
 // are "values", which are <name, data> pairs, with an associated data type.
+//
+// Note:
+// ReadValue family of functions guarantee that the return arguments
+// are not touched in case of failure.
 class RegKey {
  public:
   RegKey();
   RegKey(HKEY rootkey, const wchar_t* subkey, REGSAM access);
   ~RegKey();
 
-  bool Create(HKEY rootkey, const wchar_t* subkey, REGSAM access);
+  GONG Create(HKEY rootkey, const wchar_t* subkey, REGSAM access);
 
-  bool CreateWithDisposition(HKEY rootkey, const wchar_t* subkey,
+  GONG CreateWithDisposition(HKEY rootkey, const wchar_t* subkey,
                              DWORD* disposition, REGSAM access);
 
-  bool Open(HKEY rootkey, const wchar_t* subkey, REGSAM access);
+  GONG Open(HKEY rootkey, const wchar_t* subkey, REGSAM access);
 
   // Creates a subkey or open it if it already exists.
-  bool CreateKey(const wchar_t* name, REGSAM access);
+  GONG CreateKey(const wchar_t* name, REGSAM access);
 
   // Opens a subkey
-  bool OpenKey(const wchar_t* name, REGSAM access);
+  GONG OpenKey(const wchar_t* name, REGSAM access);
 
   void Close();
 
   DWORD ValueCount() const;
 
   // Determine the nth value's name.
-  bool ReadName(int index, std::wstring* name) const;
+  GONG ReadName(int index, std::wstring* name) const;
 
   // True while the key is valid.
   bool Valid() const { return key_ != NULL; }
 
   // Kill a key and everything that live below it; please be careful when using
   // it.
-  bool DeleteKey(const wchar_t* name);
+  GONG DeleteKey(const wchar_t* name);
 
   // Deletes a single value within the key.
-  bool DeleteValue(const wchar_t* name);
+  GONG DeleteValue(const wchar_t* name);
 
-  bool ValueExists(const wchar_t* name);
+  bool ValueExists(const wchar_t* name) const;
 
-  bool ReadValue(const wchar_t* name, void* data, DWORD* dsize,
+  GONG ReadValue(const wchar_t* name, void* data, DWORD* dsize,
                  DWORD* dtype) const;
-  bool ReadValue(const wchar_t* name, std::wstring* value) const;
-  bool ReadValueDW(const wchar_t* name, DWORD* value) const;
+  GONG ReadValue(const wchar_t* name, std::wstring* value) const;
+  GONG ReadValueDW(const wchar_t* name, DWORD* value) const;
+  GONG ReadInt64(const wchar_t* name, int64* value) const;
 
-  bool WriteValue(const wchar_t* name, const void* data, DWORD dsize,
+  GONG WriteValue(const wchar_t* name, const void* data, DWORD dsize,
                   DWORD dtype);
-  bool WriteValue(const wchar_t* name, const wchar_t* value);
-  bool WriteValue(const wchar_t* name, DWORD value);
+  GONG WriteValue(const wchar_t* name, const wchar_t* value);
+  GONG WriteValue(const wchar_t* name, DWORD value);
 
   // Starts watching the key to see if any of its values have changed.
   // The key must have been opened with the KEY_NOTIFY access privilege.
-  bool StartWatching();
+  GONG StartWatching();
 
   // If StartWatching hasn't been called, always returns false.
   // Otherwise, returns true if anything under the key has changed.
@@ -76,7 +114,7 @@ class RegKey {
 
   // Will automatically be called by destructor if not manually called
   // beforehand.  Returns true if it was watching, false otherwise.
-  bool StopWatching();
+  GONG StopWatching();
 
   inline bool IsWatching() const { return watch_event_ != 0; }
   HANDLE watch_event() const { return watch_event_; }
diff --git a/base/win/registry_unittest.cc b/base/win/registry_unittest.cc
index 524612a..a7961e9 100644
--- a/base/win/registry_unittest.cc
+++ b/base/win/registry_unittest.cc
@@ -21,14 +21,14 @@ class RegistryTest : public testing::Test {
     // Create a temporary key.
     RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS);
     key.DeleteKey(kRootKey);
-    ASSERT_FALSE(key.Open(HKEY_CURRENT_USER, kRootKey, KEY_READ));
-    ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, kRootKey, KEY_READ));
+    ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER, kRootKey, KEY_READ));
+    ASSERT_EQ(ERROR_SUCCESS, key.Create(HKEY_CURRENT_USER, kRootKey, KEY_READ));
   }
 
   virtual void TearDown() {
     // Clean up the temporary key.
     RegKey key(HKEY_CURRENT_USER, L"", KEY_SET_VALUE);
-    ASSERT_TRUE(key.DeleteKey(kRootKey));
+    ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(kRootKey));
   }
 
  private:
@@ -40,22 +40,59 @@ TEST_F(RegistryTest, ValueTest) {
 
   std::wstring foo_key(kRootKey);
   foo_key += L"\\Foo";
-  ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, foo_key.c_str(), KEY_READ));
+  ASSERT_EQ(ERROR_SUCCESS, key.Create(HKEY_CURRENT_USER, foo_key.c_str(),
+                                      KEY_READ));
 
   {
-    ASSERT_TRUE(key.Open(HKEY_CURRENT_USER, foo_key.c_str(),
-                         KEY_READ | KEY_SET_VALUE));
-
-    const wchar_t* kName = L"Bar";
-    const wchar_t* kValue = L"bar";
-    EXPECT_TRUE(key.WriteValue(kName, kValue));
-    EXPECT_TRUE(key.ValueExists(kName));
-    std::wstring out_value;
-    EXPECT_TRUE(key.ReadValue(kName, &out_value));
-    EXPECT_NE(out_value, L"");
-    EXPECT_STREQ(out_value.c_str(), kValue);
-    EXPECT_EQ(1U, key.ValueCount());
-    EXPECT_TRUE(key.DeleteValue(kName));
+    ASSERT_EQ(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER, foo_key.c_str(),
+                                      KEY_READ | KEY_SET_VALUE));
+    ASSERT_TRUE(key.Valid());
+
+    const wchar_t* kStringValueName = L"StringValue";
+    const wchar_t* kDWORDValueName = L"DWORDValue";
+    const wchar_t* kInt64ValueName = L"Int64Value";
+    const wchar_t* kStringData = L"string data";
+    const DWORD kDWORDData = 0xdeadbabe;
+    const int64 kInt64Data = 0xdeadbabedeadbabeLL;
+
+    // Test value creation
+    ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(kStringValueName, kStringData));
+    ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(kDWORDValueName, kDWORDData));
+    ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(kInt64ValueName, &kInt64Data,
+                                            sizeof(kInt64Data), REG_QWORD));
+    EXPECT_EQ(3U, key.ValueCount());
+    EXPECT_TRUE(key.ValueExists(kStringValueName));
+    EXPECT_TRUE(key.ValueExists(kDWORDValueName));
+    EXPECT_TRUE(key.ValueExists(kInt64ValueName));
+
+    // Test Read
+    std::wstring string_value;
+    DWORD dword_value = 0;
+    int64 int64_value = 0;
+    ASSERT_EQ(ERROR_SUCCESS, key.ReadValue(kStringValueName, &string_value));
+    ASSERT_EQ(ERROR_SUCCESS, key.ReadValueDW(kDWORDValueName, &dword_value));
+    ASSERT_EQ(ERROR_SUCCESS, key.ReadInt64(kInt64ValueName, &int64_value));
+    EXPECT_STREQ(kStringData, string_value.c_str());
+    EXPECT_EQ(kDWORDData, dword_value);
+    EXPECT_EQ(kInt64Data, int64_value);
+
+    // Make sure out args are not touched if ReadValue fails
+    const wchar_t* kNonExistent = L"NonExistent";
+    ASSERT_NE(ERROR_SUCCESS, key.ReadValue(kNonExistent, &string_value));
+    ASSERT_NE(ERROR_SUCCESS, key.ReadValueDW(kNonExistent, &dword_value));
+    ASSERT_NE(ERROR_SUCCESS, key.ReadInt64(kNonExistent, &int64_value));
+    EXPECT_STREQ(kStringData, string_value.c_str());
+    EXPECT_EQ(kDWORDData, dword_value);
+    EXPECT_EQ(kInt64Data, int64_value);
+
+    // Test delete
+    ASSERT_EQ(ERROR_SUCCESS, key.DeleteValue(kStringValueName));
+    ASSERT_EQ(ERROR_SUCCESS, key.DeleteValue(kDWORDValueName));
+    ASSERT_EQ(ERROR_SUCCESS, key.DeleteValue(kInt64ValueName));
+    EXPECT_EQ(0U, key.ValueCount());
+    EXPECT_FALSE(key.ValueExists(kStringValueName));
+    EXPECT_FALSE(key.ValueExists(kDWORDValueName));
+    EXPECT_FALSE(key.ValueExists(kInt64ValueName));
   }
 }
 
diff --git a/base/win/win_util.cc b/base/win/win_util.cc
index 87905ea..d3d74a2 100644
--- a/base/win/win_util.cc
+++ b/base/win/win_util.cc
@@ -93,7 +93,7 @@ bool UserAccountControlIsEnabled() {
       L"SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Policies\\System",
       KEY_READ);
   DWORD uac_enabled;
-  if (!key.ReadValueDW(L"EnableLUA", &uac_enabled))
+  if (key.ReadValueDW(L"EnableLUA", &uac_enabled) != ERROR_SUCCESS)
     return true;
   // Users can set the EnableLUA value to something arbitrary, like 2, which
   // Vista will treat as UAC enabled, so we make sure it is not set to 0.
@@ -128,12 +128,13 @@ static const char16 kAutoRunKeyPath[] =
 bool AddCommandToAutoRun(HKEY root_key, const string16& name,
                          const string16& command) {
   base::win::RegKey autorun_key(root_key, kAutoRunKeyPath, KEY_SET_VALUE);
-  return autorun_key.WriteValue(name.c_str(), command.c_str());
+  return (autorun_key.WriteValue(name.c_str(), command.c_str()) ==
+      ERROR_SUCCESS);
 }
 
 bool RemoveCommandFromAutoRun(HKEY root_key, const string16& name) {
   base::win::RegKey autorun_key(root_key, kAutoRunKeyPath, KEY_SET_VALUE);
-  return autorun_key.DeleteValue(name.c_str());
+  return (autorun_key.DeleteValue(name.c_str()) == ERROR_SUCCESS);
 }
 
 }  // namespace win