diff options
| author | amit@chromium.org <amit@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-01-19 07:28:46 +0000 |
|---|---|---|
| committer | amit@chromium.org <amit@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2011-01-19 07:28:46 +0000 |
| commit | e06f4d5c6bd7bad162c45784e39cd0114635eb42 (patch) | |
| tree | e53d6b4188af6e49393babc92a797ed5734a1026 | |
| parent | 2b107a348f2b27934fe38680ec8010d743f61765 (diff) | |
| download | chromium_src-e06f4d5c6bd7bad162c45784e39cd0114635eb42.zip chromium_src-e06f4d5c6bd7bad162c45784e39cd0114635eb42.tar.gz chromium_src-e06f4d5c6bd7bad162c45784e39cd0114635eb42.tar.bz2 | |
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
76 files changed, 943 insertions, 906 deletions
@@ -225,7 +225,7 @@ deps_os = { Var("nacl_revision")), "src/rlz": - (Var("googlecode_url") % "rlz") + "/trunk@32", + (Var("googlecode_url") % "rlz") + "/trunk@33", # Dependencies used by libjpeg-turbo "src/third_party/yasm/binaries": 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 diff --git a/ceee/ie/common/ceee_module_util.cc b/ceee/ie/common/ceee_module_util.cc index 836f031..e3d10eb 100644 --- a/ceee/ie/common/ceee_module_util.cc +++ b/ceee/ie/common/ceee_module_util.cc @@ -43,16 +43,9 @@ bool GetCeeeRegistryBoolean(const wchar_t* key, bool default_value) { base::win::RegKey hkcu(HKEY_CURRENT_USER, kRegistryPath, KEY_READ); LOG_IF(ERROR, !hkcu.Valid()) << "Could not open reg key: " << kRegistryPath; - DWORD dword_value_representation = 0; - DWORD size = sizeof(dword_value_representation); - DWORD type = REG_DWORD; - - if (!hkcu.Valid() || - !hkcu.ReadValue(key, &dword_value_representation, &size, &type)) { - return default_value; - } - - return dword_value_representation != 0; + DWORD value = default_value ? 1 : 0; + hkcu.ReadValueDW(key, &value); + return value != 0; } void SetCeeeRegistryBoolean(const wchar_t* key, bool assign_value) { @@ -60,11 +53,13 @@ void SetCeeeRegistryBoolean(const wchar_t* key, bool assign_value) { LOG_IF(ERROR, !hkcu.Valid()) << "Could not open reg key: " << kRegistryPath; DWORD dword_value_representation = assign_value ? 1 : 0; - bool write_result = hkcu.WriteValue(key, &dword_value_representation, + LONG write_result = hkcu.WriteValue(key, &dword_value_representation, sizeof(dword_value_representation), REG_DWORD); - LOG_IF(ERROR, !write_result) << "Failed to write a registry key: " << key; + LOG_IF(ERROR, write_result != ERROR_SUCCESS) + << "Failed to write a registry key: " << key + << " error: " << com::LogWe(write_result); } } // anonymous namespace @@ -111,8 +106,10 @@ std::wstring GetExtensionPath() { base::win::RegKey* keys[] = { &hkcu, &hklm }; for (int i = 0; i < arraysize(keys); ++i) { - if (keys[i]->Valid() && keys[i]->ReadValue(kRegistryValue, &crx_path)) + if (keys[i]->Valid() && + (keys[i]->ReadValue(kRegistryValue, &crx_path) == ERROR_SUCCESS)) { break; + } } if (crx_path.size() == 0u) { @@ -196,8 +193,8 @@ bool NeedToInstallExtension() { // we might need to retry installation. std::wstring version_string; base::win::RegKey hkcu(HKEY_CURRENT_USER, kRegistryPath, KEY_READ); - success = hkcu.ReadValue( - kRegistryValueCrxInstalledByVersion, &version_string); + success = hkcu.ReadValue(kRegistryValueCrxInstalledByVersion, + &version_string) == ERROR_SUCCESS; return !success || version_string != TO_L_STRING(CHROME_VERSION_STRING); } } @@ -216,17 +213,18 @@ void SetInstalledExtensionPath(const FilePath& path) { base::Time::Now().ToInternalValue(); base::win::RegKey key(HKEY_CURRENT_USER, kRegistryPath, KEY_WRITE); - bool write_result = key.WriteValue(kRegistryValueCrxInstalledTime, + LONG write_result = key.WriteValue(kRegistryValueCrxInstalledTime, &crx_time, sizeof(crx_time), REG_QWORD); - DCHECK(write_result); + DCHECK_EQ(ERROR_SUCCESS, write_result); write_result = key.WriteValue(kRegistryValueCrxInstalledPath, path.value().c_str()); - DCHECK(write_result); + DCHECK_EQ(ERROR_SUCCESS, write_result); write_result = key.WriteValue(kRegistryValueCrxInstalledByVersion, TO_L_STRING(CHROME_VERSION_STRING)); + DCHECK_EQ(ERROR_SUCCESS, write_result); } bool IsCrxOrEmpty(const std::wstring& path) { @@ -301,12 +299,13 @@ std::wstring GetCromeFrameClientStateKey() { bool GetCollectStatsConsent() { std::wstring reg_path = GetCromeFrameClientStateKey(); base::win::RegKey key(HKEY_CURRENT_USER, reg_path.c_str(), KEY_READ); - DWORD value; - if (!key.ReadValueDW(google_update::kRegUsageStatsField, &value)) { + DWORD value = 0; + if (key.ReadValueDW(google_update::kRegUsageStatsField, &value) != + ERROR_SUCCESS) { base::win::RegKey hklm_key(HKEY_LOCAL_MACHINE, reg_path.c_str(), KEY_READ); - if (!hklm_key.ReadValueDW(google_update::kRegUsageStatsField, &value)) - return false; + key.ReadValueDW(google_update::kRegUsageStatsField, &value); } + return (1 == value); } @@ -326,7 +325,7 @@ bool RefreshElevationPolicyIfNeeded() { static const wchar_t kValueName[] = L"last_elevation_refresh"; std::wstring last_elevation_refresh_version; - bool result = hkcu.ReadValue(kValueName, &last_elevation_refresh_version); + LONG result = hkcu.ReadValue(kValueName, &last_elevation_refresh_version); if (last_elevation_refresh_version == expected_version) return false; @@ -336,7 +335,8 @@ bool RefreshElevationPolicyIfNeeded() { // Write after refreshing because it's better to refresh twice, than to miss // once. result = hkcu.WriteValue(kValueName, expected_version.c_str()); - LOG_IF(ERROR, !result) << "Failed to write a registry value: " << kValueName; + LOG_IF(ERROR, result != ERROR_SUCCESS) << "Failed to write a registry value: " + << kValueName << " error: " << com::LogWe(result); return true; } diff --git a/ceee/ie/common/ceee_module_util_unittest.cc b/ceee/ie/common/ceee_module_util_unittest.cc index 5bda65d..49352c5 100644 --- a/ceee/ie/common/ceee_module_util_unittest.cc +++ b/ceee/ie/common/ceee_module_util_unittest.cc @@ -207,16 +207,20 @@ TEST_F(CeeeModuleUtilTest, GetCollectStatsConsentFromHkcu) { base::win::RegKey hkcu(HKEY_CURRENT_USER, ceee_module_util::GetCromeFrameClientStateKey().c_str(), KEY_WRITE); - ASSERT_TRUE(hkcu.WriteValue(google_update::kRegUsageStatsField, kTrue)); + ASSERT_EQ(ERROR_SUCCESS, + hkcu.WriteValue(google_update::kRegUsageStatsField, kTrue)); EXPECT_TRUE(ceee_module_util::GetCollectStatsConsent()); - ASSERT_TRUE(hkcu.WriteValue(google_update::kRegUsageStatsField, kFalse)); + ASSERT_EQ(ERROR_SUCCESS, + hkcu.WriteValue(google_update::kRegUsageStatsField, kFalse)); EXPECT_FALSE(ceee_module_util::GetCollectStatsConsent()); - ASSERT_TRUE(hkcu.WriteValue(google_update::kRegUsageStatsField, kInvalid)); + ASSERT_EQ(ERROR_SUCCESS, + hkcu.WriteValue(google_update::kRegUsageStatsField, kInvalid)); EXPECT_FALSE(ceee_module_util::GetCollectStatsConsent()); - ASSERT_TRUE(hkcu.DeleteValue(google_update::kRegUsageStatsField)); + ASSERT_EQ(ERROR_SUCCESS, + hkcu.DeleteValue(google_update::kRegUsageStatsField)); EXPECT_FALSE(ceee_module_util::GetCollectStatsConsent()); } @@ -225,16 +229,20 @@ TEST_F(CeeeModuleUtilTest, GetCollectStatsConsentFromHklm) { base::win::RegKey hklm(HKEY_LOCAL_MACHINE, ceee_module_util::GetCromeFrameClientStateKey().c_str(), KEY_WRITE); - ASSERT_TRUE(hklm.WriteValue(google_update::kRegUsageStatsField, kTrue)); + ASSERT_EQ(ERROR_SUCCESS, + hklm.WriteValue(google_update::kRegUsageStatsField, kTrue)); EXPECT_TRUE(ceee_module_util::GetCollectStatsConsent()); - ASSERT_TRUE(hklm.WriteValue(google_update::kRegUsageStatsField, kFalse)); + ASSERT_EQ(ERROR_SUCCESS, + hklm.WriteValue(google_update::kRegUsageStatsField, kFalse)); EXPECT_FALSE(ceee_module_util::GetCollectStatsConsent()); - ASSERT_TRUE(hklm.WriteValue(google_update::kRegUsageStatsField, kInvalid)); + ASSERT_EQ(ERROR_SUCCESS, + hklm.WriteValue(google_update::kRegUsageStatsField, kInvalid)); EXPECT_FALSE(ceee_module_util::GetCollectStatsConsent()); - ASSERT_TRUE(hklm.DeleteValue(google_update::kRegUsageStatsField)); + ASSERT_EQ(ERROR_SUCCESS, + hklm.DeleteValue(google_update::kRegUsageStatsField)); EXPECT_FALSE(ceee_module_util::GetCollectStatsConsent()); } @@ -246,14 +254,18 @@ TEST_F(CeeeModuleUtilTest, GetCollectStatsConsentHkcuBeforeHklm) { base::win::RegKey hklm(HKEY_LOCAL_MACHINE, ceee_module_util::GetCromeFrameClientStateKey().c_str(), KEY_WRITE); - ASSERT_TRUE(hklm.WriteValue(google_update::kRegUsageStatsField, kTrue)); + ASSERT_EQ(ERROR_SUCCESS, + hklm.WriteValue(google_update::kRegUsageStatsField, kTrue)); ASSERT_TRUE(ceee_module_util::GetCollectStatsConsent()); - ASSERT_TRUE(hkcu.WriteValue(google_update::kRegUsageStatsField, kFalse)); + ASSERT_EQ(ERROR_SUCCESS, + hkcu.WriteValue(google_update::kRegUsageStatsField, kFalse)); EXPECT_FALSE(ceee_module_util::GetCollectStatsConsent()); - ASSERT_TRUE(hkcu.DeleteValue(google_update::kRegUsageStatsField)); - ASSERT_TRUE(hklm.DeleteValue(google_update::kRegUsageStatsField)); + ASSERT_EQ(ERROR_SUCCESS, + hkcu.DeleteValue(google_update::kRegUsageStatsField)); + ASSERT_EQ(ERROR_SUCCESS, + hklm.DeleteValue(google_update::kRegUsageStatsField)); } } // namespace diff --git a/ceee/ie/common/ceee_util.cc b/ceee/ie/common/ceee_util.cc index 4d9b3d5..745e5e4 100644 --- a/ceee/ie/common/ceee_util.cc +++ b/ceee/ie/common/ceee_util.cc @@ -23,7 +23,8 @@ bool IsIeCeeeRegistered() { path += clsid; base::win::RegKey key; - return key.Open(HKEY_CLASSES_ROOT, path.c_str(), KEY_QUERY_VALUE); + return (key.Open(HKEY_CLASSES_ROOT, path.c_str(), KEY_QUERY_VALUE) == + ERROR_SUCCESS); } } // namespace ceee_util diff --git a/ceee/ie/common/ie_util.cc b/ceee/ie/common/ie_util.cc index 7c1707d..dee92d1 100644 --- a/ceee/ie/common/ie_util.cc +++ b/ceee/ie/common/ie_util.cc @@ -43,7 +43,7 @@ bool GetIeVersionString(std::wstring* version) { return false; base::win::RegKey key(HKEY_LOCAL_MACHINE, kIeVersionKey, KEY_READ); DCHECK(key.ValueExists(kIeVersionValue)); - return key.ReadValue(kIeVersionValue, version); + return (key.ReadValue(kIeVersionValue, version) == ERROR_SUCCESS); } } // namespace @@ -164,9 +164,12 @@ int GetAverageAddonTimeMs(const CLSID& addon_id, } DWORD load_time = 0; + LONG result = ERROR_SUCCESS; if (GetIeVersion() < IEVERSION_IE9) { - if (!stats_key.ReadValueDW(time_prefix.c_str(), &load_time)) { - VLOG(1) << "Can't read time: " << time_prefix; + result = stats_key.ReadValueDW(time_prefix.c_str(), &load_time); + if (result != ERROR_SUCCESS) { + VLOG(1) << "Can't read time: " << time_prefix << " error: " + << com::LogWe(result); return kInvalidTime; } } else { @@ -177,9 +180,11 @@ int GetAverageAddonTimeMs(const CLSID& addon_id, DWORD data_size = sizeof(values); DWORD data_type = REG_NONE; - if (!stats_key.ReadValue(value_name.c_str(), &values, &data_size, - &data_type)) { - VLOG(1) << "Can't read time: " << value_name; + result = stats_key.ReadValue(value_name.c_str(), &values, &data_size, + &data_type); + if (result != ERROR_SUCCESS) { + VLOG(1) << "Can't read time: " << value_name << " error: " + << com::LogWe(result); return kInvalidTime; } diff --git a/ceee/ie/common/ie_util_unittest.cc b/ceee/ie/common/ie_util_unittest.cc index 0d0eda9..672e81c 100644 --- a/ceee/ie/common/ie_util_unittest.cc +++ b/ceee/ie/common/ie_util_unittest.cc @@ -70,11 +70,11 @@ TEST_F(IeUtilTest, Ie8) { base::win::RegKey stats_key(HKEY_CURRENT_USER, kAddonStatsRegPath, KEY_WRITE); ASSERT_TRUE(stats_key.Valid()); - ASSERT_TRUE(stats_key.WriteValue(L"LoadTime", L"time")); + ASSERT_EQ(ERROR_SUCCESS, stats_key.WriteValue(L"LoadTime", L"time")); EXPECT_EQ(-1, GetAverageAddonLoadTimeMs(kAddonGuid)); DWORD time = 11; - ASSERT_TRUE(stats_key.WriteValue(L"LoadTime", time)); + ASSERT_EQ(ERROR_SUCCESS, stats_key.WriteValue(L"LoadTime", time)); EXPECT_EQ(11, GetAverageAddonLoadTimeMs(kAddonGuid)); } @@ -89,24 +89,25 @@ TEST_F(IeUtilTest, Ie9) { base::win::RegKey stats_key(HKEY_CURRENT_USER, kAddonStatsRegPath, KEY_WRITE); ASSERT_TRUE(stats_key.Valid()); - ASSERT_TRUE(stats_key.WriteValue(L"LoadTimeArray", L"time")); + ASSERT_EQ(ERROR_SUCCESS, stats_key.WriteValue(L"LoadTimeArray", L"time")); EXPECT_EQ(-1, GetAverageAddonLoadTimeMs(kAddonGuid)); DWORD time[] = {1, 2, 3, 4, -1, 10}; - ASSERT_TRUE(stats_key.WriteValue(L"LoadTimeArray", &time, sizeof(time[0]), - REG_BINARY)); + ASSERT_EQ(ERROR_SUCCESS, stats_key.WriteValue(L"LoadTimeArray", &time, + sizeof(time[0]), REG_BINARY)); EXPECT_EQ(-1, GetAverageAddonLoadTimeMs(kAddonGuid)); - ASSERT_TRUE(stats_key.WriteValue(L"LoadTimeArray", &time, 10, REG_BINARY)); + ASSERT_EQ(ERROR_SUCCESS, + stats_key.WriteValue(L"LoadTimeArray", &time, 10, REG_BINARY)); EXPECT_EQ(-1, GetAverageAddonLoadTimeMs(kAddonGuid)); - ASSERT_TRUE(stats_key.WriteValue(L"LoadTimeArray", &time, 4 * sizeof(time[0]), - REG_BINARY)); + ASSERT_EQ(ERROR_SUCCESS, stats_key.WriteValue(L"LoadTimeArray", + &time, 4 * sizeof(time[0]), REG_BINARY)); EXPECT_EQ(2, GetAverageAddonLoadTimeMs(kAddonGuid)); - ASSERT_TRUE(stats_key.WriteValue(L"LoadTimeArray", time, sizeof(time), - REG_BINARY)); + ASSERT_EQ(ERROR_SUCCESS, stats_key.WriteValue(L"LoadTimeArray", time, + sizeof(time), REG_BINARY)); EXPECT_EQ(4, GetAverageAddonLoadTimeMs(kAddonGuid)); } diff --git a/ceee/ie/plugin/toolband/toolband_proxy.cc b/ceee/ie/plugin/toolband/toolband_proxy.cc index a228cda..d6094cb 100644 --- a/ceee/ie/plugin/toolband/toolband_proxy.cc +++ b/ceee/ie/plugin/toolband/toolband_proxy.cc @@ -51,11 +51,12 @@ void CheckAsyncIidRegistered(const IID& iid, const IID& async_iid) { L"Interface\\%ls\\AsynchronousInterface", iid_str.c_str()); base::win::RegKey key; - if (key.Open(HKEY_CLASSES_ROOT, key_name.c_str(), KEY_READ)) { + if (key.Open(HKEY_CLASSES_ROOT, key_name.c_str(), KEY_READ) == + ERROR_SUCCESS) { // It's registered, the rest of this block is debug checking that // the correct IID is indeed registered for the async interface. std::wstring async_iid_str; - DCHECK(key.ReadValue(NULL, &async_iid_str)); + DCHECK_EQ(ERROR_SUCCESS, key.ReadValue(NULL, &async_iid_str)); IID read_async_iid; DCHECK(SUCCEEDED(::IIDFromString(async_iid_str.c_str(), &read_async_iid))); DCHECK(read_async_iid == async_iid); diff --git a/ceee/installer_dll/installer_helper.cc b/ceee/installer_dll/installer_helper.cc index 2128228..5c65a62 100644 --- a/ceee/installer_dll/installer_helper.cc +++ b/ceee/installer_dll/installer_helper.cc @@ -227,10 +227,11 @@ HRESULT RegisterFirefoxCeee(bool do_register) { if (file_util::PathExists(path)) { base::win::RegKey key(HKEY_LOCAL_MACHINE, L"SOFTWARE", KEY_WRITE); - if (key.CreateKey(L"Mozilla", KEY_WRITE) && - key.CreateKey(L"Firefox", KEY_WRITE) && - key.CreateKey(L"extensions", KEY_WRITE) && - key.WriteValue(kCeeeFirefoxExtensionName, path.value().c_str())) { + if ((key.CreateKey(L"Mozilla", KEY_WRITE) == ERROR_SUCCESS) && + (key.CreateKey(L"Firefox", KEY_WRITE) == ERROR_SUCCESS) && + (key.CreateKey(L"extensions", KEY_WRITE) == ERROR_SUCCESS) && + (key.WriteValue(kCeeeFirefoxExtensionName, path.value().c_str())) + == ERROR_SUCCESS) { hr = S_OK; } else { hr = com::AlwaysErrorFromLastError(); @@ -239,13 +240,12 @@ HRESULT RegisterFirefoxCeee(bool do_register) { } else { hr = S_OK; // OK if not found, then there's nothing to do base::win::RegKey key(HKEY_LOCAL_MACHINE, L"SOFTWARE", KEY_READ); - if (key.OpenKey(L"Mozilla", KEY_READ) && - key.OpenKey(L"Firefox", KEY_READ) && - key.OpenKey(L"extensions", KEY_WRITE) && + if ((key.OpenKey(L"Mozilla", KEY_READ) == ERROR_SUCCESS) && + (key.OpenKey(L"Firefox", KEY_READ) == ERROR_SUCCESS) && + (key.OpenKey(L"extensions", KEY_WRITE) == ERROR_SUCCESS) && key.ValueExists(kCeeeFirefoxExtensionName)) { - if (!key.DeleteValue(kCeeeFirefoxExtensionName)) { - hr = com::AlwaysErrorFromLastError(); - } + LONG result = key.DeleteValue(kCeeeFirefoxExtensionName); + hr = HRESULT_FROM_WIN32(result); } } @@ -258,7 +258,8 @@ HRESULT SetCeeeChannelModifier(bool new_value) { std::wstring reg_key(ceee_module_util::GetCromeFrameClientStateKey()); base::win::RegKey key; - if (!key.Open(HKEY_LOCAL_MACHINE, reg_key.c_str(), KEY_ALL_ACCESS)) { + if (key.Open(HKEY_LOCAL_MACHINE, reg_key.c_str(), KEY_ALL_ACCESS) != + ERROR_SUCCESS) { // Omaha didn't install the key. Perhaps no Omaha? That's ok. return S_OK; } diff --git a/ceee/testing/utils/test_utils.cc b/ceee/testing/utils/test_utils.cc index 282416a..59ad886 100644 --- a/ceee/testing/utils/test_utils.cc +++ b/ceee/testing/utils/test_utils.cc @@ -192,8 +192,10 @@ void ScopedRegistryOverride::Override() { ASSERT_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)); // Switch keys. ASSERT_EQ(ERROR_SUCCESS, ::RegOverridePredefKey(HKEY_CURRENT_USER, diff --git a/chrome/app/breakpad_win.cc b/chrome/app/breakpad_win.cc index 9f04967..9fe1032 100644 --- a/chrome/app/breakpad_win.cc +++ b/chrome/app/breakpad_win.cc @@ -437,17 +437,18 @@ bool ShowRestartDialogIfCrashed(bool* exit_now) { // contains the value set by policy. static bool MetricsReportingControlledByPolicy(bool* result) { std::wstring key_name = UTF8ToWide(policy::key::kMetricsReportingEnabled); - DWORD value; + DWORD value = 0; + // TODO(joshia): why hkcu_policy_key opens HKEY_LOCAL_MACHINE? base::win::RegKey hkcu_policy_key(HKEY_LOCAL_MACHINE, policy::kRegistrySubKey, KEY_READ); - if (hkcu_policy_key.ReadValueDW(key_name.c_str(), &value)) { + if (hkcu_policy_key.ReadValueDW(key_name.c_str(), &value) == ERROR_SUCCESS) { *result = value != 0; return true; } base::win::RegKey hklm_policy_key(HKEY_CURRENT_USER, policy::kRegistrySubKey, KEY_READ); - if (hklm_policy_key.ReadValueDW(key_name.c_str(), &value)) { + if (hklm_policy_key.ReadValueDW(key_name.c_str(), &value) == ERROR_SUCCESS) { *result = value != 0; return true; } diff --git a/chrome/browser/autofill/autofill_ie_toolbar_import_win.cc b/chrome/browser/autofill/autofill_ie_toolbar_import_win.cc index 05b7ade..3519de1 100644 --- a/chrome/browser/autofill/autofill_ie_toolbar_import_win.cc +++ b/chrome/browser/autofill/autofill_ie_toolbar_import_win.cc @@ -57,12 +57,13 @@ bool IsEmptySalt(std::wstring const& salt) { string16 ReadAndDecryptValue(RegKey* key, const wchar_t* value_name) { DWORD data_type = REG_BINARY; DWORD data_size = 0; - if (!key->ReadValue(value_name, NULL, &data_size, &data_type) || - !data_size || data_type != REG_BINARY) + LONG result = key->ReadValue(value_name, NULL, &data_size, &data_type); + if ((result != ERROR_SUCCESS) || !data_size || data_type != REG_BINARY) return string16(); std::vector<uint8> data; data.resize(data_size); - if (key->ReadValue(value_name, &(data[0]), &data_size, &data_type)) { + result = key->ReadValue(value_name, &(data[0]), &data_size, &data_type); + if (result == ERROR_SUCCESS) { std::string out_data; if (DecryptData(data, &out_data)) { // The actual data is in UTF16 already. @@ -125,7 +126,7 @@ bool ImportSingleProfile(FormGroup* profile, for (uint32 value_index = 0; value_index < key->ValueCount(); ++value_index) { std::wstring value_name; - if (!key->ReadName(value_index, &value_name)) + if (key->ReadName(value_index, &value_name) != ERROR_SUCCESS) continue; RegToFieldMap::const_iterator it = reg_to_field.find(value_name); if (it == reg_to_field.end()) diff --git a/chrome/browser/autofill/autofill_ie_toolbar_import_win_unittest.cc b/chrome/browser/autofill/autofill_ie_toolbar_import_win_unittest.cc index 17e85db..972badb 100644 --- a/chrome/browser/autofill/autofill_ie_toolbar_import_win_unittest.cc +++ b/chrome/browser/autofill/autofill_ie_toolbar_import_win_unittest.cc @@ -92,8 +92,8 @@ void EncryptAndWrite(RegKey* key, const ValueDescription* value) { memcpy(&data[0], value->value, data_size); std::vector<uint8> encrypted_data = EncryptData(data); - EXPECT_TRUE(key->WriteValue(value->value_name, &encrypted_data[0], - encrypted_data.size(), REG_BINARY)); + EXPECT_EQ(ERROR_SUCCESS, key->WriteValue(value->value_name, + &encrypted_data[0], encrypted_data.size(), REG_BINARY)); } void CreateSubkey(RegKey* key, wchar_t const* subkey_name, diff --git a/chrome/browser/background_mode_manager_win.cc b/chrome/browser/background_mode_manager_win.cc index 2f10843..20d649c 100644 --- a/chrome/browser/background_mode_manager_win.cc +++ b/chrome/browser/background_mode_manager_win.cc @@ -38,8 +38,11 @@ void DisableLaunchOnStartupTask::Run() { kBackgroundModeRegistrySubkey, KEY_READ); base::win::RegKey write_key(kBackgroundModeRegistryRootKey, kBackgroundModeRegistrySubkey, KEY_WRITE); - if (read_key.ValueExists(key_name) && !write_key.DeleteValue(key_name)) - NOTREACHED() << "Failed to deregister launch on login."; + if (read_key.ValueExists(key_name)) { + LONG result = write_key.DeleteValue(key_name); + DCHECK_EQ(ERROR_SUCCESS, result) << + "Failed to deregister launch on login. error: " << result; + } } void EnableLaunchOnStartupTask::Run() { @@ -56,13 +59,14 @@ void EnableLaunchOnStartupTask::Run() { std::wstring new_value = executable.value() + L" --no-startup-window"; if (read_key.ValueExists(key_name)) { std::wstring current_value; - if (read_key.ReadValue(key_name, ¤t_value) && + if ((read_key.ReadValue(key_name, ¤t_value) == ERROR_SUCCESS) && (current_value == new_value)) { return; } } - if (!write_key.WriteValue(key_name, new_value.c_str())) - NOTREACHED() << "Failed to register launch on login."; + LONG result = write_key.WriteValue(key_name, new_value.c_str()); + DCHECK_EQ(ERROR_SUCCESS, result) << + "Failed to register launch on login. error: " << result; } } // namespace diff --git a/chrome/browser/enumerate_modules_model_win.cc b/chrome/browser/enumerate_modules_model_win.cc index 79e0604..3a23d10 100644 --- a/chrome/browser/enumerate_modules_model_win.cc +++ b/chrome/browser/enumerate_modules_model_win.cc @@ -423,12 +423,12 @@ void ModuleEnumerator::ReadShellExtensions(HKEY parent) { std::wstring key(std::wstring(L"CLSID\\") + registration.Name() + L"\\InProcServer32"); base::win::RegKey clsid; - if (!clsid.Open(HKEY_CLASSES_ROOT, key.c_str(), KEY_READ)) { + if (clsid.Open(HKEY_CLASSES_ROOT, key.c_str(), KEY_READ) != ERROR_SUCCESS) { ++registration; continue; } string16 dll; - if (!clsid.ReadValue(L"", &dll)) { + if (clsid.ReadValue(L"", &dll) != ERROR_SUCCESS) { ++registration; continue; } diff --git a/chrome/browser/extensions/extension_rlz_apitest.cc b/chrome/browser/extensions/extension_rlz_apitest.cc index efb275a..f7f264d 100644 --- a/chrome/browser/extensions/extension_rlz_apitest.cc +++ b/chrome/browser/extensions/extension_rlz_apitest.cc @@ -80,14 +80,14 @@ IN_PROC_BROWSER_TEST_F(ExtensionApiTest, Rlz) { ASSERT_TRUE(key.Valid()); DWORD value; - ASSERT_TRUE(key.ReadValueDW(L"D3I", &value)); + ASSERT_EQ(ERROR_SUCCESS, key.ReadValueDW(L"D3I", &value)); ASSERT_EQ(1, value); - ASSERT_TRUE(key.ReadValueDW(L"D3S", &value)); + ASSERT_EQ(ERROR_SUCCESS, key.ReadValueDW(L"D3S", &value)); ASSERT_EQ(1, value); - ASSERT_TRUE(key.ReadValueDW(L"D3F", &value)); + ASSERT_EQ(ERROR_SUCCESS, key.ReadValueDW(L"D3F", &value)); ASSERT_EQ(1, value); - ASSERT_TRUE(key.ReadValueDW(L"D4I", &value)); + ASSERT_EQ(ERROR_SUCCESS, key.ReadValueDW(L"D4I", &value)); ASSERT_EQ(1, value); key.Open(HKEY_CURRENT_USER, L"Software\\Google\\Common\\Rlz\\Events\\D", diff --git a/chrome/browser/extensions/external_registry_extension_loader_win.cc b/chrome/browser/extensions/external_registry_extension_loader_win.cc index 11c35f1..b3037c8 100644 --- a/chrome/browser/extensions/external_registry_extension_loader_win.cc +++ b/chrome/browser/extensions/external_registry_extension_loader_win.cc @@ -49,11 +49,13 @@ void ExternalRegistryExtensionLoader::LoadOnFileThread() { std::wstring key_path = ASCIIToWide(kRegistryExtensions); key_path.append(L"\\"); key_path.append(iterator.Name()); - if (key.Open(kRegRoot, key_path.c_str(), KEY_READ)) { + if (key.Open(kRegRoot, key_path.c_str(), KEY_READ) == ERROR_SUCCESS) { std::wstring extension_path; - if (key.ReadValue(kRegistryExtensionPath, &extension_path)) { + if (key.ReadValue(kRegistryExtensionPath, &extension_path) + == ERROR_SUCCESS) { std::wstring extension_version; - if (key.ReadValue(kRegistryExtensionVersion, &extension_version)) { + if (key.ReadValue(kRegistryExtensionVersion, &extension_version) + == ERROR_SUCCESS) { std::string id = WideToASCII(iterator.Name()); StringToLowerASCII(&id); diff --git a/chrome/browser/first_run/first_run_win.cc b/chrome/browser/first_run/first_run_win.cc index ac09d18..179e43b 100644 --- a/chrome/browser/first_run/first_run_win.cc +++ b/chrome/browser/first_run/first_run_win.cc @@ -278,8 +278,10 @@ bool Upgrade::SwapNewChromeExeIfPresent() { BrowserDistribution *dist = BrowserDistribution::GetDistribution(); base::win::RegKey key; std::wstring rename_cmd; - if (key.Open(reg_root, dist->GetVersionKey().c_str(), KEY_READ) && - key.ReadValue(google_update::kRegRenameCmdField, &rename_cmd)) { + if ((key.Open(reg_root, dist->GetVersionKey().c_str(), + KEY_READ) == ERROR_SUCCESS) && + (key.ReadValue(google_update::kRegRenameCmdField, + &rename_cmd) == ERROR_SUCCESS)) { base::ProcessHandle handle; if (base::LaunchApp(rename_cmd, true, true, &handle)) { DWORD exit_code; diff --git a/chrome/browser/importer/firefox_importer_utils_win.cc b/chrome/browser/importer/firefox_importer_utils_win.cc index 7fc2a41..16696b9 100644 --- a/chrome/browser/importer/firefox_importer_utils_win.cc +++ b/chrome/browser/importer/firefox_importer_utils_win.cc @@ -31,9 +31,9 @@ int GetCurrentFirefoxMajorVersionFromRegistry() { base::win::RegKey reg_key(kFireFoxRegistryPaths[i], L"Software\\Mozilla\\Mozilla Firefox", KEY_READ); - bool result = reg_key.ReadValue(L"CurrentVersion", ver_buffer, + LONG result = reg_key.ReadValue(L"CurrentVersion", ver_buffer, &ver_buffer_length, NULL); - if (!result) + if (result != ERROR_SUCCESS) continue; highest_version = std::max(highest_version, _wtoi(ver_buffer)); } @@ -47,9 +47,9 @@ std::wstring GetFirefoxInstallPathFromRegistry() { DWORD buffer_length = sizeof(buffer); base::win::RegKey reg_key(HKEY_LOCAL_MACHINE, registry_path.c_str(), KEY_READ); - bool result = reg_key.ReadValue(L"CurrentVersion", buffer, + LONG result = reg_key.ReadValue(L"CurrentVersion", buffer, &buffer_length, NULL); - if (!result) + if (result != ERROR_SUCCESS) return std::wstring(); registry_path += L"\\" + std::wstring(buffer) + L"\\Main"; buffer_length = sizeof(buffer); @@ -57,7 +57,7 @@ std::wstring GetFirefoxInstallPathFromRegistry() { registry_path.c_str(), KEY_READ); result = reg_key_directory.ReadValue(L"Install Directory", buffer, &buffer_length, NULL); - if (!result) + if (result != ERROR_SUCCESS) return std::wstring(); return buffer; } diff --git a/chrome/browser/importer/ie_importer.cc b/chrome/browser/importer/ie_importer.cc index b26c306..484414f 100644 --- a/chrome/browser/importer/ie_importer.cc +++ b/chrome/browser/importer/ie_importer.cc @@ -263,19 +263,18 @@ void IEImporter::ImportPasswordsIE7() { base::win::RegKey key(HKEY_CURRENT_USER, kStorage2Path, KEY_READ); base::win::RegistryValueIterator reg_iterator(HKEY_CURRENT_USER, kStorage2Path); + IE7PasswordInfo password_info; while (reg_iterator.Valid() && !cancelled()) { // Get the size of the encrypted data. DWORD value_len = 0; - if (key.ReadValue(reg_iterator.Name(), NULL, &value_len, NULL) && - value_len) { + key.ReadValue(reg_iterator.Name(), NULL, &value_len, NULL); + if (value_len) { // Query the encrypted data. - std::vector<unsigned char> value; - value.resize(value_len); - if (key.ReadValue(reg_iterator.Name(), &value.front(), &value_len, - NULL)) { - IE7PasswordInfo password_info; + password_info.encrypted_data.resize(value_len); + if (key.ReadValue(reg_iterator.Name(), + &password_info.encrypted_data.front(), + &value_len, NULL) == ERROR_SUCCESS) { password_info.url_hash = reg_iterator.Name(); - password_info.encrypted_data = value; password_info.date_created = Time::Now(); bridge_->AddIE7PasswordInfo(password_info); @@ -366,7 +365,8 @@ void IEImporter::ImportSearchEngines() { base::win::RegKey sub_key(HKEY_CURRENT_USER, sub_key_name.c_str(), KEY_READ); std::wstring wide_url; - if (!sub_key.ReadValue(L"URL", &wide_url) || wide_url.empty()) { + if ((sub_key.ReadValue(L"URL", &wide_url) != ERROR_SUCCESS) || + wide_url.empty()) { VLOG(1) << "No URL for IE search engine at " << key_iterator.Name(); ++key_iterator; continue; @@ -375,9 +375,10 @@ void IEImporter::ImportSearchEngines() { // non displayable name in DisplayName, and the readable name under the // default value). std::wstring name; - if (!sub_key.ReadValue(NULL, &name) || name.empty()) { + if ((sub_key.ReadValue(NULL, &name) != ERROR_SUCCESS) || name.empty()) { // Try the displayable name. - if (!sub_key.ReadValue(L"DisplayName", &name) || name.empty()) { + if ((sub_key.ReadValue(L"DisplayName", &name) != ERROR_SUCCESS) || + name.empty()) { VLOG(1) << "No name for IE search engine at " << key_iterator.Name(); ++key_iterator; continue; @@ -433,7 +434,8 @@ void IEImporter::ImportHomepage() { base::win::RegKey key(HKEY_CURRENT_USER, kIESettingsMain, KEY_READ); std::wstring homepage_url; - if (!key.ReadValue(kIEHomepage, &homepage_url) || homepage_url.empty()) + if (key.ReadValue(kIEHomepage, &homepage_url) != ERROR_SUCCESS || + homepage_url.empty()) return; GURL homepage = GURL(homepage_url); @@ -443,8 +445,8 @@ void IEImporter::ImportHomepage() { // Check to see if this is the default website and skip import. base::win::RegKey keyDefault(HKEY_LOCAL_MACHINE, kIESettingsMain, KEY_READ); std::wstring default_homepage_url; - if (keyDefault.ReadValue(kIEDefaultHomepage, &default_homepage_url) && - !default_homepage_url.empty()) { + LONG result = keyDefault.ReadValue(kIEDefaultHomepage, &default_homepage_url); + if (result == ERROR_SUCCESS && !default_homepage_url.empty()) { if (homepage.spec() == GURL(default_homepage_url).spec()) return; } @@ -477,7 +479,8 @@ bool IEImporter::GetFavoritesInfo(IEImporter::FavoritesInfo *info) { DWORD buffer_length = sizeof(buffer); base::win::RegKey reg_key(HKEY_CURRENT_USER, L"Software\\Microsoft\\Internet Explorer\\Toolbar", KEY_READ); - if (!reg_key.ReadValue(L"LinksFolderName", buffer, &buffer_length, NULL)) + if (reg_key.ReadValue(L"LinksFolderName", buffer, + &buffer_length, NULL) != ERROR_SUCCESS) return false; info->links_folder = buffer; } else { @@ -587,8 +590,8 @@ int IEImporter::CurrentIEVersion() const { DWORD buffer_length = sizeof(buffer); base::win::RegKey reg_key(HKEY_LOCAL_MACHINE, L"Software\\Microsoft\\Internet Explorer", KEY_READ); - bool result = reg_key.ReadValue(L"Version", buffer, &buffer_length, NULL); - version = (result ? _wtoi(buffer) : 0); + LONG result = reg_key.ReadValue(L"Version", buffer, &buffer_length, NULL); + version = ((result == ERROR_SUCCESS)? _wtoi(buffer) : 0); } return version; } diff --git a/chrome/browser/plugin_service.cc b/chrome/browser/plugin_service.cc index 1f178d9..cadc7e2 100644 --- a/chrome/browser/plugin_service.cc +++ b/chrome/browser/plugin_service.cc @@ -125,12 +125,12 @@ PluginService::PluginService() HKEY_CURRENT_USER, webkit::npapi::kRegistryMozillaPlugins, KEY_NOTIFY); hklm_key_.Create( HKEY_LOCAL_MACHINE, webkit::npapi::kRegistryMozillaPlugins, KEY_NOTIFY); - if (hkcu_key_.StartWatching()) { + if (hkcu_key_.StartWatching() == ERROR_SUCCESS) { hkcu_event_.reset(new base::WaitableEvent(hkcu_key_.watch_event())); hkcu_watcher_.StartWatching(hkcu_event_.get(), this); } - if (hklm_key_.StartWatching()) { + if (hklm_key_.StartWatching() == ERROR_SUCCESS) { hklm_event_.reset(new base::WaitableEvent(hklm_key_.watch_event())); hklm_watcher_.StartWatching(hklm_event_.get(), this); } diff --git a/chrome/browser/policy/configuration_policy_provider_delegate_win.cc b/chrome/browser/policy/configuration_policy_provider_delegate_win.cc index 68947de..1f2f467 100644 --- a/chrome/browser/policy/configuration_policy_provider_delegate_win.cc +++ b/chrome/browser/policy/configuration_policy_provider_delegate_win.cc @@ -19,7 +19,7 @@ bool ReadRegistryStringValue(RegKey* key, const string16& name, DWORD key_type = 0; scoped_array<uint8> buffer; - if (!key->ReadValue(name.c_str(), 0, &value_size, &key_type)) + if (key->ReadValue(name.c_str(), 0, &value_size, &key_type) != ERROR_SUCCESS) return false; if (key_type != REG_SZ) return false; @@ -89,16 +89,14 @@ DictionaryValue* ConfigurationPolicyProviderDelegateWin::Load() { bool ConfigurationPolicyProviderDelegateWin::GetRegistryPolicyString( const string16& name, string16* result) const { - string16 path = string16(kRegistrySubKey); - RegKey policy_key; + RegKey policy_key(HKEY_LOCAL_MACHINE, kRegistrySubKey, KEY_READ); // First try the global policy. - if (policy_key.Open(HKEY_LOCAL_MACHINE, path.c_str(), KEY_READ)) { - if (ReadRegistryStringValue(&policy_key, name, result)) - return true; - policy_key.Close(); - } + if (ReadRegistryStringValue(&policy_key, name, result)) + return true; + // Fall back on user-specific policy. - if (!policy_key.Open(HKEY_CURRENT_USER, path.c_str(), KEY_READ)) + if (policy_key.Open(HKEY_CURRENT_USER, kRegistrySubKey, + KEY_READ) != ERROR_SUCCESS) return false; return ReadRegistryStringValue(&policy_key, name, result); } @@ -108,10 +106,11 @@ bool ConfigurationPolicyProviderDelegateWin::GetRegistryPolicyStringList( string16 path = string16(kRegistrySubKey); path += ASCIIToUTF16("\\") + key; RegKey policy_key; - if (!policy_key.Open(HKEY_LOCAL_MACHINE, path.c_str(), KEY_READ)) { - policy_key.Close(); + if (policy_key.Open(HKEY_LOCAL_MACHINE, path.c_str(), KEY_READ) != + ERROR_SUCCESS) { // Fall back on user-specific policy. - if (!policy_key.Open(HKEY_CURRENT_USER, path.c_str(), KEY_READ)) + if (policy_key.Open(HKEY_CURRENT_USER, path.c_str(), KEY_READ) != + ERROR_SUCCESS) return false; } string16 policy_string; @@ -125,34 +124,28 @@ bool ConfigurationPolicyProviderDelegateWin::GetRegistryPolicyStringList( bool ConfigurationPolicyProviderDelegateWin::GetRegistryPolicyBoolean( const string16& value_name, bool* result) const { - DWORD value; - RegKey hkcu_policy_key(HKEY_LOCAL_MACHINE, kRegistrySubKey, KEY_READ); - if (hkcu_policy_key.ReadValueDW(value_name.c_str(), &value)) { - *result = value != 0; - return true; - } - - RegKey hklm_policy_key(HKEY_CURRENT_USER, kRegistrySubKey, KEY_READ); - if (hklm_policy_key.ReadValueDW(value_name.c_str(), &value)) { - *result = value != 0; - return true; - } - return false; + uint32 local_result = 0; + bool ret = GetRegistryPolicyInteger(value_name, &local_result); + if (ret) + *result = local_result != 0; + return ret; } bool ConfigurationPolicyProviderDelegateWin::GetRegistryPolicyInteger( const string16& value_name, uint32* result) const { - DWORD value; - RegKey hkcu_policy_key(HKEY_LOCAL_MACHINE, kRegistrySubKey, KEY_READ); - if (hkcu_policy_key.ReadValueDW(value_name.c_str(), &value)) { + DWORD value = 0; + RegKey policy_key(HKEY_LOCAL_MACHINE, kRegistrySubKey, KEY_READ); + if (policy_key.ReadValueDW(value_name.c_str(), &value) == ERROR_SUCCESS) { *result = value; return true; } - RegKey hklm_policy_key(HKEY_CURRENT_USER, kRegistrySubKey, KEY_READ); - if (hklm_policy_key.ReadValueDW(value_name.c_str(), &value)) { - *result = value; - return true; + if (policy_key.Open(HKEY_CURRENT_USER, kRegistrySubKey, KEY_READ) == + ERROR_SUCCESS) { + if (policy_key.ReadValueDW(value_name.c_str(), &value) == ERROR_SUCCESS) { + *result = value; + return true; + } } return false; } diff --git a/chrome/browser/rlz/rlz_unittest.cc b/chrome/browser/rlz/rlz_unittest.cc index f2ca69f..1b53bf6e4 100644 --- a/chrome/browser/rlz/rlz_unittest.cc +++ b/chrome/browser/rlz/rlz_unittest.cc @@ -16,9 +16,10 @@ namespace { // is nothing to clean. bool CleanValue(const wchar_t* key_name, const wchar_t* value) { RegKey key; - if (!key.Open(HKEY_CURRENT_USER, key_name, KEY_READ | KEY_WRITE)) + if (key.Open(HKEY_CURRENT_USER, key_name, KEY_READ | KEY_WRITE) != + ERROR_SUCCESS) return false; - EXPECT_TRUE(key.DeleteValue(value)); + EXPECT_EQ(ERROR_SUCCESS, key.DeleteValue(value)); return true; } @@ -33,8 +34,8 @@ TEST(RlzLibTest, RecordProductEvent) { rlz_lib::CHROME_OMNIBOX, rlz_lib::FIRST_SEARCH)); const wchar_t kEvent1[] = L"C1F"; RegKey key1; - EXPECT_TRUE(key1.Open(HKEY_CURRENT_USER, kKeyName, KEY_READ)); - EXPECT_TRUE(key1.ReadValueDW(kEvent1, &recorded_value)); + EXPECT_EQ(ERROR_SUCCESS, key1.Open(HKEY_CURRENT_USER, kKeyName, KEY_READ)); + EXPECT_EQ(ERROR_SUCCESS, key1.ReadValueDW(kEvent1, &recorded_value)); EXPECT_EQ(1, recorded_value); EXPECT_TRUE(CleanValue(kKeyName, kEvent1)); @@ -42,9 +43,9 @@ TEST(RlzLibTest, RecordProductEvent) { rlz_lib::CHROME_HOME_PAGE, rlz_lib::SET_TO_GOOGLE)); const wchar_t kEvent2[] = L"C2S"; RegKey key2; - EXPECT_TRUE(key2.Open(HKEY_CURRENT_USER, kKeyName, KEY_READ)); + EXPECT_EQ(ERROR_SUCCESS, key2.Open(HKEY_CURRENT_USER, kKeyName, KEY_READ)); DWORD value = 0; - EXPECT_TRUE(key2.ReadValueDW(kEvent2, &recorded_value)); + EXPECT_EQ(ERROR_SUCCESS, key2.ReadValueDW(kEvent2, &recorded_value)); EXPECT_EQ(1, recorded_value); EXPECT_TRUE(CleanValue(kKeyName, kEvent2)); } @@ -55,8 +56,8 @@ TEST(RlzLibTest, CleanProductEvents) { rlz_lib::CHROME_OMNIBOX, rlz_lib::FIRST_SEARCH)); const wchar_t kEvent1[] = L"C1F"; RegKey key1; - EXPECT_TRUE(key1.Open(HKEY_CURRENT_USER, kKeyName, KEY_READ)); - EXPECT_TRUE(key1.ReadValueDW(kEvent1, &recorded_value)); + EXPECT_EQ(ERROR_SUCCESS, key1.Open(HKEY_CURRENT_USER, kKeyName, KEY_READ)); + EXPECT_EQ(ERROR_SUCCESS, key1.ReadValueDW(kEvent1, &recorded_value)); EXPECT_EQ(1, recorded_value); EXPECT_TRUE(RLZTracker::ClearAllProductEvents(rlz_lib::CHROME)); diff --git a/chrome/browser/shell_integration_win.cc b/chrome/browser/shell_integration_win.cc index b79c2d7..470a9535 100644 --- a/chrome/browser/shell_integration_win.cc +++ b/chrome/browser/shell_integration_win.cc @@ -346,7 +346,7 @@ ShellIntegration::DefaultBrowserState ShellIntegration::IsDefaultBrowser() { std::wstring key_path(kChromeProtocols[i] + ShellUtil::kRegShellOpen); base::win::RegKey key(root_key, key_path.c_str(), KEY_READ); std::wstring value; - if (!key.Valid() || !key.ReadValue(L"", &value)) + if (!key.Valid() || (key.ReadValue(L"", &value) != ERROR_SUCCESS)) return NOT_DEFAULT_BROWSER; // Need to normalize path in case it's been munged. CommandLine command_line = CommandLine::FromString(value); @@ -377,7 +377,7 @@ bool ShellIntegration::IsFirefoxDefaultBrowser() { std::wstring app_cmd; base::win::RegKey key(HKEY_CURRENT_USER, ShellUtil::kRegVistaUrlPrefs, KEY_READ); - if (key.Valid() && key.ReadValue(L"Progid", &app_cmd) && + if (key.Valid() && (key.ReadValue(L"Progid", &app_cmd) == ERROR_SUCCESS) && app_cmd == L"FirefoxURL") ff_default = true; } else { @@ -385,7 +385,7 @@ bool ShellIntegration::IsFirefoxDefaultBrowser() { key_path.append(ShellUtil::kRegShellOpen); base::win::RegKey key(HKEY_CLASSES_ROOT, key_path.c_str(), KEY_READ); std::wstring app_cmd; - if (key.Valid() && key.ReadValue(L"", &app_cmd) && + if (key.Valid() && (key.ReadValue(L"", &app_cmd) == ERROR_SUCCESS) && std::wstring::npos != StringToLowerASCII(app_cmd).find(L"firefox")) ff_default = true; } diff --git a/chrome/browser/ui/views/external_protocol_dialog.cc b/chrome/browser/ui/views/external_protocol_dialog.cc index 30aa4d8..61f0d38 100644 --- a/chrome/browser/ui/views/external_protocol_dialog.cc +++ b/chrome/browser/ui/views/external_protocol_dialog.cc @@ -176,7 +176,7 @@ std::wstring ExternalProtocolDialog::GetApplicationForProtocol( std::wstring parameters = url_spec.substr(split_offset + 1, url_spec.length() - 1); std::wstring application_to_launch; - if (cmd_key.ReadValue(NULL, &application_to_launch)) { + if (cmd_key.ReadValue(NULL, &application_to_launch) == ERROR_SUCCESS) { ReplaceSubstringsAfterOffset(&application_to_launch, 0, L"%1", parameters); return application_to_launch; } else { diff --git a/chrome/browser/ui/views/shell_dialogs_win.cc b/chrome/browser/ui/views/shell_dialogs_win.cc index f1c59b4..710aac2 100644 --- a/chrome/browser/ui/views/shell_dialogs_win.cc +++ b/chrome/browser/ui/views/shell_dialogs_win.cc @@ -75,9 +75,9 @@ static bool GetRegistryDescriptionFromExtension(const std::wstring& file_ext, DCHECK(reg_description); base::win::RegKey reg_ext(HKEY_CLASSES_ROOT, file_ext.c_str(), KEY_READ); std::wstring reg_app; - if (reg_ext.ReadValue(NULL, ®_app) && !reg_app.empty()) { + if (reg_ext.ReadValue(NULL, ®_app) == ERROR_SUCCESS && !reg_app.empty()) { base::win::RegKey reg_link(HKEY_CLASSES_ROOT, reg_app.c_str(), KEY_READ); - if (reg_link.ReadValue(NULL, reg_description)) + if (reg_link.ReadValue(NULL, reg_description) == ERROR_SUCCESS) return true; } return false; diff --git a/chrome/common/chrome_plugin_lib.cc b/chrome/common/chrome_plugin_lib.cc index fb4ac83..e19a247 100644 --- a/chrome/common/chrome_plugin_lib.cc +++ b/chrome/common/chrome_plugin_lib.cc @@ -153,11 +153,11 @@ void ChromePluginLib::LoadChromePlugins(const CPBrowserFuncs* bfuncs) { reg_path.append(iter.Name()); base::win::RegKey key(HKEY_CURRENT_USER, reg_path.c_str()); - DWORD is_persistent; - if (key.ReadValueDW(kRegistryLoadOnStartup, &is_persistent) && - is_persistent) { + DWORD is_persistent = 0; + key.ReadValueDW(kRegistryLoadOnStartup, &is_persistent); + if (is_persistent) { std::wstring path; - if (key.ReadValue(kRegistryPath, &path)) { + if (key.ReadValue(kRegistryPath, &path) == ERROR_SUCCESS) { ChromePluginLib::Create(path, bfuncs); } } diff --git a/chrome/installer/setup/chrome_frame_ready_mode.cc b/chrome/installer/setup/chrome_frame_ready_mode.cc index 6e980bb..26584d2 100644 --- a/chrome/installer/setup/chrome_frame_ready_mode.cc +++ b/chrome/installer/setup/chrome_frame_ready_mode.cc @@ -83,20 +83,18 @@ InstallStatus ChromeFrameReadyModeOptIn(const InstallerState& installer_state, // Add a work item to delete the ChromeFrameReadyMode registry value. HKEY root = system_install ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER; item_list->AddDeleteRegValueWorkItem(root, package_properties.GetStateKey(), - kChromeFrameReadyModeField, REG_QWORD); + kChromeFrameReadyModeField); // Delete the command elevation registry keys std::wstring version_key(cf->GetVersionKey()); item_list->AddDeleteRegValueWorkItem( - root, version_key, google_update::kRegCFTempOptOutCmdField, REG_SZ); + root, version_key, google_update::kRegCFTempOptOutCmdField); item_list->AddDeleteRegValueWorkItem( - root, version_key, google_update::kRegCFEndTempOptOutCmdField, REG_SZ); + root, version_key, google_update::kRegCFEndTempOptOutCmdField); item_list->AddDeleteRegValueWorkItem(root, version_key, - google_update::kRegCFOptOutCmdField, - REG_SZ); + google_update::kRegCFOptOutCmdField); item_list->AddDeleteRegValueWorkItem(root, version_key, - google_update::kRegCFOptInCmdField, - REG_SZ); + google_update::kRegCFOptInCmdField); if (!item_list->Do()) { LOG(ERROR) << "Failed to opt into GCF"; @@ -141,8 +139,7 @@ InstallStatus ChromeFrameReadyModeTempOptOut(const CommandLine& cmd_line) { while (values.Valid()) { const wchar_t* name = values.Name(); if (StartsWith(name, kChromeFramePrefix, true)) { - item_list->AddDeleteRegValueWorkItem(root, kPostPlatformUAKey, name, - REG_SZ); + item_list->AddDeleteRegValueWorkItem(root, kPostPlatformUAKey, name); } ++values; } diff --git a/chrome/installer/setup/install.cc b/chrome/installer/setup/install.cc index 0df4e69..dced77c 100644 --- a/chrome/installer/setup/install.cc +++ b/chrome/installer/setup/install.cc @@ -160,7 +160,7 @@ void AddGoogleUpdateWorkItems(const InstallationState& original_state, // Handle the case where the ClientState key doesn't exist by creating it. // This takes care of the multi-installer's package key, which is not // created by Google Update for us. - if (!key.Open(reg_root, kscan->c_str(), KEY_QUERY_VALUE) || + if (key.Open(reg_root, kscan->c_str(), KEY_QUERY_VALUE) != ERROR_SUCCESS || !other_info.Initialize(key)) { other_info.set_value(std::wstring()); } diff --git a/chrome/installer/setup/install_worker.cc b/chrome/installer/setup/install_worker.cc index e945bdb..f9a534a 100644 --- a/chrome/installer/setup/install_worker.cc +++ b/chrome/installer/setup/install_worker.cc @@ -343,8 +343,8 @@ void AddGoogleUpdateWorkItems(const InstallationState& original_state, // Handle the case where the ClientState key doesn't exist by creating it. // This takes care of the multi-installer's package key, which is not // created by Google Update for us. - if (!key.Open(reg_root, kscan->c_str(), KEY_QUERY_VALUE) || - !other_info.Initialize(key)) { + if ((key.Open(reg_root, kscan->c_str(), KEY_QUERY_VALUE) != ERROR_SUCCESS) + || (!other_info.Initialize(key))) { other_info.set_value(std::wstring()); } if (!other_info.Equals(channel_info)) { @@ -502,11 +502,9 @@ bool AppendPostInstallTasks(bool multi_install, BrowserDistribution* dist = products[i]->distribution(); std::wstring version_key(dist->GetVersionKey()); regular_update_work_items->AddDeleteRegValueWorkItem(root, version_key, - google_update::kRegOldVersionField, - true); + google_update::kRegOldVersionField); regular_update_work_items->AddDeleteRegValueWorkItem(root, version_key, - google_update::kRegRenameCmdField, - true); + google_update::kRegRenameCmdField); } post_install_task_list->AddWorkItem(regular_update_work_items.release()); @@ -790,7 +788,7 @@ void AddChromeFrameWorkItems(bool install, KEY_QUERY_VALUE).Valid()) { list->AddDeleteRegValueWorkItem(root, product.package().properties()->GetStateKey(), - installer::kChromeFrameReadyModeField, REG_QWORD); + installer::kChromeFrameReadyModeField); } const Product* chrome = installer::FindProduct(product.package().products(), @@ -812,16 +810,13 @@ void AddChromeFrameWorkItems(bool install, if (!ready_mode || !install) { list->AddDeleteRegValueWorkItem(root, version_key, - google_update::kRegCFTempOptOutCmdField, - REG_SZ); + google_update::kRegCFTempOptOutCmdField); list->AddDeleteRegValueWorkItem(root, version_key, - google_update::kRegCFEndTempOptOutCmdField, - REG_SZ); + google_update::kRegCFEndTempOptOutCmdField); list->AddDeleteRegValueWorkItem(root, version_key, - google_update::kRegCFOptOutCmdField, - REG_SZ); + google_update::kRegCFOptOutCmdField); list->AddDeleteRegValueWorkItem(root, version_key, - google_update::kRegCFOptInCmdField, REG_SZ); + google_update::kRegCFOptInCmdField); } if (update_chrome_uninstall_command) { diff --git a/chrome/installer/setup/setup_main.cc b/chrome/installer/setup/setup_main.cc index 5158bc0..b385b8b 100644 --- a/chrome/installer/setup/setup_main.cc +++ b/chrome/installer/setup/setup_main.cc @@ -167,12 +167,10 @@ installer::InstallStatus RenameChromeExecutables( std::wstring version_key(browser_dist->GetVersionKey()); install_list->AddDeleteRegValueWorkItem(reg_root, version_key, - google_update::kRegOldVersionField, - REG_SZ); + google_update::kRegOldVersionField); install_list->AddDeleteRegValueWorkItem(reg_root, version_key, - google_update::kRegRenameCmdField, - REG_SZ); + google_update::kRegRenameCmdField); } installer::InstallStatus ret = installer::RENAME_SUCCESSFUL; if (!install_list->Do()) { diff --git a/chrome/installer/setup/uninstall.cc b/chrome/installer/setup/uninstall.cc index a886e3b..e21cc01 100644 --- a/chrome/installer/setup/uninstall.cc +++ b/chrome/installer/setup/uninstall.cc @@ -181,7 +181,7 @@ bool CurrentUserHasDefaultBrowser(const Product& product) { ShellUtil::kRegShellOpen); RegKey key(HKEY_LOCAL_MACHINE, reg_key.c_str(), KEY_READ); std::wstring reg_exe; - if (key.ReadValue(L"", ®_exe) && reg_exe.length() > 2) { + if (key.ReadValue(L"", ®_exe) == ERROR_SUCCESS && reg_exe.length() > 2) { FilePath chrome_exe(product.package().path() .Append(installer::kChromeExe)); // The path in the registry will always have quotes. 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"", ®istry_chrome_exe) || + if ((key.ReadValue(L"", ®istry_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)); } diff --git a/chrome/test/mini_installer_test/chrome_mini_installer.cc b/chrome/test/mini_installer_test/chrome_mini_installer.cc index 20edf54..1f52295 100644 --- a/chrome/test/mini_installer_test/chrome_mini_installer.cc +++ b/chrome/test/mini_installer_test/chrome_mini_installer.cc @@ -365,8 +365,9 @@ bool ChromeMiniInstaller::CloseChromeBrowser() { // Checks for Chrome registry keys. bool ChromeMiniInstaller::CheckRegistryKey(const std::wstring& key_path) { RegKey key; - if (!key.Open(GetRootRegistryKey(), key_path.c_str(), KEY_ALL_ACCESS)) { - printf("Cannot open reg key\n"); + LONG ret = key.Open(GetRootRegistryKey(), key_path.c_str(), KEY_ALL_ACCESS); + if (ret != ERROR_SUCCESS) { + printf("Cannot open reg key. error: %d\n", ret); return false; } std::wstring reg_key_value_returned; @@ -380,7 +381,8 @@ bool ChromeMiniInstaller::CheckRegistryKeyOnUninstall( const std::wstring& key_path) { RegKey key; int timer = 0; - while ((key.Open(GetRootRegistryKey(), key_path.c_str(), KEY_ALL_ACCESS)) && + while ((key.Open(GetRootRegistryKey(), key_path.c_str(), + KEY_ALL_ACCESS) == ERROR_SUCCESS) && (timer < 20000)) { base::PlatformThread::Sleep(200); timer = timer + 200; @@ -444,8 +446,9 @@ void ChromeMiniInstaller::DeletePvRegistryKey() { pv_key.append(dist->GetAppGuid()); RegKey key; - if (key.Open(GetRootRegistryKey(), pv_key.c_str(), KEY_ALL_ACCESS)) - ASSERT_TRUE(key.DeleteValue(L"pv")); + if (key.Open(GetRootRegistryKey(), pv_key.c_str(), KEY_ALL_ACCESS) == + ERROR_SUCCESS) + ASSERT_EQ(ERROR_SUCCESS, key.DeleteValue(L"pv")); printf("Deleted %ls key\n", pv_key.c_str()); } @@ -523,8 +526,9 @@ bool ChromeMiniInstaller::GetChromeVersionFromRegistry( std::wstring* build_key_value) { BrowserDistribution* dist = BrowserDistribution::GetDistribution(); RegKey key(GetRootRegistryKey(), dist->GetVersionKey().c_str(), KEY_READ); - if (!key.ReadValue(L"pv", build_key_value)) { - printf("registry key not found\n"); + LONG result = key.ReadValue(L"pv", build_key_value); + if (result != ERROR_SUCCESS) { + printf("registry read for chrome version failed. error: %d\n", result); return false; } printf("Build key value is %ls\n\n", build_key_value->c_str()); diff --git a/chrome/test/plugin/plugin_test.cpp b/chrome/test/plugin/plugin_test.cpp index 2005b66..f7df668 100644 --- a/chrome/test/plugin/plugin_test.cpp +++ b/chrome/test/plugin/plugin_test.cpp @@ -85,7 +85,7 @@ class PluginTest : public UITest { base::win::RegKey regkey; if (regkey.Open(HKEY_LOCAL_MACHINE, L"Software\\Microsoft\\MediaPlayer\\ShimInclusionList", - KEY_WRITE)) { + KEY_WRITE) == ERROR_SUCCESS) { regkey.CreateKey(L"CHROME.EXE", KEY_READ); } } else if (strcmp(test_info->name(), "MediaPlayerOld") == 0) { diff --git a/chrome_frame/chrome_tab.cc b/chrome_frame/chrome_tab.cc index 280e0a4..bc4a7d1 100644 --- a/chrome_frame/chrome_tab.cc +++ b/chrome_frame/chrome_tab.cc @@ -324,16 +324,16 @@ HRESULT SetupRunOnce() { } RegKey run_once; - if (run_once.Create(hive, kRunOnce, KEY_READ | KEY_WRITE)) { + LONG ret = run_once.Create(hive, kRunOnce, KEY_READ | KEY_WRITE); + if (ret == ERROR_SUCCESS) { CommandLine run_once_cmd(chrome_launcher::GetChromeExecutablePath()); run_once_cmd.AppendSwitchASCII(switches::kAutomationClientChannelID, "0"); run_once_cmd.AppendSwitch(switches::kChromeFrame); - if (run_once.WriteValue(L"A", - run_once_cmd.command_line_string().c_str())) { - result = S_OK; - } + ret = run_once.WriteValue(L"A", + run_once_cmd.command_line_string().c_str()); } + result = HRESULT_FROM_WIN32(ret); } else { result = S_FALSE; } @@ -434,7 +434,8 @@ HRESULT SetChromeFrameUA(bool is_system, const wchar_t* value) { HKEY parent_hive = is_system ? HKEY_LOCAL_MACHINE : HKEY_CURRENT_USER; RegKey ua_key; - if (ua_key.Create(parent_hive, kPostPlatformUAKey, KEY_READ | KEY_WRITE)) { + if (ua_key.Create(parent_hive, kPostPlatformUAKey, + KEY_READ | KEY_WRITE) == ERROR_SUCCESS) { // Make sure that we unregister ChromeFrame UA strings registered previously wchar_t value_name[MAX_PATH + 1] = {}; wchar_t value_data[MAX_PATH + 1] = {}; @@ -654,7 +655,7 @@ class SecurityDescBackup { RegKey backup_key(HKEY_LOCAL_MACHINE, backup_key_name_.c_str(), KEY_READ | KEY_WRITE); if (backup_key.Valid()) { - return backup_key.WriteValue(NULL, str.GetString()); + return backup_key.WriteValue(NULL, str.GetString()) == ERROR_SUCCESS; } return false; @@ -696,15 +697,15 @@ class SecurityDescBackup { DWORD len = 0; DWORD reg_type = REG_NONE; - if (!backup_key.ReadValue(NULL, NULL, &len, ®_type)) + if (backup_key.ReadValue(NULL, NULL, &len, ®_type) != ERROR_SUCCESS) return false; if (reg_type != REG_SZ) return false; size_t wchar_count = 1 + len / sizeof(wchar_t); - if (!backup_key.ReadValue(NULL, WriteInto(sddl, wchar_count), &len, - ®_type)) { + if (backup_key.ReadValue(NULL, WriteInto(sddl, wchar_count), &len, + ®_type) != ERROR_SUCCESS) { return false; } @@ -760,16 +761,17 @@ static bool SetOrDeleteMimeHandlerKey(bool set, HKEY root_key) { if (!key.Valid()) return false; - bool result; + LONG result1 = ERROR_SUCCESS; + LONG result2 = ERROR_SUCCESS; if (set) { - result = key.WriteValue(L"ChromeTab.ChromeActiveDocument", 1); - result = key.WriteValue(L"ChromeTab.ChromeActiveDocument.1", 1) && result; + result1 = key.WriteValue(L"ChromeTab.ChromeActiveDocument", 1); + result2 = key.WriteValue(L"ChromeTab.ChromeActiveDocument.1", 1); } else { - result = key.DeleteValue(L"ChromeTab.ChromeActiveDocument"); - result = key.DeleteValue(L"ChromeTab.ChromeActiveDocument.1") && result; + result1 = key.DeleteValue(L"ChromeTab.ChromeActiveDocument"); + result2 = key.DeleteValue(L"ChromeTab.ChromeActiveDocument.1"); } - return result; + return (result2 == ERROR_SUCCESS) && (result2 == ERROR_SUCCESS); } bool RegisterSecuredMimeHandler(bool enable, bool is_system) { diff --git a/chrome_frame/crash_reporting/crash_metrics.cc b/chrome_frame/crash_reporting/crash_metrics.cc index 2759e5d..8909162 100644 --- a/chrome_frame/crash_reporting/crash_metrics.cc +++ b/chrome_frame/crash_reporting/crash_metrics.cc @@ -30,16 +30,19 @@ bool CrashMetricsReporter::SetMetric(Metric metric, int value) { DCHECK(metric >= NAVIGATION_COUNT && metric <= LAST_METRIC); base::win::RegKey metric_key; - if (metric_key.Create(HKEY_CURRENT_USER, kChromeFrameMetricsKey, - KEY_SET_VALUE)) { - if (metric_key.WriteValue(g_metric_names[metric], value)) { + LONG result = metric_key.Create(HKEY_CURRENT_USER, kChromeFrameMetricsKey, + KEY_SET_VALUE); + if (result == ERROR_SUCCESS) { + result = metric_key.WriteValue(g_metric_names[metric], value); + if (result == ERROR_SUCCESS) { return true; } else { DLOG(ERROR) << "Failed to read ChromeFrame metric:" - << g_metric_names[metric]; + << g_metric_names[metric] << " error: " << result; } } else { - DLOG(ERROR) << "Failed to create ChromeFrame metrics key"; + DLOG(ERROR) << "Failed to create ChromeFrame metrics key. error: " + << result; } return false; } @@ -50,13 +53,11 @@ int CrashMetricsReporter::GetMetric(Metric metric) { int ret = 0; base::win::RegKey metric_key; if (metric_key.Open(HKEY_CURRENT_USER, kChromeFrameMetricsKey, - KEY_QUERY_VALUE)) { - int value = 0; - if (metric_key.ReadValueDW(g_metric_names[metric], - reinterpret_cast<DWORD*>(&value))) { - ret = value; - } + KEY_QUERY_VALUE) == ERROR_SUCCESS) { + metric_key.ReadValueDW(g_metric_names[metric], + reinterpret_cast<DWORD*>(&ret)); } + return ret; } diff --git a/chrome_frame/policy_settings.cc b/chrome_frame/policy_settings.cc index 50e465f..116052b 100644 --- a/chrome_frame/policy_settings.cc +++ b/chrome_frame/policy_settings.cc @@ -70,9 +70,11 @@ void PolicySettings::ReadUrlSettings( std::wstring settings_value( ASCIIToWide(policy::key::kChromeFrameRendererSettings)); for (int i = 0; i < arraysize(kRootKeys); ++i) { - if (config_key.Open(kRootKeys[i], policy::kRegistrySubKey, KEY_READ) && - config_key.ReadValueDW(settings_value.c_str(), &value)) { - break; + if ((config_key.Open(kRootKeys[i], policy::kRegistrySubKey, + KEY_READ) == ERROR_SUCCESS) && + (config_key.ReadValueDW(settings_value.c_str(), + &value) == ERROR_SUCCESS)) { + break; } } @@ -124,10 +126,11 @@ void PolicySettings::ReadApplicationLocaleSetting( std::wstring application_locale_value( ASCIIToWide(policy::key::kApplicationLocaleValue)); for (int i = 0; i < arraysize(kRootKeys); ++i) { - if (config_key.Open(kRootKeys[i], policy::kRegistrySubKey, KEY_READ) && - config_key.ReadValue(application_locale_value.c_str(), - application_locale)) { - break; + if ((config_key.Open(kRootKeys[i], policy::kRegistrySubKey, + KEY_READ) == ERROR_SUCCESS) && + (config_key.ReadValue(application_locale_value.c_str(), + application_locale) == ERROR_SUCCESS)) { + break; } } } diff --git a/chrome_frame/ready_mode/internal/registry_ready_mode_state.cc b/chrome_frame/ready_mode/internal/registry_ready_mode_state.cc index 244d63f5..880fd51 100644 --- a/chrome_frame/ready_mode/internal/registry_ready_mode_state.cc +++ b/chrome_frame/ready_mode/internal/registry_ready_mode_state.cc @@ -34,9 +34,11 @@ HANDLE LaunchCommandDirectly(const std::wstring& command_field) { for (int i = 0; i < arraysize(roots); i++) { base::win::RegKey version_key; - if (version_key.Open(roots[i], version_key_name.c_str(), KEY_QUERY_VALUE)) { + if (version_key.Open(roots[i], version_key_name.c_str(), + KEY_QUERY_VALUE) == ERROR_SUCCESS) { std::wstring command_line; - if (version_key.ReadValue(command_field.c_str(), &command_line)) { + if (version_key.ReadValue(command_field.c_str(), + &command_line) == ERROR_SUCCESS) { HANDLE launched_process = NULL; if (base::LaunchApp(command_line, false, true, &launched_process)) { return launched_process; @@ -186,34 +188,24 @@ bool RegistryReadyModeState::GetValue(int64* value, bool* exists) { *exists = false; *value = 0; - HKEY roots[] = {HKEY_CURRENT_USER, HKEY_LOCAL_MACHINE}; - + HKEY roots[] = { HKEY_CURRENT_USER, HKEY_LOCAL_MACHINE }; + LONG result = ERROR_SUCCESS; for (int i = 0; i < arraysize(roots); i++) { base::win::RegKey config_key; - - if (config_key.Open(roots[i], key_name_.c_str(), KEY_QUERY_VALUE)) { - if (config_key.ValueExists(installer::kChromeFrameReadyModeField)) { - int64 temp; - DWORD value_size = sizeof(temp); - DWORD type = 0; - if (!config_key.ReadValue(installer::kChromeFrameReadyModeField, - &temp, &value_size, &type)) { - DLOG(ERROR) << "Failed to read from registry key " << key_name_ - << " and value " << installer::kChromeFrameReadyModeField; - return false; - } - - if (value_size != sizeof(temp) || type != REG_QWORD) { - DLOG(ERROR) << "Unexpected state found under registry key " - << key_name_ << " and value " - << installer::kChromeFrameReadyModeField; - return false; - } - - *value = temp; + result = config_key.Open(roots[i], key_name_.c_str(), KEY_QUERY_VALUE); + if (result == ERROR_SUCCESS) { + result = config_key.ReadInt64(installer::kChromeFrameReadyModeField, + value); + if (result == ERROR_SUCCESS) { *exists = true; return true; } + if (result != ERROR_FILE_NOT_FOUND) { + DLOG(ERROR) << "Failed to read from registry key " << key_name_ + << " and value " << installer::kChromeFrameReadyModeField + << " error: " << result; + return false; + } } } diff --git a/chrome_frame/test/chrome_frame_test_utils.cc b/chrome_frame/test/chrome_frame_test_utils.cc index e464d6f..beeba70 100644 --- a/chrome_frame/test/chrome_frame_test_utils.cc +++ b/chrome_frame/test/chrome_frame_test_utils.cc @@ -633,8 +633,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())); } @@ -647,7 +647,7 @@ TempRegKeyOverride::~TempRegKeyOverride() { // static void TempRegKeyOverride::DeleteAllTempKeys() { base::win::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); } } diff --git a/chrome_frame/test/perf/chrome_frame_perftest.cc b/chrome_frame/test/perf/chrome_frame_perftest.cc index e59c843..2f645d2 100644 --- a/chrome_frame/test/perf/chrome_frame_perftest.cc +++ b/chrome_frame/test/perf/chrome_frame_perftest.cc @@ -1105,7 +1105,7 @@ TEST_F(FlashCreationTest, PerfCold) { base::win::RegKey flash_key(HKEY_CLASSES_ROOT, kFlashControlKey, KEY_READ); std::wstring plugin_path; - ASSERT_TRUE(flash_key.ReadValue(L"", &plugin_path)); + ASSERT_EQ(ERROR_SUCCESS, flash_key.ReadValue(L"", &plugin_path)); ASSERT_FALSE(plugin_path.empty()); FilePath flash_path = FilePath::FromWStringHack(plugin_path); @@ -1126,7 +1126,7 @@ TEST_F(SilverlightCreationTest, DISABLED_PerfCold) { KEY_READ); std::wstring plugin_path; - ASSERT_TRUE(silverlight_key.ReadValue(L"", &plugin_path)); + ASSERT_EQ(ERROR_SUCCESS, silverlight_key.ReadValue(L"", &plugin_path)); ASSERT_FALSE(plugin_path.empty()); FilePath silverlight_path = FilePath::FromWStringHack(plugin_path); diff --git a/chrome_frame/test/policy_settings_unittest.cc b/chrome_frame/test/policy_settings_unittest.cc index 771762f..c5d7c50 100644 --- a/chrome_frame/test/policy_settings_unittest.cc +++ b/chrome_frame/test/policy_settings_unittest.cc @@ -23,7 +23,8 @@ namespace { // A best effort way to zap CF policy entries that may be in the registry. void DeleteChromeFramePolicyEntries(HKEY root) { RegKey key; - if (key.Open(root, policy::kRegistrySubKey, KEY_ALL_ACCESS)) { + if (key.Open(root, policy::kRegistrySubKey, + KEY_ALL_ACCESS) == ERROR_SUCCESS) { key.DeleteValue( ASCIIToWide(policy::key::kChromeFrameRendererSettings).c_str()); key.DeleteKey(ASCIIToWide(policy::key::kRenderInChromeFrameList).c_str()); @@ -34,8 +35,8 @@ void DeleteChromeFramePolicyEntries(HKEY root) { } bool InitializePolicyKey(HKEY policy_root, RegKey* policy_key) { - EXPECT_TRUE(policy_key->Create(policy_root, policy::kRegistrySubKey, - KEY_ALL_ACCESS)); + EXPECT_EQ(ERROR_SUCCESS, policy_key->Create(policy_root, + policy::kRegistrySubKey, KEY_ALL_ACCESS)); return policy_key->Valid(); } @@ -46,10 +47,11 @@ void WritePolicyList(RegKey* policy_key, const wchar_t* list_name, policy_key->DeleteKey(list_name); RegKey list_key; - EXPECT_TRUE(list_key.Create(policy_key->Handle(), list_name, KEY_ALL_ACCESS)); + EXPECT_EQ(ERROR_SUCCESS, list_key.Create(policy_key->Handle(), list_name, + KEY_ALL_ACCESS)); for (int i = 0; i < count; ++i) { - EXPECT_TRUE(list_key.WriteValue(base::StringPrintf(L"%i", i).c_str(), - values[i])); + EXPECT_EQ(ERROR_SUCCESS, + list_key.WriteValue(base::StringPrintf(L"%i", i).c_str(), values[i])); } } @@ -94,7 +96,8 @@ bool SetChromeApplicationLocale(HKEY policy_root, const wchar_t* locale) { std::wstring application_locale_value( ASCIIToWide(policy::key::kApplicationLocaleValue)); - EXPECT_TRUE(policy_key.WriteValue(application_locale_value.c_str(), locale)); + EXPECT_EQ(ERROR_SUCCESS, + policy_key.WriteValue(application_locale_value.c_str(), locale)); return true; } diff --git a/chrome_frame/utils.cc b/chrome_frame/utils.cc index c23080c..1445ccb 100644 --- a/chrome_frame/utils.cc +++ b/chrome_frame/utils.cc @@ -233,7 +233,8 @@ bool UtilChangePersistentNPAPIMarker(bool set) { bool success = false; if (cf_state_key.Valid()) { if (set) { - success = cf_state_key.WriteValue(kChromeFramePersistNPAPIReg, 1); + success = (cf_state_key.WriteValue(kChromeFramePersistNPAPIReg, 1) == + ERROR_SUCCESS); } else { // Unfortunately, DeleteValue returns true only if the value // previously existed, so we do a separate existence check to @@ -255,7 +256,8 @@ bool UtilIsPersistentNPAPIMarkerSet() { bool success = false; if (cf_state_key.Valid()) { DWORD val = 0; - if (cf_state_key.ReadValueDW(kChromeFramePersistNPAPIReg, &val)) { + if (cf_state_key.ReadValueDW(kChromeFramePersistNPAPIReg, &val) == + ERROR_SUCCESS) { success = (val != 0); } } @@ -676,11 +678,8 @@ int GetConfigInt(int default_value, const wchar_t* value_name) { int ret = default_value; RegKey config_key; if (config_key.Open(HKEY_CURRENT_USER, kChromeFrameConfigKey, - KEY_QUERY_VALUE)) { - int value = FALSE; - if (config_key.ReadValueDW(value_name, reinterpret_cast<DWORD*>(&value))) { - ret = value; - } + KEY_QUERY_VALUE) == ERROR_SUCCESS) { + config_key.ReadValueDW(value_name, reinterpret_cast<DWORD*>(&ret)); } return ret; @@ -694,8 +693,8 @@ bool GetConfigBool(bool default_value, const wchar_t* value_name) { bool SetConfigInt(const wchar_t* value_name, int value) { RegKey config_key; if (config_key.Create(HKEY_CURRENT_USER, kChromeFrameConfigKey, - KEY_SET_VALUE)) { - if (config_key.WriteValue(value_name, value)) { + KEY_SET_VALUE) == ERROR_SUCCESS) { + if (config_key.WriteValue(value_name, value) == ERROR_SUCCESS) { return true; } } @@ -710,8 +709,10 @@ bool SetConfigBool(const wchar_t* value_name, bool value) { bool DeleteConfigValue(const wchar_t* value_name) { RegKey config_key; if (config_key.Open(HKEY_CURRENT_USER, kChromeFrameConfigKey, - KEY_WRITE)) { - return config_key.DeleteValue(value_name); + KEY_WRITE) == ERROR_SUCCESS) { + if (config_key.DeleteValue(value_name) == ERROR_SUCCESS) { + return true; + } } return false; } @@ -728,7 +729,8 @@ bool IsGcfDefaultRenderer() { // TODO(tommi): Implement caching for this config value as it gets // checked frequently. RegKey config_key; - if (config_key.Open(HKEY_CURRENT_USER, kChromeFrameConfigKey, KEY_READ)) { + if (config_key.Open(HKEY_CURRENT_USER, kChromeFrameConfigKey, + KEY_READ) == ERROR_SUCCESS) { config_key.ReadValueDW(kEnableGCFRendererByDefault, &is_default); } } @@ -751,8 +753,10 @@ RendererType RendererTypeForUrl(const std::wstring& url) { } RegKey config_key; - if (!config_key.Open(HKEY_CURRENT_USER, kChromeFrameConfigKey, KEY_READ)) + if (config_key.Open(HKEY_CURRENT_USER, kChromeFrameConfigKey, + KEY_READ) != ERROR_SUCCESS) { return RENDERER_TYPE_UNDETERMINED; + } RendererType renderer_type = RENDERER_TYPE_UNDETERMINED; diff --git a/net/base/platform_mime_util_win.cc b/net/base/platform_mime_util_win.cc index b93008b..f9a9391 100644 --- a/net/base/platform_mime_util_win.cc +++ b/net/base/platform_mime_util_win.cc @@ -28,8 +28,8 @@ bool PlatformMimeUtil::GetPlatformMimeTypeFromExtension( bool PlatformMimeUtil::GetPreferredExtensionForMimeType( const std::string& mime_type, FilePath::StringType* ext) const { std::wstring key(L"MIME\\Database\\Content Type\\" + UTF8ToWide(mime_type)); - if (!base::win::RegKey(HKEY_CLASSES_ROOT, key.c_str(), KEY_READ).ReadValue( - L"Extension", ext)) { + if (base::win::RegKey(HKEY_CLASSES_ROOT, key.c_str(), KEY_READ).ReadValue( + L"Extension", ext) != ERROR_SUCCESS) { return false; } // Strip off the leading dot, this should always be the case. diff --git a/net/base/ssl_config_service_win.cc b/net/base/ssl_config_service_win.cc index ca20e79..dbdedb4 100644 --- a/net/base/ssl_config_service_win.cc +++ b/net/base/ssl_config_service_win.cc @@ -63,17 +63,15 @@ bool SSLConfigServiceWin::GetSSLConfigNow(SSLConfig* config) { // http://crbug.com/61455 base::ThreadRestrictions::ScopedAllowIO allow_io; RegKey internet_settings; - if (!internet_settings.Open(HKEY_CURRENT_USER, kInternetSettingsSubKeyName, - KEY_READ)) + if (internet_settings.Open(HKEY_CURRENT_USER, kInternetSettingsSubKeyName, + KEY_READ) != ERROR_SUCCESS) return false; - DWORD revocation; - if (!internet_settings.ReadValueDW(kRevocationValueName, &revocation)) - revocation = REVOCATION_DEFAULT; + DWORD revocation = REVOCATION_DEFAULT; + internet_settings.ReadValueDW(kRevocationValueName, &revocation); - DWORD protocols; - if (!internet_settings.ReadValueDW(kProtocolsValueName, &protocols)) - protocols = PROTOCOLS_DEFAULT; + DWORD protocols = PROTOCOLS_DEFAULT; + internet_settings.ReadValueDW(kProtocolsValueName, &protocols); config->rev_checking_enabled = (revocation != 0); config->ssl3_enabled = ((protocols & SSL3) != 0); @@ -120,9 +118,9 @@ void SSLConfigServiceWin::SetSSLVersionEnabled(int version, bool enabled) { base::ThreadRestrictions::ScopedAllowIO allow_io; RegKey internet_settings(HKEY_CURRENT_USER, kInternetSettingsSubKeyName, KEY_READ | KEY_WRITE); - DWORD value; - if (!internet_settings.ReadValueDW(kProtocolsValueName, &value)) - value = PROTOCOLS_DEFAULT; + DWORD value = PROTOCOLS_DEFAULT; + internet_settings.ReadValueDW(kProtocolsValueName, &value); + if (enabled) value |= version; else diff --git a/net/proxy/proxy_config_service_win.cc b/net/proxy/proxy_config_service_win.cc index 849fc24..3c2fbff 100644 --- a/net/proxy/proxy_config_service_win.cc +++ b/net/proxy/proxy_config_service_win.cc @@ -42,7 +42,7 @@ class ProxyConfigServiceWin::KeyEntry { bool StartWatching(base::win::ObjectWatcher::Delegate* delegate) { // Try to create a watch event for the registry key (which watches the // sibling tree as well). - if (!key_.StartWatching()) + if (key_.StartWatching() != ERROR_SUCCESS) return false; // Now setup an ObjectWatcher for this event, so we get OnObjectSignaled() @@ -54,7 +54,7 @@ class ProxyConfigServiceWin::KeyEntry { } bool CreateRegKey(HKEY rootkey, const wchar_t* subkey) { - return key_.Create(rootkey, subkey, KEY_NOTIFY); + return key_.Create(rootkey, subkey, KEY_NOTIFY) == ERROR_SUCCESS; } HANDLE watch_event() const { diff --git a/webkit/plugins/npapi/plugin_list_win.cc b/webkit/plugins/npapi/plugin_list_win.cc index d7f13ef..324dffb 100644 --- a/webkit/plugins/npapi/plugin_list_win.cc +++ b/webkit/plugins/npapi/plugin_list_win.cc @@ -73,7 +73,7 @@ bool GetInstalledPath(const char16* app, FilePath* out) { base::win::RegKey key(HKEY_LOCAL_MACHINE, reg_path.c_str(), KEY_READ); std::wstring path; - if (key.ReadValue(kRegistryPath, &path)) { + if (key.ReadValue(kRegistryPath, &path) == ERROR_SUCCESS) { *out = FilePath(path); return true; } @@ -95,7 +95,7 @@ void GetPluginsInRegistryDirectory( base::win::RegKey key(root_key, reg_path.c_str(), KEY_READ); std::wstring path; - if (key.ReadValue(kRegistryPath, &path)) + if (key.ReadValue(kRegistryPath, &path) == ERROR_SUCCESS) plugin_dirs->insert(FilePath(path)); } } @@ -110,7 +110,7 @@ void GetFirefoxInstalledPaths(std::vector<FilePath>* out) { it.Name() + L"\\Main"; base::win::RegKey key(HKEY_LOCAL_MACHINE, full_path.c_str(), KEY_READ); std::wstring install_dir; - if (!key.ReadValue(L"Install Directory", &install_dir)) + if (key.ReadValue(L"Install Directory", &install_dir) != ERROR_SUCCESS) continue; out->push_back(FilePath(install_dir)); } @@ -190,8 +190,10 @@ void GetJavaDirectory(std::set<FilePath>* plugin_dirs) { // 2. Read the current Java version std::wstring java_version; - if (!java_key.ReadValue(kRegistryBrowserJavaVersion, &java_version)) + if (java_key.ReadValue(kRegistryBrowserJavaVersion, &java_version) != + ERROR_SUCCESS) { java_key.ReadValue(kRegistryCurrentJavaVersion, &java_version); + } if (!java_version.empty()) { java_key.OpenKey(java_version.c_str(), KEY_QUERY_VALUE); @@ -199,7 +201,8 @@ void GetJavaDirectory(std::set<FilePath>* plugin_dirs) { // 3. Install path of the JRE binaries is specified in "JavaHome" // value under the Java version key. std::wstring java_plugin_directory; - if (java_key.ReadValue(kRegistryJavaHome, &java_plugin_directory)) { + if (java_key.ReadValue(kRegistryJavaHome, &java_plugin_directory) == + ERROR_SUCCESS) { // 4. The new plugin resides under the 'bin/new_plugin' // subdirectory. DCHECK(!java_plugin_directory.empty()); diff --git a/webkit/plugins/npapi/webplugin_delegate_impl_win.cc b/webkit/plugins/npapi/webplugin_delegate_impl_win.cc index e281720..12cd216 100644 --- a/webkit/plugins/npapi/webplugin_delegate_impl_win.cc +++ b/webkit/plugins/npapi/webplugin_delegate_impl_win.cc @@ -418,9 +418,9 @@ bool WebPluginDelegateImpl::PlatformInitialize() { // for the rest patch this function. if ((quirks_ & PLUGIN_QUIRK_PATCH_REGENUMKEYEXW) && base::win::GetVersion() == base::win::VERSION_XP && - !base::win::RegKey().Open(HKEY_LOCAL_MACHINE, + (base::win::RegKey().Open(HKEY_LOCAL_MACHINE, L"SOFTWARE\\Microsoft\\MediaPlayer\\ShimInclusionList\\chrome.exe", - KEY_READ) && + KEY_READ) != ERROR_SUCCESS) && !g_iat_patch_reg_enum_key_ex_w.Pointer()->is_patched()) { g_iat_patch_reg_enum_key_ex_w.Pointer()->Patch( L"wmpdxm.dll", "advapi32.dll", "RegEnumKeyExW", |