[Payments] Format and validate phone number according to selected country in address editor

Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

This CL changes the behavior of formatting and validating phone number
in address editor if the input number is not starts with '+'.

Previously, the phone number is formatted and validated according
to region code deduced from the application locale. 
This causes below issue as described in the bug:
  (1) Switch phone to French (Canada)
  (2) Try to enter a canadian address in the Payment Request editor 
      (android)
  (3) Phone formatting requires +33 which is in France

After this patch, the phone number is formatted and validated
according to the region code deduced from the selected country code
in the address editor. This solves above problem.

BUG=723769
Review-Url: https://codereview.chromium.org/2952673002
Cr-Commit-Position: refs/heads/master@{#481193}
Committed: https://chromium.googlesource.com/chromium/src/+/bcfccfe7b5b9b3acdda8f64d1dea9614e2d8f39a

[gogerald1, 2017-06-20 18:02:04]

Description was changed from

==========
Format and validator phone number according to selected country in address
editor

BUG=
==========

to

==========
Format and validator phone number according to selected country in address
editor

This CL is based on https://codereview.chromium.org/2924513002/

BUG=723769
==========

[gogerald1, 2017-06-20 18:02:40]

The CQ bit was checked by gogerald@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2017-06-20 18:03:05]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[gogerald1, 2017-06-20 18:29:47]

Patchset #1 (id:1) has been deleted

[gogerald1, 2017-06-20 18:31:59]

gogerald@chromium.org changed reviewers:
+ rouslan@chromium.org

[gogerald1, 2017-06-20 18:32:00]

Hi rouslan@, ptal,

[please use gerrit instead, 2017-06-20 18:36:55]

Before I start reviewing, please update the CL description to include the
following information:

1) What happens before the patch and why it's bad.

2) What does this patch do and how does that solve the problem?

[please use gerrit instead, 2017-06-20 18:37:11]

Description was changed from

==========
Format and validator phone number according to selected country in address
editor

This CL is based on https://codereview.chromium.org/2924513002/

BUG=723769
==========

to

==========
Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

BUG=723769
==========

[gogerald1, 2017-06-20 18:42:14]

Description was changed from

==========
Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

BUG=723769
==========

to

==========
Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

This patch changes the behavior of formatting and validating phone number in
address editor if the input number is not starts with '+'.

Previously, the phone number is formatted and validated according to region code
deduced from the application locale.

After this patch, the phone number is formatted and validated according to the
region code deduced from the selected country code in the address editor.

BUG=723769
==========

[gogerald1, 2017-06-20 18:42:46]

Description was changed from

==========
Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

This patch changes the behavior of formatting and validating phone number in
address editor if the input number is not starts with '+'.

Previously, the phone number is formatted and validated according to region code
deduced from the application locale.

After this patch, the phone number is formatted and validated according to the
region code deduced from the selected country code in the address editor.

BUG=723769
==========

to

==========
Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

This patch changes the behavior of formatting and validating phone number
in address editor if the input number is not starts with '+'.

Previously, the phone number is formatted and validated according
to region code deduced from the application locale.

After this patch, the phone number is formatted and validated
according to the region code deduced from the selected country code
in the address editor.

BUG=723769
==========

[gogerald1, 2017-06-20 18:43:08]

Description was changed from

==========
Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

This patch changes the behavior of formatting and validating phone number
in address editor if the input number is not starts with '+'.

Previously, the phone number is formatted and validated according
to region code deduced from the application locale.

After this patch, the phone number is formatted and validated
according to the region code deduced from the selected country code
in the address editor.

BUG=723769
==========

to

==========
Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

This CL changes the behavior of formatting and validating phone number
in address editor if the input number is not starts with '+'.

Previously, the phone number is formatted and validated according
to region code deduced from the application locale.

After this patch, the phone number is formatted and validated
according to the region code deduced from the selected country code
in the address editor.

BUG=723769
==========

[gogerald1, 2017-06-20 18:46:29]

Description was changed from

==========
Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

This CL changes the behavior of formatting and validating phone number
in address editor if the input number is not starts with '+'.

Previously, the phone number is formatted and validated according
to region code deduced from the application locale.

After this patch, the phone number is formatted and validated
according to the region code deduced from the selected country code
in the address editor.

BUG=723769
==========

to

==========
Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

This CL changes the behavior of formatting and validating phone number
in address editor if the input number is not starts with '+'.

Previously, the phone number is formatted and validated according
to region code deduced from the application locale. 
This exposes below issue as described in the bug:
  (1) Switch phone to French (Canada)
  (2) Try to enter a canadian address in the Payment Request editor 
      (android)
  (3) Phone formatting requires +33 which is in France

