[Payments] Cache payment manifests.

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 e3d4db7610798cb4e62bc732b3321090432775ec..7304de30551f09e0e9a6b440e360da44010cc338 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
@@ -7,6 +7,7 @@ package org.chromium.chrome.browser.payments;
 import android.content.pm.PackageInfo;
 import android.content.pm.ResolveInfo;
 import android.content.pm.Signature;
+import android.support.annotation.Nullable;
 
 import org.chromium.base.Log;
 import org.chromium.chrome.browser.UrlConstants;
@@ -20,6 +21,7 @@ import java.net.URI;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Formatter;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -35,7 +37,9 @@ import java.util.Set;
  * Spec:
  * https://docs.google.com/document/d/1izV4uC-tiRJG3JLooqY3YRLU22tYOsLTNq0P_InPJeE/edit#heading=h.cjp3jlnl47h5
  */
-public class PaymentManifestVerifier implements ManifestDownloadCallback, ManifestParseCallback {
+public class PaymentManifestVerifier
+        implements ManifestDownloadCallback, ManifestParseCallback,
+                   PaymentManifestWebDataService.PaymentManifestWebDataServiceCallback {
     private static final String TAG = "cr_PaymentManifest";
 
     /** Interface for the callback to invoke when finished verification. */
@@ -67,6 +71,12 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
          * @param methodName The payment method name that has an invalid payment method manifest.
          */
         void onInvalidManifest(URI methodName);
+
+        /**
+         * Called when all the operations are done. After this call, the caller can release
+         * resources used by this class.

[please use gerrit instead, 2017/04/26 15:00:44]

@param

[gogerald1, 2017/04/26 17:30:31]

Done.

+         */
+        void onVerifyFinished(PaymentManifestVerifier verifier);
     }
 
     /** Identifying information about an installed native Android payment app. */
@@ -90,6 +100,16 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
     /** A mapping from the package name to the application that matches the method name.  */
     private final Map<String, AppInfo> mMatchingApps;
 
+    /** A list of package names of the apps have been verified by using cached manifest. */
+    private final ArrayList<String> mVerifiedAppsPackageNamesByCachedManifest;

[please use gerrit instead, 2017/04/26 15:00:44]

1) List<String> instead of ArrayList<String>.
2) AppPackageNames instead of AppsPackageNames. (No "s" in "App".)

[gogerald1, 2017/04/26 17:30:31]

Done.

+
+    /** A set of package names of the apps support the verified method. */

[please use gerrit instead, 2017/04/26 15:00:44]

Please correct the grammar here. I don't understand this sentence.

[gogerald1, 2017/04/26 17:30:30]

Done.

+    private final Set<String> mSupportedAppsPackageNames;

[please use gerrit instead, 2017/04/26 15:00:44]

AppPackageNames instead of AppsPackageNames. (No "s" in "App".)

[gogerald1, 2017/04/26 17:30:31]

Done.

+
+    /** A list of manifests of the apps support the verified method. */

[please use gerrit instead, 2017/04/26 15:00:44]

Please correct grammar here as well.

[gogerald1, 2017/04/26 17:30:31]

Done.

+    private final ArrayList<WebAppManifestSection[]> mSupportedAppsParsedManifest;

[please use gerrit instead, 2017/04/26 15:00:44]

List<String> instead of ArrayList<String>.

[gogerald1, 2017/04/26 17:30:31]

Done.

+
+    private final PaymentManifestWebDataService mWebDataService;
     private final PaymentManifestParser mParser;
     private final PackageManagerDelegate mPackageManagerDelegate;
     private final ManifestVerifyCallback mCallback;
@@ -97,6 +117,9 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
     private int mPendingWebAppManifestsCount;
     private boolean mAtLeastOneManifestFailedToDownloadOrParse;
 
+    /** A flag indicates whether the manifest cache is stale or unusable. */

[please use gerrit instead, 2017/04/26 15:00:44]

/** Whether the manifest cache is stale (unusable). */

[gogerald1, 2017/04/26 17:30:30]

Done.

+    private boolean mIsManifestCacheStaleOrUnusable;
+
     /**
      * Builds the manifest verifier.
      *
@@ -104,14 +127,16 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
      *                               Must be an absolute URI with HTTPS scheme.
      * @param matchingApps           The identifying information for the native Android payment apps
      *                               that offer to handle this payment method.
+     * @param webDataService         The web data service to cache manifest.
      * @param downloader             The manifest downloader.
      * @param parser                 The manifest parser.
      * @param packageManagerDelegate The package information retriever.
      * @param callback               The callback to be notified of verification result.
      */
     public PaymentManifestVerifier(URI methodName, List<ResolveInfo> matchingApps,
-            PaymentManifestDownloader downloader, PaymentManifestParser parser,
-            PackageManagerDelegate packageManagerDelegate, ManifestVerifyCallback callback) {
+            PaymentManifestWebDataService webDataService, PaymentManifestDownloader downloader,
+            PaymentManifestParser parser, PackageManagerDelegate packageManagerDelegate,
+            ManifestVerifyCallback callback) {
         assert methodName.isAbsolute();
         assert UrlConstants.HTTPS_SCHEME.equals(methodName.getScheme());
         assert !matchingApps.isEmpty();
@@ -125,6 +150,7 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
         }
 
         mDownloader = downloader;
