Download web payment manifests.

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

Index: chrome/android/java/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFactory.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFactory.java b/chrome/android/java/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFactory.java
index 9eaf4575f4403532824ff6b0bee2559d5b482dc4..a6f0f7b09e8d50f2f18e380ba3e2c2092bf8b4d8 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFactory.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/payments/AndroidPaymentAppFactory.java
@@ -4,7 +4,6 @@
 
 package org.chromium.chrome.browser.payments;
 
-import android.content.Context;
 import android.content.Intent;
 import android.content.pm.PackageManager;
 import android.content.pm.ResolveInfo;
@@ -13,83 +12,216 @@ import android.net.Uri;
 import android.util.Pair;
 
 import org.chromium.base.ContextUtils;
-import org.chromium.chrome.browser.ChromeActivity;
 import org.chromium.chrome.browser.UrlConstants;
 import org.chromium.chrome.browser.payments.PaymentAppFactory.PaymentAppCreatedCallback;
 import org.chromium.chrome.browser.payments.PaymentAppFactory.PaymentAppFactoryAddition;
+import org.chromium.chrome.browser.payments.PaymentManifestVerifier.ManifestVerifyCallback;
 import org.chromium.content_public.browser.WebContents;
 
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.ArrayList;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 
 /** Builds instances of payment apps based on installed third party Android payment apps. */
 public class AndroidPaymentAppFactory implements PaymentAppFactoryAddition {
-    private static final String ACTION_IS_READY_TO_PAY =
-            "org.chromium.intent.action.IS_READY_TO_PAY";
-
-    /** The action name for the Pay Basic-card Intent. */
-    private static final String ACTION_PAY_BASIC_CARD = "org.chromium.intent.action.PAY_BASIC_CARD";
+    @Override
+    public void create(
+            WebContents webContents, Set<String> methods, PaymentAppCreatedCallback callback) {
+        new PaymentAppFinder(webContents, methods, callback).find();
+    }
 
     /**
-     * The basic-card payment method name used by merchant and defined by W3C:
-     * https://w3c.github.io/webpayments-methods-card/#method-id
+     * Finds installed native Android payment apps and verifies their signatures according to the
+     * payment method manifests. The manifests are located based on the payment method name, which
+     * is a URI that starts with "https://". The "basic-card" payment method is an exception: it's a
+     * common payment method that does not have a manifest and can be used by any payment app.
      */
-    private static final String BASIC_CARD_PAYMENT_METHOD = "basic-card";
+    private static class PaymentAppFinder implements ManifestVerifyCallback {
+        /** The name of the intent for the service to check whether an app is ready to pay. */
+        private static final String ACTION_IS_READY_TO_PAY =
+                "org.chromium.intent.action.IS_READY_TO_PAY";
 
-    @Override
-    public void create(WebContents webContents, Set<String> methods,
-            PaymentAppCreatedCallback callback) {
-        Context context = ChromeActivity.fromWebContents(webContents);
-        if (context == null) {
-            callback.onAllPaymentAppsCreated();
-            return;
+        /** The name of the intent for the action of paying using "basic-card" method. */
+        private static final String ACTION_PAY_BASIC_CARD =
+                "org.chromium.intent.action.PAY_BASIC_CARD";
+
+        /**
+         * The basic-card payment method name used by merchant and defined by W3C:
+         * https://w3c.github.io/webpayments-methods-card/#method-id
+         */
+        private static final String BASIC_CARD_PAYMENT_METHOD = "basic-card";
+
+        /** The maximum number of payment method manifests to download. */
+        private static final int MAX_NUMBER_OF_MANIFESTS = 10;
+
+        private final WebContents mWebContents;
+        private final PaymentAppCreatedCallback mCallback;
+
+        /** Whether "basic-card" supporting payment apps should be queried. */
+        private final boolean mQueryBasicCard;
+
+        /**
+         * A map of payment method names to the list of unverified (yet) Android apps that claim to
+         * handle these methods. Example payment method names in this data structure:
+         * "https://bobpay.com", "https://android.com/pay". Basic card is excluded.
+         */
+        private final Map<URI, Set<ResolveInfo>> mPendingApps;
+
+        /** A map of Android package name to the payment app. */
+        private final Map<String, AndroidPaymentApp> mResult;
+
+        private final Set<URI> mPaymentMethods;
+
+        private PaymentManifestParser mParser;
+
+        /**
+         * Builds a native Android payment app finder.
+         *
+         * @param webContents The web contents that invoked the web payments API.
+         * @param methods     The list of payment methods requested by the merchant. For example,
+         *                    "https://bobpay.com", "https://android.com/pay", "basic-card".
+         * @param callback    The asynchronous callback to be invoked (on the UI thread) when all
+         *                    Android payment apps have been found.
+         */
+        public PaymentAppFinder(
+                WebContents webContents, Set<String> methods, PaymentAppCreatedCallback callback) {
+            mWebContents = webContents;
+            mCallback = callback;
+            mQueryBasicCard = methods.contains(BASIC_CARD_PAYMENT_METHOD);
+            mPendingApps = new HashMap<>();
+            mResult = new HashMap<>();
+
+            mPaymentMethods = new HashSet<>();
+            for (String method : methods) {
+                assert method != null;
+                if (method.startsWith(UrlConstants.HTTPS_URL_PREFIX)) {
+                    URI uri;
+                    try {
+                        // Don't use java.net.URL, because it performs a synchronous DNS lookup in
+                        // the constructor.
+                        uri = new URI(method);
+                    } catch (URISyntaxException e) {
+                        continue;
+                    }
+                    if (uri.isAbsolute() && UrlConstants.HTTPS_SCHEME.equals(uri.getScheme())) {

[Mathieu, 2017/03/03 19:42:09]

in what case are  UrlConstants.HTTPS_SCHEME.equals(uri.getScheme()) and
method.startsWith(UrlConstants.HTTPS_URL_PREFIX) (line 102) different?

[please use gerrit instead, 2017/03/09 18:05:33]

Good point. Using assert instead, as Ganggui recommends.

+                        mPaymentMethods.add(uri);
+                    }
+                }
+            }
         }
 
-        Map<String, AndroidPaymentApp> installedApps = new HashMap<>();
-        PackageManager pm = context.getPackageManager();
-        Intent payIntent = new Intent();
+        /** Looks for native Android payment apps. */
+        public void find() {
+            PackageManager pm = ContextUtils.getApplicationContext().getPackageManager();
+            Intent payIntent = new Intent();
+            payIntent.setAction(AndroidPaymentApp.ACTION_PAY);
+
+            List<PaymentManifestVerifier> verifiers = new ArrayList<>();
+            for (URI methodName : mPaymentMethods) {
+                payIntent.setData(Uri.parse(methodName.toString()));
+                List<ResolveInfo> apps = pm.queryIntentActivities(payIntent, 0);
+                if (apps.isEmpty()) continue;
+
+                if (mParser == null) {
+                    mParser = new PaymentManifestParser();
+                    mParser.startUtilityProcess();

[Mathieu, 2017/03/03 19:42:09]

Add a comment that we start the utility process as soon as possible.

[please use gerrit instead, 2017/03/09 18:05:33]

Done.

+                }
+
+                verifiers.add(
+                        new PaymentManifestVerifier(mWebContents, methodName, apps, mParser, this));
+                mPendingApps.put(methodName, new HashSet<>(apps));
+                if (verifiers.size() == MAX_NUMBER_OF_MANIFESTS) break;
+            }
 

[Mathieu, 2017/03/03 19:42:09]

Add a comment that we do not verify app manifests for basic-card support

[please use gerrit instead, 2017/03/09 18:05:33]

Done.

-        for (String methodName : methods) {
-            if (methodName.startsWith(UrlConstants.HTTPS_URL_PREFIX)) {
-                payIntent.setAction(AndroidPaymentApp.ACTION_PAY);
-                payIntent.setData(Uri.parse(methodName));
-            } else if (methodName.equals(BASIC_CARD_PAYMENT_METHOD)) {
+            if (mQueryBasicCard) {
                 payIntent.setAction(ACTION_PAY_BASIC_CARD);

[Mathieu, 2017/03/03 19:42:09]

reusing the same intent is prone to bugs (what if some fields get set above and
we forget to change them here?). Is it expensive to create a new Intent object?

[please use gerrit instead, 2017/03/09 18:05:33]

Creating new objects is expensive on Android, so I would not do it inside of a
loop. However, one more Intent object should have small impact and sizable
maintainability improvement, so following your advice here.

                 payIntent.setData(null);
-            } else {
-                continue;
+                List<ResolveInfo> apps = pm.queryIntentActivities(payIntent, 0);
+                for (int i = 0; i < apps.size(); i++) {
+                    onValidPaymentApp(BASIC_CARD_PAYMENT_METHOD, apps.get(i));
+                }
             }
 
-            List<ResolveInfo> matches = pm.queryIntentActivities(payIntent, 0);
-            for (int i = 0; i < matches.size(); i++) {
-                ResolveInfo match = matches.get(i);
-                String packageName = match.activityInfo.packageName;
-                // Do not recommend disabled apps.
-                if (!PaymentPreferencesUtil.isAndroidPaymentAppEnabled(packageName)) continue;
-                AndroidPaymentApp installedApp = installedApps.get(packageName);
-                if (installedApp == null) {
-                    CharSequence label = match.loadLabel(pm);
-                    installedApp =
-                            new AndroidPaymentApp(webContents, packageName, match.activityInfo.name,
-                                    label == null ? "" : label.toString(), match.loadIcon(pm));
-                    callback.onPaymentAppCreated(installedApp);
-                    installedApps.put(packageName, installedApp);
-                }
-                installedApp.addMethodName(methodName);
+            if (verifiers.isEmpty()) {
+                onSearchFinished();

[Mathieu, 2017/03/03 19:42:09]

would something like mCallback.onAllPaymentAppsCreated(); be good here?

[please use gerrit instead, 2017/03/09 18:05:33]

onSearchFinished() is better because it checks for READY_TO_PAY services in
"basic-card" apps as well.

+                return;
+            }
+
+            for (int i = 0; i < verifiers.size(); i++) {
+                verifiers.get(i).verify();

[Mathieu, 2017/03/03 19:42:09]

would it be better to start verifying as soon as the verifier is added to
|verifiers|?

[please use gerrit instead, 2017/03/09 18:05:33]

That would be prone to a race condition. Suppose you have 2 payment manifests
that should be verified: https://alicepay.com and https://bobpay.com. Chrome
adds https://alicepay.com verifier to the list of verifiers and the list of
pending apps and starts verification. Then suppose for whatever reason the
verification was extremely fast and finished even before the verifier for
https://bobpay.com was created. The code in removePendingApp() removes
https://alicepay.com from the list of pending apps and sees that the list of
pending apps is now empty. This indicates that all verifiers finished. So
onSearchFinished() is called. However, the code still has not started verifying
https://bobpay.com yet. 


Granted, this probably will not happen because verification is asynchronous,
while creation of verifiers is synchronous, it's a common enough practice to
follow the pattern in this code that I'd rather prefer to be safe than sorry :-)

+            }
+        }
+
+        @Override
+        public void onValidPaymentApp(URI methodName, ResolveInfo resolveInfo) {
+            onValidPaymentApp(methodName.toString(), resolveInfo);
+            removePendingApp(methodName, resolveInfo);
+        }
+
+        /** Same as above, but also works for non-URI method names, e.g., "basic-card". */
+        private void onValidPaymentApp(String methodName, ResolveInfo resolveInfo) {
+            PackageManager pm = ContextUtils.getApplicationContext().getPackageManager();
+            String packageName = resolveInfo.activityInfo.packageName;
+            AndroidPaymentApp app = mResult.get(packageName);
+            if (app == null) {
+                CharSequence label = resolveInfo.loadLabel(pm);
+                app = new AndroidPaymentApp(mWebContents, packageName,
+                        resolveInfo.activityInfo.name, label == null ? "" : label.toString(),
+                        resolveInfo.loadIcon(pm));
+                mResult.put(packageName, app);
             }
+            app.addMethodName(methodName);
+        }
+
+        @Override
+        public void onInvalidPaymentApp(URI methodName, ResolveInfo resolveInfo) {
+            removePendingApp(methodName, resolveInfo);
+        }
+
+        /** Removes the (method, app) pair from the list of pending information to be verified. */
+        private void removePendingApp(URI methodName, ResolveInfo resolveInfo) {

[Mathieu, 2017/03/03 19:42:09]

shouldn't private methods be at the bottom, or perhaps I'm wrong/outdated on the
style

[please use gerrit instead, 2017/03/09 18:05:33]

Java methods follow "natural" method ordering. That's kind of vague, but I take
it to mean "place utility methods immediately below the last method where
they're used." onInvalidPaymentApp() is the last method to call
removePendingApp(), hence I placed removePendingApp() immediately below
onInvalidPaymentApp().

+            Set<ResolveInfo> pendingAppsForMethod = mPendingApps.get(methodName);
+            pendingAppsForMethod.remove(resolveInfo);
+            if (pendingAppsForMethod.isEmpty()) mPendingApps.remove(methodName);
+            if (mPendingApps.isEmpty()) onSearchFinished();
         }
 
-        List<ResolveInfo> matches = pm.queryIntentServices(new Intent(ACTION_IS_READY_TO_PAY), 0);
-        for (int i = 0; i < matches.size(); i++) {
-            ResolveInfo match = matches.get(i);
-            String packageName = match.serviceInfo.packageName;
-            AndroidPaymentApp installedApp = installedApps.get(packageName);
-            if (installedApp != null) installedApp.setIsReadyToPayAction(match.serviceInfo.name);
+        @Override
+        public void onInvalidManifest(URI methodName) {
+            mPendingApps.remove(methodName);
+            if (mPendingApps.isEmpty()) onSearchFinished();
         }
 
-        callback.onAllPaymentAppsCreated();
+        /**
+         * Checks for IS_READY_TO_PAY service in each valid payment app and returns the valid apps
+         * to the caller. Called when finished verifying all payment methods and apps.
+         */
+        private void onSearchFinished() {
+            if (mParser != null) {
+                mParser.stopUtilityProcess();
+                mParser = null;
+            }
+
+            PackageManager pm = ContextUtils.getApplicationContext().getPackageManager();
+            List<ResolveInfo> resolveInfos =
+                    pm.queryIntentServices(new Intent(ACTION_IS_READY_TO_PAY), 0);
+            for (int i = 0; i < resolveInfos.size(); i++) {
+                ResolveInfo resolveInfo = resolveInfos.get(i);
+                AndroidPaymentApp app = mResult.get(resolveInfo.serviceInfo.packageName);
+                if (app != null) app.setIsReadyToPayAction(resolveInfo.serviceInfo.name);
+            }
+
+            for (Map.Entry<String, AndroidPaymentApp> entry : mResult.entrySet()) {
+                mCallback.onPaymentAppCreated(entry.getValue());
+            }
+
+            mCallback.onAllPaymentAppsCreated();
+        }
     }
 
     /**