Add check for user in cros settings handler

Add check for user in cros settings handler

BUG=341969
R=nkostylev@chromium.org

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

[Nikita (slow), 2014-02-18 09:53:24]

UserManager::GetPrimaryUser() will return NULL on the login UI.

For the login UI there's a special "signin profile"
ProfileHelper::GetSigninProfile(). But keep in mind that if that method is
called when signin profile doesn't exist it will create one and return it
(that's not what you want inside session).

I think adding explicit check whether current state is logged in or not might
help.

[michaelpg, 2014-02-18 10:00:32]

On 2014/02/18 09:53:24, Nikita Kostylev wrote:
> UserManager::GetPrimaryUser() will return NULL on the login UI.
> 
> For the login UI there's a special "signin profile"
> ProfileHelper::GetSigninProfile(). But keep in mind that if that method is
> called when signin profile doesn't exist it will create one and return it
> (that's not what you want inside session).
> 
> I think adding explicit check whether current state is logged in or not might
> help.

UserManager::GetPrimaryUser() is only called it
UserManager::GetUserByProfile(profile) succeeds - in which case won't there be a
primary user?

[Nikita (slow), 2014-02-18 10:00:39]

https://codereview.chromium.org/169993002/diff/1/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc
(right):

https://codereview.chromium.org/169993002/diff/1/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc:261:
User* user = user_manager->GetUserByProfile(profile);
On the login UI UserManagerImpl::GetUserByProfile() will return NULL when you
pass signin profile to it.

So you can either rely on this logic or (what's better) do this check here as
well.

if (ProfileHelper::IsSigninProfile(profile))

[michaelpg, 2014-02-18 20:14:16]

https://codereview.chromium.org/169993002/diff/1/chrome/browser/ui/webui/opti...
File chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc
(right):

https://codereview.chromium.org/169993002/diff/1/chrome/browser/ui/webui/opti...
chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc:261:
User* user = user_manager->GetUserByProfile(profile);
On 2014/02/18 10:00:39, Nikita Kostylev wrote:
> On the login UI UserManagerImpl::GetUserByProfile() will return NULL when you
> pass signin profile to it.
> 
> So you can either rely on this logic or (what's better) do this check here as
> well.
> 
> if (ProfileHelper::IsSigninProfile(profile))

OK, I've added this check. I don't feel it's necessary in
cros_language_options_handler.cc though.

[Nikita (slow), 2014-02-18 20:23:41]

lgtm

https://codereview.chromium.org/169993002/diff/70001/chrome/browser/ui/webui/...
File chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc
(right):

https://codereview.chromium.org/169993002/diff/70001/chrome/browser/ui/webui/...
chrome/browser/ui/webui/options/chromeos/core_chromeos_options_handler.cc:262:
if (user && !ProfileHelper::IsSigninProfile(profile) &&
If this is sign in profile than user will always be NULL so second condition
will never be executed at login.

Inside session it is not needed.

Let's drop this condition then and rely on GetUserByProfile() then.

[michaelpg, 2014-02-18 20:31:56]

The CQ bit was checked by michaelpg@chromium.org

[commit-bot: I haz the power, 2014-02-18 20:33:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/michaelpg@chromium.org/169993002/140001

[commit-bot: I haz the power, 2014-02-18 23:17:05]

Change committed as 251871