Refactored ConfigurationPolicyPrefStore to not depend on chrome/.

Refactored ConfigurationPolicyPrefStore to not depend on chrome/.

Some of the chrome/browser/policy code will be moved to a new layered component.
This change prepares the ConfigurationPolicyPrefStore class for the move, by
removing its dependency on g_browser_process and BrowserThread.

BUG=271392
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=217084

[Joao da Silva, 2013-08-12 10:04:13]

PTAL

[Joao da Silva, 2013-08-12 13:14:29]

+Mattias for chrome/browser/prefs review

[bartfab (slow), 2013-08-12 13:27:53]

lgtm

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
File chrome/browser/policy/configuration_policy_pref_store.cc (right):

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
chrome/browser/policy/configuration_policy_pref_store.cc:126:
base::MessageLoop::current()->PostTask(
Nit: Retain the comment explaining why a post is necessary here.

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
File chrome/browser/policy/configuration_policy_pref_store.h (right):

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
chrome/browser/policy/configuration_policy_pref_store.h:36: PolicyLevel level);
Nit: #include "chrome/browser/policy/policy_types.h"

https://codereview.chromium.org/22812002/diff/1/chrome/browser/prefs/chrome_p...
File chrome/browser/prefs/chrome_pref_service_factory.cc (right):

https://codereview.chromium.org/22812002/diff/1/chrome/browser/prefs/chrome_p...
chrome/browser/prefs/chrome_pref_service_factory.cc:83:
policy::POLICY_LEVEL_MANDATORY));
Nit: #include "chrome/browser/policy/policy_types.h"

https://codereview.chromium.org/22812002/diff/1/chrome/browser/prefs/pref_ser...
File chrome/browser/prefs/pref_service_syncable_builder.cc (right):

https://codereview.chromium.org/22812002/diff/1/chrome/browser/prefs/pref_ser...
chrome/browser/prefs/pref_service_syncable_builder.cc:31:
policy::POLICY_LEVEL_MANDATORY));
Nit: #include "chrome/browser/policy/policy_types.h"

[Mattias Nissler (ping if slow), 2013-08-12 13:38:28]

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
File chrome/browser/policy/configuration_policy_pref_store.h (right):

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
chrome/browser/policy/configuration_policy_pref_store.h:32: // the store.. Only
policies of the given |level| will be mapped.
nit: One period is enough.

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
File chrome/browser/policy/configuration_policy_pref_store_unittest.cc (right):

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
chrome/browser/policy/configuration_policy_pref_store_unittest.cc:59:
&handler_list_,
I'm surprised the tests in these file pass with an empty handler list. Is that
OK?

[Joao da Silva, 2013-08-12 13:41:48]

I'll come back to this CL after adding a DEPS file.

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
File chrome/browser/policy/configuration_policy_pref_store_unittest.cc (right):

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
chrome/browser/policy/configuration_policy_pref_store_unittest.cc:59:
&handler_list_,
On 2013/08/12 13:38:28, Mattias Nissler wrote:
> I'm surprised the tests in these file pass with an empty handler list. Is that
> OK?

It's not empty, the default ctor adds all the handlers. Notice that
browser_policy_connector does this as well.

[Mattias Nissler (ping if slow), 2013-08-12 15:14:43]

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
File chrome/browser/policy/configuration_policy_pref_store_unittest.cc (right):

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
chrome/browser/policy/configuration_policy_pref_store_unittest.cc:59:
&handler_list_,
On 2013/08/12 13:41:48, Joao da Silva wrote:
> On 2013/08/12 13:38:28, Mattias Nissler wrote:
> > I'm surprised the tests in these file pass with an empty handler list. Is
that
> > OK?
> 
> It's not empty, the default ctor adds all the handlers. Notice that
> browser_policy_connector does this as well.

Ah, correct. All good then.

[Joao da Silva, 2013-08-12 15:41:55]

Ready for another look. Mattias, need a stamp :-)

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
File chrome/browser/policy/configuration_policy_pref_store.cc (right):

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
chrome/browser/policy/configuration_policy_pref_store.cc:126:
base::MessageLoop::current()->PostTask(
On 2013/08/12 13:27:53, bartfab wrote:
> Nit: Retain the comment explaining why a post is necessary here.

Done.

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
File chrome/browser/policy/configuration_policy_pref_store.h (right):

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
chrome/browser/policy/configuration_policy_pref_store.h:32: // the store.. Only
policies of the given |level| will be mapped.
On 2013/08/12 13:38:28, Mattias Nissler wrote:
> nit: One period is enough.

Done.

https://codereview.chromium.org/22812002/diff/1/chrome/browser/policy/configu...
chrome/browser/policy/configuration_policy_pref_store.h:36: PolicyLevel level);
On 2013/08/12 13:27:53, bartfab wrote:
> Nit: #include "chrome/browser/policy/policy_types.h"

Done.

https://codereview.chromium.org/22812002/diff/1/chrome/browser/prefs/chrome_p...
File chrome/browser/prefs/chrome_pref_service_factory.cc (right):

https://codereview.chromium.org/22812002/diff/1/chrome/browser/prefs/chrome_p...
chrome/browser/prefs/chrome_pref_service_factory.cc:83:
policy::POLICY_LEVEL_MANDATORY));
On 2013/08/12 13:27:53, bartfab wrote:
> Nit: #include "chrome/browser/policy/policy_types.h"

Done.

https://codereview.chromium.org/22812002/diff/1/chrome/browser/prefs/pref_ser...
File chrome/browser/prefs/pref_service_syncable_builder.cc (right):

https://codereview.chromium.org/22812002/diff/1/chrome/browser/prefs/pref_ser...
chrome/browser/prefs/pref_service_syncable_builder.cc:31:
policy::POLICY_LEVEL_MANDATORY));
On 2013/08/12 13:27:53, bartfab wrote:
> Nit: #include "chrome/browser/policy/policy_types.h"

Done.

[Mattias Nissler (ping if slow), 2013-08-12 16:06:14]

LGTM. You need a rebase.

[Mattias Nissler (ping if slow), 2013-08-12 16:06:32]

On 2013/08/12 16:06:14, Mattias Nissler wrote:
> LGTM. You need a rebase.

Or maybe you have rebased already, but the bots need to catch up.

[commit-bot: I haz the power, 2013-08-12 17:59:19]

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

[commit-bot: I haz the power, 2013-08-12 21:38:57]

Change committed as 217084