If user profile doesn't contain language setting, default to his Google account settings.

When user first time logs into Chromeos, he doesn't have "language" setting filled in his Chromeos profile. So he will inherit owner's locale.

We can improve this experience defaulting to his Google Account locale. So that the actual algorithm of language selection looks like this:
1) Use "language" setting from profile.
2) If failed, use "locale" setting from Google Account.
3) If failed, use owner's default.

BUG=chromium:140591
TEST=
1) Remove user from device.
2) Clear user profile in https://www.google.com/settings/chrome/sync
3) Set Account language to some different to owners locale.
4) Login to device should now either use Account language immediately after login, or "Locale changed" message should appear after Account data is synchronized.

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

[Alexander Alekseev, 2013-08-23 03:50:52]

The idea is to wait until G+ profile and Chromeos User profile are both loaded
and then select locale.

This probably should be done in ProfileManager, but it is destroyed now
immediately upon profile load, so it cannot wait untill G+ profile is loaded.
Therefore I added a flag to Profile.

Also there is no separate object for G+ profile and all loading is currently
done now in UserImageManager. Therefore locale is now set by it ;-)


Please review:

  sky@ :
chrome/test/base/testing_profile.h
chrome/browser/profiles/profile_impl.*
chrome/browser/profiles/profile.*

  sail@ :
chrome/browser/profiles/profile_downloader*

  yosin@ :
chrome/browser/profiles/off_the_record_profile_impl*

  nkostylev@ :
chrome/browser/chromeos/login/user_image_manager_impl.cc
chrome/browser/chromeos/login/login_utils.cc

  jshin@ :
ui/base/l10n/l10n_util*

[sail, 2013-08-23 04:45:20]

CL description needs work:
  - first line should be a short summary (need to update the "subject" field to
match the summary)
  - next should be a longer description of what you're doing and why (basically
your message)
  - wrap line at 80 (or 72)
  - need a TEST= section that gives step by step instructions on how to exercise
this
  - small nit, don't say "his profile". Maybe "default to the G+ profile
language".

[yosin_UTC9, 2013-08-23 07:41:12]

ACK for chrome/browser/profiles/off_the_record_profile_impl*

[sky, 2013-08-23 16:02:56]

I think you only need me for chrome/test/base/testing_profile.h. You have other
owners for profiles. That said, these methods don't feel like they belong in
Profile, but the other reviewers can better comment on that.

https://codereview.chromium.org/23095006/diff/18001/chrome/test/base/testing_...
File chrome/test/base/testing_profile.h (right):

https://codereview.chromium.org/23095006/diff/18001/chrome/test/base/testing_...
chrome/test/base/testing_profile.h:266: virtual void SetGPlusProfileLocale(const
std::string&) OVERRIDE {
nit: style guide says always name parameters (257/258 is wrong too).

[sail, 2013-08-23 17:08:48]

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/profiles/p...
File chrome/browser/profiles/profile_impl.cc (right):

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_impl.cc:146: COMPILE_ASSERT(sizeof(ProfileImpl)
<= 744u, profile_impl_size_unexpected);
Your CL will probably break this assert.
Could you move this into a separate class or into another existing class?

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/profiles/p...
File chrome/browser/profiles/profile_impl.h (right):

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_impl.h:196: #if defined(OS_CHROMEOS)
I really dislike adding new ifdefs to the code. The best way to add new platform
specific code is to have it in a platform specific file. Something like:
  my_profile_locale_cros.cc

[Nikita (slow), 2013-08-26 09:26:13]

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/user_image_manager_impl.cc (right):

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user_image_manager_impl.cc:648:
SetGPlusProfileLocale(downloader->GetProfileLocale());
nit: SetGPlusProfileLocale - this method needs to be renamed. This should be
called just AccountLocale (as in Google Account). We may later expand this API.

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user_image_manager_impl.cc:730:
SetGPlusProfileLocale(downloader->GetProfileLocale());
Should this really be called in failure case?

[jungshik at Google, 2013-08-26 16:42:50]

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/profiles/p...
File chrome/browser/profiles/profile_impl.cc (right):

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_impl.cc:1078:
l10n_util::CheckAndResolveLocale(gplus_locale, gplus_locale_.get());
This does not take into account the preferred language list in the Google user
profile. If a user's primary language is not supported by Chrome OS (let's say,
"Foo") but one of languages in the fallback list in the user's Google profile
(e.g. French) is supported by Chrome, we have to pick it up instead of falling
back to the universal (hard-coded) fallback ('en-US'). 

