The UI language rather than the locale is now used to pick Chrome's language ...

The UI language rather than the locale is now used to pick Chrome's language on Windows.  (Also fixed some unrelated lint errors.)  With this change, l10n_util::GetApplicationLocale first checks for an override of the "configured locale" (in base::i18n) containing the list of preferred Windows UI languages.  The browser triggers an override early in startup before the ApplicationLocale is determined. In effect, the browser no longer uses ICU on Windows for language detection.  (This locale override mechanism is borrowed from the OS X port.)

Changes in Chrome Frame are largely a refactor, as some Win32 code in there has been moved into base/win.

Also cleaned up language selection in installer_util so that the proper language is chosen for the EULA, installer messages, and shortcuts.  In so doing, replaced hand-crafted lists of supported languages with either auto-generated lists (static consts) or logic so that the addition of translations in the future doesn't require code motion (that being said, there may be reason to update the alias and/or wildcard tables in language_selector.cc).  In so doing, this change unlocks Amharic, Farsi, and Swahili translations for installer messages and shortcuts.

BUG=39986, 40496, 26470
TEST=New MUI/Win32 calls are tested in base/win/i18n_unittest.cc.  To test the overall functionality, uninstall Chrome, remove intl.app_locale user pref, switch to a supported display language (via the "Keyboards and Languages" tab of Win7's "Regional and Language" control panel, and install with { "distribution": { "require_eula": true } } in master_preferences (via -installerdata arg to setup.exe).  If all goes well, both EULA and outer frame are in the same language as Windows.  Also, from gwilson: "Install system-level Chrome in audit mode on a new machine, then go through the out-of-box-experience, select a language, and the in -product EULA (triggered by "require_eula" : true) and Chrome's UI should be in the language that the user selected."


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

[grt (UTC plus 2), 2010-11-01 17:25:19]

This change changes language detection on Windows in both Chrome and in
setup.exe to use the Windows UI language rather than the locale (which is used
only for formatting).  This means that we now favor choosing the user's display
language (over their locale) for both UI and formatting.  This is arguably a
better user experience since Chrome's display language will match that of
Windows.

JShin: please review the base and Chrome changes.
RobertShield: please review the Chrome Frame changes.

[grt (UTC plus 2), 2010-11-01 19:49:53]

Please hold off on the review as the change broke L10nUtilTest.GetAppLocale. 
I'm working on a refactor to get it working again, and will followup when it is
ready.

[grt (UTC plus 2), 2010-11-02 18:17:15]

I've updated the change and description.  It now passes the win trybot and is
ready for review.  As before:

JShin: please review the base and Chrome changes.
RobertShield: please review the Chrome Frame changes

Thanks.

[jungshik at Google, 2010-11-02 20:33:53]

Thank you for the CL ! 
Some comments below.

http://codereview.chromium.org/4139010/diff/33002/46001
File app/l10n_util.cc (right):

http://codereview.chromium.org/4139010/diff/33002/46001#newcode375
app/l10n_util.cc:375: std::string ui_language = l10n_util::GetLocaleOverride();
How about doing here what Linux does? Instead of just picking up the top
language (on Vista+ where multiple languages are returned), you can add to
|candidates| the list of UI language names obtained with Windows API after
canonicalizing them with base:i18n:GetCanonicalLocale? 

This is to handle when the user's preferred UI language list starts with a
language not supported by Chrome but contains one of Chrome's UI language later
in the list. For instance,  if the list is {Welsh, English (GB)}, we'd better
pick English (GB) instead of the last resort English(US).

http://codereview.chromium.org/4139010/diff/33002/46002
File app/l10n_util_win.cc (right):

http://codereview.chromium.org/4139010/diff/33002/46002#newcode170
app/l10n_util_win.cc:170: &wide_region) ||
nit: indentation

http://codereview.chromium.org/4139010/diff/33002/46008
File base/win/i18n.cc (right):

http://codereview.chromium.org/4139010/diff/33002/46008#newcode17
base/win/i18n.cc:17: std::wstring::size_type dash_position =
language_id.find(L'-');
Does Windows API always return ll or ll-RR? Does it ever return zh-Hant-HK or
more generic BCP 47 locale identifiers?  

