[Android WebAPK] Fix WebAPKs with adaptive icons requesting updates daily

This CL fixes bug where WebAPKs with adaptive icons would incorrectly
think they need to be updated due to the value of
WebApkInfo#isIconAdaptive() being incorrect. In particular, the
WebApkInfo#create() call in WebappLauncherActivity produces a WebApkInfo
where WebApkInfo#isIconAdaptive() == false if native is not yet loaded.

This CL:
- Only considers OS version when building WebApkInfo object
- Modifies WebApkUpdateManager code to enable downgrading a WebAPK from
an adaptive-primary-icon to a non-adaptive-primary-icon if the
ChromeFeatureList.WEBAPK_ADAPTIVE_ICON feature is disabled.

BUG=1058066
TEST=WebApkInfoTest.*

Change-Id: I6a00a3046bf1f82fb5fd50e171d35a49a7b6ecba
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2085392
Commit-Queue: Peter Kotwicz <pkotwicz@chromium.org>
Commit-Queue: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Yaron Friedman <yfriedman@chromium.org>
Reviewed-by: Glenn Hartmann <hartmanng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#747400}

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

@@ -291,8 +291,10 @@ public class WebApkIntentDataProviderFactory {
             }
         }
 
+        // Check the OS version because the same WebAPK is vended by the WebAPK server for all OS
+        // versions.
         boolean isPrimaryIconMaskable =
-                primaryMaskableIconId != 0 && ShortcutHelper.doesAndroidSupportMaskableIcons();
+                primaryMaskableIconId != 0 && (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O);
 
         int badgeIconId = IntentUtils.safeGetInt(bundle, WebApkMetaDataKeys.BADGE_ICON_ID, 0);
         int splashIconId = IntentUtils.safeGetInt(bundle, WebApkMetaDataKeys.SPLASH_ID, 0);

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