+        mWebDataService = webDataService;
         mParser = parser;
         mPackageManagerDelegate = packageManagerDelegate;
         mCallback = callback;
@@ -137,6 +163,10 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
             Log.d(TAG, "Unable to generate SHA-256 hashes.");
         }
         mMessageDigest = md;
+
+        mVerifiedAppsPackageNamesByCachedManifest = new ArrayList<String>();

[please use gerrit instead, 2017/04/26 15:00:44]

<> instead of <String>

[gogerald1, 2017/04/26 17:30:31]

Done.

+        mSupportedAppsPackageNames = new HashSet<String>();

[please use gerrit instead, 2017/04/26 15:00:44]

Ditto

[gogerald1, 2017/04/26 17:30:31]

Done.

+        mSupportedAppsParsedManifest = new ArrayList<WebAppManifestSection[]>();

[please use gerrit instead, 2017/04/26 15:00:44]

<> instead of <WebAppManifestSection[]>

[gogerald1, 2017/04/26 17:30:31]

Done.

     }
 
     /**
@@ -145,7 +175,7 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
      */
     public void verify() {
         if (mMessageDigest == null) {
-            mCallback.onInvalidManifest(mMethodName);
+            finishVerify(true);
             return;
         }
 
@@ -177,7 +207,16 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
             mMatchingApps.remove(invalidAppsToRemove.get(i));
         }
 