Anyway, I think you can just have GetPreferrredUILanguageList (combining
internally XP and Vista+ cases) and pass the result back to l10n_util to add
them all to |candidates| (see my comment there) and let them handle the rest.

http://codereview.chromium.org/4139010/diff/33002/46009
File base/win/i18n.h (right):

http://codereview.chromium.org/4139010/diff/33002/46009#newcode6
base/win/i18n.h:6: #define BASE_WIN_I18N_H_
How about putting this stuff into base/i18n/i18n_util_win?  If you like that,
I'll move non-rtl stuffs out of base/i18n/rtl and put them into
base/i18n/i18n_util after you're done with this CL.

http://codereview.chromium.org/4139010/diff/33002/46009#newcode21
base/win/i18n.h:21: bool GetUserPreferredUILanguages(std::vector<wchar_t>*
languages);
This and the next function are only different in the last letter. How about
GetUserPrefferedUILanguageList?

http://codereview.chromium.org/4139010/diff/33002/46009#newcode35
base/win/i18n.h:35: bool GetUserDefaultUILanguage(std::wstring* language,
std::wstring* region);
Instead of making 'customers' of these APIs try GetUserPreferredUILanguages and
GetUserDefaultUILanguage in turn, wouldn't it be better to combine these two
into a single function that returns a vector of languages (taking care of XP vs
Vista+ differences internally)? On XP, it'll just return a vector with only one
language. On Vista+, it'll return one or more languages.

http://codereview.chromium.org/4139010/diff/33002/46012
File chrome/installer/util/l10n_string_util.cc (right):

http://codereview.chromium.org/4139010/diff/33002/46012#newcode72
chrome/installer/util/l10n_string_util.cc:72: L"hk" == region_part) {
If Window Vista+ returns zh-Hans and zh-Hant, Hant should be mapped to tw and
Hans should be mapped to cn. 

Hans/Hant are better/correct names, but for various reasons, we still use tw/cn
in Chrome.

[robertshield, 2010-11-04 13:56:49]

LGTM, don't have much to add beyond Jungshik's comments.

http://codereview.chromium.org/4139010/diff/33002/46008
File base/win/i18n.cc (right):

http://codereview.chromium.org/4139010/diff/33002/46008#newcode41
base/win/i18n.cc:41: // Keep this array in sync with enum LanguageFunction.
You should add a compile-time check on the length of the array (see
COMPILE_ASSERT).

[grt (UTC plus 2), 2010-11-08 04:38:55]

Thanks for the comments, Jungshik and Robert.  I've updated the CL to
incorporate all previous comments and include new logic in the installer for
picking the right language for EULA, messages, shortcuts, etc.

http://codereview.chromium.org/4139010/diff/33002/46001
File app/l10n_util.cc (right):

http://codereview.chromium.org/4139010/diff/33002/46001#newcode375
app/l10n_util.cc:375: std::string ui_language = l10n_util::GetLocaleOverride();
On 2010/11/02 20:33:54, Jungshik Shin wrote:
> How about doing here what Linux does? Instead of just picking up the top
> language (on Vista+ where multiple languages are returned), you can add to
> |candidates| the list of UI language names obtained with Windows API after
> canonicalizing them with base:i18n:GetCanonicalLocale? 
> 
> This is to handle when the user's preferred UI language list starts with a
> language not supported by Chrome but contains one of Chrome's UI language
later
> in the list. For instance,  if the list is {Welsh, English (GB)}, we'd better
> pick English (GB) instead of the last resort English(US).  

Excellent suggestion.  Done.

http://codereview.chromium.org/4139010/diff/33002/46002
File app/l10n_util_win.cc (right):

http://codereview.chromium.org/4139010/diff/33002/46002#newcode170
app/l10n_util_win.cc:170: &wide_region) ||
On 2010/11/02 20:33:54, Jungshik Shin wrote:
> nit: indentation

Done via refactor.

http://codereview.chromium.org/4139010/diff/33002/46008
File base/win/i18n.cc (right):

