Create a list of policy changes before notifying observers

chrome/browser/policy/policy_service_impl_unittest.cc

Index: chrome/browser/policy/policy_service_impl_unittest.cc
diff --git a/chrome/browser/policy/policy_service_impl_unittest.cc b/chrome/browser/policy/policy_service_impl_unittest.cc
index 4ee3a677677c7279a941e6d154cf1f7e67e458bb..49a9fc2d2115590c7e81b1705aacd907634ec9b1 100644
--- a/chrome/browser/policy/policy_service_impl_unittest.cc
+++ b/chrome/browser/policy/policy_service_impl_unittest.cc
@@ -66,6 +66,27 @@ void AddTestPolicies(PolicyBundle* bundle,
                   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 {
@@ -275,6 +296,18 @@ TEST_F(PolicyServiceTest, NotifyObserversInMultipleNamespaces) {
                                   &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,