Show extension icons next to their top-level context menu items.

Also fix a bug in extension icon caching where we weren't keeping track of
potential resizing done by ImageLoadingTracker before setting the cached
SkBitmap.

BUG=39494
TEST=Install an extension that includes an icon and uses the experimental
context menu API. You should see the extension's icon in the context menu
next to its top-level item.

Review URL: http://codereview.chromium.org/2867008

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@50779 0039d316-1c4b-4281-b951-d872f2087c98

19 files changed, 447 insertions, 148 deletions

diff --git a/chrome/browser/extensions/extension_context_menu_api.cc b/chrome/browser/extensions/extension_context_menu_api.cc
index 554cc37..1c63dd1 100644
--- a/chrome/browser/extensions/extension_context_menu_api.cc
+++ b/chrome/browser/extensions/extension_context_menu_api.cc
@@ -188,7 +188,7 @@ bool CreateContextMenuFunction::RunImpl() {
     }
     id = menu_manager->AddChildItem(parent_id, item.release());
   } else {
-    id = menu_manager->AddContextItem(item.release());
+    id = menu_manager->AddContextItem(GetExtension(), item.release());
   }
 
   if (id <= 0)
diff --git a/chrome/browser/extensions/extension_menu_manager.cc b/chrome/browser/extensions/extension_menu_manager.cc
index 9563a0a..42e3842 100644
--- a/chrome/browser/extensions/extension_menu_manager.cc
+++ b/chrome/browser/extensions/extension_menu_manager.cc
@@ -6,6 +6,7 @@
 
 #include <algorithm>
 
+#include "app/resource_bundle.h"
 #include "base/logging.h"
 #include "base/string_util.h"
 #include "base/utf_string_conversions.h"
@@ -16,6 +17,11 @@
 #include "chrome/browser/extensions/extension_tabs_module.h"
 #include "chrome/browser/profile.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/extensions/extension_resource.h"