http://codereview.chromium.org/4139010/diff/33002/46008#newcode17
base/win/i18n.cc:17: std::wstring::size_type dash_position =
language_id.find(L'-');
On 2010/11/02 20:33:54, Jungshik Shin wrote:
> Does Windows API always return ll or ll-RR? Does it ever return zh-Hant-HK or
> more generic BCP 47 locale identifiers?  

I'm not sure how far Windows goes.  I've done as you've suggested, so it's not
an issue here.  The new code in installer_util does some splitting, but it
should be general enough to handle whatever Windows throws at it.  Please have a
look at it (installer/util/language_selector.cc).

> Anyway, I think you can just have GetPreferrredUILanguageList (combining
> internally XP and Vista+ cases) and pass the result back to l10n_util to add
> them all to |candidates| (see my comment there) and let them handle the rest.

http://codereview.chromium.org/4139010/diff/33002/46008#newcode41
base/win/i18n.cc:41: // Keep this array in sync with enum LanguageFunction.
On 2010/11/04 13:56:49, robertshield wrote:
> You should add a compile-time check on the length of the array (see
> COMPILE_ASSERT).

Sweet.  Done.

http://codereview.chromium.org/4139010/diff/33002/46009
File base/win/i18n.h (right):

http://codereview.chromium.org/4139010/diff/33002/46009#newcode6
base/win/i18n.h:6: #define BASE_WIN_I18N_H_
On 2010/11/02 20:33:54, Jungshik Shin wrote:
> How about putting this stuff into base/i18n/i18n_util_win?  If you like that,
> I'll move non-rtl stuffs out of base/i18n/rtl and put them into
> base/i18n/i18n_util after you're done with this CL. 

I think that doing so would require adding base_i18n as a prereq to setup.exe. 
Is this acceptable?

http://codereview.chromium.org/4139010/diff/33002/46009#newcode21
base/win/i18n.h:21: bool GetUserPreferredUILanguages(std::vector<wchar_t>*
languages);
On 2010/11/02 20:33:54, Jungshik Shin wrote:
> This and the next function are only different in the last letter. How about
> GetUserPrefferedUILanguageList? 
> 

Done.

http://codereview.chromium.org/4139010/diff/33002/46009#newcode35
base/win/i18n.h:35: bool GetUserDefaultUILanguage(std::wstring* language,
std::wstring* region);
On 2010/11/02 20:33:54, Jungshik Shin wrote:
> Instead of making 'customers' of these APIs try GetUserPreferredUILanguages
and
> GetUserDefaultUILanguage in turn, wouldn't it be better to combine these two
> into a single function that returns a vector of languages (taking care of XP
vs
> Vista+ differences internally)? On XP, it'll just return a vector with only
one
> language. On Vista+, it'll return one or more languages.

Yes, indeed.  I'd left them separated to make it easier to test them
individually.  I've now combined them as you suggest.

http://codereview.chromium.org/4139010/diff/33002/46012
File chrome/installer/util/l10n_string_util.cc (right):

http://codereview.chromium.org/4139010/diff/33002/46012#newcode72
chrome/installer/util/l10n_string_util.cc:72: L"hk" == region_part) {
On 2010/11/02 20:33:54, Jungshik Shin wrote:
> If Window Vista+ returns zh-Hans and zh-Hant, Hant should be mapped to tw and
> Hans should be mapped to cn. 
> 
> Hans/Hant are better/correct names, but for various reasons, we still use
tw/cn
> in Chrome.

Done (see the "exceptions" in installer/util/language_selector.cc).

[grt (UTC plus 2), 2010-11-09 18:10:48]

This change has finally passed a win trybot
(http://build.chromium.org/p/tryserver.chromium/builders/win/builds/1868) and is
ready for re-review.

[robertshield, 2010-11-11 14:42:17]

LFTM (Looks Fantastic To Me).

Nought but very minor nits below.

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
File chrome/installer/util/language_selector_unittest.cc (right):

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
chrome/installer/util/language_selector_unittest.cc:21: #else  //
defined(GOOGLE_CHROME_BUILD)
you don't really need the qualifying comments on the #else here and below (it's
fairly short).

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
chrome/installer/util/language_selector_unittest.cc:33: #endif //
!defined(GOOGLE_CHROME_BUILD)
You don't really need them on the #endif either given how short and not-nested
it is. If you really want to keep them, then it should be defined, not !defined.

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/preb...
File chrome/installer/util/prebuild/create_string_rc.py (right):

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/preb...
chrome/installer/util/prebuild/create_string_rc.py:189: % (lang.replace('_',
'-').lower(), lang))
why are these strings unicode strings, but not the ones below?

