Have LoadIconFromExtension process icon images

Image processing includes converting to gray (for disabled extensions)
and rounding the corners (for bookmark apps).

BUG=826982

Change-Id: Ie49a8928678abd884302eacb7bef5f2c21e46a35
Reviewed-on: https://chromium-review.googlesource.com/c/1366462
Commit-Queue: Nigel Tao <nigeltao@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Reviewed-by: Dominick Ng <dominickn@chromium.org>
Cr-Commit-Position: refs/heads/master@{#615745}

chrome/browser/apps/app_service/app_icon_factory.cc

@@ -14,7 +14,11 @@
 #include "base/numerics/safe_conversions.h"
 #include "base/task/post_task.h"
 #include "base/task/task_traits.h"
+#include "chrome/browser/extensions/chrome_app_icon.h"
+#include "chrome/browser/extensions/chrome_app_icon_loader.h"
 #include "chrome/browser/extensions/extension_service.h"
+#include "chrome/browser/extensions/extension_util.h"
+#include "chrome/browser/ui/app_list/md_icon_normalizer.h"
 #include "extensions/browser/extension_system.h"
 #include "extensions/browser/image_loader.h"
 #include "extensions/common/manifest_handlers/icons_handler.h"
@@ -26,6 +30,10 @@
 #include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_skia_operations.h"
 
+#if defined(OS_CHROMEOS)
+#include "chrome/browser/chromeos/extensions/gfx_utils.h"
+#endif
+
 namespace {
 
 float GetDeviceScaleFactor() {
@@ -70,12 +78,29 @@ void RunCallbackWithUncompressedImageSkia(
   std::move(callback).Run(std::move(iv));
 }
 
-// Runs |callback| passing an IconValuePtr with an uncompressed image.
+// Runs |callback| passing an IconValuePtr with a filtered, uncompressed image.
 void RunCallbackWithUncompressedImage(
+    base::OnceCallback<void(gfx::ImageSkia*)> image_filter,
     apps::mojom::Publisher::LoadIconCallback callback,
     const gfx::Image& image) {
-  RunCallbackWithUncompressedImageSkia(std::move(callback),
-                                       image.AsImageSkia());
+  gfx::ImageSkia image_skia = image.AsImageSkia();
+  std::move(image_filter).Run(&image_skia);
+  RunCallbackWithUncompressedImageSkia(std::move(callback), image_skia);
+}
+
+// Forwards to extensions::ChromeAppIcon::ApplyEffects, with subtle differences
+// in argument types. For example, resize_function is a "ResizeFunction" here,
+// but a "const ResizeFunction&" in extensions::ChromeAppIcon::ApplyEffects.
+void ChromeAppIconApplyEffects(
+    int resource_size_in_dip,
+    extensions::ChromeAppIconLoader::ResizeFunction resize_function,
+    bool apply_chrome_badge,
+    bool app_launchable,
+    bool from_bookmark,
+    gfx::ImageSkia* image_skia) {
+  extensions::ChromeAppIcon::ApplyEffects(resource_size_in_dip, resize_function,
+                                          apply_chrome_badge, app_launchable,
+                                          from_bookmark, image_skia);
 }
 
 }  // namespace
@@ -98,40 +123,43 @@ void LoadIconFromExtension(apps::mojom::IconCompression icon_compression,
         extensions::IconsInfo::GetIconResource(extension, size_hint_in_px,
                                                ExtensionIconSet::MATCH_BIGGER);
 
-    // TODO(crbug.com/826982): at some point near here, we should call into
-    // chrome/browser/extensions/chrome_app_icon.h code to get e.g. graying out
-    // disabled icons, or rounding off corners. It's not entirely trivial. The
-    // ChromeAppIcon object expects to be long-lived, calling back into the
-    // ChromeAppIconDelegate e.g. when the extension's underlying icon changes,
-    // or when an enabled/disabled state change means graying or un-graying an
-    // extension's icon.
-    //
-    // For example, in chrome/browser/ui/app_list, ExtensionAppItem implements
-    // ChromeAppIconDelegate, as the ExtensionAppItem is a long-lived object.
-    // It is a model, in the model-view-controller sense.
-    //
-    // In contrast, this API here (a Mojo IPC API) is a request-response IPC,
-    // with nothing long-lived enough to be the ChromeAppIconDelegate.
-
     switch (icon_compression) {
       case apps::mojom::IconCompression::kUnknown:
         break;
 
-      case apps::mojom::IconCompression::kUncompressed:
+      case apps::mojom::IconCompression::kUncompressed: {
+        bool apply_chrome_badge = false;
+#if defined(OS_CHROMEOS)
+        apply_chrome_badge =
+            extensions::util::ShouldApplyChromeBadge(context, extension_id);
+#endif
+
         extensions::ImageLoader::Get(context)->LoadImageAsync(
             extension, std::move(ext_resource),
             gfx::Size(size_hint_in_px, size_hint_in_px),
-            base::BindOnce(&RunCallbackWithUncompressedImage,
-                           std::move(callback)));
+            base::BindOnce(
+                &RunCallbackWithUncompressedImage,
+                base::BindOnce(
+                    &ChromeAppIconApplyEffects, size_hint_in_dip,
+                    base::BindRepeating(&app_list::MaybeResizeAndPadIconForMd),
+                    apply_chrome_badge,
+                    extensions::util::IsAppLaunchable(extension_id, context),
+                    extension->from_bookmark()),
+                std::move(callback)));
         return;
+      }
 
-      case apps::mojom::IconCompression::kCompressed:
+      case apps::mojom::IconCompression::kCompressed: {
+        // TODO(crbug.com/826982): do we also want to apply the
+        // ChromeAppIconApplyEffects image filter here? This will require
+        // decoding from and re-encoding to PNG before and after the filter.
         base::PostTaskWithTraitsAndReplyWithResult(
             FROM_HERE, {base::MayBlock(), base::TaskPriority::USER_VISIBLE},
             base::BindOnce(&ReadExtensionResource, std::move(ext_resource)),
             base::BindOnce(&RunCallbackWithCompressedData,
                            std::move(callback)));
         return;
+      }
     }
   }
 

chrome/browser/chromeos/extensions/gfx_utils.cc

@@ -189,11 +189,9 @@ const std::vector<std::string> GetEquivalentInstalledExtensions(
   return extension_ids;
 }
 
-bool MaybeApplyChromeBadge(content::BrowserContext* context,
-                           const std::string& extension_id,
-                           gfx::ImageSkia* icon_out) {
+bool ShouldApplyChromeBadge(content::BrowserContext* context,
+                            const std::string& extension_id) {
   DCHECK(context);
-  DCHECK(icon_out);
 
   Profile* profile = Profile::FromBrowserContext(context);
   // Only apply Chrome badge for the primary profile.
@@ -209,6 +207,12 @@ bool MaybeApplyChromeBadge(content::BrowserContext* context,
   if (!HasEquivalentInstalledArcApp(context, extension_id))
     return false;
 
+  return true;
+}
+
+void ApplyChromeBadge(gfx::ImageSkia* icon_out) {
+  DCHECK(icon_out);
+
   const gfx::ImageSkia* badge_image =
       ui::ResourceBundle::GetSharedInstance().GetImageSkiaNamed(
           IDR_ARC_DUAL_ICON_BADGE);
@@ -221,7 +225,6 @@ bool MaybeApplyChromeBadge(content::BrowserContext* context,
   }
   *icon_out = gfx::ImageSkiaOperations::CreateSuperimposedImage(
       *icon_out, resized_badge_image);
-  return true;
 }
 
 }  // namespace util

chrome/browser/chromeos/extensions/gfx_utils.h

@@ -39,11 +39,13 @@ const std::vector<std::string> GetEquivalentInstalledExtensions(
     content::BrowserContext* context,
     const std::string& arc_package_name);
 
-// May apply additional badge in order to distinguish dual apps from Chrome and
-// Android side. Returns whether the icon was badged.
-bool MaybeApplyChromeBadge(content::BrowserContext* context,
-                           const std::string& extension_id,
-                           gfx::ImageSkia* icon_out);
+// Returns whether to call ApplyChromeBadge.
+bool ShouldApplyChromeBadge(content::BrowserContext* context,
+                            const std::string& extension_id);
+
+// Applies an additional badge in order to distinguish dual apps from Chrome
+// and Android side.
+void ApplyChromeBadge(gfx::ImageSkia* icon_out);
 
 }  // namespace util
 }  // namespace extensions

