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 5a655ff69159738a984a27573159aa634a43c14e..3eedbb2df3443142b88c38a0ffaaf6bb0ce713dc 100644
--- a/chrome/common/extensions/extension_action.h
+++ b/chrome/common/extensions/extension_action.h
@@ -12,8 +12,11 @@
 #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_factory_delegate.h"
+#include "chrome/common/extensions/extension_icon_set.h"
 #include "third_party/skia/include/core/SkColor.h"
 #include "ui/base/animation/linear_animation.h"
 
@@ -133,9 +136,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 {

[Jeffrey Yasskin, 2012/09/13 00:23:36]

I think mpcomplete is removing the icon_paths() API entirely, so it may not be
worth refactoring it. I'm not sure of the timeline.

[tbarzic, 2012/09/13 17:57:54]

fwiw, this change removes page action icons loading from ui code, so I guess
it's a good step in "removing the icon_paths() API" direction too :)

+    return page_action_icons_.get();
+  }
+
+  // Adds an IconSet for a new page action icon.
+  void AddPageActionIcon(scoped_ptr<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; }
@@ -163,23 +172,17 @@ 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 the default icon or pare action icon has to be
+  // returned, |icon_factory| will be used to get the actual icon.
+  // |icon_factory| is used because the icon has to be loaded using
+  // extensions::IconImage, which cannot be done from chrome/common/. The
+  // factory implementation should handle asynchronous icon loading.
+  // If no icon is specified for the tab, returns the puzzle piece icon.
+  gfx::Image GetIcon(int tab_id,
+                     ExtensionIconFactoryDelegate* icon_factory) const;
 
   // Gets the icon that has been set using |SetIcon| for the tab.
   gfx::ImageSkia GetExplicitlySetIcon(int tab_id) const;
@@ -196,11 +199,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(scoped_ptr<ExtensionIconSet> icon_set) {
+     default_icon_ = icon_set.Pass();
   }
-  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.
@@ -268,6 +272,9 @@ class ExtensionAction {
   gfx::ImageSkia ApplyIconAnimation(int tab_id,
                                     const gfx::ImageSkia& orig) const;
 
+  // Returns width of the current icon for tab_id.
+  int GetIconWidth(int tab_id) const;
+
   // Paints badge with specified parameters to |canvas|.
   static void DoPaintBadge(gfx::Canvas* canvas,
                            const gfx::Rect& bounds,
@@ -323,19 +330,17 @@ class ExtensionAction {
   // NULLs to prevent the map from growing without bound.
   mutable std::map<int, base::WeakPtr<IconAnimation> > icon_animation_;
 
-  std::string default_icon_path_;
+  // ExtensionIconSet with icon image representations for default icon.

[Jeffrey Yasskin, 2012/09/13 00:23:36]

ExtensionIconSet doesn't hold icon image representations: it holds paths.

Does it actually make sense to overload ExtensionIconSet between holding paths
for several different size icons at the same scale vs. paths for several
different scale icons at the same size? I'm getting confused following these
changes because I'm not sure which meaning you're using in each place, and I'm
worried that's just going to get worse over time.

[tbarzic, 2012/09/13 02:01:01]

I look at ExtensionIconSet as not aware of scale factors. (i.e. it's just a set
of bitmaps of different sizes, and we select image representation bitmaps from
that set).

+  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_;