Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(279)

Issue 2530793002: PaymentApp: Allow multiple payment method names for one instrument. (Closed)

Created:
4 years ago by tommyt
Modified:
4 years ago
CC:
chromium-reviews, rouslan+payments_chromium.org, sebsg+paymentswatch_chromium.org, agrieve+watch_chromium.org, zino1
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

PaymentApp: Allow multiple payment method names for one instrument. This changes the name and signature of the PaymentInstrument.getInstrumentMethodName method to: Set<String> getInstrumentMethodNames() This is to match the "enabledMethods" field in the PaymentAppOption dictionary in the Payment Apps specification, which is defined to be a sequence of strings. See: https://w3c.github.io/webpayments-payment-apps-api/#payment-app-options I also change the name of PaymentInstrument.getInstrumentDetails to "invokePayment" in order to convey better that this is where the payment method specific stuff happens. For a Web Based Payment App, this method is an appropriate point to launch the payment request event into the service worker. BUG=669876 Committed: https://crrev.com/0611804b7881c52ae3637bcaaaf4831f30363adb Cr-Commit-Position: refs/heads/master@{#435587}

Patch Set 1 #

Total comments: 12

Patch Set 2 : Address Rouslan's comments #

Total comments: 1

Patch Set 3 : Remove a superfluous word from a comment #

Messages

Total messages: 33 (20 generated)
tommyt
Here's a small change in preparation for handling third party apps in the Payment Request ...
4 years ago (2016-11-24 07:19:51 UTC) #5
zino
On 2016/11/24 07:19:51, tommyt wrote: > Here's a small change in preparation for handling third ...
4 years ago (2016-11-27 18:50:15 UTC) #8
please use gerrit instead
Tommy, our goal is to use only Android native apps on Android. I've been circulating ...
4 years ago (2016-11-28 17:51:32 UTC) #9
zino
On 2016/11/28 17:51:32, rouslan wrote: > If you are interested in web-based payment apps more, ...
4 years ago (2016-11-28 22:37:11 UTC) #10
tommyt
Hi Rouslan! I was aware of the Android native apps design, and your intention to ...
4 years ago (2016-11-29 09:00:43 UTC) #11
please use gerrit instead
Sorry to have cut off your efforts so rashly. The priority for us right now ...
4 years ago (2016-11-29 14:17:18 UTC) #12
tommyt
Hey Rouslan! I'm glad to hear that there's an opening for service worker based payment ...
4 years ago (2016-11-29 14:53:05 UTC) #13
tommyt
On 2016/11/29 14:17:18, rouslan wrote: > Overall comment: I'm not convinced that PaymentInstrument.invokePayment() is > ...
4 years ago (2016-11-30 09:16:18 UTC) #14
tommyt
Thanks for your feedback Rouslan. I've tried to address your comments in the latest patchset. ...
4 years ago (2016-11-30 10:15:50 UTC) #17
please use gerrit instead
lgtm % nit https://codereview.chromium.org/2530793002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentInstrument.java File chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentInstrument.java (right): https://codereview.chromium.org/2530793002/diff/20001/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentInstrument.java#newcode69 chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentInstrument.java:69: * e.g., whether the app should ...
4 years ago (2016-11-30 15:15:35 UTC) #21
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2530793002/40001
4 years ago (2016-12-01 08:51:08 UTC) #28
commit-bot: I haz the power
Committed patchset #3 (id:40001)
4 years ago (2016-12-01 08:54:58 UTC) #31
commit-bot: I haz the power
4 years ago (2016-12-01 08:56:39 UTC) #33
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/0611804b7881c52ae3637bcaaaf4831f30363adb
Cr-Commit-Position: refs/heads/master@{#435587}

Powered by Google App Engine
This is Rietveld 408576698