rAc: update android test

retry r226004: rAc: phone number cleanup

original review: https://codereview.chromium.org/24538008/
difference to original review: android test updated.

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=226415

[Evan Stade, 2013-09-30 23:01:42]

https://codereview.chromium.org/25092011/diff/23001/chrome/android/javatests/...
File
chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java
(right):

https://codereview.chromium.org/25092011/diff/23001/chrome/android/javatests/...
chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java:29:
private static final String TEST_PHONE_UNFORMATTED = "4151234567";
As you can see, I tried (and failed) to update the android tests.

The desired behavior is that when filling a phone number into the page, there is
no formatting. See description of original bug for desired display behavior (it
depends on the data source, etc.).

[Ilya Sherman, 2013-09-30 23:29:56]

LGTM for the parts I already reviewed in the previous CL.  Seems like the
remaining changes are Android-only, but let me know if there's anything else
that I should take a look at.

[aruslan, 2013-09-30 23:56:05]

https://codereview.chromium.org/25092011/diff/23001/chrome/android/javatests/...
File
chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java
(right):

https://codereview.chromium.org/25092011/diff/23001/chrome/android/javatests/...
chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java:29:
private static final String TEST_PHONE_UNFORMATTED = "4151234567";
On 2013/09/30 23:01:42, Evan Stade wrote:
> As you can see, I tried (and failed) to update the android tests.
> 
> The desired behavior is that when filling a phone number into the page, there
is
> no formatting. See description of original bug for desired display behavior
(it
> depends on the data source, etc.).

Looking into this.

[aruslan, 2013-10-01 01:51:17]

I'm not convinced that posting the "plain" 4151234567 or 4800298227127 phone
numbers to the web page is a good idea, especially given that the user sees them
us (415)123-4567 and +48 00 (298) 22-71-27.

Note that on Android both Wallet and Autofill data comes in the form of the
wallet_address.h, so I'm tempted to say that the comment in wallet_address is
slightly incorrect.

If it's what you guys have in the spec and that matches your expectations, then
this looks good to me (modulo correct phone number in the test).

https://codereview.chromium.org/25092011/diff/23001/chrome/android/javatests/...
File
chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java
(right):

https://codereview.chromium.org/25092011/diff/23001/chrome/android/javatests/...
chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java:28:
private static final String TEST_PHONE = "(415)123-4567";
I'm sure you'd love it.
It was returning "unformatted" (aka, user-formatted) phone number because
(415)123-4567 is indeed invalid phone number.

Fix: "(415)413-0703" from http://www.fictionaltelecom.com/

https://codereview.chromium.org/25092011/diff/23001/chrome/android/javatests/...
chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java:29:
private static final String TEST_PHONE_UNFORMATTED = "4151234567";
"4154130703"

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

https://codereview.chromium.org/25092011/diff/23001/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/data_model_wrapper.cc:267: return GetInfo(type);
unreachable GetInfo.

https://codereview.chromium.org/25092011/diff/23001/components/autofill/conte...
File components/autofill/content/browser/wallet/wallet_address.cc (right):

https://codereview.chromium.org/25092011/diff/23001/components/autofill/conte...
components/autofill/content/browser/wallet/wallet_address.cc:133:
phone_object_(phone_number_, country_name_code_),
We couldn't have any issues with the field declaration order, right? Otherwise,
phone_object_(phone_number, country_name_code) looks safer.

[aruslan, 2013-10-01 01:52:05]

Looks good to me come on regex LGTM.

[Evan Stade, 2013-10-01 02:00:35]

On 2013/10/01 01:51:17, aruslan wrote:
> I'm not convinced that posting the "plain" 4151234567 or 4800298227127 phone
> numbers to the web page is a good idea, especially given that the user sees
them
> us (415)123-4567 and +48 00 (298) 22-71-27.

We are matching the behavior that Autofill has always had.

> 
> Note that on Android both Wallet and Autofill data comes in the form of the
> wallet_address.h, so I'm tempted to say that the comment in wallet_address is
> slightly incorrect.
> 
> If it's what you guys have in the spec and that matches your expectations,
then
> this looks good to me (modulo correct phone number in the test).

The spec does not touch on data formats: see
https://www.w3.org/Bugs/Public/show_bug.cgi?id=22286

https://codereview.chromium.org/25092011/diff/23001/chrome/android/javatests/...
File
chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java
(right):

https://codereview.chromium.org/25092011/diff/23001/chrome/android/javatests/...
chrome/android/javatests/src/org/chromium/chrome/browser/autofill/AutofillDialogControllerTest.java:28:
private static final String TEST_PHONE = "(415)123-4567";
On 2013/10/01 01:51:17, aruslan wrote:
> I'm sure you'd love it.
> It was returning "unformatted" (aka, user-formatted) phone number because
> (415)123-4567 is indeed invalid phone number.
> 
> Fix: "(415)413-0703" from http://www.fictionaltelecom.com/

>:(

thanks for figuring it out! Funnily enough, I had that problem in my own unit
tests, but that was last week so I forgot it could be an issue.

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

https://codereview.chromium.org/25092011/diff/23001/chrome/browser/ui/autofil...
chrome/browser/ui/autofill/data_model_wrapper.cc:267: return GetInfo(type);
On 2013/10/01 01:51:17, aruslan wrote:
> unreachable GetInfo.

Done.

https://codereview.chromium.org/25092011/diff/23001/components/autofill/conte...
File components/autofill/content/browser/wallet/wallet_address.cc (right):

https://codereview.chromium.org/25092011/diff/23001/components/autofill/conte...
components/autofill/content/browser/wallet/wallet_address.cc:133:
phone_object_(phone_number_, country_name_code_),
On 2013/10/01 01:51:17, aruslan wrote:
> We couldn't have any issues with the field declaration order, right?
Otherwise,
> phone_object_(phone_number, country_name_code) looks safer.

I did this to match L108, and yes it's safe.

[commit-bot: I haz the power, 2013-10-01 18:08:30]

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

[commit-bot: I haz the power, 2013-10-01 18:24:44]

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

[commit-bot: I haz the power, 2013-10-01 19:02:11]

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

[commit-bot: I haz the power, 2013-10-01 20:01:45]

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

[commit-bot: I haz the power, 2013-10-01 21:34:18]

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

[commit-bot: I haz the power, 2013-10-01 22:34:30]

Sorry for I got bad news for ya.
Compile failed with a clobber build on win_x64_rel.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=win_x64_re...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-10-02 03:28:36]

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

[commit-bot: I haz the power, 2013-10-02 07:21:40]

Change committed as 226415