[grt (UTC plus 2), 2010-11-11 15:23:56]

Thanks, Robert.  Please see my comments to your comments.

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
File chrome/installer/util/language_selector_unittest.cc (right):

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
chrome/installer/util/language_selector_unittest.cc:33: #endif //
!defined(GOOGLE_CHROME_BUILD)
On 2010/11/11 14:42:18, robertshield wrote:
> You don't really need them on the #endif either given how short and not-nested
> it is. If you really want to keep them, then it should be defined, not
!defined.

My style for these is to make the comment contain the logic for the preceding
code block.  So the stuff above the #else tells you the logic for the #if,
whereas the stuff above the #endif is the inverse.  This way, if you're looking
at a big hunk of code and the #if or #else is offscreen, you know when the code
will be compiled without scrolling around.

For such small code the comments don't add value, but I like the consistency.

Do these contravene the style enough that I should 1) remove the comments from
this CL when the code block is small, and 2) not do my funny logic documentation
anywhere?

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/preb...
File chrome/installer/util/prebuild/create_string_rc.py (right):

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/preb...
chrome/installer/util/prebuild/create_string_rc.py:189: % (lang.replace('_',
'-').lower(), lang))
On 2010/11/11 14:42:18, robertshield wrote:
> why are these strings unicode strings, but not the ones below?

Your guess is as good as mine.  erikwright@chromium.org, who is wise in the ways
of Python, tells me it's okay as is, but odd; possibly the result of multiple
people editing and not keeping track of things, or possibly to handle corner
cases involving Unicode and non-Unicode strings.  All language tags are ASCII,
so all should be well.  Would you like me to make all of the strings Unicode in
this file?

[robertshield, 2010-11-11 16:18:53]

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
File chrome/installer/util/language_selector_unittest.cc (right):

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
chrome/installer/util/language_selector_unittest.cc:33: #endif //
!defined(GOOGLE_CHROME_BUILD)
On 2010/11/11 15:23:57, grt wrote:
> On 2010/11/11 14:42:18, robertshield wrote:
> > You don't really need them on the #endif either given how short and
not-nested
> > it is. If you really want to keep them, then it should be defined, not
> !defined.
> 
> My style for these is to make the comment contain the logic for the preceding
> code block.  So the stuff above the #else tells you the logic for the #if,
> whereas the stuff above the #endif is the inverse.  This way, if you're
looking
> at a big hunk of code and the #if or #else is offscreen, you know when the
code
> will be compiled without scrolling around.
> 
> For such small code the comments don't add value, but I like the consistency.
> 
> Do these contravene the style enough that I should 1) remove the comments from
> this CL when the code block is small, and 2) not do my funny logic
documentation
> anywhere?

The comments are great for longer blocks, IMO they add noise for short blocks.
This is the same as how comments at the end of a long if/else statement might
make sense, but the following would be frowned upon:

if (foo) {
  DoSomething();
} else {
  DoSomethingElse();
}  // foo  <-- no

I say remove them.

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/preb...
File chrome/installer/util/prebuild/create_string_rc.py (right):

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/preb...
chrome/installer/util/prebuild/create_string_rc.py:189: % (lang.replace('_',
'-').lower(), lang))
On 2010/11/11 15:23:57, grt wrote:
> On 2010/11/11 14:42:18, robertshield wrote:
> > why are these strings unicode strings, but not the ones below?
> 
> Your guess is as good as mine.  mailto:erikwright@chromium.org, who is wise in
the ways
> of Python, tells me it's okay as is, but odd; possibly the result of multiple
> people editing and not keeping track of things, or possibly to handle corner
> cases involving Unicode and non-Unicode strings.  All language tags are ASCII,
> so all should be well.  Would you like me to make all of the strings Unicode
in
> this file?

