Move icon fallbacks into ExtensionAction.

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 4de8e1f400d2032c060ff70f9f34c7314b302fc8..82b09ae810a500db54053d001a141cc45efd6332 100644
--- a/chrome/common/extensions/extension_action.h
+++ b/chrome/common/extensions/extension_action.h
@@ -23,6 +23,7 @@ class SkDevice;
 
 namespace gfx {
 class Canvas;
+class Image;
 class Rect;
 }
 
@@ -119,6 +120,7 @@ class ExtensionAction {
 
   // static icon paths from manifest -- only used with legacy page actions API.
   std::vector<std::string>* icon_paths() { return &icon_paths_; }
+  const std::vector<std::string>* icon_paths() const { return &icon_paths_; }
 
   // Set the url which the popup will load when the user clicks this action's
   // icon.  Setting an empty URL will disable the popup for a given tab.
@@ -149,8 +151,15 @@ class ExtensionAction {
   // Set this action's icon bitmap on a specific tab.
   void SetIcon(int tab_id, const SkBitmap& bitmap);
 
-  // Get the icon for a tab, or the default if no icon was set.
-  SkBitmap GetIcon(int tab_id) const;
+  // Maps paths to icons since ExtensionAction, living in common/, can't
+  // interact with the browser to load them itself.  Pre-populate this cache
+  // with images from icon_paths() and default_icon_path().
+  typedef std::map<std::string, gfx::Image> PathToIconCache;
+
+  // 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 'cache'.  If the
+  // default icon isn't found in 'cache', returns the puzzle piece icon.
+  gfx::Image GetIcon(int tab_id, const PathToIconCache& cache) const;

[not at google - send to devlin, 2012/07/26 02:29:44]

A method like this makes me wonder if PathToIconCache should be a class like
ExtensionAction::IconCache, with an ExtensionAction curried in.

I also found it pretty confusing seeing PathToIconCache in other code when it's
just an alias for a map; I was expecting it to be a class.

---

Really I think all this icon loading logic should be pulled somewhere inside
chrome/extensions/browser and somehow combined with, or replacing,
ImageLoadingTracker. I say that because there's still a lot of shared
boilerplate for image loading in the UI code that could be cleaner.

However since I'm not prepared to do that, I don't expect anybody else to be.

[Jeffrey Yasskin, 2012/07/26 21:11:47]

Would you like me to duplicate the map<> arguments in each place I use the type?
I tend to think that kind of duplication makes code easier to read, but so many
people seem to like the typedefs that I got lazy and used one here.
I definitely agree that there should be some sort of structure for loading
images/resources from common code. I don't feel like I have enough context yet
to design it (presumably images aren't the only thing we want to load?), and I'd
rather not put a big refactoring before the grey'ed out request-to-act badges.
This was the simplest change that lets me do the icon-greying without changes to
platform-specific code.

 
   // Set this action's icon index for a specific tab.  For use with
   // icon_paths(), only used in page actions.
@@ -167,7 +176,7 @@ class ExtensionAction {
   void set_default_icon_path(const std::string& path) {
     default_icon_path_ = path;
   }
-  std::string default_icon_path() const {
+  const std::string& default_icon_path() const {
     return default_icon_path_;
   }
 
@@ -224,6 +233,10 @@ class ExtensionAction {
   void RunIconAnimation(int tab_id);
 
  private:
+  // 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;
+
   template <class T>
   struct ValueTraits {
     static T CreateEmpty() {
@@ -257,7 +270,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, SkBitmap> icon_;
+  std::map<int, gfx::Image> icon_;
   std::map<int, int> icon_index_;  // index into icon_paths_
   std::map<int, std::string> badge_text_;
   std::map<int, SkColor> badge_background_color_;