diff options
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", |