requestAutocomplete should not unlatinize administrative region names.

requestAutocomplete should not unlatinize administrative region names.

requestAutocomplete canonicalizes administrative region names before
sending an address to Wallet. For example, "California" becomes "CA".
"California" is a "name" and "CA" is a "key". Similar logic cannot be
used for switching from "lname" to "key", however. For example, "Tokyo"
should not become "東京都", because we should avoid mixed-language
addresses. This patch prevents unlatinizing of administrative region
names when canonicalizing an address to be sent to Wallet.

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

[please use gerrit instead, 2014-04-14 20:51:50]

Evan: PTAL Patch Set 1.

[Evan Stade, 2014-04-15 01:12:55]

On 2014/04/14 20:51:50, Rouslan Solomakhin wrote:
> Evan: PTAL Patch Set 1.

Doesn't wallet want/require the canonicalized (non-latin) name for the region? I
feel like storing it in canonicalized format is fine, but filling into the page
should match the address's language.

[Evan Stade, 2014-04-16 01:40:29]

https://codereview.chromium.org/237693002/diff/1/third_party/libaddressinput/...
File third_party/libaddressinput/chromium/cpp/test/address_validator_test.cc
(right):

https://codereview.chromium.org/237693002/diff/1/third_party/libaddressinput/...
third_party/libaddressinput/chromium/cpp/test/address_validator_test.cc:588:
address.administrative_area = "Tokyo";
I feel like we'd still want to normalize somewhat, e.g. TOKYO becomes Tokyo,
Québec becomes Quebec (or vice versa, depending on the language).

[please use gerrit instead, 2014-04-16 21:44:48]

Evan: PTAL Patch Set 2.

https://codereview.chromium.org/237693002/diff/1/third_party/libaddressinput/...
File third_party/libaddressinput/chromium/cpp/test/address_validator_test.cc
(right):

https://codereview.chromium.org/237693002/diff/1/third_party/libaddressinput/...
third_party/libaddressinput/chromium/cpp/test/address_validator_test.cc:588:
address.administrative_area = "Tokyo";
On 2014/04/16 01:40:29, Evan Stade wrote:
> I feel like we'd still want to normalize somewhat, e.g. TOKYO becomes Tokyo,
> Québec becomes Quebec (or vice versa, depending on the language).

Done.

[Evan Stade, 2014-04-16 21:55:46]

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

https://codereview.chromium.org/237693002/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/rule.cc:379: bool
canonicalize_latin_name,
|canonicalize_latin_name| is not quite optimal imo, because in either case
you're doing some degree of canonicalization/normalization. It should be
|keep_input_latin| or |keep_input_script| or something like that.

https://codereview.chromium.org/237693002/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/rule.cc:395: bool
Rule::GetMatchingSubKey(const std::string& target,
seems like GetMatchingSubKey and GetMatchingValue should be able to share an
implementation. The sig would look something like:

GetMatchingValue(string target,
                 vector values_to_compare,
                 vector values_to_pick_from,
                 string* output) {
  ...
}

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

https://codereview.chromium.org/237693002/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/test/address_validator_test.cc:590:
EXPECT_EQ("TOKYO", address.administrative_area);
-_-

Hopefully one day this becomes Tokyo, right?

[please use gerrit instead, 2014-04-16 22:53:26]

Evan: PTAL Patch Set 3.

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

https://codereview.chromium.org/237693002/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/rule.cc:379: bool
canonicalize_latin_name,
On 2014/04/16 21:55:46, Evan Stade wrote:
> |canonicalize_latin_name| is not quite optimal imo, because in either case
> you're doing some degree of canonicalization/normalization. It should be
> |keep_input_latin| or |keep_input_script| or something like that.

Done.

https://codereview.chromium.org/237693002/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/rule.cc:395: bool
Rule::GetMatchingSubKey(const std::string& target,
On 2014/04/16 21:55:46, Evan Stade wrote:
> seems like GetMatchingSubKey and GetMatchingValue should be able to share an
> implementation. The sig would look something like:
> 
> GetMatchingValue(string target,
>                  vector values_to_compare,
>                  vector values_to_pick_from,
>                  string* output) {
>   ...
> }

Done.

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

https://codereview.chromium.org/237693002/diff/40001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/test/address_validator_test.cc:590:
EXPECT_EQ("TOKYO", address.administrative_area);
On 2014/04/16 21:55:46, Evan Stade wrote:
> -_-
> 
> Hopefully one day this becomes Tokyo, right?

lol. i18n team is in process of fixing capitalization in their data.

[Evan Stade, 2014-04-16 23:32:32]

lgtm

https://codereview.chromium.org/237693002/diff/60001/third_party/libaddressin...
File third_party/libaddressinput/chromium/cpp/src/rule.cc (right):

https://codereview.chromium.org/237693002/diff/60001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/rule.cc:207: std::string* sub_key)
{
s/SubKey/Value
s/sub_key/output_value or picked_value or something

[please use gerrit instead, 2014-04-17 00:40:46]

Sending to CQ.

https://codereview.chromium.org/237693002/diff/60001/third_party/libaddressin...
File third_party/libaddressinput/chromium/cpp/src/rule.cc (right):

https://codereview.chromium.org/237693002/diff/60001/third_party/libaddressin...
third_party/libaddressinput/chromium/cpp/src/rule.cc:207: std::string* sub_key)
{
On 2014/04/16 23:32:32, Evan Stade wrote:
> s/SubKey/Value
> s/sub_key/output_value or picked_value or something

Done.

[please use gerrit instead, 2014-04-17 00:40:49]

The CQ bit was checked by rouslan@chromium.org

[commit-bot: I haz the power, 2014-04-17 00:41:14]

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

[commit-bot: I haz the power, 2014-04-17 05:31:28]

Change committed as 264435