[Payment Request] Update the logic when obtaining results from editors

[Payment Request] Update the logic when obtaining results from editors

There were bugs in the handling of the result coming back from an editor. We now have improved logic around:
* Receiving |null| from the editor (cancelling out of an Add flow)
* Receiving an incomplete result (cancelling out of an Edit for an incomplete item)
* Receiving a complete item.

Also updated the tests and added a few new ones.

BUG=673371
TEST=PaymentRequest* (java)

Committed: https://crrev.com/8824e3d6fcef98df0702cf0c0dd6c3845d067ff6
Cr-Commit-Position: refs/heads/master@{#439038}

[Mathieu, 2016-12-15 15:57:41]

Description was changed from

==========
[Payment Request] DRAFT

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

to

==========
[Payment Request] Update the logic when obtaining results from editors

There were bugs in the handling of the result coming back from an editor. We now
have improved logic with more comments.

Also updated the tests and added a few new ones.

BUG=673371
TEST=PaymentRequest* (java)
==========

[Mathieu, 2016-12-15 15:57:41]

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

[Mathieu, 2016-12-15 15:57:56]

Patchset #2 (id:20001) has been deleted

[Mathieu, 2016-12-15 15:58:03]

Patchset #1 (id:1) has been deleted

[Mathieu, 2016-12-15 16:04:55]

Patchset #1 (id:40001) has been deleted

[Mathieu, 2016-12-15 16:05:04]

Patchset #1 (id:60001) has been deleted

[Mathieu, 2016-12-15 16:06:09]

Description was changed from

==========
[Payment Request] Update the logic when obtaining results from editors

There were bugs in the handling of the result coming back from an editor. We now
have improved logic with more comments.

Also updated the tests and added a few new ones.

BUG=673371
TEST=PaymentRequest* (java)
==========

to

==========
[Payment Request] Update the logic when obtaining results from editors

There were bugs in the handling of the result coming back from an editor. We now
have improved logic around:
* Receiving |null| from the editor (cancelling out of an Add flow)
* Receiving an incomplete result (cancelling out of an Edit for an incomplete
item)
* Receiving a complete item.

Also updated the tests and added a few new ones.

BUG=673371
TEST=PaymentRequest* (java)
==========

[Mathieu, 2016-12-15 16:06:12]

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

[commit-bot: I haz the power, 2016-12-15 16:06:36]

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

[Mathieu, 2016-12-15 16:06:59]

Hi Rouslan, 

PTAL. Scheduled a CQ dry run.

[Mathieu, 2016-12-15 16:19:19]

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

[commit-bot: I haz the power, 2016-12-15 16:19:37]

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

[Mathieu, 2016-12-15 16:45:37]

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

[commit-bot: I haz the power, 2016-12-15 16:46:02]

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, 2016-12-15 18:11:21]

lgtm % comments

https://codereview.chromium.org/2571513004/diff/120001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2571513004/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:940:
boolean isAddressComplete = editedAddress.isComplete();
Variable is used only once immediately after it was declared. It's OK to use
editAddress.isComplete() directly in the if statement.

https://codereview.chromium.org/2571513004/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1016:
boolean isCardComplete = editedCard.isComplete();
Inline?

https://codereview.chromium.org/2571513004/diff/120001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestNoShippingTest.java
(right):

https://codereview.chromium.org/2571513004/diff/120001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestNoShippingTest.java:106:
// still being ready to pay.
... with the previously selected credit card.

https://codereview.chromium.org/2571513004/diff/120001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestNoShippingTest.java:372:
private void fillNewCardForm(String cardNumber, String nameOnCard, int month,
int year,
Java style is to place helper methods immediately after where they're used. Can
you put this under testAddTemporaryCardAndPay() on line 155? That's the last
place where this helper was used.

[commit-bot: I haz the power, 2016-12-15 18:26:12]

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

[commit-bot: I haz the power, 2016-12-15 18:26:12]

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, 2016-12-15 22:03:06]

Thanks

https://codereview.chromium.org/2571513004/diff/120001/chrome/android/java/sr...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2571513004/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:940:
boolean isAddressComplete = editedAddress.isComplete();
On 2016/12/15 18:11:20, rouslan wrote:
> Variable is used only once immediately after it was declared. It's OK to use
> editAddress.isComplete() directly in the if statement.

Done.

https://codereview.chromium.org/2571513004/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1016:
boolean isCardComplete = editedCard.isComplete();
On 2016/12/15 18:11:20, rouslan wrote:
> Inline?

