Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(6180)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: chrome/browser/policy/policy_service_impl_unittest.cc

Issue 10386097: Refactored ConfigurationPolicyProvider to provide PolicyBundles instead of PolicyMaps. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Fixed nit Created 8 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « chrome/browser/policy/policy_service_impl.cc ('k') | chrome/browser/policy/policy_service_stub.h » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: chrome/browser/policy/policy_service_impl_unittest.cc
diff --git a/chrome/browser/policy/policy_service_unittest.cc b/chrome/browser/policy/policy_service_impl_unittest.cc
similarity index 62%
rename from chrome/browser/policy/policy_service_unittest.cc
rename to chrome/browser/policy/policy_service_impl_unittest.cc
index 27cf3b3435c4ee37b1636665988e29761eb98759..2f5ee2c5cfa37dcf0c5d9fd9e6dc84dd2d93a9f8 100644
--- a/chrome/browser/policy/policy_service_unittest.cc
+++ b/chrome/browser/policy/policy_service_impl_unittest.cc
@@ -24,6 +24,10 @@ namespace policy {
namespace {
+const char kExtension[] = "extension-id";
+const char kSameLevelPolicy[] = "policy-same-level-and-scope";
+const char kDiffLevelPolicy[] = "chrome-diff-level-and-scope";
+
class MockPolicyServiceObserver : public PolicyService::Observer {
public:
virtual ~MockPolicyServiceObserver() {}
@@ -45,6 +49,23 @@ MATCHER_P(ValueEquals, expected, "") {
return base::Value::Equals(arg, expected);
}
+// Helper that fills |bundle| with test policies.
+void AddTestPolicies(PolicyBundle* bundle,
+ const char* value,
+ PolicyLevel level,
+ PolicyScope scope) {
+ PolicyMap* policy_map = &bundle->Get(POLICY_DOMAIN_CHROME, "");
+ policy_map->Set(kSameLevelPolicy, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+ base::Value::CreateStringValue(value));
+ policy_map->Set(kDiffLevelPolicy, level, scope,
+ base::Value::CreateStringValue(value));
+ policy_map = &bundle->Get(POLICY_DOMAIN_EXTENSIONS, kExtension);
+ policy_map->Set(kSameLevelPolicy, POLICY_LEVEL_MANDATORY,
+ POLICY_SCOPE_USER, base::Value::CreateStringValue(value));
+ policy_map->Set(kDiffLevelPolicy, level, scope,
+ base::Value::CreateStringValue(value));
+}
+
} // namespace
class PolicyServiceTest : public testing::Test {
@@ -52,13 +73,6 @@ class PolicyServiceTest : public testing::Test {
PolicyServiceTest() {}
void SetUp() OVERRIDE {
- EXPECT_CALL(provider0_, ProvideInternal(_))
- .WillRepeatedly(CopyPolicyMap(&policy0_));
- EXPECT_CALL(provider1_, ProvideInternal(_))
- .WillRepeatedly(CopyPolicyMap(&policy1_));
- EXPECT_CALL(provider2_, ProvideInternal(_))
- .WillRepeatedly(CopyPolicyMap(&policy2_));
-
EXPECT_CALL(provider0_, IsInitializationComplete())
.WillRepeatedly(Return(true));
EXPECT_CALL(provider1_, IsInitializationComplete())
@@ -68,6 +82,7 @@ class PolicyServiceTest : public testing::Test {
policy0_.Set("pre", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
base::Value::CreateIntegerValue(13));
+ provider0_.UpdateChromePolicy(policy0_);
PolicyServiceImpl::Providers providers;
providers.push_back(&provider0_);
@@ -85,9 +100,7 @@ class PolicyServiceTest : public testing::Test {
bool VerifyPolicies(PolicyDomain domain,
const std::string& component_id,
const PolicyMap& expected) {
- const PolicyMap* policies =
- policy_service_->GetPolicies(domain, component_id);
- return policies && policies->Equals(expected);
+ return policy_service_->GetPolicies(domain, component_id).Equals(expected);
}
protected:
@@ -127,12 +140,12 @@ TEST_F(PolicyServiceTest, NotifyObservers) {
EXPECT_CALL(observer, OnPolicyUpdated(POLICY_DOMAIN_CHROME, "",
PolicyEquals(&expectedPrevious),
PolicyEquals(&expectedCurrent)));
- provider0_.NotifyPolicyUpdated();
+ provider0_.UpdateChromePolicy(policy0_);
Mock::VerifyAndClearExpectations(&observer);
// No changes.
EXPECT_CALL(observer, OnPolicyUpdated(_, _, _, _)).Times(0);
- provider0_.NotifyPolicyUpdated();
+ provider0_.UpdateChromePolicy(policy0_);
Mock::VerifyAndClearExpectations(&observer);
EXPECT_TRUE(VerifyPolicies(POLICY_DOMAIN_CHROME, "", expectedCurrent));
@@ -145,7 +158,7 @@ TEST_F(PolicyServiceTest, NotifyObservers) {
EXPECT_CALL(observer, OnPolicyUpdated(POLICY_DOMAIN_CHROME, "",
PolicyEquals(&expectedPrevious),
PolicyEquals(&expectedCurrent)));
- provider0_.NotifyPolicyUpdated();
+ provider0_.UpdateChromePolicy(policy0_);
Mock::VerifyAndClearExpectations(&observer);
// Removed policy.
@@ -155,7 +168,7 @@ TEST_F(PolicyServiceTest, NotifyObservers) {
EXPECT_CALL(observer, OnPolicyUpdated(POLICY_DOMAIN_CHROME, "",
PolicyEquals(&expectedPrevious),
PolicyEquals(&expectedCurrent)));
- provider0_.NotifyPolicyUpdated();
+ provider0_.UpdateChromePolicy(policy0_);
Mock::VerifyAndClearExpectations(&observer);
// Changed policy.
@@ -168,18 +181,88 @@ TEST_F(PolicyServiceTest, NotifyObservers) {
EXPECT_CALL(observer, OnPolicyUpdated(POLICY_DOMAIN_CHROME, "",
PolicyEquals(&expectedPrevious),
PolicyEquals(&expectedCurrent)));
- provider0_.NotifyPolicyUpdated();
+ provider0_.UpdateChromePolicy(policy0_);
Mock::VerifyAndClearExpectations(&observer);
// No changes again.
EXPECT_CALL(observer, OnPolicyUpdated(_, _, _, _)).Times(0);
- provider0_.NotifyPolicyUpdated();
+ provider0_.UpdateChromePolicy(policy0_);
Mock::VerifyAndClearExpectations(&observer);
EXPECT_TRUE(VerifyPolicies(POLICY_DOMAIN_CHROME, "", expectedCurrent));
policy_service_->RemoveObserver(POLICY_DOMAIN_CHROME, "", &observer);
}
+TEST_F(PolicyServiceTest, NotifyObserversInMultipleNamespaces) {
+ const std::string kExtension0("extension-0");
+ const std::string kExtension1("extension-1");
+ const std::string kExtension2("extension-2");
+ MockPolicyServiceObserver chrome_observer;
+ MockPolicyServiceObserver extension0_observer;
+ MockPolicyServiceObserver extension1_observer;
+ MockPolicyServiceObserver extension2_observer;
+ policy_service_->AddObserver(POLICY_DOMAIN_CHROME, "", &chrome_observer);
+ policy_service_->AddObserver(
+ POLICY_DOMAIN_EXTENSIONS, kExtension0, &extension0_observer);
Mattias Nissler (ping if slow) 2012/05/15 09:40:33 I think this would become more readable if you put
Joao da Silva 2012/05/15 13:06:31 Done.
+ policy_service_->AddObserver(
+ POLICY_DOMAIN_EXTENSIONS, kExtension1, &extension1_observer);
+ policy_service_->AddObserver(
+ POLICY_DOMAIN_EXTENSIONS, kExtension2, &extension2_observer);
+
+ PolicyMap policy_map;
+ policy_map.Set("policy", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+ base::Value::CreateStringValue("value"));
+
+ scoped_ptr<PolicyBundle> bundle(new PolicyBundle());
+ // The initial setup includes a policy for chrome that is now changing.
+ bundle->Get(POLICY_DOMAIN_CHROME, "").CopyFrom(policy_map);
+ bundle->Get(POLICY_DOMAIN_EXTENSIONS, kExtension0).CopyFrom(policy_map);
+ bundle->Get(POLICY_DOMAIN_EXTENSIONS, kExtension1).CopyFrom(policy_map);
+
+ EXPECT_CALL(chrome_observer,
+ OnPolicyUpdated(POLICY_DOMAIN_CHROME, "", _, _));
Mattias Nissler (ping if slow) 2012/05/15 09:40:33 add a matcher for checking the correct PolicyMap g
Joao da Silva 2012/05/15 13:06:31 Done.
+ EXPECT_CALL(extension0_observer,
+ OnPolicyUpdated(POLICY_DOMAIN_EXTENSIONS, kExtension0, _, _));
+ EXPECT_CALL(extension1_observer,
+ OnPolicyUpdated(POLICY_DOMAIN_EXTENSIONS, kExtension1, _, _));
+ EXPECT_CALL(extension2_observer, OnPolicyUpdated(_, _, _, _)).Times(0);
+ provider0_.UpdatePolicy(bundle.Pass());
+ Mock::VerifyAndClearExpectations(&chrome_observer);
+ Mock::VerifyAndClearExpectations(&extension0_observer);
+ Mock::VerifyAndClearExpectations(&extension1_observer);
+ Mock::VerifyAndClearExpectations(&extension2_observer);
+
+ // Chrome policy stays the same, kExtension0 is gone, kExtension1 changes,
+ // and kExtension2 is new.
+ bundle.reset(new PolicyBundle());
+ bundle->Get(POLICY_DOMAIN_CHROME, "").CopyFrom(policy_map);
+ policy_map.Set("policy", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+ base::Value::CreateStringValue("another value"));
+ bundle->Get(POLICY_DOMAIN_EXTENSIONS, kExtension1).CopyFrom(policy_map);
+ bundle->Get(POLICY_DOMAIN_EXTENSIONS, kExtension2).CopyFrom(policy_map);
+
+ EXPECT_CALL(chrome_observer, OnPolicyUpdated(_, _, _, _)).Times(0);
+ EXPECT_CALL(extension0_observer,
+ OnPolicyUpdated(POLICY_DOMAIN_EXTENSIONS, kExtension0, _, _));
+ EXPECT_CALL(extension1_observer,
+ OnPolicyUpdated(POLICY_DOMAIN_EXTENSIONS, kExtension1, _, _));
+ EXPECT_CALL(extension2_observer,
+ OnPolicyUpdated(POLICY_DOMAIN_EXTENSIONS, kExtension2, _, _));
+ provider0_.UpdatePolicy(bundle.Pass());
+ Mock::VerifyAndClearExpectations(&chrome_observer);
+ Mock::VerifyAndClearExpectations(&extension0_observer);
+ Mock::VerifyAndClearExpectations(&extension1_observer);
+ Mock::VerifyAndClearExpectations(&extension2_observer);
+
+ policy_service_->RemoveObserver(POLICY_DOMAIN_CHROME, "", &chrome_observer);
+ policy_service_->RemoveObserver(
Mattias Nissler (ping if slow) 2012/05/15 09:40:33 same comment regarding line breaking.
Joao da Silva 2012/05/15 13:06:31 Done.
+ POLICY_DOMAIN_EXTENSIONS, kExtension0, &extension0_observer);
+ policy_service_->RemoveObserver(
+ POLICY_DOMAIN_EXTENSIONS, kExtension1, &extension1_observer);
+ policy_service_->RemoveObserver(
+ POLICY_DOMAIN_EXTENSIONS, kExtension2, &extension2_observer);
+}
+
TEST_F(PolicyServiceTest, Priorities) {
PolicyMap expected;
expected.Set("pre", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
@@ -192,22 +275,22 @@ TEST_F(PolicyServiceTest, Priorities) {
base::Value::CreateIntegerValue(1));
policy2_.Set("aaa", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
base::Value::CreateIntegerValue(2));
- provider0_.NotifyPolicyUpdated();
- provider1_.NotifyPolicyUpdated();
- provider2_.NotifyPolicyUpdated();
+ provider0_.UpdateChromePolicy(policy0_);
+ provider1_.UpdateChromePolicy(policy1_);
+ provider2_.UpdateChromePolicy(policy2_);
EXPECT_TRUE(VerifyPolicies(POLICY_DOMAIN_CHROME, "", expected));
expected.Set("aaa", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
base::Value::CreateIntegerValue(1));
policy0_.Erase("aaa");
- provider0_.NotifyPolicyUpdated();
+ provider0_.UpdateChromePolicy(policy0_);
EXPECT_TRUE(VerifyPolicies(POLICY_DOMAIN_CHROME, "", expected));
expected.Set("aaa", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
base::Value::CreateIntegerValue(2));
policy1_.Set("aaa", POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_USER,
base::Value::CreateIntegerValue(1));
- provider1_.NotifyPolicyUpdated();
+ provider1_.UpdateChromePolicy(policy1_);
EXPECT_TRUE(VerifyPolicies(POLICY_DOMAIN_CHROME, "", expected));
}
@@ -231,14 +314,14 @@ TEST_F(PolicyServiceTest, PolicyChangeRegistrar) {
EXPECT_CALL(*this, OnPolicyValueUpdated(NULL, ValueEquals(&kValue0)));
policy0_.Set("aaa", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
kValue0.DeepCopy());
- provider0_.NotifyPolicyUpdated();
+ provider0_.UpdateChromePolicy(policy0_);
Mock::VerifyAndClearExpectations(this);
// Changing other values doesn't trigger a notification.
EXPECT_CALL(*this, OnPolicyValueUpdated(_, _)).Times(0);
policy0_.Set("bbb", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
kValue0.DeepCopy());
- provider0_.NotifyPolicyUpdated();
+ provider0_.UpdateChromePolicy(policy0_);
Mock::VerifyAndClearExpectations(this);
// Modifying the value triggers a notification.
@@ -247,13 +330,13 @@ TEST_F(PolicyServiceTest, PolicyChangeRegistrar) {
ValueEquals(&kValue1)));
policy0_.Set("aaa", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
kValue1.DeepCopy());
- provider0_.NotifyPolicyUpdated();
+ provider0_.UpdateChromePolicy(policy0_);
Mock::VerifyAndClearExpectations(this);
// Removing the value triggers a notification.
EXPECT_CALL(*this, OnPolicyValueUpdated(ValueEquals(&kValue1), NULL));
policy0_.Erase("aaa");
- provider0_.NotifyPolicyUpdated();
+ provider0_.UpdateChromePolicy(policy0_);
Mock::VerifyAndClearExpectations(this);
// No more notifications after destroying the registrar.
@@ -263,7 +346,7 @@ TEST_F(PolicyServiceTest, PolicyChangeRegistrar) {
kValue1.DeepCopy());
policy0_.Set("pre", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
kValue1.DeepCopy());
- provider0_.NotifyPolicyUpdated();
+ provider0_.UpdateChromePolicy(policy0_);
Mock::VerifyAndClearExpectations(this);
}
@@ -288,7 +371,7 @@ TEST_F(PolicyServiceTest, RefreshPolicies) {
base::FundamentalValue kValue0(0);
policy0_.Set("aaa", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
kValue0.DeepCopy());
- provider0_.NotifyPolicyUpdated();
+ provider0_.UpdateChromePolicy(policy0_);
loop.RunAllPending();
Mock::VerifyAndClearExpectations(this);
@@ -296,7 +379,7 @@ TEST_F(PolicyServiceTest, RefreshPolicies) {
base::FundamentalValue kValue1(1);
policy1_.Set("aaa", POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_USER,
kValue1.DeepCopy());
- provider1_.NotifyPolicyUpdated();
+ provider1_.UpdateChromePolicy(policy1_);
loop.RunAllPending();
Mock::VerifyAndClearExpectations(this);
@@ -306,7 +389,7 @@ TEST_F(PolicyServiceTest, RefreshPolicies) {
EXPECT_CALL(*this, OnPolicyRefresh()).Times(0);
policy1_.Set("bbb", POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_USER,
kValue1.DeepCopy());
- provider1_.NotifyPolicyUpdated();
+ provider1_.UpdateChromePolicy(policy1_);
loop.RunAllPending();
Mock::VerifyAndClearExpectations(this);
@@ -322,7 +405,7 @@ TEST_F(PolicyServiceTest, RefreshPolicies) {
EXPECT_CALL(*this, OnPolicyRefresh()).Times(0);
policy2_.Set("bbb", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
kValue0.DeepCopy());
- provider2_.NotifyPolicyUpdated();
+ provider2_.UpdateChromePolicy(policy2_);
loop.RunAllPending();
Mock::VerifyAndClearExpectations(this);
@@ -331,16 +414,46 @@ TEST_F(PolicyServiceTest, RefreshPolicies) {
base::FundamentalValue kValue2(2);
policy0_.Set("aaa", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
kValue2.DeepCopy());
- provider0_.NotifyPolicyUpdated();
- provider1_.NotifyPolicyUpdated();
+ provider0_.UpdateChromePolicy(policy0_);
+ provider1_.UpdateChromePolicy(policy1_);
loop.RunAllPending();
Mock::VerifyAndClearExpectations(this);
- const PolicyMap* policies = policy_service_->GetPolicies(
+ const PolicyMap& policies = policy_service_->GetPolicies(
POLICY_DOMAIN_CHROME, "");
- ASSERT_TRUE(policies);
- EXPECT_TRUE(base::Value::Equals(&kValue2, policies->GetValue("aaa")));
- EXPECT_TRUE(base::Value::Equals(&kValue0, policies->GetValue("bbb")));
+ EXPECT_TRUE(base::Value::Equals(&kValue2, policies.GetValue("aaa")));
+ EXPECT_TRUE(base::Value::Equals(&kValue0, policies.GetValue("bbb")));
+}
+
+TEST_F(PolicyServiceTest, NamespaceMerge) {
+ scoped_ptr<PolicyBundle> bundle0(new PolicyBundle());
+ scoped_ptr<PolicyBundle> bundle1(new PolicyBundle());
+ scoped_ptr<PolicyBundle> bundle2(new PolicyBundle());
+
+ AddTestPolicies(
+ bundle0.get(), "bundle0", POLICY_LEVEL_RECOMMENDED, POLICY_SCOPE_USER);
Mattias Nissler (ping if slow) 2012/05/15 09:40:33 break line after second parameter?
Joao da Silva 2012/05/15 13:06:31 Done.
+ AddTestPolicies(
+ bundle1.get(), "bundle1", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER);
+ AddTestPolicies(
+ bundle2.get(), "bundle2", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE);
+
+ provider0_.UpdatePolicy(bundle0.Pass());
+ provider1_.UpdatePolicy(bundle1.Pass());
+ provider2_.UpdatePolicy(bundle2.Pass());
+
+ PolicyMap expected;
+ // For policies of the same level and scope, the first provider takes
+ // precedence, on every namespace.
+ expected.Set(kSameLevelPolicy, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+ base::Value::CreateStringValue("bundle0"));
+ // For policies with different levels and scopes, the highest priority
+ // level/scope combination takes precedence, on every namespace.
+ expected.Set(kDiffLevelPolicy, POLICY_LEVEL_MANDATORY, POLICY_SCOPE_MACHINE,
+ base::Value::CreateStringValue("bundle2"));
+ EXPECT_TRUE(policy_service_->GetPolicies(POLICY_DOMAIN_CHROME, "")
+ .Equals(expected));
+ EXPECT_TRUE(policy_service_->GetPolicies(POLICY_DOMAIN_EXTENSIONS, kExtension)
+ .Equals(expected));
}
} // namespace policy
« no previous file with comments | « chrome/browser/policy/policy_service_impl.cc ('k') | chrome/browser/policy/policy_service_stub.h » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698