Use web-app manifest format for Android payment apps.

chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java

Index: chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java b/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java
index 0b20667fd2bdc140c1e0aca261263622b773c3f5..73ed77c2d051dd27efb255f4bbc40e61f71284a7 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/payments/PaymentManifestVerifier.java
@@ -14,7 +14,7 @@ import org.chromium.components.payments.PaymentManifestDownloader;
 import org.chromium.components.payments.PaymentManifestDownloader.ManifestDownloadCallback;
 import org.chromium.components.payments.PaymentManifestParser;
 import org.chromium.components.payments.PaymentManifestParser.ManifestParseCallback;
-import org.chromium.payments.mojom.PaymentManifestSection;
+import org.chromium.payments.mojom.WebAppManifestSection;
 
 import java.net.URI;
 import java.security.MessageDigest;
@@ -84,11 +84,13 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
 
     private final PaymentManifestDownloader mDownloader;
     private final URI mMethodName;
-    private final List<AppInfo> mMatchingApps;
+    private final Set<AppInfo> mMatchingApps;
     private final PaymentManifestParser mParser;
     private final PackageManagerDelegate mPackageManagerDelegate;
     private final ManifestVerifyCallback mCallback;
     private final MessageDigest mMessageDigest;
+    private int mPendingWebAppManifestsNumber;

[gogerald1, 2017/04/07 19:36:16]

mPendingWebAppManifestsCount would be a little better?

[please use gerrit instead, 2017/04/08 18:54:48]

Done.

+    private boolean mAtLeastOneManifestFailedToDownloadOrParse;
 
     /**
      * Builds the manifest verifier.
@@ -110,7 +112,7 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
         assert !matchingApps.isEmpty();
 
         mMethodName = methodName;
-        mMatchingApps = new ArrayList<>();
+        mMatchingApps = new HashSet<>();
         for (int i = 0; i < matchingApps.size(); i++) {
             AppInfo appInfo = new AppInfo();
             appInfo.resolveInfo = matchingApps.get(i);
@@ -126,7 +128,9 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
             md = MessageDigest.getInstance("SHA-256");
         } catch (NoSuchAlgorithmException e) {
             // Intentionally ignore.
-            Log.d(TAG, "Unable to generate SHA-256 hashes. Only \"package\": \"*\" supported.");
+            Log.d(TAG,
+                    "Unable to generate SHA-256 hashes. Only \"supported_origins\": [\"*\"] "
+                            + "is supported.");
         }
         mMessageDigest = md;
     }
@@ -136,43 +140,54 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
      * privileges to handle this payment method.
      */
     public void verify() {
-        mDownloader.download(mMethodName, this);
+        mDownloader.downloadPaymentMethodManifest(mMethodName, this);
     }
 
     @Override
