0

Use RegistryOverrideManager properly in RLZ tests.

Previously, rlz/test/rlz_test_helpers.cc and
chrome/browser/rlz/rlz_unittest.cc each did somewhat different things to
prepare the registry for tests. Now, rlz_test_helpers.cc does the prep
the right way using RegistryOverrideManager and rlz_unittests.cc uses
rlz_test_helpers.cc.

This unblocks the refactor in r234367 that was reverted in r234627.

BUG=314800
R=rogerta@chromium.org, tommycli@chromium.org

Review URL: https://codereview.chromium.org/63153009

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@235090 0039d316-1c4b-4281-b951-d872f2087c98
This commit is contained in:
grt@chromium.org
2013-11-14 07:43:20 +00:00
parent b7c04c1149
commit 20402235d7
10 changed files with 149 additions and 219 deletions

@ -4,32 +4,23 @@
#include "chrome/browser/rlz/rlz.h"
#include "base/files/scoped_temp_dir.h"
#include "base/memory/scoped_ptr.h"
#include "base/message_loop/message_loop.h"
#include "base/path_service.h"
#include "base/strings/stringprintf.h"
#include "base/strings/utf_string_conversions.h"
#include "base/threading/thread.h"
#include "chrome/browser/chrome_notification_types.h"
#include "chrome/browser/google/google_util.h"
#include "chrome/browser/omnibox/omnibox_log.h"
#include "chrome/browser/profiles/profile.h"
#include "chrome/common/env_vars.h"
#include "chrome/installer/util/browser_distribution.h"
#include "chrome/installer/util/google_update_constants.h"
#include "content/public/browser/navigation_entry.h"
#include "content/public/browser/notification_details.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_source.h"
#include "rlz/test/rlz_test_helpers.h"
#include "testing/gtest/include/gtest/gtest.h"
#if defined(OS_WIN)
#include "base/test/test_reg_util_win.h"
#include "base/win/registry.h"
#include "rlz/win/lib/rlz_lib.h" // InitializeTempHivesForTesting
#elif defined(OS_POSIX)
#include "rlz/lib/rlz_value_store.h" // SetRlzStoreDirectory
#endif
using content::NavigationEntry;
@ -39,17 +30,10 @@ using testing::AssertionFailure;
#if defined(OS_WIN)
using base::win::RegKey;
using registry_util::RegistryOverrideManager;
#endif
namespace {
#if defined(OS_WIN)
// Registry path to overridden hive.
const wchar_t kRlzTempHkcu[] = L"rlz_hkcu";
const wchar_t kRlzTempHklm[] = L"rlz_hklm";
#endif
// Dummy RLZ string for the access points.
const char kOmniboxRlzString[] = "test_omnibox";
const char kHomepageRlzString[] = "test_homepage";
@ -170,12 +154,10 @@ class TestRLZTracker : public RLZTracker {
DISALLOW_COPY_AND_ASSIGN(TestRLZTracker);
};
class RlzLibTest : public testing::Test {
public:
virtual void SetUp() OVERRIDE;
virtual void TearDown() OVERRIDE;
class RlzLibTest : public RlzLibTestNoMachineState {
protected:
virtual void SetUp() OVERRIDE;
void SetMainBrand(const char* brand);
void SetReactivationBrand(const char* brand);
#if defined(OS_WIN)
@ -191,52 +173,13 @@ class RlzLibTest : public testing::Test {
void ExpectReactivationRlzPingSent(bool expected);
TestRLZTracker tracker_;
#if defined(OS_WIN)
RegistryOverrideManager override_manager_;
#elif defined(OS_POSIX)
base::ScopedTempDir temp_dir_;
#if defined(OS_POSIX)
scoped_ptr<google_util::BrandForTesting> brand_override_;
#endif
};
void RlzLibTest::SetUp() {
testing::Test::SetUp();
#if defined(OS_WIN)
// Before overriding HKLM for the tests, we need to set it up correctly
// so that the rlz_lib calls work. This needs to be done before we do the
// override.
string16 temp_hklm_path = base::StringPrintf(
L"%ls\\%ls",
RegistryOverrideManager::kTempTestKeyPath,
kRlzTempHklm);
base::win::RegKey hklm;
ASSERT_EQ(ERROR_SUCCESS, hklm.Create(HKEY_CURRENT_USER,
temp_hklm_path.c_str(),
KEY_READ));
string16 temp_hkcu_path = base::StringPrintf(
L"%ls\\%ls",
RegistryOverrideManager::kTempTestKeyPath,
kRlzTempHkcu);
base::win::RegKey hkcu;
ASSERT_EQ(ERROR_SUCCESS, hkcu.Create(HKEY_CURRENT_USER,
temp_hkcu_path.c_str(),
KEY_READ));
rlz_lib::InitializeTempHivesForTesting(hklm, hkcu);
// Its important to override HKLM before HKCU because of the registry
// initialization performed above.
override_manager_.OverrideRegistry(HKEY_LOCAL_MACHINE, kRlzTempHklm);
override_manager_.OverrideRegistry(HKEY_CURRENT_USER, kRlzTempHkcu);
#elif defined(OS_POSIX)
ASSERT_TRUE(temp_dir_.CreateUniqueTempDir());
rlz_lib::testing::SetRlzStoreDirectory(temp_dir_.path());
#endif
RlzLibTestNoMachineState::SetUp();
// Make sure a non-organic brand code is set in the registry or the RLZTracker
// is pretty much a no-op.
@ -244,13 +187,6 @@ void RlzLibTest::SetUp() {
SetReactivationBrand("");
}
void RlzLibTest::TearDown() {
#if defined(OS_POSIX)
rlz_lib::testing::SetRlzStoreDirectory(base::FilePath());
#endif
testing::Test::TearDown();
}
void RlzLibTest::SetMainBrand(const char* brand) {
#if defined(OS_WIN)
SetRegistryBrandValue(google_update::kRegRLZBrandField, brand);

@ -472,6 +472,7 @@
'../content/content.gyp:content_app_both',
'../net/net.gyp:net',
'../net/net.gyp:net_test_support',
'../rlz/rlz.gyp:test_support_rlz',
'../sync/sync.gyp:test_support_sync_api',
'../sync/sync.gyp:test_support_sync_core',
'../sync/sync.gyp:test_support_sync_internal_api',

@ -52,6 +52,22 @@ class URLRequestContextGetter;
namespace rlz_lib {
// All functions return true on success and false on error.
// This implemenation is thread safe.
//
// Each prototype mentions the registry access requirements:
//
// HKLM read: Will work from any process and at any privilege level on Vista.
// HKCU read: Calls made from the SYSTEM account must pass the current user's
// SID as the optional 'sid' param. Can be called from low integrity
// process on Vista.
// HKCU write: Calls made from the SYSTEM account must pass the current user's
// SID as the optional 'sid' param. Calls require at least medium
// integrity on Vista (e.g. Toolbar will need to use their broker)
// HKLM write: Calls must be made from an account with admin rights. No SID
// need be passed when running as SYSTEM.
// Functions which do not access registry will be marked with "no restrictions".
class ScopedRlzValueStoreLock;
// The maximum length of an access points RLZ in bytes.

@ -68,7 +68,6 @@
'win/lib/process_info.h',
'win/lib/registry_util.cc',
'win/lib/registry_util.h',
'win/lib/rlz_lib.h',
'win/lib/rlz_lib_win.cc',
'win/lib/rlz_value_store_registry.cc',
'win/lib/rlz_value_store_registry.h',
@ -110,11 +109,26 @@
# TODO(jschuh): crbug.com/167187 fix size_t to int truncations.
'msvs_disabled_warnings': [ 4267, ],
},
{
'target_name': 'test_support_rlz',
'type': 'static_library',
'dependencies': [
':rlz_lib',
'../base/base.gyp:base',
'../base/base.gyp:test_support_base',
'../testing/gtest.gyp:gtest',
],
'sources': [
'test/rlz_test_helpers.cc',
'test/rlz_test_helpers.h',
],
},
{
'target_name': 'rlz_unittests',
'type': 'executable',
'dependencies': [
':rlz_lib',
':test_support_rlz',
'../base/base.gyp:base',
'../base/base.gyp:base_prefs',
'../testing/gmock.gyp:gmock',
@ -129,8 +143,6 @@
'lib/machine_id_unittest.cc',
'lib/rlz_lib_test.cc',
'lib/string_utils_unittest.cc',
'test/rlz_test_helpers.cc',
'test/rlz_test_helpers.h',
'test/rlz_unittest_main.cc',
'win/lib/machine_deal_test.cc',
],

@ -6,62 +6,129 @@
#include "rlz_test_helpers.h"
#include <map>
#include <vector>
#include "base/strings/string16.h"
#include "rlz/lib/rlz_lib.h"
#include "testing/gtest/include/gtest/gtest.h"
#if defined(OS_WIN)
#include <shlwapi.h>
#include "base/win/registry.h"
#include "rlz/win/lib/rlz_lib.h"
#include "base/win/windows_version.h"
#elif defined(OS_POSIX)
#include "base/files/file_path.h"
#include "rlz/lib/rlz_value_store.h"
#endif
#if defined(OS_WIN)
namespace {
const wchar_t* kHKCUReplacement = L"Software\\Google\\RlzUtilUnittest\\HKCU";
const wchar_t* kHKLMReplacement = L"Software\\Google\\RlzUtilUnittest\\HKLM";
// Path to recursively copy into the replacemment hives. These are needed
// to make sure certain win32 APIs continue to run correctly once the real
// hives are replaced.
const wchar_t kHKLMAccessProviders[] =
L"System\\CurrentControlSet\\Control\\Lsa\\AccessProviders";
void OverrideRegistryHives() {
// Wipe the keys we redirect to.
// This gives us a stable run, even in the presence of previous
// crashes or failures.
LSTATUS err = SHDeleteKey(HKEY_CURRENT_USER, kHKCUReplacement);
EXPECT_TRUE(err == ERROR_SUCCESS || err == ERROR_FILE_NOT_FOUND);
err = SHDeleteKey(HKEY_CURRENT_USER, kHKLMReplacement);
EXPECT_TRUE(err == ERROR_SUCCESS || err == ERROR_FILE_NOT_FOUND);
struct RegistryValue {
string16 name;
DWORD type;
std::vector<uint8> data;
};
// Create the keys we're redirecting HKCU and HKLM to.
base::win::RegKey hkcu;
base::win::RegKey hklm;
ASSERT_EQ(ERROR_SUCCESS,
hkcu.Create(HKEY_CURRENT_USER, kHKCUReplacement, KEY_READ));
ASSERT_EQ(ERROR_SUCCESS,
hklm.Create(HKEY_CURRENT_USER, kHKLMReplacement, KEY_READ));
struct RegistryKeyData {
std::vector<RegistryValue> values;
std::map<string16, RegistryKeyData> keys;
};
rlz_lib::InitializeTempHivesForTesting(hklm, hkcu);
void ReadRegistryTree(const base::win::RegKey& src, RegistryKeyData* data) {
// First read values.
{
base::win::RegistryValueIterator i(src.Handle(), L"");
data->values.clear();
data->values.reserve(i.ValueCount());
for (; i.Valid(); ++i) {
RegistryValue& value = *data->values.insert(data->values.end(),
RegistryValue());
const uint8* data = reinterpret_cast<const uint8*>(i.Value());
value.name.assign(i.Name());
value.type = i.Type();
value.data.assign(data, data + i.ValueSize());
}
}
// And do the switcharoo.
ASSERT_EQ(ERROR_SUCCESS,
::RegOverridePredefKey(HKEY_CURRENT_USER, hkcu.Handle()));
ASSERT_EQ(ERROR_SUCCESS,
::RegOverridePredefKey(HKEY_LOCAL_MACHINE, hklm.Handle()));
// Next read subkeys recursively.
for (base::win::RegistryKeyIterator i(src.Handle(), L"");
i.Valid(); ++i) {
ReadRegistryTree(base::win::RegKey(src.Handle(), i.Name(), KEY_READ),
&data->keys[string16(i.Name())]);
}
}
void UndoOverrideRegistryHives() {
// Undo the redirection.
EXPECT_EQ(ERROR_SUCCESS, ::RegOverridePredefKey(HKEY_CURRENT_USER, NULL));
EXPECT_EQ(ERROR_SUCCESS, ::RegOverridePredefKey(HKEY_LOCAL_MACHINE, NULL));
void WriteRegistryTree(const RegistryKeyData& data, base::win::RegKey* dest) {
// First write values.
for (size_t i = 0; i < data.values.size(); ++i) {
const RegistryValue& value = data.values[i];
dest->WriteValue(value.name.c_str(),
value.data.size() ? &value.data[0] : NULL,
static_cast<DWORD>(value.data.size()),
value.type);
}
// Next write values recursively.
for (std::map<string16, RegistryKeyData>::const_iterator iter =
data.keys.begin();
iter != data.keys.end(); ++iter) {
WriteRegistryTree(iter->second,
&base::win::RegKey(dest->Handle(), iter->first.c_str(),
KEY_ALL_ACCESS));
}
}
// Initialize temporary HKLM/HKCU registry hives used for testing.
// Testing RLZ requires reading and writing to the Windows registry. To keep
// the tests isolated from the machine's state, as well as to prevent the tests
// from causing side effects in the registry, HKCU and HKLM are overridden for
// the duration of the tests. RLZ tests don't expect the HKCU and KHLM hives to
// be empty though, and this function initializes the minimum value needed so
// that the test will run successfully.
void InitializeRegistryOverridesForTesting(
registry_util::RegistryOverrideManager* override_manager) {
// For the moment, the HKCU hive requires no initialization.
const bool do_copy = (base::win::GetVersion() >= base::win::VERSION_WIN7);
RegistryKeyData data;
if (do_copy) {
// Copy the following HKLM subtrees to the temporary location so that the
// win32 APIs used by the tests continue to work:
//
// HKLM\System\CurrentControlSet\Control\Lsa\AccessProviders
//
// This seems to be required since Win7.
ReadRegistryTree(base::win::RegKey(HKEY_LOCAL_MACHINE,
kHKLMAccessProviders,
KEY_READ), &data);
}
override_manager->OverrideRegistry(HKEY_LOCAL_MACHINE, L"rlz_temp_hklm");
override_manager->OverrideRegistry(HKEY_CURRENT_USER, L"rlz_temp_hkcu");
if (do_copy) {
WriteRegistryTree(data, &base::win::RegKey(HKEY_LOCAL_MACHINE,
kHKLMAccessProviders,
KEY_ALL_ACCESS));
}
}
} // namespace
#endif // defined(OS_WIN)
void RlzLibTestNoMachineState::SetUp() {
#if defined(OS_WIN)
OverrideRegistryHives();
InitializeRegistryOverridesForTesting(&override_manager_);
#elif defined(OS_MACOSX)
base::mac::ScopedNSAutoreleasePool pool;
#endif // defined(OS_WIN)
@ -72,11 +139,9 @@ void RlzLibTestNoMachineState::SetUp() {
}
void RlzLibTestNoMachineState::TearDown() {
#if defined(OS_WIN)
UndoOverrideRegistryHives();
#elif defined(OS_POSIX)
#if defined(OS_POSIX)
rlz_lib::testing::SetRlzStoreDirectory(base::FilePath());
#endif // defined(OS_WIN)
#endif // defined(OS_POSIX)
}
void RlzLibTestBase::SetUp() {

@ -14,20 +14,27 @@
#include "base/files/scoped_temp_dir.h"
#endif
#if defined(OS_WIN)
#include "base/test/test_reg_util_win.h"
#endif
class RlzLibTestNoMachineState : public ::testing::Test {
protected:
virtual void SetUp() OVERRIDE;
virtual void TearDown() OVERRIDE;
#if defined(OS_POSIX)
base::ScopedTempDir temp_dir_;
base::ScopedTempDir temp_dir_;
#endif
#if defined(OS_WIN)
registry_util::RegistryOverrideManager override_manager_;
#endif
};
class RlzLibTestBase : public RlzLibTestNoMachineState {
protected:
virtual void SetUp() OVERRIDE;
};
#endif // RLZ_TEST_RLZ_TEST_HELPERS_H

@ -4,7 +4,7 @@
//
// Functions exported by the RLZ DLL.
#include "rlz/win/lib/rlz_lib.h"
#include "rlz/lib/rlz_lib.h"
#define RLZ_DLL_EXPORT extern "C" __declspec(dllexport)

@ -8,7 +8,7 @@
#define RLZ_WIN_LIB_MACHINE_DEAL_H_
#include <string>
#include "rlz/win/lib/rlz_lib.h"
#include "rlz/lib/rlz_lib.h"
namespace rlz_lib {

@ -1,56 +0,0 @@
// Copyright (c) 2012 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
//
// A library to manage RLZ information for access-points shared
// across different client applications.
//
// All functions return true on success and false on error.
// This implemenation is thread safe.
//
// Each prototype mentions the registry access requirements:
//
// HKLM read: Will work from any process and at any privilege level on Vista.
// HKCU read: Calls made from the SYSTEM account must pass the current user's
// SID as the optional 'sid' param. Can be called from low integrity
// process on Vista.
// HKCU write: Calls made from the SYSTEM account must pass the current user's
// SID as the optional 'sid' param. Calls require at least medium
// integrity on Vista (e.g. Toolbar will need to use their broker)
// HKLM write: Calls must be made from an account with admin rights. No SID
// need be passed when running as SYSTEM.
// Functions which do not access registry will be marked with "no restrictions".
#ifndef RLZ_WIN_LIB_RLZ_LIB_H_
#define RLZ_WIN_LIB_RLZ_LIB_H_
// Clients can get away by just including rlz/lib/rlz_lib.h. This file only
// contains function definitions for files used by tests. It's mostly kept
// around for backwards-compatibility.
#include "rlz/lib/rlz_lib.h"
#include "base/win/registry.h"
namespace rlz_lib {
#if defined(OS_WIN)
// Initialize temporary HKLM/HKCU registry hives used for testing.
// Testing RLZ requires reading and writing to the Windows registry. To keep
// the tests isolated from the machine's state, as well as to prevent the tests
// from causing side effects in the registry, HKCU and HKLM are overridden for
// the duration of the tests. RLZ tests don't expect the HKCU and KHLM hives to
// be empty though, and this function initializes the minimum value needed so
// that the test will run successfully.
//
// The two arguments to this function should be the keys that will represent
// the HKLM and HKCU registry hives during the tests. This function should be
// called *before* the hives are overridden.
void InitializeTempHivesForTesting(const base::win::RegKey& temp_hklm_key,
const base::win::RegKey& temp_hkcu_key);
#endif // defined(OS_WIN)
} // namespace rlz_lib
#endif // RLZ_WIN_LIB_RLZ_LIB_H_

@ -5,7 +5,7 @@
// A library to manage RLZ information for access-points shared
// across different client applications.
#include "rlz/win/lib/rlz_lib.h"
#include "rlz/lib/rlz_lib.h"
#include <windows.h>
#include <aclapi.h>
@ -13,42 +13,11 @@
#include "base/basictypes.h"
#include "base/win/registry.h"
#include "base/win/windows_version.h"
#include "rlz/lib/assert.h"
#include "rlz/lib/rlz_value_store.h"
#include "rlz/win/lib/machine_deal.h"
#include "rlz/win/lib/rlz_value_store_registry.h"
namespace {
// Path to recursively copy into the replacemment hives. These are needed
// to make sure certain win32 APIs continue to run correctly once the real
// hives are replaced.
const wchar_t* kHKLMAccessProviders =
L"System\\CurrentControlSet\\Control\\Lsa\\AccessProviders";
// Helper functions
void CopyRegistryTree(const base::win::RegKey& src, base::win::RegKey* dest) {
// First copy values.
for (base::win::RegistryValueIterator i(src.Handle(), L"");
i.Valid(); ++i) {
dest->WriteValue(i.Name(), reinterpret_cast<const void*>(i.Value()),
i.ValueSize(), i.Type());
}
// Next copy subkeys recursively.
for (base::win::RegistryKeyIterator i(src.Handle(), L"");
i.Valid(); ++i) {
base::win::RegKey subkey(dest->Handle(), i.Name(), KEY_ALL_ACCESS);
CopyRegistryTree(base::win::RegKey(src.Handle(), i.Name(), KEY_READ),
&subkey);
}
}
} // namespace anonymous
namespace rlz_lib {
// OEM Deal confirmation storage functions.
@ -237,24 +206,4 @@ bool SetMachineDealCodeFromPingResponse(const char* response) {
return MachineDealCode::SetFromPingResponse(response);
}
void InitializeTempHivesForTesting(const base::win::RegKey& temp_hklm_key,
const base::win::RegKey& temp_hkcu_key) {
// For the moment, the HKCU hive requires no initialization.
if (base::win::GetVersion() >= base::win::VERSION_WIN7) {
// Copy the following HKLM subtrees to the temporary location so that the
// win32 APIs used by the tests continue to work:
//
// HKLM\System\CurrentControlSet\Control\Lsa\AccessProviders
//
// This seems to be required since Win7.
base::win::RegKey dest(temp_hklm_key.Handle(), kHKLMAccessProviders,
KEY_ALL_ACCESS);
CopyRegistryTree(base::win::RegKey(HKEY_LOCAL_MACHINE,
kHKLMAccessProviders,
KEY_READ),
&dest);
}
}
} // namespace rlz_lib