Done.

https://codereview.chromium.org/2571513004/diff/120001/chrome/android/javates...
File
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestNoShippingTest.java
(right):

https://codereview.chromium.org/2571513004/diff/120001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestNoShippingTest.java:106:
// still being ready to pay.
On 2016/12/15 18:11:20, rouslan wrote:
> ... with the previously selected credit card.

Done.

https://codereview.chromium.org/2571513004/diff/120001/chrome/android/javates...
chrome/android/javatests/src/org/chromium/chrome/browser/payments/PaymentRequestNoShippingTest.java:372:
private void fillNewCardForm(String cardNumber, String nameOnCard, int month,
int year,
On 2016/12/15 18:11:20, rouslan wrote:
> Java style is to place helper methods immediately after where they're used.
Can
> you put this under testAddTemporaryCardAndPay() on line 155? That's the last
> place where this helper was used.

Done.

[Mathieu, 2016-12-15 22:03:10]

The CQ bit was checked by mathp@chromium.org

[Mathieu, 2016-12-15 22:03: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/2571513004/#ps160001
(title: "fix flakiness")

[commit-bot: I haz the power, 2016-12-15 22:04:22]

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, 2016-12-16 00:11:32]

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

[commit-bot: I haz the power, 2016-12-16 00:11:33]

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, 2016-12-16 01:05:58]

The CQ bit was checked by mathp@chromium.org

[Mathieu, 2016-12-16 01:05:59]

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/2571513004/#ps180001
(title: "addressing test failures")

[commit-bot: I haz the power, 2016-12-16 01:07:06]

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, 2016-12-16 05:41:49]

CQ is committing da patch.

Bot data: {"patchset_id": 180001, "attempt_start_ts": 1481850358895010,
"parent_rev": "532a488d1a0470c6274d136dee788440d24088f7", "commit_rev":
"0bf60094bbaa087dc8480b05be15eb3e23d72a3c"}

[commit-bot: I haz the power, 2016-12-16 05:42:20]

Description was changed from

==========
[Payment Request] Update the logic when obtaining results from editors

There were bugs in the handling of the result coming back from an editor. We now
have improved logic around:
* Receiving |null| from the editor (cancelling out of an Add flow)
* Receiving an incomplete result (cancelling out of an Edit for an incomplete
item)
* Receiving a complete item.

Also updated the tests and added a few new ones.

BUG=673371
TEST=PaymentRequest* (java)
==========

to

==========
[Payment Request] Update the logic when obtaining results from editors

There were bugs in the handling of the result coming back from an editor. We now
have improved logic around:
* Receiving |null| from the editor (cancelling out of an Add flow)
* Receiving an incomplete result (cancelling out of an Edit for an incomplete
item)
* Receiving a complete item.

Also updated the tests and added a few new ones.

BUG=673371
TEST=PaymentRequest* (java)

Review-Url: https://codereview.chromium.org/2571513004
==========

[commit-bot: I haz the power, 2016-12-16 05:42:21]

Committed patchset #6 (id:180001)

[commit-bot: I haz the power, 2016-12-16 05:46:34]

Description was changed from

==========
[Payment Request] Update the logic when obtaining results from editors

There were bugs in the handling of the result coming back from an editor. We now
have improved logic around:
* Receiving |null| from the editor (cancelling out of an Add flow)
* Receiving an incomplete result (cancelling out of an Edit for an incomplete
item)
* Receiving a complete item.

Also updated the tests and added a few new ones.

BUG=673371
TEST=PaymentRequest* (java)

Review-Url: https://codereview.chromium.org/2571513004
==========

to

==========
[Payment Request] Update the logic when obtaining results from editors

There were bugs in the handling of the result coming back from an editor. We now
have improved logic around:
* Receiving |null| from the editor (cancelling out of an Add flow)
* Receiving an incomplete result (cancelling out of an Edit for an incomplete
item)
* Receiving a complete item.

Also updated the tests and added a few new ones.

BUG=673371
TEST=PaymentRequest* (java)

Committed: https://crrev.com/8824e3d6fcef98df0702cf0c0dd6c3845d067ff6
Cr-Commit-Position: refs/heads/master@{#439038}
==========

[commit-bot: I haz the power, 2016-12-16 05:46:35]

Patchset 6 (id:??) landed as
https://crrev.com/8824e3d6fcef98df0702cf0c0dd6c3845d067ff6
Cr-Commit-Position: refs/heads/master@{#439038}