maskable: Add/expose FindIconMatchBigger in app_icon_manager.

This is used to synchronously query for available icons matching the
desired parameters. Needed for app_icon_factory.
Will allow other ReadSmallest[Compressed]Icon calls to take a set
purpose and size instead of a vector<IconPurpose> and min_size.

Bug: 1102701
Change-Id: I592db59435badbbd6e63bf6c485b505ad4c86088
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2355159
Commit-Queue: Glen Robertson <glenrob@chromium.org>
Reviewed-by: Alexey Baskakov <loyso@chromium.org>
Cr-Commit-Position: refs/heads/master@{#798563}

chrome/browser/web_applications/components/app_icon_manager.h

@@ -11,6 +11,7 @@
 
 #include "base/callback_forward.h"
 #include "base/macros.h"
+#include "base/optional.h"
 #include "chrome/browser/web_applications/components/web_app_id.h"
 #include "chrome/common/web_application_info.h"
 #include "third_party/skia/include/core/SkBitmap.h"
@@ -47,11 +48,21 @@ class AppIconManager {
       const AppId& app_id,
       IconPurpose purpose,
       const std::vector<SquareSizePx>& icon_sizes_in_px) const = 0;
-  // Returns whether there is a downloaded icon matching |icon_size_in_px| for
-  // any of the given |purposes|.
+  struct IconSizeAndPurpose {
+    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.
+  virtual base::Optional<IconSizeAndPurpose> FindIconMatchBigger(
+      const AppId& app_id,
+      const std::vector<IconPurpose>& purposes,
+      SquareSizePx min_size) const = 0;
+  // Returns whether there is a downloaded icon of at least |min_size| for any
+  // of the given |purposes|.
   virtual bool HasSmallestIcon(const AppId& app_id,
                                const std::vector<IconPurpose>& purposes,
-                               SquareSizePx min_icon_size) const = 0;
+                               SquareSizePx min_size) const = 0;
 
   using ReadIconsCallback =
       base::OnceCallback<void(std::map<SquareSizePx, SkBitmap> icon_bitmaps)>;
@@ -83,9 +94,9 @@ class AppIconManager {
 
   using ReadIconWithPurposeCallback =
       base::OnceCallback<void(IconPurpose, const SkBitmap&)>;
-  // For each of |purposes|, in the given order, finds the smallest icon with
-  // size at least |icon_size_in_px|. Returns the first icon found, as a bitmap.
-  // Returns empty SkBitmap in |callback| if IO error.
+  // For each of |purposes|, in the given order, looks for an icon with size at
+  // least |min_icon_size|. Returns the first icon found, as a bitmap. Returns
+  // an empty SkBitmap in |callback| if IO error.
   virtual void ReadSmallestIcon(const AppId& app_id,
                                 const std::vector<IconPurpose>& purposes,
                                 SquareSizePx min_icon_size,
@@ -99,9 +110,9 @@ class AppIconManager {
 
   using ReadCompressedIconWithPurposeCallback =
       base::OnceCallback<void(IconPurpose, std::vector<uint8_t> data)>;
-  // For each of |purposes|, in the given order, finds the smallest icon with
-  // size at least |icon_size_in_px|. Returns the first icon found, compressed
-  // as PNG. Returns empty |data| in |callback| if IO error.
+  // For each of |purposes|, in the given order, looks for an icon with size at
+  // least |min_icon_size|. Returns the first icon found, compressed as PNG.
+  // Returns empty |data| in |callback| if IO error.
   virtual void ReadSmallestCompressedIcon(
       const AppId& app_id,
       const std::vector<IconPurpose>& purposes,

chrome/browser/web_applications/extensions/bookmark_app_icon_manager.cc

@@ -16,7 +16,6 @@
 #include "base/task/thread_pool.h"
 #include "chrome/browser/extensions/menu_manager.h"
 #include "chrome/browser/profiles/profile.h"
-#include "chrome/browser/web_applications/extensions/bookmark_app_registrar.h"
 #include "chrome/browser/web_applications/extensions/bookmark_app_util.h"
 #include "content/public/browser/browser_thread.h"
 #include "extensions/browser/image_loader.h"
@@ -202,23 +201,34 @@ bool BookmarkAppIconManager::HasIcons(
   return true;
 }
 
-bool BookmarkAppIconManager::HasSmallestIcon(
+base::Optional<web_app::AppIconManager::IconSizeAndPurpose>
+BookmarkAppIconManager::FindIconMatchBigger(
     const web_app::AppId& app_id,
     const std::vector<IconPurpose>& purposes,
-    SquareSizePx icon_size_in_px) const {
+    SquareSizePx min_size) const {
   const Extension* app = GetBookmarkApp(profile_, app_id);
   if (!app)
-    return false;
+    return base::nullopt;
   // Legacy bookmark apps handle IconPurpose::ANY icons only.
   if (!base::Contains(purposes, IconPurpose::ANY))
-    return false;
+    return base::nullopt;
 
   const ExtensionIconSet& icons = IconsInfo::GetIcons(app);
+  const std::string& path = icons.Get(min_size, ExtensionIconSet::MATCH_BIGGER);
+  // Returns 0 if path is empty or not found.
+  int found_icon_size = icons.GetIconSizeFromPath(path);
 
-  const std::string& path =
-      icons.Get(icon_size_in_px, ExtensionIconSet::MATCH_BIGGER);
+  if (found_icon_size == 0)
+    return base::nullopt;
 
-  return !path.empty();
+  return IconSizeAndPurpose{found_icon_size, IconPurpose::ANY};
+}
+
+bool BookmarkAppIconManager::HasSmallestIcon(
+    const web_app::AppId& app_id,
+    const std::vector<IconPurpose>& purposes,
+    SquareSizePx min_size) const {
+  return FindIconMatchBigger(app_id, purposes, min_size).has_value();
 }
 
 void BookmarkAppIconManager::ReadIcons(

chrome/browser/web_applications/extensions/bookmark_app_icon_manager.h

@@ -28,9 +28,13 @@ class BookmarkAppIconManager : public web_app::AppIconManager {
       const web_app::AppId& app_id,
       IconPurpose purpose,
       const std::vector<SquareSizePx>& icon_sizes_in_px) const override;
+  base::Optional<IconSizeAndPurpose> FindIconMatchBigger(
+      const web_app::AppId& app_id,
+      const std::vector<IconPurpose>& purposes,
+      SquareSizePx min_size) const override;
   bool HasSmallestIcon(const web_app::AppId& app_id,
                        const std::vector<IconPurpose>& purposes,
-                       SquareSizePx icon_size_in_px) const override;
+                       SquareSizePx min_size) const override;
   void ReadIcons(const web_app::AppId& app_id,
                  IconPurpose purpose,
                  const std::vector<SquareSizePx>& icon_sizes_in_px,

chrome/browser/web_applications/web_app_icon_manager.cc

@@ -569,12 +569,34 @@ bool WebAppIconManager::HasIcons(
                            icon_sizes_in_px);
 }
 
+base::Optional<AppIconManager::IconSizeAndPurpose>
+WebAppIconManager::FindIconMatchBigger(const AppId& app_id,
+                                       const std::vector<IconPurpose>& purposes,
+                                       SquareSizePx min_size) const {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
+  const WebApp* web_app = registrar_.GetAppById(app_id);
+  if (!web_app)
+    return base::nullopt;
+
+  // Must iterate through purposes in order given.
+  for (IconPurpose purpose : purposes) {
+    const std::vector<SquareSizePx>& sizes =
+        web_app->downloaded_icon_sizes(purpose);
+    DCHECK(base::STLIsSorted(sizes));
+    for (SquareSizePx size : sizes) {
+      if (size >= min_size)
+        return IconSizeAndPurpose{size, purpose};
+    }
+  }
+
+  return base::nullopt;
+}
+
 bool WebAppIconManager::HasSmallestIcon(
     const AppId& app_id,
     const std::vector<IconPurpose>& purposes,
-    SquareSizePx min_size_in_px) const {
-  return FindDownloadedIconMatchBigger(app_id, purposes, min_size_in_px)
-      .has_value();
+    SquareSizePx min_size) const {
+  return FindIconMatchBigger(app_id, purposes, min_size).has_value();
 }
 
 void WebAppIconManager::ReadIcons(const AppId& app_id,
@@ -638,8 +660,8 @@ void WebAppIconManager::ReadSmallestIcon(
     ReadIconWithPurposeCallback callback) const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  base::Optional<WebAppIconManager::SizeAndPurpose> best_icon =
-      FindDownloadedIconMatchBigger(app_id, purposes, min_size_in_px);
+  base::Optional<IconSizeAndPurpose> best_icon =
+      FindIconMatchBigger(app_id, purposes, min_size_in_px);
   DCHECK(best_icon.has_value());
   IconId icon_id(app_id, best_icon->purpose, best_icon->size_px);
   ReadIconCallback wrapped = base::BindOnce(
@@ -659,8 +681,8 @@ void WebAppIconManager::ReadSmallestCompressedIcon(
     ReadCompressedIconWithPurposeCallback callback) const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  base::Optional<WebAppIconManager::SizeAndPurpose> best_icon =
-      FindDownloadedIconMatchBigger(app_id, purposes, min_size_in_px);
+  base::Optional<IconSizeAndPurpose> best_icon =
+      FindIconMatchBigger(app_id, purposes, min_size_in_px);
   DCHECK(best_icon.has_value());
   IconId icon_id(app_id, best_icon->purpose, best_icon->size_px);
   ReadCompressedIconCallback wrapped =
@@ -693,11 +715,10 @@ void WebAppIconManager::ReadIconAndResize(const AppId& app_id,
                                           IconPurpose purpose,
                                           SquareSizePx desired_icon_size,
                                           ReadIconsCallback callback) const {
-  base::Optional<SizeAndPurpose> best_icon =
-      FindDownloadedIconMatchBigger(app_id, {purpose}, desired_icon_size);
+  base::Optional<IconSizeAndPurpose> best_icon =
+      FindIconMatchBigger(app_id, {purpose}, desired_icon_size);
   if (!best_icon) {
-    best_icon =
-        FindDownloadedIconMatchSmaller(app_id, {purpose}, desired_icon_size);
+    best_icon = FindIconMatchSmaller(app_id, {purpose}, desired_icon_size);
   }
 
   if (!best_icon) {
@@ -719,35 +740,11 @@ void WebAppIconManager::SetFaviconReadCallbackForTesting(
   favicon_read_callback_ = std::move(callback);
 }
 
-base::Optional<WebAppIconManager::SizeAndPurpose>
-WebAppIconManager::FindDownloadedIconMatchBigger(
-    const AppId& app_id,
-    const std::vector<IconPurpose>& purposes,
-    SquareSizePx desired_size) const {
-  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
-  const WebApp* web_app = registrar_.GetAppById(app_id);
-  if (!web_app)
-    return base::nullopt;
-
-  // Must iterate through purposes in order given.
-  for (IconPurpose purpose : purposes) {
-    const std::vector<SquareSizePx>& sizes =
-        web_app->downloaded_icon_sizes(purpose);
-    DCHECK(base::STLIsSorted(sizes));
-    for (SquareSizePx size : sizes) {
-      if (size >= desired_size)
-        return WebAppIconManager::SizeAndPurpose{size, purpose};
-    }
-  }
-
-  return base::nullopt;
-}
-
-base::Optional<WebAppIconManager::SizeAndPurpose>
-WebAppIconManager::FindDownloadedIconMatchSmaller(
+base::Optional<AppIconManager::IconSizeAndPurpose>
+WebAppIconManager::FindIconMatchSmaller(
     const AppId& app_id,
     const std::vector<IconPurpose>& purposes,
-    SquareSizePx desired_size) const {
+    SquareSizePx max_size) const {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
   const WebApp* web_app = registrar_.GetAppById(app_id);
   if (!web_app)
@@ -759,8 +756,8 @@ WebAppIconManager::FindDownloadedIconMatchSmaller(
         web_app->downloaded_icon_sizes(purpose);
     DCHECK(base::STLIsSorted(sizes));
     for (SquareSizePx size : base::Reversed(sizes)) {
-      if (size <= desired_size)
-        return WebAppIconManager::SizeAndPurpose{size, purpose};
+      if (size <= max_size)
+        return IconSizeAndPurpose{size, purpose};
     }
   }
 

chrome/browser/web_applications/web_app_icon_manager.h

@@ -57,9 +57,13 @@ class WebAppIconManager : public AppIconManager, public AppRegistrarObserver {
       const AppId& app_id,
       IconPurpose purpose,
       const std::vector<SquareSizePx>& icon_sizes_in_px) const override;
+  base::Optional<IconSizeAndPurpose> FindIconMatchBigger(
+      const AppId& app_id,
+      const std::vector<IconPurpose>& purposes,
+      SquareSizePx min_size) const override;
   bool HasSmallestIcon(const AppId& app_id,
                        const std::vector<IconPurpose>& purposes,
-                       SquareSizePx min_size_in_px) const override;
+                       SquareSizePx min_size) const override;
   void ReadIcons(const AppId& app_id,
                  IconPurpose purpose,
                  const std::vector<SquareSizePx>& icon_sizes,
@@ -96,18 +100,10 @@ class WebAppIconManager : public AppIconManager, public AppRegistrarObserver {
   void SetFaviconReadCallbackForTesting(FaviconReadCallback callback);
 
  private:
-  struct SizeAndPurpose {
-    SquareSizePx size_px = 0;
-    IconPurpose purpose = IconPurpose::ANY;
-  };
-  base::Optional<WebAppIconManager::SizeAndPurpose>
-  FindDownloadedIconMatchBigger(const AppId& app_id,
-                                const std::vector<IconPurpose>& purposes,
-                                SquareSizePx desired_size) const;
-  base::Optional<SizeAndPurpose> FindDownloadedIconMatchSmaller(
+  base::Optional<IconSizeAndPurpose> FindIconMatchSmaller(
       const AppId& app_id,
       const std::vector<IconPurpose>& purposes,
-      SquareSizePx desired_size) const;
+      SquareSizePx max_size) const;
 
   void ReadFavicon(const AppId& app_id);
   void OnReadFavicon(const AppId& app_id, const SkBitmap&);

chrome/browser/web_applications/web_app_icon_manager_unittest.cc

@@ -36,6 +36,12 @@
 
 namespace web_app {
 
+namespace {
+
+using IconSizeAndPurpose = AppIconManager::IconSizeAndPurpose;
+
+}  // namespace
+
 class WebAppIconManagerTest : public WebAppTest {
   void SetUp() override {
     WebAppTest::SetUp();
@@ -131,6 +137,41 @@ class WebAppIconManagerTest : public WebAppTest {
     return downloaded_shortcuts_menu_icons_sizes;
   }
 
+  struct PurposeAndBitmap {
+    IconPurpose purpose;
+    SkBitmap bitmap;
+  };
+  PurposeAndBitmap ReadSmallestIcon(const AppId& app_id,
+                                    const std::vector<IconPurpose>& purposes,
+                                    SquareSizePx min_icon_size) {
+    PurposeAndBitmap result;
+    base::RunLoop run_loop;
+    icon_manager().ReadSmallestIcon(
+        app_id, purposes, min_icon_size,
+        base::BindLambdaForTesting(
+            [&](IconPurpose purpose, const SkBitmap& bitmap) {
+              result.purpose = purpose;
+              result.bitmap = bitmap;
+              run_loop.Quit();
+            }));
+    run_loop.Run();
+    return result;
+  }
+
+  SkBitmap ReadSmallestIconAny(const AppId& app_id,
+                               SquareSizePx min_icon_size) {
+    SkBitmap result;
+    base::RunLoop run_loop;
+    icon_manager().ReadSmallestIconAny(
+        app_id, min_icon_size,
+        base::BindLambdaForTesting([&](const SkBitmap& bitmap) {
+          result = bitmap;
+          run_loop.Quit();
+        }));
+    run_loop.Run();
+    return result;
+  }
+
   struct PurposeAndData {
     IconPurpose purpose;
     std::vector<uint8_t> data;
@@ -658,6 +699,11 @@ TEST_F(WebAppIconManagerTest, FindExact) {
   }
 }
 
+// Simple struct doesn't have an operator==.
+bool operator==(const IconSizeAndPurpose& a, const IconSizeAndPurpose& b) {
+  return a.size_px == b.size_px && a.purpose == b.purpose;
+}
+
 TEST_F(WebAppIconManagerTest, FindSmallest) {
   auto web_app = CreateWebApp();
   const AppId app_id = web_app->app_id();
@@ -665,49 +711,79 @@ TEST_F(WebAppIconManagerTest, FindSmallest) {
   const std::vector<int> sizes_px{10, 60, 50, 20, 30};
   const std::vector<SkColor> colors{SK_ColorRED, SK_ColorYELLOW, SK_ColorGREEN,
                                     SK_ColorBLUE, SK_ColorMAGENTA};
-  WriteIcons(app_id, {IconPurpose::ANY}, sizes_px, colors);
+  WriteIcons(app_id, {IconPurpose::ANY, IconPurpose::MASKABLE}, sizes_px,
+             colors);
 
   web_app->SetDownloadedIconSizes(IconPurpose::ANY, sizes_px);
+  // Pretend we only have one size of maskable icon.
+  web_app->SetDownloadedIconSizes(IconPurpose::MASKABLE, {20});
 
   controller().RegisterApp(std::move(web_app));
 
   EXPECT_FALSE(icon_manager().HasSmallestIcon(app_id, {IconPurpose::ANY}, 70));
+  EXPECT_EQ(base::nullopt,
+            icon_manager().FindIconMatchBigger(app_id, {IconPurpose::ANY}, 70));
+
   EXPECT_FALSE(icon_manager().HasSmallestIcon(
       app_id, {IconPurpose::ANY, IconPurpose::MASKABLE}, 70));
+  EXPECT_EQ(base::nullopt,
+            icon_manager().FindIconMatchBigger(
+                app_id, {IconPurpose::ANY, IconPurpose::MASKABLE}, 70));
+
   EXPECT_FALSE(
       icon_manager().HasSmallestIcon(app_id, {IconPurpose::MASKABLE}, 40));
+  EXPECT_EQ(base::nullopt, icon_manager().FindIconMatchBigger(
+                               app_id, {IconPurpose::MASKABLE}, 40));
 
   EXPECT_TRUE(icon_manager().HasSmallestIcon(
       app_id, {IconPurpose::MASKABLE, IconPurpose::ANY}, 40));
+  EXPECT_EQ((IconSizeAndPurpose{50, IconPurpose::ANY}),
+            icon_manager()
+                .FindIconMatchBigger(
+                    app_id, {IconPurpose::MASKABLE, IconPurpose::ANY}, 40)
+                .value());
+
   EXPECT_TRUE(icon_manager().HasSmallestIcon(
       app_id, {IconPurpose::ANY, IconPurpose::MASKABLE}, 20));
+  EXPECT_EQ((IconSizeAndPurpose{20, IconPurpose::ANY}),
+            icon_manager()
+                .FindIconMatchBigger(
+                    app_id, {IconPurpose::ANY, IconPurpose::MASKABLE}, 20)
+                .value());
 
-  {
-    base::RunLoop run_loop;
+  EXPECT_TRUE(icon_manager().HasSmallestIcon(
+      app_id, {IconPurpose::MASKABLE, IconPurpose::ANY}, 10));
+  EXPECT_EQ((IconSizeAndPurpose{20, IconPurpose::MASKABLE}),
+            icon_manager()
+                .FindIconMatchBigger(
+                    app_id, {IconPurpose::MASKABLE, IconPurpose::ANY}, 10)
+                .value());
 
+  {
     EXPECT_TRUE(icon_manager().HasSmallestIcon(app_id, {IconPurpose::ANY}, 40));
-    icon_manager().ReadSmallestIconAny(
-        app_id, 40, base::BindLambdaForTesting([&](const SkBitmap& bitmap) {
+    SkBitmap bitmap = ReadSmallestIconAny(app_id, 40);
     EXPECT_FALSE(bitmap.empty());
     EXPECT_EQ(SK_ColorGREEN, bitmap.getColor(0, 0));
-          run_loop.Quit();
-        }));
-
-    run_loop.Run();
   }
-
   {
-    base::RunLoop run_loop;
-
     EXPECT_TRUE(icon_manager().HasSmallestIcon(app_id, {IconPurpose::ANY}, 20));
-    icon_manager().ReadSmallestIconAny(
-        app_id, 20, base::BindLambdaForTesting([&](const SkBitmap& bitmap) {
+    SkBitmap bitmap = ReadSmallestIconAny(app_id, 20);
     EXPECT_FALSE(bitmap.empty());
     EXPECT_EQ(SK_ColorBLUE, bitmap.getColor(0, 0));
-          run_loop.Quit();
-        }));
-
-    run_loop.Run();
+  }
+  {
+    PurposeAndBitmap result =
+        ReadSmallestIcon(app_id, {IconPurpose::ANY, IconPurpose::MASKABLE}, 20);
+    EXPECT_FALSE(result.bitmap.empty());
+    EXPECT_EQ(IconPurpose::ANY, result.purpose);
+    EXPECT_EQ(SK_ColorBLUE, result.bitmap.getColor(0, 0));
+  }
+  {
+    PurposeAndBitmap result =
+        ReadSmallestIcon(app_id, {IconPurpose::MASKABLE, IconPurpose::ANY}, 20);
+    EXPECT_FALSE(result.bitmap.empty());
+    EXPECT_EQ(IconPurpose::MASKABLE, result.purpose);
+    EXPECT_EQ(SK_ColorBLUE, result.bitmap.getColor(0, 0));
   }
 }