Support custom icon in search result.

chrome/browser/ui/app_list/search/launcher_search/extension_badged_icon_image.cc

Index: chrome/browser/ui/app_list/search/launcher_search/extension_badged_icon_image.cc
diff --git a/chrome/browser/ui/app_list/search/launcher_search/extension_badged_icon_image.cc b/chrome/browser/ui/app_list/search/launcher_search/extension_badged_icon_image.cc
new file mode 100644
index 0000000000000000000000000000000000000000..c163668b68aebe231118f26126f8d37c7dbfe0d5
--- /dev/null
+++ b/chrome/browser/ui/app_list/search/launcher_search/extension_badged_icon_image.cc
@@ -0,0 +1,138 @@
+// Copyright 2015 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/ui/app_list/search/launcher_search/extension_badged_icon_image.h"
+
+#include "chrome/browser/extensions/extension_util.h"
+#include "extensions/browser/image_loader.h"
+#include "extensions/common/constants.h"
+#include "extensions/common/file_util.h"
+#include "extensions/common/manifest_handlers/icons_handler.h"
+#include "skia/ext/image_operations.h"
+#include "ui/gfx/canvas.h"
+#include "ui/gfx/geometry/size.h"
+#include "ui/gfx/image/canvas_image_source.h"
+#include "ui/gfx/image/image_skia_operations.h"
+
+namespace {
+
+class BadgedIconSource : public gfx::CanvasImageSource {
+ public:
+  BadgedIconSource(const gfx::ImageSkia& custom_icon,
+                   const gfx::ImageSkia& extension_icon,
+                   const gfx::Size& icon_size)
+      : CanvasImageSource(icon_size, false),
+        custom_icon_(custom_icon),
+        extension_icon_(extension_icon) {}
+
+  void Draw(gfx::Canvas* canvas) override {
+    canvas->DrawImageInt(custom_icon_, 0, 0);

[Matt Giuca, 2015/04/22 03:31:37]

Have you tested what happens if the custom icon has the wrong size? It probably
should get scaled to |icon_size|.

[yawano, 2015/04/22 12:23:47]

Fixed the call of extensions::ImageLoader to resize custom icon to |icon_size|.

+
+    // Badged icon size is 2/3 of custom icon.
+    int badge_dimension = size().width() * 2 / 3;
+    gfx::Size badge_size = gfx::Size(badge_dimension, badge_dimension);
+    gfx::ImageSkia resized_extension_icon =

[Matt Giuca, 2015/04/22 03:31:37]

Can you do this in the constructor and store the resized_extension_icon instead
of storing the extension icon?

[yawano, 2015/04/22 12:23:47]

Done.

+        gfx::ImageSkiaOperations::CreateResizedImage(
+            extension_icon_, skia::ImageOperations::ResizeMethod::RESIZE_GOOD,
+            badge_size);
+    canvas->DrawImageInt(resized_extension_icon,
+                         size().width() - badge_size.width(),
+                         size().height() - badge_size.height());
+  }
+
+ private:
+  gfx::ImageSkia custom_icon_;
+  gfx::ImageSkia extension_icon_;
+
+  DISALLOW_COPY_AND_ASSIGN(BadgedIconSource);
+};
+
+}  // namespace
+
+namespace app_list {
+
+ExtensionBadgedIconImage::ExtensionBadgedIconImage(
+    const GURL& icon_url,
+    Profile* profile,
+    const extensions::Extension* extension,
+    const int icon_dimension)
+    : extension_icon_image_(profile,
+                            extension,
+                            extensions::IconsInfo::GetIcons(extension),
+                            icon_dimension,
+                            extensions::util::GetDefaultExtensionIcon(),
+                            this),
+      icon_size_(icon_dimension, icon_dimension),
+      weak_ptr_factory_(this) {
+  // If valid icon_url is provided as chrome-extension scheme with the host of
+  // |extension|, load custom icon.
+  if (icon_url.is_valid() && icon_url.SchemeIs(extensions::kExtensionScheme) &&
+      icon_url.host() == extension->id()) {

[Matt Giuca, 2015/04/22 03:31:37]

I think you should differentiate between |icon_url.is_empty()| (which is
documented as being valid, meaning we just want the extension icon), and
|icon_url| being an invalid URL (e.g., not in the chrome-extension scheme).

In the latter case, some error should be communicated, even if it is just
printed to the JavaScript console. Is it possible to do that? But you should not
print an error simply if the URL is empty.

[yawano, 2015/04/22 12:23:47]

Done.

[Matt Giuca, 2015/04/24 04:26:46]

Thanks for adding error checking!

+    const base::FilePath& file_path =
+        extensions::file_util::ExtensionURLToRelativeFilePath(icon_url);
+    const extensions::ExtensionResource& resource =
+        extension->GetResource(file_path);
+    extensions::ImageLoader::Get(profile)->LoadImageAsync(
+        extension, resource, icon_size_,
+        base::Bind(&ExtensionBadgedIconImage::OnCustomIconLoaded,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+
+  Update();
+}
+
+ExtensionBadgedIconImage::~ExtensionBadgedIconImage() {
+}
+
+void ExtensionBadgedIconImage::AddObserver(Observer* observer) {
+  observers_.insert(observer);
+}
+
+void ExtensionBadgedIconImage::RemoveObserver(Observer* observer) {
+  observers_.erase(observer);
+}
+
+void ExtensionBadgedIconImage::OnExtensionIconImageChanged(
+    extensions::IconImage* image) {
+  Update();
+}
+
+gfx::ImageSkia ExtensionBadgedIconImage::GetIconImage() const {
+  return badged_icon_image_;
+}
+
+void ExtensionBadgedIconImage::OnCustomIconLoaded(const gfx::Image& image) {
+  custom_icon_image_ = image.AsImageSkia();

[Matt Giuca, 2015/04/22 03:31:37]

Have you tested what happens if the file could not be loaded? Just from looking
at the code, it seems that it will result in a null ImageSkia (which I see you
check for later).

But as above, I think there should be some error communicated (possibly to the
JavaScript console) if this happens. Maybe you can check for that here (but I'm
not sure how hard it is to get into the extension's JS console from here).

[yawano, 2015/04/22 12:23:47]

I tested this and confirmed that empty image is returned for a case that it
failed to load an image. I added code to show warning message in developer
console for that case.

Since this is not a fatal error, it shows message as warning. Should it be error
message?

[Matt Giuca, 2015/04/24 04:26:46]

A warning is fine.

+  Update();
+}
+
+void ExtensionBadgedIconImage::Update() {
+  // If extension_icon_image is not available, return immediately.
+  if (extension_icon_image_.image_skia().isNull())
+    return;
+
+  // When custom icon image is not available, simply use extension icon image
+  // without badge.
+  if (custom_icon_image_.isNull()) {
+    SetIconImage(extension_icon_image_.image_skia());
+    return;
+  }
+
+  // Create badged icon image.
+  gfx::ImageSkia badged_icon_image(
+      new BadgedIconSource(custom_icon_image_,
+                           extension_icon_image_.image_skia(), icon_size_),
+      icon_size_);
+  SetIconImage(badged_icon_image);
+}
+
+void ExtensionBadgedIconImage::SetIconImage(const gfx::ImageSkia& icon_image) {
+  badged_icon_image_ = icon_image;
+
+  for (auto* observer : observers_) {
+    observer->OnIconImageChanged(this);
+  }
+}
+
+}  // namespace app_list