[Payments] PR: Basic test for the order summary section.

[Payments] PR: Basic test for the order summary section.

Tests the basic structure and navigation to the order summary page.

BUG=679734
TEST=PaymentRequest* interactive_ui_tests

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

[Mathieu, 2017-01-26 19:39:44]

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

[Mathieu, 2017-01-26 19:39:45]

Hi Rouslan and Scott, PTAL

[please use gerrit instead, 2017-01-26 20:44:44]

lgtm % nit

https://codereview.chromium.org/2656823006/diff/1/chrome/browser/ui/views/pay...
File chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h (right):

https://codereview.chromium.org/2656823006/diff/1/chrome/browser/ui/views/pay...
chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h:13: enum
DialogViewID {
Should be an enum class.

"Provide enums as full classes, with no implicit conversion to booleans or
integers. Provide an explicit underlying type for enum classes and regular
enums."

https://chromium-cpp.appspot.com/

https://codereview.chromium.org/2656823006/diff/1/chrome/test/data/payments/n...
File chrome/test/data/payments/no_shipping.js (right):

https://codereview.chromium.org/2656823006/diff/1/chrome/test/data/payments/n...
chrome/test/data/payments/no_shipping.js:37: .catch(function(error) {
print(error); });
Bless your heart, clang-format: you can't format JS at all :-)

[Mathieu, 2017-01-26 22:41:33]

Thanks

https://codereview.chromium.org/2656823006/diff/1/chrome/browser/ui/views/pay...
File chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h (right):

https://codereview.chromium.org/2656823006/diff/1/chrome/browser/ui/views/pay...
chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h:13: enum
DialogViewID {
On 2017/01/26 20:44:43, rouslan wrote:
> Should be an enum class.
> 
> "Provide enums as full classes, with no implicit conversion to booleans or
> integers. Provide an explicit underlying type for enum classes and regular
> enums."
> 
> https://chromium-cpp.appspot.com/

Thanks. "You learn something new..."

https://codereview.chromium.org/2656823006/diff/1/chrome/test/data/payments/n...
File chrome/test/data/payments/no_shipping.js (right):

https://codereview.chromium.org/2656823006/diff/1/chrome/test/data/payments/n...
chrome/test/data/payments/no_shipping.js:37: .catch(function(error) {
print(error); });
On 2017/01/26 20:44:43, rouslan wrote:
> Bless your heart, clang-format: you can't format JS at all :-)

Ah! Not sure if you're serious, but I've reverted it. It does look better the
other way...

[sky, 2017-01-26 23:09:45]

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

https://codereview.chromium.org/2656823006/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h:5: // This
defines an enumeration of IDs that can uniquely identify a view within
I recommend moving this above the enum.

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

https://codereview.chromium.org/2656823006/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_interactive_uitest_base.cc:101:
new DialogEventObserver(DialogEvent::ORDER_SUMMARY_OPENED));
MakeUnique.

https://codereview.chromium.org/2656823006/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_interactive_uitest_base.cc:132:
new TestChromePaymentRequestDelegate(web_contents, this /* observer */,
MakeUnique.

https://codereview.chromium.org/2656823006/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_interactive_uitest_base.cc:143:
delegate_->dialog_view()->GetViewByID(static_cast<ViewID>(view_id));
ViewID->int

https://codereview.chromium.org/2656823006/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_interactive_uitest_base.cc:148:
content::RunMessageLoop();
Is there a reason you're not using a RunLoop and supplying the quit closure from
it to the MoveMouseToCenterAndPress?

https://codereview.chromium.org/2656823006/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_interactive_uitest_base.cc:153:
views::View* view = dialog_view()->GetViewByID(static_cast<ViewID>(view_id));
ViewID->int

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

https://codereview.chromium.org/2656823006/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_interactive_uitest_base.h:132:
TestChromePaymentRequestDelegate* delegate_;
Initialize to nullptr.

[Mathieu, 2017-01-27 00:36:42]

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

[Mathieu, 2017-01-27 00:36:50]

Thanks Scott, PTAL

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

https://codereview.chromium.org/2656823006/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_dialog_view_ids.h:5: // This
defines an enumeration of IDs that can uniquely identify a view within
On 2017/01/26 23:09:45, sky wrote:
> I recommend moving this above the enum.

Done.

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

https://codereview.chromium.org/2656823006/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_interactive_uitest_base.cc:101:
new DialogEventObserver(DialogEvent::ORDER_SUMMARY_OPENED));
On 2017/01/26 23:09:45, sky wrote:
> MakeUnique.

Acknowledged.

https://codereview.chromium.org/2656823006/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_interactive_uitest_base.cc:132:
new TestChromePaymentRequestDelegate(web_contents, this /* observer */,
On 2017/01/26 23:09:45, sky wrote:
> MakeUnique.

Acknowledged.

https://codereview.chromium.org/2656823006/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_interactive_uitest_base.cc:143:
delegate_->dialog_view()->GetViewByID(static_cast<ViewID>(view_id));
On 2017/01/26 23:09:45, sky wrote:
> ViewID->int

Done.

https://codereview.chromium.org/2656823006/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_interactive_uitest_base.cc:148:
content::RunMessageLoop();
On 2017/01/26 23:09:45, sky wrote:
> Is there a reason you're not using a RunLoop and supplying the quit closure
from
> it to the MoveMouseToCenterAndPress?

That works too!

https://codereview.chromium.org/2656823006/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_interactive_uitest_base.cc:153:
views::View* view = dialog_view()->GetViewByID(static_cast<ViewID>(view_id));
On 2017/01/26 23:09:45, sky wrote:
> ViewID->int

Done.

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

https://codereview.chromium.org/2656823006/diff/20001/chrome/browser/ui/views...
chrome/browser/ui/views/payments/payment_request_interactive_uitest_base.h:132:
TestChromePaymentRequestDelegate* delegate_;
On 2017/01/26 23:09:45, sky wrote:
> Initialize to nullptr.

Done.

[commit-bot: I haz the power, 2017-01-27 00:38:40]

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-01-27 00:54:06]

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

[commit-bot: I haz the power, 2017-01-27 00:54:07]

Dry run: Try jobs failed on following builders:
  linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)

[Mathieu, 2017-01-27 01:21:23]

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

[commit-bot: I haz the power, 2017-01-27 01:22:37]

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-01-27 03:22:29]

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

[commit-bot: I haz the power, 2017-01-27 03:22:30]

Dry run: This issue passed the CQ dry run.

[sky, 2017-01-27 16:21:53]

LGTM

[Mathieu, 2017-01-27 16:22:49]

The CQ bit was checked by mathp@chromium.org

[Mathieu, 2017-01-27 16:22:49]

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/2656823006/#ps60001
(title: "Rebase")

[commit-bot: I haz the power, 2017-01-27 16:23:10]

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-01-27 16:37:17]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1485534169082900,
"parent_rev": "45c1e695513a13455323fbb639a15276d52b5c2d", "commit_rev":
"b90312cf0db250b338a3b56f21a491d665b841bf"}

[commit-bot: I haz the power, 2017-01-27 16:37:52]

Description was changed from

==========
[Payments] PR: Basic test for the order summary section.

Tests the basic structure and navigation to the order summary page.

BUG=679734
TEST=PaymentRequest* interactive_ui_tests
==========

to

==========
[Payments] PR: Basic test for the order summary section.

Tests the basic structure and navigation to the order summary page.

BUG=679734
TEST=PaymentRequest* interactive_ui_tests

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

[commit-bot: I haz the power, 2017-01-27 16:37:53]

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