-        if (!mMatchingApps.isEmpty()) mDownloader.downloadPaymentMethodManifest(mMethodName, this);
+        if (mMatchingApps.isEmpty()) {
+            finishVerify(true);
+            return;
+        }
+
+        // Try to fetch manifest from the cache first.
+        if (!mWebDataService.getPaymentMethodManifest(mMethodName.toString(), this)) {
+            mIsManifestCacheStaleOrUnusable = true;
+            mDownloader.downloadPaymentMethodManifest(mMethodName, this);
+        }
     }
 
     /**
@@ -201,6 +240,64 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
     }
 
     @Override
+    public void onPaymentMethodManifestFetched(String[] appsPackageNames) {

[please use gerrit instead, 2017/04/26 15:00:44]

appPackageNames instead of appsPackageNames (no "s" in "app").

[gogerald1, 2017/04/26 17:30:30]

Done.

+        Set<String> fetchedApps = new HashSet<>(Arrays.asList(appsPackageNames));
+        Set<String> matchingApps = mMatchingApps.keySet();
+        // The cache may be stale if it doesn't contain all matching apps, so switch to download the
+        // manifest online immediately.
+        if (!fetchedApps.containsAll(matchingApps)) {
+            mIsManifestCacheStaleOrUnusable = true;
+            mDownloader.downloadPaymentMethodManifest(mMethodName, this);
+            return;
+        }
+
+        mPendingWebAppManifestsCount = matchingApps.size();
+        for (int i = 0; i < appsPackageNames.length; i++) {
+            if (!mWebDataService.getPaymentWebAppManifest(appsPackageNames[i], this)) {
+                mIsManifestCacheStaleOrUnusable = true;
+                mPendingWebAppManifestsCount = 0;
+                mDownloader.downloadPaymentMethodManifest(mMethodName, this);
+                return;
+            }
+        }
+    }
+
+    @Override
+    public void onPaymentWebAppManifestFetched(WebAppManifestSection[] manifest) {
+        if (mIsManifestCacheStaleOrUnusable) return;
+
+        if (manifest == null || manifest.length == 0) {
+            mIsManifestCacheStaleOrUnusable = true;
+            mPendingWebAppManifestsCount = 0;
+            mDownloader.downloadPaymentMethodManifest(mMethodName, this);
+            return;
+        }
+
+        String verifiedAppPackageName = verifyAppWithWebAppManifest(manifest);
+        if (verifiedAppPackageName != null) {
+            // Do not notify onValidPaymentApp immediately in case of fetching the other web app's
+            // manifest failed. Switch to download manifest online in that case immediately.
+            mVerifiedAppsPackageNamesByCachedManifest.add(verifiedAppPackageName);
+        }
+
+        mPendingWebAppManifestsCount--;
+        if (mPendingWebAppManifestsCount != 0) return;
+
+        for (int i = 0; i < mVerifiedAppsPackageNamesByCachedManifest.size(); i++) {
+            String appPackageName = mVerifiedAppsPackageNamesByCachedManifest.get(i);
+            mCallback.onValidPaymentApp(mMethodName, mMatchingApps.get(appPackageName).resolveInfo);
+            mMatchingApps.remove(appPackageName);
+        }
+
+        for (Map.Entry<String, AppInfo> entry : mMatchingApps.entrySet()) {
+            mCallback.onInvalidPaymentApp(mMethodName, entry.getValue().resolveInfo);
+        }
+
+        // Download and parse manifest to refresh cache.
+        mDownloader.downloadPaymentMethodManifest(mMethodName, this);
+    }
+
+    @Override
     public void onPaymentMethodManifestDownloadSuccess(String content) {
         mParser.parsePaymentMethodManifest(content, this);
     }
@@ -233,6 +330,46 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
 
         if (mAtLeastOneManifestFailedToDownloadOrParse) return;
 
+        for (int i = 0; i < manifest.length; i++) {
+            mSupportedAppsPackageNames.add(manifest[i].id);
+        }
+        mSupportedAppsParsedManifest.add(manifest);
+
+        // Do not verify payment app if it has already been verified by cached manifest.
+        if (mIsManifestCacheStaleOrUnusable) {
+            String verifiedAppPackageName = verifyAppWithWebAppManifest(manifest);
+            if (verifiedAppPackageName != null) {
+                mCallback.onValidPaymentApp(
+                        mMethodName, mMatchingApps.get(verifiedAppPackageName).resolveInfo);
+                mMatchingApps.remove(verifiedAppPackageName);
+            }
+        }
+
+        mPendingWebAppManifestsCount--;
+        if (mPendingWebAppManifestsCount != 0) return;
+
+        // Do not notify onInvalidPaymentApp if it has already be notified when checking by cached
+        // manifest.
+        if (mIsManifestCacheStaleOrUnusable) {
+            for (Map.Entry<String, AppInfo> entry : mMatchingApps.entrySet()) {
+                mCallback.onInvalidPaymentApp(mMethodName, entry.getValue().resolveInfo);
+            }
+        }
+
+        // Cache supported apps package names.

[please use gerrit instead, 2017/04/26 15:00:43]

Make sure to cache only if all manifests downloaded successfully. We don't want
to store partially correct data on disk. (Partially correct is slightly
corrupted.)

[gogerald1, 2017/04/26 17:30:31]

This is guaranteed by mPendingWebAppManifestsCount and
mAtLeastOneManifestFailedToDownloadOrParse

+        mWebDataService.addPaymentMethodManifest(mMethodName.toString(),
+                mSupportedAppsPackageNames.toArray(new String[mSupportedAppsPackageNames.size()]));
+
+        // Cache parsed apps manifests.
+        for (int i = 0; i < mSupportedAppsParsedManifest.size(); i++) {
+            mWebDataService.addPaymentWebAppManifest(mSupportedAppsParsedManifest.get(i));
+        }
+
+        finishVerify(false);
+    }
+
+    @Nullable
+    private String verifyAppWithWebAppManifest(WebAppManifestSection[] manifest) {
         List<Set<String>> sectionsFingerprints = new ArrayList<>();
         for (int i = 0; i < manifest.length; i++) {
             WebAppManifestSection section = manifest[i];
@@ -249,18 +386,11 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
             if (appInfo != null && appInfo.version >= section.minVersion
                     && appInfo.sha256CertFingerprints != null
                     && appInfo.sha256CertFingerprints.equals(sectionsFingerprints.get(i))) {
-                mCallback.onValidPaymentApp(mMethodName, appInfo.resolveInfo);
-                mMatchingApps.remove(section.id);
-                break;
+                return section.id;
             }
         }
 
-        mPendingWebAppManifestsCount--;
-        if (mPendingWebAppManifestsCount == 0) {
-            for (Map.Entry<String, AppInfo> entry : mMatchingApps.entrySet()) {
-                mCallback.onInvalidPaymentApp(mMethodName, entry.getValue().resolveInfo);
-            }
-        }
+        return null;
     }
 
     @Override
@@ -268,7 +398,8 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
         if (mAtLeastOneManifestFailedToDownloadOrParse) return;
         mAtLeastOneManifestFailedToDownloadOrParse = true;
 
-        mCallback.onInvalidManifest(mMethodName);
+        // Do not notify onInvalidManifest if the cached payment manifest is valid.
+        finishVerify(mIsManifestCacheStaleOrUnusable);
     }
 
     @Override
@@ -276,6 +407,12 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
         if (mAtLeastOneManifestFailedToDownloadOrParse) return;
         mAtLeastOneManifestFailedToDownloadOrParse = true;
 
-        mCallback.onInvalidManifest(mMethodName);
+        // Do not notify onInvalidManifest if the cached payment manifest is valid.
+        finishVerify(mIsManifestCacheStaleOrUnusable);
+    }
+
+    private void finishVerify(boolean invalidMethod) {
+        if (invalidMethod) mCallback.onInvalidManifest(mMethodName);
+        mCallback.onVerifyFinished(this);

[please use gerrit instead, 2017/04/26 15:00:43]

Including the comments, you use 12 lines for finishVerify() method and calls to
it. If you inline the calls instead, you will use 9 lines of code without the
need for clarifying comments. In addition, a boolean parameter is hard to
understand for engineers that maintain your code and, therefore, likely to
introduce bugs. Let's not use this method, but inline the function instead.

[gogerald1, 2017/04/26 17:30:31]

Done.

     }
 }