[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 3d5f87c8d8dcbf6a9c9f8954930855c6ef75608f..48d77113c047e2857c14493e1f2e4e839e9641b5 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,19 +7,17 @@ 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;
-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.WebAppManifestSection;
 
 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 +33,10 @@ 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 PaymentManifestDownloader.ManifestDownloadCallback,
+                   PaymentManifestParser.ManifestParseCallback,
+                   PaymentManifestWebDataService.PaymentManifestWebDataServiceCallback {
     private static final String TAG = "cr_PaymentManifest";
 
     /** Interface for the callback to invoke when finished verification. */
@@ -90,6 +91,12 @@ 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 set of apps verified by using cached manifest. */

[please use gerrit instead, 2017/04/24 18:22:30]

Please clarify in the comment that these are app package names.

[gogerald1, 2017/04/26 13:46:31]

Done.

+    private final Set<String> mVerifiedAppsByCachedManifest;

[please use gerrit instead, 2017/04/24 18:22:30]

s/Apps/PackageNames/

[gogerald1, 2017/04/26 13:46:31]

Done.

+
+    /** A set of web apps Ids of the verified method supports. */

[please use gerrit instead, 2017/04/24 18:22:30]

Clarify the comment that these are app package names.

[gogerald1, 2017/04/26 13:46:31]

Done.

+    private final Set<String> mSupportedWebAppsIdsOfTheMethod;

[please use gerrit instead, 2017/04/24 18:22:31]

s/WebAppIds/PackageNames/

[gogerald1, 2017/04/26 13:46:31]

Done.

+
     private final PaymentManifestParser mParser;
     private final PackageManagerDelegate mPackageManagerDelegate;
     private final ManifestVerifyCallback mCallback;
@@ -98,6 +105,12 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
     private boolean mAtLeastOneManifestFailedToDownloadOrParse;
 
     /**
+     * A flag indicates whether there is a web app Id exists in the cache but failed to fetch

[please use gerrit instead, 2017/04/24 18:22:31]

"Fetch" is an overloaded term. Downloading can also be called fetching. But here
you're talking about fetching from cache. Use the phrase "fetch from cache"
instead.

[gogerald1, 2017/04/26 13:46:31]

Done.

+     * its manifest.
+     */
+    private boolean mAtLeastOneWebAppManifestFailedToFetch;

[please use gerrit instead, 2017/04/24 18:22:30]

mIsCacheStale might be a better name.

[gogerald1, 2017/04/26 13:46:30]

Done.

+
+    /**
      * Builds the manifest verifier.
      *
      * @param methodName             The name of the payment method name that apps offer to handle.
@@ -137,6 +150,9 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
             Log.d(TAG, "Unable to generate SHA-256 hashes.");
         }
         mMessageDigest = md;
+
+        mVerifiedAppsByCachedManifest = new HashSet<>();
+        mSupportedWebAppsIdsOfTheMethod = new HashSet<>();
     }
 
     /**
@@ -177,7 +193,16 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
             mMatchingApps.remove(invalidAppsToRemove.get(i));
         }
 
-        if (!mMatchingApps.isEmpty()) mDownloader.downloadPaymentMethodManifest(mMethodName, this);
+        if (mMatchingApps.isEmpty()) {
+            mCallback.onInvalidManifest(mMethodName);
+            return;
+        }
+
+        // Try to fetch manifest from the cache first.
+        if (!PaymentManifestWebDataService.getInstance().getPaymentMethodManifest(
+                    mMethodName.toString(), this)) {

[please use gerrit instead, 2017/04/24 18:22:30]

Can you also make sure that the cache is used only up to a certain number of
days? Let's 90 for now. If the cache is older than 90 days, clear it and
redownload the web app manifest.

[gogerald1, 2017/04/26 13:46:31]

No necessary now since we refresh cache after each PR

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

If download fails after every PR for 90 days, we should stop using the cache.

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

OKay, I will fix this in the next CL by introducing a preference to record
refreshing date.

+            mDownloader.downloadPaymentMethodManifest(mMethodName, this);
+        }

[please use gerrit instead, 2017/04/24 18:22:31]

Let's attempt a download regardless of cache state. The downloaded manifests
should be used to refresh the cache. Like this:

Get apps->Get cache->Success-------------------> Show UI.
       \           \->Fail----                 /
        \                     \               /
         ->Download manifests-> Update cache-

Does my "diagram" make sense?

[gogerald1, 2017/04/26 13:46:31]

I do not do cache check and manifest download in parallel since it's complicated
to sync them and cache check should be quick.
1) Check cache first.
2) If failed, then start download and parse manifest as before, update cache at
the end.
3) If success, notify caller to enable UI, then start download and parse
manifest to update cache without notifying caller again. 

     }
 
     /**
@@ -199,6 +224,59 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
     }
 
     @Override
+    public void onPaymentMethodManifestFetched(String[] webAppsIds) {

[please use gerrit instead, 2017/04/24 18:22:30]

What string identifies a web app?

[gogerald1, 2017/04/26 13:46:31]

Done.

+        Set<String> fetchedApps = new HashSet<>(Arrays.asList(webAppsIds));
+        Set<String> matchingApps = mMatchingApps.keySet();
+        // The cache may be outdated if it doesn't contain all matching apps, so download the
+        // manifest online.
+        if (!fetchedApps.containsAll(matchingApps)) {
+            mDownloader.downloadPaymentMethodManifest(mMethodName, this);
+            return;
+        }
+
+        mPendingWebAppManifestsCount = matchingApps.size();
+        for (String app : fetchedApps) {

[please use gerrit instead, 2017/04/24 18:22:31]

Let's iterate over webAppIds using "for (int i = 0..." type loop, because it's
more efficient than "for (String..." type loop.

[gogerald1, 2017/04/26 13:46:30]

Done.

+            if (!PaymentManifestWebDataService.getInstance().getPaymentWebAppManifest(app, this)) {
+                mPendingWebAppManifestsCount = 0;
+                mAtLeastOneWebAppManifestFailedToFetch = true;
+                mDownloader.downloadPaymentMethodManifest(mMethodName, this);
+                break;
+            }
+        }
+    }
+
+    @Override
+    public void onPaymentWebAppManifestFetched(WebAppManifestSection[] manifest) {
+        if (mAtLeastOneWebAppManifestFailedToFetch) return;
+
+        if (manifest == null || manifest.length == 0) {
+            mPendingWebAppManifestsCount = 0;
+            mAtLeastOneWebAppManifestFailedToFetch = true;
+            mDownloader.downloadPaymentMethodManifest(mMethodName, this);
+            return;
+        }
+
+        String verifiedApp = verifyAppWithWebAppManifest(manifest);

[please use gerrit instead, 2017/04/24 18:22:30]

What is this string? A package name?

[gogerald1, 2017/04/26 13:46:31]

Done. Yes, renamed the temp variable

+        if (verifiedApp != 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.
+            mVerifiedAppsByCachedManifest.add(verifiedApp);
+        }
+
+        mPendingWebAppManifestsCount--;
+        if (mPendingWebAppManifestsCount != 0) return;
+
+        for (String app : mVerifiedAppsByCachedManifest) {

[please use gerrit instead, 2017/04/24 18:22:30]

You only ever add thing to mVerifiedAppsByCachedManifest and then iterate over
it. An ArrayList would work better here, because then you would be able to use
the more efficient "for (int i =0..." type loop here instead.

[gogerald1, 2017/04/26 13:46:31]

Done.

+            mCallback.onValidPaymentApp(mMethodName, mMatchingApps.get(app).resolveInfo);
+            mMatchingApps.remove(app);
+        }
+
+        for (Map.Entry<String, AppInfo> entry : mMatchingApps.entrySet()) {
+            mCallback.onInvalidPaymentApp(mMethodName, entry.getValue().resolveInfo);
+        }
+    }
+
+    @Override
     public void onPaymentMethodManifestDownloadSuccess(String content) {
         mParser.parsePaymentMethodManifest(content, this);
     }
@@ -230,6 +308,31 @@ public class PaymentManifestVerifier implements ManifestDownloadCallback, Manife
 
         if (mAtLeastOneManifestFailedToDownloadOrParse) return;
 
+        for (int i = 0; i < manifest.length; i++) {
+            mSupportedWebAppsIdsOfTheMethod.add(manifest[i].id);
+        }
+
+        String verifiedApp = verifyAppWithWebAppManifest(manifest);
+        if (verifiedApp != null) {
+            mCallback.onValidPaymentApp(mMethodName, mMatchingApps.get(verifiedApp).resolveInfo);
+            mMatchingApps.remove(verifiedApp);
+        }
+
+        mPendingWebAppManifestsCount--;
+        if (mPendingWebAppManifestsCount != 0) return;
+
+        for (Map.Entry<String, AppInfo> entry : mMatchingApps.entrySet()) {
+            mCallback.onInvalidPaymentApp(mMethodName, entry.getValue().resolveInfo);
+        }
+
+        // Cache supported web apps Ids of the method.
+        PaymentManifestWebDataService.getInstance().addPaymentMethodManifest(mMethodName.toString(),
+                mSupportedWebAppsIdsOfTheMethod.toArray(
+                        new String[mSupportedWebAppsIdsOfTheMethod.size()]));
+    }
+
+    @Nullable
+    private String verifyAppWithWebAppManifest(WebAppManifestSection[] manifest) {
         List<Set<String>> sectionsFingerprints = new ArrayList<>();
         for (int i = 0; i < manifest.length; i++) {
             WebAppManifestSection section = manifest[i];
@@ -246,18 +349,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