Change browser/page action default icon defined in manifest to support hidpi.

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 d111678b525a41e04d0c2eed4cd7a02cae1a11fa..0754bbc0c3af4240ffc5ffc6166973b82389262c 100644
--- a/chrome/common/extensions/extension_action.h
+++ b/chrome/common/extensions/extension_action.h
@@ -12,8 +12,10 @@
 #include "base/basictypes.h"
 #include "base/memory/linked_ptr.h"
 #include "base/memory/scoped_ptr.h"
+#include "base/memory/scoped_vector.h"
 #include "base/memory/weak_ptr.h"
 #include "base/observer_list.h"
+#include "chrome/common/extensions/extension_icon_set.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/base/animation/linear_animation.h"
 
@@ -56,6 +58,19 @@ class ExtensionAction {
     ACTIVE,
   };
 
+  // Since ExtensionAction, living in common/, can't interact with the browser

[Jeffrey Yasskin, 2012/09/01 03:54:33]

I skeched
https://chromiumcodereview.appspot.com/10833069/diff/1/chrome/common/extensio...
to do this sort of thing. It doesn't really seem like something that should be
embedded into ExtensionAction, although making it specific to images could make
sense.

I believe aa and kalman were concerned about how it would be implemented in
non-browser processes that can't necessarily load images.

Are these layer-reversing interfaces normally named with a "Delegate" suffix?

s/the UI code/the browser code/, right? This won't get implemented separately
for win/mac/linux, right?

[tbarzic, 2012/09/05 01:05:29]

OK, I've moved this interface out of ExtensionAction.
 
Yes, they are.. 
I had put "UI code" since the object is owned (and passed to |GetIcon|) by UI
code (even though the implementation is common and in chrome/browser).

+  // to load images, the UI code will have to load the images for each path for
+  // us by implementing this interface and pass it to |GetIcon| method.
+  // Whenever icon to return is defined by its path (stored in ExtensionIconSet)

[Jeffrey Yasskin, 2012/09/01 03:54:33]

I don't understand this sentence, or how an ExtensionIconSet and action_type map
to a returned icon.

+  // We will call Icon::Factory::GetIcon to get actual icon.
+  class IconFactory {
+   public:
+    virtual ~IconFactory() {}
+
+    virtual gfx::ImageSkia GetIcon(const ExtensionIconSet* paths,
+                                   Type action_type) = 0;
+  };
+
   // A fade-in animation.
   class IconAnimation : public ui::LinearAnimation,
                         public base::SupportsWeakPtr<IconAnimation> {
@@ -130,9 +145,15 @@ class ExtensionAction {
   std::string id() const { return id_; }
   void set_id(const std::string& id) { id_ = id; }
 
-  // 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_; }
+  const std::vector<const ExtensionIconSet*>& page_action_icons() const {
+    return page_action_icons_.get();
+  }
+
+  // Adds an IconSet for a new page action icon.
+  void AddPageActionIcon(const ExtensionIconSet* icon_set);
+
+  // Checks if specified icon index is valid.
+  bool IsValidIconIndex(int index) const;
 
   bool has_changed() const { return has_changed_; }
   void set_has_changed(bool value) { has_changed_ = value; }
@@ -160,23 +181,13 @@ class ExtensionAction {
   // bitmap or a path. However, conceptually, there is only one default icon.
   // Setting the default icon using a path clears the bitmap and vice-versa.
 
-  // Since ExtensionAction, living in common/, can't interact with the browser
-  // to load images, the UI code needs to load the images for each path.  For
-  // each path in default_icon_path() and icon_paths(), load the image there
-  // using an ImageLoadingTracker and call CacheIcon(path, image) with the
-  // result.
-  //
-  // If an image is cached redundantly, the first load will be used.
-  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 gfx::Image& image);
 
-  // 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
-  // arguments to CacheIcon().  If the default icon isn't found in the cache,
-  // returns the puzzle piece icon.
-  gfx::Image GetIcon(int tab_id) const;
+  // Get the icon for a tab. If icon to return is defined by its path,
+  // |icon_factory| will be used to get the actual icon.
+  // If no icon is specified for the tab, returns the puzzle piece icon.
+  gfx::Image GetIcon(int tab_id, IconFactory* icon_factory) const;
 
   // Set this action's icon index for a specific tab.  For use with
   // icon_paths(), only used in page actions.
@@ -190,11 +201,12 @@ class ExtensionAction {
 
   // Non-tab-specific icon path. This is used to support the default_icon key of
   // page and browser actions.
-  void set_default_icon_path(const std::string& path) {
-    default_icon_path_ = path;
+  void set_default_icon(const ExtensionIconSet* icon_set) {

[Jeffrey Yasskin, 2012/09/01 03:54:33]

When you take ownership of a passed pointer, take it as a scoped_ptr<> to
communicate that.

[tbarzic, 2012/09/05 01:05:29]

Done.

+     default_icon_.reset(icon_set);
   }
-  const std::string& default_icon_path() const {
-    return default_icon_path_;
+
+  const ExtensionIconSet* default_icon() const {
+    return default_icon_.get();
   }
 
   // Set this action's badge text on a specific tab.
@@ -324,19 +336,17 @@ class ExtensionAction {
   // without bound.
   mutable std::map<int, base::WeakPtr<IconAnimationWrapper> > icon_animation_;
 
-  std::string default_icon_path_;
+  // ExtensionIconSet with icon image representations for default icon.
+  scoped_ptr<const ExtensionIconSet> default_icon_;
+
+  // ExtensionIconSets with icon image representations for deprecated page
+  // action icons.
+  ScopedVector<const ExtensionIconSet> page_action_icons_;
 
   // The id for the ExtensionAction, for example: "RssPageAction". This is
   // needed for compat with an older version of the page actions API.
   std::string id_;
 
-  // A list of paths to icons this action might show. This is needed to support
-  // the legacy setIcon({iconIndex:...} method of the page actions API.
-  std::vector<std::string> icon_paths_;
-
-  // Saves the arguments from CacheIcon() calls.
-  std::map<std::string, gfx::ImageSkia> path_to_icon_cache_;
-
   // True if the ExtensionAction's settings have changed from what was
   // specified in the manifest.
   bool has_changed_;