Determine language code and type of format for address.

Determine language code and type of format for address.

BuildComponents() in address_ui.h should take the UI language and
determine the following two questions.

(1) Whether to show UI components based on local or latinized address
    format.

If the UI language does not match the default language for the country,
and latinized address format is available, then use the latinized
format. All countries with a latinized address format also have the
default language specified.

(2) Which language code should be used to format the address after the
    user provides it.

If the UI language is one of the languages supported by the country,
then use the UI language. If the country lists supported languages, none
of which match the UI language, then pick the first supported language.
If the country does not have supported languages (e.g. Antarctica), then
use the UI language for formatting. If using the latinized format and
the UI language is not one of the supported languages, then the
formatting language is [country's default language] + "-latn".

BUG=354954

[please use gerrit instead, 2014-03-21 19:06:36]

Evan: PTAL Patch Set 1.

[Evan Stade, 2014-03-21 23:44:53]

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
File third_party/libaddressinput/chromium/cpp/src/address_data.cc (right):

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/address_data.cc:70: std::string
norm_rule_lang = NormalizeLanguageCode(rule.GetLanguage());
nit: I would delete the norm_ part of each of these var names

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
File third_party/libaddressinput/chromium/cpp/src/address_ui.cc (right):

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/address_ui.cc:81: const
std::string& norm_default_lang,
ditto

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
File third_party/libaddressinput/chromium/cpp/src/normalize_language_code.h
(right):

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/normalize_language_code.h:26:
std::string NormalizeLanguageCode(const std::string& language_code);
I don't think we should add a new file for every utility function

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
File third_party/libaddressinput/chromium/cpp/test/address_ui_test.cc (right):

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/test/address_ui_test.cc:157:
INSTANTIATE_TEST_CASE_P(
you know I disdain test_case_p

[please use gerrit instead, 2014-03-22 01:34:29]

Evan: PTAL Patch Set 2 and questions below.

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
File third_party/libaddressinput/chromium/cpp/src/address_data.cc (right):

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/address_data.cc:70: std::string
norm_rule_lang = NormalizeLanguageCode(rule.GetLanguage());
On 2014/03/21 23:44:54, Evan Stade wrote:
> nit: I would delete the norm_ part of each of these var names

Done.

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
File third_party/libaddressinput/chromium/cpp/src/address_ui.cc (right):

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/address_ui.cc:81: const
std::string& norm_default_lang,
On 2014/03/21 23:44:54, Evan Stade wrote:
> ditto

How would I distinguish normalized UI language from the actual UI language?

I search for normalized UI language, but return the actual UI language. For
example, if the region supports "zh", but the UI language is "zh-hans", then I
return "zh-hans" here.

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
File third_party/libaddressinput/chromium/cpp/src/normalize_language_code.h
(right):

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/normalize_language_code.h:26:
std::string NormalizeLanguageCode(const std::string& language_code);
On 2014/03/21 23:44:54, Evan Stade wrote:
> I don't think we should add a new file for every utility function

This function is shared between address_data.cc and address_ui.cc. Where do you
propose we keep it?

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
File third_party/libaddressinput/chromium/cpp/test/address_ui_test.cc (right):

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/test/address_ui_test.cc:157:
INSTANTIATE_TEST_CASE_P(
On 2014/03/21 23:44:54, Evan Stade wrote:
> you know I disdain test_case_p

Replaced with an array of structs.

[Evan Stade, 2014-03-24 15:46:06]

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
File third_party/libaddressinput/chromium/cpp/src/normalize_language_code.h
(right):

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/normalize_language_code.h:26:
std::string NormalizeLanguageCode(const std::string& language_code);
On 2014/03/22 01:34:30, Rouslan Solomakhin wrote:
> On 2014/03/21 23:44:54, Evan Stade wrote:
> > I don't think we should add a new file for every utility function
> 
> This function is shared between address_data.cc and address_ui.cc. Where do
you
> propose we keep it?

in a util file

[please use gerrit instead, 2014-03-24 18:20:58]

Evan: PTAL Patch Set 3.

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
File third_party/libaddressinput/chromium/cpp/src/normalize_language_code.h
(right):

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/normalize_language_code.h:26:
std::string NormalizeLanguageCode(const std::string& language_code);
On 2014/03/24 15:46:07, Evan Stade wrote:
> On 2014/03/22 01:34:30, Rouslan Solomakhin wrote:
> > On 2014/03/21 23:44:54, Evan Stade wrote:
> > > I don't think we should add a new file for every utility function
> > 
> > This function is shared between address_data.cc and address_ui.cc. Where do
> you
> > propose we keep it?
> 
> in a util file

Done.

[Evan Stade, 2014-03-24 19:04:15]

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
File third_party/libaddressinput/chromium/cpp/src/address_ui.cc (right):

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/address_ui.cc:81: const
std::string& norm_default_lang,
On 2014/03/22 01:34:30, Rouslan Solomakhin wrote:
> On 2014/03/21 23:44:54, Evan Stade wrote:
> > ditto
> 
> How would I distinguish normalized UI language from the actual UI language?
> 
> I search for normalized UI language, but return the actual UI language. For
> example, if the region supports "zh", but the UI language is "zh-hans", then I
> return "zh-hans" here.

maybe it's confusing because there's too many parameters. Why do you need to
pass "zh-hans" as well as "zh"? That just opens the door for problematic
situations where, e.g., they don't match.

https://codereview.chromium.org/208243005/diff/100001/third_party/libaddressi...
File third_party/libaddressinput/chromium/cpp/src/util.h (right):

https://codereview.chromium.org/208243005/diff/100001/third_party/libaddressi...
third_party/libaddressinput/chromium/cpp/src/util.h:30: std::string
NormalizeLanguageCode(const std::string& language_code);
string_split, string_compare, time_to_string, canonicalize_string should
probably all go in here. Also, calling it string_util or something is a bit
better than just "util", and why isn't it in the util/ directory?

[please use gerrit instead, 2014-03-24 22:56:28]

Evan: PTAL Patch Set 4.

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
File third_party/libaddressinput/chromium/cpp/src/address_ui.cc (right):

https://codereview.chromium.org/208243005/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/address_ui.cc:81: const
std::string& norm_default_lang,
On 2014/03/24 19:04:15, Evan Stade wrote:
> On 2014/03/22 01:34:30, Rouslan Solomakhin wrote:
> > On 2014/03/21 23:44:54, Evan Stade wrote:
> > > ditto
> > 
> > How would I distinguish normalized UI language from the actual UI language?
> > 
> > I search for normalized UI language, but return the actual UI language. For
> > example, if the region supports "zh", but the UI language is "zh-hans", then
I
> > return "zh-hans" here.
> 
> maybe it's confusing because there's too many parameters. Why do you need to
> pass "zh-hans" as well as "zh"? That just opens the door for problematic
> situations where, e.g., they don't match.

Made the function less confusing by passing in only |rule| and
|ui_language_code|.

https://codereview.chromium.org/208243005/diff/100001/third_party/libaddressi...
File third_party/libaddressinput/chromium/cpp/src/util.h (right):

https://codereview.chromium.org/208243005/diff/100001/third_party/libaddressi...
third_party/libaddressinput/chromium/cpp/src/util.h:30: std::string
NormalizeLanguageCode(const std::string& language_code);
On 2014/03/24 19:04:15, Evan Stade wrote:
> string_split, string_compare, time_to_string, canonicalize_string should
> probably all go in here. Also, calling it string_util or something is a bit
> better than just "util", and why isn't it in the util/ directory?

Done, except for canonicalize_string, because that's a class, not a function.

[Evan Stade, 2014-03-24 23:00:33]

lgtm

[please use gerrit instead, 2014-03-24 23:01:42]

The CQ bit was checked by rouslan@chromium.org

[commit-bot: I haz the power, 2014-03-24 23:03:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rouslan@chromium.org/208243005/190001

[commit-bot: I haz the power, 2014-03-24 23:19:52]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-24 23:19:52]

Try jobs failed on following builders:
  tryserver.chromium on mac_chromium_rel

[please use gerrit instead, 2014-03-24 23:23:03]

The CQ bit was checked by rouslan@chromium.org

[commit-bot: I haz the power, 2014-03-24 23:23:07]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rouslan@chromium.org/208243005/190001

[commit-bot: I haz the power, 2014-03-25 00:31:20]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-25 00:31:21]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_rel

[please use gerrit instead, 2014-03-25 01:48:13]

The CQ bit was checked by rouslan@chromium.org

[commit-bot: I haz the power, 2014-03-25 01:51:55]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rouslan@chromium.org/208243005/210001

[commit-bot: I haz the power, 2014-03-25 04:48:02]

Change committed as 259150

[earthdok, 2014-03-25 14:44:15]

On 2014/03/25 04:48:02, I haz the power (commit-bot) wrote:
> Change committed as 259150

This broke compile on one of our bots. Bug filed on the libaddressinput tracker:
https://code.google.com/p/libaddressinput/issues/detail?id=5

Rouslan, could you please somehow fix this ASAP? (I.e. either roll the fix into
Chromium or revert this change until then.) Thanks.

[erikchen2, 2014-03-25 17:10:57]

A revert of this CL has been created in
https://codereview.chromium.org/211443003/ by erikchen@google.com.

The reason for reverting is: compile error
https://code.google.com/p/libaddressinput/issues/detail?id=5.

[please use gerrit instead, 2014-03-25 17:57:15]

Patch Set 6 contains the fix for the compile error. Relanding.

[please use gerrit instead, 2014-03-25 17:57:33]

The CQ bit was checked by rouslan@chromium.org

[commit-bot: I haz the power, 2014-03-25 17:58:11]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/rouslan@chromium.org/208243005/370001

[commit-bot: I haz the power, 2014-03-25 19:58:24]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-25 19:58:25]

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

[Evan Stade, 2014-03-25 20:01:00]

Common practice is to not reuse the same codereview issue for a re-commit.
Instead upload to a new issue (first the old patch, then the new one), linking
to this original review.

[please use gerrit instead, 2014-03-25 20:54:07]

On 2014/03/25 20:01:00, Evan Stade wrote:
> Common practice is to not reuse the same codereview issue for a re-commit.
> Instead upload to a new issue (first the old patch, then the new one), linking
> to this original review.

Thanks for the top.

[please use gerrit instead, 2014-03-25 20:55:39]

On 2014/03/25 20:54:07, Rouslan Solomakhin wrote:
> On 2014/03/25 20:01:00, Evan Stade wrote:
> > Common practice is to not reuse the same codereview issue for a re-commit.
> > Instead upload to a new issue (first the old patch, then the new one),
linking
> > to this original review.
> 
> Thanks for the top.

*tip*

[please use gerrit instead, 2014-03-25 21:05:23]

Relanding with the fix in https://codereview.chromium.org/198923010/.