The Linux locale fallback has a similar mechanism because 'LANGUAGE' environment
variable can have a list of languages (e.g. 'foo:fr-CA:fr-FR').  

If you don't want to do now, at least add TODO comment about this.

[Alexander Alekseev, 2013-09-06 19:52:07]

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/user_image_manager_impl.cc (right):

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user_image_manager_impl.cc:648:
SetGPlusProfileLocale(downloader->GetProfileLocale());
On 2013/08/26 09:26:13, nkostylev (OOO Aug 27 - Sep15) wrote:
> nit: SetGPlusProfileLocale - this method needs to be renamed. This should be
> called just AccountLocale (as in Google Account). We may later expand this
API.

Done.

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user_image_manager_impl.cc:730:
SetGPlusProfileLocale(downloader->GetProfileLocale());
On 2013/08/26 09:26:13, nkostylev (OOO Aug 27 - Sep15) wrote:
> Should this really be called in failure case?

Yes. I've commented it in the new patch set.

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/profiles/p...
File chrome/browser/profiles/profile_impl.cc (right):

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_impl.cc:146: COMPILE_ASSERT(sizeof(ProfileImpl)
<= 744u, profile_impl_size_unexpected);
On 2013/08/23 17:08:48, sail wrote:
> Your CL will probably break this assert.
> Could you move this into a separate class or into another existing class?

Done.

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_impl.cc:1078:
l10n_util::CheckAndResolveLocale(gplus_locale, gplus_locale_.get());
On 2013/08/26 16:42:50, Jungshik Shin wrote:
> This does not take into account the preferred language list in the Google user
> profile. If a user's primary language is not supported by Chrome OS (let's
say,
> "Foo") but one of languages in the fallback list in the user's Google profile
> (e.g. French) is supported by Chrome, we have to pick it up instead of falling
> back to the universal (hard-coded) fallback ('en-US'). 
> 
> The Linux locale fallback has a similar mechanism because 'LANGUAGE'
environment
> variable can have a list of languages (e.g. 'foo:fr-CA:fr-FR').  
> 
> If you don't want to do now, at least add TODO comment about this. 

I like the idea, but I couldn't find "list of languages" in API.
https://developers.google.com/+/api/latest/people
- doesn't list anything alike.

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/profiles/p...
File chrome/browser/profiles/profile_impl.h (right):

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_impl.h:196: #if defined(OS_CHROMEOS)
On 2013/08/23 17:08:48, sail wrote:
> I really dislike adding new ifdefs to the code. The best way to add new
platform
> specific code is to have it in a platform specific file. Something like:
>   my_profile_locale_cros.cc

Done.

https://codereview.chromium.org/23095006/diff/18001/chrome/test/base/testing_...
File chrome/test/base/testing_profile.h (right):