They should be consistent. Inconsistencies like this make people wonder whether
there's a motive behind them. 

If none of the strings need to be unicode, lets make them all ascii. If some
need to be unicode, lets make them all that way.

[grt (UTC plus 2), 2010-11-11 16:29:26]

Thanks for the schoolin'.  I've changed all strings for the header file to be
ASCII (the .rc file requires Unicode, so I've left the strings for it alone).

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
File chrome/installer/util/language_selector_unittest.cc (right):

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
chrome/installer/util/language_selector_unittest.cc:21: #else  //
defined(GOOGLE_CHROME_BUILD)
On 2010/11/11 14:42:18, robertshield wrote:
> you don't really need the qualifying comments on the #else here and below
(it's
> fairly short).

Done.

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
chrome/installer/util/language_selector_unittest.cc:33: #endif //
!defined(GOOGLE_CHROME_BUILD)
On 2010/11/11 16:18:53, robertshield wrote:
> On 2010/11/11 15:23:57, grt wrote:
> > On 2010/11/11 14:42:18, robertshield wrote:
> > > You don't really need them on the #endif either given how short and
> not-nested
> > > it is. If you really want to keep them, then it should be defined, not
> > !defined.
> > 
> > My style for these is to make the comment contain the logic for the
preceding
> > code block.  So the stuff above the #else tells you the logic for the #if,
> > whereas the stuff above the #endif is the inverse.  This way, if you're
> looking
> > at a big hunk of code and the #if or #else is offscreen, you know when the
> code
> > will be compiled without scrolling around.
> > 
> > For such small code the comments don't add value, but I like the
consistency.
> > 
> > Do these contravene the style enough that I should 1) remove the comments
from
> > this CL when the code block is small, and 2) not do my funny logic
> documentation
> > anywhere?
> 
> The comments are great for longer blocks, IMO they add noise for short blocks.
> This is the same as how comments at the end of a long if/else statement might
> make sense, but the following would be frowned upon:
> 
> if (foo) {
>   DoSomething();
> } else {
>   DoSomethingElse();
> }  // foo  <-- no
> 
> I say remove them.

Done.

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/preb...
File chrome/installer/util/prebuild/create_string_rc.py (right):

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/preb...
chrome/installer/util/prebuild/create_string_rc.py:189: % (lang.replace('_',
'-').lower(), lang))
On 2010/11/11 16:18:53, robertshield wrote:
> On 2010/11/11 15:23:57, grt wrote:
> > On 2010/11/11 14:42:18, robertshield wrote:
> > > why are these strings unicode strings, but not the ones below?
> > 
> > Your guess is as good as mine.  mailto:erikwright@chromium.org, who is wise
in
> the ways
> > of Python, tells me it's okay as is, but odd; possibly the result of
multiple
> > people editing and not keeping track of things, or possibly to handle corner
> > cases involving Unicode and non-Unicode strings.  All language tags are
ASCII,
> > so all should be well.  Would you like me to make all of the strings Unicode
> in
> > this file?
> 
> They should be consistent. Inconsistencies like this make people wonder
whether
> there's a motive behind them. 
> 
> If none of the strings need to be unicode, lets make them all ascii. If some
> need to be unicode, lets make them all that way.

Done.

[jungshik at Google, 2010-11-11 20:30:45]

Thank you for all the changes. Very nice ! And sorry for the delay. 
They look great except for a few minor points below. 

