[Payments] Add the "Cards accepted" row at the top of CC editor.

[Payments] Add the "Cards accepted" row at the top of CC editor.

No actual validation done using this data.

BUG=687601
TEST=PaymentRequest*CreditCard* interactive_ui_tests

Review-Url: https://codereview.chromium.org/2698353002
Cr-Commit-Position: refs/heads/master@{#451343}
Committed: https://chromium.googlesource.com/chromium/src/+/c2d07f96730f454ad72b9c35ddb98f93240b843c

[Mathieu, 2017-02-16 22:01:18]

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

[Mathieu, 2017-02-16 22:01:19]

Hi, PTAL! Thanks

[Mathieu, 2017-02-16 22:01:28]

Description was changed from

==========
[Payments] Add the "Cards accepted" row at the topic of CC editor.

No actual validation done using this data.

BUG=687601
TEST=PaymentRequest*CreditCard* interactive_ui_tests
==========

to

==========
[Payments] Add the "Cards accepted" row at the top of CC editor.

No actual validation done using this data.

BUG=687601
TEST=PaymentRequest*CreditCard* interactive_ui_tests
==========

[Mathieu, 2017-02-16 22:01:39]

Patchset #1 (id:1) has been deleted

[please use gerrit instead, 2017-02-16 22:16:12]

w00t w00t!

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

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:93:
views::View* CreditCardEditorViewController::CreateHeaderView() {
std::unique_ptr<views::View>

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:94:
views::View* view = new views::View();
Ditto

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:98:
views::BoxLayout* layout = new views::BoxLayout(
Would be nice to use std::unique_ptr<views::BoxLayout>, but I'm getting in the
weeds here and I hear that Views memory management can be non-standard.

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:107:
views::Label* label = new views::Label(
uniqe_ptr...

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:116:
views::View* icons_row = new views::View();
unique_ptr?

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:117:
views::BoxLayout* icons_layout = new views::BoxLayout(
unique_ptr!

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:124:
views::ImageView* card_icon_view = new views::ImageView();
.

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

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/editor_view_controller.cc:64:
content_view->AddChildView(CreateHeaderView());
CreateHeaderView()->release(), when you start using uniqe_ptr.

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/editor_view_controller.cc:69:
content_view->AddChildView(CreateInputField(field).release());
I thought {} were always required for the "for" loops?

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/editor_view_controller.h (right):

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/editor_view_controller.h:86: virtual
views::View* CreateHeaderView() = 0;
unique_ptr

https://codereview.chromium.org/2698353002/diff/20001/components/payments/pay...
File components/payments/payment_request.h (right):

https://codereview.chromium.org/2698353002/diff/20001/components/payments/pay...
components/payments/payment_request.h:108: const std::set<std::string>&
supported_card_networks() {
Let's use a vector, because order carries meaning to merchants. For example,
AmEx might pay to the merchant to show AmEx icon first. Chrome on Android
currently preserves the merchant's order when displaying the header in the card
editor. This order can also be used as a signal when sorting credit cards, but
we haven't done that yet, as that's lower priority, given that frecency does a
great job so far.

[Mathieu, 2017-02-17 15:01:38]

Patchset #2 (id:40001) has been deleted

[Mathieu, 2017-02-17 15:02:04]

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

[commit-bot: I haz the power, 2017-02-17 15:02:18]

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

[Mathieu, 2017-02-17 15:06:41]

Patchset #2 (id:60001) has been deleted

[Mathieu, 2017-02-17 15:06:47]

PTAL!

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

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:93:
views::View* CreditCardEditorViewController::CreateHeaderView() {
On 2017/02/16 22:16:07, rouslan wrote:
> std::unique_ptr<views::View>

Done.

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:94:
views::View* view = new views::View();
On 2017/02/16 22:16:06, rouslan wrote:
> Ditto

Done.

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:98:
views::BoxLayout* layout = new views::BoxLayout(
On 2017/02/16 22:16:07, rouslan wrote:
> Would be nice to use std::unique_ptr<views::BoxLayout>, but I'm getting in the
> weeds here and I hear that Views memory management can be non-standard.

I've kept "new" for the layouts, to follow convention in other files. I'll check
with Anthony when he's back.

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:107:
views::Label* label = new views::Label(
On 2017/02/16 22:16:07, rouslan wrote:
> uniqe_ptr...

Done.

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:116:
views::View* icons_row = new views::View();
On 2017/02/16 22:16:08, rouslan wrote:
> unique_ptr?

Done.

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:117:
views::BoxLayout* icons_layout = new views::BoxLayout(
On 2017/02/16 22:16:08, rouslan wrote:
> unique_ptr!

Done.

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/credit_card_editor_view_controller.cc:124:
views::ImageView* card_icon_view = new views::ImageView();
On 2017/02/16 22:16:07, rouslan wrote:
> .

Done.

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

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/editor_view_controller.cc:64:
content_view->AddChildView(CreateHeaderView());
On 2017/02/16 22:16:09, rouslan wrote:
> CreateHeaderView()->release(), when you start using uniqe_ptr.

Done.

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/editor_view_controller.cc:69:
content_view->AddChildView(CreateInputField(field).release());
On 2017/02/16 22:16:09, rouslan wrote:
> I thought {} were always required for the "for" loops?

Sure

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
File chrome/browser/ui/views/payments/editor_view_controller.h (right):

https://codereview.chromium.org/2698353002/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/editor_view_controller.h:86: virtual
views::View* CreateHeaderView() = 0;
On 2017/02/16 22:16:10, rouslan wrote:
> unique_ptr

Done.

https://codereview.chromium.org/2698353002/diff/20001/components/payments/pay...
File components/payments/payment_request.h (right):

https://codereview.chromium.org/2698353002/diff/20001/components/payments/pay...
components/payments/payment_request.h:108: const std::set<std::string>&
supported_card_networks() {
On 2017/02/16 22:16:11, rouslan wrote:
> Let's use a vector, because order carries meaning to merchants. For example,
> AmEx might pay to the merchant to show AmEx icon first. Chrome on Android
> currently preserves the merchant's order when displaying the header in the
card
> editor. This order can also be used as a signal when sorting credit cards, but
> we haven't done that yet, as that's lower priority, given that frecency does a
> great job so far.

Done

[Mathieu, 2017-02-17 15:06:51]

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

[commit-bot: I haz the power, 2017-02-17 15:07:10]

Dry run: 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-02-17 15:49:04]

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

[commit-bot: I haz the power, 2017-02-17 15:49:04]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2017-02-17 15:52:35]

lgtm % comment

https://codereview.chromium.org/2698353002/diff/80001/components/payments/pay...
File components/payments/payment_request.h (right):

https://codereview.chromium.org/2698353002/diff/80001/components/payments/pay...
components/payments/payment_request.h:100: void PopulateValidatedMethodData(
Should be private.

[Mathieu, 2017-02-17 15:57:03]

Thanks, submitting

https://codereview.chromium.org/2698353002/diff/80001/components/payments/pay...
File components/payments/payment_request.h (right):

https://codereview.chromium.org/2698353002/diff/80001/components/payments/pay...
components/payments/payment_request.h:100: void PopulateValidatedMethodData(
On 2017/02/17 15:52:35, rouslan wrote:
> Should be private.

Done.

[Mathieu, 2017-02-17 15:57:10]

The CQ bit was checked by mathp@chromium.org

[Mathieu, 2017-02-17 15:57:10]

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/2698353002/#ps100001
(title: "private")

[commit-bot: I haz the power, 2017-02-17 15:57:26]

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-02-17 17:24:18]

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

[commit-bot: I haz the power, 2017-02-17 17:24:20]

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

[Mathieu, 2017-02-17 17:47:10]

The CQ bit was checked by mathp@chromium.org

[commit-bot: I haz the power, 2017-02-17 17:47:51]

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-02-17 18:33:55]

CQ is committing da patch.

Bot data: {"patchset_id": 100001, "attempt_start_ts": 1487353630070410,
"parent_rev": "a2191ee363dcb554d237ee14faf21db825f146a5", "commit_rev":
"c2d07f96730f454ad72b9c35ddb98f93240b843c"}

[commit-bot: I haz the power, 2017-02-17 18:34:29]

Description was changed from

==========
[Payments] Add the "Cards accepted" row at the top of CC editor.

No actual validation done using this data.

BUG=687601
TEST=PaymentRequest*CreditCard* interactive_ui_tests
==========

to

==========
[Payments] Add the "Cards accepted" row at the top of CC editor.

No actual validation done using this data.

BUG=687601
TEST=PaymentRequest*CreditCard* interactive_ui_tests

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

[commit-bot: I haz the power, 2017-02-17 18:34:31]

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