rAc: phone number cleanup

rAc: phone number cleanup

1) fill Wallet phone number data in a format that's more similar to Autofill (i.e. whole number without any formatting)
2) display Wallet phone numbers in rAc in US-national format
3) display Autofill phone numbers in rAc in user-supplied format, unless there is no formatting. In that case add formatting.

BUG=297204
R=isherman@chromium.org

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

[Evan Stade, 2013-09-26 21:42:59]

if this lgty I can work on fixing the tests that no doubt will break.

[Ilya Sherman, 2013-09-26 22:01:06]

LGTM.  Thanks, this IMO does a great job of preserving user formatting where it
might matter, and cleaning it up everywhere else.

https://codereview.chromium.org/24538008/diff/25001/chrome/browser/ui/autofil...
File chrome/browser/ui/autofill/data_model_wrapper.cc (right):

https://codereview.chromium.org/24538008/diff/25001/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/data_model_wrapper.cc:143: base::string16
phone_number = values[GetVariantForType(type)];
Optional nit: const-ref?

https://codereview.chromium.org/24538008/diff/25001/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/data_model_wrapper.cc:145: if
(ContainsOnlyChars(phone_number, ASCIIToUTF16("0123456789"))) {
nit: Please add a comment describing what this block is all about.

https://codereview.chromium.org/24538008/diff/25001/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/data_model_wrapper.cc:155: return GetInfo(type);
Optional nit: Perhaps this should call DataModelWrapper::GetInfoForDisplay()
instead, in case that ever evolves to do something more complicated than call
GetInfo()?

https://codereview.chromium.org/24538008/diff/25001/components/autofill/core/...
File components/autofill/core/browser/phone_number_i18n.cc (right):

https://codereview.chromium.org/24538008/diff/25001/components/autofill/core/...
components/autofill/core/browser/phone_number_i18n.cc:267: }
Please still move this above the PhoneObject constructor ;)

Though, it might be cleaner to just add a GetNationalFormattedNumber() method to
the PhoneObject class.

[Evan Stade, 2013-09-27 00:29:17]

test added if you want to take a look at it.

https://codereview.chromium.org/24538008/diff/25001/chrome/browser/ui/autofil...
File chrome/browser/ui/autofill/data_model_wrapper.cc (right):

https://codereview.chromium.org/24538008/diff/25001/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/data_model_wrapper.cc:143: base::string16
phone_number = values[GetVariantForType(type)];
On 2013/09/26 22:01:06, Ilya Sherman wrote:
> Optional nit: const-ref?

Done.

https://codereview.chromium.org/24538008/diff/25001/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/data_model_wrapper.cc:145: if
(ContainsOnlyChars(phone_number, ASCIIToUTF16("0123456789"))) {
On 2013/09/26 22:01:06, Ilya Sherman wrote:
> nit: Please add a comment describing what this block is all about.

Done.

https://codereview.chromium.org/24538008/diff/25001/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/data_model_wrapper.cc:155: return GetInfo(type);
On 2013/09/26 22:01:06, Ilya Sherman wrote:
> Optional nit: Perhaps this should call DataModelWrapper::GetInfoForDisplay()
> instead, in case that ever evolves to do something more complicated than call
> GetInfo()?

Done.

https://codereview.chromium.org/24538008/diff/25001/components/autofill/core/...
File components/autofill/core/browser/phone_number_i18n.cc (right):

https://codereview.chromium.org/24538008/diff/25001/components/autofill/core/...
components/autofill/core/browser/phone_number_i18n.cc:267: }
On 2013/09/26 22:01:06, Ilya Sherman wrote:
> Please still move this above the PhoneObject constructor ;)
> 
> Though, it might be cleaner to just add a GetNationalFormattedNumber() method
to
> the PhoneObject class.

Done.

[Ilya Sherman, 2013-09-27 00:32:06]

Still LGTM, thanks.

https://codereview.chromium.org/24538008/diff/59001/chrome/browser/ui/autofil...
File chrome/browser/ui/autofill/data_model_wrapper.cc (right):

https://codereview.chromium.org/24538008/diff/59001/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/data_model_wrapper.cc:143: const base::string16
phone_number = values[GetVariantForType(type)];
Optional nit: const *ref*?

[Evan Stade, 2013-09-27 00:43:20]

https://codereview.chromium.org/24538008/diff/59001/chrome/browser/ui/autofil...
File chrome/browser/ui/autofill/data_model_wrapper.cc (right):

https://codereview.chromium.org/24538008/diff/59001/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/data_model_wrapper.cc:143: const base::string16
phone_number = values[GetVariantForType(type)];
On 2013/09/27 00:32:06, Ilya Sherman wrote:
> Optional nit: const *ref*?

facepalm. Done.

[commit-bot: I haz the power, 2013-09-27 22:17:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/estade@chromium.org/24538008/97001

[commit-bot: I haz the power, 2013-09-28 03:34:28]

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

[Evan Stade, 2013-09-30 17:38:58]

Committed patchset #9 manually as r226004 (presubmit successful).