Create a list of policy changes before notifying observers

chrome/browser/policy/policy_service_impl_unittest.cc

 // 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.
 
 #include "chrome/browser/policy/policy_service_impl.h"
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/message_loop.h"
@@ skipping 48 matching lines @@
                   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));
 }
 
+// Observer class that changes the policy in the passed provider when the
+// callback is invoked.
+class ChangePolicyObserver : public PolicyService::Observer {

[Joao da Silva, 2012/12/22 21:42:00]

Suggestion: the MockPolicyServiceObserver class above could be reused, and the
action on OnPolicyUpdated() could be set with an EXPECT_CALL and a custom
ACTION_P:

ACTION_P(UpdateChromePolicy, provider, policy) {
  provider->UpdateChromePolicy(*policy);
}

TEST(...) {
  MockPolicyServiceObserver observer;
  PolicyMap new_policy;
  // setup ...
  EXPECT_CALL(observer, OnPolicyUpdated(_, _, _,
_).WillOnce(UpdateChromePolicy(&provider0_, &new_policy));
  ...
}

[Andrew T Wilson (Slow), 2013/01/04 15:54:48]

Apparently we're trying to get rid of ACTION_P:
https://code.google.com/p/chromium/issues/detail?id=165595

So I'm going to use my current Fake approach.

+ public:
+  explicit ChangePolicyObserver(MockConfigurationPolicyProvider* provider)
+      : provider_(provider) {}
+
+  virtual void OnPolicyUpdated(PolicyDomain domain,
+                               const std::string& ns,
+                               const PolicyMap& previous,
+                               const PolicyMap& current) {

[Joao da Silva, 2012/12/22 21:42:00]

OVERRIDE

[Andrew T Wilson (Slow), 2013/01/04 15:54:48]

Done.

+    PolicyMap new_policy;
+    new_policy.Set("foo", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+                   base::Value::CreateIntegerValue(14));
+    provider_->UpdateChromePolicy(new_policy);
+  }
+
+ private:
+  MockConfigurationPolicyProvider* provider_;
+};
+
 }  // namespace
 
 class PolicyServiceTest : public testing::Test {
  public:
   PolicyServiceTest() {}
 
   virtual void SetUp() OVERRIDE {
     EXPECT_CALL(provider0_, IsInitializationComplete())
         .WillRepeatedly(Return(true));
     EXPECT_CALL(provider1_, IsInitializationComplete())
@@ skipping 189 matching lines @@
                               PolicyEquals(&policy_map)));
   provider0_.UpdatePolicy(bundle.Pass());
   Mock::VerifyAndClearExpectations(&chrome_observer);
   Mock::VerifyAndClearExpectations(&extension_observer);
 
   policy_service_->RemoveObserver(POLICY_DOMAIN_CHROME, &chrome_observer);
   policy_service_->RemoveObserver(POLICY_DOMAIN_EXTENSIONS,
                                   &extension_observer);
 }
 
+TEST_F(PolicyServiceTest, ObserverChangesPolicy) {
+  ChangePolicyObserver observer(&provider0_);
+  policy_service_->AddObserver(POLICY_DOMAIN_CHROME, &observer);
+  policy0_.Set("aaa", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+               base::Value::CreateIntegerValue(123));
+  policy0_.Set("bbb", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
+               base::Value::CreateIntegerValue(1234));
+  // Should not crash.
+  provider0_.UpdateChromePolicy(policy0_);
+  policy_service_->RemoveObserver(POLICY_DOMAIN_CHROME, &observer);
+}
+
 TEST_F(PolicyServiceTest, Priorities) {
   PolicyMap expected;
   expected.Set("pre", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
                base::Value::CreateIntegerValue(13));
   expected.Set("aaa", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
                base::Value::CreateIntegerValue(0));
   policy0_.Set("aaa", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
                base::Value::CreateIntegerValue(0));
   policy1_.Set("aaa", POLICY_LEVEL_MANDATORY, POLICY_SCOPE_USER,
                base::Value::CreateIntegerValue(1));
@@ skipping 174 matching lines @@
   // 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