PaymentApp: Implement invokePaymentApp for Android

PaymentApp: Implement invokePaymentApp for Android

This change implements the
ServiceWorkerPaymentAppBridge.invokePaymentApp() function along with the
corresponding native InvokePaymentApp() function.

The signature of PaymentInstrument.invokePaymentApp() has also been
changed to add the payment details modifiers field, and to align the
ordering and naming of the arguments with the definition of the
PaymentAppRequest dictionary in the specification:

    https://w3c.github.io/webpayments-payment-apps-api/#sec-app-request

BUG=669876
Review-Url: https://codereview.chromium.org/2640743005
Cr-Commit-Position: refs/heads/master@{#445301}
Committed: https://chromium.googlesource.com/chromium/src/+/dfd9bf1edeca16610876ba4e33150401143b1924

[tommyt, 2017-01-19 10:05:29]

The CQ bit was checked by tommyt@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 10:05:44]

Dry run: 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, 2017-01-19 10:32:21]

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

[commit-bot: I haz the power, 2017-01-19 10:32:22]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[tommyt, 2017-01-19 12:17:27]

The CQ bit was checked by tommyt@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-19 12:17:35]

Dry run: 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, 2017-01-19 12:50:31]

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

[commit-bot: I haz the power, 2017-01-19 12:50:32]

Dry run: This issue passed the CQ dry run.

[tommyt, 2017-01-19 12:53:12]

tommyt@opera.com changed reviewers:
+ jinho.bang@samsung.com, rouslan@chromium.org

[tommyt, 2017-01-19 12:53:13]

PTAL :)

[please use gerrit instead, 2017-01-19 16:45:05]

https://codereview.chromium.org/2640743005/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(left):

https://codereview.chromium.org/2640743005/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:628:
if (mModifiers == null) mModifiers = new ArrayMap<>();
I would prefer to keep null ArrayMap if the merchant website does not provide
the modifiers parameter. That's less memory intensive, which is important on
older devices.

https://codereview.chromium.org/2640743005/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2640743005/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1048:
if (mModifiers.containsKey(instrumentMethodName)) {
You can check mModifiers != null here, if you keep mModifiers null as I
suggested above.

https://codereview.chromium.org/2640743005/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1054:
mRawTotal, mRawLineItems, Collections.unmodifiableMap(modifiers), this);
Might be a good idea to make mRawLineItems unmodifiable as well.

https://codereview.chromium.org/2640743005/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ServiceWorkerPaymentAppBridge.java
(right):

https://codereview.chromium.org/2640743005/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ServiceWorkerPaymentAppBridge.java:73:
* app.
Please align indentation of "app" with "The" above.

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
File chrome/browser/android/payments/service_worker_payment_app_bridge.cc
(right):

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:88:
payments::mojom::PaymentAppRequestDataPtr data =
using payments::mojom::PaymentAppRequestDataPtr;

(to make your life easier)

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:89:
payments::mojom::PaymentAppRequestData::New();
using payments::mojom::PaymentAppRequestData;

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:91:
data->origin = web_contents->GetLastCommittedURL().GetOrigin();
PaymentRequestImpl.java has mOrigin that it passes to payment apps. Let's do the
same here to be consistent.

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:93: for
(int i = 0; i < env->GetArrayLength(jmethod_data); i++) {
GetArrayLength returns jsize type, so let's use that for the index.

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:96:
payments::mojom::PaymentMethodDataPtr methodData =
using payments::mojom::PaymentMethodDataPtr;

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:97:
payments::mojom::PaymentMethodData::New();
using payments::mojom::PaymentMethodData;

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:108:
data->methodData.push_back(std::move(methodData));
#include <utility>

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:111:
data->total = payments::mojom::PaymentItem::New();
using payments::mojom::PaymentItem;

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:115:
data->total->amount = payments::mojom::PaymentCurrencyAmount::New();
using payments::mojom::PaymentCurrencyAmount;

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:126:
payments::mojom::PaymentDetailsModifierPtr modifier =
using payments::mojom::PaymentDetailsModifierPtr;

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:127:
payments::mojom::PaymentDetailsModifier::New();
using payments::mojom::PaymentDetailsModifier;

[jbudorick, 2017-01-19 18:55:29]

Description was changed from

==========
PaymentApp: Implement invokePaymentApp for Android

This change implements the
ServiceWorkerPaymentAppBridge.invokePaymentApp() function along with the
corresponding native InvokePaymentApp() function.

The signature of PaymentInstrument.invokePaymentApp() has also been
changed to add the payment details modifiers field, and to align the
ordering and naming of the arguments with the definition of the
PaymentAppRequest dictionary in the specification:

    https://w3c.github.io/webpayments-payment-apps-api/#sec-app-request

BUG=669876
==========

to

==========
PaymentApp: Implement invokePaymentApp for Android

This change implements the
ServiceWorkerPaymentAppBridge.invokePaymentApp() function along with the
corresponding native InvokePaymentApp() function.

The signature of PaymentInstrument.invokePaymentApp() has also been
changed to add the payment details modifiers field, and to align the
ordering and naming of the arguments with the definition of the
PaymentAppRequest dictionary in the specification:

    https://w3c.github.io/webpayments-payment-apps-api/#sec-app-request

BUG=669876
==========

[tommyt, 2017-01-20 09:32:27]

The CQ bit was checked by tommyt@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-20 09:32:38]

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

