Use web-app manifest format for Android payment apps.

Use web-app manifest format for Android payment apps.

Payment manifest has been split in two:

   1. Payment method manifest. Example:

   {
    "default_applications": ["https://bobpay.com/app.json"],
    "supported_origins": ["https://alicepay.com"]
   }

   2. Web app manifest. Example:

   {
    "related_applications": [{
      "platform": "play",
      "id": "com.bobpay.app",
      "min_version": "1",
      "fingerprints": [{
        "type": "sha256_cert",
        "value": "59:5C:88:65:FF:C4:E8:20:CF:F7:3E:C8:64:D0:95:F0:06:19"
      }]
    }]
   }

Only "default_applications" is used in this patch. The
"supported_origins" field will be used in an upcoming patch.

Using these manifests is behind a flag:
chrome://flags/#android-payment-apps

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/YlMpaO-E2mE/-_GgiMiRCgAJ

Design:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE

Explainer:
https://github.com/zkoch/zkoch.github.io/blob/master/pmi-v2.md

BUG=683329
Review-Url: https://codereview.chromium.org/2802043002
Cr-Commit-Position: refs/heads/master@{#463342}
Committed: https://chromium.googlesource.com/chromium/src/+/672f4038a349267944659172c475417bca72b22f

[please use gerrit instead, 2017-04-06 17:04:19]

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

[please use gerrit instead, 2017-04-06 17:05:53]

Description was changed from

==========
Use web-app manifest format for Android payment apps.

Payment manifest has been split in two:

   1. Payment method manifest. Example:

   {
    "default_applications": ["https://bobpay.com/app.json"],
    "supported_origins": ["https://alicepay.com"]
   }

   2. Web app manifest. Example:

   {
    "related_applications": [{
      "platform": "play",
      "id": "com.bobpay.app",
      "min_version": "1",
      "fingerprints": [{
        "type": "sha256_cert",
        "value": "59:5C:88:65:FF:C4:E8:20:CF:F7:3E:C8:64:D0:95:F0:06:19"
      }]
    }]
   }

Using these manifests is behind a flag:
chrome://flags/#android-payment-apps

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/YlMpaO-E2mE/-_GgiMiR...

Spec:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE

Explainer:
https://github.com/zkoch/zkoch.github.io/blob/master/pmi-v2.md

Discussion:
https://github.com/w3c/webpayments/issues/225

BUG=683329
==========

to

==========
Use web-app manifest format for Android payment apps.

Payment manifest has been split in two:

   1. Payment method manifest. Example:

   {
    "default_applications": ["https://bobpay.com/app.json"],
    "supported_origins": ["https://alicepay.com"]
   }

   2. Web app manifest. Example:

   {
    "related_applications": [{
      "platform": "play",
      "id": "com.bobpay.app",
      "min_version": "1",
      "fingerprints": [{
        "type": "sha256_cert",
        "value": "59:5C:88:65:FF:C4:E8:20:CF:F7:3E:C8:64:D0:95:F0:06:19"
      }]
    }]
   }

Using these manifests is behind a flag:
chrome://flags/#android-payment-apps

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/YlMpaO-E2mE/-_GgiMiR...

Design:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE

Explainer:
https://github.com/zkoch/zkoch.github.io/blob/master/pmi-v2.md

Discussion:
https://github.com/w3c/webpayments/issues/225

BUG=683329
==========

[please use gerrit instead, 2017-04-06 17:17:54]

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

[commit-bot: I haz the power, 2017-04-06 17:18:30]

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-04-06 18:04:45]

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

[commit-bot: I haz the power, 2017-04-06 18:04:46]

Dry run: Try jobs failed on following builders:
  linux_chromium_chromeos_rel_ng on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[please use gerrit instead, 2017-04-06 19:26:49]

Description was changed from

==========
Use web-app manifest format for Android payment apps.

Payment manifest has been split in two:

   1. Payment method manifest. Example:

   {
    "default_applications": ["https://bobpay.com/app.json"],
    "supported_origins": ["https://alicepay.com"]
   }

   2. Web app manifest. Example:

   {
    "related_applications": [{
      "platform": "play",
      "id": "com.bobpay.app",
      "min_version": "1",
      "fingerprints": [{
        "type": "sha256_cert",
        "value": "59:5C:88:65:FF:C4:E8:20:CF:F7:3E:C8:64:D0:95:F0:06:19"
      }]
    }]
   }

