Select Best Home Tab Icon For Tabbed PWAs

Currently, the first home tab icon that is provided by the user is being used. We want to select the best home tab icon by finding the icon with the smallest size with a purpose that is preferably 'ANY'.

Design-Document: https://docs.google.com/document/d/1lG-IzUbq4dYgNcgyicLk0_RiGHZsJA6H1vv9fBjHd2w/edit?usp=sharing&resourcekey=0-b5ch9EAJKJ8n04rKeUqlTQ

Change-Id: I111667b428762fb24fdead190f88c59d1cf6cde0
Bug: 1381377
Co-author: gracekan@
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4227773
Reviewed-by: Alan Cutter <alancutter@chromium.org>
Commit-Queue: Kasie Wang <kasiew@google.com>
Cr-Commit-Position: refs/heads/main@{#1104892}

chrome/browser/ui/views/web_apps/web_app_tab_strip_browsertest.cc

@@ -335,7 +335,7 @@ IN_PROC_BROWSER_TEST_F(WebAppTabStripBrowserTest, NoHomeTabIcons) {
   EXPECT_EQ(tab_strip->active_index(), 0);
 }
 
-IN_PROC_BROWSER_TEST_F(WebAppTabStripBrowserTest, HomeTabIcons) {
+IN_PROC_BROWSER_TEST_F(WebAppTabStripBrowserTest, SelectingBestHomeTabIcon) {
   GURL start_url =
       embedded_test_server()->GetURL("/web_apps/tab_strip_customizations.html");
 

chrome/browser/ui/web_applications/web_app_browser_controller.cc

@@ -66,6 +66,8 @@
 
 namespace {
 
+const int kMinimumHomeTabIconSizeInPx = 16;
+
 #if BUILDFLAG(IS_CHROMEOS)
 constexpr char kRelationship[] = "delegate_permission/common.handle_all_urls";
 #endif
@@ -365,8 +367,8 @@ gfx::ImageSkia WebAppBrowserController::GetHomeTabIcon() const {
     if (const auto* params =
             absl::get_if<blink::Manifest::HomeTabParams>(&tab_strip.home_tab)) {
       if (!params->icons.empty()) {
-        provider_->icon_manager().ReadAllHomeTabIcons(
-            app_id(), params->icons,
+        provider_->icon_manager().ReadBestHomeTabIcon(
+            app_id(), params->icons, kMinimumHomeTabIconSizeInPx,
             base::BindOnce(&WebAppBrowserController::OnReadHomeTabIcon,
                            weak_ptr_factory_.GetWeakPtr()));
       }
@@ -633,14 +635,13 @@ void WebAppBrowserController::OnLoadIcon(apps::IconValuePtr icon_value) {
 }
 
 void WebAppBrowserController::OnReadHomeTabIcon(
-    HomeTabIconBitmaps home_tab_icon_bitmaps) const {
-  if (home_tab_icon_bitmaps.empty()) {
+    SkBitmap home_tab_icon_bitmap) const {
+  if (home_tab_icon_bitmap.empty()) {
     DLOG(ERROR) << "Failed to read icon for the pinned home tab";
     return;
   }
-  // TODO (crbug.com/1381377): Select the most appropriate icon instead of just
-  // picking the first one
-  home_tab_icon_ = gfx::ImageSkia::CreateFrom1xBitmap(home_tab_icon_bitmaps[0]);
+
+  home_tab_icon_ = gfx::ImageSkia::CreateFrom1xBitmap(home_tab_icon_bitmap);
   if (auto* contents = web_contents()) {
     contents->NotifyNavigationStateChanged(content::INVALIDATE_TYPE_TAB);
   }

chrome/browser/ui/web_applications/web_app_browser_controller.h

@@ -144,7 +144,8 @@ class WebAppBrowserController : public AppBrowserController,
 
   // Invoked when the icon is loaded.
   void OnLoadIcon(apps::IconValuePtr icon_value);
-  void OnReadHomeTabIcon(HomeTabIconBitmaps home_tab_icon_bitmaps) const;
+
+  void OnReadHomeTabIcon(SkBitmap home_tab_icon_bitmap) const;
   void OnReadIcon(IconPurpose purpose, SkBitmap bitmap);
   void PerformDigitalAssetLinkVerification(Browser* browser);
 

chrome/browser/web_applications/web_app_icon_manager.cc

@@ -5,6 +5,7 @@
 #include "chrome/browser/web_applications/web_app_icon_manager.h"
 
 #include <array>
+#include <cmath>
 #include <cstdint>
 #include <functional>
 #include <initializer_list>
@@ -110,6 +111,11 @@ struct IconId {
   SquareSizePx size;
 };
 
+struct IconSrcAndSize {
+  GURL src = GURL();
+  SquareSizePx size_px = 0;
+};
+
 // This is a private implementation detail of WebAppIconManager, where and how
 // to store icon files.
 // `app_manifest_resources_directory` is the path to the app-specific
@@ -302,7 +308,7 @@ TypedResult<SkBitmap> ReadShortcutsMenuIconBlocking(
 }
 
 // Returns empty SkBitmap if any errors occurred.
-TypedResult<SkBitmap> ReadHomeTabIconBlocking(
+TypedResult<SkBitmap> ReadHomeTabIconFromFileBlocking(
     scoped_refptr<FileUtilsWrapper> utils,
     const base::FilePath& web_apps_directory,
     const AppId& app_id,
@@ -455,21 +461,72 @@ TypedResult<ShortcutsMenuIconBitmaps> ReadShortcutsMenuIconsBlocking(
   return results;
 }
 
-TypedResult<HomeTabIconBitmaps> ReadHomeTabIconsBlocking(
+// Returns an IconSrcAndSize for the smallest icon that is larger than the
+// minimum size specified. If there is no icon larger than the minimum size
+// the IconSrcAndSize of the largest icon available is returned. An icon
+// prioritises matching purpose before size. The purpose of the returned icon is
+// specified by the decreasing order of preference in |purposes|.
+absl::optional<IconSrcAndSize> FindBestImageResourceMatch(
+    const std::vector<IconPurpose>& purposes,
+    const std::vector<blink::Manifest::ImageResource>& icons,
+    SquareSizePx min_size) {
+  IconSrcAndSize best_match;
+  IconSrcAndSize biggest_icon;
+  best_match.size_px = INT_MAX;
+  biggest_icon.size_px = INT_MIN;
+
+  for (const IconPurpose& purpose : purposes) {
+    for (const blink::Manifest::ImageResource& icon : icons) {
+      // TODO(crbug.com/1381377): Need to add check if icon has been
+      // successfully downloaded.
+      if (!base::Contains(icon.purpose, purpose)) {
+        continue;
+      }
+      for (const gfx::Size& icon_size : icon.sizes) {
+        if (icon_size.width() > biggest_icon.size_px) {
+          biggest_icon.size_px = icon_size.width();
+          biggest_icon.src = icon.src;
+        }
+        if (icon_size.width() < min_size) {
+          continue;
+        }
+        if (icon_size.width() > best_match.size_px) {
+          continue;
+        }
+        best_match.size_px = icon_size.width();
+        best_match.src = icon.src;
+      }
+    }
+    if (!best_match.src.is_empty()) {
+      return best_match;
+    }
+    if (!biggest_icon.src.is_empty()) {
+      return biggest_icon;
+    }
+  }
+
+  return absl::nullopt;
+}
+
+TypedResult<SkBitmap> ReadHomeTabIconBlocking(
     scoped_refptr<FileUtilsWrapper> utils,
     const base::FilePath& web_apps_directory,
     const AppId& app_id,
-    const std::vector<blink::Manifest::ImageResource>& icons) {
-  TypedResult<HomeTabIconBitmaps> results;
-  for (const blink::Manifest::ImageResource& icon : icons) {
-    TypedResult<SkBitmap> result = ReadHomeTabIconBlocking(
-        utils, web_apps_directory, app_id, icon.src, icon.sizes[0].width());
-    result.DepositErrorLog(results.error_log);
-    if (!result.value.empty()) {
-      results.value.push_back(std::move(result.value));
-    }
+    const std::vector<blink::Manifest::ImageResource>& icons,
+    const SquareSizePx min_home_tab_icon_size_px) {
+  absl::optional<IconSrcAndSize> best_icon = FindBestImageResourceMatch(
+      {IconPurpose::ANY}, icons, min_home_tab_icon_size_px);
+  TypedResult<SkBitmap> result;
+
+  // Returns empty SkBitmap if |best_icon| not found
+  if (!best_icon.has_value()) {
+    result.error_log = {CreateError({"No suitable home tab icon found"})};
+  } else {
+    result = ReadHomeTabIconFromFileBlocking(
+        utils, web_apps_directory, app_id, best_icon->src, best_icon->size_px);
   }
-  return results;
+
+  return result;
 }
 
 TypedResult<WebAppIconManager::ShortcutIconDataVector>
@@ -842,7 +899,7 @@ class WriteIconsJob {
   AppId app_id_;
   IconBitmaps icon_bitmaps_;
   ShortcutsMenuIconBitmaps shortcuts_menu_icon_bitmaps_;
-  HomeTabIconBitmaps home_tab_icon_bitmaps_;
+  SkBitmap home_tab_icon_bitmap_;
   IconsMap other_icons_;
 };
 
@@ -1056,21 +1113,22 @@ void WebAppIconManager::ReadAllShortcutsMenuIcons(
                      GetWeakPtr(), std::move(callback)));
 }
 
-void WebAppIconManager::ReadAllHomeTabIcons(
+void WebAppIconManager::ReadBestHomeTabIcon(
     const AppId& app_id,
     const std::vector<blink::Manifest::ImageResource>& icons,
+    const SquareSizePx min_home_tab_icon_size_px,
     ReadHomeTabIconsCallback callback) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   const WebApp* web_app = registrar_->GetAppById(app_id);
   if (!web_app) {
-    std::move(callback).Run(HomeTabIconBitmaps{});
+    std::move(callback).Run(SkBitmap());
     return;
   }
   icon_task_runner_->PostTaskAndReplyWithResult(
       FROM_HERE,
-      base::BindOnce(ReadHomeTabIconsBlocking, utils_, web_apps_directory_,
-                     app_id, icons),
-      base::BindOnce(&LogErrorsCallCallback<HomeTabIconBitmaps>, GetWeakPtr(),
+      base::BindOnce(ReadHomeTabIconBlocking, utils_, web_apps_directory_,
+                     app_id, icons, min_home_tab_icon_size_px),
+      base::BindOnce(&LogErrorsCallCallback<SkBitmap>, GetWeakPtr(),
                      std::move(callback)));
 }
 

chrome/browser/web_applications/web_app_icon_manager.h

@@ -26,6 +26,7 @@
 #include "third_party/abseil-cpp/absl/types/optional.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/image/image_skia.h"
+#include "web_app_install_info.h"
 
 class Profile;
 
@@ -80,6 +81,7 @@ class WebAppIconManager : public WebAppInstallManagerObserver {
     SquareSizePx size_px = 0;
     IconPurpose purpose = IconPurpose::ANY;
   };
+
   // For each of |purposes|, in the given order, looks for an icon with size at
   // least |min_icon_size|. Returns information on the first icon found.
   absl::optional<IconSizeAndPurpose> FindIconMatchBigger(
@@ -136,14 +138,15 @@ class WebAppIconManager : public WebAppInstallManagerObserver {
                                  ReadShortcutsMenuIconsCallback callback);
 
   using ReadHomeTabIconsCallback =
-      base::OnceCallback<void(HomeTabIconBitmaps home_tab_icon_bitmaps)>;
+      base::OnceCallback<void(SkBitmap home_tab_icon_bitmap)>;
 
   // Reads bitmap for the home tab icon. Returns a SkBitmap
   // in |callback| if the icon exists. Otherwise, if it doesn't
   // exist, the SkBitmap is empty.
-  void ReadAllHomeTabIcons(
+  void ReadBestHomeTabIcon(
       const AppId& app_id,
       const std::vector<blink::Manifest::ImageResource>& icons,
+      const SquareSizePx min_home_tab_icon_size_px,
       ReadHomeTabIconsCallback callback);
 
   using ReadIconWithPurposeCallback =

chrome/browser/web_applications/web_app_icon_manager_unittest.cc

@@ -50,6 +50,7 @@ namespace web_app {
 namespace {
 
 using IconSizeAndPurpose = WebAppIconManager::IconSizeAndPurpose;
+const int kMinimumHomeTabIconSizeInPx = 16;
 
 }  // namespace
 
@@ -741,6 +742,28 @@ TEST_F(WebAppIconManagerTest, ReadAllShortcutMenuIconsWithTimestamp) {
       time_data_map[1][IconPurpose::MONOCHROME][icon_size::k128].is_null());
 }
 
+TEST_F(WebAppIconManagerTest, NoHomeTabIcons) {
+  auto web_app = test::CreateWebApp();
+  const AppId app_id = web_app->app_id();
+  const std::vector<blink::Manifest::ImageResource>& icons{};
+  SkBitmap result;
+
+  AddAppToRegistry(std::move(web_app));
+  {
+    base::RunLoop run_loop;
+
+    icon_manager().ReadBestHomeTabIcon(
+        app_id, icons, kMinimumHomeTabIconSizeInPx,
+        base::BindLambdaForTesting([&](SkBitmap home_tab_icon) {
+          result = std::move(home_tab_icon);
+          run_loop.Quit();
+        }));
+
+    run_loop.Run();
+    EXPECT_TRUE(result.height() == 0 && result.width() == 0);
+  }
+}
+
 TEST_F(WebAppIconManagerTest, ReadAllIcons_AnyAndMaskable) {
   auto web_app = test::CreateWebApp();
   const AppId app_id = web_app->app_id();

chrome/test/data/web_apps/tab_strip_customizations.json

@@ -15,7 +15,7 @@
       "icons" : [
         {
           "src": "blue-192.png",
-          "sizes": "192x192",
+          "sizes": "200x200 192x192 300x300",
           "type": "image/png"
         }
       ]