[tommyt, 2017-01-20 09:33:28]

tommyt@opera.com changed reviewers:
+ miguelg@chromium.org

[tommyt, 2017-01-20 09:33:30]

Adding miguelg for the file:

  chrome/browser/android/payments/service_worker_payment_app_bridge.cc

https://codereview.chromium.org/2640743005/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(left):

https://codereview.chromium.org/2640743005/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:628:
if (mModifiers == null) mModifiers = new ArrayMap<>();
On 2017/01/19 16:45:04, rouslan wrote:
> I would prefer to keep null ArrayMap if the merchant website does not provide
> the modifiers parameter. That's less memory intensive, which is important on
> older devices.

Done.

https://codereview.chromium.org/2640743005/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java
(right):

https://codereview.chromium.org/2640743005/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1048:
if (mModifiers.containsKey(instrumentMethodName)) {
On 2017/01/19 16:45:04, rouslan wrote:
> You can check mModifiers != null here, if you keep mModifiers null as I
> suggested above.

Done.

https://codereview.chromium.org/2640743005/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentRequestImpl.java:1054:
mRawTotal, mRawLineItems, Collections.unmodifiableMap(modifiers), this);
On 2017/01/19 16:45:04, rouslan wrote:
> Might be a good idea to make mRawLineItems unmodifiable as well.

It should already be unmodifiable. It is assigned an unmodifiable value on line
605, like this:

    mRawLineItems =
Collections.unmodifiableList(Arrays.asList(details.displayItems));

I could remove the Collections.unmodifiableList() call from line 605 and add it
to this line, if you want? It would arguably be tidier to keep all the relevant
unmodifiableX() calls in one place.

https://codereview.chromium.org/2640743005/diff/20001/chrome/android/java/src...
File
chrome/android/java/src/org/chromium/chrome/browser/payments/ServiceWorkerPaymentAppBridge.java
(right):

https://codereview.chromium.org/2640743005/diff/20001/chrome/android/java/src...
chrome/android/java/src/org/chromium/chrome/browser/payments/ServiceWorkerPaymentAppBridge.java:73:
* app.
On 2017/01/19 16:45:04, rouslan wrote:
> Please align indentation of "app" with "The" above.

Done.

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
File chrome/browser/android/payments/service_worker_payment_app_bridge.cc
(right):

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:88:
payments::mojom::PaymentAppRequestDataPtr data =
On 2017/01/19 16:45:04, rouslan wrote:
> using payments::mojom::PaymentAppRequestDataPtr;
> 
> (to make your life easier)

Done.

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:89:
payments::mojom::PaymentAppRequestData::New();
On 2017/01/19 16:45:05, rouslan wrote:
> using payments::mojom::PaymentAppRequestData;

Done.

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:91:
data->origin = web_contents->GetLastCommittedURL().GetOrigin();
On 2017/01/19 16:45:05, rouslan wrote:
> PaymentRequestImpl.java has mOrigin that it passes to payment apps. Let's do
the
> same here to be consistent.

