Pass the previous and the current PolicyMap to Observers of the PolicyService.

Pass the previous and the current PolicyMap to Observers of the PolicyService.

BUG=124374
TEST=PolicyServiceTest.NotifyObservers is green


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=133476

[Mattias Nissler (ping if slow), 2012-04-23 14:20:44]

http://codereview.chromium.org/10174003/diff/1/chrome/browser/policy/policy_s...
File chrome/browser/policy/policy_service_impl.cc (right):

http://codereview.chromium.org/10174003/diff/1/chrome/browser/policy/policy_s...
chrome/browser/policy/policy_service_impl.cc:140: // contains the current
values.
I think this comment is redundant and can be removed (it just explains the
behavior of the Swap one line before, which does exactly what it's expected to
do).

http://codereview.chromium.org/10174003/diff/1/chrome/browser/policy/policy_s...
File chrome/browser/policy/policy_service_unittest.cc (right):

http://codereview.chromium.org/10174003/diff/1/chrome/browser/policy/policy_s...
chrome/browser/policy/policy_service_unittest.cc:103:
EXPECT_TRUE(current.Equals(expectedCurrent));
This pattern does manually what you can achieve by using a matcher that on the
EXPECT_CALL for parameters 3 and 4 in line 98. See MATCHER_P in the gmock docs.
(Also below).

[Joao da Silva, 2012-04-23 14:53:19]

http://codereview.chromium.org/10174003/diff/1/chrome/browser/policy/policy_s...
File chrome/browser/policy/policy_service_impl.cc (right):

http://codereview.chromium.org/10174003/diff/1/chrome/browser/policy/policy_s...
chrome/browser/policy/policy_service_impl.cc:140: // contains the current
values.
On 2012/04/23 14:20:44, Mattias Nissler wrote:
> I think this comment is redundant and can be removed (it just explains the
> behavior of the Swap one line before, which does exactly what it's expected to
> do).

Done.

http://codereview.chromium.org/10174003/diff/1/chrome/browser/policy/policy_s...
File chrome/browser/policy/policy_service_unittest.cc (right):

http://codereview.chromium.org/10174003/diff/1/chrome/browser/policy/policy_s...
chrome/browser/policy/policy_service_unittest.cc:103:
EXPECT_TRUE(current.Equals(expectedCurrent));
On 2012/04/23 14:20:44, Mattias Nissler wrote:
> This pattern does manually what you can achieve by using a matcher that on the
> EXPECT_CALL for parameters 3 and 4 in line 98. See MATCHER_P in the gmock
docs.
> (Also below).

Thanks for the tip, MATCHER is quite cool! PTAL.

[Mattias Nissler (ping if slow), 2012-04-23 15:08:55]

LGTM w/ nits

http://codereview.chromium.org/10174003/diff/4002/chrome/browser/policy/polic...
File chrome/browser/policy/policy_service_unittest.cc (right):

http://codereview.chromium.org/10174003/diff/4002/chrome/browser/policy/polic...
chrome/browser/policy/policy_service_unittest.cc:31: // their expectec values.
nit: expected

http://codereview.chromium.org/10174003/diff/4002/chrome/browser/policy/polic...
chrome/browser/policy/policy_service_unittest.cc:112:
base::Value::CreateIntegerValue(456));
indentation

http://codereview.chromium.org/10174003/diff/4002/chrome/browser/policy/polic...
chrome/browser/policy/policy_service_unittest.cc:113:
provider0_.AddMandatoryPolicy("bbb", base::Value::CreateIntegerValue(456));
nit: You have added a newline in the corresponding line of the block above (line
96). Either add one here as well (and below) or remove the one above.

[Joao da Silva, 2012-04-23 15:22:10]

Thanks for reviewing!

http://codereview.chromium.org/10174003/diff/4002/chrome/browser/policy/polic...
File chrome/browser/policy/policy_service_unittest.cc (right):

http://codereview.chromium.org/10174003/diff/4002/chrome/browser/policy/polic...
chrome/browser/policy/policy_service_unittest.cc:31: // their expectec values.
On 2012/04/23 15:08:55, Mattias Nissler wrote:
> nit: expected

Done.

http://codereview.chromium.org/10174003/diff/4002/chrome/browser/policy/polic...
chrome/browser/policy/policy_service_unittest.cc:112:
base::Value::CreateIntegerValue(456));
On 2012/04/23 15:08:55, Mattias Nissler wrote:
> indentation

Done.

http://codereview.chromium.org/10174003/diff/4002/chrome/browser/policy/polic...
chrome/browser/policy/policy_service_unittest.cc:113:
provider0_.AddMandatoryPolicy("bbb", base::Value::CreateIntegerValue(456));
On 2012/04/23 15:08:55, Mattias Nissler wrote:
> nit: You have added a newline in the corresponding line of the block above
(line
> 96). Either add one here as well (and below) or remove the one above.

Removed line above.

[commit-bot: I haz the power, 2012-04-23 16:41:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/joaodasilva@chromium.org/10174003/11002

[commit-bot: I haz the power, 2012-04-23 18:15:40]

Change committed as 133476