summaryrefslogtreecommitdiffstats
path: root/chrome
diff options
context:
space:
mode:
authoramit@chromium.org <amit@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-01-19 07:28:46 +0000
committeramit@chromium.org <amit@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2011-01-19 07:28:46 +0000
commite06f4d5c6bd7bad162c45784e39cd0114635eb42 (patch)
treee53d6b4188af6e49393babc92a797ed5734a1026 /chrome
parent2b107a348f2b27934fe38680ec8010d743f61765 (diff)
downloadchromium_src-e06f4d5c6bd7bad162c45784e39cd0114635eb42.zip
chromium_src-e06f4d5c6bd7bad162c45784e39cd0114635eb42.tar.gz
chromium_src-e06f4d5c6bd7bad162c45784e39cd0114635eb42.tar.bz2
Regkey functions return error code instead of bool
Change the Regkey helper to consistently use and return LONG instead of bool. Fix RegKey usage all over the code base and get rid of workarounds due to lack of return value. Reviewers: brettw: everything (skip parts for other reviewers if you wish) robertshield,grt: chrome_frame, installer siggi: ceee BUG=none TEST=covered by existing tests Review URL: http://codereview.chromium.org/6090006 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@71768 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'chrome')
-rw-r--r--chrome/app/breakpad_win.cc7
-rw-r--r--chrome/browser/autofill/autofill_ie_toolbar_import_win.cc9
-rw-r--r--chrome/browser/autofill/autofill_ie_toolbar_import_win_unittest.cc4
-rw-r--r--chrome/browser/background_mode_manager_win.cc14
-rw-r--r--chrome/browser/enumerate_modules_model_win.cc4
-rw-r--r--chrome/browser/extensions/extension_rlz_apitest.cc8
-rw-r--r--chrome/browser/extensions/external_registry_extension_loader_win.cc8
-rw-r--r--chrome/browser/first_run/first_run_win.cc6
-rw-r--r--chrome/browser/importer/firefox_importer_utils_win.cc10
-rw-r--r--chrome/browser/importer/ie_importer.cc37
-rw-r--r--chrome/browser/plugin_service.cc4
-rw-r--r--chrome/browser/policy/configuration_policy_provider_delegate_win.cc57
-rw-r--r--chrome/browser/rlz/rlz_unittest.cc17
-rw-r--r--chrome/browser/shell_integration_win.cc6
-rw-r--r--chrome/browser/ui/views/external_protocol_dialog.cc2
-rw-r--r--chrome/browser/ui/views/shell_dialogs_win.cc4
-rw-r--r--chrome/common/chrome_plugin_lib.cc8
-rw-r--r--chrome/installer/setup/chrome_frame_ready_mode.cc15
-rw-r--r--chrome/installer/setup/install.cc2
-rw-r--r--chrome/installer/setup/install_worker.cc23
-rw-r--r--chrome/installer/setup/setup_main.cc6
-rw-r--r--chrome/installer/setup/uninstall.cc2
-rw-r--r--chrome/installer/util/channel_info.cc5
-rw-r--r--chrome/installer/util/create_reg_key_work_item.cc2
-rw-r--r--chrome/installer/util/create_reg_key_work_item_unittest.cc81
-rw-r--r--chrome/installer/util/delete_after_reboot_helper.cc7
-rw-r--r--chrome/installer/util/delete_reg_value_work_item.cc104
-rw-r--r--chrome/installer/util/delete_reg_value_work_item.h16
-rw-r--r--chrome/installer/util/delete_reg_value_work_item_unittest.cc31
-rw-r--r--chrome/installer/util/google_chrome_distribution.cc4
-rw-r--r--chrome/installer/util/google_update_settings.cc57
-rw-r--r--chrome/installer/util/google_update_settings_unittest.cc55
-rw-r--r--chrome/installer/util/install_util.cc43
-rw-r--r--chrome/installer/util/install_util_unittest.cc14
-rw-r--r--chrome/installer/util/installation_state.cc7
-rw-r--r--chrome/installer/util/package_properties_unittest.cc4
-rw-r--r--chrome/installer/util/package_unittest.cc7
-rw-r--r--chrome/installer/util/product.cc32
-rw-r--r--chrome/installer/util/product_unittest.cc9
-rw-r--r--chrome/installer/util/set_reg_value_work_item.cc207
-rw-r--r--chrome/installer/util/set_reg_value_work_item.h14
-rw-r--r--chrome/installer/util/set_reg_value_work_item_unittest.cc55
-rw-r--r--chrome/installer/util/shell_util.cc19
-rw-r--r--chrome/installer/util/work_item.cc6
-rw-r--r--chrome/installer/util/work_item.h3
-rw-r--r--chrome/installer/util/work_item_list.cc7
-rw-r--r--chrome/installer/util/work_item_list.h5
-rw-r--r--chrome/installer/util/work_item_list_unittest.cc41
-rw-r--r--chrome/test/mini_installer_test/chrome_mini_installer.cc18
-rw-r--r--chrome/test/plugin/plugin_test.cpp2
50 files changed, 525 insertions, 583 deletions
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, &current_value) &&
+ if ((read_key.ReadValue(key_name, &current_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, &reg_app) && !reg_app.empty()) {
+ if (reg_ext.ReadValue(NULL, &reg_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"", &reg_exe) && reg_exe.length() > 2) {
+ if (key.ReadValue(L"", &reg_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"", &registry_chrome_exe) ||
+ if ((key.ReadValue(L"", &registry_chrome_exe) != ERROR_SUCCESS) ||
registry_chrome_exe.length() < 2)
return false;
@@ -583,15 +583,16 @@ void ShellUtil::GetRegisteredBrowsers(BrowserDistribution* dist,
RegKey capabilities(root, (key + L"\\Capabilities").c_str(), KEY_READ);
std::wstring name;
if (!capabilities.Valid() ||
- !capabilities.ReadValue(L"ApplicationName", &name)) {
+ capabilities.ReadValue(L"ApplicationName", &name) != ERROR_SUCCESS) {
RegKey base_key(root, key.c_str(), KEY_READ);
- if (!base_key.ReadValue(L"", &name))
+ if (base_key.ReadValue(L"", &name) != ERROR_SUCCESS)
continue;
}
RegKey install_info(root, (key + L"\\InstallInfo").c_str(), KEY_READ);
std::wstring command;
if (!install_info.Valid() ||
- !install_info.ReadValue(L"ReinstallCommand", &command))
+ (install_info.ReadValue(L"ReinstallCommand",
+ &command) != ERROR_SUCCESS))
continue;
if (!name.empty() && !command.empty() &&
name.find(dist->GetApplicationName()) == std::wstring::npos)
diff --git a/chrome/installer/util/work_item.cc b/chrome/installer/util/work_item.cc
index 5ed352b..546b9ebc 100644
--- a/chrome/installer/util/work_item.cc
+++ b/chrome/installer/util/work_item.cc
@@ -49,10 +49,8 @@ DeleteRegKeyWorkItem* WorkItem::CreateDeleteRegKeyWorkItem(
DeleteRegValueWorkItem* WorkItem::CreateDeleteRegValueWorkItem(
HKEY predefined_root,
const std::wstring& key_path,
- const std::wstring& value_name,
- DWORD type) {
- return new DeleteRegValueWorkItem(predefined_root, key_path,
- value_name, type);
+ const std::wstring& value_name) {
+ return new DeleteRegValueWorkItem(predefined_root, key_path, value_name);
}
DeleteTreeWorkItem* WorkItem::CreateDeleteTreeWorkItem(
diff --git a/chrome/installer/util/work_item.h b/chrome/installer/util/work_item.h
index 12446be..7fd5db2 100644
--- a/chrome/installer/util/work_item.h
+++ b/chrome/installer/util/work_item.h
@@ -82,8 +82,7 @@ class WorkItem {
static DeleteRegValueWorkItem* CreateDeleteRegValueWorkItem(
HKEY predefined_root,
const std::wstring& key_path,
- const std::wstring& value_name,
- DWORD type);
+ const std::wstring& value_name);
// Create a DeleteTreeWorkItem that recursively deletes a file system
// hierarchy at the given root path. A key file can be optionally specified
diff --git a/chrome/installer/util/work_item_list.cc b/chrome/installer/util/work_item_list.cc
index 41ee169..28e2829 100644
--- a/chrome/installer/util/work_item_list.cc
+++ b/chrome/installer/util/work_item_list.cc
@@ -110,12 +110,9 @@ WorkItem* WorkItemList::AddDeleteRegKeyWorkItem(HKEY predefined_root,
WorkItem* WorkItemList::AddDeleteRegValueWorkItem(
HKEY predefined_root,
const std::wstring& key_path,
- const std::wstring& value_name,
- DWORD type) {
+ const std::wstring& value_name) {
WorkItem* item = WorkItem::CreateDeleteRegValueWorkItem(predefined_root,
- key_path,
- value_name,
- type);
+ key_path, value_name);
AddWorkItem(item);
return item;
}
diff --git a/chrome/installer/util/work_item_list.h b/chrome/installer/util/work_item_list.h
index 5792d0f..23af15b 100644
--- a/chrome/installer/util/work_item_list.h
+++ b/chrome/installer/util/work_item_list.h
@@ -61,9 +61,8 @@ class WorkItemList : public WorkItem {
// Add a DeleteRegValueWorkItem that deletes registry value of type REG_SZ
// or REG_DWORD.
virtual WorkItem* AddDeleteRegValueWorkItem(HKEY predefined_root,
- const std::wstring& key_path,
- const std::wstring& value_name,
- DWORD type);
+ const std::wstring& key_path,
+ const std::wstring& value_name);
// Add a DeleteTreeWorkItem that recursively deletes a file system
// hierarchy at the given root path. A key file can be optionally specified
diff --git a/chrome/installer/util/work_item_list_unittest.cc b/chrome/installer/util/work_item_list_unittest.cc
index 97a6927..bd8f515 100644
--- a/chrome/installer/util/work_item_list_unittest.cc
+++ b/chrome/installer/util/work_item_list_unittest.cc
@@ -28,8 +28,9 @@ class WorkItemListTest : public testing::Test {
// Create a temporary key for testing
RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS);
key.DeleteKey(test_root);
- ASSERT_FALSE(key.Open(HKEY_CURRENT_USER, test_root, KEY_READ));
- ASSERT_TRUE(key.Create(HKEY_CURRENT_USER, test_root, KEY_READ));
+ ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER, test_root, KEY_READ));
+ ASSERT_EQ(ERROR_SUCCESS,
+ key.Create(HKEY_CURRENT_USER, test_root, KEY_READ));
// Create a temp directory for test.
ASSERT_TRUE(PathService::Get(base::DIR_TEMP, &test_dir_));
@@ -47,7 +48,7 @@ class WorkItemListTest : public testing::Test {
ASSERT_FALSE(file_util::PathExists(test_dir_));
// Clean up the temporary key
RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS);
- ASSERT_TRUE(key.DeleteKey(test_root));
+ ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(test_root));
}
FilePath test_dir_;
@@ -88,9 +89,10 @@ TEST_F(WorkItemListTest, ExecutionSuccess) {
// Verify all WorkItems have been executed.
RegKey key;
- EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+ EXPECT_EQ(ERROR_SUCCESS,
+ key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
std::wstring read_out;
- EXPECT_TRUE(key.ReadValue(name.c_str(), &read_out));
+ EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
EXPECT_EQ(0, read_out.compare(data_str));
key.Close();
EXPECT_TRUE(file_util::PathExists(dir_to_create));
@@ -100,7 +102,8 @@ TEST_F(WorkItemListTest, ExecutionSuccess) {
// Verify everything is rolled back.
// The value must have been deleted first in roll back otherwise the key
// can not be deleted.
- EXPECT_FALSE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+ EXPECT_NE(ERROR_SUCCESS,
+ key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
EXPECT_FALSE(file_util::PathExists(top_dir_to_create));
}
@@ -145,17 +148,19 @@ TEST_F(WorkItemListTest, ExecutionFailAndRollback) {
// Verify the first 2 WorkItems have been executed.
RegKey key;
- EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+ EXPECT_EQ(ERROR_SUCCESS,
+ key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
key.Close();
EXPECT_TRUE(file_util::PathExists(dir_to_create));
// The last one should not be there.
- EXPECT_FALSE(key.Open(HKEY_CURRENT_USER, not_created_key.c_str(),
- KEY_READ));
+ EXPECT_NE(ERROR_SUCCESS,
+ key.Open(HKEY_CURRENT_USER, not_created_key.c_str(), KEY_READ));
work_item_list->Rollback();
// Verify everything is rolled back.
- EXPECT_FALSE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+ EXPECT_NE(ERROR_SUCCESS,
+ key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
EXPECT_FALSE(file_util::PathExists(top_dir_to_create));
}
@@ -196,9 +201,10 @@ TEST_F(WorkItemListTest, ConditionalExecutionSuccess) {
// Verify all WorkItems have been executed.
RegKey key;
- EXPECT_TRUE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+ EXPECT_EQ(ERROR_SUCCESS,
+ key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
std::wstring read_out;
- EXPECT_TRUE(key.ReadValue(name.c_str(), &read_out));
+ EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
EXPECT_EQ(0, read_out.compare(data_str));
key.Close();
EXPECT_TRUE(file_util::PathExists(dir_to_create));
@@ -208,7 +214,8 @@ TEST_F(WorkItemListTest, ConditionalExecutionSuccess) {
// Verify everything is rolled back.
// The value must have been deleted first in roll back otherwise the key
// can not be deleted.
- EXPECT_FALSE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+ EXPECT_NE(ERROR_SUCCESS,
+ key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
EXPECT_FALSE(file_util::PathExists(top_dir_to_create));
}
@@ -250,9 +257,10 @@ TEST_F(WorkItemListTest, ConditionalExecutionConditionFailure) {
// Verify that the WorkItems added as part of the conditional list have NOT
// been executed.
RegKey key;
- EXPECT_FALSE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+ EXPECT_NE(ERROR_SUCCESS,
+ key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
std::wstring read_out;
- EXPECT_FALSE(key.ReadValue(name.c_str(), &read_out));
+ EXPECT_NE(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out));
key.Close();
// Verify that the other work item was executed.
@@ -263,7 +271,8 @@ TEST_F(WorkItemListTest, ConditionalExecutionConditionFailure) {
// Verify everything is rolled back.
// The value must have been deleted first in roll back otherwise the key
// can not be deleted.
- EXPECT_FALSE(key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
+ EXPECT_NE(ERROR_SUCCESS,
+ key.Open(HKEY_CURRENT_USER, key_to_create.c_str(), KEY_READ));
EXPECT_FALSE(file_util::PathExists(top_dir_to_create));
}
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) {