-    public void onManifestDownloadSuccess(String content) {
-        mParser.parse(content, this);
+    public void onPaymentMethodManifestDownloadSuccess(String content) {
+        mParser.parsePaymentMethodManifest(content, this);
+    }
+
+    @Override
+    public void onWebAppManifestDownloadSuccess(String content) {
+        if (mAtLeastOneManifestFailedToDownloadOrParse) return;
+        mParser.parseWebAppManifest(content, this);
     }
 
     @Override
     public void onManifestDownloadFailure() {
+        if (mAtLeastOneManifestFailedToDownloadOrParse) return;
+        mAtLeastOneManifestFailedToDownloadOrParse = true;
+
         mCallback.onInvalidManifest(mMethodName);

[gogerald1, 2017/04/07 19:36:16]

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.

[please use gerrit instead, 2017/04/08 18:54:48]

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.

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

Acknowledged.

     }
 
     @Override
-    public void onManifestParseSuccess(PaymentManifestSection[] manifest) {
+    public void onPaymentMethodManifestParseSuccess(URI[] webAppManifestUris) {

[gogerald1, 2017/04/07 19:36:16]

move this function under onPaymentMethodManifestDownloadSuccess? They both for
payment method. Find it easier for me to read in that way (top down).

[please use gerrit instead, 2017/04/08 18:54:48]

Done.

+        assert webAppManifestUris != null;
+        assert webAppManifestUris.length > 0;

[gogerald1, 2017/04/07 19:36:16]

might assert mPendingWebAppManifestsNumber == 0 since
onPaymentMethodManifestParseSuccess is expected to be called once for a method I
guess.

[please use gerrit instead, 2017/04/08 18:54:48]

Done.

+
+        if (mAtLeastOneManifestFailedToDownloadOrParse) return;

[gogerald1, 2017/04/07 19:36:16]

mAtLeastOneManifestFailedToDownloadOrParse can not be true until now, right?
Might assert it.

[please use gerrit instead, 2017/04/08 18:54:47]

Done.

+
+        mPendingWebAppManifestsNumber = webAppManifestUris.length;
+        for (int i = 0; i < webAppManifestUris.length; i++) {
+            mDownloader.downloadWebAppManifest(webAppManifestUris[i], this);
+        }
+    }
+
+    @Override
+    public void onWebAppManifestParseSuccess(WebAppManifestSection[] manifest) {
         assert manifest != null;
         assert manifest.length > 0;
 
-        for (int i = 0; i < manifest.length; i++) {
-            PaymentManifestSection section = manifest[i];
-            // "package": "*" in the manifest file indicates an unrestricted payment method. Any app
-            // can use this payment method name.
-            if ("*".equals(section.packageName)) {
-                for (int j = 0; j < mMatchingApps.size(); j++) {
-                    mCallback.onValidPaymentApp(mMethodName, mMatchingApps.get(j).resolveInfo);
-                }
-                return;
-            }
-        }
+        if (mAtLeastOneManifestFailedToDownloadOrParse) return;
 
         if (mMessageDigest == null) {
             mCallback.onInvalidManifest(mMethodName);
             return;
         }
 
-        for (int i = 0; i < mMatchingApps.size(); i++) {
-            AppInfo appInfo = mMatchingApps.get(i);
+        for (AppInfo appInfo : mMatchingApps) {

[gogerald1, 2017/04/07 19:36:16]

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?

[please use gerrit instead, 2017/04/08 18:54:48]

Yes, because there's exactly one PaymentManifestVerifier per payment method
manifest file.
I don't follow. Please elaborate.
Done.

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

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:46:06]

Done.

             PackageInfo packageInfo = mPackageManagerDelegate.getPackageInfoWithSignatures(
                     appInfo.resolveInfo.activityInfo.packageName);
 
@@ -183,8 +198,8 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
             appInfo.version = packageInfo.versionCode;
             appInfo.sha256CertFingerprints = new HashSet<>();
             Signature[] signatures = packageInfo.signatures;
-            for (int j = 0; j < signatures.length; j++) {
-                mMessageDigest.update(signatures[j].toByteArray());
+            for (int i = 0; i < signatures.length; i++) {
+                mMessageDigest.update(signatures[i].toByteArray());
 
                 // The digest is reset after completing the hash computation.
                 appInfo.sha256CertFingerprints.add(byteArrayToString(mMessageDigest.digest()));
@@ -193,31 +208,35 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
 
         List<Set<String>> sectionsFingerprints = new ArrayList<>();
         for (int i = 0; i < manifest.length; i++) {
-            PaymentManifestSection section = manifest[i];
+            WebAppManifestSection section = manifest[i];
             Set<String> fingerprints = new HashSet<>();
-            if (section.sha256CertFingerprints != null) {
-                for (int j = 0; j < section.sha256CertFingerprints.length; j++) {
-                    fingerprints.add(byteArrayToString(section.sha256CertFingerprints[j]));
-                }
+            for (int j = 0; j < section.fingerprints.length; j++) {
+                fingerprints.add(byteArrayToString(section.fingerprints[j]));
             }
             sectionsFingerprints.add(fingerprints);
         }
 
-        for (int i = 0; i < mMatchingApps.size(); i++) {
-            AppInfo appInfo = mMatchingApps.get(i);
-            boolean isAllowed = false;
-            for (int j = 0; j < manifest.length; j++) {
-                PaymentManifestSection section = manifest[j];
-                if (appInfo.resolveInfo.activityInfo.packageName.equals(section.packageName)
-                        && appInfo.version >= section.version
+        Set<AppInfo> matchingAppsToRemove = new HashSet<>();
+        for (AppInfo appInfo : mMatchingApps) {
+            for (int i = 0; i < manifest.length; i++) {
+                WebAppManifestSection section = manifest[i];
+                if (appInfo.resolveInfo.activityInfo.packageName.equals(section.id)
+                        && appInfo.version >= section.minVersion
                         && appInfo.sha256CertFingerprints != null
-                        && appInfo.sha256CertFingerprints.equals(sectionsFingerprints.get(j))) {
+                        && appInfo.sha256CertFingerprints.equals(sectionsFingerprints.get(i))) {
+                    matchingAppsToRemove.add(appInfo);
                     mCallback.onValidPaymentApp(mMethodName, appInfo.resolveInfo);
-                    isAllowed = true;
                     break;
                 }
             }
-            if (!isAllowed) mCallback.onInvalidPaymentApp(mMethodName, appInfo.resolveInfo);
+        }
+        mMatchingApps.removeAll(matchingAppsToRemove);
+
+        mPendingWebAppManifestsNumber--;
+        if (mPendingWebAppManifestsNumber == 0) {
+            for (AppInfo appInfo : mMatchingApps) {
+                mCallback.onInvalidPaymentApp(mMethodName, appInfo.resolveInfo);
+            }
         }
     }
 
@@ -241,6 +260,9 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
 
     @Override
     public void onManifestParseFailure() {
+        if (mAtLeastOneManifestFailedToDownloadOrParse) return;
+        mAtLeastOneManifestFailedToDownloadOrParse = true;
+
         mCallback.onInvalidManifest(mMethodName);
     }
 }