https://codereview.chromium.org/23095006/diff/18001/chrome/test/base/testing_...
chrome/test/base/testing_profile.h:266: virtual void SetGPlusProfileLocale(const
std::string&) OVERRIDE {
On 2013/08/23 16:02:56, sky wrote:
> nit: style guide says always name parameters (257/258 is wrong too).

Done.

[Alexander Alekseev, 2013-09-06 19:54:17]

Please review updated version. I moved all data from Profile to User object.

Added dpolukhin to reviewers as Nikita is temporary not available.

[Dmitry Polukhin, 2013-09-06 20:30:04]

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/user.h (right):

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user.h:179: // Used only by UserManagerImpl
Do we need such comment? It will be outdated very soon. Anyway if someone needs
to see all use-cases, he can use codesearch.

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user.h:231: bool has_account() const;
I would call it has_gaia_account to make it clear what type of account it is.

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user_manager_impl.cc:755: // TODO(alemate):
Jungshik Shin: 2013/08/26 16:42:50
Please file issue about and add here. TODO(alemate) with the comment and link to
issue should be enough. Please remove 'Jungshik Shin: 2013/08/26 16:42:50'

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user_manager_impl.cc:798: const std::string&
username_hash;
Please make private members last.

https://codereview.chromium.org/23095006/diff/33001/ui/base/l10n/l10n_util.h
File ui/base/l10n/l10n_util.h (right):

https://codereview.chromium.org/23095006/diff/33001/ui/base/l10n/l10n_util.h#...
ui/base/l10n/l10n_util.h:24: #if !defined(OS_MACOSX)
I don't like platform specific functions in very common headers. But it up to
ui/base OWNERS to decide. Possible alternative it to have it for all platforms
but trigger NOTIMPLEMENTED or just return false.

[sail, 2013-09-09 22:02:17]

profiles/* LGTM

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/profiles/p...
File chrome/browser/profiles/profile_downloader_unittest.cc (right):

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_downloader_unittest.cc:39: std::string
parsed_locale;
could you add a test for this?

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/profiles/p...
File chrome/browser/profiles/profile_impl.cc (right):

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_impl.cc:11: #include "base/debug/stack_trace.h"
don't need?

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_impl.cc:33: #include
"chrome/browser/chromeos/login/language_switch_menu.h"
don't need?

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_impl.cc:1043: // (1) first save it to temporary
backup;
is this comment still valid?

[Alexander Alekseev, 2013-09-11 18:53:02]

Moved away  #if !defined(OS_MACOSX) from header.
Added unit test for locale field in GAIA account JSON parse.

Please review.

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/user.h (right):

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user.h:179: // Used only by UserManagerImpl
On 2013/09/06 20:30:05, Dmitry Polukhin wrote:
> Do we need such comment? It will be outdated very soon. Anyway if someone
needs
> to see all use-cases, he can use codesearch.

Done.

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user.h:231: bool has_account() const;
On 2013/09/06 20:30:05, Dmitry Polukhin wrote:
> I would call it has_gaia_account to make it clear what type of account it is.

Done.

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user_manager_impl.cc:755: // TODO(alemate):
Jungshik Shin: 2013/08/26 16:42:50
On 2013/09/06 20:30:05, Dmitry Polukhin wrote:
> Please file issue about and add here. TODO(alemate) with the comment and link
to
> issue should be enough. Please remove 'Jungshik Shin: 2013/08/26 16:42:50'

Done.

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user_manager_impl.cc:798: const std::string&
username_hash;
On 2013/09/06 20:30:05, Dmitry Polukhin wrote:
> Please make private members last.

Done.

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/profiles/p...
File chrome/browser/profiles/profile_downloader_unittest.cc (right):

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_downloader_unittest.cc:39: std::string
parsed_locale;
On 2013/09/09 22:02:17, sail wrote:
> could you add a test for this?

Done.

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/profiles/p...
File chrome/browser/profiles/profile_impl.cc (right):

https://codereview.chromium.org/23095006/diff/33001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_impl.cc:1043: // (1) first save it to temporary
backup;
On 2013/09/09 22:02:17, sail wrote:
> is this comment still valid?

I think so, as kApplicationLocale is not priority preference yet. (it's still
TODO to make it a priority).

https://codereview.chromium.org/23095006/diff/33001/ui/base/l10n/l10n_util.h
File ui/base/l10n/l10n_util.h (right):

https://codereview.chromium.org/23095006/diff/33001/ui/base/l10n/l10n_util.h#...
ui/base/l10n/l10n_util.h:24: #if !defined(OS_MACOSX)
On 2013/09/06 20:30:05, Dmitry Polukhin wrote:
> I don't like platform specific functions in very common headers. But it up to
> ui/base OWNERS to decide. Possible alternative it to have it for all platforms
> but trigger NOTIMPLEMENTED or just return false.

Done.

[sail, 2013-09-11 19:00:37]

lgtm again, thanks for the test

https://codereview.chromium.org/23095006/diff/57001/chrome/browser/profiles/p...
File chrome/browser/profiles/profile_downloader_unittest.cc (right):

https://codereview.chromium.org/23095006/diff/57001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_downloader_unittest.cc:91: // Try different
locales.
join these two comments into one sentence?

[Dmitry Polukhin, 2013-09-11 19:04:41]

LGTM

[Alexander Alekseev, 2013-09-12 18:00:48]

jshin@ , please review ui/base/l10n/l10n_util*

[Dmitry Polukhin, 2013-09-12 19:41:24]

+ tony@ for OWNER review in ui/base/l10n/

[Dmitry Polukhin, 2013-09-14 15:58:46]

For multi-profile support this CL needs a bit more work.

https://codereview.chromium.org/23095006/diff/57001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

https://codereview.chromium.org/23095006/diff/57001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user_manager_impl.cc:680:
g_browser_process->profile_manager()->GetDefaultProfile(), user);
Here you need to use profile associated with username, instead of default
profile.

https://codereview.chromium.org/23095006/diff/57001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user_manager_impl.cc:804: User*
UserManagerImpl::GetUserByProfile(Profile* profile) {
This can be done simpler. You can get user email from profile (GetProfileName)
and than call FindUser. The only trick that you need to canonicalize and
sanitize email before calling FindUser.

[Alexander Alekseev, 2013-09-16 18:39:07]

Added Multi Profile support.

tony@, jshin@ - please review, as we need at least one OWNER for
ui/base/l10n/l10n_util.*

https://codereview.chromium.org/23095006/diff/57001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

https://codereview.chromium.org/23095006/diff/57001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user_manager_impl.cc:680:
g_browser_process->profile_manager()->GetDefaultProfile(), user);
On 2013/09/14 15:58:46, Dmitry Polukhin wrote:
> Here you need to use profile associated with username, instead of default
> profile.

Done.

https://codereview.chromium.org/23095006/diff/57001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user_manager_impl.cc:804: User*
UserManagerImpl::GetUserByProfile(Profile* profile) {
On 2013/09/14 15:58:46, Dmitry Polukhin wrote:
> This can be done simpler. You can get user email from profile (GetProfileName)
> and than call FindUser. The only trick that you need to canonicalize and
> sanitize email before calling FindUser.

As far as I know, GetProfileName() is a real e-mail only occasionally. It is
sometimes empty or sometimes a fake ID.

[tony, 2013-09-16 18:47:44]

ui/base/l10n LGTM

https://codereview.chromium.org/23095006/diff/74001/ui/base/l10n/l10n_util.h
File ui/base/l10n/l10n_util.h (right):

https://codereview.chromium.org/23095006/diff/74001/ui/base/l10n/l10n_util.h#...
ui/base/l10n/l10n_util.h:24: // Returns true if succeeded.
Nit: This comment is awkward.  I would say, "This method translates a generic
locale name to one of the locally defined ones.
This method returns true if it succeeds."

[Dmitry Polukhin, 2013-09-16 19:02:25]

LGTM but please rebase and test with latest multi-profile changes before commit

https://codereview.chromium.org/23095006/diff/57001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

https://codereview.chromium.org/23095006/diff/57001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user_manager_impl.cc:804: User*
UserManagerImpl::GetUserByProfile(Profile* profile) {
On 2013/09/16 18:39:07, alemate wrote:
> On 2013/09/14 15:58:46, Dmitry Polukhin wrote:
> > This can be done simpler. You can get user email from profile
(GetProfileName)
> > and than call FindUser. The only trick that you need to canonicalize and
> > sanitize email before calling FindUser.
> 
> As far as I know, GetProfileName() is a real e-mail only occasionally. It is
> sometimes empty or sometimes a fake ID.

GetProfileName is set in InitProfilePreferences and it can be empty only for
supervised users but such users are not allowed for multi-profile.

Please rebase your patch and test with latest changes before commit.

[jungshik at Google, 2013-09-18 08:50:03]

LGTM 2 for the l10n part.

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/profiles/p...
File chrome/browser/profiles/profile_impl.cc (right):

https://codereview.chromium.org/23095006/diff/18001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_impl.cc:1078:
l10n_util::CheckAndResolveLocale(gplus_locale, gplus_locale_.get());
On 2013/09/06 19:52:08, alemate wrote:
> On 2013/08/26 16:42:50, Jungshik Shin wrote:
> > This does not take into account the preferred language list in the Google
user
> > profile. If a user's primary language is not supported by Chrome OS (let's
> say,
> > "Foo") but one of languages in the fallback list in the user's Google
profile
> > (e.g. French) is supported by Chrome, we have to pick it up instead of
falling
> > back to the universal (hard-coded) fallback ('en-US'). 
> > 
> > The Linux locale fallback has a similar mechanism because 'LANGUAGE'
> environment
> > variable can have a list of languages (e.g. 'foo:fr-CA:fr-FR').  
> > 
> > If you don't want to do now, at least add TODO comment about this. 
> 
> I like the idea, but I couldn't find "list of languages" in API.
> https://developers.google.com/+/api/latest/people
> - doesn't list anything alike.

Thanks for digging that up. I've alerted an engineer working on the language
setting of Google users. He's not aware of this API.

[Alexander Alekseev, 2013-09-18 20:21:18]

I've probably fixed all issues.
So I'll ship the latest one I get all approvals. Thank you.

https://codereview.chromium.org/23095006/diff/57001/chrome/browser/chromeos/l...
File chrome/browser/chromeos/login/user_manager_impl.cc (right):

https://codereview.chromium.org/23095006/diff/57001/chrome/browser/chromeos/l...
chrome/browser/chromeos/login/user_manager_impl.cc:804: User*
UserManagerImpl::GetUserByProfile(Profile* profile) {
On 2013/09/16 19:02:26, Dmitry Polukhin wrote:
> On 2013/09/16 18:39:07, alemate wrote:
> > On 2013/09/14 15:58:46, Dmitry Polukhin wrote:
> > > This can be done simpler. You can get user email from profile
> (GetProfileName)
> > > and than call FindUser. The only trick that you need to canonicalize and
> > > sanitize email before calling FindUser.
> > 
> > As far as I know, GetProfileName() is a real e-mail only occasionally. It is
> > sometimes empty or sometimes a fake ID.
> 
> GetProfileName is set in InitProfilePreferences and it can be empty only for
> supervised users but such users are not allowed for multi-profile.
> 
> Please rebase your patch and test with latest changes before commit.

Done.

https://codereview.chromium.org/23095006/diff/57001/chrome/browser/profiles/p...
File chrome/browser/profiles/profile_downloader_unittest.cc (right):

https://codereview.chromium.org/23095006/diff/57001/chrome/browser/profiles/p...
chrome/browser/profiles/profile_downloader_unittest.cc:91: // Try different
locales.
On 2013/09/11 19:00:38, sail wrote:
> join these two comments into one sentence?

Done.

https://codereview.chromium.org/23095006/diff/74001/ui/base/l10n/l10n_util.h
File ui/base/l10n/l10n_util.h (right):

https://codereview.chromium.org/23095006/diff/74001/ui/base/l10n/l10n_util.h#...
ui/base/l10n/l10n_util.h:24: // Returns true if succeeded.
On 2013/09/16 18:47:45, tony wrote:
> Nit: This comment is awkward.  I would say, "This method translates a generic
> locale name to one of the locally defined ones.
> This method returns true if it succeeds."

Done.

[Alexander Alekseev, 2013-09-18 20:22:22]

I meant to say:
- I'm going to ship the latest one as I already have all approvals.

[Dmitry Polukhin, 2013-09-19 16:31:13]

LGTM but please re-run trybots

[commit-bot: I haz the power, 2013-09-19 17:58:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alemate@chromium.org/23095006/99001

[commit-bot: I haz the power, 2013-09-19 21:26:04]

Retried try job too often on linux_aura for step(s) browser_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_aura...

[commit-bot: I haz the power, 2013-09-20 01:15:25]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alemate@chromium.org/23095006/99001

[commit-bot: I haz the power, 2013-09-20 01:15:36]

Failed to apply patch for chrome/browser/chromeos/login/user_manager_impl.cc:
While running patch -p1 --forward --force --no-backup-if-mismatch;
  patching file chrome/browser/chromeos/login/user_manager_impl.cc
  Hunk #2 succeeded at 238 with fuzz 2.
  Hunk #3 succeeded at 698 (offset 1 line).
  Hunk #4 succeeded at 794 (offset 1 line).
  Hunk #5 FAILED at 911.
  1 out of 5 hunks FAILED -- saving rejects to file
chrome/browser/chromeos/login/user_manager_impl.cc.rej

Patch:       chrome/browser/chromeos/login/user_manager_impl.cc
Index: chrome/browser/chromeos/login/user_manager_impl.cc
diff --git a/chrome/browser/chromeos/login/user_manager_impl.cc
b/chrome/browser/chromeos/login/user_manager_impl.cc
index
ddbbb2afb731ba426001f36eaac0eae83ab26862..54b44ed9e9bb89550584b02a78b8604d55d2e412
100644
--- a/chrome/browser/chromeos/login/user_manager_impl.cc
+++ b/chrome/browser/chromeos/login/user_manager_impl.cc
@@ -28,17 +28,20 @@
 #include "chrome/browser/chromeos/login/auth_sync_observer.h"
 #include "chrome/browser/chromeos/login/auth_sync_observer_factory.h"
 #include "chrome/browser/chromeos/login/default_pinned_apps_field_trial.h"
+#include "chrome/browser/chromeos/login/language_switch_menu.h"
 #include "chrome/browser/chromeos/login/login_display.h"
 #include "chrome/browser/chromeos/login/login_utils.h"
 #include "chrome/browser/chromeos/login/remove_user_delegate.h"
 #include "chrome/browser/chromeos/login/user_image_manager_impl.h"
 #include "chrome/browser/chromeos/login/wizard_controller.h"
 #include "chrome/browser/chromeos/policy/device_local_account.h"
+#include "chrome/browser/chromeos/profiles/profile_helper.h"
 #include "chrome/browser/chromeos/session_length_limiter.h"
 #include "chrome/browser/chromeos/settings/cros_settings_names.h"
 #include "chrome/browser/managed_mode/managed_user_service.h"
 #include "chrome/browser/policy/browser_policy_connector.h"
 #include "chrome/browser/prefs/scoped_user_pref_update.h"
+#include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/sync/profile_sync_service.h"
 #include "chrome/browser/sync/profile_sync_service_factory.h"
@@ -235,6 +238,9 @@ UserManagerImpl::UserManagerImpl()
       content::NotificationService::AllSources());
   registrar_.Add(this, chrome::NOTIFICATION_LOGIN_USER_PROFILE_PREPARED,
       content::NotificationService::AllSources());
+  registrar_.Add(this,
+                 chrome::NOTIFICATION_PROFILE_CREATED,
+                 content::NotificationService::AllSources());
   RetrieveTrustedDevicePolicies();
   cros_settings_->AddSettingsObserver(kAccountsPrefDeviceLocalAccounts,
                                       this);
@@ -691,12 +697,34 @@ User::OAuthTokenStatus
UserManagerImpl::LoadUserOAuthStatus(
 
 void UserManagerImpl::SaveUserDisplayName(const std::string& username,
                                           const string16& display_name) {
+  UpdateUserAccountDataImpl(username, display_name, NULL);
+}
+
+void UserManagerImpl::UpdateUserAccountData(const std::string& username,
+                                            const string16& display_name,
+                                            const std::string& locale) {
+  UpdateUserAccountDataImpl(username, display_name, &locale);
+}
+
+void UserManagerImpl::UpdateUserAccountDataImpl(const std::string& username,
+                                                const string16& display_name,
+                                                const std::string* locale) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
   User* user = FindUserAndModify(username);
   if (!user)
     return;  // Ignore if there is no such user.
 
+  // locale is not NULL if User Account has been downloaded
+  // (i.e. it is UpdateUserAccountData(), not SaveUserDisplayName() )
+  if (locale != NULL) {
+    user->SetAccountLocale(*locale);
+    RespectLocalePreference(GetProfileByUser(user), user);
+  }
+
+  if (display_name.empty())
+    return;
+
   user->set_display_name(display_name);
 
   // Do not update local store if data stored or cached outside the user's
@@ -765,6 +793,97 @@ std::string UserManagerImpl::GetUserDisplayEmail(
   return user ? user->display_email() : username;
 }
 
+// TODO(alemate): http://crbug.com/288941 : Respect preferred language list in
+// the Google user profile.
+void UserManagerImpl::RespectLocalePreference(Profile* profile,
+                                              const User* user) const {
+  if (g_browser_process == NULL)
+    return;
+  if ((user == NULL) || (user != GetPrimaryUser()) ||
+      (!user->is_profile_created()))
+    return;
+
+  // In case of Multi Profile mode we don't apply profile locale because it is
+  // unsafe.
+  if (GetLoggedInUsers().size() != 1)
+    return;
+
+  if (profile != ProfileManager::GetDefaultProfile())
+    return;
+  const PrefService* prefs = profile->GetPrefs();
+  if (prefs == NULL)
+    return;
+
+  std::string pref_locale;
+  const std::string pref_app_locale =
+      prefs->GetString(prefs::kApplicationLocale);
+  const std::string pref_bkup_locale =
+      prefs->GetString(prefs::kApplicationLocaleBackup);
+
+  pref_locale = pref_app_locale;
+  if (pref_locale.empty())
+    pref_locale = pref_bkup_locale;
+
+  const std::string* account_locale = NULL;
+  if (pref_locale.empty() && user->has_gaia_account()) {
+    if (user->GetAccountLocale() == NULL)
+      return;  // wait until Account profile is loaded.
+    account_locale = user->GetAccountLocale();
+    pref_locale = *account_locale;
+  }
+  const std::string global_app_locale =
+      g_browser_process->GetApplicationLocale();
+  if (pref_locale.empty())
+    pref_locale = global_app_locale;
+  DCHECK(!pref_locale.empty());
+  LOG(WARNING) << "RespectLocalePreference: "
+               << "app_locale='" << pref_app_locale << "', "
+               << "bkup_locale='" << pref_bkup_locale << "', "
+               << (account_locale != NULL
+                       ? (std::string("account_locale='") + (*account_locale) +
+                          "'. ")
+                       : (std::string("account_locale - unused. ")))
+               << " Selected '" << pref_locale << "'";
+  profile->ChangeAppLocale(pref_locale, Profile::APP_LOCALE_CHANGED_VIA_LOGIN);
+  // Here we don't enable keyboard layouts. Input methods are set up when
+  // the user first logs in. Then the user may customize the input methods.
+  // Hence changing input methods here, just because the user's UI language
+  // is different from the login screen UI language, is not desirable. Note
+  // that input method preferences are synced, so users can use their
+  // farovite input methods as soon as the preferences are synced.
+  chromeos::LanguageSwitchMenu::SwitchLanguage(pref_locale);
+}
+
+class UserHashMatcher {
+ public:
+  explicit UserHashMatcher(const std::string& h) : username_hash(h) {}
+  bool operator()(const User* user) const {
+    return user->username_hash() == username_hash;
+  }
+
+ private:
+  const std::string& username_hash;
+};
+
+// Returns NULL if user is not created
+User* UserManagerImpl::GetUserByProfile(Profile* profile) const {
+  if (CommandLine::ForCurrentProcess()->HasSwitch(::switches::kMultiProfiles))
{
+    const std::string username_hash =
+        ProfileHelper::GetUserIdHashFromProfile(profile);
+    const UserList& users = GetUsers();
+    const UserList::const_iterator pos = std::find_if(
+        users.begin(), users.end(), UserHashMatcher(username_hash));
+    return (pos != users.end()) ? *pos : NULL;
+  }
+  return active_user_;
+}
+
+Profile* UserManagerImpl::GetProfileByUser(const User* user) const {
+  if (CommandLine::ForCurrentProcess()->HasSwitch(::switches::kMultiProfiles))
+    return ProfileHelper::GetProfileByUserIdHash(user->username_hash());
+  return g_browser_process->profile_manager()->GetDefaultProfile();
+}
+
 void UserManagerImpl::Observe(int type,
                               const content::NotificationSource& source,
                               const content::NotificationDetails& details) {
@@ -792,6 +911,16 @@ void UserManagerImpl::Observe(int type,
         }
       }
       break;
+    case chrome::NOTIFICATION_PROFILE_CREATED: {
+      Profile* profile = content::Source<Profile>(source).ptr();
+      User* user = GetUserByProfile(profile);
+      if (user != NULL) {
+        user->set_profile_is_created();
+        if (user == active_user_)
+          RespectLocalePreference(profile, user);
+      }
+      break;
+    }
     case chrome::NOTIFICATION_SYSTEM_SETTING_CHANGED: {
       std::string changed_setting =
           *content::Details<const std::string>(details).ptr();

[commit-bot: I haz the power, 2013-09-20 08:01:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alemate@chromium.org/23095006/43001

[commit-bot: I haz the power, 2013-09-20 10:38:27]

Retried try job too often on win7_aura for step(s) interactive_ui_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win7_aura&...

[commit-bot: I haz the power, 2013-09-20 10:55:53]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alemate@chromium.org/23095006/43001

[commit-bot: I haz the power, 2013-09-20 15:15:16]

Retried try job too often on win_rel for step(s) sync_integration_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-09-20 17:18:47]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alemate@chromium.org/23095006/43001

[commit-bot: I haz the power, 2013-09-20 21:57:53]

Retried try job too often on win_rel for step(s) sync_integration_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-09-23 12:37:46]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alemate@chromium.org/23095006/167001

[commit-bot: I haz the power, 2013-09-23 17:45:16]

Retried try job too often on win_rel for step(s) sync_integration_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-09-23 18:08:17]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alemate@chromium.org/23095006/167001

[commit-bot: I haz the power, 2013-09-23 22:26:12]

Retried try job too often on win_rel for step(s) sync_integration_tests
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_rel&nu...

[commit-bot: I haz the power, 2013-09-23 23:28:43]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/alemate@chromium.org/23095006/167001

[commit-bot: I haz the power, 2013-09-24 02:49:42]

Change committed as 224862