Added PolicyChangeRegistrar, to observe specific policies at the PolicyService.

Added PolicyChangeRegistrar, to observe specific policies at the PolicyService.

BUG=124374
TEST=PolicyServiceTest.* in unit_tests are green


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

[Joao da Silva, 2012-04-23 15:35:56]

And here's the other half :-) Please review.

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

https://chromiumcodereview.appspot.com/10191005/diff/3005/chrome/browser/poli...
File chrome/browser/policy/policy_service.h (right):

https://chromiumcodereview.appspot.com/10191005/diff/3005/chrome/browser/poli...
chrome/browser/policy/policy_service.h:127: Observer* observer_;
How about making this a base::Callback?

https://chromiumcodereview.appspot.com/10191005/diff/3005/chrome/browser/poli...
chrome/browser/policy/policy_service.h:128: std::set<std::string> observing_;
Could use a better name, observed_policies_ for example.

[Joao da Silva, 2012-04-23 16:52:28]

PTAL.

https://chromiumcodereview.appspot.com/10191005/diff/3005/chrome/browser/poli...
File chrome/browser/policy/policy_service.h (right):

https://chromiumcodereview.appspot.com/10191005/diff/3005/chrome/browser/poli...
chrome/browser/policy/policy_service.h:127: Observer* observer_;
On 2012/04/23 15:42:50, Mattias Nissler wrote:
> How about making this a base::Callback?

Done. I've mapped a callback per policy name, so that the callback only takes
the old/new values and doesn't need the policy_name too, and the invoked
function doesn't have to "switch" on the policy_name. Users of the registrar can
always bind that information to the callback, if they want to receive it. wdyt?

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

LGTM w/ a strongly-encouraged nit :)

https://chromiumcodereview.appspot.com/10191005/diff/9001/chrome/browser/poli...
File chrome/browser/policy/policy_service_unittest.cc (right):

https://chromiumcodereview.appspot.com/10191005/diff/9001/chrome/browser/poli...
chrome/browser/policy/policy_service_unittest.cc:35:
MOCK_METHOD2(OnPolicyValueUpdated, void(const base::Value*,
You could put that mock method on the Test class for simplicity.

[Joao da Silva, 2012-04-23 17:29:26]

Thanks for the review :-)

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

http://codereview.chromium.org/10191005/diff/9001/chrome/browser/policy/polic...
chrome/browser/policy/policy_service_unittest.cc:35:
MOCK_METHOD2(OnPolicyValueUpdated, void(const base::Value*,
On 2012/04/23 17:14:18, Mattias Nissler wrote:
> You could put that mock method on the Test class for simplicity.

Done.

[commit-bot: I haz the power, 2012-04-24 08:51:48]

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

[commit-bot: I haz the power, 2012-04-24 10:14:36]

Change committed as 133656