chrome/browser/extensions/chrome_app_icon.cc

@@ -61,6 +61,37 @@ class RoundedCornersImageSource : public gfx::CanvasImageSource {
 
 }  // namespace
 
+// static
+void ChromeAppIcon::ApplyEffects(int resource_size_in_dip,
+                                 const ResizeFunction& resize_function,
+                                 bool apply_chrome_badge,
+                                 bool app_launchable,
+                                 bool from_bookmark,
+                                 gfx::ImageSkia* image_skia) {
+  if (!resize_function.is_null()) {
+    resize_function.Run(gfx::Size(resource_size_in_dip, resource_size_in_dip),
+                        image_skia);
+  }
+
+#if defined(OS_CHROMEOS)
+  if (apply_chrome_badge) {
+    util::ApplyChromeBadge(image_skia);
+  }
+#endif
+
+  if (!app_launchable) {
+    constexpr color_utils::HSL shift = {-1, 0, 0.6};
+    *image_skia =
+        gfx::ImageSkiaOperations::CreateHSLShiftedImage(*image_skia, shift);
+  }
+
+  if (from_bookmark) {
+    *image_skia =
+        gfx::ImageSkia(std::make_unique<RoundedCornersImageSource>(*image_skia),
+                       image_skia->size());
+  }
+}
+
 ChromeAppIcon::ChromeAppIcon(ChromeAppIconDelegate* delegate,
                              content::BrowserContext* browser_context,
                              DestroyedCallback destroyed_callback,
@@ -110,27 +141,21 @@ void ChromeAppIcon::UpdateIcon() {
   DCHECK(icon_);
 
   image_skia_ = icon_->image_skia();
-  if (!resize_function_.is_null()) {
-    resize_function_.Run(
-        gfx::Size(resource_size_in_dip_, resource_size_in_dip_), &image_skia_);
-  }
+
+  bool apply_chrome_badge = false;
 #if defined(OS_CHROMEOS)
-  icon_is_badged_ =
-      util::MaybeApplyChromeBadge(browser_context_, app_id_, &image_skia_);
+  icon_is_badged_ = util::ShouldApplyChromeBadge(browser_context_, app_id_);
+  apply_chrome_badge = icon_is_badged_;
 #endif
 
-  if (!util::IsAppLaunchable(app_id_, browser_context_)) {
-    const color_utils::HSL shift = {-1, 0, 0.6};
-    image_skia_ =
-        gfx::ImageSkiaOperations::CreateHSLShiftedImage(image_skia_, shift);
-  }
+  bool app_launchable = util::IsAppLaunchable(app_id_, browser_context_);
 
-  const Extension* extension = GetExtension();
-  if (extension && extension->from_bookmark()) {
-    image_skia_ =
-        gfx::ImageSkia(std::make_unique<RoundedCornersImageSource>(image_skia_),
-                       image_skia_.size());
-  }
+  const Extension* extension =
+      ExtensionRegistry::Get(browser_context_)->GetInstalledExtension(app_id_);
+  bool from_bookmark = extension && extension->from_bookmark();
+
+  ApplyEffects(resource_size_in_dip_, resize_function_, apply_chrome_badge,
+               app_launchable, from_bookmark, &image_skia_);
 
   delegate_->OnIconUpdated(this);
 }

chrome/browser/extensions/chrome_app_icon.h

@@ -33,6 +33,15 @@ class ChromeAppIcon : public IconImage::Observer {
   using ResizeFunction =
       base::RepeatingCallback<void(const gfx::Size&, gfx::ImageSkia*)>;
 
+  // Applies image processing effects to |image_skia|, such as resizing, adding
+  // badges, converting to gray and rounding corners.
+  static void ApplyEffects(int resource_size_in_dip,
+                           const ResizeFunction& resize_function,
+                           bool apply_chrome_badge,
+                           bool app_launchable,
+                           bool from_bookmark,
+                           gfx::ImageSkia* image_skia);
+
   // |resize_function| overrides icon resizing behavior if non-null. Otherwise
   // IconLoader with perform the resizing. In both cases |resource_size_in_dip|
   // is used to pick the correct icon representation from resources.