Done.

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:93: for
(int i = 0; i < env->GetArrayLength(jmethod_data); i++) {
On 2017/01/19 16:45:04, rouslan wrote:
> GetArrayLength returns jsize type, so let's use that for the index.

Done.

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:96:
payments::mojom::PaymentMethodDataPtr methodData =
On 2017/01/19 16:45:05, rouslan wrote:
> using payments::mojom::PaymentMethodDataPtr;

Done.

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:97:
payments::mojom::PaymentMethodData::New();
On 2017/01/19 16:45:05, rouslan wrote:
> using payments::mojom::PaymentMethodData;

Done.

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:108:
data->methodData.push_back(std::move(methodData));
On 2017/01/19 16:45:05, rouslan wrote:
> #include <utility>

Done.

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:111:
data->total = payments::mojom::PaymentItem::New();
On 2017/01/19 16:45:05, rouslan wrote:
> using payments::mojom::PaymentItem;

Done.

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:115:
data->total->amount = payments::mojom::PaymentCurrencyAmount::New();
On 2017/01/19 16:45:05, rouslan wrote:
> using payments::mojom::PaymentCurrencyAmount;

Done.

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:126:
payments::mojom::PaymentDetailsModifierPtr modifier =
On 2017/01/19 16:45:05, rouslan wrote:
> using payments::mojom::PaymentDetailsModifierPtr;

Done.

https://codereview.chromium.org/2640743005/diff/20001/chrome/browser/android/...
chrome/browser/android/payments/service_worker_payment_app_bridge.cc:127:
payments::mojom::PaymentDetailsModifier::New();
On 2017/01/19 16:45:05, rouslan wrote:
> using payments::mojom::PaymentDetailsModifier;

Done.

[Miguel Garcia, 2017-01-20 09:45:12]

miguelg@chromium.org changed reviewers:
+ tedchoc@chromium.org

[Miguel Garcia, 2017-01-20 09:45:13]

Payments is being handled by the frontend team in MTV, moving this to Ted.

[Miguel Garcia, 2017-01-20 09:45:15]

Payments is being handled by the frontend team in MTV, moving this to Ted.

[commit-bot: I haz the power, 2017-01-20 09:55:53]

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

[commit-bot: I haz the power, 2017-01-20 09:55:54]

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...)

[tommyt, 2017-01-20 10:45:33]

The CQ bit was checked by tommyt@opera.com to run a CQ dry run

[commit-bot: I haz the power, 2017-01-20 10:45:45]

Dry run: 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, 2017-01-20 11:23:05]

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

[commit-bot: I haz the power, 2017-01-20 11:23:06]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2017-01-20 17:52:40]

lgtm

[Ted C, 2017-01-20 18:10:31]

On 2017/01/20 17:52:40, rouslan wrote:
> lgtm

owners lgtm

[zino, 2017-01-20 21:30:47]

lgtm

[tommyt, 2017-01-22 08:31:37]

The CQ bit was checked by tommyt@opera.com

[commit-bot: I haz the power, 2017-01-22 08:31:45]

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, 2017-01-22 09:07:47]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1485073897224680,
"parent_rev": "22ef2dc84e0dbf3df1e2a327d87d44de6ace11e6", "commit_rev":
"dfd9bf1edeca16610876ba4e33150401143b1924"}

[commit-bot: I haz the power, 2017-01-22 09:08:41]

Description was changed from

==========
PaymentApp: Implement invokePaymentApp for Android

This change implements the
ServiceWorkerPaymentAppBridge.invokePaymentApp() function along with the
corresponding native InvokePaymentApp() function.

The signature of PaymentInstrument.invokePaymentApp() has also been
changed to add the payment details modifiers field, and to align the
ordering and naming of the arguments with the definition of the
PaymentAppRequest dictionary in the specification:

    https://w3c.github.io/webpayments-payment-apps-api/#sec-app-request

BUG=669876
==========

to

==========
PaymentApp: Implement invokePaymentApp for Android

This change implements the
ServiceWorkerPaymentAppBridge.invokePaymentApp() function along with the
corresponding native InvokePaymentApp() function.

The signature of PaymentInstrument.invokePaymentApp() has also been
changed to add the payment details modifiers field, and to align the
ordering and naming of the arguments with the definition of the
PaymentAppRequest dictionary in the specification:

    https://w3c.github.io/webpayments-payment-apps-api/#sec-app-request

BUG=669876

Review-Url: https://codereview.chromium.org/2640743005
Cr-Commit-Position: refs/heads/master@{#445301}
Committed:
https://chromium.googlesource.com/chromium/src/+/dfd9bf1edeca16610876ba4e3315...
==========

[commit-bot: I haz the power, 2017-01-22 09:08:42]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/dfd9bf1edeca16610876ba4e3315...