Add calls to the server to request WebAPK updates.

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..3710ec0ef9c3ae294b6d39c69553fa7b748d7b70 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
@@ -19,6 +19,13 @@ import org.chromium.chrome.browser.tab.Tab;
  */
 public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.Callback {
     /**
+     * Called when the process of checking Web Manifest update is complete.
+     */
+    public interface CheckIfUpgradeNeededCallback {

[pkotwicz, 2016/08/11 22:02:43]

Nits:
- Rename this class to Callback .You can get away with such a simple name
because this class has only one callback
- I think that onUpdateNeededCheckFinished() is slightly clearer than
onCheckIfUpdateNeededFinished() because makes it clear that "the check has
finished".

[Xi Han, 2016/08/15 21:38:44]

Done.

+        public void onCheckIfUpdateNeededFinished(boolean isUpgraded, WebappInfo newInfo);

[pkotwicz, 2016/08/11 22:02:43]

Nit: "public" in interfaces is redundant. See
http://stackoverflow.com/questions/161633/should-methods-in-a-java-interface-...

Looks like I made this very mistake in ManifestUpgradeDetectorFetcher

[Xi Han, 2016/08/15 21:38:44]

Done.

+    }
+
+    /**
      * 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
      * {@link WebappInfo}. The names must stay in sync with
@@ -37,12 +44,20 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
      * Fetches the WebAPK's Web Manifest from the web.
      */
     private ManifestUpgradeDetectorFetcher mFetcher;
+    private CheckIfUpgradeNeededCallback mUpgradeCheckCallback;

[pkotwicz, 2016/08/11 22:02:43]

Nit: Just rename this member to "mCallback"

[Xi Han, 2016/08/15 21:38:44]

Done.

 
     private static final String TAG = "cr_UpgradeDetector";
 
-    public ManifestUpgradeDetector(Tab tab, WebappInfo info) {
+    public ManifestUpgradeDetector(Tab tab, WebappInfo info,
+            CheckIfUpgradeNeededCallback callback) {
         mTab = tab;
         mWebappInfo = info;
+        mUpgradeCheckCallback = callback;
+        getMetaDataFromAndroidManifest();
+    }
+
+    public boolean isWebContentsGone() {
+        return mTab == null || mTab.getWebContents() == null;

[pkotwicz, 2016/08/11 22:02:43]

Why are you checking whether |mTab| is null? Isn't it a bug to initialize
ManifestUpgradeDetector() with a null tab?

[Xi Han, 2016/08/15 21:38:44]

As discussed offline, move this check to |ManifestUpgradeDetectorFetcher|.

     }
 
     /**
@@ -56,7 +71,6 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
 
         if (mFetcher != null) return;
 
-        getMetaDataFromAndroidManifest();
         mFetcher = createFetcher(
                 mTab, mWebappInfo.scopeUri().toString(), mWebappInfo.webManifestUri().toString());
         mFetcher.start(this);
@@ -106,9 +120,9 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
                 scopeUrl, mWebappInfo.encodedIcon(), name, shortName, displayMode, orientation,
                 mWebappInfo.source(), themeColor, backgroundColor, mWebappInfo.isIconGenerated(),
                 mWebappInfo.webApkPackageName(), mWebappInfo.webManifestUri().toString());
-        if (requireUpgrade(newInfo)) {
-            upgrade();
-        }
+        boolean isUpgraded = requireUpgrade(newInfo);

[pkotwicz, 2016/08/11 22:02:43]

Nit: I think that |upgradeRequired| is a more appropriate name than |isUpgraded|
(Otherwise it would have been a super fast upgrade)

[Yaron, 2016/08/12 21:16:16]

same with the method name (I thought I commented on this on the last patchset)

[Xi Han, 2016/08/15 21:38:44]

Done.

+        mUpgradeCheckCallback.onCheckIfUpdateNeededFinished(
+                isUpgraded, isUpgraded ? newInfo : null);
 
         onComplete();

[Yaron, 2016/08/12 21:16:16]

i would remove this as well. the callback is likely to handle this logic now.

[Xi Han, 2016/08/15 21:38:44]

Hmm, this function is used by ManifestUpgradeDetectorTest. Since |upgrade()| is
also removed, remove the |onComplete| but add two local variables to track the
states.

     }
@@ -141,7 +155,5 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
         return false;
     }
 
-    protected void upgrade() {}
-
     protected void onComplete() {}
 }