BTW, when a user select 'Amharic' (or Swahili) in the Chrome download page,
Omaha will set the UI language to Amharic and Chrome will start in that
language, right?

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/l10n...
File chrome/installer/util/l10n_string_util.cc (right):

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/l10n...
chrome/installer/util/l10n_string_util.cc:53: StringToUpperASCII(&language);
Thanks to this change, the missing entries in the previous version of this file
(like new languages we added after version 1) are supported. Nice !

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
File chrome/installer/util/language_selector.cc (right):

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
chrome/installer/util/language_selector.cc:68: { L"zh-mo", IDS_L10N_OFFSET_ZH_TW
}
Does Windows have zh-SG (Singpore)? If so, zh-sg has to be mapped to ZH_CN.

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
chrome/installer/util/language_selector.cc:83: // Use the Portugal region for
anything Portugese.
I guess it's better to map this to pt_BR (Brazil is  a lot more populous than
Portugal).

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
chrome/installer/util/language_selector.cc:163: std::wstring* script_part,
std::wstring* region_part) {
Have you considered using ICU's Locale class? That's just a thin class for
making it unnecessary to parse locale IDs ourselves. It has methods like
getScript, getLanguage, getCountry.  See 
http://icu-project.org/apiref/icu4c452/classLocale.html

[grt (UTC plus 2), 2010-11-12 04:22:02]

Thanks for the review.  Regarding Amharic and the other languages that had been
left out of l10n_string_util.cc, yes, Chrome itself would pick up the Omaha
setting.  The installer, however, would not use Amharic for any diagnostic
messages, Start menu shortcuts, etc.  Now it will.

Please see my comment regarding pt-BR; I'll wait until I hear from you before
submitting this CL.

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/l10n...
File chrome/installer/util/l10n_string_util.cc (right):

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/l10n...
chrome/installer/util/l10n_string_util.cc:53: StringToUpperASCII(&language);
On 2010/11/11 20:30:45, Jungshik Shin wrote:
> Thanks to this change, the missing entries in the previous version of this
file
> (like new languages we added after version 1) are supported. Nice !

Thanks.

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
File chrome/installer/util/language_selector.cc (right):

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
chrome/installer/util/language_selector.cc:68: { L"zh-mo", IDS_L10N_OFFSET_ZH_TW
}
On 2010/11/11 20:30:45, Jungshik Shin wrote:
> Does Windows have zh-SG (Singpore)? If so, zh-sg has to be mapped to ZH_CN. 

Yes, Windows does.  This is handled by the wildcards list below: zh-* is mapped
to ZH_CN.  Although come to think of it, it's better to have the entry in the
alias list for two reasons: 1) it's not obvious that the wildcard entry will do
the trick, and 2) we want a candidate list of "zh-sg, en-us" to result in zh-cn
(via the alias) rather than en-us (via an exact match).  So I've added it to the
list.  Thanks for prompting me to think this through a bit more.  I've also
added a unit test to make sure that zh-sg will win in this case.

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
chrome/installer/util/language_selector.cc:83: // Use the Portugal region for
anything Portugese.
On 2010/11/11 20:30:45, Jungshik Shin wrote:
> I guess it's better to map this to pt_BR (Brazil is  a lot more populous than
> Portugal). 

The previous behavior was to only use pt-br when it was specifically requested;
see
http://src.chromium.org/viewvc/chrome/trunk/src/chrome/installer/util/l10n_st....
 Would you prefer that I change it?

http://codereview.chromium.org/4139010/diff/125001/chrome/installer/util/lang...
chrome/installer/util/language_selector.cc:163: std::wstring* script_part,
std::wstring* region_part) {
On 2010/11/11 20:30:45, Jungshik Shin wrote:
> Have you considered using ICU's Locale class? That's just a thin class for
> making it unnecessary to parse locale IDs ourselves. It has methods like
> getScript, getLanguage, getCountry.  See 
> http://icu-project.org/apiref/icu4c452/classLocale.html

Thanks for the pointer.  As it happens, I'd have to introduce a dependency on
ICU for installer_util_nacl_win64, so I've chosen not to use the Locale class. 
Instead, I removed this function altogether in favor of a simple
language.substr(0, language.find(L'-')) in MatchLanguageOffset below.

[jungshik at Google, 2010-11-15 23:26:12]

Great ! Thank you for addressing my comments. 

As for pt -> pt-BR, I've had a small cl in my local tree (for l10n_util for
chrome), but haven't checked in. Anyway, I think it's good to have that in the
installer. 

You can land now with that changed.

Thanks again for this nice CL !