Using these manifests is behind a flag:
chrome://flags/#android-payment-apps

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/YlMpaO-E2mE/-_GgiMiR...

Design:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE

Explainer:
https://github.com/zkoch/zkoch.github.io/blob/master/pmi-v2.md

Discussion:
https://github.com/w3c/webpayments/issues/225

BUG=683329
==========

to

==========
Use web-app manifest format for Android payment apps.

Payment manifest has been split in two:

   1. Payment method manifest. Example:

   {
    "default_applications": ["https://bobpay.com/app.json"],
    "supported_origins": ["https://alicepay.com"]
   }

   2. Web app manifest. Example:

   {
    "related_applications": [{
      "platform": "play",
      "id": "com.bobpay.app",
      "min_version": "1",
      "fingerprints": [{
        "type": "sha256_cert",
        "value": "59:5C:88:65:FF:C4:E8:20:CF:F7:3E:C8:64:D0:95:F0:06:19"
      }]
    }]
   }

Using these manifests is behind a flag:
chrome://flags/#android-payment-apps

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/YlMpaO-E2mE/-_GgiMiR...

Design:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE

Explainer:
https://github.com/zkoch/zkoch.github.io/blob/master/pmi-v2.md

BUG=683329
==========

[please use gerrit instead, 2017-04-06 20:02:47]

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

[commit-bot: I haz the power, 2017-04-06 20:03:39]

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, 2017-04-06 20:11:03]

Patchset #1 (id:1) has been deleted

[please use gerrit instead, 2017-04-06 20:11:13]

Patchset #1 (id:20001) has been deleted

[commit-bot: I haz the power, 2017-04-06 21:19:21]

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

[commit-bot: I haz the power, 2017-04-06 21:19:21]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2017-04-06 22:40:57]

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

[commit-bot: I haz the power, 2017-04-06 22:42:19]

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, 2017-04-06 22:43:23]

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

[please use gerrit instead, 2017-04-06 22:43:24]

Patchset #1 (id:40001) has been deleted

[please use gerrit instead, 2017-04-06 22:43:36]

Patchset #1 (id:60001) has been deleted

[commit-bot: I haz the power, 2017-04-06 22:43:42]

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, 2017-04-06 22:47:03]

rouslan@chromium.org changed reviewers:
+ gogerald@chromium.org, mathp@chromium.org, palmer@google.com

[please use gerrit instead, 2017-04-06 22:47:04]

Mathieu, ptal C++.

Ganggui, ptal Java.

Chris, ptal payment_manifest_parser.mojom.

[please use gerrit instead, 2017-04-06 23:00:17]

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

[commit-bot: I haz the power, 2017-04-06 23:00:30]

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-04-07 02:52:16]

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

[commit-bot: I haz the power, 2017-04-07 02:52:16]

Dry run: This issue passed the CQ dry run.

[Mathieu, 2017-04-07 13:29:07]

