webapp:Update manifest validation to always require a Purpose:ANY icon.

Apps should always declare at least one Purpose::ANY icon (or no purpose
declared) to be handled properly on all platforms. Even when a maskable
icon is preferred as the primary icon (eg. on Android) an 'any' purpose
icon is still needed for other contexts.

Companion CL to update DevTools error messages: crrev.com/c/2557277

Bug: 1151835
Change-Id: I1995b1677d1392a5e852e544f34435826cce4e7c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2599733Reviewed-by: Dominick Ng <dominickn@chromium.org>
Commit-Queue: Glen Robertson <glenrob@chromium.org>
Cr-Commit-Position: refs/heads/master@{#840070}

chrome/browser/android/webapps/add_to_homescreen_data_fetcher_unittest.cc

@@ -139,8 +139,7 @@ class TestInstallableManager : public InstallableManager {
       is_installable = false;
     } else if (params.valid_manifest && params.has_worker) {
       if (!IsManifestValidForWebApp(manifest_,
-                                    true /* check_webapp_manifest_display */,
-                                    false /* prefer_maskable_icon */)) {
+                                    true /* check_webapp_manifest_display */)) {
         code = valid_manifest_->errors.at(0);
         is_installable = false;
       } else if (!is_installable_) {

components/webapps/installable/installable_manager.cc

@@ -138,44 +138,23 @@ bool IsIconTypeSupported(const blink::Manifest::ImageResource& icon) {
   return false;
 }
 
-// Returns true if |manifest| specifies an SVG or PNG icon that either
-// 1. has IconPurpose::ANY, with height and width >= kMinimumPrimaryIconSizeInPx
-// (or size "any")
-// 2. if maskable icon is preferred, has IconPurpose::MASKABLE with height and
-// width >= kMinimumPrimaryAdaptiveLauncherIconSizeInPx (or size "any")
-
-bool DoesManifestContainRequiredIcon(const blink::Manifest& manifest,
-                                     bool prefer_maskable_icon) {
+// Returns whether |manifest| specifies an SVG or PNG icon that has
+// IconPurpose::ANY, with size >= kMinimumPrimaryIconSizeInPx (or size "any").
+bool DoesManifestContainRequiredIcon(const blink::Manifest& manifest) {
   for (const auto& icon : manifest.icons) {
     if (!IsIconTypeSupported(icon))
       continue;
 
-    if (!(base::Contains(icon.purpose,
-                         blink::mojom::ManifestImageResource_Purpose::ANY) ||
-          (prefer_maskable_icon &&
-           base::Contains(
-               icon.purpose,
-               blink::mojom::ManifestImageResource_Purpose::MASKABLE)))) {
+    if (!base::Contains(icon.purpose, IconPurpose::ANY))
       continue;
-    }
 
     for (const auto& size : icon.sizes) {
       if (size.IsEmpty())  // "any"
         return true;
-      if (base::Contains(icon.purpose,
-                         blink::mojom::ManifestImageResource_Purpose::ANY) &&
-          size.width() >= kMinimumPrimaryIconSizeInPx &&
+      if (size.width() >= kMinimumPrimaryIconSizeInPx &&
           size.height() >= kMinimumPrimaryIconSizeInPx) {
         return true;
       }
-      if (prefer_maskable_icon &&
-          base::Contains(
-              icon.purpose,
-              blink::mojom::ManifestImageResource_Purpose::MASKABLE) &&
-          size.height() >= kMinimumPrimaryAdaptiveLauncherIconSizeInPx &&
-          size.width() >= kMinimumPrimaryAdaptiveLauncherIconSizeInPx) {
-        return true;
-      }
     }
   }
 
@@ -581,8 +560,7 @@ void InstallableManager::WorkOnTask() {
                           GetMinimumPrimaryIconSizeInPx(), IconPurpose::ANY,
                           IconUsage::kPrimary);
   } else if (params.valid_manifest && !valid_manifest_->fetched) {
-    CheckManifestValid(params.check_webapp_manifest_display,
-                       params.prefer_maskable_icon);
+    CheckManifestValid(params.check_webapp_manifest_display);
   } else if (params.fetch_screenshots && !IsScreenshotsFetchComplete()) {
     CheckAndFetchScreenshots();
   } else if (params.has_worker && !worker_->fetched) {
@@ -640,21 +618,20 @@ void InstallableManager::OnDidGetManifest(const GURL& manifest_url,
   WorkOnTask();
 }
 
-void InstallableManager::CheckManifestValid(bool check_webapp_manifest_display,
-                                            bool prefer_maskable_icon) {
+void InstallableManager::CheckManifestValid(
+    bool check_webapp_manifest_display) {
   DCHECK(!valid_manifest_->fetched);
   DCHECK(!manifest().IsEmpty());
 
-  valid_manifest_->is_valid = IsManifestValidForWebApp(
-      manifest(), check_webapp_manifest_display, prefer_maskable_icon);
+  valid_manifest_->is_valid =
+      IsManifestValidForWebApp(manifest(), check_webapp_manifest_display);
   valid_manifest_->fetched = true;
   WorkOnTask();
 }
 
 bool InstallableManager::IsManifestValidForWebApp(
     const blink::Manifest& manifest,
-    bool check_webapp_manifest_display,
-    bool prefer_maskable_icon) {
+    bool check_webapp_manifest_display) {
   bool is_valid = true;
   if (manifest.IsEmpty()) {
     valid_manifest_->errors.push_back(MANIFEST_EMPTY);
@@ -694,7 +671,7 @@ bool InstallableManager::IsManifestValidForWebApp(
     }
   }
 
-  if (!DoesManifestContainRequiredIcon(manifest, prefer_maskable_icon)) {
+  if (!DoesManifestContainRequiredIcon(manifest)) {
     valid_manifest_->errors.push_back(MANIFEST_MISSING_SUITABLE_ICON);
     is_valid = false;
   }

components/webapps/installable/installable_manager.h

@@ -213,11 +213,9 @@ class InstallableManager
   void OnDidGetManifest(const GURL& manifest_url,
                         const blink::Manifest& manifest);
 
-  void CheckManifestValid(bool check_webapp_manifest_display,
-                          bool prefer_maskable_icon);
+  void CheckManifestValid(bool check_webapp_manifest_display);
   bool IsManifestValidForWebApp(const blink::Manifest& manifest,
-                                bool check_webapp_manifest_display,
-                                bool prefer_maskable_icon);
+                                bool check_webapp_manifest_display);
   void CheckServiceWorker();
   void OnDidCheckHasServiceWorker(
       base::TimeTicks check_service_worker_start_time,

components/webapps/installable/installable_manager_unittest.cc

@@ -43,12 +43,11 @@ class InstallableManagerUnitTest : public testing::Test {
   }
 
   bool IsManifestValid(const blink::Manifest& manifest,
-                       bool check_webapp_manifest_display = true,
-                       bool prefer_maskable_icon = false) {
+                       bool check_webapp_manifest_display = true) {
     // Explicitly reset the error code before running the method.
     manager_->set_valid_manifest_error(NO_ERROR_DETECTED);
-    return manager_->IsManifestValidForWebApp(
-        manifest, check_webapp_manifest_display, prefer_maskable_icon);
+    return manager_->IsManifestValidForWebApp(manifest,
+                                              check_webapp_manifest_display);
   }
 
   InstallableStatusCode GetErrorCode() {
@@ -218,32 +217,6 @@ TEST_F(InstallableManagerUnitTest, ManifestRequiresPurposeAny) {
   EXPECT_EQ(NO_ERROR_DETECTED, GetErrorCode());
 }
 
-TEST_F(InstallableManagerUnitTest,
-       ManifestRequiresPurposeAnyOrMaskableWhenPreferringMaskable) {
-  blink::Manifest manifest = GetValidManifest();
-
-  EXPECT_TRUE(IsManifestValid(manifest, true, true));
-  EXPECT_EQ(NO_ERROR_DETECTED, GetErrorCode());
-
-  manifest.icons[0].purpose[0] = IconPurpose::MASKABLE;
-  EXPECT_TRUE(IsManifestValid(manifest, true, true));
-  EXPECT_EQ(NO_ERROR_DETECTED, GetErrorCode());
-
-  // The icon MUST have IconPurpose::ANY or IconPurpose::Maskable.
-  manifest.icons[0].purpose[0] = IconPurpose::MONOCHROME;
-  EXPECT_FALSE(IsManifestValid(manifest, true, true));
-  EXPECT_EQ(MANIFEST_MISSING_SUITABLE_ICON, GetErrorCode());
-
-  // If one of the icon purposes match the requirement, it should be accepted.
-  manifest.icons[0].purpose.push_back(IconPurpose::ANY);
-  EXPECT_TRUE(IsManifestValid(manifest, true, true));
-  EXPECT_EQ(NO_ERROR_DETECTED, GetErrorCode());
-
-  manifest.icons[0].purpose[1] = IconPurpose::MASKABLE;
-  EXPECT_TRUE(IsManifestValid(manifest, true, true));
-  EXPECT_EQ(NO_ERROR_DETECTED, GetErrorCode());
-}
-
 TEST_F(InstallableManagerUnitTest, ManifestRequiresMinimalSize) {
   blink::Manifest manifest = GetValidManifest();
 
@@ -275,32 +248,6 @@ TEST_F(InstallableManagerUnitTest, ManifestRequiresMinimalSize) {
   manifest.icons[0].sizes[1] = gfx::Size(0, 0);
   EXPECT_TRUE(IsManifestValid(manifest));
   EXPECT_EQ(NO_ERROR_DETECTED, GetErrorCode());
-
-  // When preferring maskable icons, both non-maskable and maskable
-  // icons are smaller than required.
-  manifest.icons[0].sizes.clear();
-  manifest.icons[0].sizes.push_back(gfx::Size(1, 1));
-
-  blink::Manifest::ImageResource maskable_icon;
-  maskable_icon.type = base::ASCIIToUTF16("image/png");
-  maskable_icon.sizes.push_back(gfx::Size(82, 82));
-  maskable_icon.purpose.push_back(IconPurpose::MASKABLE);
-  manifest.icons.push_back(maskable_icon);
-  EXPECT_FALSE(IsManifestValid(manifest, true, true));
-  EXPECT_EQ(MANIFEST_MISSING_SUITABLE_ICON, GetErrorCode());
-
-  // When preferring maskable icons, the maskable icon is smaller than required.
-  manifest.icons[0].sizes[0] = gfx::Size(144, 144);
-  manifest.icons[1].sizes[0] = gfx::Size(82, 82);
-  EXPECT_TRUE(IsManifestValid(manifest, true, true));
-  EXPECT_EQ(NO_ERROR_DETECTED, GetErrorCode());
-
-  // When preferring maskable icons, the maskable icon satisfies the size
-  // requirement though the non maskable one doesn't
-  manifest.icons[0].sizes[0] = gfx::Size(1, 1);
-  manifest.icons[1].sizes[0] = gfx::Size(83, 83);
-  EXPECT_TRUE(IsManifestValid(manifest, true, true));
-  EXPECT_EQ(NO_ERROR_DETECTED, GetErrorCode());
 }
 
 TEST_F(InstallableManagerUnitTest, ManifestDisplayModes) {

components/webapps/installable/installable_params.h

@@ -29,6 +29,7 @@ struct InstallableParams {
   bool valid_primary_icon = false;
 
   // Whether to prefer an icon with purpose 'maskable' for the primary icon.
+  // An icon with purpose 'any' is still required for a valid manifest.
   bool prefer_maskable_icon = false;
 
   // Check whether there is a fetchable, non-empty icon in the manifest