PaymentRequest.canMakeActivePayment()

[rsolomakhin]

Hello TAG!

I'm requesting a TAG review of:

• Name: PaymentRequest.canMakeActivePayment()
• Explainer: https://github.com/zkoch/zkoch.github.io/blob/master/pr-detect-avail.md
• Example code: http://rsolomakhin.github.io/pr/pr.js

    var request = new PaymentRequest(supportedPaymentMethods, shoppingCartContents);
    if (request.canMakeActivePayment) {
      request.canMakeActivePayment().then(result => {
        console.log(result ? "Can make active payment" : "Cannot make active payment");
      }).catch(err => {
        console.log(err);
      });
    }

• Primary contacts: @rsolomakhin and @zkoch

Further details:

• Relevant time constraints or deadlines: Would like to ship patch in Chrome M56.

You should also know that...

[please tell us anything you think is relevant to this review]

We'd prefer the TAG provide feedback as (please select one):

• open issues in our Github repo for each point of feedback
• open a single issue in our Github repo for the entire review
• leave review feedback as a comment in this issue and @-notify @rsolomakhin and @zkoch

[sideshowbarker]

Relevant time constraints or deadlines: Would like to ship patch in Chrome M56.

Chrome 56 is targeted for stable non‐beta release around mid-January, right?

If so, by what date at the latest would you expect to have received review by the TAG and resolved any issues with the API that TAG might bring up during the review?

[rsolomakhin]

Before December would be ideal.

[dbaron]

@rsolomakhin Do you have a link to the specification as well?

Also, I'm a little curious about the naming of canMakeActivePayment. What does the word “active” mean in this context? (It makes more sense to me in the context of the canMakePaymentsWithActiveCard method that you refer to in the explainer.)

[dbaron]

Ah, following the link through the blink-dev thread, it looks like the spec text is under development at w3c/payment-request#316 and may land in the Payment Request API specification soon.

[rsolomakhin]

The word "active" has also been discussed in web payments WG github. Were you able to find it?

[dbaron]

Ah, I found the discussion of "active" in w3c/payment-request#310 . (Though I see substantial concern about the name there as well.)

I'm also curious how well this aligns with naming of other examples in the Web platform of functions returning promises that resolve to a boolean result. The ones I found (from searching Gecko's WebIDL) are has and delete on CacheStorage, remove and removeDeep on Directory, load on MediaKeySession, unsubscribe on PushSubscription, unregister on ServiceWorkerRegistration, and persisted and persist on StorageManager. Of those, I think has is the only one that sounds like it returns a boolean. (I guess the naming convention for functions returning promises should match the naming convention of synchronous functions, but it just feels weird in this case. Maybe I'm just not used to it yet.)

[dbaron]

OK, so a few somewhat blunt comments here:

First, it's a bit unusual to ask for TAG review of something that doesn't have a specification. That said, it's not something I would want to discourage, since a group might have a particular architectural issue they want feedback on that seems likely to be important or controversial. However, although I'm an outsider to the Blink project, it does seem (to my understanding of it) somewhat contrary to the intent of Blink's launch process's rationale for requiring asking for TAG review; it says, for example:

In practice, we strive to ensure that the features we ship by default have open standards.

and it feels to me that part of the purpose of asking for TAG review is not only to request review that the API fits with the way Web APIs are designed, but also that it fits with the way that they're specified, so that they have a good path towards being implemented interoperably by multiple implementations.

Second, it feels in other ways that you're trying to cram this through without listening to feedback. The first and second comments by others in w3c/payment-request#310 and my initial reaction here were all saying that the name doesn't make sense, and you don't seem to have responded to that concern other than to explain why other proposals are also misleading. However, it does look like the method has been renamed in response to a different concern, although the explainer hasn't been updated, and, to be honest, it's not clear how much of the explainer even still applies following that change. (That is, does the API still meet the rationale for adding it, after that change?)

---

Further comments, looking at the still unmerged pull request (although I don't know if it's possible to link to the current state of that pull request, or if the current state will even be preserved in the future to make these comments make sense):

In order to prevent the page from probing different payment methods supported by user, canMakePayment
can only be called once per top-level domain. Multiple calls to canMakePayment will result in
rejection of the promise with QuotaExceededError. To reduce privacy risks, implementations MAY limit calls
to canMakePayment for a certain period of time before allowing top-level origin to call
canMakePayment again. However canMakePayment can be called multiple times with same
set of supportedMethods per top-level origin.

This text is quite unclear; it doesn't clearly explain how the passing of time and calls from different TLDs or origins interact to lead to QuotaExceededError. It seems like it should be a clearer English summary of what the algorithm below it leads to.

5. Store canMakePaymentPromise in request@[[\canMakePaymentPromise]].

It's not clear what purpose this serves, since this property doesn't appear to be read from, and since multiple calls to canMakePayment() will overwrite this value.

(within point 7) Let cachedSupportedMethods be supportedMethods sequences from each PaymentMethodData from previous call to PaymentRequest. Let cachedResponse be response from previous call to canMakePayment. Let canMakePaymentQuotaReached be boolean value of previous call to canMakePayment for the topLevelOrigin.

This seems oddly less precise than the rest of the algorithm, which does things by storing variables in places. Is it the previous call made by the same origin? Which step in this algorithm does that call need to have reached for it to count? Is previous defined by the order of call or the order of promise resolution?

9. If cachedSupportedMethods is preset and matches supportMethods, then resolve canMakePaymentPromise with cachedResponse. Else, set cachedSupportedMethods to supportedMethods.

It seems like the "Else" part of this should come after step 10. Otherwise if you have three calls in succession, with the first having one set of methods, and the second and third having another, then the first call will return a real result, the second call will be rejected with QuotaExceededError, and the third call will return the cached response from the first call (which has different methods, but which were incorrectly stored in the second call).

(This is one of the disadvantages of writing specs so algorithmically: it's harder to detect bugs in the specifications.)

[triblondon]

Picked up at Boston F2F.

Seems that the current thinking on this is that the method should compute the intersection of the payment methods accepted by the website and the methods offered by the user, and return true if the result is non-empty set. However, to defend against fishing expeditions, there needs to be some constraint on how many times this method can be called with different params. @slightlyoff determined that in Chrome, the result is cached for 30 mins for an origin, so if you make a second invocation with different params you will get the previously cached result (which may be - somewhat intentionally - wrong for the new invocation)

Resolved that @dbaron would review the spec and we'll follow up on next week's call.

[dbaron]

Current spec is https://w3c.github.io/browser-payment-api/#canmakepayment-method

[triblondon]

At Tokyo F2F we agreed that we're happy with this as it stands.