+#include "gfx/favicon_size.h"
+#include "gfx/size.h"
+#include "grit/theme_resources.h"
+#include "skia/ext/image_operations.h"
 #include "webkit/glue/context_menu.h"
 
 ExtensionMenuItem::ExtensionMenuItem(const std::string& extension_id,
@@ -96,7 +102,9 @@ void ExtensionMenuItem::AddChild(ExtensionMenuItem* item) {
   children_.push_back(item);
 }
 
-ExtensionMenuManager::ExtensionMenuManager() : next_item_id_(1) {
+ExtensionMenuManager::ExtensionMenuManager()
+    : next_item_id_(1),
+      ALLOW_THIS_IN_INITIALIZER_LIST(image_tracker_(this)) {
   registrar_.Add(this, NotificationType::EXTENSION_UNLOADED,
                  NotificationService::AllSources());
 }
@@ -126,21 +134,38 @@ const ExtensionMenuItem::List* ExtensionMenuManager::MenuItems(
   return NULL;
 }
 
-int ExtensionMenuManager::AddContextItem(ExtensionMenuItem* item) {
+int ExtensionMenuManager::AddContextItem(Extension* extension,
+                                         ExtensionMenuItem* item) {
   const std::string& extension_id = item->extension_id();
   // The item must have a non-empty extension id.
   if (extension_id.empty())
     return 0;
 
+  DCHECK_EQ(extension->id(), extension_id);
+
   DCHECK_EQ(0, item->id());
   item->set_id(next_item_id_++);
 
+  bool first_item = !ContainsKey(context_items_, extension_id);
   context_items_[extension_id].push_back(item);
   items_by_id_[item->id()] = item;
 
   if (item->type() == ExtensionMenuItem::RADIO && item->checked())
     RadioItemSelected(item);
 
+  // If this is the first item for this extension, start loading its icon.
+  if (first_item) {
+    ExtensionResource icon_resource;
+    extension->GetIconPathAllowLargerSize(&icon_resource,
+                                          Extension::EXTENSION_ICON_BITTY);
+    if (!icon_resource.extension_root().empty()) {
+      image_tracker_.LoadImage(extension,
+                               icon_resource,
+                               gfx::Size(kFavIconSize, kFavIconSize),
+                               ImageLoadingTracker::CACHE);
+    }
+  }
+
   return item->id();
 }
 
@@ -233,6 +258,7 @@ bool ExtensionMenuManager::RemoveContextMenuItem(int id) {
     return false;
   }
 
+  bool result = false;
   ExtensionMenuItem::List& list = i->second;
   ExtensionMenuItem::List::iterator j;
   for (j = list.begin(); j < list.end(); ++j) {
@@ -241,14 +267,20 @@ bool ExtensionMenuManager::RemoveContextMenuItem(int id) {
       delete *j;
       list.erase(j);
       items_by_id_.erase(id);
-      return true;
+      result = true;
+      break;
     } else if ((*j)->RemoveChild(id)) {
       items_by_id_.erase(id);
-      return true;
+      result = true;
+      break;
     }
   }
-  NOTREACHED();  // The check at the very top should prevent getting here.
-  return false;
+  DCHECK(result);  // The check at the very top should have prevented this.
+
+  if (list.empty() && ContainsKey(extension_icons_, extension_id))
+    extension_icons_.erase(extension_id);
+
+  return result;
 }
 
 void ExtensionMenuManager::RemoveAllContextItems(std::string extension_id) {
@@ -267,6 +299,9 @@ void ExtensionMenuManager::RemoveAllContextItems(std::string extension_id) {
   }
   STLDeleteElements(&context_items_[extension_id]);
   context_items_.erase(extension_id);
+
+  if (ContainsKey(extension_icons_, extension_id))
+    extension_icons_.erase(extension_id);
 }
 
 ExtensionMenuItem* ExtensionMenuManager::GetItemById(int id) const {
@@ -422,3 +457,57 @@ void ExtensionMenuManager::Observe(NotificationType type,
     RemoveAllContextItems(extension->id());
   }
 }
+
+const SkBitmap& ExtensionMenuManager::GetIconForExtension(
+    const std::string& extension_id) {
+  const SkBitmap* result = NULL;
+  if (ContainsKey(extension_icons_, extension_id)) {
+    result = &(extension_icons_[extension_id]);
+  } else {
+    EnsureDefaultIcon();
+    result = &default_icon_;
+  }
+  DCHECK(result);
+  DCHECK(result->width() == kFavIconSize);
+  DCHECK(result->height() == kFavIconSize);
+  return *result;
+}
+
+void ExtensionMenuManager::OnImageLoaded(SkBitmap* image,
+                                         ExtensionResource resource,
+                                         int index) {
+  if (!image)
+    return;
+
+  const std::string extension_id = resource.extension_id();
+
+  // Make sure we still have menu items for this extension (since image loading
+  // is asynchronous, there's a slight chance they may have all been removed
+  // while the icon was loading).
+  if (!ContainsKey(context_items_, extension_id))
+    return;
+
+  if (image->width() == kFavIconSize && image->height() == kFavIconSize)
+    extension_icons_[extension_id] = *image;
+  else
+    extension_icons_[extension_id] = ScaleToFavIconSize(*image);
+}
+
+void ExtensionMenuManager::EnsureDefaultIcon() {
+  if (default_icon_.empty()) {
+    ResourceBundle& rb = ResourceBundle::GetSharedInstance();
+    SkBitmap* src = rb.GetBitmapNamed(IDR_EXTENSIONS_SECTION);
+    if (src->width() == kFavIconSize && src->height() == kFavIconSize) {
+      default_icon_ = *src;
+    } else {
+      default_icon_ = SkBitmap(ScaleToFavIconSize(*src));
+    }
+  }
+}
+
+SkBitmap ExtensionMenuManager::ScaleToFavIconSize(const SkBitmap& source) {
+  return skia::ImageOperations::Resize(source,
+                                       skia::ImageOperations::RESIZE_LANCZOS3,
+                                       kFavIconSize,
+                                       kFavIconSize);
+}
diff --git a/chrome/browser/extensions/extension_menu_manager.h b/chrome/browser/extensions/extension_menu_manager.h
index e85f5d5..c3f82c5 100644
--- a/chrome/browser/extensions/extension_menu_manager.h
+++ b/chrome/browser/extensions/extension_menu_manager.h
@@ -14,11 +14,14 @@
 #include "base/linked_ptr.h"
 #include "base/stl_util-inl.h"
 #include "base/string16.h"
+#include "chrome/browser/extensions/image_loading_tracker.h"
 #include "chrome/common/notification_observer.h"
 #include "chrome/common/notification_registrar.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 
 struct ContextMenuParams;
 
+class Extension;
 class ExtensionMessageService;
 class Profile;
 class TabContents;
@@ -167,7 +170,9 @@ class ExtensionMenuItem {
 };
 
 // This class keeps track of menu items added by extensions.
-class ExtensionMenuManager : public NotificationObserver {
+class ExtensionMenuManager
+    : public ImageLoadingTracker::Observer,
+      public NotificationObserver {
  public:
   ExtensionMenuManager();
   virtual ~ExtensionMenuManager();
@@ -182,9 +187,11 @@ class ExtensionMenuManager : public NotificationObserver {
   // put them into a submenu if there are more than 1.
   const ExtensionMenuItem::List* MenuItems(const std::string& extension_id);
 
-  // Takes ownership of |item|. Returns the id assigned to the item. Has the
-  // side-effect of incrementing the next_item_id_ member.
-  int AddContextItem(ExtensionMenuItem* item);
+  // Adds a top-level menu item for an extension, requiring the |extension|
+  // pointer so it can load the icon for the extension. Takes ownership of
+  // |item|. Returns the id assigned to the item. Has the side-effect of
+  // incrementing the next_item_id_ member.
+  int AddContextItem(Extension* extension, ExtensionMenuItem* item);
 
   // Add an item as a child of another item which has been previously added, and
   // takes ownership of |item|. Returns the id assigned to the item, or 0 on
@@ -217,6 +224,15 @@ class ExtensionMenuManager : public NotificationObserver {
   virtual void Observe(NotificationType type, const NotificationSource& source,
                        const NotificationDetails& details);
 
+  // This returns a bitmap of width/height kFavIconSize, loaded either from an
+  // entry specified in the extension's 'icon' section of the manifest, or a
+  // default extension icon.
+  const SkBitmap& GetIconForExtension(const std::string& extension_id);
+
+  // Implements the ImageLoadingTracker::Observer interface.
+  virtual void OnImageLoaded(SkBitmap* image, ExtensionResource resource,
+                             int index);
+
  private:
   // This is a helper function which takes care of de-selecting any other radio
   // items in the same group (i.e. that are adjacent in the list).
@@ -225,6 +241,13 @@ class ExtensionMenuManager : public NotificationObserver {
   // Returns true if item is a descendant of an item with id |ancestor_id|.
   bool DescendantOf(ExtensionMenuItem* item, int ancestor_id);
 
+  // Makes sure we've done one-time initialization of the default extension icon
+  // default_icon_.
+  void EnsureDefaultIcon();
+
+  // Helper function to return a copy of |src| scaled to kFavIconSize.
+  SkBitmap ScaleToFavIconSize(const SkBitmap& src);
+
   // We keep items organized by mapping an extension id to a list of items.
   typedef std::map<std::string, ExtensionMenuItem::List> MenuItemMap;
   MenuItemMap context_items_;
@@ -239,6 +262,15 @@ class ExtensionMenuManager : public NotificationObserver {
 
   NotificationRegistrar registrar_;
 
+  // Used for loading extension icons.
+  ImageLoadingTracker image_tracker_;
+
+  // Maps extension id to an SkBitmap with the icon for that extension.
+  std::map<std::string, SkBitmap> extension_icons_;
+
+  // The default icon we'll use if an extension doesn't have one.
+  SkBitmap default_icon_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionMenuManager);
 };
 
diff --git a/chrome/browser/extensions/extension_menu_manager_unittest.cc b/chrome/browser/extensions/extension_menu_manager_unittest.cc
index 2abeef1..04baf5ff 100644
--- a/chrome/browser/extensions/extension_menu_manager_unittest.cc
+++ b/chrome/browser/extensions/extension_menu_manager_unittest.cc
@@ -10,6 +10,7 @@
 #include "base/values.h"
 #include "chrome/browser/extensions/extension_menu_manager.h"
 #include "chrome/browser/extensions/extension_message_service.h"
+#include "chrome/browser/extensions/test_extension_prefs.h"
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
@@ -30,25 +31,24 @@ class ExtensionMenuManagerTest : public testing::Test {
   ExtensionMenuManagerTest() {}
   ~ExtensionMenuManagerTest() {}
 
-  // Returns a test item with some default values you can override if you want
-  // to by passing in |properties| (currently just extension_id). Caller owns
-  // the return value and is responsible for freeing it.
-  static ExtensionMenuItem* CreateTestItem(DictionaryValue* properties) {
-    std::string extension_id = "0123456789";  // A default dummy value.
-    if (properties && properties->HasKey(L"extension_id"))
-      EXPECT_TRUE(properties->GetString(L"extension_id", &extension_id));
-
+  // Returns a test item.
+  static ExtensionMenuItem* CreateTestItem(Extension* extension) {
     ExtensionMenuItem::Type type = ExtensionMenuItem::NORMAL;
     ExtensionMenuItem::ContextList contexts(ExtensionMenuItem::ALL);
     ExtensionMenuItem::ContextList enabled_contexts = contexts;
-    std::string title = "test";
-
-    return new ExtensionMenuItem(extension_id, title, false, type, contexts,
+    return new ExtensionMenuItem(extension->id(), "test", false, type, contexts,
                                  enabled_contexts);
   }
 
+  // Creates and returns a test Extension. The caller does *not* own the return
+  // value.
+  Extension* AddExtension(std::string name) {
+    return prefs_.AddExtension(name);
+  }
+
  protected:
   ExtensionMenuManager manager_;
+  TestExtensionPrefs prefs_;
 
  private:
   DISALLOW_COPY_AND_ASSIGN(ExtensionMenuManagerTest);
@@ -56,10 +56,12 @@ class ExtensionMenuManagerTest : public testing::Test {
 
 // Tests adding, getting, and removing items.
 TEST_F(ExtensionMenuManagerTest, AddGetRemoveItems) {
+  Extension* extension = AddExtension("test");
+
   // Add a new item, make sure you can get it back.
-  ExtensionMenuItem* item1 = CreateTestItem(NULL);
+  ExtensionMenuItem* item1 = CreateTestItem(extension);
   ASSERT_TRUE(item1 != NULL);
-  int id1 = manager_.AddContextItem(item1);  // Ownership transferred.
+  int id1 = manager_.AddContextItem(extension, item1);
   ASSERT_GT(id1, 0);
   ASSERT_EQ(item1, manager_.GetItemById(id1));
   const ExtensionMenuItem::List* items =
@@ -68,8 +70,8 @@ TEST_F(ExtensionMenuManagerTest, AddGetRemoveItems) {
   ASSERT_EQ(item1, items->at(0));
 
   // Add a second item, make sure it comes back too.
-  ExtensionMenuItem* item2 = CreateTestItem(NULL);
-  int id2 = manager_.AddContextItem(item2);  // Ownership transferred.
+  ExtensionMenuItem* item2 = CreateTestItem(extension);
+  int id2 = manager_.AddContextItem(extension, item2);
   ASSERT_GT(id2, 0);
   ASSERT_NE(id1, id2);
   ASSERT_EQ(item2, manager_.GetItemById(id2));
@@ -79,9 +81,9 @@ TEST_F(ExtensionMenuManagerTest, AddGetRemoveItems) {
   ASSERT_EQ(item2, items->at(1));
 
   // Try adding item 3, then removing it.
-  ExtensionMenuItem* item3 = CreateTestItem(NULL);
+  ExtensionMenuItem* item3 = CreateTestItem(extension);
   std::string extension_id = item3->extension_id();
-  int id3 = manager_.AddContextItem(item3);  // Ownership transferred.
+  int id3 = manager_.AddContextItem(extension, item3);
   ASSERT_GT(id3, 0);
   ASSERT_EQ(item3, manager_.GetItemById(id3));
   ASSERT_EQ(3u, manager_.MenuItems(extension_id)->size());
@@ -95,23 +97,22 @@ TEST_F(ExtensionMenuManagerTest, AddGetRemoveItems) {
 
 // Test adding/removing child items.
 TEST_F(ExtensionMenuManagerTest, ChildFunctions) {
-  DictionaryValue properties;
-  properties.SetString(L"extension_id", "1111");
-  ExtensionMenuItem* item1 = CreateTestItem(&properties);
+  Extension* extension1 = AddExtension("1111");
+  Extension* extension2 = AddExtension("2222");
+  Extension* extension3 = AddExtension("3333");
 
-  properties.SetString(L"extension_id", "2222");
-  ExtensionMenuItem* item2 = CreateTestItem(&properties);
-  ExtensionMenuItem* item2_child = CreateTestItem(&properties);
-  ExtensionMenuItem* item2_grandchild = CreateTestItem(&properties);
+  ExtensionMenuItem* item1 = CreateTestItem(extension1);
+  ExtensionMenuItem* item2 = CreateTestItem(extension2);
+  ExtensionMenuItem* item2_child = CreateTestItem(extension2);
+  ExtensionMenuItem* item2_grandchild = CreateTestItem(extension2);
 
   // This third item we expect to fail inserting, so we use a scoped_ptr to make
   // sure it gets deleted.
-  properties.SetString(L"extension_id", "3333");
-  scoped_ptr<ExtensionMenuItem> item3(CreateTestItem(&properties));
+  scoped_ptr<ExtensionMenuItem> item3(CreateTestItem(extension3));
 
   // Add in the first two items.
-  int id1 = manager_.AddContextItem(item1);  // Ownership transferred.
-  int id2 = manager_.AddContextItem(item2);  // Ownership transferred.
+  int id1 = manager_.AddContextItem(extension1, item1);
+  int id2 = manager_.AddContextItem(extension2, item2);
 
   ASSERT_NE(id1, id2);
 
@@ -150,13 +151,15 @@ TEST_F(ExtensionMenuManagerTest, ChildFunctions) {
 
 // Tests changing parents.
 TEST_F(ExtensionMenuManagerTest, ChangeParent) {
+  Extension* extension1 = AddExtension("1111");
+
   // First create two items and add them both to the manager.
-  ExtensionMenuItem* item1 = CreateTestItem(NULL);
-  ExtensionMenuItem* item2 = CreateTestItem(NULL);
+  ExtensionMenuItem* item1 = CreateTestItem(extension1);
+  ExtensionMenuItem* item2 = CreateTestItem(extension1);
 
-  int id1 = manager_.AddContextItem(item1);
+  int id1 = manager_.AddContextItem(extension1, item1);
   ASSERT_GT(id1, 0);
-  int id2 = manager_.AddContextItem(item2);
+  int id2 = manager_.AddContextItem(extension1, item2);
   ASSERT_GT(id2, 0);
 
   const ExtensionMenuItem::List* items =
@@ -167,7 +170,7 @@ TEST_F(ExtensionMenuManagerTest, ChangeParent) {
 
   // Now create a third item, initially add it as a child of item1, then move
   // it to be a child of item2.
-  ExtensionMenuItem* item3 = CreateTestItem(NULL);
+  ExtensionMenuItem* item3 = CreateTestItem(extension1);
 
   int id3 = manager_.AddChildItem(id1, item3);
   ASSERT_GT(id3, 0);
@@ -218,10 +221,9 @@ TEST_F(ExtensionMenuManagerTest, ChangeParent) {
   ASSERT_EQ(item3, item1->children()[0]);
 
   // Make sure you can't move a node to be a child of another extension's item.
-  DictionaryValue properties;
-  properties.SetString(L"extension_id", "4444");
-  ExtensionMenuItem* item4 = CreateTestItem(&properties);
-  int id4 = manager_.AddContextItem(item4);
+  Extension* extension2 = AddExtension("2222");
+  ExtensionMenuItem* item4 = CreateTestItem(extension2);
+  int id4 = manager_.AddContextItem(extension2, item4);
   ASSERT_GT(id4, 0);
   ASSERT_FALSE(manager_.ChangeParent(id4, id1));
   ASSERT_FALSE(manager_.ChangeParent(id1, id4));
@@ -233,46 +235,33 @@ TEST_F(ExtensionMenuManagerTest, ChangeParent) {
 // Tests that we properly remove an extension's menu item when that extension is
 // unloaded.
 TEST_F(ExtensionMenuManagerTest, ExtensionUnloadRemovesMenuItems) {
-  ScopedTempDir temp_dir;
-  ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
-
   NotificationService* notifier = NotificationService::current();
   ASSERT_TRUE(notifier != NULL);
 
   // Create a test extension.
-  DictionaryValue extension_properties;
-  extension_properties.SetString(extension_manifest_keys::kVersion, "1");
-  extension_properties.SetString(extension_manifest_keys::kName, "Test");
-  Extension extension(temp_dir.path().AppendASCII("extension"));
-  std::string errors;
-  ASSERT_TRUE(extension.InitFromValue(extension_properties,
-                                      false,  // No public key required.
-                                      &errors)) << errors;
+  Extension* extension1 = AddExtension("1111");
 
   // Create an ExtensionMenuItem and put it into the manager.
-  DictionaryValue item_properties;
-  item_properties.SetString(L"extension_id", extension.id());
-  ExtensionMenuItem* item1 = CreateTestItem(&item_properties);
-  ASSERT_EQ(extension.id(), item1->extension_id());
-  int id1 = manager_.AddContextItem(item1);  // Ownership transferred.
+  ExtensionMenuItem* item1 = CreateTestItem(extension1);
+  ASSERT_EQ(extension1->id(), item1->extension_id());
+  int id1 = manager_.AddContextItem(extension1, item1);
   ASSERT_GT(id1, 0);
-  ASSERT_EQ(1u, manager_.MenuItems(extension.id())->size());
+  ASSERT_EQ(1u, manager_.MenuItems(extension1->id())->size());
 
   // Create a menu item with a different extension id and add it to the manager.
-  std::string alternate_extension_id = "0000";
-  item_properties.SetString(L"extension_id", alternate_extension_id);
-  ExtensionMenuItem* item2 = CreateTestItem(&item_properties);
+  Extension* extension2 = AddExtension("2222");
+  ExtensionMenuItem* item2 = CreateTestItem(extension2);
   ASSERT_NE(item1->extension_id(), item2->extension_id());
-  int id2 = manager_.AddContextItem(item2);  // Ownership transferred.
+  int id2 = manager_.AddContextItem(extension2, item2);
   ASSERT_GT(id2, 0);
 
   // Notify that the extension was unloaded, and make sure the right item is
   // gone.
   notifier->Notify(NotificationType::EXTENSION_UNLOADED,
                    Source<Profile>(NULL),
-                   Details<Extension>(&extension));
-  ASSERT_EQ(NULL, manager_.MenuItems(extension.id()));
-  ASSERT_EQ(1u, manager_.MenuItems(alternate_extension_id)->size());
+                   Details<Extension>(extension1));
+  ASSERT_EQ(NULL, manager_.MenuItems(extension1->id()));
+  ASSERT_EQ(1u, manager_.MenuItems(extension2->id())->size());
   ASSERT_TRUE(manager_.GetItemById(id1) == NULL);
   ASSERT_TRUE(manager_.GetItemById(id2) != NULL);
 }
@@ -308,35 +297,34 @@ TEST_F(ExtensionMenuManagerTest, RemoveAll) {
   // Try removing all items for an extension id that doesn't have any items.
   manager_.RemoveAllContextItems("CCCC");
 
-  // Add 2 top-level and one child item for extension id AAAA.
-  DictionaryValue properties;
-  properties.SetString(L"extension_id", "AAAA");
-  ExtensionMenuItem* item1 = CreateTestItem(&properties);
-  ExtensionMenuItem* item2 = CreateTestItem(&properties);
-  ExtensionMenuItem* item3 = CreateTestItem(&properties);
-  int id1 = manager_.AddContextItem(item1);
-  int id2 = manager_.AddContextItem(item2);
+  // Add 2 top-level and one child item for extension 1.
+  Extension* extension1 = AddExtension("1111");
+  ExtensionMenuItem* item1 = CreateTestItem(extension1);
+  ExtensionMenuItem* item2 = CreateTestItem(extension1);
+  ExtensionMenuItem* item3 = CreateTestItem(extension1);
+  int id1 = manager_.AddContextItem(extension1, item1);
+  int id2 = manager_.AddContextItem(extension1, item2);
   EXPECT_GT(id1, 0);
   EXPECT_GT(id2, 0);
   int id3 = manager_.AddChildItem(id1, item3);
   EXPECT_GT(id3, 0);
 
-  // Add one top-level item for extension id BBBB.
-  properties.SetString(L"extension_id", "BBBB");
-  ExtensionMenuItem* item4 = CreateTestItem(&properties);
-  manager_.AddContextItem(item4);
+  // Add one top-level item for extension 2.
+  Extension* extension2 = AddExtension("2222");
+  ExtensionMenuItem* item4 = CreateTestItem(extension2);
+  manager_.AddContextItem(extension2, item4);
 
-  EXPECT_EQ(2u, manager_.MenuItems("AAAA")->size());
-  EXPECT_EQ(1u, manager_.MenuItems("BBBB")->size());
+  EXPECT_EQ(2u, manager_.MenuItems(extension1->id())->size());
+  EXPECT_EQ(1u, manager_.MenuItems(extension2->id())->size());
 
-  // Remove the BBBB item.
-  manager_.RemoveAllContextItems("BBBB");
-  EXPECT_EQ(2u, manager_.MenuItems("AAAA")->size());
-  EXPECT_EQ(NULL, manager_.MenuItems("BBBB"));
+  // Remove extension2's item.
+  manager_.RemoveAllContextItems(extension2->id());
+  EXPECT_EQ(2u, manager_.MenuItems(extension1->id())->size());
+  EXPECT_EQ(NULL, manager_.MenuItems(extension2->id()));
 
-  // Remove the AAAA items.
-  manager_.RemoveAllContextItems("AAAA");
-  EXPECT_EQ(NULL, manager_.MenuItems("AAAA"));
+  // Remove extension1's items.
+  manager_.RemoveAllContextItems(extension1->id());
+  EXPECT_EQ(NULL, manager_.MenuItems(extension1->id()));
 }
 
 TEST_F(ExtensionMenuManagerTest, ExecuteCommand) {
@@ -355,8 +343,9 @@ TEST_F(ExtensionMenuManagerTest, ExecuteCommand) {
   params.selection_text = L"Hello World";
   params.is_editable = false;
 
-  ExtensionMenuItem* item = CreateTestItem(NULL);
-  int id = manager_.AddContextItem(item);  // Ownership transferred.
+  Extension* extension = AddExtension("test");
+  ExtensionMenuItem* item = CreateTestItem(extension);
+  int id = manager_.AddContextItem(extension, item);
   ASSERT_GT(id, 0);
 
   EXPECT_CALL(profile, GetExtensionMessageService())
diff --git a/chrome/browser/extensions/extension_protocols.cc b/chrome/browser/extensions/extension_protocols.cc
index 0a868f4..83ef404 100644
--- a/chrome/browser/extensions/extension_protocols.cc
+++ b/chrome/browser/extensions/extension_protocols.cc
@@ -85,9 +85,10 @@ static URLRequestJob* CreateExtensionURLRequestJob(URLRequest* request,
     return new URLRequestErrorJob(request, net::ERR_ADDRESS_UNREACHABLE);
 
   // chrome-extension://extension-id/resource/path.js
-  FilePath directory_path = context->GetPathForExtension(request->url().host());
+  const std::string& extension_id = request->url().host();
+  FilePath directory_path = context->GetPathForExtension(extension_id);
   if (directory_path.value().empty()) {
-    LOG(WARNING) << "Failed to GetPathForExtension: " << request->url().host();
+    LOG(WARNING) << "Failed to GetPathForExtension: " << extension_id;
     return NULL;
   }
 
@@ -117,7 +118,7 @@ static URLRequestJob* CreateExtensionURLRequestJob(URLRequest* request,
   }
   // TODO(tc): Move all of these files into resources.pak so we don't break
   // when updating on Linux.
-  ExtensionResource resource(directory_path,
+  ExtensionResource resource(extension_id, directory_path,
       extension_file_util::ExtensionURLToRelativeFilePath(request->url()));
 
   return new URLRequestFileJob(request,
@@ -134,7 +135,7 @@ static URLRequestJob* CreateUserScriptURLRequestJob(URLRequest* request,
   // chrome-user-script:/user-script-name.user.js
   FilePath directory_path = context->user_script_dir_path();
 
-  ExtensionResource resource(directory_path,
+  ExtensionResource resource(request->url().host(), directory_path,
       extension_file_util::ExtensionURLToRelativeFilePath(request->url()));
 
   return new URLRequestFileJob(request, resource.GetFilePath());
diff --git a/chrome/browser/extensions/extensions_service_unittest.cc b/chrome/browser/extensions/extensions_service_unittest.cc
index f3660cf..cc50bab 100644
--- a/chrome/browser/extensions/extensions_service_unittest.cc
+++ b/chrome/browser/extensions/extensions_service_unittest.cc
@@ -257,7 +257,7 @@ void ExtensionsServiceTestBase::InitializeExtensionsService(
       new JsonPrefStore( /* user defined preference values */
           pref_file,
           ChromeThread::GetMessageLoopProxyForThread(ChromeThread::FILE)),
-      NULL /* No suggested preference values */ )));
+      NULL /* No suggested preference values */)));
 
   Profile::RegisterUserPrefs(prefs_.get());
   browser::RegisterUserPrefs(prefs_.get());
@@ -634,12 +634,14 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) {
   EXPECT_EQ("https://*.google.com/*",
             scripts[0].url_patterns()[2].GetAsString());
   EXPECT_EQ(2u, scripts[0].js_scripts().size());
-  ExtensionResource resource00(scripts[0].js_scripts()[0].extension_root(),
+  ExtensionResource resource00(extension->id(),
+                               scripts[0].js_scripts()[0].extension_root(),
                                scripts[0].js_scripts()[0].relative_path());
   FilePath expected_path(extension->path().AppendASCII("script1.js"));
   ASSERT_TRUE(file_util::AbsolutePath(&expected_path));
   EXPECT_TRUE(resource00.ComparePathWithDefault(expected_path));
-  ExtensionResource resource01(scripts[0].js_scripts()[1].extension_root(),
+  ExtensionResource resource01(extension->id(),
+                               scripts[0].js_scripts()[1].extension_root(),
                                scripts[0].js_scripts()[1].relative_path());
   expected_path = extension->path().AppendASCII("script2.js");
   ASSERT_TRUE(file_util::AbsolutePath(&expected_path));
@@ -647,7 +649,8 @@ TEST_F(ExtensionsServiceTest, LoadAllExtensionsFromDirectorySuccess) {
   EXPECT_TRUE(extension->plugins().empty());
   EXPECT_EQ(1u, scripts[1].url_patterns().size());
   EXPECT_EQ("http://*.news.com/*", scripts[1].url_patterns()[0].GetAsString());
-  ExtensionResource resource10(scripts[1].js_scripts()[0].extension_root(),
+  ExtensionResource resource10(extension->id(),
+                               scripts[1].js_scripts()[0].extension_root(),
                                scripts[1].js_scripts()[0].relative_path());
   expected_path =
       extension->path().AppendASCII("js_files").AppendASCII("script3.js");
diff --git a/chrome/browser/extensions/file_reader_unittest.cc b/chrome/browser/extensions/file_reader_unittest.cc
index dbe9ceb..b45afeb 100644
--- a/chrome/browser/extensions/file_reader_unittest.cc
+++ b/chrome/browser/extensions/file_reader_unittest.cc
@@ -10,6 +10,7 @@
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/browser/extensions/file_reader.h"
 #include "chrome/common/chrome_paths.h"
+#include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_resource.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -51,7 +52,10 @@ class Receiver {
 void RunBasicTest(const char* filename) {
   FilePath path;
   PathService::Get(chrome::DIR_TEST_DATA, &path);
-  ExtensionResource resource(path, FilePath().AppendASCII(filename));
+  std::string extension_id;
+  Extension::GenerateId("test", &extension_id);
+  ExtensionResource resource(extension_id, path,
+                             FilePath().AppendASCII(filename));
   path = path.AppendASCII(filename);
 
   std::string file_contents;
@@ -80,7 +84,9 @@ TEST_F(FileReaderTest, BiggerFile) {
 TEST_F(FileReaderTest, NonExistantFile) {
   FilePath path;
   PathService::Get(chrome::DIR_TEST_DATA, &path);
-  ExtensionResource resource(path, FilePath(
+  std::string extension_id;
+  Extension::GenerateId("test", &extension_id);
+  ExtensionResource resource(extension_id, path, FilePath(
       FILE_PATH_LITERAL("file_that_does_not_exist")));
   path = path.AppendASCII("file_that_does_not_exist");
 
diff --git a/chrome/browser/extensions/image_loading_tracker.cc b/chrome/browser/extensions/image_loading_tracker.cc
index ea174ff..5be22d8 100644
--- a/chrome/browser/extensions/image_loading_tracker.cc
+++ b/chrome/browser/extensions/image_loading_tracker.cc
@@ -14,6 +14,8 @@
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "webkit/glue/image_decoder.h"
 
+ImageLoadingTracker::Observer::~Observer() {}
+
 ////////////////////////////////////////////////////////////////////////////////
 // ImageLoadingTracker::ImageLoader
 
@@ -54,7 +56,7 @@ class ImageLoadingTracker::ImageLoader
     std::string file_contents;
     FilePath path = resource.GetFilePath();
     if (path.empty() || !file_util::ReadFileToString(path, &file_contents)) {
-      ReportBack(NULL, resource, id);
+      ReportBack(NULL, resource, gfx::Size(), id);
       return;
     }
 
@@ -65,10 +67,12 @@ class ImageLoadingTracker::ImageLoader
     scoped_ptr<SkBitmap> decoded(new SkBitmap());
     *decoded = decoder.Decode(data, file_contents.length());
     if (decoded->empty()) {
-      ReportBack(NULL, resource, id);
+      ReportBack(NULL, resource, gfx::Size(), id);
       return;  // Unable to decode.
     }
 
+    gfx::Size original_size(decoded->width(), decoded->height());
+
     if (decoded->width() > max_size.width() ||
         decoded->height() > max_size.height()) {
       // The bitmap is too big, re-sample.
@@ -77,25 +81,25 @@ class ImageLoadingTracker::ImageLoader
           max_size.width(), max_size.height());
     }
 
-    ReportBack(decoded.release(), resource, id);
+    ReportBack(decoded.release(), resource, original_size, id);
   }
 
   void ReportBack(SkBitmap* image, const ExtensionResource& resource,
-                  int id) {
+                  const gfx::Size& original_size, int id) {
     DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
 
     ChromeThread::PostTask(
         callback_thread_id_, FROM_HERE,
         NewRunnableMethod(this, &ImageLoader::ReportOnUIThread,
-                          image, resource, id));
+                          image, resource, original_size, id));
   }
 
   void ReportOnUIThread(SkBitmap* image, ExtensionResource resource,
-                        int id) {
+                        const gfx::Size& original_size, int id) {
     DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::FILE));
 
     if (tracker_)
-      tracker_->OnImageLoaded(image, resource, id);
+      tracker_->OnImageLoaded(image, resource, original_size, id);
 
     delete image;
   }
@@ -138,16 +142,16 @@ void ImageLoadingTracker::LoadImage(Extension* extension,
   // back.
   int id = next_id_++;
   if (resource.relative_path().empty()) {
-    OnImageLoaded(NULL, resource, id);
+    OnImageLoaded(NULL, resource, max_size, id);
     return;
   }
 
   DCHECK(extension->path() == resource.extension_root());
 
   // See if the extension has the image already.
-  if (extension->HasCachedImage(resource)) {
-    SkBitmap image = extension->GetCachedImage(resource);
-    OnImageLoaded(&image, resource, id);
+  if (extension->HasCachedImage(resource, max_size)) {
+    SkBitmap image = extension->GetCachedImage(resource, max_size);
+    OnImageLoaded(&image, resource, max_size, id);
     return;
   }
 
@@ -165,10 +169,12 @@ void ImageLoadingTracker::LoadImage(Extension* extension,
 void ImageLoadingTracker::OnImageLoaded(
     SkBitmap* image,
     const ExtensionResource& resource,
+    const gfx::Size& original_size,
     int id) {
   LoadMap::iterator i = load_map_.find(id);
   if (i != load_map_.end()) {
-    i->second->SetCachedImage(resource, image ? *image : SkBitmap());
+    i->second->SetCachedImage(resource, image ? *image : SkBitmap(),
+                              original_size);
     load_map_.erase(i);
   }
 
diff --git a/chrome/browser/extensions/image_loading_tracker.h b/chrome/browser/extensions/image_loading_tracker.h
index 653a9a9..e32166a 100644
--- a/chrome/browser/extensions/image_loading_tracker.h
+++ b/chrome/browser/extensions/image_loading_tracker.h
@@ -43,6 +43,8 @@ class ImageLoadingTracker : public NotificationObserver {
 
   class Observer {
    public:
+    virtual ~Observer();
+
     // Will be called when the image with the given index has loaded.
     // The |image| is owned by the tracker, so the observer should make a copy
     // if they need to access it after this call. |image| can be null if valid
@@ -72,10 +74,11 @@ class ImageLoadingTracker : public NotificationObserver {
   // When an image has finished loaded and been resized on the file thread, it
   // is posted back to this method on the original thread.  This method then
   // calls the observer's OnImageLoaded and deletes the ImageLoadingTracker if
-  // it was the last image in the list.
+  // it was the last image in the list. The |original_size| should be the size
+  // of the image before any resizing was done.
   // |image| may be null if the file failed to decode.
   void OnImageLoaded(SkBitmap* image, const ExtensionResource& resource,
-                     int id);
+                     const gfx::Size& original_size, int id);
 
   // NotificationObserver method. If an extension is uninstalled while we're
   // waiting for the image we remove the entry from load_map_.
diff --git a/chrome/browser/extensions/image_loading_tracker_unittest.cc b/chrome/browser/extensions/image_loading_tracker_unittest.cc
index 4659496..974ce54 100644
--- a/chrome/browser/extensions/image_loading_tracker_unittest.cc
+++ b/chrome/browser/extensions/image_loading_tracker_unittest.cc
@@ -102,11 +102,12 @@ TEST_F(ImageLoadingTrackerTest, Cache) {
 
   ExtensionResource image_resource =
       extension->GetIconPath(Extension::EXTENSION_ICON_SMALLISH);
+  gfx::Size max_size(Extension::EXTENSION_ICON_SMALLISH,
+                     Extension::EXTENSION_ICON_SMALLISH);
   ImageLoadingTracker loader(static_cast<ImageLoadingTracker::Observer*>(this));
   loader.LoadImage(extension.get(),
                    image_resource,
-                   gfx::Size(Extension::EXTENSION_ICON_SMALLISH,
-                             Extension::EXTENSION_ICON_SMALLISH),
+                   max_size,
                    ImageLoadingTracker::CACHE);
 
   // The image isn't cached, so we should not have received notification.
@@ -121,17 +122,16 @@ TEST_F(ImageLoadingTrackerTest, Cache) {
   EXPECT_EQ(Extension::EXTENSION_ICON_SMALLISH, image_.width());
 
   // The image should be cached in the Extension.
-  EXPECT_TRUE(extension->HasCachedImage(image_resource));
+  EXPECT_TRUE(extension->HasCachedImage(image_resource, max_size));
 
   // Make sure the image is in the extension.
   EXPECT_EQ(Extension::EXTENSION_ICON_SMALLISH,
-            extension->GetCachedImage(image_resource).width());
+            extension->GetCachedImage(image_resource, max_size).width());
 
   // Ask the tracker for the image again, this should call us back immediately.
   loader.LoadImage(extension.get(),
                    image_resource,
-                   gfx::Size(Extension::EXTENSION_ICON_SMALLISH,
-                             Extension::EXTENSION_ICON_SMALLISH),
+                   max_size,
                    ImageLoadingTracker::CACHE);
   // We should have gotten the image.
   EXPECT_EQ(1, image_loaded_count());
diff --git a/chrome/browser/tab_contents/render_view_context_menu.cc b/chrome/browser/tab_contents/render_view_context_menu.cc
index ef4e6d7..aaed42e 100644
--- a/chrome/browser/tab_contents/render_view_context_menu.cc
+++ b/chrome/browser/tab_contents/render_view_context_menu.cc
@@ -41,6 +41,7 @@
 #include "chrome/common/chrome_switches.h"
 #include "chrome/common/pref_names.h"
 #include "chrome/common/url_constants.h"
+#include "gfx/favicon_size.h"
 #include "grit/generated_resources.h"
 #include "net/base/escape.h"
 #include "net/url_request/url_request.h"
@@ -195,6 +196,7 @@ void RenderViewContextMenu::AppendExtensionItems(
     menu_model_.AddSubMenu(menu_id, title, submenu);
     RecursivelyAppendExtensionItems(submenu_items, submenu, index);
   }
+  SetExtensionIcon(extension_id);
 }
 
 void RenderViewContextMenu::RecursivelyAppendExtensionItems(
@@ -253,6 +255,20 @@ void RenderViewContextMenu::RecursivelyAppendExtensionItems(
   }
 }
 
+void RenderViewContextMenu::SetExtensionIcon(const std::string& extension_id) {
+  ExtensionsService* service = profile_->GetExtensionsService();
+  ExtensionMenuManager* menu_manager = service->menu_manager();
+
+  int index = menu_model_.GetItemCount() - 1;
+  DCHECK(index >= 0);
+
+  const SkBitmap& icon = menu_manager->GetIconForExtension(extension_id);
+  DCHECK(icon.width() == kFavIconSize);
+  DCHECK(icon.height() == kFavIconSize);
+
+  menu_model_.SetIcon(index, icon);
+}
+
 void RenderViewContextMenu::AppendAllExtensionItems() {
   extension_item_map_.clear();
   ExtensionsService* service = profile_->GetExtensionsService();
diff --git a/chrome/browser/tab_contents/render_view_context_menu.h b/chrome/browser/tab_contents/render_view_context_menu.h
index 97d0d5c..2874aee 100644
--- a/chrome/browser/tab_contents/render_view_context_menu.h
+++ b/chrome/browser/tab_contents/render_view_context_menu.h
@@ -91,6 +91,8 @@ class RenderViewContextMenu : public menus::SimpleMenuModel::Delegate {
       const std::vector<ExtensionMenuItem*>& items,
       menus::SimpleMenuModel* menu_model,
       int *index);
+  // This will set the icon on the most recently-added item in the menu_model_.
+  void SetExtensionIcon(const std::string& extension_id);
 
   // Opens the specified URL string in a new tab.  If |in_current_window| is
   // false, a new window is created to hold the new tab.
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 47216fc..3631af0 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -760,11 +760,11 @@ ExtensionResource Extension::GetResource(const std::string& relative_path) {
 #elif defined(OS_WIN)
   FilePath relative_file_path(UTF8ToWide(relative_path));
 #endif
-  return ExtensionResource(path(), relative_file_path);
+  return ExtensionResource(id(), path(), relative_file_path);
 }
 
 ExtensionResource Extension::GetResource(const FilePath& relative_file_path) {
-  return ExtensionResource(path(), relative_file_path);
+  return ExtensionResource(id(), path(), relative_file_path);
 }
 
 // TODO(rafaelw): Move ParsePEMKeyBytes, ProducePEM & FormatPEMForOutput to a
@@ -1584,28 +1584,63 @@ void Extension::SetBackgroundPageReady() {
       NotificationService::NoDetails());
 }
 
+static std::string SizeToString(const gfx::Size& max_size) {
+  return IntToString(max_size.width()) + "x" + IntToString(max_size.height());
+}
+
 void Extension::SetCachedImage(const ExtensionResource& source,
-                               const SkBitmap& image) {
+                               const SkBitmap& image,
+                               const gfx::Size& original_size) {
   DCHECK(source.extension_root() == path());  // The resource must come from
                                               // this extension.
-  image_cache_[source.relative_path()] = image;
+  const FilePath& path = source.relative_path();
+  gfx::Size actual_size(image.width(), image.height());
+  if (actual_size == original_size) {
+    image_cache_[ImageCacheKey(path, std::string())] = image;
+  } else {
+    image_cache_[ImageCacheKey(path, SizeToString(actual_size))] = image;
+  }
 }
 
-bool Extension::HasCachedImage(const ExtensionResource& source) {
+bool Extension::HasCachedImage(const ExtensionResource& source,
+                               const gfx::Size& max_size) {
   DCHECK(source.extension_root() == path());  // The resource must come from
                                               // this extension.
-  return image_cache_.find(source.relative_path()) != image_cache_.end();
+  return GetCachedImageImpl(source, max_size) != NULL;
 }
 
-SkBitmap Extension::GetCachedImage(const ExtensionResource& source) {
+SkBitmap Extension::GetCachedImage(const ExtensionResource& source,
+                                   const gfx::Size& max_size) {
   DCHECK(source.extension_root() == path());  // The resource must come from
                                               // this extension.
-  ImageCache::iterator i = image_cache_.find(source.relative_path());
-  if (i == image_cache_.end())
-    return SkBitmap();
-  return i->second;
+  SkBitmap* image = GetCachedImageImpl(source, max_size);
+  return image ? *image : SkBitmap();
 }
 
+SkBitmap* Extension::GetCachedImageImpl(const ExtensionResource& source,
+                                        const gfx::Size& max_size) {
+  const FilePath& path = source.relative_path();
+
+  // Look for exact size match.
+  ImageCache::iterator i = image_cache_.find(
+      ImageCacheKey(path, SizeToString(max_size)));
+  if (i != image_cache_.end())
+    return &(i->second);
+
+  // If we have the original size version cached, return that if it's small
+  // enough.
+  i = image_cache_.find(ImageCacheKey(path, std::string()));
+  if (i != image_cache_.end()) {
+    SkBitmap& image = i->second;
+    if (image.width() <= max_size.width() &&
+        image.height() <= max_size.height())
+      return &(i->second);
+  }
+
+  return NULL;
+}
+
+
 ExtensionResource Extension::GetIconPath(Icons icon) {
   std::map<int, std::string>::const_iterator iter = icons_.find(icon);
   if (iter == icons_.end())
diff --git a/chrome/common/extensions/extension.h b/chrome/common/extensions/extension.h
index 1af1d23..0d4c10a 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -17,6 +17,7 @@
 #include "chrome/common/extensions/extension_extent.h"
 #include "chrome/common/extensions/user_script.h"
 #include "chrome/common/extensions/url_pattern.h"
+#include "gfx/size.h"
 #include "googleurl/src/gurl.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
 #include "third_party/skia/include/core/SkBitmap.h"
@@ -354,14 +355,29 @@ class Extension {
   void set_being_upgraded(bool value) { being_upgraded_ = value; }
 
   // Image cache related methods. These are only valid on the UI thread and
-  // not maintained by this class. See ImageLoadingTracker for usage.
+  // not maintained by this class. See ImageLoadingTracker for usage. The
+  // |original_size| parameter should be the size of the image at |source|
+  // before any scaling may have been done to produce the pixels in |image|.
   void SetCachedImage(const ExtensionResource& source,
-                      const SkBitmap& image);
-  bool HasCachedImage(const ExtensionResource& source);
-  SkBitmap GetCachedImage(const ExtensionResource& source);
+                      const SkBitmap& image,
+                      const gfx::Size& original_size);
+  bool HasCachedImage(const ExtensionResource& source,
+                      const gfx::Size& max_size);
+  SkBitmap GetCachedImage(const ExtensionResource& source,
+                          const gfx::Size& max_size);
 
  private:
-  typedef std::map<FilePath, SkBitmap> ImageCache;
+  // We keep a cache of images loaded from extension resources based on their
+  // path and a string representation of a size that may have been used to
+  // scale it (or the empty string if the image is at its original size).
+  typedef std::pair<FilePath, std::string> ImageCacheKey;
+  typedef std::map<ImageCacheKey, SkBitmap> ImageCache;
+
+  // Helper function for implementing HasCachedImage/GetCachedImage. A return
+  // value of NULL means there is no matching image cached (we allow caching an
+  // empty SkBitmap).
+  SkBitmap* GetCachedImageImpl(const ExtensionResource& source,
+                               const gfx::Size& max_size);
 
   // Helper method that loads a UserScript object from a
   // dictionary in the content_script list of the manifest.
@@ -525,8 +541,7 @@ class Extension {
   int launch_width_;
   int launch_height_;
 
-  // Cached images for this extension. This maps from the relative_path of the
-  // resource to the cached image.
+  // Cached images for this extension.
   ImageCache image_cache_;
 
   // The omnibox keyword for this extension, or empty if there is none.
diff --git a/chrome/common/extensions/extension_resource.cc b/chrome/common/extensions/extension_resource.cc
index eb5b3626..d3c5898 100644
--- a/chrome/common/extensions/extension_resource.cc
+++ b/chrome/common/extensions/extension_resource.cc
@@ -14,9 +14,11 @@ bool ExtensionResource::check_for_file_thread_ = false;
 ExtensionResource::ExtensionResource() {
 }
 
-ExtensionResource::ExtensionResource(const FilePath& extension_root,
+ExtensionResource::ExtensionResource(const std::string& extension_id,
+                                     const FilePath& extension_root,
                                      const FilePath& relative_path)
-    : extension_root_(extension_root),
+    : extension_id_(extension_id),
+      extension_root_(extension_root),
       relative_path_(relative_path) {
 }
 
diff --git a/chrome/common/extensions/extension_resource.h b/chrome/common/extensions/extension_resource.h
index 8856414..295754e 100644
--- a/chrome/common/extensions/extension_resource.h
+++ b/chrome/common/extensions/extension_resource.h
@@ -5,6 +5,8 @@
 #ifndef CHROME_COMMON_EXTENSIONS_EXTENSION_RESOURCE_H_
 #define CHROME_COMMON_EXTENSIONS_EXTENSION_RESOURCE_H_
 
+#include <string>
+
 #include "base/file_path.h"
 #include "base/platform_thread.h"
 
@@ -16,7 +18,8 @@ class ExtensionResource {
  public:
   ExtensionResource();
 
-  ExtensionResource(const FilePath& extension_root,
+  ExtensionResource(const std::string& extension_id,
+                    const FilePath& extension_root,
                     const FilePath& relative_path);
 
   // Returns actual path to the resource (default or locale specific). In the
@@ -51,14 +54,20 @@ class ExtensionResource {
   static void CheckFileAccessFromFileThread();
 
   // Getters
+  const std::string& extension_id() const { return extension_id_; }
   const FilePath& extension_root() const { return extension_root_; }
   const FilePath& relative_path() const { return relative_path_; }
 
+  bool empty() { return extension_root().empty(); }
+
   // Unit test helpers.
   FilePath::StringType NormalizeSeperators(FilePath::StringType path) const;
   bool ComparePathWithDefault(const FilePath& path) const;
 
  private:
+  // The id of the extension that this resource is associated with.
+  std::string extension_id_;
+
   // Extension root.
   FilePath extension_root_;
 
diff --git a/chrome/common/extensions/extension_resource_unittest.cc b/chrome/common/extensions/extension_resource_unittest.cc
index 258227f..189e4d6 100644
--- a/chrome/common/extensions/extension_resource_unittest.cc
+++ b/chrome/common/extensions/extension_resource_unittest.cc
@@ -33,7 +33,9 @@ TEST(ExtensionResourceTest, CreateWithMissingResourceOnDisk) {
   ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &root_path));
   FilePath relative_path;
   relative_path = relative_path.AppendASCII("cira.js");
-  ExtensionResource resource(root_path, relative_path);
+  std::string extension_id;
+  Extension::GenerateId("test", &extension_id);
+  ExtensionResource resource(extension_id, root_path, relative_path);
 
   // The path doesn't exist on disk, we will be returned an empty path.
   EXPECT_EQ(root_path.value(), resource.extension_root().value());
@@ -68,7 +70,10 @@ TEST(ExtensionResourceTest, CreateWithAllResourcesOnDisk) {
   }
 
   FilePath path;
-  ExtensionResource resource(temp.path(), FilePath().AppendASCII(filename));
+  std::string extension_id;
+  Extension::GenerateId("test", &extension_id);
+  ExtensionResource resource(extension_id, temp.path(),
+                             FilePath().AppendASCII(filename));
   FilePath resolved_path = resource.GetFilePath();
 
   FilePath expected_path;
diff --git a/chrome/common/extensions/extension_unittest.cc b/chrome/common/extensions/extension_unittest.cc
index 6db3f61..72da48d 100644
--- a/chrome/common/extensions/extension_unittest.cc
+++ b/chrome/common/extensions/extension_unittest.cc
@@ -20,9 +20,12 @@
 #include "chrome/common/extensions/extension_action.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/extension_error_utils.h"
+#include "chrome/common/extensions/extension_resource.h"
 #include "chrome/common/json_value_serializer.h"
 #include "chrome/common/url_constants.h"
+#include "gfx/codec/png_codec.h"
 #include "net/base/mime_sniffer.h"
+#include "skia/ext/image_operations.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace keys = extension_manifest_keys;
@@ -844,3 +847,86 @@ TEST(ExtensionTest, IsPrivilegeIncrease) {
         << kTests[i].base_name;
   }
 }
+
+// Returns a copy of |source| resized to |size| x |size|.
+static SkBitmap ResizedCopy(const SkBitmap& source, int size) {
+  return skia::ImageOperations::Resize(source,
+                                       skia::ImageOperations::RESIZE_LANCZOS3,
+                                       size,
+                                       size);
+}
+
+static bool SizeEquals(const SkBitmap& bitmap, const gfx::Size& size) {
+  return bitmap.width() == size.width() && bitmap.height() == size.height();
+}
+
+TEST(ExtensionTest, ImageCaching) {
+  FilePath path;
+  ASSERT_TRUE(PathService::Get(chrome::DIR_TEST_DATA, &path));
+  path = path.AppendASCII("extensions");
+
+  // Initialize the Extension.
+  std::string errors;
+  scoped_ptr<Extension> extension(new Extension(path));
+  DictionaryValue values;
+  values.SetString(keys::kName, "test");
+  values.SetString(keys::kVersion, "0.1");
+  ASSERT_TRUE(extension->InitFromValue(values, false, &errors));
+
+  // Create an ExtensionResource pointing at an icon.
+  FilePath icon_relative_path(FILE_PATH_LITERAL("icon3.png"));
+  ExtensionResource resource(extension->id(),
+                             extension->path(),
+                             icon_relative_path);
+
+  // Read in the icon file.
+  FilePath icon_absolute_path = extension->path().Append(icon_relative_path);
+  std::string raw_png;
+  ASSERT_TRUE(file_util::ReadFileToString(icon_absolute_path, &raw_png));
+  SkBitmap image;
+  ASSERT_TRUE(gfx::PNGCodec::Decode(
+      reinterpret_cast<const unsigned char*>(raw_png.data()),
+      raw_png.length(),
+      &image));
+
+  // Make sure the icon file is the size we expect.
+  gfx::Size original_size(66, 66);
+  ASSERT_EQ(image.width(), original_size.width());
+  ASSERT_EQ(image.height(), original_size.height());
+
+  // Create two resized versions at size 16x16 and 24x24.
+  SkBitmap image16 = ResizedCopy(image, 16);
+  SkBitmap image24 = ResizedCopy(image, 24);
+
+  gfx::Size size16(16, 16);
+  gfx::Size size24(24, 24);
+
+  // Cache the 16x16 copy.
+  EXPECT_FALSE(extension->HasCachedImage(resource, size16));
+  extension->SetCachedImage(resource, image16, original_size);
+  EXPECT_TRUE(extension->HasCachedImage(resource, size16));
+  EXPECT_TRUE(SizeEquals(extension->GetCachedImage(resource, size16), size16));
+  EXPECT_FALSE(extension->HasCachedImage(resource, size24));
+  EXPECT_FALSE(extension->HasCachedImage(resource, original_size));
+
+  // Cache the 24x24 copy.
+  extension->SetCachedImage(resource, image24, original_size);
+  EXPECT_TRUE(extension->HasCachedImage(resource, size24));
+  EXPECT_TRUE(SizeEquals(extension->GetCachedImage(resource, size24), size24));
+  EXPECT_FALSE(extension->HasCachedImage(resource, original_size));
+
+  // Cache the original, and verify that it gets returned when we ask for a
+  // max_size that is larger than the original.
+  gfx::Size size128(128, 128);
+  EXPECT_TRUE(image.width() < size128.width() &&
+              image.height() < size128.height());
+  extension->SetCachedImage(resource, image, original_size);
+  EXPECT_TRUE(extension->HasCachedImage(resource, original_size));
+  EXPECT_TRUE(extension->HasCachedImage(resource, size128));
+  EXPECT_TRUE(SizeEquals(extension->GetCachedImage(resource, original_size),
+                         original_size));
+  EXPECT_TRUE(SizeEquals(extension->GetCachedImage(resource, size128),
+                         original_size));
+  EXPECT_EQ(extension->GetCachedImage(resource, original_size).getPixels(),
+            extension->GetCachedImage(resource, size128).getPixels());
+}
diff --git a/chrome/test/data/extensions/icon3.png b/chrome/test/data/extensions/icon3.png
new file mode 100755
index 0000000..4421311
--- /dev/null
+++ b/chrome/test/data/extensions/icon3.png