[LargeSplash:1]InstallableManager fetch splash instead of badge

This CL adds splash icon fetching to InstallableManager and removes
badge icon part.

A bool valid_splash_icon is added to InstallableParams but
valid_badge_icon is not removed yet since the usages of
"params.valid_badge_icon" are not removed yet. (going to be remove in
the following CLs).
Now InstallableParams class has 9 booleans, so chromium style check
require it to have a explicit constructor. The constructor (and
associate .cc file) will be removed once valid_badge_icon is removed.

With this CL, badge icon will not be fetched even if valid_badge_icon is
set. It's ok to do so since it's not used.

Bug: 1043271
Change-Id: Ife4e99afe4fa9d94580adfff3b9ba6a6f312254c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2037813
Commit-Queue: Ella Ge <eirage@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Reviewed-by: Peter Kotwicz <pkotwicz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744013}

chrome/browser/BUILD.gn

@@ -627,6 +627,7 @@ jumbo_static_library("browser") {
     "installable/installable_manager.h",
     "installable/installable_metrics.cc",
     "installable/installable_metrics.h",
+    "installable/installable_params.cc",
     "installable/installable_params.h",
     "installable/installable_task_queue.cc",
     "installable/installable_task_queue.h",

chrome/browser/android/shortcut_helper.cc

@@ -228,12 +228,6 @@ int ShortcutHelper::GetMinimumSplashImageSizeInPx() {
   return g_minimum_splash_image_size;
 }
 
-int ShortcutHelper::GetIdealBadgeIconSizeInPx() {
-  if (g_ideal_badge_icon_size == -1)
-    GetIconSizes();
-  return g_ideal_badge_icon_size;
-}
-
 int ShortcutHelper::GetIdealAdaptiveLauncherIconSizeInPx() {
   if (g_ideal_adaptive_launcher_icon_size == -1)
     GetIconSizes();

chrome/browser/android/shortcut_helper.h

@@ -60,9 +60,6 @@ class ShortcutHelper {
   // screen.
   static int GetMinimumSplashImageSizeInPx();
 
-  // Returns the ideal size for a badge icon of a WebAPK.
-  static int GetIdealBadgeIconSizeInPx();
-
   // Returns the ideal size for an adaptive launcher icon of a WebAPK
   static int GetIdealAdaptiveLauncherIconSizeInPx();
 

chrome/browser/android/webapps/add_to_homescreen_data_fetcher_unittest.cc

@@ -467,10 +467,9 @@ TEST_F(AddToHomescreenDataFetcherTest, InstallableManifest) {
   EXPECT_EQ(fetcher->shortcut_info().best_primary_icon_url,
             GURL(kDefaultIconUrl));
 
-  // Check that the badge icon is requested.
-  EXPECT_FALSE(fetcher->badge_icon().drawsNothing());
-  EXPECT_EQ(fetcher->shortcut_info().best_badge_icon_url,
-            GURL(kDefaultIconUrl));
+  // No badge icon as InstallableManager does not fetch badge icon.
+  EXPECT_TRUE(fetcher->badge_icon().drawsNothing());
+  EXPECT_TRUE(fetcher->shortcut_info().best_badge_icon_url.is_empty());
   CheckHistograms(histograms);
 }
 

chrome/browser/installable/installable_data.cc

@@ -12,8 +12,8 @@ InstallableData::InstallableData(std::vector<InstallableStatusCode> errors,
                                  const GURL& primary_icon_url,
                                  const SkBitmap* primary_icon,
                                  bool has_maskable_primary_icon,
-                                 const GURL& badge_icon_url,
-                                 const SkBitmap* badge_icon,
+                                 const GURL& splash_icon_url,
+                                 const SkBitmap* splash_icon,
                                  bool valid_manifest,
                                  bool has_worker)
     : errors(std::move(errors)),
@@ -22,8 +22,10 @@ InstallableData::InstallableData(std::vector<InstallableStatusCode> errors,
       primary_icon_url(primary_icon_url),
       primary_icon(primary_icon),
       has_maskable_primary_icon(has_maskable_primary_icon),
-      badge_icon_url(badge_icon_url),
-      badge_icon(badge_icon),
+      badge_icon_url(GURL::EmptyGURL()),
+      badge_icon(nullptr),
+      splash_icon_url(splash_icon_url),
+      splash_icon(splash_icon),
       valid_manifest(valid_manifest),
       has_worker(has_worker) {}
 

chrome/browser/installable/installable_data.h

@@ -25,8 +25,8 @@ struct InstallableData {
                   const GURL& primary_icon_url,
                   const SkBitmap* primary_icon,
                   bool has_maskable_primary_icon,
-                  const GURL& badge_icon_url,
-                  const SkBitmap* badge_icon,
+                  const GURL& splash_icon_url,
+                  const SkBitmap* splash_icon,
                   bool valid_manifest,
                   bool has_worker);
   ~InstallableData();
@@ -66,6 +66,17 @@ struct InstallableData {
   // using it.
   const SkBitmap* badge_icon;
 
+  // The URL of the chosen splash icon.
+  const GURL& splash_icon_url;
+
+  // nullptr if the most appropriate splash icon couldn't be determined or
+  // downloaded. The underlying splash icon is owned by the InstallableManager;
+  // clients must copy the bitmap if they want to use it. Since the splash
+  // icon is optional, no error code is set if it cannot be fetched, and clients
+  // specifying |valid_splash_icon| must check that the bitmap exists before
+  // using it.
+  const SkBitmap* splash_icon;
+
   // true if the site has a valid, installable web app manifest. If
   // |valid_manifest| or |has_worker| was true and the site isn't installable,
   // the reason will be in |errors|.

chrome/browser/installable/installable_manager.cc

@@ -56,10 +56,6 @@ const int kMinimumPrimaryIconSizeInPx = 144;
 // resized).
 const int kMinimumPrimaryAdaptiveLauncherIconSizeInPx = 83;
 
-#if !defined(OS_ANDROID)
-const int kMinimumBadgeIconSizeInPx = 72;
-#endif
-
 int GetIdealPrimaryIconSizeInPx() {
 #if defined(OS_ANDROID)
   return ShortcutHelper::GetIdealHomescreenIconSizeInPx();
@@ -76,19 +72,27 @@ int GetMinimumPrimaryIconSizeInPx() {
 #endif
 }
 
-int GetIdealBadgeIconSizeInPx() {
+int GetIdealPrimaryAdaptiveLauncherIconSizeInPx() {
 #if defined(OS_ANDROID)
-  return ShortcutHelper::GetIdealBadgeIconSizeInPx();
+  return ShortcutHelper::GetIdealAdaptiveLauncherIconSizeInPx();
 #else
-  return kMinimumBadgeIconSizeInPx;
+  return kMinimumPrimaryAdaptiveLauncherIconSizeInPx;
 #endif
 }
 
-int GetIdealPrimaryAdaptiveLauncherIconSizeInPx() {
+int GetIdealSplashIconSizeInPx() {
 #if defined(OS_ANDROID)
-  return ShortcutHelper::GetIdealAdaptiveLauncherIconSizeInPx();
+  return ShortcutHelper::GetIdealSplashImageSizeInPx();
 #else
-  return kMinimumPrimaryAdaptiveLauncherIconSizeInPx;
+  return kMinimumPrimaryIconSizeInPx;
+#endif
+}
+
+int GetMinimumSplashIconSizeInPx() {
+#if defined(OS_ANDROID)
+  return ShortcutHelper::GetMinimumSplashImageSizeInPx();
+#else
+  return kMinimumPrimaryIconSizeInPx;
 #endif
 }
 
@@ -373,17 +377,17 @@ std::vector<InstallableStatusCode> InstallableManager::GetErrors(
     // If the icon is MASKABLE, ignore any error since we want to fallback to
     // fetch IconPurpose::ANY.
     if (icon.error != NO_ERROR_DETECTED &&
-        icon.purpose != IconPurpose::MASKABLE) {
+        icon.purpose != IconPurpose::MASKABLE)
       errors.push_back(icon.error);
-    }
   }
 
-  if (params.valid_badge_icon) {
-    IconProperty& icon = icons_[IconUsage::kBadge];
+  if (params.valid_splash_icon) {
+    IconProperty& icon = icons_[IconUsage::kSplash];
 
-    // If the error is NO_ACCEPTABLE_ICON, there is no icon suitable as a badge
-    // in the manifest. Ignore this case since we only want to fail the check if
-    // there was a suitable badge icon specified and we couldn't fetch it.
+    // If the error is NO_ACCEPTABLE_ICON, there is no icon suitable as a splash
+    // icon in the manifest. Ignore this case since we only want to fail the
+    // check if there was a suitable splash icon specified and we couldn't fetch
+    // it.
     if (icon.error != NO_ERROR_DETECTED && icon.error != NO_ACCEPTABLE_ICON)
       errors.push_back(icon.error);
   }
@@ -445,7 +449,7 @@ bool InstallableManager::IsComplete(const InstallableParams& params) const {
          (!params.has_worker || worker_->fetched) &&
          (!params.valid_primary_icon ||
           IsIconFetchComplete(IconUsage::kPrimary)) &&
-         (!params.valid_badge_icon || IsIconFetchComplete(IconUsage::kBadge));
+         (!params.valid_splash_icon || IsIconFetchComplete(IconUsage::kSplash));
 }
 
 void InstallableManager::Reset() {
@@ -469,7 +473,7 @@ void InstallableManager::SetManifestDependentTasksComplete() {
   valid_manifest_->fetched = true;
   worker_->fetched = true;
   SetIconFetched(IconUsage::kPrimary);
-  SetIconFetched(IconUsage::kBadge);
+  SetIconFetched(IconUsage::kSplash);
 }
 
 void InstallableManager::CleanupAndStartNextTask() {
@@ -490,20 +494,20 @@ void InstallableManager::RunCallback(
   IconProperty null_icon;
   IconProperty* primary_icon = &null_icon;
   bool has_maskable_primary_icon = false;
-  IconProperty* badge_icon = &null_icon;
+  IconProperty* splash_icon = &null_icon;
 
   if (params.valid_primary_icon && IsIconFetchComplete(IconUsage::kPrimary)) {
     primary_icon = &icons_[IconUsage::kPrimary];
     has_maskable_primary_icon =
         (primary_icon->purpose == IconPurpose::MASKABLE);
   }
-  if (params.valid_badge_icon && IsIconFetchComplete(IconUsage::kBadge))
-    badge_icon = &icons_[IconUsage::kBadge];
+  if (params.valid_splash_icon && IsIconFetchComplete(IconUsage::kSplash))
+    splash_icon = &icons_[IconUsage::kSplash];
 
   InstallableData data = {
       std::move(errors),   manifest_url(),           &manifest(),
       primary_icon->url,   primary_icon->icon.get(), has_maskable_primary_icon,
-      badge_icon->url,     badge_icon->icon.get(),   valid_manifest_->is_valid,
+      splash_icon->url,    splash_icon->icon.get(),  valid_manifest_->is_valid,
       worker_->has_worker,
   };
 
@@ -549,11 +553,11 @@ void InstallableManager::WorkOnTask() {
                        params.prefer_maskable_icon);
   } else if (params.has_worker && !worker_->fetched) {
     CheckServiceWorker();
-  } else if (params.valid_badge_icon &&
-             !IsIconFetchComplete(IconUsage::kBadge)) {
-    CheckAndFetchBestIcon(GetIdealBadgeIconSizeInPx(),
-                          GetIdealBadgeIconSizeInPx(), IconPurpose::BADGE,
-                          IconUsage::kBadge);
+  } else if (params.valid_splash_icon &&
+             !IsIconFetchComplete(IconUsage::kSplash)) {
+    CheckAndFetchBestIcon(GetIdealSplashIconSizeInPx(),
+                          GetMinimumSplashIconSizeInPx(), IconPurpose::ANY,
+                          IconUsage::kSplash);
   } else {
     NOTREACHED();
   }

chrome/browser/installable/installable_manager.h

@@ -95,7 +95,7 @@ class InstallableManager
 
   using IconPurpose = blink::Manifest::ImageResource::Purpose;
 
-  enum class IconUsage { kPrimary, kBadge };
+  enum class IconUsage { kPrimary, kSplash };
 
   struct EligiblityProperty {
     EligiblityProperty();

chrome/browser/installable/installable_manager_unittest.cc

@@ -205,7 +205,7 @@ TEST_F(InstallableManagerUnitTest, ManifestRequiresPurposeAny) {
   blink::Manifest manifest = GetValidManifest();
 
   // The icon MUST have IconPurpose::ANY at least.
-  manifest.icons[0].purpose[0] = IconPurpose::BADGE;
+  manifest.icons[0].purpose[0] = IconPurpose::MASKABLE;
   EXPECT_FALSE(IsManifestValid(manifest));
   EXPECT_EQ(MANIFEST_MISSING_SUITABLE_ICON, GetErrorCode());
 

chrome/browser/installable/installable_params.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/installable/installable_params.h"
+
+InstallableParams::InstallableParams() = default;
+InstallableParams::InstallableParams(const InstallableParams&) = default;

chrome/browser/installable/installable_params.h

@@ -11,6 +11,8 @@
 // true, otherwise, all tasks will be run and a complete list of errors will be
 // returned.
 struct InstallableParams {
+  InstallableParams();
+  InstallableParams(const InstallableParams&);
   // Check whether the current WebContents is eligible to be installed, i.e it:
   //  - is served over HTTPS
   //  - is a top-level frame
@@ -28,6 +30,10 @@ struct InstallableParams {
   // conforming to the badge icon size parameters.
   bool valid_badge_icon = false;
 
+  // Check whether there is a fetchable, non-empty icon in the manifest
+  // conforming to the splash icon size parameters.
+  bool valid_splash_icon = false;
+
   // Check whether the site has a manifest valid for a web app.
   bool valid_manifest = false;
 

chrome/browser/installable/installable_task_queue_unittest.cc

@@ -13,7 +13,7 @@ struct TaskParams {
   bool valid_manifest = false;
   bool has_worker = false;
   bool valid_primary_icon = false;
-  bool valid_badge_icon = false;
+  bool valid_splash_icon = false;
 };
 
 // Constructs an InstallableTask, with the supplied bools stored in it.
@@ -22,7 +22,7 @@ InstallableTask CreateTask(const TaskParams& params) {
   task.params.valid_manifest = params.valid_manifest;
   task.params.has_worker = params.has_worker;
   task.params.valid_primary_icon = params.valid_primary_icon;
-  task.params.valid_badge_icon = params.valid_badge_icon;
+  task.params.valid_splash_icon = params.valid_splash_icon;
   return task;
 }
 
@@ -30,7 +30,7 @@ bool IsEqual(const TaskParams& params, const InstallableTask& task) {
   return task.params.valid_manifest == params.valid_manifest &&
          task.params.has_worker == params.has_worker &&
          task.params.valid_primary_icon == params.valid_primary_icon &&
-         task.params.valid_badge_icon == params.valid_badge_icon;
+         task.params.valid_splash_icon == params.valid_splash_icon;
 }
 
 class InstallableTaskQueueUnitTest : public testing::Test {};

chrome/test/data/banners/manifest_bad_badge.json → chrome/test/data/banners/manifest_bad_non_maskable_icon.json

 {
   "name": "Manifest test app",
   "icons": [
-    {
-      "src": "launcher-icon-1x.png",
-      "sizes": "48x48",
-      "type": "image/png"
-    },
-    {
-      "src": "launcher-icon-1-5x.png",
-      "sizes": "72x72",
-      "type": "image/png"
-    },
-    {
-      "src": "bad_icon.png",
-      "sizes": "96x96",
-      "type": "image/png",
-      "purpose": "any badge"
-    },
-    {
-      "src": "launcher-icon-3x.png",
-      "sizes": "144x144",
-      "type": "image/png"
-    },
     {
       "src": "launcher-icon-4x.png",
       "sizes": "192x192",
-      "type": "image/png"
+      "type": "image/png",
+      "purpose": "maskable"
     },
     {
-      "src": "image-512px.png",
+      "src": "bad_icon.png",
       "sizes": "512x512",
       "type": "image/png"
     }
@@ -36,4 +16,4 @@
   "start_url": "manifest_test_page.html",
   "display": "standalone",
   "orientation": "landscape"
-}
+}
\ No newline at end of file

chrome/test/data/banners/manifest_one_maskable.json

+{
+  "name": "Manifest test app",
+  "icons": [
+    {
+      "src": "image-512px.png",
+      "sizes": "512x512",
+      "type": "image/png",
+      "purpose": "maskable"
+    }
+  ],
+  "start_url": "manifest_test_page.html",
+  "display": "standalone",
+  "orientation": "landscape"
+}