NotSupportedError for payment apps without instruments.

NotSupportedError for payment apps without instruments.

If the merchant website requests only a payment app that happens to not
have any payment instruments and no ability to add an instrument from
within PaymentRequest UI, then reject the show() promise with
NotSupportedError.

BUG=628685
Committed: https://crrev.com/036c952c9a98ce9b25d7020fe865727b172d501a
Cr-Commit-Position: refs/heads/master@{#406558}

[please use gerrit instead, 2016-07-15 23:48:03]

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

[please use gerrit instead, 2016-07-15 23:48:03]

dfalcantara@, ptal.

[gone, 2016-07-18 17:46:49]

lgtm

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

https://chromiumcodereview.appspot.com/2159513002/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:239:
mCanAddAutofillPaymentInstruments = app instanceof AutofillPaymentApp;
|app| can't ever be null, right?  I think null passes this check.

https://chromiumcodereview.appspot.com/2159513002/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:856:
if (!mPendingApps.isEmpty()) return;
Can you add comments here about why these early exits process in this order?

https://chromiumcodereview.appspot.com/2159513002/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:869:
if (mPaymentInformationCallback == null) return;
Why'd you switch to an early return instead of doing the != null check?

[please use gerrit instead, 2016-07-19 23:44:26]

Patchset #2 (id:20001) has been deleted

[please use gerrit instead, 2016-07-19 23:44:35]

Patchset #2 (id:40001) has been deleted

[please use gerrit instead, 2016-07-19 23:48:20]

Will send to CQ after http://crrev.com/2149023005 lands.

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

https://chromiumcodereview.appspot.com/2159513002/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:239:
mCanAddAutofillPaymentInstruments = app instanceof AutofillPaymentApp;
On 2016/07/18 17:46:48, dfalcantara wrote:
> |app| can't ever be null, right?  I think null passes this check.

It's never null here, because we call app.getSupportedMethodNames() above. If
it's null, it would've NullPointerException-crashed by now :-)

https://chromiumcodereview.appspot.com/2159513002/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:856:
if (!mPendingApps.isEmpty()) return;
On 2016/07/18 17:46:48, dfalcantara wrote:
> Can you add comments here about why these early exits process in this order?

Done.

https://chromiumcodereview.appspot.com/2159513002/diff/1/chrome/android/java/...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:869:
if (mPaymentInformationCallback == null) return;
On 2016/07/18 17:46:48, dfalcantara wrote:
> Why'd you switch to an early return instead of doing the != null check?

I was trying to make the code more clear by having only early returns. However,
I see that this is much more readable:

if (mPaymentInformationCallback != null) providePaymentInformation();

than this:

if (mPaymentInformationCallback == null) return;
providePaymentInformation();

So I've changed this one back to its original form.

[please use gerrit instead, 2016-07-20 14:08:29]

The CQ bit was checked by rouslan@chromium.org

[please use gerrit instead, 2016-07-20 14:08:30]

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

[commit-bot: I haz the power, 2016-07-20 14:08:50]

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-07-20 15:08:11]

Committed patchset #4 (id:100001)

[commit-bot: I haz the power, 2016-07-20 15:08:15]

CQ bit was unchecked.

[commit-bot: I haz the power, 2016-07-20 15:12:10]

Description was changed from

==========
NotSupportedError for payment apps without instruments.

If the merchant website requests only a payment app that happens to not
have any payment instruments and no ability to add an instrument from
within PaymentRequest UI, then reject the show() promise with
NotSupportedError.

BUG=628685
==========

to

==========
NotSupportedError for payment apps without instruments.

If the merchant website requests only a payment app that happens to not
have any payment instruments and no ability to add an instrument from
within PaymentRequest UI, then reject the show() promise with
NotSupportedError.

BUG=628685

Committed: https://crrev.com/036c952c9a98ce9b25d7020fe865727b172d501a
Cr-Commit-Position: refs/heads/master@{#406558}
==========

[commit-bot: I haz the power, 2016-07-20 15:12:11]

Patchset 4 (id:??) landed as
https://crrev.com/036c952c9a98ce9b25d7020fe865727b172d501a
Cr-Commit-Position: refs/heads/master@{#406558}