Android integration tests for PaymentRequest

Android integration tests for PaymentRequest UI

This patch adds several integration tests for PaymentRequest UI on
Android:

- Clicking X should cancel the PaymentRequest.

- Clicking EDIT and then X should cancel the PaymentRequest.

- Clicking EDIT and then CANCEL should cancel the PaymentRequest.

- Clicking PAY should provide the merchant with an autofill credit
  card and (if requested) an autofill profile as a shipping address.

BUG=608223
Committed: https://crrev.com/68aec80e25847bd6fe7763edfff90e44f8557bc2
Cr-Commit-Position: refs/heads/master@{#391304}

[please use gerrit instead, 2016-05-01 22:53:01]

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

[please use gerrit instead, 2016-05-01 22:53:02]

dfalcantara@, ptal.

[please use gerrit instead, 2016-05-02 16:13:49]

jdonnelly@, fyi feel free to use chrome/test/data/android/payments/*_test.html
for your needs on iOS.

[Justin Donnelly, 2016-05-02 21:37:13]

On 2016/05/02 16:13:49, Rouslan wrote:
> jdonnelly@, fyi feel free to use chrome/test/data/android/payments/*_test.html
> for your needs on iOS.

Cool! Thanks for the heads up.

[please use gerrit instead, 2016-05-02 23:19:27]

dfalcantara@: I am rebasing on top of your UI changes.

[please use gerrit instead, 2016-05-02 23:29:36]

Rebase complete in patch 2.

[gone, 2016-05-02 23:39:11]

https://chromiumcodereview.appspot.com/1941623002/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java
(right):

https://chromiumcodereview.appspot.com/1941623002/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:174:
sShowResultDelayMs = SHOW_RESULT_DELAY_MS;
Kind of weird to be storing this value as a static.  Is it possible to pass it
directly to the UI, then have the setShowResultDelayForTest function be
non-static?

https://chromiumcodereview.appspot.com/1941623002/diff/1/chrome/android/javat...
File
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestTestBase.java
(right):

https://chromiumcodereview.appspot.com/1941623002/diff/1/chrome/android/javat...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestTestBase.java:64:
DOMUtils.waitForNonZeroNodeBounds(mWebContentsRef.get(), "buy");
I'm worried about this test being flaky because of how the page loads.  There's
been cases where a flake happened because the test thought the page had locked
all elements into place, then the renderer moved the element elsewhere.  I've
also seen it break when the page was too big for the button being clicked to
appear on screen (think smaller devices).    

The current workaround for the first problem is to add this above your DOMUtils
calls:
assertWaitForPageScaleFactorMatch(1);

I'm not sure how well it really works (I've had bad luck with 1 and have had to
edit my test pages to use 0.5 or 2 or something), but this test might come back
to haunt you without doing something similar.

[please use gerrit instead, 2016-05-03 02:36:25]

Patchset #3 (id:40001) has been deleted

[please use gerrit instead, 2016-05-03 02:36:33]

Patchset #2 (id:20001) has been deleted

[please use gerrit instead, 2016-05-03 02:38:31]

dfalcantara@, ptal patch 2.

I've simplified the UI *ForTest() code, made some static methods into instance
methods, and changed the test web page scale to 1.

https://chromiumcodereview.appspot.com/1941623002/diff/1/chrome/android/java/...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java
(right):

https://chromiumcodereview.appspot.com/1941623002/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:174:
sShowResultDelayMs = SHOW_RESULT_DELAY_MS;
On 2016/05/02 23:39:11, dfalcantara wrote:
> Kind of weird to be storing this value as a static.  Is it possible to pass it
> directly to the UI, then have the setShowResultDelayForTest function be
> non-static?

Done.

https://chromiumcodereview.appspot.com/1941623002/diff/1/chrome/android/javat...
File
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestTestBase.java
(right):

https://chromiumcodereview.appspot.com/1941623002/diff/1/chrome/android/javat...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestTestBase.java:64:
DOMUtils.waitForNonZeroNodeBounds(mWebContentsRef.get(), "buy");
On 2016/05/02 23:39:11, dfalcantara wrote:
> I'm worried about this test being flaky because of how the page loads. 
There's
> been cases where a flake happened because the test thought the page had locked
> all elements into place, then the renderer moved the element elsewhere.  I've
> also seen it break when the page was too big for the button being clicked to
> appear on screen (think smaller devices).    
> 
> The current workaround for the first problem is to add this above your
DOMUtils
> calls:
> assertWaitForPageScaleFactorMatch(1);
> 
> I'm not sure how well it really works (I've had bad luck with 1 and have had
to
> edit my test pages to use 0.5 or 2 or something), but this test might come
back
> to haunt you without doing something similar.

Done.

[please use gerrit instead, 2016-05-03 03:09:26]

Description was changed from

==========
Android integration tests for PaymentRequest

BUG=608223
==========

to

==========
Android integration tests for PaymentRequest UI

This patch adds several integration tests for PaymentRequest UI on
Android:

- Clicking X should close the PaymentRequest UI.

- Clicking EDIT and then X should close the UI.

- Clicking EDIT and then CANCEL should close the UI.

- Clicking PAY will provide to merchant an autofill credit card and (if
  requested) an autofill profile as a shipping address.

BUG=608223
==========

[please use gerrit instead, 2016-05-03 03:10:00]

Description was changed from

==========
Android integration tests for PaymentRequest UI

This patch adds several integration tests for PaymentRequest UI on
Android:

- Clicking X should close the PaymentRequest UI.

- Clicking EDIT and then X should close the UI.

- Clicking EDIT and then CANCEL should close the UI.

- Clicking PAY will provide to merchant an autofill credit card and (if
  requested) an autofill profile as a shipping address.

BUG=608223
==========

to

==========
Android integration tests for PaymentRequest UI

This patch adds several integration tests for PaymentRequest UI on
Android:

- Clicking X should close the PaymentRequest UI.

- Clicking EDIT and then X should close the UI.

- Clicking EDIT and then CANCEL should close the UI.

- Clicking PAY will provide to merchant with an autofill credit card and
  (if requested) an autofill profile as a shipping address.

BUG=608223
==========

[please use gerrit instead, 2016-05-03 03:11:14]

Description was changed from

==========
Android integration tests for PaymentRequest UI

This patch adds several integration tests for PaymentRequest UI on
Android:

- Clicking X should close the PaymentRequest UI.

- Clicking EDIT and then X should close the UI.

- Clicking EDIT and then CANCEL should close the UI.

- Clicking PAY will provide to merchant with an autofill credit card and
  (if requested) an autofill profile as a shipping address.

BUG=608223
==========

to

==========
Android integration tests for PaymentRequest UI

This patch adds several integration tests for PaymentRequest UI on
Android:

- Clicking X should cancel the PaymentRequest.

- Clicking EDIT and then X should cancel the PaymentRequest.

- Clicking EDIT and then CANCEL should cancel the PaymentRequest.

- Clicking PAY will provide to merchant with an autofill credit card and
  (if requested) an autofill profile as a shipping address.

BUG=608223
==========

[gone, 2016-05-03 05:44:19]

Nits.  Not going to block my lgtm.

https://chromiumcodereview.appspot.com/1941623002/diff/60001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskPrompt.java
(right):

https://chromiumcodereview.appspot.com/1941623002/diff/60001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskPrompt.java:72:
private int mShowResultDelayMs = 1000;
I'd move the 1000 out to a javadoc'd private static final int to explain the
rationale behind 1 second, then set this field to that.

https://chromiumcodereview.appspot.com/1941623002/diff/60001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java
(right):

https://chromiumcodereview.appspot.com/1941623002/diff/60001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:132:
private int mShowResultDelayMs = 3000;
Like above, I'd still keep the 3000 value as a private static final int with an
explanation of what it's for, then set this field to that value.

https://chromiumcodereview.appspot.com/1941623002/diff/60001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:155:
String title, String origin) {
Should this and the other one assert that sCurrentUIForTest is null when this
call is made?

https://chromiumcodereview.appspot.com/1941623002/diff/60001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:529:
public void onDismiss(DialogInterface dialog) {
I don't think you need to run mDismissCallback anywhere else: this class is
already set as the dismiss listener, and the other places you call
mDismissCallback.run() are right before mDialog.dismiss() calls.  I think you
might actually be double-firing, here.  Could double-check with logs to make
sure.

[please use gerrit instead, 2016-05-03 17:08:13]

Description was changed from

==========
Android integration tests for PaymentRequest UI

This patch adds several integration tests for PaymentRequest UI on
Android:

- Clicking X should cancel the PaymentRequest.

- Clicking EDIT and then X should cancel the PaymentRequest.

- Clicking EDIT and then CANCEL should cancel the PaymentRequest.

- Clicking PAY will provide to merchant with an autofill credit card and
  (if requested) an autofill profile as a shipping address.

BUG=608223
==========

to

==========
Android integration tests for PaymentRequest UI

This patch adds several integration tests for PaymentRequest UI on
Android:

- Clicking X should cancel the PaymentRequest.

- Clicking EDIT and then X should cancel the PaymentRequest.

- Clicking EDIT and then CANCEL should cancel the PaymentRequest.

- Clicking PAY will provide the merchant with an autofill credit card
  and (if requested) an autofill profile as a shipping address.

BUG=608223
==========

[please use gerrit instead, 2016-05-03 17:08:38]

Description was changed from

==========
Android integration tests for PaymentRequest UI

This patch adds several integration tests for PaymentRequest UI on
Android:

- Clicking X should cancel the PaymentRequest.

- Clicking EDIT and then X should cancel the PaymentRequest.

- Clicking EDIT and then CANCEL should cancel the PaymentRequest.

- Clicking PAY will provide the merchant with an autofill credit card
  and (if requested) an autofill profile as a shipping address.

BUG=608223
==========

to

==========
Android integration tests for PaymentRequest UI

This patch adds several integration tests for PaymentRequest UI on
Android:

- Clicking X should cancel the PaymentRequest.

- Clicking EDIT and then X should cancel the PaymentRequest.

- Clicking EDIT and then CANCEL should cancel the PaymentRequest.

- Clicking PAY should provide the merchant with an autofill credit
  card and (if requested) an autofill profile as a shipping address.

BUG=608223
==========

[please use gerrit instead, 2016-05-03 17:11:55]

The CQ bit was checked by rouslan@chromium.org

[please use gerrit instead, 2016-05-03 17:11:56]

The patchset sent to the CQ was uploaded after l-g-t-m from
dfalcantara@chromium.org
Link to the patchset: https://codereview.chromium.org/1941623002/#ps80001
(title: "Address comments")

[please use gerrit instead, 2016-05-03 17:12:09]

Sending to cq.

https://chromiumcodereview.appspot.com/1941623002/diff/60001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskPrompt.java
(right):

https://chromiumcodereview.appspot.com/1941623002/diff/60001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/autofill/CardUnmaskPrompt.java:72:
private int mShowResultDelayMs = 1000;
On 2016/05/03 05:44:19, dfalcantara wrote:
> I'd move the 1000 out to a javadoc'd private static final int to explain the
> rationale behind 1 second, then set this field to that.

Done.

https://chromiumcodereview.appspot.com/1941623002/diff/60001/chrome/android/j...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java
(right):

https://chromiumcodereview.appspot.com/1941623002/diff/60001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:132:
private int mShowResultDelayMs = 3000;
On 2016/05/03 05:44:19, dfalcantara wrote:
> Like above, I'd still keep the 3000 value as a private static final int with
an
> explanation of what it's for, then set this field to that value.

Done.

https://chromiumcodereview.appspot.com/1941623002/diff/60001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:155:
String title, String origin) {
On 2016/05/03 05:44:19, dfalcantara wrote:
> Should this and the other one assert that sCurrentUIForTest is null when this
> call is made?

Done.

https://chromiumcodereview.appspot.com/1941623002/diff/60001/chrome/android/j...
chrome/android/java/src/org/chromium/chrome/browser/payments/ui/PaymentRequestUI.java:529:
public void onDismiss(DialogInterface dialog) {
On 2016/05/03 05:44:19, dfalcantara wrote:
> I don't think you need to run mDismissCallback anywhere else: this class is
> already set as the dismiss listener, and the other places you call
> mDismissCallback.run() are right before mDialog.dismiss() calls.  I think you
> might actually be double-firing, here.  Could double-check with logs to make
> sure.

Done.

[commit-bot: I haz the power, 2016-05-03 17:12:30]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1941623002/80001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1941623002/80001

[commit-bot: I haz the power, 2016-05-03 18:24:03]

Description was changed from

==========
Android integration tests for PaymentRequest UI

This patch adds several integration tests for PaymentRequest UI on
Android:

- Clicking X should cancel the PaymentRequest.

- Clicking EDIT and then X should cancel the PaymentRequest.

- Clicking EDIT and then CANCEL should cancel the PaymentRequest.

- Clicking PAY should provide the merchant with an autofill credit
  card and (if requested) an autofill profile as a shipping address.

BUG=608223
==========

to

==========
Android integration tests for PaymentRequest UI

This patch adds several integration tests for PaymentRequest UI on
Android:

- Clicking X should cancel the PaymentRequest.

- Clicking EDIT and then X should cancel the PaymentRequest.

- Clicking EDIT and then CANCEL should cancel the PaymentRequest.

- Clicking PAY should provide the merchant with an autofill credit
  card and (if requested) an autofill profile as a shipping address.

BUG=608223
==========

[commit-bot: I haz the power, 2016-05-03 18:24:04]

Committed patchset #3 (id:80001)

[commit-bot: I haz the power, 2016-05-03 18:26:18]

Description was changed from

==========
Android integration tests for PaymentRequest UI

This patch adds several integration tests for PaymentRequest UI on
Android:

- Clicking X should cancel the PaymentRequest.

- Clicking EDIT and then X should cancel the PaymentRequest.

- Clicking EDIT and then CANCEL should cancel the PaymentRequest.

- Clicking PAY should provide the merchant with an autofill credit
  card and (if requested) an autofill profile as a shipping address.

BUG=608223
==========

to

==========
Android integration tests for PaymentRequest UI

This patch adds several integration tests for PaymentRequest UI on
Android:

- Clicking X should cancel the PaymentRequest.

- Clicking EDIT and then X should cancel the PaymentRequest.

- Clicking EDIT and then CANCEL should cancel the PaymentRequest.

- Clicking PAY should provide the merchant with an autofill credit
  card and (if requested) an autofill profile as a shipping address.

BUG=608223

Committed: https://crrev.com/68aec80e25847bd6fe7763edfff90e44f8557bc2
Cr-Commit-Position: refs/heads/master@{#391304}
==========

[commit-bot: I haz the power, 2016-05-03 18:26:19]

Patchset 3 (id:??) landed as
https://crrev.com/68aec80e25847bd6fe7763edfff90e44f8557bc2
Cr-Commit-Position: refs/heads/master@{#391304}

[sclittle, 2016-05-03 22:42:04]

A revert of this CL (patchset #3 id:80001) has been created in
https://codereview.chromium.org/1945643003/ by sclittle@chromium.org.

The reason for reverting is: These integration tests are currently failing on
the bots:
https://build.chromium.org/p/chromium.linux/builders/Android%20GN/builds/34100,
https://build.chromium.org/p/chromium.linux/builders/Android%20GN/builds/34101
.