After this patch, the phone number is formatted and validated
according to the region code deduced from the selected country code
in the address editor.

BUG=723769
==========

[gogerald1, 2017-06-20 18:46:50]

Description was changed from

==========
Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

This CL changes the behavior of formatting and validating phone number
in address editor if the input number is not starts with '+'.

Previously, the phone number is formatted and validated according
to region code deduced from the application locale. 
This exposes below issue as described in the bug:
  (1) Switch phone to French (Canada)
  (2) Try to enter a canadian address in the Payment Request editor 
      (android)
  (3) Phone formatting requires +33 which is in France

After this patch, the phone number is formatted and validated
according to the region code deduced from the selected country code
in the address editor.

BUG=723769
==========

to

==========
Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

This CL changes the behavior of formatting and validating phone number
in address editor if the input number is not starts with '+'.

Previously, the phone number is formatted and validated according
to region code deduced from the application locale. 
This causes below issue as described in the bug:
  (1) Switch phone to French (Canada)
  (2) Try to enter a canadian address in the Payment Request editor 
      (android)
  (3) Phone formatting requires +33 which is in France

After this patch, the phone number is formatted and validated
according to the region code deduced from the selected country code
in the address editor.

BUG=723769
==========

[please use gerrit instead, 2017-06-20 18:48:29]

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java
(right):

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java:62:
mPhoneValidator = new PhoneNumberValidator();
Both validator and phone number are "country-aware," but the reader of this line
would not get this idea. Please make the names of these classes consistent.

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java
(right):

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:488:
mContext.getString(R.string.autofill_credit_card_editor_number), null, null,
Multiple nulls in a row are getting confused. Please add comments about what
these nulls are.

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:513:
null, mContext.getString(R.string.payments_field_required_validation_message),
Ditto.

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ContactEditor.java
(right):

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ContactEditor.java:155:
mPayerNames, null, null, null,
Ditto

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFreeShippingTest.java
(right):

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFreeShippingTest.java:138:
"Alice", "Supreme Court", "Airport Road", "Kabul", "1043", "020-253-0000"},
Why these changes?

https://codereview.chromium.org/2952673002/diff/20001/chrome/browser/autofill...
File chrome/browser/autofill/android/phone_number_util_android.cc (right):

https://codereview.chromium.org/2952673002/diff/20001/chrome/browser/autofill...
chrome/browser/autofill/android/phone_number_util_android.cc:81: env,
FormatPhoneNumber(
The "jcountry_code.is_null() ? " check should go between "env," and "Format.."

[gogerald1, 2017-06-20 18:50:38]

Description was changed from

==========
Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

This CL changes the behavior of formatting and validating phone number
in address editor if the input number is not starts with '+'.

Previously, the phone number is formatted and validated according
to region code deduced from the application locale. 
This causes below issue as described in the bug:
  (1) Switch phone to French (Canada)
  (2) Try to enter a canadian address in the Payment Request editor 
      (android)
  (3) Phone formatting requires +33 which is in France

After this patch, the phone number is formatted and validated
according to the region code deduced from the selected country code
in the address editor.

BUG=723769
==========

to

==========
Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

This CL changes the behavior of formatting and validating phone number
in address editor if the input number is not starts with '+'.

Previously, the phone number is formatted and validated according
to region code deduced from the application locale. 
This causes below issue as described in the bug:
  (1) Switch phone to French (Canada)
  (2) Try to enter a canadian address in the Payment Request editor 
      (android)
  (3) Phone formatting requires +33 which is in France

After this patch, the phone number is formatted and validated
according to the region code deduced from the selected country code
in the address editor. This solves above problem.

BUG=723769
==========

[gogerald1, 2017-06-20 19:12:27]

The CQ bit was checked by gogerald@chromium.org to run a CQ dry run

[gogerald1, 2017-06-20 19:12:40]

another look?

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java
(right):

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/AddressEditor.java:62:
mPhoneValidator = new PhoneNumberValidator();
On 2017/06/20 18:48:29, rouslan wrote:
> Both validator and phone number are "country-aware," but the reader of this
line
> would not get this idea. Please make the names of these classes consistent.

Done.

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java
(right):

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:488:
mContext.getString(R.string.autofill_credit_card_editor_number), null, null,
On 2017/06/20 18:48:29, rouslan wrote:
> Multiple nulls in a row are getting confused. Please add comments about what
> these nulls are.

Done.

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/CardEditor.java:513:
null, mContext.getString(R.string.payments_field_required_validation_message),
On 2017/06/20 18:48:29, rouslan wrote:
> Ditto.

Done.

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ContactEditor.java
(right):

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ContactEditor.java:155:
mPayerNames, null, null, null,
On 2017/06/20 18:48:29, rouslan wrote:
> Ditto

Done.

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/javatest...
File
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFreeShippingTest.java
(right):

https://codereview.chromium.org/2952673002/diff/20001/chrome/android/javatest...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestFreeShippingTest.java:138:
"Alice", "Supreme Court", "Airport Road", "Kabul", "1043", "020-253-0000"},
On 2017/06/20 18:48:29, rouslan wrote:
> Why these changes?

