Implement users options handling in login screen.

Implement users options handling in login screen.

- Use BrowserProcess's local_state as a cache to users settings;
- Update login code to use the cached users settings per chromium-os:6789

BUG=chromium-os:6789
TEST=Verify fix for chromium-os:6789.

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

[zel, 2010-09-20 20:14:06]

LGTM

[tfarina, 2010-09-20 21:56:53]

http://codereview.chromium.org/3413021/diff/1/8
File chrome/browser/chromeos/login/user_manager.cc (right):

http://codereview.chromium.org/3413021/diff/1/8#newcode222
chrome/browser/chromeos/login/user_manager.cc:222: std::string user_email =
it->email();
I think you don't need a temp variable. You could just compare email to
it->email(), no?

[tfarina, 2010-09-20 22:00:26]

http://codereview.chromium.org/3413021/diff/1/6
File chrome/browser/chromeos/login/existing_user_controller.h (right):

http://codereview.chromium.org/3413021/diff/1/6#newcode122
chrome/browser/chromeos/login/existing_user_controller.h:122: virtual void
OnCheckWhiteListCompleted(
Probably you can move |bool success| to the end of the function declaration, and
align |email| with it in the next line.

[Dmitry Polukhin, 2010-09-21 06:33:29]

http://codereview.chromium.org/3413021/diff/1/5
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

http://codereview.chromium.org/3413021/diff/1/5#newcode282
chrome/browser/chromeos/login/existing_user_controller.cc:282:
UTF16ToUTF8(password_),
Please clear password_ as soon as you don't need it anymore. We should try to
reduce number of password copies as much as possible and clear it as soon as we
don't need it anymore to reduce probability of accidental leaking password to
logs and dumps.

[Nikita (slow), 2010-09-21 14:06:30]

http://codereview.chromium.org/3413021/diff/1/5
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

http://codereview.chromium.org/3413021/diff/1/5#newcode124
chrome/browser/chromeos/login/existing_user_controller.cc:124: // TODO(xiyuan):
Clean user profile whose email is not in whitelist.
Please file an issue on that.

http://codereview.chromium.org/3413021/diff/1/5#newcode288
chrome/browser/chromeos/login/existing_user_controller.cc:288: authenticator_ =
LoginUtils::Get()->CreateAuthenticator(this);
I'd add check for UserCrosSettingsProvider::cached_allow_bwsi() here because it
might be initiated via key accelerator.

[xiyuan, 2010-09-21 16:24:04]

http://codereview.chromium.org/3413021/diff/1/5
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

http://codereview.chromium.org/3413021/diff/1/5#newcode124
chrome/browser/chromeos/login/existing_user_controller.cc:124: // TODO(xiyuan):
Clean user profile whose email is not in whitelist.
On 2010/09/21 14:06:30, Nikita Kostylev wrote:
> Please file an issue on that.

Done. http://crosbug.com/6882

http://codereview.chromium.org/3413021/diff/1/5#newcode282
chrome/browser/chromeos/login/existing_user_controller.cc:282:
UTF16ToUTF8(password_),
On 2010/09/21 06:33:29, Dmitry Polukhin wrote:
> Please clear password_ as soon as you don't need it anymore. We should try to
> reduce number of password copies as much as possible and clear it as soon as
we
> don't need it anymore to reduce probability of accidental leaking password to
> logs and dumps.

Done.

http://codereview.chromium.org/3413021/diff/1/5#newcode288
chrome/browser/chromeos/login/existing_user_controller.cc:288: authenticator_ =
LoginUtils::Get()->CreateAuthenticator(this);
On 2010/09/21 14:06:30, Nikita Kostylev wrote:
> I'd add check for UserCrosSettingsProvider::cached_allow_bwsi() here because
it
> might be initiated via key accelerator.

Done.

http://codereview.chromium.org/3413021/diff/1/6
File chrome/browser/chromeos/login/existing_user_controller.h (right):

http://codereview.chromium.org/3413021/diff/1/6#newcode122
chrome/browser/chromeos/login/existing_user_controller.h:122: virtual void
OnCheckWhiteListCompleted(
On 2010/09/20 22:00:26, tfarina wrote:
> Probably you can move |bool success| to the end of the function declaration,
and
> align |email| with it in the next line.

Done.

http://codereview.chromium.org/3413021/diff/1/8
File chrome/browser/chromeos/login/user_manager.cc (right):

http://codereview.chromium.org/3413021/diff/1/8#newcode222
chrome/browser/chromeos/login/user_manager.cc:222: std::string user_email =
it->email();
On 2010/09/20 21:56:53, tfarina wrote:
> I think you don't need a temp variable. You could just compare email to
> it->email(), no?

Done.

[Nikita (slow), 2010-09-22 13:45:05]

LGTM

[Nikita (slow), 2010-11-03 16:58:56]

I found out that this CL disabled a "feature" - changing user picture when
logging in as a "new user" with one of the existing users.

http://codereview.chromium.org/3413021/diff/2002/10004
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

http://codereview.chromium.org/3413021/diff/2002/10004#newcode469
chrome/browser/chromeos/login/existing_user_controller.cc:469:
!UserManager::Get()->IsKnownUser(username)) {
Was this an intentional change? It doesn't seem that this condition is needed
for http://crosbug.com/6789.

It was a "feature" to change a user picture.

[xiyuan, 2010-11-03 17:34:36]

http://codereview.chromium.org/3413021/diff/2002/10004
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

http://codereview.chromium.org/3413021/diff/2002/10004#newcode469
chrome/browser/chromeos/login/existing_user_controller.cc:469:
!UserManager::Get()->IsKnownUser(username)) {
On 2010/11/03 16:58:56, Nikita Kostylev wrote:
> Was this an intentional change? It doesn't seem that this condition is needed
> for http://crosbug.com/6789.
> 
> It was a "feature" to change a user picture.

I thought it was a bug. This code path is from new user pod click. I was annoyed
when I was making the change and don't know it's a feature.

Please feel free to revert this. Or you want me to do it?

[Nikita (slow), 2010-11-03 17:42:25]

http://codereview.chromium.org/3413021/diff/2002/10004
File chrome/browser/chromeos/login/existing_user_controller.cc (right):

http://codereview.chromium.org/3413021/diff/2002/10004#newcode469
chrome/browser/chromeos/login/existing_user_controller.cc:469:
!UserManager::Get()->IsKnownUser(username)) {
On 2010/11/03 17:34:36, xiyuan wrote:
> On 2010/11/03 16:58:56, Nikita Kostylev wrote:
> > Was this an intentional change? It doesn't seem that this condition is
needed
> > for http://crosbug.com/6789.
> > 
> > It was a "feature" to change a user picture.
> 
> I thought it was a bug. This code path is from new user pod click. I was
annoyed
> when I was making the change and don't know it's a feature.
> 
> Please feel free to revert this. Or you want me to do it? 

There were some other reports too so it's ok to leave it as it is.
We have another way: delete user pod, login as a new user again. It wouldn't
work for owner though. But picture change will be supported at the settings >
users page.