c++ lgtm, thanks

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
File components/payments/content/android/payment_manifest_parser_android.cc
(right):

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
components/payments/content/android/payment_manifest_parser_android.cc:33: if
(webAppManifestUrls.empty()) {
c++ case plz

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
components/payments/content/android/payment_manifest_parser_android.cc:55: void
OnWebAppManifestParsed(
Could benefit from comments. Now that the WebAppManifest is parsed, mention what
are the remaining steps. Is it strictly copying the results to java?

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
File components/payments/content/payment_manifest_parser.mojom (right):

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
components/payments/content/payment_manifest_parser.mojom:22: =>
(array<url.mojom.Url> webappManifestUrls);
*webAppManifestUrls?

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
File components/payments/content/payment_manifest_parser_host.cc (right):

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
components/payments/content/payment_manifest_parser_host.cc:75: const
std::vector<GURL>& webAppManifestUrls) {
c++ case

[Mathieu, 2017-04-07 13:32:34]

nit

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
File components/payments/content/utility/payment_manifest_parser.cc (right):

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
components/payments/content/utility/payment_manifest_parser.cc:44: if
(!dict->GetList("default_applications", &list) || !list)
same as below, and throughout

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
components/payments/content/utility/payment_manifest_parser.cc:54: if
(!list->GetString(i, &item) && item.empty()) {
I think the second condition is pretty much implied if the first returns false 

https://cs.chromium.org/chromium/src/base/values.h?rcl=0551782d3e45a41949e250...

[please use gerrit instead, 2017-04-07 14:24:39]

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

[commit-bot: I haz the power, 2017-04-07 14:24:48]

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-04-07 15:22:28]

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

[commit-bot: I haz the power, 2017-04-07 15:22:30]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2017-04-07 16:56:30]

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
File components/payments/content/android/payment_manifest_parser_android.cc
(right):

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
components/payments/content/android/payment_manifest_parser_android.cc:33: if
(webAppManifestUrls.empty()) {
On 2017/04/07 13:29:06, Mathieu wrote:
> c++ case plz

Done.

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
components/payments/content/android/payment_manifest_parser_android.cc:55: void
OnWebAppManifestParsed(
On 2017/04/07 13:29:06, Mathieu wrote:
> Could benefit from comments. Now that the WebAppManifest is parsed, mention
what
> are the remaining steps. Is it strictly copying the results to java?

Done. It's strictly a copy.

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
File components/payments/content/payment_manifest_parser.mojom (right):

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
components/payments/content/payment_manifest_parser.mojom:22: =>
(array<url.mojom.Url> webappManifestUrls);
On 2017/04/07 13:29:06, Mathieu wrote:
> *webAppManifestUrls?

Done.

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
File components/payments/content/payment_manifest_parser_host.cc (right):

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
components/payments/content/payment_manifest_parser_host.cc:75: const
std::vector<GURL>& webAppManifestUrls) {
On 2017/04/07 13:29:06, Mathieu wrote:
> c++ case

Done.

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
File components/payments/content/utility/payment_manifest_parser.cc (right):

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
components/payments/content/utility/payment_manifest_parser.cc:44: if
(!dict->GetList("default_applications", &list) || !list)
On 2017/04/07 13:32:34, Mathieu wrote:
> same as below, and throughout

Here you're right. Done.

https://codereview.chromium.org/2802043002/diff/100001/components/payments/co...
components/payments/content/utility/payment_manifest_parser.cc:54: if
(!list->GetString(i, &item) && item.empty()) {
On 2017/04/07 13:32:34, Mathieu wrote:
> I think the second condition is pretty much implied if the first returns false

> 
>
https://cs.chromium.org/chromium/src/base/values.h?rcl=0551782d3e45a41949e250...

If you have [""], then list->GetString(0, &item) will return `true` for the
empty string. Empty string is not valid in this algorithm, so I need to check
it.

[palmer, 2017-04-07 18:14:28]

palmer@chromium.org changed reviewers:
+ palmer@chromium.org

[palmer, 2017-04-07 18:14:29]

lgtm

https://codereview.chromium.org/2802043002/diff/120001/components/payments/co...
File components/payments/content/utility/payment_manifest_parser.h (right):

https://codereview.chromium.org/2802043002/diff/120001/components/payments/co...
components/payments/content/utility/payment_manifest_parser.h:18: // only in a
utility process.
Nit: Maybe specify "sandboxed" utility process.

[please use gerrit instead, 2017-04-07 18:48:03]

rouslan@chromium.org changed reviewers:
- palmer@google.com

[please use gerrit instead, 2017-04-07 19:23:08]

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

[please use gerrit instead, 2017-04-07 19:23:20]

https://codereview.chromium.org/2802043002/diff/120001/components/payments/co...
File components/payments/content/utility/payment_manifest_parser.h (right):

https://codereview.chromium.org/2802043002/diff/120001/components/payments/co...
components/payments/content/utility/payment_manifest_parser.h:18: // only in a
utility process.
On 2017/04/07 18:14:29, palmer wrote:
> Nit: Maybe specify "sandboxed" utility process.

Done.

[commit-bot: I haz the power, 2017-04-07 19:24:04]

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

[gogerald1, 2017-04-07 19:36:17]

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

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java:92:
private int mPendingWebAppManifestsNumber;
mPendingWebAppManifestsCount would be a little better?

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java:162:
mCallback.onInvalidManifest(mMethodName);
Do we want to fail the payment method if one web app manifest downloaded failed?
This sounds unfair for the other apps support this method since the web app
manifest is maintained by payment app but not payment method provider.

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java:166:
public void onPaymentMethodManifestParseSuccess(URI[] webAppManifestUris) {
move this function under onPaymentMethodManifestDownloadSuccess? They both for
payment method. Find it easier for me to read in that way (top down).

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java:168:
assert webAppManifestUris.length > 0;
might assert mPendingWebAppManifestsNumber == 0 since
onPaymentMethodManifestParseSuccess is expected to be called once for a method I
guess.

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java:170:
if (mAtLeastOneManifestFailedToDownloadOrParse) return;
mAtLeastOneManifestFailedToDownloadOrParse can not be true until now, right?
Might assert it.

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java:190:
for (AppInfo appInfo : mMatchingApps) {
Is onPaymentMethodManifestParseSuccess called only once for all the web app
manifests? Looks not true through below mPendingWebAppManifestsNumber--. 

It looks very inefficient. Can we figure out which appInfo this web app manifest
corresponds to?

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/junit/s...
File
chrome/android/junit/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFinderTest.java
(right):

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/junit/s...
chrome/android/junit/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFinderTest.java:280:
.when(packageManagerDelegate.getServicesThatCanRespondToIntent(
Mockito.when could be in single line

https://codereview.chromium.org/2802043002/diff/120001/components/payments/co...
File
components/payments/content/android/java/src/org/chromium/components/payments/PaymentManifestDownloader.java
(right):

https://codereview.chromium.org/2802043002/diff/120001/components/payments/co...
components/payments/content/android/java/src/org/chromium/components/payments/PaymentManifestDownloader.java:32:
* @param contents The successfully downloaded web app manifest.
content

[commit-bot: I haz the power, 2017-04-07 20:33:44]

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

[commit-bot: I haz the power, 2017-04-07 20:33:45]

Dry run: This issue passed the CQ dry run.

[please use gerrit instead, 2017-04-08 17:55:21]

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

[commit-bot: I haz the power, 2017-04-08 17:55:30]

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, 2017-04-08 18:54:48]

Ganggui, ptal patch 5.

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

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java:92:
private int mPendingWebAppManifestsNumber;
On 2017/04/07 19:36:16, gogerald1 wrote:
> mPendingWebAppManifestsCount would be a little better?

Done.

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java:162:
mCallback.onInvalidManifest(mMethodName);
On 2017/04/07 19:36:16, gogerald1 wrote:
> Do we want to fail the payment method if one web app manifest downloaded
failed?
> This sounds unfair for the other apps support this method since the web app
> manifest is maintained by payment app but not payment method provider.

The code currently downloads only the default web app manifest, which is owned
by the same person that owns the payment method. So we're not punishing person A
for mistakes of person B.

An upcoming patch will also download the other manifests. At that time we will
need to be careful to avoid the situation that you've described. That is not a
concert today, however.

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java:166:
public void onPaymentMethodManifestParseSuccess(URI[] webAppManifestUris) {
On 2017/04/07 19:36:16, gogerald1 wrote:
> move this function under onPaymentMethodManifestDownloadSuccess? They both for
> payment method. Find it easier for me to read in that way (top down).

Done.

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java:168:
assert webAppManifestUris.length > 0;
On 2017/04/07 19:36:16, gogerald1 wrote:
> might assert mPendingWebAppManifestsNumber == 0 since
> onPaymentMethodManifestParseSuccess is expected to be called once for a method
I
> guess.

Done.

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java:170:
if (mAtLeastOneManifestFailedToDownloadOrParse) return;
On 2017/04/07 19:36:16, gogerald1 wrote:
> mAtLeastOneManifestFailedToDownloadOrParse can not be true until now, right?
> Might assert it.

Done.

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java:190:
for (AppInfo appInfo : mMatchingApps) {
On 2017/04/07 19:36:16, gogerald1 wrote:
> Is onPaymentMethodManifestParseSuccess called only once for all the web app
> manifests?

Yes, because there's exactly one PaymentManifestVerifier per payment method
manifest file.

> Looks not true through below mPendingWebAppManifestsNumber--.

I don't follow. Please elaborate.

> It looks very inefficient. Can we figure out which appInfo this web app
manifest
> corresponds to?

Done.

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/junit/s...
File
chrome/android/junit/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFinderTest.java
(right):

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/junit/s...
chrome/android/junit/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFinderTest.java:280:
.when(packageManagerDelegate.getServicesThatCanRespondToIntent(
On 2017/04/07 19:36:16, gogerald1 wrote:
> Mockito.when could be in single line

Done.

https://codereview.chromium.org/2802043002/diff/120001/components/payments/co...
File
components/payments/content/android/java/src/org/chromium/components/payments/PaymentManifestDownloader.java
(right):

https://codereview.chromium.org/2802043002/diff/120001/components/payments/co...
components/payments/content/android/java/src/org/chromium/components/payments/PaymentManifestDownloader.java:32:
* @param contents The successfully downloaded web app manifest.
On 2017/04/07 19:36:16, gogerald1 wrote:
> content

Done.

[commit-bot: I haz the power, 2017-04-08 20:52:15]

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

[commit-bot: I haz the power, 2017-04-08 20:52:16]

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

[gogerald1, 2017-04-10 15:16:24]

lgtm

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

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java:162:
mCallback.onInvalidManifest(mMethodName);
On 2017/04/08 18:54:48, ಠ_ಠ wrote:
> On 2017/04/07 19:36:16, gogerald1 wrote:
> > Do we want to fail the payment method if one web app manifest downloaded
> failed?
> > This sounds unfair for the other apps support this method since the web app
> > manifest is maintained by payment app but not payment method provider.
> 
> The code currently downloads only the default web app manifest, which is owned
> by the same person that owns the payment method. So we're not punishing person
A
> for mistakes of person B.
> 
> An upcoming patch will also download the other manifests. At that time we will
> need to be careful to avoid the situation that you've described. That is not a
> concert today, however.

Acknowledged.

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java:190:
for (AppInfo appInfo : mMatchingApps) {
On 2017/04/08 18:54:48, ಠ_ಠ wrote:
> On 2017/04/07 19:36:16, gogerald1 wrote:
> > Is onPaymentMethodManifestParseSuccess called only once for all the web app
> > manifests?
> 
> Yes, because there's exactly one PaymentManifestVerifier per payment method
> manifest file.
> 
> > Looks not true through below mPendingWebAppManifestsNumber--.
> 
> I don't follow. Please elaborate.
> 
> > It looks very inefficient. Can we figure out which appInfo this web app
> manifest
> > corresponds to?
> 
> Done.

From mMatchingApps, mPendingWebAppManifestsNumber and
mAtLeastOneManifestFailedToDownloadOrParse, each payment method corresponds to
multiple payment apps. However, we also rely on the fact that we only download
the default payment app's manifest currently (each payment method corresponds to
one default payment app I guess). That's the part confused me. I think it could
be cleared after the next CL, but might be better to mention it in this CL
description.

[please use gerrit instead, 2017-04-10 15:44:21]

Description was changed from

==========
Use web-app manifest format for Android payment apps.

Payment manifest has been split in two:

   1. Payment method manifest. Example:

   {
    "default_applications": ["https://bobpay.com/app.json"],
    "supported_origins": ["https://alicepay.com"]
   }

   2. Web app manifest. Example:

   {
    "related_applications": [{
      "platform": "play",
      "id": "com.bobpay.app",
      "min_version": "1",
      "fingerprints": [{
        "type": "sha256_cert",
        "value": "59:5C:88:65:FF:C4:E8:20:CF:F7:3E:C8:64:D0:95:F0:06:19"
      }]
    }]
   }

Using these manifests is behind a flag:
chrome://flags/#android-payment-apps

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/YlMpaO-E2mE/-_GgiMiR...

Design:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE

Explainer:
https://github.com/zkoch/zkoch.github.io/blob/master/pmi-v2.md

BUG=683329
==========

to

==========
Use web-app manifest format for Android payment apps.

Payment manifest has been split in two:

   1. Payment method manifest. Example:

   {
    "default_applications": ["https://bobpay.com/app.json"],
    "supported_origins": ["https://alicepay.com"]
   }

   2. Web app manifest. Example:

   {
    "related_applications": [{
      "platform": "play",
      "id": "com.bobpay.app",
      "min_version": "1",
      "fingerprints": [{
        "type": "sha256_cert",
        "value": "59:5C:88:65:FF:C4:E8:20:CF:F7:3E:C8:64:D0:95:F0:06:19"
      }]
    }]
   }

Only "default_applications" is used in this patch. The
"supported_origins" field will be used in an upcoming patch.

Using these manifests is behind a flag:
chrome://flags/#android-payment-apps

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/YlMpaO-E2mE/-_GgiMiR...

Design:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE

Explainer:
https://github.com/zkoch/zkoch.github.io/blob/master/pmi-v2.md

BUG=683329
==========

[please use gerrit instead, 2017-04-10 15:46:06]

Sending to cq.

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

https://codereview.chromium.org/2802043002/diff/120001/chrome/android/java/sr...
chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java:190:
for (AppInfo appInfo : mMatchingApps) {
On 2017/04/10 15:16:24, gogerald1 wrote:
> On 2017/04/08 18:54:48, ಠ_ಠ wrote:
> > On 2017/04/07 19:36:16, gogerald1 wrote:
> > > Is onPaymentMethodManifestParseSuccess called only once for all the web
app
> > > manifests?
> > 
> > Yes, because there's exactly one PaymentManifestVerifier per payment method
> > manifest file.
> > 
> > > Looks not true through below mPendingWebAppManifestsNumber--.
> > 
> > I don't follow. Please elaborate.
> > 
> > > It looks very inefficient. Can we figure out which appInfo this web app
> > manifest
> > > corresponds to?
> > 
> > Done.
> 
> From mMatchingApps, mPendingWebAppManifestsNumber and
> mAtLeastOneManifestFailedToDownloadOrParse, each payment method corresponds to
> multiple payment apps. However, we also rely on the fact that we only download
> the default payment app's manifest currently (each payment method corresponds
to
> one default payment app I guess). That's the part confused me. I think it
could
> be cleared after the next CL, but might be better to mention it in this CL
> description.

Done.

[please use gerrit instead, 2017-04-10 16:06:26]

The CQ bit was checked by rouslan@chromium.org

[please use gerrit instead, 2017-04-10 16:06:27]

The patchset sent to the CQ was uploaded after l-g-t-m from mathp@chromium.org,
palmer@chromium.org
Link to the patchset: https://codereview.chromium.org/2802043002/#ps160001
(title: "Address java comments.")

[commit-bot: I haz the power, 2017-04-10 16:06: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-04-10 18:53:09]

CQ is committing da patch.

Bot data: {"patchset_id": 160001, "attempt_start_ts": 1491840386756180,
"parent_rev": "c694aa0db08db59679ff65171b05b1d228f3c189", "commit_rev":
"672f4038a349267944659172c475417bca72b22f"}

[commit-bot: I haz the power, 2017-04-10 18:54:12]

Description was changed from

==========
Use web-app manifest format for Android payment apps.

Payment manifest has been split in two:

   1. Payment method manifest. Example:

   {
    "default_applications": ["https://bobpay.com/app.json"],
    "supported_origins": ["https://alicepay.com"]
   }

   2. Web app manifest. Example:

   {
    "related_applications": [{
      "platform": "play",
      "id": "com.bobpay.app",
      "min_version": "1",
      "fingerprints": [{
        "type": "sha256_cert",
        "value": "59:5C:88:65:FF:C4:E8:20:CF:F7:3E:C8:64:D0:95:F0:06:19"
      }]
    }]
   }

Only "default_applications" is used in this patch. The
"supported_origins" field will be used in an upcoming patch.

Using these manifests is behind a flag:
chrome://flags/#android-payment-apps

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/YlMpaO-E2mE/-_GgiMiR...

Design:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE

Explainer:
https://github.com/zkoch/zkoch.github.io/blob/master/pmi-v2.md

BUG=683329
==========

to

==========
Use web-app manifest format for Android payment apps.

Payment manifest has been split in two:

   1. Payment method manifest. Example:

   {
    "default_applications": ["https://bobpay.com/app.json"],
    "supported_origins": ["https://alicepay.com"]
   }

   2. Web app manifest. Example:

   {
    "related_applications": [{
      "platform": "play",
      "id": "com.bobpay.app",
      "min_version": "1",
      "fingerprints": [{
        "type": "sha256_cert",
        "value": "59:5C:88:65:FF:C4:E8:20:CF:F7:3E:C8:64:D0:95:F0:06:19"
      }]
    }]
   }

Only "default_applications" is used in this patch. The
"supported_origins" field will be used in an upcoming patch.

Using these manifests is behind a flag:
chrome://flags/#android-payment-apps

Intent to implement:
https://groups.google.com/a/chromium.org/d/msg/blink-dev/YlMpaO-E2mE/-_GgiMiR...

Design:
https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE

Explainer:
https://github.com/zkoch/zkoch.github.io/blob/master/pmi-v2.md

BUG=683329

Review-Url: https://codereview.chromium.org/2802043002
Cr-Commit-Position: refs/heads/master@{#463342}
Committed:
https://chromium.googlesource.com/chromium/src/+/672f4038a349267944659172c475...
==========

[commit-bot: I haz the power, 2017-04-10 18:54:14]

Committed patchset #5 (id:160001) as
https://chromium.googlesource.com/chromium/src/+/672f4038a349267944659172c475...