Update WebAPKs even if the WebAPK start URL has no Web Manifest part 1/3

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 3020fae90ce1c6d949f7fad5e14e10de6b0a83ed..1a80a90f8abfeeaf785a91eca81f361f768888b1 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
@@ -5,26 +5,38 @@
 package org.chromium.chrome.browser.webapps;
 
 import android.graphics.Bitmap;
-import android.os.Bundle;
 import android.text.TextUtils;
 
 import org.chromium.base.Log;
-import org.chromium.chrome.browser.ShortcutHelper;
 import org.chromium.chrome.browser.tab.Tab;
-import org.chromium.chrome.browser.util.IntentUtils;
 import org.chromium.chrome.browser.util.UrlUtilities;
-import org.chromium.webapk.lib.common.WebApkMetaDataKeys;
 
 /**
  * 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 {
-    /**
-     * Called when the process of checking Web Manifest update is complete.
-     */
     public interface Callback {
-        public void onUpgradeNeededCheckFinished(boolean needsUpgrade, FetchedManifestData data);
+        /**
+         * Called when the Web Manifest for the initial URL load has been fetched (successfully or
+         * unsuccessfully).
+         * TODO(pkotwicz): Add calls to {@link #onFinishedFetchingWebManifestForInitialUrl}.
+         * @param needsUpgrade Whether the WebAPK should be updated because the Web Manifest has
+         *        changed. False if the Web Manifest could not be fetched.
+         * @param data The fetched Web Manifest data. Null if the initial URL does not point to a

[dominickn, 2016/10/24 11:34:59]

Nit: align "Whether" and "The" on the param lines to make reading this easier.

+         *        Web Manifest.
+         */
+        void onFinishedFetchingWebManifestForInitialUrl(
+                boolean needsUpgrade, FetchedManifestData data);
+
+        /**
+         * Called when the Web Manifest has been successfully fetched (including on the initial URL
+         * load).
+         * @param needsUpgrade Whether the WebAPK should be updated because the Web Manifest has
+         *        changed.
+         * @param data The fetched Web Manifest data.
+         */
+        void onGotManifestData(boolean needsUpgrade, FetchedManifestData data);
     }
 
     private static final String TAG = "cr_UpgradeDetector";
@@ -56,11 +68,7 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
     /**
      * Web Manifest data at time that the WebAPK was generated.
      */
-    private WebappInfo mWebappInfo;
-    private String mManifestUrl;
-    private String mStartUrl;
-    private String mIconUrl;
-    private String mIconMurmur2Hash;
+    private WebApkMetaData mMetaData;
 
     /**
      * Fetches the WebAPK's Web Manifest from the web.
@@ -77,19 +85,10 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
      * @param metadata Metadata from WebAPK's Android Manifest.
      * @param callback Called once it has been determined whether the WebAPK needs to be upgraded.
      */
-    public ManifestUpgradeDetector(Tab tab, WebappInfo info, Bundle metadata, Callback callback) {
+    public ManifestUpgradeDetector(Tab tab, WebApkMetaData metaData, Callback callback) {
         mTab = tab;
-        mWebappInfo = info;
+        mMetaData = metaData;
         mCallback = callback;
-        parseMetaData(metadata);
-    }
-
-    public String getManifestUrl() {
-        return mManifestUrl;
-    }
-
-    public String getWebApkPackageName() {
-        return mWebappInfo.webApkPackageName();
     }
 
     /**
@@ -98,11 +97,11 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
     public boolean start() {
         if (mFetcher != null) return false;
 
-        if (TextUtils.isEmpty(mManifestUrl)) {
+        if (TextUtils.isEmpty(mMetaData.manifestUrl)) {
             return false;
         }
 
-        mFetcher = createFetcher(mTab, mWebappInfo.scopeUri().toString(), mManifestUrl);
+        mFetcher = createFetcher(mTab, mMetaData.scope, mMetaData.manifestUrl);
         mFetcher.start(this);
         return true;
     }
@@ -115,14 +114,6 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
         return new ManifestUpgradeDetectorFetcher(tab, scopeUrl, manifestUrl);
     }
 
-    private void parseMetaData(Bundle metadata) {
-        mManifestUrl = IntentUtils.safeGetString(metadata, WebApkMetaDataKeys.WEB_MANIFEST_URL);
-        mStartUrl = IntentUtils.safeGetString(metadata, WebApkMetaDataKeys.START_URL);
-        mIconUrl = IntentUtils.safeGetString(metadata, WebApkMetaDataKeys.ICON_URL);
-        mIconMurmur2Hash = Long.toString(WebApkMetaDataUtils.getLongFromMetaData(
-                metadata, WebApkMetaDataKeys.ICON_MURMUR2_HASH, 0));
-    }
-
     /**
      * Puts the object in a state where it is safe to be destroyed.
      */
@@ -143,10 +134,6 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
         mFetcher.destroy();
         mFetcher = null;
 
-        if (TextUtils.isEmpty(scopeUrl)) {
-            scopeUrl = ShortcutHelper.getScopeFromUrl(startUrl);
-        }
-
         FetchedManifestData fetchedData = new FetchedManifestData();
         fetchedData.startUrl = startUrl;
         fetchedData.scopeUrl = scopeUrl;
@@ -163,34 +150,34 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
         // TODO(hanxi): crbug.com/627824. Validate whether the new fetched data is
         // WebAPK-compatible.
         boolean upgrade = needsUpgrade(fetchedData);
-        mCallback.onUpgradeNeededCheckFinished(upgrade, fetchedData);
+        mCallback.onGotManifestData(upgrade, fetchedData);
     }
 
     /**
      * Checks whether the WebAPK needs to be upgraded provided the fetched manifest data.
      */
     private boolean needsUpgrade(FetchedManifestData fetchedData) {
-        if (!urlsMatchIgnoringFragments(mIconUrl, fetchedData.iconUrl)
-                || !mIconMurmur2Hash.equals(fetchedData.iconMurmur2Hash)) {
+        if (!urlsMatchIgnoringFragments(mMetaData.iconUrl, fetchedData.iconUrl)
+                || !mMetaData.iconMurmur2Hash.equals(fetchedData.iconMurmur2Hash)) {
             return true;
         }
 
-        if (!urlsMatchIgnoringFragments(mWebappInfo.scopeUri().toString(), fetchedData.scopeUrl)) {
+        if (!urlsMatchIgnoringFragments(mMetaData.scope, fetchedData.scopeUrl)) {
             // 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(),
+                    + "the one fetched from Web Manifest(%s).", mMetaData.scope,
                     fetchedData.scopeUrl);
             return true;
         }
 
-        if (!urlsMatchIgnoringFragments(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) {
+        if (!urlsMatchIgnoringFragments(mMetaData.startUrl, fetchedData.startUrl)
+                || !TextUtils.equals(mMetaData.shortName, fetchedData.shortName)
+                || !TextUtils.equals(mMetaData.name, fetchedData.name)
+                || mMetaData.backgroundColor != fetchedData.backgroundColor
+                || mMetaData.themeColor != fetchedData.themeColor
+                || mMetaData.orientation != fetchedData.orientation
+                || mMetaData.displayMode != fetchedData.displayMode) {
             return true;
         }