Convert extension action icons code to use ImageSkia instead of SkBitmap

chrome/common/extensions/extension_action.h

Index: chrome/common/extensions/extension_action.h
diff --git a/chrome/common/extensions/extension_action.h b/chrome/common/extensions/extension_action.h
index 5fc860fee53e6f6ec19de3d56e8579039c2c7186..3ef3aba762af05b8bd5a3573d6cc3ca6b6d9a7ad 100644
--- a/chrome/common/extensions/extension_action.h
+++ b/chrome/common/extensions/extension_action.h
@@ -24,7 +24,9 @@ class SkDevice;
 namespace gfx {
 class Canvas;
 class Image;
+class ImageSkia;
 class Rect;
+class Size;
 }
 
 // ExtensionAction encapsulates the state of a browser action, page action, or
@@ -165,7 +167,7 @@ class ExtensionAction {
   void CacheIcon(const std::string& path, const gfx::Image& icon);
 
   // Set this action's icon bitmap on a specific tab.
-  void SetIcon(int tab_id, const SkBitmap& bitmap);
+  void SetIcon(int tab_id, const gfx::Image& bitmap);
 
   // Get the icon for a tab, or the default if no icon was set for this tab,
   // retrieving icons that have been specified by path from the previous
@@ -237,6 +239,11 @@ class ExtensionAction {
   // If the specified tab has a badge, paint it into the provided bounds.
   void PaintBadge(gfx::Canvas* canvas, const gfx::Rect& bounds, int tab_id);

[Jeffrey Yasskin, 2012/08/09 21:39:04]

Do the new functions let you make this private or remove it? That would be cool.

Don't worry about this if it would need any changes outside this file.

[tbarzic, 2012/08/10 06:24:08]

we still need this..

 
+  // Returns icon image with badge for specified tab.
+  gfx::ImageSkia GetIconWithBadge(const gfx::ImageSkia& icon,
+                                  int tab_id,
+                                  const gfx::Size& spacing) const;
+
   // Gets a weak reference to the icon animation for a tab, if any. The
   // reference will only have a value while the animation is running.
   base::WeakPtr<IconAnimation> GetIconAnimation(int tab_id) const;
@@ -245,7 +252,10 @@ class ExtensionAction {
   // Runs an animation on a tab.
   void RunIconAnimation(int tab_id);
 
+ private:
   class IconAnimationWrapper;
+  class AnimatedIconImageSource;

[Jeffrey Yasskin, 2012/08/09 21:39:04]

Do these need to be members of ExtensionAction, or can they just be local
classes in extension_action.cc?

[tbarzic, 2012/08/10 06:24:08]

IconWithBadgeImageSource uses private DoPaintBadge. 

I can remove AnimatedIconImageSource, though (it depends on IconAnimatedWrapper,
but that can (should) be changed)

+  class IconWithBadgeImageSource;
 
   // Finds the icon animation wrapper for a tab, if any.  If the animation for
   // this tab has recently completed, also removes up any other dead wrappers
@@ -254,7 +264,16 @@ class ExtensionAction {
 
   // If the icon animation is running on tab |tab_id|, applies it to
   // |orig| and returns the result. Otherwise, just returns |orig|.
-  gfx::Image ApplyIconAnimation(int tab_id, const gfx::Image& orig) const;
+  gfx::ImageSkia ApplyIconAnimation(int tab_id,
+                                    const gfx::ImageSkia& orig) const;
+
+  // Paints badge with specified parameters to |canvas|.
+  static void DoPaintBadge(gfx::Canvas* canvas,
+                           const gfx::Rect& bounds,
+                           const std::string& text,
+                           const SkColor& text_color_in,
+                           const SkColor& background_color_in,
+                           int icon_width);
 
   template <class T>
   struct ValueTraits {
@@ -289,7 +308,7 @@ class ExtensionAction {
   // kDefaultTabId), or tab-specific state (stored with the tab_id as the key).
   std::map<int, GURL> popup_url_;
   std::map<int, std::string> title_;
-  std::map<int, gfx::Image> icon_;
+  std::map<int, gfx::ImageSkia> icon_;

[Jeffrey Yasskin, 2012/08/09 21:39:04]

Why ImageSkia instead of Image? It would be good to have a comment in gfx/image/
saying how to decide between them, although that's separate from this CL.

[tbarzic, 2012/08/10 06:24:08]

we are using ImageSkia internally inside this class, so I figured I would make
that obvious...my understanding is that we use gfx::Image when backing image
type is undetermined.. but I could be terribly wrong here :) 

[Jeffrey Yasskin, 2012/08/10 07:40:35]

I know less about this than you do. :)

   std::map<int, int> icon_index_;  // index into icon_paths_
   std::map<int, std::string> badge_text_;
   std::map<int, SkColor> badge_background_color_;
@@ -315,7 +334,7 @@ class ExtensionAction {
   std::vector<std::string> icon_paths_;
 
   // Saves the arguments from CacheIcon() calls.
-  std::map<std::string, gfx::Image> path_to_icon_cache_;
+  std::map<std::string, gfx::ImageSkia> path_to_icon_cache_;
 
   DISALLOW_COPY_AND_ASSIGN(ExtensionAction);
 };