Pass icon and icon murmur2 hash to WebApkInstaller when updating WebAPKs.

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 00891cb1b931f7d281f3bb54f14d0e0ba095b308..41ba8e3576dc02038bc0981d1fcca20aa0da3f1f 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
@@ -24,9 +24,7 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
      * Called when the process of checking Web Manifest update is complete.
      */

[pkotwicz, 2016/08/19 23:38:17]

Nit: Can you make this interface public?

I do not know what the convention is. My current principle is "not to use
package scope unless there is a reason for it" 

[Xi Han, 2016/08/26 17:54:08]

Done.

     interface Callback {
-        // TODO(hanxi): crbug.com/639000. Pass the icon url and icon murmur2 hash to the caller.
-        // Change the interface by using {@link FetchedManifestData} instead of {@link WebappInfo}.
-        public void onUpgradeNeededCheckFinished(boolean isUpgraded, WebappInfo newInfo);
+        public void onUpgradeNeededCheckFinished(boolean isUpgraded, FetchedManifestData data);
     }
 
     /**
@@ -47,7 +45,7 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
     /**
      * Fetched Web Manifest data.
      */

[pkotwicz, 2016/08/19 23:38:17]

Nit: "static class" -> "public static class" (My current principle is to avoid
package scope. I do not know whether this principle is correct)

[Xi Han, 2016/08/26 17:54:08]

I don't know, personally I would prefer to give something that is considered as
"internal" a scope that is enough for all its consumers.

-    private static class FetchedManifestData {
+    static class FetchedManifestData {
         public String startUrl;
         public String scopeUrl;
         public String name;
@@ -113,6 +111,10 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
         return mManifestUrl;
     }
 

[pkotwicz, 2016/08/19 23:38:17]

Nit: getWebApkPakcageName() -> getWebApkPackageName()

[Xi Han, 2016/08/26 17:54:08]

Sorry for the typo.

+    public String getWebApkPakcageName() {
+        return mWebappInfo.webApkPackageName();
+    }
+
     /**
      * Starts fetching the web manifest resources.
      */
@@ -188,17 +190,10 @@ public class ManifestUpgradeDetector implements ManifestUpgradeDetectorFetcher.C
         fetchedData.themeColor = themeColor;
         fetchedData.backgroundColor = backgroundColor;
 
-        // TODO(hanxi): crbug.com/627824. Validate whether the new WebappInfo is
+        // TODO(hanxi): crbug.com/627824. Validate whether the new featched data is

[pkotwicz, 2016/08/19 23:38:17]

Nit: featched -> fetched

[Xi Han, 2016/08/26 17:54:08]

Done.

         // WebAPK-compatible.
         boolean upgradeRequired = requireUpgrade(fetchedData);
-        WebappInfo newInfo = null;
-        if (upgradeRequired) {
-            newInfo = WebappInfo.create(mWebappInfo.id(), startUrl, scopeUrl,
-                    "", name, shortName, displayMode, orientation,
-                    mWebappInfo.source(), themeColor, backgroundColor,
-                    mWebappInfo.isIconGenerated(), mWebappInfo.webApkPackageName());
-        }
-        mCallback.onUpgradeNeededCheckFinished(upgradeRequired, newInfo);
+        mCallback.onUpgradeNeededCheckFinished(upgradeRequired, fetchedData);
     }
 
     /**