[Payments] In shipping address editor, enable required + phone validation.

[Payments] In shipping address editor, enable required + phone validation.

Should be tested as part of upcoming browser_tests.

New function in validation.h is not unit-tested because it would be
a replica of phone_number_i18n_unittest.cc

BUG=703754
TEST=None, yet.

Review-Url: https://codereview.chromium.org/2773383002
Cr-Commit-Position: refs/heads/master@{#459816}
Committed: https://chromium.googlesource.com/chromium/src/+/8e102df2b584bdd38c62ee31f35c0a4eccff5691

[Mathieu, 2017-03-27 00:07:17]

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

[Mathieu, 2017-03-27 00:07:21]

Patchset #1 (id:1) has been deleted

[commit-bot: I haz the power, 2017-03-27 00:07:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[Mathieu, 2017-03-27 00:07:37]

mathp@chromium.org changed reviewers:
+ anthonyvd@chromium.org

[Mathieu, 2017-03-27 00:07:38]

PTAL

[Mathieu, 2017-03-27 00:07:49]

Description was changed from

==========
[Payments] In shipping address editor, enable required + phone validation.

Should be tested as part of upcoming browser_tests.

New function in validation.h is not unit-tested because it would be
a replica of phone_number_i18n_unittest.cc

BUG=703754
TEST=None, yet.
==========

to

==========
[Payments] In shipping address editor, enable required + phone validation.

Should be tested as part of upcoming browser_tests.

New function in validation.h is not unit-tested because it would be
a replica of phone_number_i18n_unittest.cc

BUG=703754
TEST=None, yet.
==========

[Mathieu, 2017-03-27 00:07:57]

On 2017/03/27 00:07:38, Mathieu wrote:
> PTAL

[commit-bot: I haz the power, 2017-03-27 02:10:00]

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

[commit-bot: I haz the power, 2017-03-27 02:10:01]

Dry run: This issue passed the CQ dry run.

[MAD, 2017-03-27 13:29:13]

mad@chromium.org changed reviewers:
+ mad@chromium.org

[MAD, 2017-03-27 13:29:14]

Just a small suggestion.

BYE
MAD

https://codereview.chromium.org/2773383002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc
(right):

https://codereview.chromium.org/2773383002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc:327:
bool is_required_valid = !field_.required;
Why not simply return true here when !field.required?

[Mathieu, 2017-03-27 13:39:04]

Thanks!

https://codereview.chromium.org/2773383002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc
(right):

https://codereview.chromium.org/2773383002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/shipping_address_editor_view_controller.cc:327:
bool is_required_valid = !field_.required;
On 2017/03/27 13:29:13, MAD wrote:
> Why not simply return true here when !field.required?

I need to "clear" the suggestion if it's valid. Thanks for the question, it made
me realize that I was missing such a call on line 323.

[anthonyvd, 2017-03-27 14:05:08]

lgtm % nit

https://codereview.chromium.org/2773383002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/shipping_address_editor_view_controller.h
(right):

https://codereview.chromium.org/2773383002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/shipping_address_editor_view_controller.h:61:
ShippingAddressEditorViewController* controller_;
nit: comment about why the raw pointer is OK?

[Mathieu, 2017-03-27 16:36:11]

Thanks!

https://codereview.chromium.org/2773383002/diff/40001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/shipping_address_editor_view_controller.h
(right):

https://codereview.chromium.org/2773383002/diff/40001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/shipping_address_editor_view_controller.h:61:
ShippingAddressEditorViewController* controller_;
On 2017/03/27 14:05:08, anthonyvd wrote:
> nit: comment about why the raw pointer is OK?

Done.

[Mathieu, 2017-03-27 16:36:15]

The CQ bit was checked by mathp@chromium.org

[Mathieu, 2017-03-27 16:36:24]

The CQ bit was checked by mathp@chromium.org

[Mathieu, 2017-03-27 16:36:24]

The patchset sent to the CQ was uploaded after l-g-t-m from
anthonyvd@chromium.org
Link to the patchset: https://codereview.chromium.org/2773383002/#ps60001
(title: "addressed comment")

[commit-bot: I haz the power, 2017-03-27 16:36: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-03-27 17:34:49]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1490632584198390,
"parent_rev": "dfd6192f78ab47e5a3b5fe92a0415a81fec0f2a4", "commit_rev":
"8e102df2b584bdd38c62ee31f35c0a4eccff5691"}

[commit-bot: I haz the power, 2017-03-27 17:35:35]

Description was changed from

==========
[Payments] In shipping address editor, enable required + phone validation.

Should be tested as part of upcoming browser_tests.

New function in validation.h is not unit-tested because it would be
a replica of phone_number_i18n_unittest.cc

BUG=703754
TEST=None, yet.
==========

to

==========
[Payments] In shipping address editor, enable required + phone validation.

Should be tested as part of upcoming browser_tests.

New function in validation.h is not unit-tested because it would be
a replica of phone_number_i18n_unittest.cc

BUG=703754
TEST=None, yet.

Review-Url: https://codereview.chromium.org/2773383002
Cr-Commit-Position: refs/heads/master@{#459816}
Committed:
https://chromium.googlesource.com/chromium/src/+/8e102df2b584bdd38c62ee31f35c...
==========

[commit-bot: I haz the power, 2017-03-27 17:35:36]

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