Send all of the icon URLs listed in Web Manifest to WebAPK Server.

chrome/android/junit/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorTest.java

Index: chrome/android/junit/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorTest.java
diff --git a/chrome/android/junit/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorTest.java b/chrome/android/junit/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorTest.java
index 442fffe290e716e6a790ead556310628c4e22d49..df9a3f99cf63a327407e299e40da9e732cdc9be6 100644
--- a/chrome/android/junit/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorTest.java
+++ b/chrome/android/junit/src/org/chromium/chrome/browser/webapps/ManifestUpgradeDetectorTest.java
@@ -30,6 +30,9 @@ import org.chromium.chrome.browser.tab.Tab;
 import org.chromium.content_public.common.ScreenOrientationValues;
 import org.chromium.testing.local.LocalRobolectricTestRunner;
 
+import java.util.HashMap;
+import java.util.HashSet;
+
 /**
  * Tests the ManifestUpgradeDetector.
  */
@@ -66,12 +69,6 @@ public class ManifestUpgradeDetectorTest {
     }
 
     /**
-     * The WebappInfoCreationData is the data extracted from a WebAPK. It is used to create a
-     * WebappInfo for WebAPKs.
-     */
-    private static class WebappInfoCreationData extends FetchedManifestData {}
-
-    /**
      * ManifestUpgradeDetector subclass which:
      * - Stubs out ManifestUpgradeDetectorFetcher.
      * - Uses {@link fetchedData} passed into the constructor as the "Downloaded Manifest Data".
@@ -97,8 +94,9 @@ public class ManifestUpgradeDetectorTest {
                     ManifestUpgradeDetectorFetcher.Callback callback =
                             (ManifestUpgradeDetectorFetcher.Callback) invocation.getArguments()[0];
                     callback.onGotManifestData(mFetchedData.startUrl, mFetchedData.scopeUrl,
-                            mFetchedData.name, mFetchedData.shortName, mFetchedData.iconUrl,
-                            mFetchedData.iconMurmur2Hash, mFetchedData.icon,
+                            mFetchedData.name, mFetchedData.shortName, mFetchedData.bestIconUrl,
+                            mFetchedData.bestIconMurmur2Hash, mFetchedData.bestIcon,
+                            mFetchedData.iconUrls.toArray(new String[0]),
                             mFetchedData.displayMode, mFetchedData.orientation,
                             mFetchedData.themeColor, mFetchedData.backgroundColor);
                     return null;
@@ -138,37 +136,44 @@ public class ManifestUpgradeDetectorTest {
      */
     private FetchedManifestData createDefaultFetchedManifestData() {
         FetchedManifestData data = new FetchedManifestData();
-        populateDataWithDefaults(data);
+        data.startUrl = WEBAPK_START_URL;
+        data.scopeUrl = WEBAPK_SCOPE_URL;
+        data.name = WEBAPK_NAME;
+        data.shortName = WEBAPK_SHORT_NAME;
+        data.bestIconUrl = WEBAPK_ICON_URL;
+        data.bestIconMurmur2Hash = WEBAPK_ICON_MURMUR2_HASH;
+        data.bestIcon = createBitmap(Color.GREEN);
+        data.iconUrls = new HashSet<String>();
+        data.iconUrls.add(data.bestIconUrl);
+        data.displayMode = WEBAPK_DISPLAY_MODE;
+        data.orientation = WEBAPK_ORIENTATION;
+        data.themeColor = WEBAPK_THEME_COLOR;
+        data.backgroundColor = WEBAPK_BACKGROUND_COLOR;
         return data;
     }
 
     /**
-     * Create a default data. The WebappInfoCreationData is the data that the WebAPK was created
+     * Create a default data. The WebApkMetaData is the data that the WebAPK was created
      * with.
      */
