diff options
26 files changed, 361 insertions, 284 deletions
diff --git a/chrome/browser/policy/asynchronous_policy_loader.cc b/chrome/browser/policy/asynchronous_policy_loader.cc index 7cbc5ef..104003a 100644 --- a/chrome/browser/policy/asynchronous_policy_loader.cc +++ b/chrome/browser/policy/asynchronous_policy_loader.cc @@ -8,7 +8,6 @@ #include "base/bind_helpers.h" #include "base/message_loop.h" #include "chrome/browser/policy/policy_bundle.h" -#include "chrome/browser/policy/policy_map.h" #include "content/public/browser/browser_thread.h" using content::BrowserThread; @@ -28,7 +27,7 @@ void AsynchronousPolicyLoader::Init(const UpdateCallback& callback) { update_callback_ = callback; // Load initial policy synchronously at startup. - scoped_ptr<PolicyMap> policy(delegate_->Load()); + scoped_ptr<PolicyBundle> policy(delegate_->Load()); if (policy.get()) UpdatePolicy(policy.Pass()); @@ -58,9 +57,8 @@ AsynchronousPolicyLoader::~AsynchronousPolicyLoader() { void AsynchronousPolicyLoader::Reload(bool force) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); - if (delegate_.get()) { - PostUpdatePolicyTask(delegate_->Load()); - } + if (delegate_.get()) + PostUpdatePolicyTask(delegate_->Load().Pass()); } void AsynchronousPolicyLoader::CancelReloadTask() { @@ -100,21 +98,17 @@ void AsynchronousPolicyLoader::StopOnFileThread() { CancelReloadTask(); } -void AsynchronousPolicyLoader::PostUpdatePolicyTask(PolicyMap* new_policy) { - scoped_ptr<PolicyMap> policy(new_policy); +void AsynchronousPolicyLoader::PostUpdatePolicyTask( + scoped_ptr<PolicyBundle> bundle) { origin_loop_->PostTask( FROM_HERE, base::Bind(&AsynchronousPolicyLoader::UpdatePolicy, - this, base::Passed(&policy))); + this, base::Passed(&bundle))); } -void AsynchronousPolicyLoader::UpdatePolicy(scoped_ptr<PolicyMap> policy) { - if (!stopped_) { - // TODO(joaodasilva): make this load policy from other namespaces too. - scoped_ptr<PolicyBundle> bundle(new PolicyBundle()); - bundle->Get(POLICY_DOMAIN_CHROME, std::string()).Swap(policy.get()); +void AsynchronousPolicyLoader::UpdatePolicy(scoped_ptr<PolicyBundle> bundle) { + if (!stopped_) update_callback_.Run(bundle.Pass()); - } } void AsynchronousPolicyLoader::InitAfterFileThreadAvailable() { diff --git a/chrome/browser/policy/asynchronous_policy_loader.h b/chrome/browser/policy/asynchronous_policy_loader.h index 19fa919..ee62cdc 100644 --- a/chrome/browser/policy/asynchronous_policy_loader.h +++ b/chrome/browser/policy/asynchronous_policy_loader.h @@ -18,7 +18,6 @@ class MessageLoop; namespace policy { class PolicyBundle; -class PolicyMap; // Used by the implementation of asynchronous policy provider to manage the // tasks on the FILE thread that do the heavy lifting of loading policies. @@ -51,9 +50,8 @@ class AsynchronousPolicyLoader friend class base::RefCountedThreadSafe<AsynchronousPolicyLoader>; virtual ~AsynchronousPolicyLoader(); - // Schedules a call to UpdatePolicy on |origin_loop_|. Takes ownership of - // |new_policy|. - void PostUpdatePolicyTask(PolicyMap* new_policy); + // Schedules a call to UpdatePolicy on |origin_loop_|. + void PostUpdatePolicyTask(scoped_ptr<PolicyBundle> bundle); AsynchronousPolicyProvider::Delegate* delegate() { return delegate_.get(); @@ -88,7 +86,7 @@ class AsynchronousPolicyLoader // Invokes the |update_callback_| with a new PolicyBundle that maps // the chrome namespace to |policy|. Must be called on |origin_loop_| so that // it's safe to invoke |update_callback_|. - void UpdatePolicy(scoped_ptr<PolicyMap> policy); + void UpdatePolicy(scoped_ptr<PolicyBundle> policy); // Provides the low-level mechanics for loading policy. scoped_ptr<AsynchronousPolicyProvider::Delegate> delegate_; diff --git a/chrome/browser/policy/asynchronous_policy_loader_unittest.cc b/chrome/browser/policy/asynchronous_policy_loader_unittest.cc index befb626..8828735 100644 --- a/chrome/browser/policy/asynchronous_policy_loader_unittest.cc +++ b/chrome/browser/policy/asynchronous_policy_loader_unittest.cc @@ -20,6 +20,16 @@ using ::testing::_; namespace policy { +namespace { + +void AddSequencedTestPolicy(PolicyBundle* bundle, int* number) { + bundle->Get(POLICY_DOMAIN_CHROME, "") + .Set("id", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, + base::Value::CreateIntegerValue(++(*number))); +} + +} // namespace + class AsynchronousPolicyLoaderTest : public AsynchronousPolicyTestBase { public: AsynchronousPolicyLoaderTest() {} @@ -43,44 +53,35 @@ class AsynchronousPolicyLoaderTest : public AsynchronousPolicyTestBase { DISALLOW_COPY_AND_ASSIGN(AsynchronousPolicyLoaderTest); }; -ACTION(CreateTestPolicyMap) { - return new PolicyMap(); -} - -ACTION_P(CreateSequencedTestPolicyMap, number) { - PolicyMap* test_policy_map = new PolicyMap(); - test_policy_map->Set("id", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - Value::CreateIntegerValue(++(*number))); - return test_policy_map; -} - TEST_F(AsynchronousPolicyLoaderTest, InitialLoad) { - PolicyMap template_policy; - template_policy.Set("test", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - Value::CreateIntegerValue(123)); - PolicyMap* result = new PolicyMap(); - result->CopyFrom(template_policy); + PolicyBundle template_bundle; + template_bundle.Get(POLICY_DOMAIN_CHROME, "") + .Set("test", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, + base::Value::CreateIntegerValue(123)); + template_bundle.Get(POLICY_DOMAIN_EXTENSIONS, "extension-id") + .Set("test", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, + base::Value::CreateIntegerValue(123)); ProviderDelegateMock* delegate = new ProviderDelegateMock(); - EXPECT_CALL(*delegate, Load()).WillOnce(Return(result)); + EXPECT_CALL(*delegate, MockLoad()).WillOnce(Return(&template_bundle)); scoped_refptr<AsynchronousPolicyLoader> loader = new AsynchronousPolicyLoader(delegate, 10); EXPECT_FALSE(bundle_.get()); loader->Init(update_callback_); ASSERT_TRUE(bundle_.get()); - const PolicyMap& chrome_policy = - bundle_->Get(POLICY_DOMAIN_CHROME, std::string()); - EXPECT_TRUE(chrome_policy.Equals(template_policy)); + EXPECT_TRUE(bundle_->Equals(template_bundle)); } // Verify that the fallback policy requests are made. TEST_F(AsynchronousPolicyLoaderTest, InitialLoadWithFallback) { int policy_number = 0; + PolicyBundle bundle0; + AddSequencedTestPolicy(&bundle0, &policy_number); + PolicyBundle bundle1; + AddSequencedTestPolicy(&bundle1, &policy_number); InSequence s; ProviderDelegateMock* delegate = new ProviderDelegateMock(); - EXPECT_CALL(*delegate, Load()).WillOnce( - CreateSequencedTestPolicyMap(&policy_number)); - EXPECT_CALL(*delegate, Load()).WillOnce( - CreateSequencedTestPolicyMap(&policy_number)); + EXPECT_CALL(*delegate, MockLoad()).WillOnce(Return(&bundle0)); + EXPECT_CALL(*delegate, MockLoad()).WillOnce(Return(&bundle1)); scoped_refptr<AsynchronousPolicyLoader> loader = new AsynchronousPolicyLoader(delegate, 10); loader->Init(update_callback_); @@ -91,16 +92,17 @@ TEST_F(AsynchronousPolicyLoaderTest, InitialLoadWithFallback) { const PolicyMap& chrome_policy = bundle_->Get(POLICY_DOMAIN_CHROME, std::string()); base::FundamentalValue expected(policy_number); - EXPECT_TRUE(Value::Equals(&expected, chrome_policy.GetValue("id"))); + EXPECT_TRUE(base::Value::Equals(&expected, chrome_policy.GetValue("id"))); EXPECT_EQ(1U, chrome_policy.size()); } // Ensure that calling stop on the loader stops subsequent reloads from // happening. TEST_F(AsynchronousPolicyLoaderTest, Stop) { + PolicyBundle bundle; ProviderDelegateMock* delegate = new ProviderDelegateMock(); - ON_CALL(*delegate, Load()).WillByDefault(CreateTestPolicyMap()); - EXPECT_CALL(*delegate, Load()).Times(1); + ON_CALL(*delegate, MockLoad()).WillByDefault(Return(&bundle)); + EXPECT_CALL(*delegate, MockLoad()).Times(1); scoped_refptr<AsynchronousPolicyLoader> loader = new AsynchronousPolicyLoader(delegate, 10); loader->Init(update_callback_); @@ -115,12 +117,12 @@ TEST_F(AsynchronousPolicyLoaderTest, Stop) { TEST_F(AsynchronousPolicyLoaderTest, ProviderNotificationOnPolicyChange) { InSequence s; MockConfigurationPolicyObserver observer; - int policy_number_1 = 0; - int policy_number_2 = 0; + int policy_number = 0; + PolicyBundle bundle; + AddSequencedTestPolicy(&bundle, &policy_number); ProviderDelegateMock* delegate = new ProviderDelegateMock(); - EXPECT_CALL(*delegate, Load()).WillOnce( - CreateSequencedTestPolicyMap(&policy_number_1)); + EXPECT_CALL(*delegate, MockLoad()).WillOnce(Return(&bundle)); scoped_refptr<AsynchronousPolicyLoader> loader = new AsynchronousPolicyLoader(delegate, 10); @@ -130,16 +132,15 @@ TEST_F(AsynchronousPolicyLoaderTest, ProviderNotificationOnPolicyChange) { registrar.Init(&provider, &observer); Mock::VerifyAndClearExpectations(delegate); - EXPECT_CALL(*delegate, Load()).WillOnce( - CreateSequencedTestPolicyMap(&policy_number_2)); + EXPECT_CALL(*delegate, MockLoad()).WillOnce(Return(&bundle)); EXPECT_CALL(observer, OnUpdatePolicy(_)).Times(1); provider.RefreshPolicies(); loop_.RunAllPending(); Mock::VerifyAndClearExpectations(delegate); Mock::VerifyAndClearExpectations(&observer); - EXPECT_CALL(*delegate, Load()).WillOnce( - CreateSequencedTestPolicyMap(&policy_number_1)); + AddSequencedTestPolicy(&bundle, &policy_number); + EXPECT_CALL(*delegate, MockLoad()).WillOnce(Return(&bundle)); EXPECT_CALL(observer, OnUpdatePolicy(_)).Times(1); provider.RefreshPolicies(); loop_.RunAllPending(); diff --git a/chrome/browser/policy/asynchronous_policy_provider.h b/chrome/browser/policy/asynchronous_policy_provider.h index 26e85e8..000b8a6 100644 --- a/chrome/browser/policy/asynchronous_policy_provider.h +++ b/chrome/browser/policy/asynchronous_policy_provider.h @@ -16,7 +16,6 @@ namespace policy { class AsynchronousPolicyLoader; class PolicyBundle; -class PolicyMap; // Policy provider that loads policy asynchronously. Providers should subclass // from this class if loading the policy requires disk access or must for some @@ -32,9 +31,8 @@ class AsynchronousPolicyProvider public: virtual ~Delegate() {} - // Load policy from the delegate's source, and return a PolicyMap. Ownership - // is transferred to the caller. - virtual PolicyMap* Load() = 0; + // Load policy from the delegate's source, and return a PolicyBundle. + virtual scoped_ptr<PolicyBundle> Load() = 0; }; // Assumes ownership of |loader|. diff --git a/chrome/browser/policy/asynchronous_policy_provider_unittest.cc b/chrome/browser/policy/asynchronous_policy_provider_unittest.cc index 166526b..65230bb 100644 --- a/chrome/browser/policy/asynchronous_policy_provider_unittest.cc +++ b/chrome/browser/policy/asynchronous_policy_provider_unittest.cc @@ -23,46 +23,36 @@ namespace policy { // Creating the provider should provide initial policy. TEST_F(AsynchronousPolicyTestBase, Provide) { InSequence s; - PolicyMap* policies = new PolicyMap(); - policies->Set(key::kSyncDisabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - Value::CreateBooleanValue(true)); + PolicyBundle bundle; + bundle.Get(POLICY_DOMAIN_CHROME, "") + .Set(key::kSyncDisabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, + base::Value::CreateBooleanValue(true)); ProviderDelegateMock* delegate = new ProviderDelegateMock(); - EXPECT_CALL(*delegate, Load()).WillOnce(Return(policies)); + EXPECT_CALL(*delegate, MockLoad()).WillOnce(Return(&bundle)); AsynchronousPolicyProvider provider( GetChromePolicyDefinitionList(), new AsynchronousPolicyLoader(delegate, 10)); - PolicyMap policy_map; - provider.Provide(&policy_map); - base::FundamentalValue expected(true); - EXPECT_TRUE(Value::Equals(&expected, - policy_map.GetValue(key::kSyncDisabled))); - EXPECT_EQ(1U, policy_map.size()); + EXPECT_TRUE(provider.policies().Equals(bundle)); } - // Trigger a refresh manually and ensure that policy gets reloaded. TEST_F(AsynchronousPolicyTestBase, ProvideAfterRefresh) { InSequence s; - PolicyMap* original_policies = new PolicyMap(); - original_policies->Set(key::kSyncDisabled, POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, Value::CreateBooleanValue(true)); + PolicyBundle original_policies; + original_policies.Get(POLICY_DOMAIN_CHROME, "") + .Set(key::kSyncDisabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, + base::Value::CreateBooleanValue(true)); ProviderDelegateMock* delegate = new ProviderDelegateMock(); - EXPECT_CALL(*delegate, Load()).WillOnce(Return(original_policies)); - PolicyMap* refresh_policies = new PolicyMap(); - refresh_policies->Set(key::kJavascriptEnabled, POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, Value::CreateBooleanValue(true)); - EXPECT_CALL(*delegate, Load()).WillOnce(Return(refresh_policies)); + EXPECT_CALL(*delegate, MockLoad()).WillOnce(Return(&original_policies)); + PolicyBundle refresh_policies; + refresh_policies.Get(POLICY_DOMAIN_CHROME, "") + .Set(key::kJavascriptEnabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, + base::Value::CreateBooleanValue(true)); + EXPECT_CALL(*delegate, MockLoad()).WillOnce(Return(&refresh_policies)); AsynchronousPolicyLoader* loader = new AsynchronousPolicyLoader(delegate, 10); - AsynchronousPolicyProvider provider(GetChromePolicyDefinitionList(), - loader); + AsynchronousPolicyProvider provider(GetChromePolicyDefinitionList(), loader); // The original policies have been loaded. - PolicyMap policy_map; - provider.Provide(&policy_map); - EXPECT_EQ(1U, policy_map.size()); - base::FundamentalValue expected(true); - EXPECT_TRUE(Value::Equals(&expected, - policy_map.GetValue(key::kSyncDisabled))); - EXPECT_FALSE(policy_map.Get(key::kJavascriptEnabled)); + EXPECT_TRUE(provider.policies().Equals(original_policies)); MockConfigurationPolicyObserver observer; ConfigurationPolicyObserverRegistrar registrar; @@ -71,12 +61,7 @@ TEST_F(AsynchronousPolicyTestBase, ProvideAfterRefresh) { provider.RefreshPolicies(); loop_.RunAllPending(); // The refreshed policies are now provided. - policy_map.Clear(); - provider.Provide(&policy_map); - EXPECT_EQ(1U, policy_map.size()); - EXPECT_TRUE(Value::Equals(&expected, - policy_map.GetValue(key::kJavascriptEnabled))); - EXPECT_FALSE(policy_map.Get(key::kSyncDisabled)); + EXPECT_TRUE(provider.policies().Equals(refresh_policies)); } } // namespace policy diff --git a/chrome/browser/policy/asynchronous_policy_test_base.h b/chrome/browser/policy/asynchronous_policy_test_base.h index 887f21d..c47398d 100644 --- a/chrome/browser/policy/asynchronous_policy_test_base.h +++ b/chrome/browser/policy/asynchronous_policy_test_base.h @@ -16,7 +16,7 @@ namespace policy { -class PolicyMap; +class PolicyBundle; // A delegate for testing that can feed arbitrary information to the loader. class ProviderDelegateMock : public AsynchronousPolicyProvider::Delegate { @@ -24,7 +24,21 @@ class ProviderDelegateMock : public AsynchronousPolicyProvider::Delegate { ProviderDelegateMock(); virtual ~ProviderDelegateMock(); - MOCK_METHOD0(Load, PolicyMap*()); + // Load() returns a scoped_ptr<PolicyBundle> but it can't be mocked because + // scoped_ptr is moveable but not copyable. This override forwards the + // call to MockLoad() which returns a PolicyBundle*, and returns a copy + // wrapped in a passed scoped_ptr. + virtual scoped_ptr<PolicyBundle> Load() OVERRIDE { + scoped_ptr<PolicyBundle> bundle; + PolicyBundle* loaded = MockLoad(); + if (loaded) { + bundle.reset(new PolicyBundle()); + bundle->CopyFrom(*loaded); + } + return bundle.Pass(); + } + + MOCK_METHOD0(MockLoad, PolicyBundle*()); private: DISALLOW_COPY_AND_ASSIGN(ProviderDelegateMock); diff --git a/chrome/browser/policy/cloud_policy_provider_unittest.cc b/chrome/browser/policy/cloud_policy_provider_unittest.cc index 8a96ec3..d7919dc 100644 --- a/chrome/browser/policy/cloud_policy_provider_unittest.cc +++ b/chrome/browser/policy/cloud_policy_provider_unittest.cc @@ -10,6 +10,8 @@ #include "chrome/browser/policy/cloud_policy_cache_base.h" #include "chrome/browser/policy/cloud_policy_provider.h" #include "chrome/browser/policy/mock_configuration_policy_provider.h" +#include "chrome/browser/policy/policy_bundle.h" +#include "chrome/browser/policy/policy_map.h" #include "content/test/test_browser_thread.h" #include "policy/policy_constants.h" #include "testing/gmock/include/gmock/gmock.h" @@ -108,7 +110,6 @@ class CloudPolicyProviderTest : public testing::Test { TEST_F(CloudPolicyProviderTest, Initialization) { EXPECT_FALSE(cloud_policy_provider_.IsInitializationComplete()); - // The provider only becomes initialized when it has all caches, and the // caches are ready too. AddDeviceCache(); @@ -118,15 +119,11 @@ TEST_F(CloudPolicyProviderTest, Initialization) { AddUserCache(); EXPECT_FALSE(user_policy_cache_.IsReady()); EXPECT_FALSE(cloud_policy_provider_.IsInitializationComplete()); - - PolicyMap policy; - EXPECT_TRUE(cloud_policy_provider_.Provide(&policy)); - EXPECT_TRUE(policy.empty()); - + PolicyBundle expected_bundle; + EXPECT_TRUE(cloud_policy_provider_.policies().Equals(expected_bundle)); SetUserCacheReady(); EXPECT_TRUE(cloud_policy_provider_.IsInitializationComplete()); - EXPECT_TRUE(cloud_policy_provider_.Provide(&policy)); - EXPECT_TRUE(policy.empty()); + EXPECT_TRUE(cloud_policy_provider_.policies().Equals(expected_bundle)); const std::string kUrl("http://chromium.org"); user_policy_cache_.mutable_policy()->Set(key::kHomepageLocation, @@ -136,14 +133,10 @@ TEST_F(CloudPolicyProviderTest, Initialization) { EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_)); user_policy_cache_.NotifyObservers(); Mock::VerifyAndClearExpectations(&observer_); - EXPECT_TRUE(cloud_policy_provider_.Provide(&policy)); - - PolicyMap expected; - expected.Set(key::kHomepageLocation, - POLICY_LEVEL_MANDATORY, - POLICY_SCOPE_USER, - Value::CreateStringValue(kUrl)); - EXPECT_TRUE(policy.Equals(expected)); + expected_bundle.Get(POLICY_DOMAIN_CHROME, "") + .Set(key::kHomepageLocation, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, + base::Value::CreateStringValue(kUrl)); + EXPECT_TRUE(cloud_policy_provider_.policies().Equals(expected_bundle)); } TEST_F(CloudPolicyProviderTest, RefreshPolicies) { @@ -194,9 +187,8 @@ TEST_F(CloudPolicyProviderTest, MergeProxyPolicies) { SetDeviceCacheReady(); SetUserCacheReady(); EXPECT_TRUE(cloud_policy_provider_.IsInitializationComplete()); - PolicyMap policy; - EXPECT_TRUE(cloud_policy_provider_.Provide(&policy)); - EXPECT_TRUE(policy.empty()); + PolicyBundle expected_bundle; + EXPECT_TRUE(cloud_policy_provider_.policies().Equals(expected_bundle)); // User policy takes precedence over device policy. EXPECT_CALL(observer_, OnUpdatePolicy(&cloud_policy_provider_)); @@ -213,25 +205,13 @@ TEST_F(CloudPolicyProviderTest, MergeProxyPolicies) { user_policy_cache_.NotifyObservers(); Mock::VerifyAndClearExpectations(&observer_); - EXPECT_TRUE(cloud_policy_provider_.Provide(&policy)); - // The deprecated proxy policies are converted to the new ProxySettings. - EXPECT_TRUE(policy.Get(key::kProxyMode) == NULL); - EXPECT_TRUE(policy.Get(key::kProxyServerMode) == NULL); - EXPECT_TRUE(policy.Get(key::kProxyServer) == NULL); - EXPECT_TRUE(policy.Get(key::kProxyPacUrl) == NULL); - - EXPECT_EQ(1u, policy.size()); - const PolicyMap::Entry* entry = policy.Get(key::kProxySettings); - ASSERT_TRUE(entry != NULL); - ASSERT_TRUE(entry->value != NULL); - EXPECT_EQ(POLICY_LEVEL_MANDATORY, entry->level); - EXPECT_EQ(POLICY_SCOPE_USER, entry->scope); - const DictionaryValue* settings; - ASSERT_TRUE(entry->value->GetAsDictionary(&settings)); - std::string mode; - EXPECT_TRUE(settings->GetString(key::kProxyMode, &mode)); - EXPECT_EQ("user mode", mode); + base::DictionaryValue* proxy_settings = new base::DictionaryValue(); + proxy_settings->SetString(key::kProxyMode, "user mode"); + expected_bundle.Get(POLICY_DOMAIN_CHROME, "") + .Set(key::kProxySettings, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, + proxy_settings); + EXPECT_TRUE(cloud_policy_provider_.policies().Equals(expected_bundle)); } #endif diff --git a/chrome/browser/policy/config_dir_policy_provider.cc b/chrome/browser/policy/config_dir_policy_provider.cc index e8cb43c..a98a28c 100644 --- a/chrome/browser/policy/config_dir_policy_provider.cc +++ b/chrome/browser/policy/config_dir_policy_provider.cc @@ -6,13 +6,14 @@ #include <algorithm> #include <set> +#include <string> #include "base/file_path.h" #include "base/file_util.h" #include "base/json/json_file_value_serializer.h" #include "base/logging.h" #include "base/platform_file.h" -#include "chrome/browser/policy/policy_map.h" +#include "chrome/browser/policy/policy_bundle.h" namespace policy { @@ -24,7 +25,7 @@ ConfigDirPolicyProviderDelegate::ConfigDirPolicyProviderDelegate( level_(level), scope_(scope) {} -PolicyMap* ConfigDirPolicyProviderDelegate::Load() { +scoped_ptr<PolicyBundle> ConfigDirPolicyProviderDelegate::Load() { // Enumerate the files and sort them lexicographically. std::set<FilePath> files; file_util::FileEnumerator file_enumerator(config_file_path(), false, @@ -37,7 +38,7 @@ PolicyMap* ConfigDirPolicyProviderDelegate::Load() { // The files are processed in reverse order because |MergeFrom| gives priority // to existing keys, but the ConfigDirPolicyProvider gives priority to the // last file in lexicographic order. - PolicyMap* policy = new PolicyMap; + scoped_ptr<PolicyBundle> bundle(new PolicyBundle()); for (std::set<FilePath>::reverse_iterator config_file_iter = files.rbegin(); config_file_iter != files.rend(); ++config_file_iter) { JSONFileValueSerializer deserializer(*config_file_iter); @@ -58,10 +59,10 @@ PolicyMap* ConfigDirPolicyProviderDelegate::Load() { } PolicyMap file_policy; file_policy.LoadFrom(dictionary_value, level_, scope_); - policy->MergeFrom(file_policy); + bundle->Get(POLICY_DOMAIN_CHROME, std::string()).MergeFrom(file_policy); } - return policy; + return bundle.Pass(); } base::Time ConfigDirPolicyProviderDelegate::GetLastModification() { diff --git a/chrome/browser/policy/config_dir_policy_provider.h b/chrome/browser/policy/config_dir_policy_provider.h index 3c1c626..1a8fbbb 100644 --- a/chrome/browser/policy/config_dir_policy_provider.h +++ b/chrome/browser/policy/config_dir_policy_provider.h @@ -8,7 +8,6 @@ #include "base/time.h" #include "chrome/browser/policy/file_based_policy_provider.h" -#include "chrome/browser/policy/policy_map.h" class FilePath; @@ -39,7 +38,7 @@ class ConfigDirPolicyProviderDelegate PolicyScope scope); // FileBasedPolicyProvider::ProviderDelegate implementation. - virtual PolicyMap* Load() OVERRIDE; + virtual scoped_ptr<PolicyBundle> Load() OVERRIDE; virtual base::Time GetLastModification() OVERRIDE; private: diff --git a/chrome/browser/policy/config_dir_policy_provider_unittest.cc b/chrome/browser/policy/config_dir_policy_provider_unittest.cc index 0f3bbe6..24bb85d 100644 --- a/chrome/browser/policy/config_dir_policy_provider_unittest.cc +++ b/chrome/browser/policy/config_dir_policy_provider_unittest.cc @@ -11,6 +11,8 @@ #include "base/values.h" #include "chrome/browser/policy/config_dir_policy_provider.h" #include "chrome/browser/policy/configuration_policy_provider_test.h" +#include "chrome/browser/policy/policy_bundle.h" +#include "chrome/browser/policy/policy_map.h" namespace policy { @@ -54,7 +56,8 @@ class TestHarness : public PolicyProviderTestHarness { DISALLOW_COPY_AND_ASSIGN(TestHarness); }; -TestHarness::TestHarness() {} +TestHarness::TestHarness() + : PolicyProviderTestHarness(POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE) {} TestHarness::~TestHarness() {} @@ -149,9 +152,10 @@ TEST_F(ConfigDirPolicyLoaderTest, ReadPrefsEmpty) { ConfigDirPolicyProviderDelegate loader(harness_.test_dir(), POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE); - scoped_ptr<PolicyMap> policy(loader.Load()); - EXPECT_TRUE(policy.get()); - EXPECT_TRUE(policy->empty()); + scoped_ptr<PolicyBundle> bundle(loader.Load()); + ASSERT_TRUE(bundle.get()); + const PolicyBundle kEmptyBundle; + EXPECT_TRUE(bundle->Equals(kEmptyBundle)); } // Reading from a non-existent directory should result in an empty preferences @@ -162,9 +166,10 @@ TEST_F(ConfigDirPolicyLoaderTest, ReadPrefsNonExistentDirectory) { ConfigDirPolicyProviderDelegate loader(non_existent_dir, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE); - scoped_ptr<PolicyMap> policy(loader.Load()); - EXPECT_TRUE(policy.get()); - EXPECT_TRUE(policy->empty()); + scoped_ptr<PolicyBundle> bundle(loader.Load()); + ASSERT_TRUE(bundle.get()); + const PolicyBundle kEmptyBundle; + EXPECT_TRUE(bundle->Equals(kEmptyBundle)); } // Test merging values from different files. @@ -186,11 +191,12 @@ TEST_F(ConfigDirPolicyLoaderTest, ReadPrefsMergePrefs) { ConfigDirPolicyProviderDelegate loader(harness_.test_dir(), POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER); - scoped_ptr<PolicyMap> policy(loader.Load()); - EXPECT_TRUE(policy.get()); - PolicyMap expected; - expected.LoadFrom(&test_dict_foo, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER); - EXPECT_TRUE(policy->Equals(expected)); + scoped_ptr<PolicyBundle> bundle(loader.Load()); + ASSERT_TRUE(bundle.get()); + PolicyBundle expected_bundle; + expected_bundle.Get(POLICY_DOMAIN_CHROME, "") + .LoadFrom(&test_dict_foo, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER); + EXPECT_TRUE(bundle->Equals(expected_bundle)); } } // namespace policy diff --git a/chrome/browser/policy/configuration_policy_provider.cc b/chrome/browser/policy/configuration_policy_provider.cc index 435fb9b..fd90b17 100644 --- a/chrome/browser/policy/configuration_policy_provider.cc +++ b/chrome/browser/policy/configuration_policy_provider.cc @@ -21,37 +21,9 @@ const char* kProxyPolicies[] = { key::kProxyBypassList, }; -} // namespace - -ConfigurationPolicyProvider::Observer::~Observer() {} - -void ConfigurationPolicyProvider::Observer::OnProviderGoingAway( - ConfigurationPolicyProvider* provider) {} - -// Class ConfigurationPolicyProvider. - -ConfigurationPolicyProvider::ConfigurationPolicyProvider( - const PolicyDefinitionList* policy_list) - : policy_definition_list_(policy_list) { -} - -ConfigurationPolicyProvider::~ConfigurationPolicyProvider() { - FOR_EACH_OBSERVER(ConfigurationPolicyProvider::Observer, - observer_list_, - OnProviderGoingAway(this)); -} - -bool ConfigurationPolicyProvider::Provide(PolicyMap* result) { - result->CopyFrom(policy_bundle_.Get(POLICY_DOMAIN_CHROME, std::string())); - return true; -} - -bool ConfigurationPolicyProvider::IsInitializationComplete() const { - return true; -} - -// static -void ConfigurationPolicyProvider::FixDeprecatedPolicies(PolicyMap* policies) { +// Helper that converts deprecated chrome policies into their corresponding +// actual policies. +void FixDeprecatedPolicies(PolicyMap* policies) { // Proxy settings have been configured by 5 policies that didn't mix well // together, and maps of policies had to take this into account when merging // policy sources. The proxy settings will eventually be configured by a @@ -89,9 +61,34 @@ void ConfigurationPolicyProvider::FixDeprecatedPolicies(PolicyMap* policies) { } } +} // namespace + +ConfigurationPolicyProvider::Observer::~Observer() {} + +void ConfigurationPolicyProvider::Observer::OnProviderGoingAway( + ConfigurationPolicyProvider* provider) {} + +ConfigurationPolicyProvider::ConfigurationPolicyProvider( + const PolicyDefinitionList* policy_list) + : policy_definition_list_(policy_list) { +} + +ConfigurationPolicyProvider::~ConfigurationPolicyProvider() { + FOR_EACH_OBSERVER(ConfigurationPolicyProvider::Observer, + observer_list_, + OnProviderGoingAway(this)); +} + +bool ConfigurationPolicyProvider::IsInitializationComplete() const { + return true; +} + void ConfigurationPolicyProvider::UpdatePolicy( scoped_ptr<PolicyBundle> bundle) { - policy_bundle_.Swap(bundle.get()); + if (bundle.get()) + policy_bundle_.Swap(bundle.get()); + else + policy_bundle_.Clear(); FixDeprecatedPolicies( &policy_bundle_.Get(POLICY_DOMAIN_CHROME, std::string())); FOR_EACH_OBSERVER(ConfigurationPolicyProvider::Observer, diff --git a/chrome/browser/policy/configuration_policy_provider.h b/chrome/browser/policy/configuration_policy_provider.h index f12b285..d7b0e02 100644 --- a/chrome/browser/policy/configuration_policy_provider.h +++ b/chrome/browser/policy/configuration_policy_provider.h @@ -14,7 +14,6 @@ namespace policy { struct PolicyDefinitionList; -class PolicyMap; // A mostly-abstract super class for platform-specific policy providers. // Platform-specific policy providers (Windows Group Policy, gconf, @@ -32,13 +31,6 @@ class ConfigurationPolicyProvider { virtual ~ConfigurationPolicyProvider(); - // Fills the given |result| with the current policy values. Returns true if - // the policies were provided. This is used mainly by the - // ConfigurationPolicyPrefStore, which retrieves policy values from here. - // DEPRECATED: this call is going away, use policies() instead. - // http://crbug.com/108993 - bool Provide(PolicyMap* result); - // Returns the current PolicyBundle. const PolicyBundle& policies() const { return policy_bundle_; } @@ -54,11 +46,6 @@ class ConfigurationPolicyProvider { // OnUpdatePolicy won't be called if that happens. virtual void RefreshPolicies() = 0; - // Utility method that converts deprecated policies into their corresponding - // actual policies. Subclasses can use this to fix deprecated policies in - // PolicyMaps that they obtained from elsewhere. - static void FixDeprecatedPolicies(PolicyMap* policies); - protected: // Subclasses must invoke this to update the policies currently served by // this provider. UpdatePolicy() takes ownership of |policies|. diff --git a/chrome/browser/policy/configuration_policy_provider_delegate_win.cc b/chrome/browser/policy/configuration_policy_provider_delegate_win.cc index e1136ac..f430e55 100644 --- a/chrome/browser/policy/configuration_policy_provider_delegate_win.cc +++ b/chrome/browser/policy/configuration_policy_provider_delegate_win.cc @@ -4,6 +4,8 @@ #include "chrome/browser/policy/configuration_policy_provider_delegate_win.h" +#include <string> + #include <string.h> #include "base/basictypes.h" @@ -14,6 +16,8 @@ #include "base/utf_string_conversions.h" #include "base/values.h" #include "base/win/registry.h" +#include "chrome/browser/policy/policy_bundle.h" +#include "chrome/browser/policy/policy_map.h" #include "policy/policy_constants.h" using base::win::RegKey; @@ -54,8 +58,9 @@ ConfigurationPolicyProviderDelegateWin::ConfigurationPolicyProviderDelegateWin( registry_key_(registry_key), level_(level) {} -PolicyMap* ConfigurationPolicyProviderDelegateWin::Load() { - PolicyMap* result = new PolicyMap(); +scoped_ptr<PolicyBundle> ConfigurationPolicyProviderDelegateWin::Load() { + scoped_ptr<PolicyBundle> bundle(new PolicyBundle()); + PolicyMap& chrome_policy = bundle->Get(POLICY_DOMAIN_CHROME, std::string()); const PolicyDefinitionList::Entry* current; for (current = policy_definition_list_->begin; current != policy_definition_list_->end; @@ -111,9 +116,9 @@ PolicyMap* ConfigurationPolicyProviderDelegateWin::Load() { NOTREACHED(); } if (value) - result->Set(current->name, level_, scope, value); + chrome_policy.Set(current->name, level_, scope, value); } - return result; + return bundle.Pass(); } bool ConfigurationPolicyProviderDelegateWin::GetRegistryPolicyString( diff --git a/chrome/browser/policy/configuration_policy_provider_delegate_win.h b/chrome/browser/policy/configuration_policy_provider_delegate_win.h index 8fc2385..013f8a8 100644 --- a/chrome/browser/policy/configuration_policy_provider_delegate_win.h +++ b/chrome/browser/policy/configuration_policy_provider_delegate_win.h @@ -8,7 +8,6 @@ #include "base/string16.h" #include "chrome/browser/policy/asynchronous_policy_provider.h" -#include "chrome/browser/policy/policy_map.h" namespace policy { @@ -22,7 +21,7 @@ class ConfigurationPolicyProviderDelegateWin virtual ~ConfigurationPolicyProviderDelegateWin() {} // AsynchronousPolicyProvider::Delegate overrides: - virtual PolicyMap* Load() OVERRIDE; + virtual scoped_ptr<PolicyBundle> Load() OVERRIDE; private: // Methods to perform type-specific policy lookups in the registry. diff --git a/chrome/browser/policy/configuration_policy_provider_mac.cc b/chrome/browser/policy/configuration_policy_provider_mac.cc index 3d0d89a..ce03b14 100644 --- a/chrome/browser/policy/configuration_policy_provider_mac.cc +++ b/chrome/browser/policy/configuration_policy_provider_mac.cc @@ -4,6 +4,8 @@ #include "chrome/browser/policy/configuration_policy_provider_mac.h" +#include <string> + #include "base/file_path.h" #include "base/file_util.h" #include "base/mac/foundation_util.h" @@ -12,6 +14,7 @@ #include "base/platform_file.h" #include "base/sys_string_conversions.h" #include "base/values.h" +#include "chrome/browser/policy/policy_bundle.h" #include "chrome/browser/policy/policy_map.h" #include "chrome/browser/preferences_mac.h" #include "chrome/common/chrome_paths.h" @@ -84,9 +87,10 @@ MacPreferencesPolicyProviderDelegate::MacPreferencesPolicyProviderDelegate( MacPreferencesPolicyProviderDelegate::~MacPreferencesPolicyProviderDelegate() {} -PolicyMap* MacPreferencesPolicyProviderDelegate::Load() { +scoped_ptr<PolicyBundle> MacPreferencesPolicyProviderDelegate::Load() { preferences_->AppSynchronize(kCFPreferencesCurrentApplication); - PolicyMap* policies = new PolicyMap; + scoped_ptr<PolicyBundle> bundle(new PolicyBundle()); + PolicyMap& chrome_policy = bundle->Get(POLICY_DOMAIN_CHROME, std::string()); const PolicyDefinitionList::Entry* current; for (current = policy_list_->begin; current != policy_list_->end; ++current) { @@ -107,10 +111,10 @@ PolicyMap* MacPreferencesPolicyProviderDelegate::Load() { base::Value* policy = CreateValueFromProperty(value); if (policy) - policies->Set(current->name, level_, POLICY_SCOPE_USER, policy); + chrome_policy.Set(current->name, level_, POLICY_SCOPE_USER, policy); } - return policies; + return bundle.Pass(); } base::Time MacPreferencesPolicyProviderDelegate::GetLastModification() { diff --git a/chrome/browser/policy/configuration_policy_provider_mac.h b/chrome/browser/policy/configuration_policy_provider_mac.h index 6edd765..6f51e75 100644 --- a/chrome/browser/policy/configuration_policy_provider_mac.h +++ b/chrome/browser/policy/configuration_policy_provider_mac.h @@ -10,7 +10,6 @@ #include "base/memory/scoped_ptr.h" #include "chrome/browser/policy/file_based_policy_provider.h" -#include "chrome/browser/policy/policy_map.h" class MacPreferences; @@ -32,7 +31,7 @@ class MacPreferencesPolicyProviderDelegate virtual ~MacPreferencesPolicyProviderDelegate(); // FileBasedPolicyLoader::Delegate implementation. - virtual PolicyMap* Load() OVERRIDE; + virtual scoped_ptr<PolicyBundle> Load() OVERRIDE; virtual base::Time GetLastModification() OVERRIDE; // Converts a CFPropertyListRef to the equivalent base::Value. CFDictionary diff --git a/chrome/browser/policy/configuration_policy_provider_mac_unittest.cc b/chrome/browser/policy/configuration_policy_provider_mac_unittest.cc index de73d31..adcc8e6 100644 --- a/chrome/browser/policy/configuration_policy_provider_mac_unittest.cc +++ b/chrome/browser/policy/configuration_policy_provider_mac_unittest.cc @@ -11,6 +11,7 @@ #include "chrome/browser/policy/asynchronous_policy_test_base.h" #include "chrome/browser/policy/configuration_policy_provider_mac.h" #include "chrome/browser/policy/configuration_policy_provider_test.h" +#include "chrome/browser/policy/policy_bundle.h" #include "chrome/browser/policy/policy_map.h" #include "chrome/browser/preferences_mock_mac.h" #include "policy/policy_constants.h" @@ -147,7 +148,8 @@ class TestHarness : public PolicyProviderTestHarness { DISALLOW_COPY_AND_ASSIGN(TestHarness); }; -TestHarness::TestHarness() {} +TestHarness::TestHarness() + : PolicyProviderTestHarness(POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER) {} TestHarness::~TestHarness() {} @@ -254,11 +256,9 @@ TEST_F(ConfigurationPolicyProviderMacTest, Invalid) { mandatory_provider_.RefreshPolicies(); recommended_provider_.RefreshPolicies(); loop_.RunAllPending(); - PolicyMap policy_map; - EXPECT_TRUE(mandatory_provider_.Provide(&policy_map)); - EXPECT_TRUE(policy_map.empty()); - EXPECT_TRUE(recommended_provider_.Provide(&policy_map)); - EXPECT_TRUE(policy_map.empty()); + const PolicyBundle kEmptyBundle; + EXPECT_TRUE(mandatory_provider_.policies().Equals(kEmptyBundle)); + EXPECT_TRUE(recommended_provider_.policies().Equals(kEmptyBundle)); } TEST_F(ConfigurationPolicyProviderMacTest, TestNonForcedValue) { @@ -274,11 +274,12 @@ TEST_F(ConfigurationPolicyProviderMacTest, TestNonForcedValue) { mandatory_provider_.RefreshPolicies(); recommended_provider_.RefreshPolicies(); loop_.RunAllPending(); - PolicyMap policy_map; - EXPECT_TRUE(mandatory_provider_.Provide(&policy_map)); - EXPECT_TRUE(policy_map.empty()); - EXPECT_TRUE(recommended_provider_.Provide(&policy_map)); - EXPECT_EQ(1U, policy_map.size()); + PolicyBundle expected_bundle; + EXPECT_TRUE(mandatory_provider_.policies().Equals(expected_bundle)); + expected_bundle.Get(POLICY_DOMAIN_CHROME, "") + .Set(test_policy_definitions::kKeyString, POLICY_LEVEL_RECOMMENDED, + POLICY_SCOPE_USER, base::Value::CreateStringValue("string value")); + EXPECT_TRUE(recommended_provider_.policies().Equals(expected_bundle)); } TEST_F(ConfigurationPolicyProviderMacTest, TestConversions) { diff --git a/chrome/browser/policy/configuration_policy_provider_test.cc b/chrome/browser/policy/configuration_policy_provider_test.cc index 0ffa0e4..3f1084f 100644 --- a/chrome/browser/policy/configuration_policy_provider_test.cc +++ b/chrome/browser/policy/configuration_policy_provider_test.cc @@ -5,11 +5,13 @@ #include "chrome/browser/policy/configuration_policy_provider_test.h" #include "base/bind.h" +#include "base/memory/scoped_ptr.h" #include "base/values.h" #include "chrome/browser/policy/asynchronous_policy_loader.h" #include "chrome/browser/policy/asynchronous_policy_provider.h" #include "chrome/browser/policy/configuration_policy_provider.h" #include "chrome/browser/policy/mock_configuration_policy_provider.h" +#include "chrome/browser/policy/policy_bundle.h" #include "chrome/browser/policy/policy_map.h" #include "policy/policy_constants.h" #include "testing/gmock/include/gmock/gmock.h" @@ -41,10 +43,20 @@ const PolicyDefinitionList kList = { } // namespace test_policy_definitions -PolicyProviderTestHarness::PolicyProviderTestHarness() {} +PolicyProviderTestHarness::PolicyProviderTestHarness(PolicyLevel level, + PolicyScope scope) + : level_(level), scope_(scope) {} PolicyProviderTestHarness::~PolicyProviderTestHarness() {} +PolicyLevel PolicyProviderTestHarness::policy_level() const { + return level_; +} + +PolicyScope PolicyProviderTestHarness::policy_scope() const { + return scope_; +} + ConfigurationPolicyProviderTest::ConfigurationPolicyProviderTest() {} ConfigurationPolicyProviderTest::~ConfigurationPolicyProviderTest() {} @@ -61,9 +73,8 @@ void ConfigurationPolicyProviderTest::SetUp() { // are fired now. loop_.RunAllPending(); - PolicyMap policy_map; - EXPECT_TRUE(provider_->Provide(&policy_map)); - EXPECT_TRUE(policy_map.empty()); + const PolicyBundle kEmptyBundle; + EXPECT_TRUE(provider_->policies().Equals(kEmptyBundle)); } void ConfigurationPolicyProviderTest::TearDown() { @@ -81,19 +92,22 @@ void ConfigurationPolicyProviderTest::CheckValue( install_value.Run(); provider_->RefreshPolicies(); loop_.RunAllPending(); - PolicyMap policy_map; - EXPECT_TRUE(provider_->Provide(&policy_map)); - EXPECT_EQ(1U, policy_map.size()); - EXPECT_TRUE(base::Value::Equals(&expected_value, - policy_map.GetValue(policy_name))); + PolicyBundle expected_bundle; + expected_bundle.Get(POLICY_DOMAIN_CHROME, "") + .Set(policy_name, + test_harness_->policy_level(), + test_harness_->policy_scope(), + expected_value.DeepCopy()); + EXPECT_TRUE(provider_->policies().Equals(expected_bundle)); + // TODO(joaodasilva): set the policy in the POLICY_DOMAIN_EXTENSIONS too, + // and extend the |expected_bundle|, once all providers are ready. } TEST_P(ConfigurationPolicyProviderTest, Empty) { provider_->RefreshPolicies(); loop_.RunAllPending(); - PolicyMap policy_map; - EXPECT_TRUE(provider_->Provide(&policy_map)); - EXPECT_TRUE(policy_map.empty()); + const PolicyBundle kEmptyBundle; + EXPECT_TRUE(provider_->policies().Equals(kEmptyBundle)); } TEST_P(ConfigurationPolicyProviderTest, StringValue) { @@ -173,9 +187,8 @@ TEST_P(ConfigurationPolicyProviderTest, DictionaryValue) { } TEST_P(ConfigurationPolicyProviderTest, RefreshPolicies) { - PolicyMap policy_map; - EXPECT_TRUE(provider_->Provide(&policy_map)); - EXPECT_EQ(0U, policy_map.size()); + PolicyBundle bundle; + EXPECT_TRUE(provider_->policies().Equals(bundle)); // OnUpdatePolicy is called even when there are no changes. MockConfigurationPolicyObserver observer; @@ -186,8 +199,7 @@ TEST_P(ConfigurationPolicyProviderTest, RefreshPolicies) { loop_.RunAllPending(); Mock::VerifyAndClearExpectations(&observer); - EXPECT_TRUE(provider_->Provide(&policy_map)); - EXPECT_EQ(0U, policy_map.size()); + EXPECT_TRUE(provider_->policies().Equals(bundle)); // OnUpdatePolicy is called when there are changes. test_harness_->InstallStringPolicy(test_policy_definitions::kKeyString, @@ -197,9 +209,12 @@ TEST_P(ConfigurationPolicyProviderTest, RefreshPolicies) { loop_.RunAllPending(); Mock::VerifyAndClearExpectations(&observer); - policy_map.Clear(); - EXPECT_TRUE(provider_->Provide(&policy_map)); - EXPECT_EQ(1U, policy_map.size()); + bundle.Get(POLICY_DOMAIN_CHROME, "") + .Set(test_policy_definitions::kKeyString, + test_harness_->policy_level(), + test_harness_->policy_scope(), + base::Value::CreateStringValue("value")); + EXPECT_TRUE(provider_->policies().Equals(bundle)); } TEST(ConfigurationPolicyProviderTest, FixDeprecatedPolicies) { @@ -220,12 +235,16 @@ TEST(ConfigurationPolicyProviderTest, FixDeprecatedPolicies) { POLICY_SCOPE_USER, base::Value::CreateStringValue("http://example.com/wpad.dat")); - ConfigurationPolicyProvider::FixDeprecatedPolicies(&policy_map); - base::DictionaryValue expected; - expected.SetInteger(key::kProxyServerMode, 3); - EXPECT_EQ(1U, policy_map.size()); - EXPECT_TRUE(base::Value::Equals(&expected, - policy_map.GetValue(key::kProxySettings))); + MockConfigurationPolicyProvider provider; + provider.UpdateChromePolicy(policy_map); + + PolicyBundle expected_bundle; + base::DictionaryValue* expected_value = new base::DictionaryValue(); + expected_value->SetInteger(key::kProxyServerMode, 3); + expected_bundle.Get(POLICY_DOMAIN_CHROME, "") + .Set(key::kProxySettings, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, + expected_value); + EXPECT_TRUE(provider.policies().Equals(expected_bundle)); } } // namespace policy diff --git a/chrome/browser/policy/configuration_policy_provider_test.h b/chrome/browser/policy/configuration_policy_provider_test.h index b77c11e..7782036 100644 --- a/chrome/browser/policy/configuration_policy_provider_test.h +++ b/chrome/browser/policy/configuration_policy_provider_test.h @@ -48,7 +48,9 @@ extern const PolicyDefinitionList kList; // ConfigurationPolicyProviderTest below. class PolicyProviderTestHarness { public: - PolicyProviderTestHarness(); + // |level| and |scope| are the level and scope of the policies returned by + // the providers from CreateProvider(). + PolicyProviderTestHarness(PolicyLevel level, PolicyScope scope); virtual ~PolicyProviderTestHarness(); // Actions to run at gtest SetUp() time. @@ -58,6 +60,10 @@ class PolicyProviderTestHarness { virtual AsynchronousPolicyProvider* CreateProvider( const PolicyDefinitionList* policy_definition_list) = 0; + // Returns the policy level and scope set by the policy provider. + PolicyLevel policy_level() const; + PolicyScope policy_scope() const; + // Helpers to configure the environment the policy provider reads from. virtual void InstallEmptyPolicy() = 0; virtual void InstallStringPolicy(const std::string& policy_name, @@ -73,6 +79,9 @@ class PolicyProviderTestHarness { const base::DictionaryValue* policy_value) = 0; private: + PolicyLevel level_; + PolicyScope scope_; + DISALLOW_COPY_AND_ASSIGN(PolicyProviderTestHarness); }; diff --git a/chrome/browser/policy/configuration_policy_provider_win_unittest.cc b/chrome/browser/policy/configuration_policy_provider_win_unittest.cc index 008fdc1..80e534e 100644 --- a/chrome/browser/policy/configuration_policy_provider_win_unittest.cc +++ b/chrome/browser/policy/configuration_policy_provider_win_unittest.cc @@ -13,6 +13,7 @@ #include "chrome/browser/policy/asynchronous_policy_test_base.h" #include "chrome/browser/policy/configuration_policy_provider_test.h" #include "chrome/browser/policy/configuration_policy_provider_win.h" +#include "chrome/browser/policy/policy_bundle.h" #include "chrome/browser/policy/policy_map.h" #include "policy/policy_constants.h" #include "testing/gtest/include/gtest/gtest.h" @@ -58,7 +59,7 @@ class ScopedGroupPolicyRegistrySandbox { class TestHarness : public PolicyProviderTestHarness { public: - explicit TestHarness(HKEY hive); + explicit TestHarness(HKEY hive, PolicyScope scope); virtual ~TestHarness(); virtual void SetUp() OVERRIDE; @@ -131,8 +132,8 @@ void ScopedGroupPolicyRegistrySandbox::DeleteKeys() { key.DeleteKey(L""); } -TestHarness::TestHarness(HKEY hive) - : hive_(hive) {} +TestHarness::TestHarness(HKEY hive, PolicyScope scope) + : PolicyProviderTestHarness(POLICY_LEVEL_MANDATORY, scope), hive_(hive) {} TestHarness::~TestHarness() {} @@ -199,12 +200,12 @@ void TestHarness::InstallDictionaryPolicy( // static PolicyProviderTestHarness* TestHarness::CreateHKCU() { - return new TestHarness(HKEY_CURRENT_USER); + return new TestHarness(HKEY_CURRENT_USER, POLICY_SCOPE_USER); } // static PolicyProviderTestHarness* TestHarness::CreateHKLM() { - return new TestHarness(HKEY_LOCAL_MACHINE); + return new TestHarness(HKEY_LOCAL_MACHINE, POLICY_SCOPE_MACHINE); } } // namespace @@ -243,11 +244,13 @@ TEST_F(ConfigurationPolicyProviderWinTest, HKLMOverHKCU) { provider_.RefreshPolicies(); loop_.RunAllPending(); - PolicyMap policy_map; - provider_.Provide(&policy_map); - const base::Value* value = - policy_map.GetValue(test_policy_definitions::kKeyString); - EXPECT_TRUE(base::StringValue("hklm").Equals(value)); + PolicyBundle expected_bundle; + expected_bundle.Get(POLICY_DOMAIN_CHROME, "") + .Set(test_policy_definitions::kKeyString, + POLICY_LEVEL_MANDATORY, + POLICY_SCOPE_MACHINE, + base::Value::CreateStringValue("hklm")); + EXPECT_TRUE(provider_.policies().Equals(expected_bundle)); } } // namespace policy diff --git a/chrome/browser/policy/file_based_policy_loader.cc b/chrome/browser/policy/file_based_policy_loader.cc index 763cb99..c0430f7 100644 --- a/chrome/browser/policy/file_based_policy_loader.cc +++ b/chrome/browser/policy/file_based_policy_loader.cc @@ -6,7 +6,7 @@ #include "base/files/file_path_watcher.h" #include "base/memory/ref_counted.h" -#include "chrome/browser/policy/policy_map.h" +#include "chrome/browser/policy/policy_bundle.h" #include "content/public/browser/browser_thread.h" using ::base::files::FilePathWatcher; @@ -74,7 +74,8 @@ void FileBasedPolicyLoader::Reload(bool force) { DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE)); if (!delegate()) { - PostUpdatePolicyTask(NULL); + scoped_ptr<PolicyBundle> empty_bundle; + PostUpdatePolicyTask(empty_bundle.Pass()); return; } @@ -87,7 +88,7 @@ void FileBasedPolicyLoader::Reload(bool force) { } // Load the policy definitions. - scoped_ptr<PolicyMap> new_policy(delegate()->Load()); + scoped_ptr<PolicyBundle> bundle(delegate()->Load()); // Check again in case the directory has changed while reading it. if (!force && !IsSafeToReloadPolicy(now, &delay)) { @@ -95,7 +96,7 @@ void FileBasedPolicyLoader::Reload(bool force) { return; } - PostUpdatePolicyTask(new_policy.release()); + PostUpdatePolicyTask(bundle.Pass()); ScheduleFallbackReloadTask(); } diff --git a/chrome/browser/policy/file_based_policy_provider.h b/chrome/browser/policy/file_based_policy_provider.h index 6480b44..dbc8f59 100644 --- a/chrome/browser/policy/file_based_policy_provider.h +++ b/chrome/browser/policy/file_based_policy_provider.h @@ -25,7 +25,7 @@ class FileBasedPolicyProvider : public AsynchronousPolicyProvider { virtual ~ProviderDelegate(); // AsynchronousPolicyProvider::Delegate implementation: - virtual PolicyMap* Load() = 0; + virtual scoped_ptr<PolicyBundle> Load() = 0; // Gets the last modification timestamp for the policy information from the // filesystem. Returns base::Time() if the information is not present, in diff --git a/chrome/browser/policy/file_based_policy_provider_unittest.cc b/chrome/browser/policy/file_based_policy_provider_unittest.cc index bdfc4e7..8699f68 100644 --- a/chrome/browser/policy/file_based_policy_provider_unittest.cc +++ b/chrome/browser/policy/file_based_policy_provider_unittest.cc @@ -9,6 +9,7 @@ #include "chrome/browser/policy/configuration_policy_provider.h" #include "chrome/browser/policy/file_based_policy_provider.h" #include "chrome/browser/policy/mock_configuration_policy_provider.h" +#include "chrome/browser/policy/policy_bundle.h" #include "chrome/browser/policy/policy_map.h" #include "policy/policy_constants.h" #include "testing/gmock/include/gmock/gmock.h" @@ -25,7 +26,22 @@ class FileBasedPolicyProviderDelegateMock public: FileBasedPolicyProviderDelegateMock() : FileBasedPolicyProvider::ProviderDelegate(FilePath()) {} - MOCK_METHOD0(Load, PolicyMap*()); + + // Load() returns a scoped_ptr<PolicyBundle> but it can't be mocked because + // scoped_ptr is moveable but not copyable. This override forwards the + // call to MockLoad() which returns a PolicyBundle*, and returns a copy + // wrapped in a passed scoped_ptr. + virtual scoped_ptr<PolicyBundle> Load() OVERRIDE { + scoped_ptr<PolicyBundle> bundle; + PolicyBundle* loaded = MockLoad(); + if (loaded) { + bundle.reset(new PolicyBundle()); + bundle->CopyFrom(*loaded); + } + return bundle.Pass(); + } + + MOCK_METHOD0(MockLoad, PolicyBundle*()); MOCK_METHOD0(GetLastModification, base::Time()); }; @@ -36,23 +52,20 @@ TEST_F(AsynchronousPolicyTestBase, ProviderInit) { EXPECT_CALL(*provider_delegate, GetLastModification()).WillRepeatedly( Return(last_modified)); InSequence s; - EXPECT_CALL(*provider_delegate, Load()).WillOnce(Return(new PolicyMap)); - PolicyMap* policies = new PolicyMap(); - policies->Set(key::kSyncDisabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - Value::CreateBooleanValue(true)); + PolicyBundle empty_bundle; + EXPECT_CALL(*provider_delegate, MockLoad()).WillOnce(Return(&empty_bundle)); + PolicyBundle bundle; + bundle.Get(POLICY_DOMAIN_CHROME, "") + .Set(key::kSyncDisabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, + base::Value::CreateBooleanValue(true)); // A second call to Load gets triggered during the provider's construction // when the file watcher is initialized, since this file may have changed // between the initial load and creating watcher. - EXPECT_CALL(*provider_delegate, Load()).WillOnce(Return(policies)); + EXPECT_CALL(*provider_delegate, MockLoad()).WillOnce(Return(&bundle)); FileBasedPolicyProvider provider(GetChromePolicyDefinitionList(), provider_delegate); loop_.RunAllPending(); - PolicyMap policy_map; - provider.Provide(&policy_map); - base::FundamentalValue expected(true); - EXPECT_TRUE(Value::Equals(&expected, - policy_map.GetValue(key::kSyncDisabled))); - EXPECT_EQ(1U, policy_map.size()); + EXPECT_TRUE(provider.policies().Equals(bundle)); } TEST_F(AsynchronousPolicyTestBase, ProviderRefresh) { @@ -62,32 +75,29 @@ TEST_F(AsynchronousPolicyTestBase, ProviderRefresh) { EXPECT_CALL(*provider_delegate, GetLastModification()).WillRepeatedly( Return(last_modified)); InSequence s; - EXPECT_CALL(*provider_delegate, Load()).WillOnce(Return(new PolicyMap)); + PolicyBundle empty_bundle; + EXPECT_CALL(*provider_delegate, MockLoad()).WillOnce(Return(&empty_bundle)); FileBasedPolicyProvider file_based_provider(GetChromePolicyDefinitionList(), provider_delegate); // A second call to Load gets triggered during the provider's construction // when the file watcher is initialized, since this file may have changed // between the initial load and creating watcher. - EXPECT_CALL(*provider_delegate, Load()).WillOnce(Return(new PolicyMap)); + EXPECT_CALL(*provider_delegate, MockLoad()).WillOnce(Return(&empty_bundle)); loop_.RunAllPending(); // A third and final call to Load is made by the explicit Reload. This // should be the one that provides the current policy. - PolicyMap* policies = new PolicyMap(); - policies->Set(key::kSyncDisabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, - Value::CreateBooleanValue(true)); - EXPECT_CALL(*provider_delegate, Load()).WillOnce(Return(policies)); + PolicyBundle bundle; + bundle.Get(POLICY_DOMAIN_CHROME, "") + .Set(key::kSyncDisabled, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, + base::Value::CreateBooleanValue(true)); + EXPECT_CALL(*provider_delegate, MockLoad()).WillOnce(Return(&bundle)); MockConfigurationPolicyObserver observer; ConfigurationPolicyObserverRegistrar registrar; registrar.Init(&file_based_provider, &observer); EXPECT_CALL(observer, OnUpdatePolicy(&file_based_provider)).Times(1); file_based_provider.RefreshPolicies(); loop_.RunAllPending(); - PolicyMap policy_map; - file_based_provider.Provide(&policy_map); - base::FundamentalValue expected(true); - EXPECT_TRUE(Value::Equals(&expected, - policy_map.GetValue(key::kSyncDisabled))); - EXPECT_EQ(1U, policy_map.size()); + EXPECT_TRUE(file_based_provider.policies().Equals(bundle)); } } // namespace policy diff --git a/chrome/browser/policy/policy_bundle.cc b/chrome/browser/policy/policy_bundle.cc index ed852cd..b589714 100644 --- a/chrome/browser/policy/policy_bundle.cc +++ b/chrome/browser/policy/policy_bundle.cc @@ -86,6 +86,31 @@ void PolicyBundle::MergeFrom(const PolicyBundle& other) { } } +bool PolicyBundle::Equals(const PolicyBundle& other) const { + // Equals() has the peculiarity that an entry with an empty PolicyMap equals + // an non-existant entry. This handles usage of non-const Get() that doesn't + // insert any policies. + const_iterator it_this = begin(); + const_iterator it_other = other.begin(); + + while (true) { + // Skip empty PolicyMaps. + while (it_this != end() && it_this->second->empty()) + ++it_this; + while (it_other != other.end() && it_other->second->empty()) + ++it_other; + if (it_this == end() || it_other == other.end()) + break; + if (it_this->first != it_other->first || + !it_this->second->Equals(*it_other->second)) { + return false; + } + ++it_this; + ++it_other; + } + return it_this == end() && it_other == other.end(); +} + PolicyBundle::const_iterator PolicyBundle::begin() const { return policy_bundle_.begin(); } diff --git a/chrome/browser/policy/policy_bundle.h b/chrome/browser/policy/policy_bundle.h index c17ef99..c2df8a9 100644 --- a/chrome/browser/policy/policy_bundle.h +++ b/chrome/browser/policy/policy_bundle.h @@ -49,6 +49,9 @@ class PolicyBundle { // See PolicyMap::MergeFrom for details on merging individual PolicyMaps. void MergeFrom(const PolicyBundle& other); + // Returns true if |other| has the same keys and value as |this|. + bool Equals(const PolicyBundle& other) const; + // Returns iterators to the beginning and end of the underlying container. // These can be used to iterate over and read the PolicyMaps, but not to // modify them. diff --git a/chrome/browser/policy/policy_bundle_unittest.cc b/chrome/browser/policy/policy_bundle_unittest.cc index 6c955aa..2d5ebae 100644 --- a/chrome/browser/policy/policy_bundle_unittest.cc +++ b/chrome/browser/policy/policy_bundle_unittest.cc @@ -188,4 +188,43 @@ TEST(PolicyBundleTest, MergeFrom) { merged.Get(POLICY_DOMAIN_EXTENSIONS, kExtension2).Equals(policy2)); } +TEST(PolicyBundleTest, Equals) { + PolicyBundle bundle; + AddTestPolicies(&bundle.Get(POLICY_DOMAIN_CHROME, std::string())); + AddTestPolicies(&bundle.Get(POLICY_DOMAIN_EXTENSIONS, kExtension0)); + + PolicyBundle other; + EXPECT_FALSE(bundle.Equals(other)); + other.CopyFrom(bundle); + EXPECT_TRUE(bundle.Equals(other)); + + AddTestPolicies(&bundle.Get(POLICY_DOMAIN_EXTENSIONS, kExtension1)); + EXPECT_FALSE(bundle.Equals(other)); + other.CopyFrom(bundle); + EXPECT_TRUE(bundle.Equals(other)); + AddTestPolicies(&other.Get(POLICY_DOMAIN_EXTENSIONS, kExtension2)); + EXPECT_FALSE(bundle.Equals(other)); + + other.CopyFrom(bundle); + bundle.Get(POLICY_DOMAIN_CHROME, std::string()) + .Set(kPolicy0, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, + base::Value::CreateIntegerValue(123)); + EXPECT_FALSE(bundle.Equals(other)); + other.CopyFrom(bundle); + EXPECT_TRUE(bundle.Equals(other)); + bundle.Get(POLICY_DOMAIN_CHROME, std::string()) + .Set(kPolicy0, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE, + base::Value::CreateIntegerValue(123)); + EXPECT_FALSE(bundle.Equals(other)); + + // Test non-const Get(). + bundle.Clear(); + other.Clear(); + PolicyMap& policy_map = bundle.Get(POLICY_DOMAIN_CHROME, ""); + EXPECT_TRUE(bundle.Equals(other)); + policy_map.Set(kPolicy0, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER, + base::Value::CreateIntegerValue(123)); + EXPECT_FALSE(bundle.Equals(other)); +} + } // namespace policy |