Added a policy_switches.cc file in the policy component.

Added a policy_switches.cc file in the policy component.

Moved the policy-specific switches into this file, and refactored the policy
code to include the new file instead of chrome_switches.cc. This removes
one more dependency on //chrome code from the policy code.

BUG=271392
R=bauerb@chromium.org, dconnelly@chromium.org, jochen@chromium.org

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

[Joao da Silva, 2013-10-15 09:56:29]

Please review:

dconnelly: the whole thing
jochen: components/OWNERS update
bauerb: chrome/browser/prefs changes

Thanks!

[dconnelly, 2013-10-15 10:05:35]

lgtm

[Bernhard Bauer, 2013-10-15 19:48:09]

https://codereview.chromium.org/26972003/diff/10001/chrome/browser/prefs/comm...
File chrome/browser/prefs/command_line_pref_store.cc (right):

https://codereview.chromium.org/26972003/diff/10001/chrome/browser/prefs/comm...
chrome/browser/prefs/command_line_pref_store.cc:78: {
policy::switches::kDisableCloudPolicyOnSignin,
Does this switch actually need to be in components/ if it's only used in chrome/
code?

[Joao da Silva, 2013-10-16 09:50:34]

@bauerb: see inline, PTAL

@jochen: friendly ping

https://codereview.chromium.org/26972003/diff/10001/chrome/browser/prefs/comm...
File chrome/browser/prefs/command_line_pref_store.cc (right):

https://codereview.chromium.org/26972003/diff/10001/chrome/browser/prefs/comm...
chrome/browser/prefs/command_line_pref_store.cc:78: {
policy::switches::kDisableCloudPolicyOnSignin,
On 2013/10/15 19:48:09, Bernhard Bauer wrote:
> Does this switch actually need to be in components/ if it's only used in
chrome/
> code?

It's used by user_cloud_policy_manager_factory and that class will move into the
component.

[Bernhard Bauer, 2013-10-16 11:18:30]

https://codereview.chromium.org/26972003/diff/10001/chrome/browser/prefs/comm...
File chrome/browser/prefs/command_line_pref_store.cc (right):

https://codereview.chromium.org/26972003/diff/10001/chrome/browser/prefs/comm...
chrome/browser/prefs/command_line_pref_store.cc:78: {
policy::switches::kDisableCloudPolicyOnSignin,
On 2013/10/16 09:50:34, Joao da Silva wrote:
> On 2013/10/15 19:48:09, Bernhard Bauer wrote:
> > Does this switch actually need to be in components/ if it's only used in
> chrome/
> > code?
> 
> It's used by user_cloud_policy_manager_factory and that class will move into
the
> component.

But it's also mapped to a preference. Could you just look up the pref value in
UserCloudPolicyManagerFactory?

[Joao da Silva, 2013-10-16 11:38:00]

On 2013/10/16 11:18:30, Bernhard Bauer wrote:
>
https://codereview.chromium.org/26972003/diff/10001/chrome/browser/prefs/comm...
> File chrome/browser/prefs/command_line_pref_store.cc (right):
> 
>
https://codereview.chromium.org/26972003/diff/10001/chrome/browser/prefs/comm...
> chrome/browser/prefs/command_line_pref_store.cc:78: {
> policy::switches::kDisableCloudPolicyOnSignin,
> On 2013/10/16 09:50:34, Joao da Silva wrote:
> > On 2013/10/15 19:48:09, Bernhard Bauer wrote:
> > > Does this switch actually need to be in components/ if it's only used in
> > chrome/
> > > code?
> > 
> > It's used by user_cloud_policy_manager_factory and that class will move into
> the
> > component.
> 
> But it's also mapped to a preference. Could you just look up the pref value in
> UserCloudPolicyManagerFactory?

Unfortunately no, because the UserCloudPolicyManager is a dependency of the
PrefService and must be created before the profile PrefService.

Note also that that pref is not in local state.

[jochen (gone - plz use gerrit), 2013-10-16 17:32:46]

components/OWNERS lgtm

[Bernhard Bauer, 2013-10-16 17:35:28]

On 2013/10/16 11:38:00, Joao da Silva wrote:
> On 2013/10/16 11:18:30, Bernhard Bauer wrote:
> >
>
https://codereview.chromium.org/26972003/diff/10001/chrome/browser/prefs/comm...
> > File chrome/browser/prefs/command_line_pref_store.cc (right):
> > 
> >
>
https://codereview.chromium.org/26972003/diff/10001/chrome/browser/prefs/comm...
> > chrome/browser/prefs/command_line_pref_store.cc:78: {
> > policy::switches::kDisableCloudPolicyOnSignin,
> > On 2013/10/16 09:50:34, Joao da Silva wrote:
> > > On 2013/10/15 19:48:09, Bernhard Bauer wrote:
> > > > Does this switch actually need to be in components/ if it's only used in
> > > chrome/
> > > > code?
> > > 
> > > It's used by user_cloud_policy_manager_factory and that class will move
into
> > the
> > > component.
> > 
> > But it's also mapped to a preference. Could you just look up the pref value
in
> > UserCloudPolicyManagerFactory?
> 
> Unfortunately no, because the UserCloudPolicyManager is a dependency of the
> PrefService and must be created before the profile PrefService.
> 
> Note also that that pref is not in local state.

Could we do it the other way then and just check the command line switch? It
seems a bit confusing to have this in two different places.

[Joao da Silva, 2013-10-16 17:51:05]

On Wed, Oct 16, 2013 at 7:35 PM, <bauerb@chromium.org> wrote:

> On 2013/10/16 11:38:00, Joao da Silva wrote:
>
>> On 2013/10/16 11:18:30, Bernhard Bauer wrote:
>> >
>>
>
> https://codereview.chromium.**org/26972003/diff/10001/**
>
chrome/browser/prefs/command_**line_pref_store.cc<https://codereview.chromium.org/26972003/diff/10001/chrome/browser/prefs/command_line_pref_store.cc>
>
>> > File chrome/browser/prefs/command_**line_pref_store.cc (right):
>> >
>> >
>>
>
> https://codereview.chromium.**org/26972003/diff/10001/**
>
chrome/browser/prefs/command_**line_pref_store.cc#newcode78<https://codereview.chromium.org/26972003/diff/10001/chrome/browser/prefs/command_line_pref_store.cc#newcode78>
>
>> > chrome/browser/prefs/command_**line_pref_store.cc:78: {
>> > policy::switches::**kDisableCloudPolicyOnSignin,
>> > On 2013/10/16 09:50:34, Joao da Silva wrote:
>> > > On 2013/10/15 19:48:09, Bernhard Bauer wrote:
>> > > > Does this switch actually need to be in components/ if it's only
>> used in
>> > > chrome/
>> > > > code?
>> > >
>> > > It's used by user_cloud_policy_manager_**factory and that class will
>> move
>>
> into
>
>> > the
>> > > component.
>> >
>> > But it's also mapped to a preference. Could you just look up the pref
>> value
>>
> in
>
>> > UserCloudPolicyManagerFactory?
>>
>
>  Unfortunately no, because the UserCloudPolicyManager is a dependency of
>> the
>> PrefService and must be created before the profile PrefService.
>>
>
>  Note also that that pref is not in local state.
>>
>
> Could we do it the other way then and just check the command line switch?
> It
> seems a bit confusing to have this in two different places.
>
>
That sounds reasonable. In fact, I think this flag isn't even needed
anymore and Drew planned to remove it some time ago.

@Drew, can we get right of --disable-cloud-policy-on-signin and
prefs::kDisableCloudPolicyOnSignin?

@Bernhard: in any case that's for another CL, and this one is just about
moving things around without changing behavior. Can I haz stamp? :-)



>
https://codereview.chromium.**org/26972003/<https://codereview.chromium.org/2...
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Joao da Silva, 2013-10-16 17:51:32]

+atwilson this time

@Drew, can we get right of --disable-cloud-policy-on-signin and prefs::
kDisableCloudPolicyOnSignin?


On Wed, Oct 16, 2013 at 7:50 PM, Joao da Silva <joaodasilva@chromium.org>wrote:

> On Wed, Oct 16, 2013 at 7:35 PM, <bauerb@chromium.org> wrote:
>
>> On 2013/10/16 11:38:00, Joao da Silva wrote:
>>
>>> On 2013/10/16 11:18:30, Bernhard Bauer wrote:
>>> >
>>>
>>
>> https://codereview.chromium.**org/26972003/diff/10001/**
>>
chrome/browser/prefs/command_**line_pref_store.cc<https://codereview.chromium.org/26972003/diff/10001/chrome/browser/prefs/command_line_pref_store.cc>
>>
>>> > File chrome/browser/prefs/command_**line_pref_store.cc (right):
>>> >
>>> >
>>>
>>
>> https://codereview.chromium.**org/26972003/diff/10001/**
>>
chrome/browser/prefs/command_**line_pref_store.cc#newcode78<https://codereview.chromium.org/26972003/diff/10001/chrome/browser/prefs/command_line_pref_store.cc#newcode78>
>>
>>> > chrome/browser/prefs/command_**line_pref_store.cc:78: {
>>> > policy::switches::**kDisableCloudPolicyOnSignin,
>>> > On 2013/10/16 09:50:34, Joao da Silva wrote:
>>> > > On 2013/10/15 19:48:09, Bernhard Bauer wrote:
>>> > > > Does this switch actually need to be in components/ if it's only
>>> used in
>>> > > chrome/
>>> > > > code?
>>> > >
>>> > > It's used by user_cloud_policy_manager_**factory and that class
>>> will move
>>>
>> into
>>
>>> > the
>>> > > component.
>>> >
>>> > But it's also mapped to a preference. Could you just look up the pref
>>> value
>>>
>> in
>>
>>> > UserCloudPolicyManagerFactory?
>>>
>>
>>  Unfortunately no, because the UserCloudPolicyManager is a dependency of
>>> the
>>> PrefService and must be created before the profile PrefService.
>>>
>>
>>  Note also that that pref is not in local state.
>>>
>>
>> Could we do it the other way then and just check the command line switch?
>> It
>> seems a bit confusing to have this in two different places.
>>
>>
> That sounds reasonable. In fact, I think this flag isn't even needed
> anymore and Drew planned to remove it some time ago.
>
> @Drew, can we get right of --disable-cloud-policy-on-signin and
> prefs::kDisableCloudPolicyOnSignin?
>
> @Bernhard: in any case that's for another CL, and this one is just about
> moving things around without changing behavior. Can I haz stamp? :-)
>
>
>
>>
https://codereview.chromium.**org/26972003/<https://codereview.chromium.org/2...
>>
>
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[Bernhard Bauer, 2013-10-16 17:51:35]

OK, you can haz stamp: LGTM.

[commit-bot: I haz the power, 2013-10-16 17:56:17]

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

[commit-bot: I haz the power, 2013-10-16 20:46:45]

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

[commit-bot: I haz the power, 2013-10-16 21:42:07]

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

[commit-bot: I haz the power, 2013-10-17 01:46:53]

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

[Joao da Silva, 2013-10-17 08:58:46]

Committed patchset #4 manually as r229084 (presubmit successful).