Update WebAPK if homescreen icon changed.

chrome/android/java/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetector.java

Index: chrome/android/java/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetector.java
diff --git a/chrome/android/java/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetector.java b/chrome/android/java/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetector.java
index 5ac4b9d5f0987380001ce309a25499f6c15d60cc..85f673821085910d6d54aa9aa02fa266059d9d69 100644
--- a/chrome/android/java/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetector.java
+++ b/chrome/android/java/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetector.java
@@ -6,18 +6,22 @@ package org.chromium.chrome.browser.webapps;
 
 import android.content.pm.ApplicationInfo;
 import android.content.pm.PackageManager;
+import android.graphics.Bitmap;
 import android.net.Uri;
+import android.os.Bundle;
 import android.text.TextUtils;
 
 import org.chromium.base.ContextUtils;
 import org.chromium.base.Log;
+import org.chromium.chrome.browser.ShortcutHelper;
 import org.chromium.chrome.browser.tab.Tab;
 
 /**
  * This class checks whether the WebAPK needs to be re-installed and sends a request to re-install
  * the WebAPK if it needs to be re-installed.
  */
-public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.Callback {
+public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.Callback,
+                                                ShortcutHelper.FetchHomescreenImageCallback {
     /**
      * The names of <meta-data> in the WebAPK's AndroidManifest.xml whose values are needed to
      * determine whether a WebAPK needs to be upgraded but which are not present in
@@ -25,20 +29,70 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
      * {@linkorg.chromium.webapk.lib.runtime_library.HostBrowserLauncher}.
      */
     public static final String META_DATA_START_URL = "org.chromium.webapk.shell_apk.startUrl";
+    public static final String META_DATA_ICON_URL = "org.chromium.webapk.shell_apk.iconUrl";
+    public static final String META_DATA_ICON_MURMUR2_HASH =
+            "org.chromium.webapk.shell_apk.iconMurmur2Hash";
+
+    private static final String TAG = "cr_UpgradeDetector";
+
+    /**
+     * Fetched Web Manifest data.
+     */
+    private static class FetchedManifestData {
+        public String startUrl;
+        public String scopeUrl;
+        public String name;
+        public String shortName;
+        public String iconUrl;
+
+        // Hash of untransformed icon bytes. The hash should have been taken prior to any
+        // encoding/decoding.
+        public long iconMurmur2Hash;
+
+        public Bitmap icon;
+        public int displayMode;
+        public int orientation;
+        public long themeColor;
+        public long backgroundColor;
+    }
 
     /** The WebAPK's tab. */
     private final Tab mTab;
 
+    /**
+     * Web Manifest data at time that the WebAPK was generated.
+     */
     private WebappInfo mWebappInfo;
-
     private String mStartUrl;
+    private String mIconUrl;
+    private long mIconMurmur2Hash;
 
     /**
      * Fetches the WebAPK's Web Manifest from the web.
      */
     private ManifestUpgradeDetectorFetcher mFetcher;
 
-    private static final String TAG = "cr_UpgradeDetector";
+    /**
+     * Fetched Web Manifest data.
+     */
+    private FetchedManifestData mFetchedData;
+
+    /**
+     * Gets the long value from a Bundle. The long should be terminated with 'L'. This function is
+     * more reliable than {@link Bundle#getLong()} which returns 0 if the value is below
+     * Float.MAX_VALUE. Returns 0 if the value could not be parsed.
+     */
+    private static long getLongFromBundle(Bundle bundle, String key) {
+        String value = bundle.getString(key);
+        if (value == null || !value.endsWith("L")) {
+            return 0;
+        }
+        try {
+            return Long.parseLong(value.substring(0, value.length() - 1));
+        } catch (NumberFormatException e) {
+        }
+        return 0;
+    }
 
     public ManifestUpgradeDetector(Tab tab, WebappInfo info) {
         mTab = tab;
@@ -72,10 +126,13 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
 
     private void getMetaDataFromAndroidManifest() {
         try {
-            ApplicationInfo appinfo =
+            ApplicationInfo appInfo =
                     ContextUtils.getApplicationContext().getPackageManager().getApplicationInfo(
                             mWebappInfo.webApkPackageName(), PackageManager.GET_META_DATA);
-            mStartUrl = appinfo.metaData.getString(META_DATA_START_URL);
+            Bundle metaData = appInfo.metaData;
+            mStartUrl = metaData.getString(META_DATA_START_URL);
+            mIconUrl = metaData.getString(META_DATA_ICON_URL);
+            mIconMurmur2Hash = getLongFromBundle(metaData, META_DATA_ICON_MURMUR2_HASH);
         } catch (PackageManager.NameNotFoundException e) {
             e.printStackTrace();
         }
@@ -96,17 +153,61 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
      */
     @Override
     public void onGotManifestData(String startUrl, String scopeUrl, String name, String shortName,
-            int displayMode, int orientation, long themeColor, long backgroundColor) {
+            String iconUrl, int displayMode, int orientation, long themeColor,
+            long backgroundColor) {
         mFetcher.destroy();
         mFetcher = null;
 
+        if (TextUtils.isEmpty(iconUrl)) {
+            /**
+             * The icon URL in the fetched manifest is empty when:
+             * - The Web Manifest specified an icon at the time that the WebAPK was generated but no
+             *   longer does.
+             * - The page's favicon was used as the WebAPK's homescreen icon when the WebAPK was

[dominickn, 2016/08/15 05:27:34]

Should a WebAPK be generated if we used a page's favicon instead of a manifest
icon? If we're using the same criteria as we do for app banners, then we
shouldn't generate WebAPKs for this. It would also greatly simplify the
ManifestUpgradeDetector because you would be guaranteed that a manifest icon was
used (and hence you could just run InstallableManager.GetData).

+             *   generated and the Web Manifest still does not specify any icons.
+             * For the sake of simplicity fetch the old homescreen icon to check whether it has
+             * changed. We do not support downgrading a WebAPK from using a custom icon (specified
+             * by the Web Manifest or by the page favicon) to not using an icon at all.
+             */
+            iconUrl = mIconUrl;
+        }
+
+        mFetchedData = new FetchedManifestData();
+        mFetchedData.startUrl = startUrl;
+        mFetchedData.scopeUrl = scopeUrl;
+        mFetchedData.name = name;
+        mFetchedData.shortName = shortName;
+        mFetchedData.iconUrl = iconUrl;
+        mFetchedData.displayMode = displayMode;
+        mFetchedData.orientation = orientation;
+        mFetchedData.themeColor = themeColor;
+        mFetchedData.backgroundColor = backgroundColor;
+
+        if (!TextUtils.isEmpty(iconUrl)) {
+            fetchHomescreenImage(iconUrl, this);
+        } else {
+            onFetchedHomescreenImage(null, 0);
+        }
+    }
+
+    /**
+     * Fetches the image at {@link imageUrl} and scales it so that it can be used on the homescreen.
+     * In dedicated function for the sake of tests.
+     */
+    protected void fetchHomescreenImage(
+            String imageUrl, ShortcutHelper.FetchHomescreenImageCallback callback) {
+        ShortcutHelper.fetchHomescreenImage(mTab.getWebContents(), imageUrl, callback);
+    }
+
+    @Override
+    public void onFetchedHomescreenImage(Bitmap bitmap, long murmur2Hash) {
+        mFetchedData.icon = bitmap;
+        mFetchedData.iconMurmur2Hash = murmur2Hash;
+
         // TODO(hanxi): crbug.com/627824. Validate whether the new WebappInfo is
         // WebAPK-compatible.
-        final WebappInfo newInfo = WebappInfo.create(mWebappInfo.id(), startUrl,
-                scopeUrl, mWebappInfo.encodedIcon(), name, shortName, displayMode, orientation,
-                mWebappInfo.source(), themeColor, backgroundColor, mWebappInfo.isIconGenerated(),
-                mWebappInfo.webApkPackageName(), mWebappInfo.webManifestUri().toString());
-        if (requireUpgrade(newInfo)) {
+
+        if (requireUpgrade(mFetchedData)) {
             upgrade();
         }
 
@@ -114,27 +215,37 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
     }
 
     /**
-     * Checks whether the WebAPK needs to be upgraded provided the new Web Manifest info.
+     * Checks whether the WebAPK needs to be upgraded provided the fetched manifest data.
      */
-    private boolean requireUpgrade(WebappInfo newInfo) {
-        boolean scopeMatch = mWebappInfo.scopeUri().equals(newInfo.scopeUri());
+    private boolean requireUpgrade(FetchedManifestData fetchedData) {
+        /**
+         * Only check whether icon URL differs if Chrome was able to fetch the bitmap at the icon
+         * URL (no 404).
+         */
+        if (fetchedData.icon != null) {
+            if (!TextUtils.equals(mIconUrl, fetchedData.iconUrl)
+                    || mIconMurmur2Hash != fetchedData.iconMurmur2Hash) {
+                return true;
+            }
+        }
+
+        boolean scopeMatch = mWebappInfo.scopeUri().toString().equals(fetchedData.scopeUrl);
         if (!scopeMatch) {
             // Sometimes the scope doesn't match due to a missing "/" at the end of the scope URL.
             // Print log to find such cases.
             Log.d(TAG, "Needs to request update since the scope from WebappInfo (%s) doesn't match"
                     + "the one fetched from Web Manifest(%s).", mWebappInfo.scopeUri().toString(),
-                    newInfo.scopeUri().toString());
+                    fetchedData.scopeUrl);
             return true;
         }
 
-        // TODO(hanxi): Add icon comparison.
-        if (!TextUtils.equals(mStartUrl, newInfo.uri().toString())
-                || !TextUtils.equals(mWebappInfo.shortName(), newInfo.shortName())
-                || !TextUtils.equals(mWebappInfo.name(), newInfo.name())
-                || mWebappInfo.backgroundColor() != newInfo.backgroundColor()
-                || mWebappInfo.themeColor() != newInfo.themeColor()
-                || mWebappInfo.orientation() != newInfo.orientation()
-                || mWebappInfo.displayMode() != newInfo.displayMode()) {
+        if (!TextUtils.equals(mStartUrl, fetchedData.startUrl)
+                || !TextUtils.equals(mWebappInfo.shortName(), fetchedData.shortName)
+                || !TextUtils.equals(mWebappInfo.name(), fetchedData.name)
+                || mWebappInfo.backgroundColor() != fetchedData.backgroundColor
+                || mWebappInfo.themeColor() != fetchedData.themeColor
+                || mWebappInfo.orientation() != fetchedData.orientation
+                || mWebappInfo.displayMode() != fetchedData.displayMode) {
             return true;
         }