-    private WebappInfoCreationData createDefaultWebappInfoCreationData() {
-        WebappInfoCreationData data = new WebappInfoCreationData();
-        populateDataWithDefaults(data);
-        return data;
-    }
-
-    private void populateDataWithDefaults(FetchedManifestData data) {
+    private WebApkMetaData createDefaultWebApkMetaData() {
+        WebApkMetaData data = new WebApkMetaData();
         data.startUrl = WEBAPK_START_URL;
-        data.scopeUrl = WEBAPK_SCOPE_URL;
+        data.scope = WEBAPK_SCOPE_URL;
         data.name = WEBAPK_NAME;
         data.shortName = WEBAPK_SHORT_NAME;
-        data.iconUrl = WEBAPK_ICON_URL;
-        data.iconMurmur2Hash = WEBAPK_ICON_MURMUR2_HASH;
-        data.icon = createBitmap(Color.GREEN);
         data.displayMode = WEBAPK_DISPLAY_MODE;
         data.orientation = WEBAPK_ORIENTATION;
         data.themeColor = WEBAPK_THEME_COLOR;
         data.backgroundColor = WEBAPK_BACKGROUND_COLOR;

[pkotwicz, 2016/11/14 22:43:11]

Nit: Can you add a new line for clarity. You are  initializing
iconUrlAndMurmur2HashMap in two lines which is kind of different.

[Xi Han, 2016/11/15 20:07:07]

Done.

+        data.iconUrlAndMurmur2HashMap = new HashMap<String, String>();
+        data.iconUrlAndMurmur2HashMap.put(WEBAPK_ICON_URL, WEBAPK_ICON_MURMUR2_HASH);

[pkotwicz, 2016/11/14 22:43:11]

Can you please add a new line for clarity

[Xi Han, 2016/11/15 20:07:07]

Done.

+        return data;
     }
 
     private TestManifestUpgradeDetector createDetectorWithFetchedData(
             FetchedManifestData fetchedData, TestCallback callback) {
-        return createDetector(createDefaultWebappInfoCreationData(), fetchedData, callback);
+        return createDetector(createDefaultWebApkMetaData(), fetchedData, callback);
     }
 
     /**
@@ -178,20 +183,20 @@ public class ManifestUpgradeDetectorTest {
      * @param fetchedData Data fetched by ManifestUpgradeDetector.
      * @param callback Callback to call when the upgrade check is complete.
      */
-    private TestManifestUpgradeDetector createDetector(WebappInfoCreationData oldData,
+    private TestManifestUpgradeDetector createDetector(WebApkMetaData oldData,
             FetchedManifestData fetchedData, TestCallback callback) {
         WebApkMetaData metaData = new WebApkMetaData();
         metaData.manifestUrl = WEBAPK_MANIFEST_URL;
         metaData.startUrl = oldData.startUrl;
-        metaData.scope = oldData.scopeUrl;
+        metaData.scope = oldData.scope;
         metaData.name = oldData.name;
         metaData.shortName = oldData.shortName;
         metaData.displayMode = oldData.displayMode;
         metaData.orientation = oldData.orientation;
         metaData.themeColor = oldData.themeColor;
         metaData.backgroundColor = oldData.backgroundColor;
-        metaData.iconUrl = oldData.iconUrl;
-        metaData.iconMurmur2Hash = oldData.iconMurmur2Hash;
+        metaData.iconUrlAndMurmur2HashMap = new HashMap<String, String>(
+                oldData.iconUrlAndMurmur2HashMap);
         return new TestManifestUpgradeDetector(null, metaData, fetchedData, callback);
     }
 
@@ -222,12 +227,12 @@ public class ManifestUpgradeDetectorTest {
      */
     @Test
     public void testManifestEmptyScopeShouldNotUpgrade() {
-        WebappInfoCreationData oldData = createDefaultWebappInfoCreationData();
+        WebApkMetaData oldData = createDefaultWebApkMetaData();
         // webapk_installer.cc sets the scope to the default scope if the scope is empty.
-        oldData.scopeUrl = ShortcutHelper.getScopeFromUrl(oldData.startUrl);
+        oldData.scope = ShortcutHelper.getScopeFromUrl(oldData.startUrl);
         FetchedManifestData fetchedData = createDefaultFetchedManifestData();
         fetchedData.scopeUrl = "";
-        Assert.assertTrue(!oldData.scopeUrl.equals(fetchedData.scopeUrl));
+        Assert.assertTrue(!oldData.scope.equals(fetchedData.scopeUrl));
 
         TestCallback callback = new TestCallback();
         TestManifestUpgradeDetector detector = createDetector(oldData, fetchedData, callback);
@@ -242,11 +247,11 @@ public class ManifestUpgradeDetectorTest {
      */
     @Test
     public void testManifestNonEmptyScopeToEmptyScopeShouldUpgrade() {
-        WebappInfoCreationData oldData = createDefaultWebappInfoCreationData();
+        WebApkMetaData oldData = createDefaultWebApkMetaData();
         oldData.startUrl = "/fancy/scope/special/snowflake.html";
-        oldData.scopeUrl = "/fancy/scope/";
+        oldData.scope = "/fancy/scope/";
         Assert.assertTrue(
-                !oldData.scopeUrl.equals(ShortcutHelper.getScopeFromUrl(oldData.startUrl)));
+                !oldData.scope.equals(ShortcutHelper.getScopeFromUrl(oldData.startUrl)));
         FetchedManifestData fetchedData = createDefaultFetchedManifestData();
         fetchedData.startUrl = "/fancy/scope/special/snowflake.html";
         fetchedData.scopeUrl = "";
@@ -266,8 +271,8 @@ public class ManifestUpgradeDetectorTest {
     @Test
     public void testHomescreenIconChangeShouldUpgrade() {
         FetchedManifestData fetchedData = createDefaultFetchedManifestData();
-        fetchedData.iconMurmur2Hash = WEBAPK_ICON_MURMUR2_HASH + "1";
-        fetchedData.icon = createBitmap(Color.BLUE);
+        fetchedData.bestIconMurmur2Hash = WEBAPK_ICON_MURMUR2_HASH + "1";
+        fetchedData.bestIcon = createBitmap(Color.BLUE);
         TestCallback callback = new TestCallback();
         TestManifestUpgradeDetector detector = createDetectorWithFetchedData(fetchedData, callback);
 
@@ -284,7 +289,7 @@ public class ManifestUpgradeDetectorTest {
     @Test
     public void testHomescreenIconUrlChangeShouldUpgrade() {
         FetchedManifestData fetchedData = createDefaultFetchedManifestData();
-        fetchedData.iconUrl = "/icon2.png";
+        fetchedData.bestIconUrl = "/icon2.png";
 
         TestCallback callback = new TestCallback();
         TestManifestUpgradeDetector detector = createDetectorWithFetchedData(fetchedData, callback);
@@ -292,4 +297,39 @@ public class ManifestUpgradeDetectorTest {
         Assert.assertTrue(callback.mWasCalled);
         Assert.assertTrue(callback.mIsUpgraded);
     }
+
+    /** Test that an upgrade is requested when a new icon URL is added to the Web Manifest. */
+    @Test
+    public void testIconUrlsChangeShouleUpgrade() {
+        FetchedManifestData fetchedData = createDefaultFetchedManifestData();
+        fetchedData.iconUrls.add("/icon2.png");
+
+        TestCallback callback = new TestCallback();
+        TestManifestUpgradeDetector detector = createDetectorWithFetchedData(fetchedData, callback);
+        detector.start();
+        Assert.assertTrue(callback.mWasCalled);
+        Assert.assertTrue(callback.mIsUpgraded);
+    }
+
+    /**
+     * Test that an upgrade isn't requested when there isn't any change of icon URLs in the Web
+     * Manifest.

[pkotwicz, 2016/11/14 22:43:11]

How about: "Test that an upgrade isn't requested when the Web Manifest has
multiple icons and the Web Manifest has not changed."

[Xi Han, 2016/11/15 20:07:07]

Done.

+     */
+    @Test
+    public void testNoIconUrlsChangeShouldNotUpgrade() {
+        String icon1 = "/icon1.png";
+        String icon2 = "/icon2.png";
+        WebApkMetaData oldData = createDefaultWebApkMetaData();
+        oldData.iconUrlAndMurmur2HashMap.put(icon1, "11");
+        oldData.iconUrlAndMurmur2HashMap.put(icon2, "22");
+
+        FetchedManifestData fetchedData = createDefaultFetchedManifestData();
+        fetchedData.iconUrls.add(icon1);
+        fetchedData.iconUrls.add(icon2);

[pkotwicz, 2016/11/14 22:43:11]

For the sake of fun, make bestIconUrl and bestIconMurmur2Hash point to the
second icon
(To make sure that we don't do any fancy handling with the first icon)

[Xi Han, 2016/11/15 20:07:07]

Done.

+        TestCallback callback = new TestCallback();
+        TestManifestUpgradeDetector detector = createDetector(oldData, fetchedData, callback);
+        detector.start();
+        Assert.assertTrue(callback.mWasCalled);
+        Assert.assertFalse(callback.mIsUpgraded);
+    }
 }