diff options
author | gab <gab@chromium.org> | 2015-07-01 19:59:41 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-07-02 03:00:23 +0000 |
commit | 69821f190e784c132d598e07433165c934ee3087 (patch) | |
tree | 3c37fe06e317a9911d501847ad0a934509710af9 /chrome/installer/util | |
parent | c967bfa4a80df291fbefdd8931b7f5fe9414bea7 (diff) | |
download | chromium_src-69821f190e784c132d598e07433165c934ee3087.zip chromium_src-69821f190e784c132d598e07433165c934ee3087.tar.gz chromium_src-69821f190e784c132d598e07433165c934ee3087.tar.bz2 |
Introduce a SetRegValueWorkItem overload that accepts a callback instead
of a fixed value.
The callback is used to determine the desired value after inspecting the
existing value.
Also modified the existing test's paradigm to bring it up to modern days
(from initial.commit!) and added some more tests to cover this new use case.
Bypassing the wstring usage presubmit warnings because we will likely want
to merge this, will consider fixing the API on trunk after.
BUG=488247, 502363
NOPRESUBMIT=true
Review URL: https://codereview.chromium.org/1220473002
Cr-Commit-Position: refs/heads/master@{#337164}
Diffstat (limited to 'chrome/installer/util')
-rw-r--r-- | chrome/installer/util/set_reg_value_work_item.cc | 73 | ||||
-rw-r--r-- | chrome/installer/util/set_reg_value_work_item.h | 12 | ||||
-rw-r--r-- | chrome/installer/util/set_reg_value_work_item_unittest.cc | 375 | ||||
-rw-r--r-- | chrome/installer/util/work_item.cc | 13 | ||||
-rw-r--r-- | chrome/installer/util/work_item.h | 16 | ||||
-rw-r--r-- | chrome/installer/util/work_item_list.cc | 15 | ||||
-rw-r--r-- | chrome/installer/util/work_item_list.h | 10 |
7 files changed, 362 insertions, 152 deletions
diff --git a/chrome/installer/util/set_reg_value_work_item.cc b/chrome/installer/util/set_reg_value_work_item.cc index edad940..dcb5a2e 100644 --- a/chrome/installer/util/set_reg_value_work_item.cc +++ b/chrome/installer/util/set_reg_value_work_item.cc @@ -9,6 +9,40 @@ #include "base/win/registry.h" #include "chrome/installer/util/logging_installer.h" +namespace { + +// Transforms |str_value| into the byte-by-byte representation of its underlying +// string, stores the result in |binary_data|. +void StringToBinaryData(const std::wstring& str_value, + std::vector<uint8>* binary_data) { + DCHECK(binary_data); + const uint8* data = reinterpret_cast<const uint8*>(str_value.c_str()); + binary_data->assign(data, data + (str_value.length() + 1) * sizeof(wchar_t)); +} + +// Transforms |binary_data| into its wstring representation (assuming +// |binary_data| is a sequence of wchar_t's). +void BinaryDataToString(const std::vector<uint8>& binary_data, + std::wstring* str_value) { + DCHECK(str_value); + if (binary_data.size() < sizeof(wchar_t)) { + str_value->clear(); + return; + } + + str_value->assign(reinterpret_cast<const wchar_t*>(&binary_data[0]), + binary_data.size() / sizeof(wchar_t)); + + // Trim off a trailing string terminator, if present. + if (!str_value->empty()) { + auto iter = str_value->end(); + if (*--iter == L'\0') + str_value->erase(iter); + } +} + +} // namespace + SetRegValueWorkItem::~SetRegValueWorkItem() { } @@ -29,8 +63,7 @@ SetRegValueWorkItem::SetRegValueWorkItem(HKEY predefined_root, DCHECK(wow64_access == 0 || wow64_access == KEY_WOW64_32KEY || wow64_access == KEY_WOW64_64KEY); - const uint8* data = reinterpret_cast<const uint8*>(value_data.c_str()); - value_.assign(data, data + (value_data.length() + 1) * sizeof(wchar_t)); + StringToBinaryData(value_data, &value_); } SetRegValueWorkItem::SetRegValueWorkItem(HKEY predefined_root, @@ -75,6 +108,27 @@ SetRegValueWorkItem::SetRegValueWorkItem(HKEY predefined_root, value_.assign(data, data + sizeof(value_data)); } +SetRegValueWorkItem::SetRegValueWorkItem( + HKEY predefined_root, + const std::wstring& key_path, + REGSAM wow64_access, + const std::wstring& value_name, + const GetValueFromExistingCallback& get_value_callback) + : predefined_root_(predefined_root), + key_path_(key_path), + value_name_(value_name), + get_value_callback_(get_value_callback), + overwrite_(true), + wow64_access_(wow64_access), + status_(SET_VALUE), + type_(REG_SZ), + previous_type_(0) { + DCHECK(wow64_access == 0 || + wow64_access == KEY_WOW64_32KEY || + wow64_access == KEY_WOW64_64KEY); + // Nothing to do, |get_value_callback| will fill |value_| later. +} + bool SetRegValueWorkItem::Do() { LONG result = ERROR_SUCCESS; base::win::RegKey key; @@ -118,6 +172,21 @@ bool SetRegValueWorkItem::Do() { } } + if (!get_value_callback_.is_null()) { + // Although this could be made more generic, for now this assumes the + // |type_| of |value_| is REG_SZ. + DCHECK_EQ(static_cast<DWORD>(REG_SZ), type_); + + // Fill |previous_value_str| with the wstring representation of the binary + // data in |previous_value_| as long as it's of type REG_SZ (leave it empty + // otherwise). + std::wstring previous_value_str; + if (previous_type_ == REG_SZ) + BinaryDataToString(previous_value_, &previous_value_str); + + StringToBinaryData(get_value_callback_.Run(previous_value_str), &value_); + } + result = key.WriteValue(value_name_.c_str(), &value_[0], static_cast<DWORD>(value_.size()), type_); if (result != ERROR_SUCCESS) { diff --git a/chrome/installer/util/set_reg_value_work_item.h b/chrome/installer/util/set_reg_value_work_item.h index 0c4d0c8..6ccb377 100644 --- a/chrome/installer/util/set_reg_value_work_item.h +++ b/chrome/installer/util/set_reg_value_work_item.h @@ -10,6 +10,7 @@ #include <string> #include <vector> +#include "base/callback.h" #include "chrome/installer/util/work_item.h" // A WorkItem subclass that sets a registry value with REG_SZ, REG_DWORD, or @@ -63,6 +64,13 @@ class SetRegValueWorkItem : public WorkItem { int64 value_data, bool overwrite); + // Implies |overwrite_| and TYPE_SZ for now. + SetRegValueWorkItem(HKEY predefined_root, + const std::wstring& key_path, + REGSAM wow64_access, + const std::wstring& value_name, + const GetValueFromExistingCallback& get_value_callback); + // 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. HKEY predefined_root_; @@ -73,6 +81,10 @@ class SetRegValueWorkItem : public WorkItem { // Name of the value to be set. std::wstring value_name_; + // If this is set, it will be used to get the desired value to be set based on + // the existing value in the registry. + const GetValueFromExistingCallback get_value_callback_; + // Whether to overwrite the existing value under the target key. bool overwrite_; 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 163f888..3916a34 100644 --- a/chrome/installer/util/set_reg_value_work_item_unittest.cc +++ b/chrome/installer/util/set_reg_value_work_item_unittest.cc @@ -4,307 +4,382 @@ #include <windows.h> +#include <string> + +#include "base/bind.h" #include "base/files/file_path.h" +#include "base/macros.h" #include "base/memory/scoped_ptr.h" #include "base/strings/string_util.h" +#include "base/test/test_reg_util_win.h" #include "base/win/registry.h" #include "chrome/installer/util/set_reg_value_work_item.h" #include "chrome/installer/util/work_item.h" #include "testing/gtest/include/gtest/gtest.h" -using base::win::RegKey; - namespace { -const wchar_t kTestRoot[] = L"TempTemp"; +const wchar_t kTestKey[] = L"TempTemp"; const wchar_t kDataStr1[] = L"data_111"; const wchar_t kDataStr2[] = L"data_222"; const wchar_t kNameStr[] = L"name_str"; const wchar_t kNameDword[] = L"name_dword"; -const DWORD dword1 = 0; -const DWORD dword2 = 1; +const DWORD kDword1 = 12345; +const DWORD kDword2 = 6789; class SetRegValueWorkItemTest : public testing::Test { protected: + SetRegValueWorkItemTest() {} + void SetUp() override { - // Create a temporary key for testing - RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS); - key.DeleteKey(kTestRoot); - ASSERT_NE(ERROR_SUCCESS, key.Open(HKEY_CURRENT_USER, kTestRoot, KEY_READ)); - ASSERT_EQ(ERROR_SUCCESS, - key.Create(HKEY_CURRENT_USER, kTestRoot, KEY_READ)); - } - void TearDown() override { - logging::CloseLogFile(); - // Clean up the temporary key - RegKey key(HKEY_CURRENT_USER, L"", KEY_ALL_ACCESS); - ASSERT_EQ(ERROR_SUCCESS, key.DeleteKey(kTestRoot)); + registry_override_manager_.OverrideRegistry(HKEY_CURRENT_USER); + + // Create a temporary key for testing. + ASSERT_NE(ERROR_SUCCESS, + test_key_.Open(HKEY_CURRENT_USER, kTestKey, KEY_READ)); + ASSERT_EQ(ERROR_SUCCESS, test_key_.Create(HKEY_CURRENT_USER, kTestKey, + KEY_READ | KEY_SET_VALUE)); } + + base::win::RegKey test_key_; + + private: + registry_util::RegistryOverrideManager registry_override_manager_; + + DISALLOW_COPY_AND_ASSIGN(SetRegValueWorkItemTest); }; } // namespace // Write a new value without overwrite flag. The value should be set. TEST_F(SetRegValueWorkItemTest, WriteNewNonOverwrite) { - RegKey key; - - std::wstring parent_key(kTestRoot); - parent_key.append(&base::FilePath::kSeparators[0], 1); - parent_key.append(L"WriteNewNonOverwrite"); - ASSERT_EQ(ERROR_SUCCESS, - key.Create(HKEY_CURRENT_USER, parent_key.c_str(), KEY_READ)); - - std::wstring name_str(kNameStr); - std::wstring data_str(kDataStr1); scoped_ptr<SetRegValueWorkItem> work_item1( WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER, - parent_key, + kTestKey, WorkItem::kWow64Default, - name_str, - data_str, + kNameStr, + kDataStr1, false)); - std::wstring name_dword(kNameDword); scoped_ptr<SetRegValueWorkItem> work_item2( WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER, - parent_key, + kTestKey, WorkItem::kWow64Default, - name_dword, - dword1, + kNameDword, + kDword1, false)); - EXPECT_TRUE(work_item1->Do()); - EXPECT_TRUE(work_item2->Do()); + ASSERT_TRUE(work_item1->Do()); + ASSERT_TRUE(work_item2->Do()); std::wstring read_out; DWORD 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(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out)); + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValueDW(kNameDword, &read_dword)); EXPECT_EQ(read_out, kDataStr1); - EXPECT_EQ(read_dword, dword1); + EXPECT_EQ(read_dword, kDword1); work_item1->Rollback(); work_item2->Rollback(); // Rollback should delete the value. - EXPECT_FALSE(key.HasValue(name_str.c_str())); - EXPECT_FALSE(key.HasValue(name_dword.c_str())); + EXPECT_FALSE(test_key_.HasValue(kNameStr)); + EXPECT_FALSE(test_key_.HasValue(kNameDword)); } // Write a new value with overwrite flag. The value should be set. TEST_F(SetRegValueWorkItemTest, WriteNewOverwrite) { - RegKey key; - - std::wstring parent_key(kTestRoot); - parent_key.append(&base::FilePath::kSeparators[0], 1); - parent_key.append(L"WriteNewOverwrite"); - ASSERT_EQ(ERROR_SUCCESS, - key.Create(HKEY_CURRENT_USER, parent_key.c_str(), KEY_READ)); - - std::wstring name_str(kNameStr); - std::wstring data_str(kDataStr1); scoped_ptr<SetRegValueWorkItem> work_item1( WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER, - parent_key, + kTestKey, WorkItem::kWow64Default, - name_str, - data_str, + kNameStr, + kDataStr1, true)); - std::wstring name_dword(kNameDword); scoped_ptr<SetRegValueWorkItem> work_item2( WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER, - parent_key, + kTestKey, WorkItem::kWow64Default, - name_dword, - dword1, + kNameDword, + kDword1, true)); - EXPECT_TRUE(work_item1->Do()); - EXPECT_TRUE(work_item2->Do()); + ASSERT_TRUE(work_item1->Do()); + ASSERT_TRUE(work_item2->Do()); std::wstring read_out; DWORD 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(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out)); + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValueDW(kNameDword, &read_dword)); EXPECT_EQ(read_out, kDataStr1); - EXPECT_EQ(read_dword, dword1); + EXPECT_EQ(read_dword, kDword1); work_item1->Rollback(); work_item2->Rollback(); // Rollback should delete the value. - EXPECT_FALSE(key.HasValue(name_str.c_str())); - EXPECT_FALSE(key.HasValue(name_dword.c_str())); + EXPECT_FALSE(test_key_.HasValue(kNameStr)); + EXPECT_FALSE(test_key_.HasValue(kNameDword)); } // Write to an existing value without overwrite flag. There should be // no change. TEST_F(SetRegValueWorkItemTest, WriteExistingNonOverwrite) { - RegKey key; - - std::wstring parent_key(kTestRoot); - parent_key.append(&base::FilePath::kSeparators[0], 1); - parent_key.append(L"WriteExistingNonOverwrite"); - 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(kNameStr); - ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(name.c_str(), kDataStr1)); + ASSERT_EQ(ERROR_SUCCESS, test_key_.WriteValue(kNameStr, kDataStr1)); - std::wstring data(kDataStr2); scoped_ptr<SetRegValueWorkItem> work_item( WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER, - parent_key, + kTestKey, WorkItem::kWow64Default, - name, - data, + kNameStr, + kDataStr2, false)); - EXPECT_TRUE(work_item->Do()); + ASSERT_TRUE(work_item->Do()); std::wstring read_out; - EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out)); - EXPECT_EQ(0, read_out.compare(kDataStr1)); + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out)); + EXPECT_EQ(read_out, kDataStr1); work_item->Rollback(); - EXPECT_TRUE(key.HasValue(name.c_str())); - EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out)); + EXPECT_TRUE(test_key_.HasValue(kNameStr)); + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out)); EXPECT_EQ(read_out, kDataStr1); // Now test REG_DWORD value. // Write data to the value we are going to set. - name.assign(kNameDword); - ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(name.c_str(), dword1)); + ASSERT_EQ(ERROR_SUCCESS, test_key_.WriteValue(kNameDword, kDword1)); work_item.reset(WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER, - parent_key, + kTestKey, WorkItem::kWow64Default, - name, - dword2, + kNameDword, + kDword2, false)); - EXPECT_TRUE(work_item->Do()); + ASSERT_TRUE(work_item->Do()); DWORD read_dword; - EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name.c_str(), &read_dword)); - EXPECT_EQ(read_dword, dword1); + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValueDW(kNameDword, &read_dword)); + EXPECT_EQ(read_dword, kDword1); work_item->Rollback(); - EXPECT_TRUE(key.HasValue(name.c_str())); - EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name.c_str(), &read_dword)); - EXPECT_EQ(read_dword, dword1); + EXPECT_TRUE(test_key_.HasValue(kNameDword)); + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValueDW(kNameDword, &read_dword)); + EXPECT_EQ(read_dword, kDword1); } // Write to an existing value with overwrite flag. The value should be // overwritten. TEST_F(SetRegValueWorkItemTest, WriteExistingOverwrite) { - RegKey key; - - std::wstring parent_key(kTestRoot); - parent_key.append(&base::FilePath::kSeparators[0], 1); - parent_key.append(L"WriteExistingOverwrite"); - 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(kNameStr); - ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(name.c_str(), kDataStr1)); + ASSERT_EQ(ERROR_SUCCESS, test_key_.WriteValue(kNameStr, kDataStr1)); - std::wstring name_empty(L"name_empty"); - ASSERT_EQ(ERROR_SUCCESS, RegSetValueEx(key.Handle(), name_empty.c_str(), NULL, + const wchar_t kNameEmpty[] = L"name_empty"; + ASSERT_EQ(ERROR_SUCCESS, RegSetValueEx(test_key_.Handle(), kNameEmpty, NULL, REG_SZ, NULL, 0)); - std::wstring data(kDataStr2); scoped_ptr<SetRegValueWorkItem> work_item1( WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER, - parent_key, + kTestKey, WorkItem::kWow64Default, - name, - data, + kNameStr, + kDataStr2, true)); scoped_ptr<SetRegValueWorkItem> work_item2( WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER, - parent_key, + kTestKey, WorkItem::kWow64Default, - name_empty, - data, + kNameEmpty, + kDataStr2, true)); - EXPECT_TRUE(work_item1->Do()); - EXPECT_TRUE(work_item2->Do()); + ASSERT_TRUE(work_item1->Do()); + ASSERT_TRUE(work_item2->Do()); std::wstring read_out; - EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out)); - EXPECT_EQ(0, read_out.compare(kDataStr2)); + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out)); + EXPECT_EQ(read_out, kDataStr2); - EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name_empty.c_str(), &read_out)); - EXPECT_EQ(0, read_out.compare(kDataStr2)); + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameEmpty, &read_out)); + EXPECT_EQ(read_out, kDataStr2); work_item1->Rollback(); work_item2->Rollback(); - EXPECT_TRUE(key.HasValue(name.c_str())); - EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name.c_str(), &read_out)); + EXPECT_TRUE(test_key_.HasValue(kNameStr)); + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out)); EXPECT_EQ(read_out, kDataStr1); DWORD type = 0; DWORD size = 0; - EXPECT_EQ(ERROR_SUCCESS, key.ReadValue(name_empty.c_str(), NULL, &size, - &type)); + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameEmpty, NULL, &size, &type)); EXPECT_EQ(REG_SZ, type); EXPECT_EQ(0, size); // Now test REG_DWORD value. // Write data to the value we are going to set. - name.assign(kNameDword); - ASSERT_EQ(ERROR_SUCCESS, key.WriteValue(name.c_str(), dword1)); + ASSERT_EQ(ERROR_SUCCESS, test_key_.WriteValue(kNameDword, kDword1)); scoped_ptr<SetRegValueWorkItem> work_item3( WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER, - parent_key, + kTestKey, WorkItem::kWow64Default, - name, - dword2, + kNameDword, + kDword2, true)); - EXPECT_TRUE(work_item3->Do()); + ASSERT_TRUE(work_item3->Do()); DWORD read_dword; - EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name.c_str(), &read_dword)); - EXPECT_EQ(read_dword, dword2); + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValueDW(kNameDword, &read_dword)); + EXPECT_EQ(read_dword, kDword2); work_item3->Rollback(); - EXPECT_TRUE(key.HasValue(name.c_str())); - EXPECT_EQ(ERROR_SUCCESS, key.ReadValueDW(name.c_str(), &read_dword)); - EXPECT_EQ(read_dword, dword1); + EXPECT_TRUE(test_key_.HasValue(kNameDword)); + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValueDW(kNameDword, &read_dword)); + EXPECT_EQ(read_dword, kDword1); } // Write a value to a non-existing key. This should fail. TEST_F(SetRegValueWorkItemTest, WriteNonExistingKey) { - RegKey key; - - std::wstring parent_key(kTestRoot); - parent_key.append(&base::FilePath::kSeparators[0], 1); - parent_key.append(L"WriteNonExistingKey"); + std::wstring non_existing(kTestKey); + non_existing.append(&base::FilePath::kSeparators[0], 1); + non_existing.append(L"NonExistingKey"); - std::wstring name(L"name"); - std::wstring data(kDataStr1); scoped_ptr<SetRegValueWorkItem> work_item( WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER, - parent_key, + non_existing.c_str(), WorkItem::kWow64Default, - name, - data, + kNameStr, + kDataStr1, false)); EXPECT_FALSE(work_item->Do()); work_item.reset(WorkItem::CreateSetRegValueWorkItem(HKEY_CURRENT_USER, - parent_key, + non_existing.c_str(), WorkItem::kWow64Default, - name, - dword1, + kNameStr, + kDword1, false)); EXPECT_FALSE(work_item->Do()); } + +// Verifies that |actual_previous_value| is |expected_previous_value| and +// returns |desired_new_value| to replace it. Unconditionally increments +// |invocation_count| to allow tests to assert correctness of the callee's +// behaviour. +std::wstring VerifyPreviousValueAndReplace( + int* invocation_count, + const std::wstring& expected_previous_value, + const std::wstring& desired_new_value, + const std::wstring& actual_previous_value) { + ++(*invocation_count); + EXPECT_EQ(expected_previous_value, actual_previous_value); + return desired_new_value; +} + +// Modify an existing value with the callback API. +TEST_F(SetRegValueWorkItemTest, ModifyExistingWithCallback) { + // Write |kDataStr1| to the value we are going to modify. + ASSERT_EQ(ERROR_SUCCESS, test_key_.WriteValue(kNameStr, kDataStr1)); + + int callback_invocation_count = 0; + + scoped_ptr<SetRegValueWorkItem> work_item( + WorkItem::CreateSetRegValueWorkItem( + HKEY_CURRENT_USER, + kTestKey, + WorkItem::kWow64Default, + kNameStr, + base::Bind(&VerifyPreviousValueAndReplace, &callback_invocation_count, + kDataStr1, kDataStr2))); + + // The callback should not be used until the item is executed. + EXPECT_EQ(0, callback_invocation_count); + + ASSERT_TRUE(work_item->Do()); + + std::wstring read_out; + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out)); + EXPECT_EQ(read_out, kDataStr2); + + // The callback should have been used only once to achieve this result. + EXPECT_EQ(1, callback_invocation_count); + + work_item->Rollback(); + + EXPECT_TRUE(test_key_.HasValue(kNameStr)); + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out)); + EXPECT_EQ(read_out, kDataStr1); + + // The callback should not have been used again for the rollback. + EXPECT_EQ(1, callback_invocation_count); +} + +// Modify a non-existing value with the callback API. +TEST_F(SetRegValueWorkItemTest, ModifyNonExistingWithCallback) { + int callback_invocation_count = 0; + + scoped_ptr<SetRegValueWorkItem> work_item( + WorkItem::CreateSetRegValueWorkItem( + HKEY_CURRENT_USER, + kTestKey, + WorkItem::kWow64Default, + kNameStr, + base::Bind(&VerifyPreviousValueAndReplace, &callback_invocation_count, + L"", kDataStr1))); + + EXPECT_EQ(0, callback_invocation_count); + + ASSERT_TRUE(work_item->Do()); + + std::wstring read_out; + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_out)); + EXPECT_EQ(read_out, kDataStr1); + + EXPECT_EQ(1, callback_invocation_count); + + work_item->Rollback(); + + EXPECT_FALSE(test_key_.HasValue(kNameStr)); + + EXPECT_EQ(1, callback_invocation_count); +} + +// Modify an existing value which is not a string (REG_SZ) with the string +// callback API. +TEST_F(SetRegValueWorkItemTest, ModifyExistingNonStringWithStringCallback) { + // Write |kDword1| to the value we are going to modify. + ASSERT_EQ(ERROR_SUCCESS, test_key_.WriteValue(kNameStr, kDword1)); + + int callback_invocation_count = 0; + + scoped_ptr<SetRegValueWorkItem> work_item( + WorkItem::CreateSetRegValueWorkItem( + HKEY_CURRENT_USER, + kTestKey, + WorkItem::kWow64Default, + kNameStr, + base::Bind(&VerifyPreviousValueAndReplace, &callback_invocation_count, + L"", kDataStr1))); + + EXPECT_EQ(0, callback_invocation_count); + + ASSERT_TRUE(work_item->Do()); + + std::wstring read_str_out; + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValue(kNameStr, &read_str_out)); + EXPECT_EQ(read_str_out, kDataStr1); + + EXPECT_EQ(1, callback_invocation_count); + + work_item->Rollback(); + + DWORD read_dword_out = 0; + EXPECT_TRUE(test_key_.HasValue(kNameStr)); + EXPECT_EQ(ERROR_SUCCESS, test_key_.ReadValueDW(kNameStr, &read_dword_out)); + EXPECT_EQ(read_dword_out, kDword1); + + EXPECT_EQ(1, callback_invocation_count); +} diff --git a/chrome/installer/util/work_item.cc b/chrome/installer/util/work_item.cc index 3b2db8e..01d2b19 100644 --- a/chrome/installer/util/work_item.cc +++ b/chrome/installer/util/work_item.cc @@ -131,6 +131,19 @@ SetRegValueWorkItem* WorkItem::CreateSetRegValueWorkItem( overwrite); } +SetRegValueWorkItem* WorkItem::CreateSetRegValueWorkItem( + HKEY predefined_root, + const std::wstring& key_path, + REGSAM wow64_access, + const std::wstring& value_name, + const GetValueFromExistingCallback& get_value_callback) { + return new SetRegValueWorkItem(predefined_root, + key_path, + wow64_access, + value_name, + get_value_callback); +} + SelfRegWorkItem* WorkItem::CreateSelfRegWorkItem(const std::wstring& dll_path, bool do_register, bool user_level_registration) { diff --git a/chrome/installer/util/work_item.h b/chrome/installer/util/work_item.h index b19f5c2..a8c5376 100644 --- a/chrome/installer/util/work_item.h +++ b/chrome/installer/util/work_item.h @@ -37,6 +37,12 @@ class FilePath; // sequence of actions during install/update/uninstall. class WorkItem { public: + // A callback that returns the desired value based on the |existing_value|. + // |existing_value| will be empty if the value didn't previously exist or + // existed under a non-string type. + using GetValueFromExistingCallback = + base::Callback<std::wstring(const std::wstring& existing_value)>; + // All registry operations can be instructed to operate on a specific view // of the registry by specifying a REGSAM value to the wow64_access parameter. // The wow64_access parameter can be one of: @@ -158,6 +164,16 @@ class WorkItem { int64 value_data, bool overwrite); + // Create a SetRegValueWorkItem that sets a registry value based on the value + // provided by |get_value_callback| given the existing value under + // |key_path\value_name|. + static SetRegValueWorkItem* CreateSetRegValueWorkItem( + HKEY predefined_root, + const std::wstring& key_path, + REGSAM wow64_access, + const std::wstring& value_name, + const GetValueFromExistingCallback& get_value_callback); + // Add a SelfRegWorkItem that registers or unregisters a DLL at the // specified path. static SelfRegWorkItem* CreateSelfRegWorkItem(const std::wstring& dll_path, diff --git a/chrome/installer/util/work_item_list.cc b/chrome/installer/util/work_item_list.cc index 5656ce1..cb227ea 100644 --- a/chrome/installer/util/work_item_list.cc +++ b/chrome/installer/util/work_item_list.cc @@ -208,6 +208,21 @@ WorkItem* WorkItemList::AddSetRegValueWorkItem(HKEY predefined_root, return item; } +WorkItem* WorkItemList::AddSetRegValueWorkItem( + HKEY predefined_root, + const std::wstring& key_path, + REGSAM wow64_access, + const std::wstring& value_name, + const WorkItem::GetValueFromExistingCallback& get_value_callback) { + WorkItem* item = WorkItem::CreateSetRegValueWorkItem(predefined_root, + key_path, + wow64_access, + value_name, + get_value_callback); + AddWorkItem(item); + return item; +} + WorkItem* WorkItemList::AddSelfRegWorkItem(const std::wstring& dll_path, bool do_register, bool user_level_registration) { diff --git a/chrome/installer/util/work_item_list.h b/chrome/installer/util/work_item_list.h index 74c0562..aca5e72 100644 --- a/chrome/installer/util/work_item_list.h +++ b/chrome/installer/util/work_item_list.h @@ -120,6 +120,16 @@ class WorkItemList : public WorkItem { int64 value_data, bool overwrite); + // Add a SetRegValueWorkItem that sets a registry value based on the value + // provided by |get_value_callback| given the existing value under + // |key_path\value_name|. + virtual WorkItem* AddSetRegValueWorkItem( + HKEY predefined_root, + const std::wstring& key_path, + REGSAM wow64_access, + const std::wstring& value_name, + const WorkItem::GetValueFromExistingCallback& get_value_callback); + // Add a SelfRegWorkItem that registers or unregisters a DLL at the // specified path. If user_level_registration is true, then alternate // registration and unregistration entry point names will be used. |