[cros] Set profile pref for locally managed users

[cros] Set profile pref for locally managed users
Depends on https://codereview.chromium.org/12225051

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

[Nikita (slow), 2013-02-08 10:49:08]

dpolukhin@ > chromeos/login

rpetterson@ > profiles/*

[Nikita (slow), 2013-02-08 15:48:26]

Please take a look, thanks!

[Dmitry Polukhin, 2013-02-08 17:26:53]

https://codereview.chromium.org/12225046/diff/5/chrome/browser/chromeos/login...
File chrome/browser/chromeos/login/login_utils.cc (right):

https://codereview.chromium.org/12225046/diff/5/chrome/browser/chromeos/login...
chrome/browser/chromeos/login/login_utils.cc:509: // Processing
Profile::CREATE_STATUS_INITIALIZED notification.
Could you please remove this comment and make it clear from the code that it is
Profile::CREATE_STATUS_INITIALIZED? Switch above makes code harder to read
without adding value.

https://codereview.chromium.org/12225046/diff/5/chrome/browser/profiles/profi...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/12225046/diff/5/chrome/browser/profiles/profi...
chrome/browser/profiles/profile_manager.cc:479: // Chrome OS specific note:
since we pass string16() here as the icon_url,
Why do we need this change in this case??? If all real things happen in
CREATE_STATUS_CREATED notification, can we remove this code?

[Nikita (slow), 2013-02-08 21:55:05]

https://codereview.chromium.org/12225046/diff/5/chrome/browser/profiles/profi...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/12225046/diff/5/chrome/browser/profiles/profi...
chrome/browser/profiles/profile_manager.cc:479: // Chrome OS specific note:
since we pass string16() here as the icon_url,
On 2013/02/08 17:26:53, Dmitry Polukhin wrote:
> Why do we need this change in this case??? If all real things happen in
> CREATE_STATUS_CREATED notification, can we remove this code?

I agree that we could always pass false here for |is_managed| as we do for other
parameters and I will leave this comment because it summarizes the difference
between multi-profile handling (profile cache structure) on CrOS.

[rpetterson, 2013-02-08 22:37:56]

https://codereview.chromium.org/12225046/diff/5/chrome/browser/profiles/profi...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/12225046/diff/5/chrome/browser/profiles/profi...
chrome/browser/profiles/profile_manager.cc:479: // Chrome OS specific note:
since we pass string16() here as the icon_url,
On 2013/02/08 21:55:05, Nikita Kostylev wrote:
> On 2013/02/08 17:26:53, Dmitry Polukhin wrote:
> > Why do we need this change in this case??? If all real things happen in
> > CREATE_STATUS_CREATED notification, can we remove this code?
> 
> I agree that we could always pass false here for |is_managed| as we do for
other
> parameters and I will leave this comment because it summarizes the difference
> between multi-profile handling (profile cache structure) on CrOS.

nit: The comment could be a bit clearer: which property will get initialized in
the Profile::CREATE_STATUS_CREATED callback? The icon_url or the is_managed
value?

Otherwise, LGTM for profiles.

[Dmitry Polukhin, 2013-02-09 04:21:37]

LGTM but please fix my comment in login_utils.cc before committing the code.

[Nikita (slow), 2013-02-11 09:42:12]

Extracted method UserProfileInitialized(user_profile).

https://codereview.chromium.org/12225046/diff/5/chrome/browser/chromeos/login...
File chrome/browser/chromeos/login/login_utils.cc (right):

https://codereview.chromium.org/12225046/diff/5/chrome/browser/chromeos/login...
chrome/browser/chromeos/login/login_utils.cc:509: // Processing
Profile::CREATE_STATUS_INITIALIZED notification.
On 2013/02/08 17:26:53, Dmitry Polukhin wrote:
> Could you please remove this comment and make it clear from the code that it
is
> Profile::CREATE_STATUS_INITIALIZED? Switch above makes code harder to read
> without adding value.

Done.

https://codereview.chromium.org/12225046/diff/5/chrome/browser/profiles/profi...
File chrome/browser/profiles/profile_manager.cc (right):

https://codereview.chromium.org/12225046/diff/5/chrome/browser/profiles/profi...
chrome/browser/profiles/profile_manager.cc:479: // Chrome OS specific note:
since we pass string16() here as the icon_url,
On 2013/02/08 22:37:56, rpetterson wrote:
> On 2013/02/08 21:55:05, Nikita Kostylev wrote:
> > On 2013/02/08 17:26:53, Dmitry Polukhin wrote:
> > > Why do we need this change in this case??? If all real things happen in
> > > CREATE_STATUS_CREATED notification, can we remove this code?
> > 
> > I agree that we could always pass false here for |is_managed| as we do for
> other
> > parameters and I will leave this comment because it summarizes the
difference
> > between multi-profile handling (profile cache structure) on CrOS.
> 
> nit: The comment could be a bit clearer: which property will get initialized
in
> the Profile::CREATE_STATUS_CREATED callback? The icon_url or the is_managed
> value?
> 
> Otherwise, LGTM for profiles.

Done.

[Dmitry Polukhin, 2013-02-11 09:50:12]

LGTM with nit

https://codereview.chromium.org/12225046/diff/7004/chrome/browser/chromeos/lo...
File chrome/browser/chromeos/login/login_utils.cc (right):

https://codereview.chromium.org/12225046/diff/7004/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/login_utils.cc:506: case
Profile::CREATE_STATUS_CREATED: {
nit, please remove {} they are not required.

[Nikita (slow), 2013-02-11 10:16:39]

https://codereview.chromium.org/12225046/diff/7004/chrome/browser/chromeos/lo...
File chrome/browser/chromeos/login/login_utils.cc (right):

https://codereview.chromium.org/12225046/diff/7004/chrome/browser/chromeos/lo...
chrome/browser/chromeos/login/login_utils.cc:506: case
Profile::CREATE_STATUS_CREATED: {
On 2013/02/11 09:50:12, Dmitry Polukhin wrote:
> nit, please remove {} they are not required.

Done.