@@ -378,8 +378,9 @@ public class WebApkUpdateManager implements WebApkUpdateDataFetcher.Observer, De
             return WebApkUpdateReason.DISPLAY_MODE_DIFFERS;
         } else if (!oldInfo.shareTarget().equals(fetchedInfo.shareTarget())) {
             return WebApkUpdateReason.WEB_SHARE_TARGET_DIFFERS;
-        } else if (ShortcutHelper.doesAndroidSupportMaskableIcons()
-                && oldInfo.isIconAdaptive() != fetchedInfo.isIconAdaptive()) {
+        } else if (oldInfo.isIconAdaptive() != fetchedInfo.isIconAdaptive()
+                && (!fetchedInfo.isIconAdaptive()
+                        || ShortcutHelper.doesAndroidSupportMaskableIcons())) {
             return WebApkUpdateReason.PRIMARY_ICON_MASKABLE_DIFFERS;
         } else if (shortcutsDiffer(oldInfo.shortcutItems(), fetchedInfo.shortcutItems())) {
             return WebApkUpdateReason.SHORTCUTS_DIFFER;

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

@@ -9,6 +9,8 @@ import android.content.res.Resources;
 import android.graphics.Bitmap;
 import android.graphics.drawable.BitmapDrawable;
 
+import androidx.annotation.VisibleForTesting;
+
 import org.chromium.base.ApiCompatibilityUtils;
 import org.chromium.base.ContextUtils;
 import org.chromium.chrome.browser.ShortcutHelper;
@@ -49,6 +51,11 @@ public class WebappIcon {
         return mBitmap;
     }
 
+    @VisibleForTesting
+    public int resourceIdForTesting() {
+        return mResourceId;
+    }
+
     private Bitmap generateBitmap() {
         if (mEncoded != null) {
             return ShortcutHelper.decodeBitmapFromString(mEncoded);

chrome/android/javatests/src/org/chromium/chrome/browser/webapps/WebApkUpdateManagerTest.java

@@ -30,6 +30,7 @@ import org.chromium.chrome.test.util.browser.Features;
 import org.chromium.chrome.test.util.browser.webapps.WebappTestPage;
 import org.chromium.content_public.browser.test.util.TestThreadUtils;
 import org.chromium.content_public.common.ScreenOrientationValues;
+import org.chromium.net.test.EmbeddedTestServer;
 import org.chromium.net.test.EmbeddedTestServerRule;
 import org.chromium.webapk.lib.client.WebApkVersion;
 
@@ -77,6 +78,7 @@ public class WebApkUpdateManagerTest {
 
     private ChromeActivity mActivity;
     private Tab mTab;
+    private EmbeddedTestServer mTestServer;
 
     /**
      * Subclass of {@link WebApkUpdateManager} which notifies the {@link CallbackHelper} passed to
@@ -128,15 +130,15 @@ public class WebApkUpdateManagerTest {
 
     public CreationData defaultCreationData() {
         CreationData creationData = new CreationData();
-        creationData.manifestUrl = mTestServerRule.getServer().getURL(WEBAPK_MANIFEST_URL);
-        creationData.startUrl = mTestServerRule.getServer().getURL(WEBAPK_START_URL);
-        creationData.scope = mTestServerRule.getServer().getURL(WEBAPK_SCOPE_URL);
+        creationData.manifestUrl = mTestServer.getURL(WEBAPK_MANIFEST_URL);
+        creationData.startUrl = mTestServer.getURL(WEBAPK_START_URL);
+        creationData.scope = mTestServer.getURL(WEBAPK_SCOPE_URL);
         creationData.name = WEBAPK_NAME;
         creationData.shortName = WEBAPK_SHORT_NAME;
 
         creationData.iconUrlToMurmur2HashMap = new HashMap<String, String>();
         creationData.iconUrlToMurmur2HashMap.put(
-                mTestServerRule.getServer().getURL(WEBAPK_ICON_URL), WEBAPK_ICON_MURMUR2_HASH);
+                mTestServer.getURL(WEBAPK_ICON_URL), WEBAPK_ICON_MURMUR2_HASH);
 
         creationData.displayMode = WEBAPK_DISPLAY_MODE;
         creationData.orientation = WEBAPK_ORIENTATION;
@@ -153,6 +155,7 @@ public class WebApkUpdateManagerTest {
         RecordHistogram.setDisabledForTests(true);
         mActivity = mActivityTestRule.getActivity();
         mTab = mActivity.getActivityTab();
+        mTestServer = mTestServerRule.getServer();
 
         TestFetchStorageCallback callback = new TestFetchStorageCallback();
         WebappRegistry.getInstance().register(WEBAPK_ID, callback);
@@ -202,11 +205,11 @@ public class WebApkUpdateManagerTest {
     public void testCanonicalUrlsIdenticalShouldNotUpgrade() throws Exception {
         // URL canonicalization should replace "%74" with 't'.
         CreationData creationData = defaultCreationData();
-        creationData.startUrl = mTestServerRule.getServer().getURL(
-                "/chrome/test/data/banners/manifest_%74est_page.html");
+        creationData.startUrl =
+                mTestServer.getURL("/chrome/test/data/banners/manifest_%74est_page.html");
 
         WebappTestPage.navigateToServiceWorkerPageWithManifest(
-                mTestServerRule.getServer(), mTab, WEBAPK_MANIFEST_URL);
+                mTestServer, mTab, WEBAPK_MANIFEST_URL);
         Assert.assertFalse(checkUpdateNeeded(creationData));
     }
 
@@ -219,11 +222,11 @@ public class WebApkUpdateManagerTest {
     public void testCanonicalUrlsDifferentShouldUpgrade() throws Exception {
         // URL canonicalization should replace "%62" with 'b'.
         CreationData creationData = defaultCreationData();
-        creationData.startUrl = mTestServerRule.getServer().getURL(
-                "/chrome/test/data/banners/manifest_%62est_page.html");
+        creationData.startUrl =
+                mTestServer.getURL("/chrome/test/data/banners/manifest_%62est_page.html");
 
         WebappTestPage.navigateToServiceWorkerPageWithManifest(
-                mTestServerRule.getServer(), mTab, WEBAPK_MANIFEST_URL);
+                mTestServer, mTab, WEBAPK_MANIFEST_URL);
         Assert.assertTrue(checkUpdateNeeded(creationData));
     }
 
@@ -232,11 +235,11 @@ public class WebApkUpdateManagerTest {
     @Feature({"WebApk"})
     public void testNoUpdateForPagesWithoutWST() throws Exception {
         CreationData creationData = defaultCreationData();
-        creationData.startUrl = mTestServerRule.getServer().getURL(
-                "/chrome/test/data/banners/manifest_test_page.html");
+        creationData.startUrl =
+                mTestServer.getURL("/chrome/test/data/banners/manifest_test_page.html");
 
         WebappTestPage.navigateToServiceWorkerPageWithManifest(
-                mTestServerRule.getServer(), mTab, WEBAPK_MANIFEST_URL);
+                mTestServer, mTab, WEBAPK_MANIFEST_URL);
         Assert.assertFalse(checkUpdateNeeded(creationData));
     }
 
@@ -257,13 +260,32 @@ public class WebApkUpdateManagerTest {
     }
 
     private void testNewMaskableIconShouldUpdate() throws Exception {
-        CreationData creationData = defaultCreationData();
-        creationData.startUrl = mTestServerRule.getServer().getURL(
-                "/chrome/test/data/banners/manifest_test_page.html");
-        creationData.isPrimaryIconMaskable = true;
+        final String maskableManifestUrl = "/chrome/test/data/banners/manifest_maskable.json";
+
+        CreationData creationData = new CreationData();
+        creationData.manifestUrl = mTestServer.getURL(maskableManifestUrl);
+        creationData.startUrl =
+                mTestServer.getURL("/chrome/test/data/banners/manifest_test_page.html");
+        creationData.scope = mTestServer.getURL("/chrome/test/data/banners/");
+        creationData.name = "Manifest test app";
+        creationData.shortName = creationData.name;
+
+        creationData.iconUrlToMurmur2HashMap = new HashMap<String, String>();
+        creationData.iconUrlToMurmur2HashMap.put(
+                mTestServer.getURL("/chrome/test/data/banners/launcher-icon-4x.png"),
+                "8692598279279335241");
+        creationData.iconUrlToMurmur2HashMap.put(
+                mTestServer.getURL("/chrome/test/data/banners/launcher-icon-3x.png"),
+                "16812314236514539104");
+        creationData.displayMode = WebDisplayMode.STANDALONE;
+        creationData.orientation = ScreenOrientationValues.LANDSCAPE;
+        creationData.themeColor = 2147483648L;
+        creationData.backgroundColor = 2147483648L;
+        creationData.isPrimaryIconMaskable = false;
+        creationData.shortcuts = new ArrayList<>();
 
         WebappTestPage.navigateToServiceWorkerPageWithManifest(
-                mTestServerRule.getServer(), mTab, WEBAPK_MANIFEST_URL);
+                mTestServer, mTab, maskableManifestUrl);
 
         Assert.assertEquals(
                 ShortcutHelper.doesAndroidSupportMaskableIcons(), checkUpdateNeeded(creationData));
@@ -274,20 +296,18 @@ public class WebApkUpdateManagerTest {
     @Feature({"WebApk"})
     public void testManifestWithExtraShortcutsDoesNotCauseUpdate() throws Exception {
         CreationData creationData = defaultCreationData();
-        creationData.startUrl = mTestServerRule.getServer().getURL(
-                "/chrome/test/data/banners/manifest_test_page.html");
+        creationData.startUrl =
+                mTestServer.getURL("/chrome/test/data/banners/manifest_test_page.html");
 
-        creationData.manifestUrl =
-                mTestServerRule.getServer().getURL(WEBAPK_MANIFEST_TOO_MANY_SHORTCUTS_URL);
+        creationData.manifestUrl = mTestServer.getURL(WEBAPK_MANIFEST_TOO_MANY_SHORTCUTS_URL);
         for (int i = 0; i < 4; i++) {
             creationData.shortcuts.add(new WebApkExtras.ShortcutItem("name" + String.valueOf(i),
-                    "short_name",
-                    mTestServerRule.getServer().getURL(WEBAPK_SCOPE_URL + "launch_url"), "", ""));
+                    "short_name", mTestServer.getURL(WEBAPK_SCOPE_URL + "launch_url"), "", ""));
         }
 
         // The fifth shortcut should be ignored.
         WebappTestPage.navigateToServiceWorkerPageWithManifest(
-                mTestServerRule.getServer(), mTab, WEBAPK_MANIFEST_TOO_MANY_SHORTCUTS_URL);
+                mTestServer, mTab, WEBAPK_MANIFEST_TOO_MANY_SHORTCUTS_URL);
         Assert.assertFalse(checkUpdateNeeded(creationData));
     }
 }

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

@@ -23,6 +23,7 @@ import org.robolectric.RuntimeEnvironment;
 import org.robolectric.android.XmlResourceParserImpl;
 import org.robolectric.annotation.Config;
 import org.robolectric.res.ResourceTable;
+import org.robolectric.util.ReflectionHelpers;
 import org.w3c.dom.Document;
 
 import org.chromium.base.test.BaseRobolectricTestRunner;
@@ -64,6 +65,8 @@ public class WebApkInfoTest {
     private static final String MANIFEST_URL = "https://www.google.com/alphabet.json";
     private static final String ICON_URL = "https://www.google.com/scope/worm.png";
     private static final String ICON_MURMUR2_HASH = "5";
+    private static final int PRIMARY_ICON_ID = 12;
+    private static final int PRIMARY_MASKABLE_ICON_ID = 14;
     private static final int SOURCE = ShortcutSource.NOTIFICATION;
 
     /** Fakes the Resources object, allowing lookup of String value. */
@@ -175,6 +178,10 @@ public class WebApkInfoTest {
 
     @Test
     public void testSanity() {
+        // Test guidelines:
+        // - Stubbing out native calls in this test likely means that there is a bug.
+        // - For every WebApkInfo boolean there should be a test which tests both values.
+
         Bundle bundle = new Bundle();
         bundle.putString(WebApkMetaDataKeys.SCOPE, SCOPE);
         bundle.putString(WebApkMetaDataKeys.NAME, NAME);
@@ -188,6 +195,8 @@ public class WebApkInfoTest {
         bundle.putString(WebApkMetaDataKeys.START_URL, START_URL);
         bundle.putString(WebApkMetaDataKeys.ICON_URLS_AND_ICON_MURMUR2_HASHES,
                 ICON_URL + " " + ICON_MURMUR2_HASH);
+        bundle.putInt(WebApkMetaDataKeys.ICON_ID, PRIMARY_ICON_ID);
+        bundle.putInt(WebApkMetaDataKeys.MASKABLE_ICON_ID, PRIMARY_MASKABLE_ICON_ID);
 
         Bundle shareActivityBundle = new Bundle();
         shareActivityBundle.putString(WebApkMetaDataKeys.SHARE_ACTION, "action0");
@@ -238,7 +247,8 @@ public class WebApkInfoTest {
         Assert.assertEquals(
                 (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M), info.isSplashProvidedByWebApk());
 
-        Assert.assertEquals(null, info.icon().bitmap());
+        Assert.assertEquals(PRIMARY_MASKABLE_ICON_ID, info.icon().resourceIdForTesting());
+        Assert.assertEquals(true, info.isIconAdaptive());
         Assert.assertEquals(null, info.badgeIcon().bitmap());
         Assert.assertEquals(null, info.splashIcon().bitmap());
 
@@ -254,6 +264,45 @@ public class WebApkInfoTest {
                 shareTarget.getFileAccepts());
     }
 
+    /**
+     * Test that {@link WebApkInfo#create()} ignores the maskable icon on pre-Android-O
+     * Android OSes.
+     */
+    @Test
+    public void testOsVersionDoesNotSupportAdaptive() {
+        ReflectionHelpers.setStaticField(Build.VERSION.class, "SDK_INT", Build.VERSION_CODES.N);
+
+        Bundle bundle = new Bundle();
+        bundle.putInt(WebApkMetaDataKeys.ICON_ID, PRIMARY_ICON_ID);
+        bundle.putInt(WebApkMetaDataKeys.MASKABLE_ICON_ID, PRIMARY_MASKABLE_ICON_ID);
+        bundle.putString(WebApkMetaDataKeys.START_URL, START_URL);
+        WebApkTestHelper.registerWebApkWithMetaData(
+                WEBAPK_PACKAGE_NAME, bundle, null /* shareTargetMetaData */);
+
+        Intent intent = createMinimalWebApkIntent(WEBAPK_PACKAGE_NAME, START_URL);
+        WebApkInfo info = WebApkInfo.create(intent);
+        Assert.assertEquals(PRIMARY_ICON_ID, info.icon().resourceIdForTesting());
+        Assert.assertEquals(false, info.isIconAdaptive());
+    }
+
+    /**
+     * Test that {@link WebApkInfo#create()} selects {@link WebApkMetaDataKeys.ICON_ID} if no
+     * maskable icon is provided and that the icon is tagged as non-maskable.
+     */
+    @Test
+    public void testNoMaskableIcon() {
+        Bundle bundle = new Bundle();
+        bundle.putInt(WebApkMetaDataKeys.ICON_ID, PRIMARY_ICON_ID);
+        bundle.putString(WebApkMetaDataKeys.START_URL, START_URL);
+        WebApkTestHelper.registerWebApkWithMetaData(
+                WEBAPK_PACKAGE_NAME, bundle, null /* shareTargetMetaData */);
+
+        Intent intent = createMinimalWebApkIntent(WEBAPK_PACKAGE_NAME, START_URL);
+        WebApkInfo info = WebApkInfo.create(intent);
+        Assert.assertEquals(PRIMARY_ICON_ID, info.icon().resourceIdForTesting());
+        Assert.assertEquals(false, info.isIconAdaptive());
+    }
+
     /**
      * Test that {@link WebApkInfo#create()} populates {@link WebApkInfo#url()} with the start URL
      * from the intent not the start URL in the WebAPK's meta data. When a WebAPK is launched via a