Add support for a managed user pref store to PrefService.

Add support for a managed user pref store to PrefService.

TBR=phajdan.jr@chromium.org
BUG=280674
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=220274

[Bernhard Bauer, 2013-08-29 07:01:20]

Please review.

[Mattias Nissler (ping if slow), 2013-08-29 07:53:58]

LGTM

[Bernhard Bauer, 2013-08-29 08:51:40]

TBR: Paweł for chrome/test/base/testing_pref_service_syncable.cc (updating a
constructor signature)

[commit-bot: I haz the power, 2013-08-29 08:55:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/bauerb@chromium.org/23592013/1

[commit-bot: I haz the power, 2013-08-29 11:53:43]

Change committed as 220274

[bartfab (slow), 2013-08-29 12:02:02]

Would "supervised_user_prefs" not have been better naming? We already have
"managed prefs" and "user prefs". Now we have "managed_user_prefs," which are
not a combination of the two above, even though the name implies it :(.

https://chromiumcodereview.appspot.com/23592013/diff/1/base/prefs/pref_value_...
File base/prefs/pref_value_store.h (right):

https://chromiumcodereview.appspot.com/23592013/diff/1/base/prefs/pref_value_...
base/prefs/pref_value_store.h:35: //   |managed_prefs| contains all preferences
from mandatory policies.
|managed_user_prefs| is missing in this comment.

[Bernhard Bauer, 2013-08-29 12:19:35]

On 2013/08/29 12:02:02, bartfab wrote:
> Would "supervised_user_prefs" not have been better naming? We already have
> "managed prefs" and "user prefs". Now we have "managed_user_prefs," which are
> not a combination of the two above, even though the name implies it :(.

Hm, the term "supervised user" is only used in very few places in the code; it's
mostly in the user-visible strings. Chrome OS uses "locally managed user" in
some places, should we maybe use that?

>
https://chromiumcodereview.appspot.com/23592013/diff/1/base/prefs/pref_value_...
> File base/prefs/pref_value_store.h (right):
> 
>
https://chromiumcodereview.appspot.com/23592013/diff/1/base/prefs/pref_value_...
> base/prefs/pref_value_store.h:35: //   |managed_prefs| contains all
preferences
> from mandatory policies.
> |managed_user_prefs| is missing in this comment.

I'll create a followup CL.

[bartfab (slow), 2013-08-29 12:32:14]

On 2013/08/29 12:19:35, Bernhard Bauer wrote:
> On 2013/08/29 12:02:02, bartfab wrote:
> > Would "supervised_user_prefs" not have been better naming? We already have
> > "managed prefs" and "user prefs". Now we have "managed_user_prefs," which
are
> > not a combination of the two above, even though the name implies it :(.
> 
> Hm, the term "supervised user" is only used in very few places in the code;
it's
> mostly in the user-visible strings. Chrome OS uses "locally managed user" in
> some places, should we maybe use that?

It's one of those features that have changed names several times. I see no need
to go back and consolidate naming in existing code but it would be great to
settle on a single, good name for all new code. I actually like "supervised
user" most, also because it is consistent with the UI.

> 
> >
>
https://chromiumcodereview.appspot.com/23592013/diff/1/base/prefs/pref_value_...
> > File base/prefs/pref_value_store.h (right):
> > 
> >
>
https://chromiumcodereview.appspot.com/23592013/diff/1/base/prefs/pref_value_...
> > base/prefs/pref_value_store.h:35: //   |managed_prefs| contains all
> preferences
> > from mandatory policies.
> > |managed_user_prefs| is missing in this comment.
> 
> I'll create a followup CL.

[Bernhard Bauer, 2013-08-29 12:42:22]

OK... do we really want to use the new name in *all* new code though? For
example, I have a class that deals with managed user settings (as in, the
Sync data type of that name), so I'd like to keep using that term there.

So, seeing as we're newly introducing the concept of preferences for
supervised users to the prefs code, how about we use "supervised user" in
prefs code, and keep "managed user settings" etc. in managed_mode/ code?


On Thu, Aug 29, 2013 at 2:32 PM, <bartfab@chromium.org> wrote:

> On 2013/08/29 12:19:35, Bernhard Bauer wrote:
>
>> On 2013/08/29 12:02:02, bartfab wrote:
>> > Would "supervised_user_prefs" not have been better naming? We already
>> have
>> > "managed prefs" and "user prefs". Now we have "managed_user_prefs,"
>> which
>>
> are
>
>> > not a combination of the two above, even though the name implies it :(.
>>
>
>  Hm, the term "supervised user" is only used in very few places in the
>> code;
>>
> it's
>
>> mostly in the user-visible strings. Chrome OS uses "locally managed user"
>> in
>> some places, should we maybe use that?
>>
>
> It's one of those features that have changed names several times. I see no
> need
> to go back and consolidate naming in existing code but it would be great to
> settle on a single, good name for all new code. I actually like "supervised
> user" most, also because it is consistent with the UI.
>
>
>
>  >
>>
>
> https://chromiumcodereview.**appspot.com/23592013/diff/1/**
>
base/prefs/pref_value_store.h<https://chromiumcodereview.appspot.com/23592013/diff/1/base/prefs/pref_value_store.h>
>
>> > File base/prefs/pref_value_store.h (right):
>> >
>> >
>>
>
> https://chromiumcodereview.**appspot.com/23592013/diff/1/**
>
base/prefs/pref_value_store.h#**newcode35<https://chromiumcodereview.appspot.com/23592013/diff/1/base/prefs/pref_value_store.h#newcode35>
>
>> > base/prefs/pref_value_store.h:**35: //   |managed_prefs| contains all
>> preferences
>> > from mandatory policies.
>> > |managed_user_prefs| is missing in this comment.
>>
>
>  I'll create a followup CL.
>>
>
>
>
>
https://chromiumcodereview.**appspot.com/23592013/<https://chromiumcodereview...
>

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

[bartfab (slow), 2013-08-29 12:47:01]

On 2013/08/29 12:42:22, Bernhard Bauer wrote:
> OK... do we really want to use the new name in *all* new code though? For
> example, I have a class that deals with managed user settings (as in, the
> Sync data type of that name), so I'd like to keep using that term there.
> 
> So, seeing as we're newly introducing the concept of preferences for
> supervised users to the prefs code, how about we use "supervised user" in
> prefs code, and keep "managed user settings" etc. in managed_mode/ code?

Your call. I am all for consistent naming. But what "consistent" here is a
matter of interpretation - consistent across all new code or consistent with the
surrounding code in different places.

> 
> 
> On Thu, Aug 29, 2013 at 2:32 PM, <mailto:bartfab@chromium.org> wrote:
> 
> > On 2013/08/29 12:19:35, Bernhard Bauer wrote:
> >
> >> On 2013/08/29 12:02:02, bartfab wrote:
> >> > Would "supervised_user_prefs" not have been better naming? We already
> >> have
> >> > "managed prefs" and "user prefs". Now we have "managed_user_prefs,"
> >> which
> >>
> > are
> >
> >> > not a combination of the two above, even though the name implies it :(.
> >>
> >
> >  Hm, the term "supervised user" is only used in very few places in the
> >> code;
> >>
> > it's
> >
> >> mostly in the user-visible strings. Chrome OS uses "locally managed user"
> >> in
> >> some places, should we maybe use that?
> >>
> >
> > It's one of those features that have changed names several times. I see no
> > need
> > to go back and consolidate naming in existing code but it would be great to
> > settle on a single, good name for all new code. I actually like "supervised
> > user" most, also because it is consistent with the UI.
> >
> >
> >
> >  >
> >>
> >
> > https://chromiumcodereview.**appspot.com/23592013/diff/1/**
> >
>
base/prefs/pref_value_store.h<https://chromiumcodereview.appspot.com/23592013/diff/1/base/prefs/pref_value_store.h>
> >
> >> > File base/prefs/pref_value_store.h (right):
> >> >
> >> >
> >>
> >
> > https://chromiumcodereview.**appspot.com/23592013/diff/1/**
> >
>
base/prefs/pref_value_store.h#**newcode35<https://chromiumcodereview.appspot.com/23592013/diff/1/base/prefs/pref_value_store.h#newcode35>
> >
> >> > base/prefs/pref_value_store.h:**35: //   |managed_prefs| contains all
> >> preferences
> >> > from mandatory policies.
> >> > |managed_user_prefs| is missing in this comment.
> >>
> >
> >  I'll create a followup CL.
> >>
> >
> >
> >
> >
>
https://chromiumcodereview.**appspot.com/23592013/%3Chttps://chromiumcoderevi...>
> >
> 
> To unsubscribe from this group and stop receiving emails from it, send an
email
> to mailto:chromium-reviews+unsubscribe@chromium.org.

[Paweł Hajdan Jr., 2013-08-29 17:42:08]

chrome/test LGTM