Trigger chrome.storage.onChanged events for policy updates on the 'managed' namespace.

Trigger chrome.storage.onChanged events for policy updates on the 'managed' namespace.

BUG=108992
TEST=ExtensionSettingsApiTest.ManagedStorageEvents


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

[Joao da Silva, 2012-07-24 14:11:49]

This should be the last major change for this work (recommended policy values is
still pending, but isn't high priority right now).

Please have a look. Thanks!

http://codereview.chromium.org/10807086/diff/1/chrome/browser/extensions/sett...
File chrome/browser/extensions/settings/settings_apitest.cc (left):

http://codereview.chromium.org/10807086/diff/1/chrome/browser/extensions/sett...
chrome/browser/extensions/settings/settings_apitest.cc:461: 
This was not necessary for this test, but |policy_provider_| may not exist if
ENABLE_CONFIGURATION_POLICY isn't defined.

Turns out the linux_redux and android bots don't run nor build browser_tests :-/

[not at google - send to devlin, 2012-07-25 00:49:02]

lgtm

http://codereview.chromium.org/10807086/diff/7001/chrome/browser/extensions/s...
File chrome/browser/extensions/settings/managed_value_store_cache.cc (right):

http://codereview.chromium.org/10807086/diff/7001/chrome/browser/extensions/s...
chrome/browser/extensions/settings/managed_value_store_cache.cc:26:
policy_service_->AddObserver(policy::POLICY_DOMAIN_EXTENSIONS, this);
Just FYI, there is ScopedObserver in base; a little awkward to use, but it would
mean not needing to RemoveObserver manually in the destructor.

http://codereview.chromium.org/10807086/diff/7001/chrome/browser/extensions/s...
File chrome/browser/extensions/settings/settings_apitest.cc (right):

http://codereview.chromium.org/10807086/diff/7001/chrome/browser/extensions/s...
chrome/browser/extensions/settings/settings_apitest.cc:459:
ExtensionTestMessageListener readyListener("ready", false);
should be ready_listener not readyListener.

http://codereview.chromium.org/10807086/diff/7001/chrome/test/data/extensions...
File
chrome/test/data/extensions/api_test/settings/managed_storage_events/background.js
(right):

http://codereview.chromium.org/10807086/diff/7001/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/settings/managed_storage_events/background.js:9:
null,
can be left out entirely (new-ish code in schema_generated_bindings.js figures
out that it should be null).

[Joao da Silva, 2012-07-25 12:52:12]

Thanks for yet another review :-) Will land if the bots are green.

http://codereview.chromium.org/10807086/diff/7001/chrome/browser/extensions/s...
File chrome/browser/extensions/settings/managed_value_store_cache.cc (right):

http://codereview.chromium.org/10807086/diff/7001/chrome/browser/extensions/s...
chrome/browser/extensions/settings/managed_value_store_cache.cc:26:
policy_service_->AddObserver(policy::POLICY_DOMAIN_EXTENSIONS, this);
On 2012/07/25 00:49:02, kalman wrote:
> Just FYI, there is ScopedObserver in base; a little awkward to use, but it
would
> mean not needing to RemoveObserver manually in the destructor.

Can't be used here for 2 reasons: PolicyService::Add/RemoveObserver doesn't take
only the observer as an argument, and the remove call is done before the dtor
runs.

http://codereview.chromium.org/10807086/diff/7001/chrome/browser/extensions/s...
File chrome/browser/extensions/settings/settings_apitest.cc (right):

http://codereview.chromium.org/10807086/diff/7001/chrome/browser/extensions/s...
chrome/browser/extensions/settings/settings_apitest.cc:459:
ExtensionTestMessageListener readyListener("ready", false);
On 2012/07/25 00:49:02, kalman wrote:
> should be ready_listener not readyListener.

Done.

http://codereview.chromium.org/10807086/diff/7001/chrome/test/data/extensions...
File
chrome/test/data/extensions/api_test/settings/managed_storage_events/background.js
(right):

http://codereview.chromium.org/10807086/diff/7001/chrome/test/data/extensions...
chrome/test/data/extensions/api_test/settings/managed_storage_events/background.js:9:
null,
On 2012/07/25 00:49:02, kalman wrote:
> can be left out entirely (new-ish code in schema_generated_bindings.js figures
> out that it should be null).

Nice, done (also removed this in the other tests).

[commit-bot: I haz the power, 2012-07-25 16:16:12]

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

[commit-bot: I haz the power, 2012-07-25 17:26:25]

Change committed as 148350