They are invalid numbers in those countries, according to
https://codereview.chromium.org/2924513002/

https://codereview.chromium.org/2952673002/diff/20001/chrome/browser/autofill...
File chrome/browser/autofill/android/phone_number_util_android.cc (right):

https://codereview.chromium.org/2952673002/diff/20001/chrome/browser/autofill...
chrome/browser/autofill/android/phone_number_util_android.cc:81: env,
FormatPhoneNumber(
On 2017/06/20 18:48:29, rouslan wrote:
> The "jcountry_code.is_null() ? " check should go between "env," and "Format.."

Done.

[commit-bot: I haz the power, 2017-06-20 19:12:57]

Dry run: CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[please use gerrit instead, 2017-06-20 19:48:03]

lgtm

[gogerald1, 2017-06-20 20:25:06]

Thanks, sending to CQ, note that phone number is still formatted and validated
according to locale in contact editor, might need to optimize it in the future.

[gogerald1, 2017-06-20 20:25:11]

The CQ bit was checked by gogerald@chromium.org

[gogerald1, 2017-06-20 20:25:12]

The patchset sent to the CQ was uploaded after l-g-t-m from rouslan@chromium.org
Link to the patchset: https://codereview.chromium.org/2952673002/#ps60001
(title: "Fix tests")

[commit-bot: I haz the power, 2017-06-20 20:25:46]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-20 20:41:40]

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

[commit-bot: I haz the power, 2017-06-20 20:41:41]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[gogerald1, 2017-06-20 20:45:33]

gogerald@chromium.org changed reviewers:
+ sebsg@chromium.org

[gogerald1, 2017-06-20 20:45:33]

Hi sebsg@, ptal of the changes in
chrome/browser/autofill/android/phone_number_util_android.cc

[sebsg, 2017-06-21 13:46:27]

lgtm

[sebsg, 2017-06-21 13:46:49]

The CQ bit was checked by sebsg@chromium.org

[commit-bot: I haz the power, 2017-06-21 13:47:05]

CQ is trying da patch.

Follow status at:
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-06-21 13:51:43]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1498052809988970,
"parent_rev": "f3fef0310ea8695b0f89be171f5c4d612a54bf22", "commit_rev":
"bcfccfe7b5b9b3acdda8f64d1dea9614e2d8f39a"}

[commit-bot: I haz the power, 2017-06-21 13:51:55]

Description was changed from

==========
Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

This CL changes the behavior of formatting and validating phone number
in address editor if the input number is not starts with '+'.

Previously, the phone number is formatted and validated according
to region code deduced from the application locale. 
This causes below issue as described in the bug:
  (1) Switch phone to French (Canada)
  (2) Try to enter a canadian address in the Payment Request editor 
      (android)
  (3) Phone formatting requires +33 which is in France

After this patch, the phone number is formatted and validated
according to the region code deduced from the selected country code
in the address editor. This solves above problem.

BUG=723769
==========

to

==========
Format and validate phone number according to selected country in address editor

This CL is based on https://codereview.chromium.org/2924513002/

This CL changes the behavior of formatting and validating phone number
in address editor if the input number is not starts with '+'.

Previously, the phone number is formatted and validated according
to region code deduced from the application locale. 
This causes below issue as described in the bug:
  (1) Switch phone to French (Canada)
  (2) Try to enter a canadian address in the Payment Request editor 
      (android)
  (3) Phone formatting requires +33 which is in France

After this patch, the phone number is formatted and validated
according to the region code deduced from the selected country code
in the address editor. This solves above problem.

BUG=723769

Review-Url: https://codereview.chromium.org/2952673002
Cr-Commit-Position: refs/heads/master@{#481193}
Committed:
https://chromium.googlesource.com/chromium/src/+/bcfccfe7b5b9b3acdda8f64d1dea...
==========

[commit-bot: I haz the power, 2017-06-21 13:51:57]

Committed patchset #3 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/bcfccfe7b5b9b3acdda8f64d1dea...