Adds ability for ImageLoadingTracker to cache images.

BUG=none
TEST=none

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

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

19 files changed, 354 insertions, 49 deletions

diff --git a/chrome/browser/cocoa/extensions/browser_action_button.mm b/chrome/browser/cocoa/extensions/browser_action_button.mm
index fe456c6..0794b25 100644
--- a/chrome/browser/cocoa/extensions/browser_action_button.mm
+++ b/chrome/browser/cocoa/extensions/browser_action_button.mm
@@ -57,9 +57,10 @@ class ExtensionImageTrackerBridge : public NotificationObserver,
     // changed at runtime, so we can load this now and cache it.
     std::string path = extension->browser_action()->default_icon_path();
     if (!path.empty()) {
-      tracker_.LoadImage(extension->GetResource(path),
-          gfx::Size(Extension::kBrowserActionIconMaxSize,
-                    Extension::kBrowserActionIconMaxSize));
+      tracker_.LoadImage(extension, extension->GetResource(path),
+                         gfx::Size(Extension::kBrowserActionIconMaxSize,
+                                   Extension::kBrowserActionIconMaxSize),
+                         ImageLoadingTracker::DONT_CACHE);
     }
     registrar_.Add(this, NotificationType::EXTENSION_BROWSER_ACTION_UPDATED,
                    Source<ExtensionAction>(extension->browser_action()));
diff --git a/chrome/browser/cocoa/location_bar_view_mac.mm b/chrome/browser/cocoa/location_bar_view_mac.mm
index 0035101..8ba39e83 100644
--- a/chrome/browser/cocoa/location_bar_view_mac.mm
+++ b/chrome/browser/cocoa/location_bar_view_mac.mm
@@ -627,9 +627,10 @@ LocationBarViewMac::PageActionImageView::PageActionImageView(
 
   for (std::vector<std::string>::iterator iter = icon_paths.begin();
        iter != icon_paths.end(); ++iter) {
-    tracker_.LoadImage(extension->GetResource(*iter),
+    tracker_.LoadImage(extension, extension->GetResource(*iter),
                        gfx::Size(Extension::kPageActionIconMaxSize,
-                                 Extension::kPageActionIconMaxSize));
+                                 Extension::kPageActionIconMaxSize),
+                       ImageLoadingTracker::DONT_CACHE);
   }
 
   registrar_.Add(this, NotificationType::EXTENSION_HOST_VIEW_SHOULD_CLOSE,
diff --git a/chrome/browser/extensions/extension_install_ui.cc b/chrome/browser/extensions/extension_install_ui.cc
index b95b957..3accc44 100644
--- a/chrome/browser/extensions/extension_install_ui.cc
+++ b/chrome/browser/extensions/extension_install_ui.cc
@@ -377,9 +377,10 @@ void ExtensionInstallUI::ShowConfirmation(PromptType prompt_type) {
   prompt_type_ = prompt_type;
   ExtensionResource image =
       extension_->GetIconPath(Extension::EXTENSION_ICON_LARGE);
-  tracker_.LoadImage(image,
+  tracker_.LoadImage(extension_, image,
                      gfx::Size(Extension::EXTENSION_ICON_LARGE,
-                               Extension::EXTENSION_ICON_LARGE));
+                               Extension::EXTENSION_ICON_LARGE),
+                     ImageLoadingTracker::DONT_CACHE);
 }
 
 #if defined(OS_MACOSX)
diff --git a/chrome/browser/extensions/image_loading_tracker.cc b/chrome/browser/extensions/image_loading_tracker.cc
index a1f4ad2..cdd1fef 100644
--- a/chrome/browser/extensions/image_loading_tracker.cc
+++ b/chrome/browser/extensions/image_loading_tracker.cc
@@ -7,6 +7,8 @@
 #include "base/file_util.h"
 #include "chrome/browser/chrome_thread.h"
 #include "chrome/common/extensions/extension_resource.h"
+#include "chrome/common/notification_service.h"
+#include "chrome/common/notification_type.h"
 #include "skia/ext/image_operations.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "webkit/glue/image_decoder.h"
@@ -33,23 +35,25 @@ class ImageLoadingTracker::ImageLoader
 
   // Instructs the loader to load a task on the File thread.
   void LoadImage(const ExtensionResource& resource,
-                 const gfx::Size& max_size) {
+                 const gfx::Size& max_size,
+                 int id) {
     DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::FILE));
     ChromeThread::PostTask(
         ChromeThread::FILE, FROM_HERE,
         NewRunnableMethod(this, &ImageLoader::LoadOnFileThread, resource,
-                          max_size));
+                          max_size, id));
   }
 
   void LoadOnFileThread(ExtensionResource resource,
-                        const gfx::Size& max_size) {
+                        const gfx::Size& max_size,
+                        int id) {
     DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
 
     // Read the file from disk.
     std::string file_contents;
     FilePath path = resource.GetFilePath();
     if (path.empty() || !file_util::ReadFileToString(path, &file_contents)) {
-      ReportBack(NULL, resource);
+      ReportBack(NULL, resource, id);
       return;
     }
 
@@ -60,7 +64,7 @@ class ImageLoadingTracker::ImageLoader
     scoped_ptr<SkBitmap> decoded(new SkBitmap());
     *decoded = decoder.Decode(data, file_contents.length());
     if (decoded->empty()) {
-      ReportBack(NULL, resource);
+      ReportBack(NULL, resource, id);
       return;  // Unable to decode.
     }
 
@@ -72,26 +76,27 @@ class ImageLoadingTracker::ImageLoader
           max_size.width(), max_size.height());
     }
 
-    ReportBack(decoded.release(), resource);
+    ReportBack(decoded.release(), resource, id);
   }
 
-  void ReportBack(SkBitmap* image, const ExtensionResource& resource) {
+  void ReportBack(SkBitmap* image, const ExtensionResource& resource,
+                  int id) {
     DCHECK(ChromeThread::CurrentlyOn(ChromeThread::FILE));
 
     ChromeThread::PostTask(
         callback_thread_id_, FROM_HERE,
         NewRunnableMethod(this, &ImageLoader::ReportOnUIThread,
-                                image, resource));
+                          image, resource, id));
   }
 
-  void ReportOnUIThread(SkBitmap* image, ExtensionResource resource) {
+  void ReportOnUIThread(SkBitmap* image, ExtensionResource resource,
+                        int id) {
     DCHECK(!ChromeThread::CurrentlyOn(ChromeThread::FILE));
 
     if (tracker_)
-      tracker_->OnImageLoaded(image, resource);
+      tracker_->OnImageLoaded(image, resource, id);
 
-    if (image)
-      delete image;
+    delete image;
   }
 
  private:
@@ -110,7 +115,11 @@ class ImageLoadingTracker::ImageLoader
 
 ImageLoadingTracker::ImageLoadingTracker(Observer* observer)
     : observer_(observer),
-      responses_(0) {
+      next_id_(0) {
+  registrar_.Add(this, NotificationType::EXTENSION_UNLOADED,
+                 NotificationService::AllSources());
+  registrar_.Add(this, NotificationType::EXTENSION_UNLOADED_DISABLED,
+                 NotificationService::AllSources());
 }
 
 ImageLoadingTracker::~ImageLoadingTracker() {
@@ -120,23 +129,63 @@ ImageLoadingTracker::~ImageLoadingTracker() {
     loader_->StopTracking();
 }
 
-void ImageLoadingTracker::LoadImage(const ExtensionResource& resource,
-                                    gfx::Size max_size) {
+void ImageLoadingTracker::LoadImage(Extension* extension,
+                                    const ExtensionResource& resource,
+                                    const gfx::Size& max_size,
+                                    CacheParam cache) {
+  DCHECK(extension->path() == resource.extension_root());
+
   // If we don't have a path we don't need to do any further work, just respond
   // back.
+  int id = next_id_++;
   if (resource.relative_path().empty()) {
-    OnImageLoaded(NULL, resource);
+    OnImageLoaded(NULL, resource, id);
+    return;
+  }
+
+  // See if the extension has the image already.
+  if (extension->HasCachedImage(resource)) {
+    SkBitmap image = extension->GetCachedImage(resource);
+    OnImageLoaded(&image, resource, id);
     return;
   }
 
+  if (cache == CACHE) {
+    load_map_[id] = extension;
+  }
+
   // Instruct the ImageLoader to load this on the File thread. LoadImage does
   // not block.
   if (!loader_)
     loader_ = new ImageLoader(this);
-  loader_->LoadImage(resource, max_size);
+  loader_->LoadImage(resource, max_size, id);
 }
 
 void ImageLoadingTracker::OnImageLoaded(
-    SkBitmap* image, const ExtensionResource& resource) {
-  observer_->OnImageLoaded(image, resource, responses_++);
+    SkBitmap* image,
+    const ExtensionResource& resource,
+    int id) {
+  LoadMap::iterator i = load_map_.find(id);
+  if (i != load_map_.end()) {
+    i->second->SetCachedImage(resource, image ? *image : SkBitmap());
+    load_map_.erase(i);
+  }
+
+  observer_->OnImageLoaded(image, resource, id);
+}
+
+void ImageLoadingTracker::Observe(NotificationType type,
+                                  const NotificationSource& source,
+                                  const NotificationDetails& details) {
+  DCHECK(type == NotificationType::EXTENSION_UNLOADED ||
+         type == NotificationType::EXTENSION_UNLOADED_DISABLED);
+
+  Extension* extension = Details<Extension>(details).ptr();
+
+  // Remove all entries in the load_map_ referencing the extension. This ensures
+  // we don't attempt to cache the image when the load completes.
+  for (LoadMap::iterator i = load_map_.begin(); i != load_map_.end(); ++i) {
+    if (i->second == extension)
+      load_map_.erase(i);
+  }
 }
diff --git a/chrome/browser/extensions/image_loading_tracker.h b/chrome/browser/extensions/image_loading_tracker.h
index adcf009..e4df4cf 100644
--- a/chrome/browser/extensions/image_loading_tracker.h
+++ b/chrome/browser/extensions/image_loading_tracker.h
@@ -5,8 +5,12 @@
 #ifndef CHROME_BROWSER_EXTENSIONS_IMAGE_LOADING_TRACKER_H_
 #define CHROME_BROWSER_EXTENSIONS_IMAGE_LOADING_TRACKER_H_
 
+#include <map>
+
 #include "base/ref_counted.h"
 #include "chrome/common/extensions/extension.h"
+#include "chrome/common/notification_observer.h"
+#include "chrome/common/notification_registrar.h"
 
 class ExtensionResource;
 class SkBitmap;
@@ -23,12 +27,20 @@ namespace gfx {
 // To use this class, have your class derive from ImageLoadingTracker::Observer,
 // and add a member variable ImageLoadingTracker tracker_. Then override
 // Observer::OnImageLoaded and call:
-//   tracker_.LoadImage(resource, max_size);
+//   tracker_.LoadImage(extension, resource, max_size, false);
 // ... and wait for OnImageLoaded to be called back on you with a pointer to the
 // SkBitmap loaded.
+// NOTE: if the image is available already (or the resource is not valid), the
+// Observer is notified immediately from the call to LoadImage. In other words,
+// by the time LoadImage returns the observer has been notified.
 //
-class ImageLoadingTracker {
+class ImageLoadingTracker : public NotificationObserver {
  public:
+  enum CacheParam {
+    CACHE,
+    DONT_CACHE
+  };
+
   class Observer {
    public:
     // Will be called when the image with the given index has loaded.
@@ -37,7 +49,7 @@ class ImageLoadingTracker {
     // image was not found or it failed to decode. |resource| is the
     // ExtensionResource where the |image| came from and the |index| represents
     // the index of the image just loaded (starts at 0 and increments every
-    // time this function is called).
+    // time LoadImage is called).
     virtual void OnImageLoaded(SkBitmap* image, ExtensionResource resource,
                                int index) = 0;
   };
@@ -47,10 +59,14 @@ class ImageLoadingTracker {
 
   // Specify image resource to load. If the loaded image is larger than
   // |max_size| it will be resized to those dimensions.
-  void LoadImage(const ExtensionResource& resource,
-                 gfx::Size max_size);
+  void LoadImage(Extension* extension,
+                 const ExtensionResource& resource,
+                 const gfx::Size& max_size,
+                 CacheParam cache);
 
  private:
+  typedef std::map<int, Extension*> LoadMap;
+
   class ImageLoader;
 
   // When an image has finished loaded and been resized on the file thread, it
@@ -58,17 +74,31 @@ class ImageLoadingTracker {
   // calls the observer's OnImageLoaded and deletes the ImageLoadingTracker if
   // it was the last image in the list.
   // |image| may be null if the file failed to decode.
-  void OnImageLoaded(SkBitmap* image, const ExtensionResource& resource);
+  void OnImageLoaded(SkBitmap* image, const ExtensionResource& resource,
+                     int id);
+
+  // NotificationObserver method. If an extension is uninstalled while we're
+  // waiting for the image we remove the entry from load_map_.
+  virtual void Observe(NotificationType type,
+                       const NotificationSource& source,
+                       const NotificationDetails& details);
 
   // The view that is waiting for the image to load.
   Observer* observer_;
 
-  // The number of times we've reported back.
-  int responses_;
+  // ID to use for next image requested. This is an ever increasing integer.
+  int next_id_;
 
   // The object responsible for loading the image on the File thread.
   scoped_refptr<ImageLoader> loader_;
 
+  // If LoadImage is told to cache the result an entry is added here. The
+  // integer identifies the id assigned to the request. If the extension is
+  // deleted while fetching the image the entry is removed from the map.
+  LoadMap load_map_;
+
+  NotificationRegistrar registrar_;
+
   DISALLOW_COPY_AND_ASSIGN(ImageLoadingTracker);
 };
 
diff --git a/chrome/browser/extensions/image_loading_tracker_unittest.cc b/chrome/browser/extensions/image_loading_tracker_unittest.cc
new file mode 100644
index 0000000..dde786e
--- /dev/null
+++ b/chrome/browser/extensions/image_loading_tracker_unittest.cc
@@ -0,0 +1,175 @@
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "base/message_loop.h"
+#include "base/path_service.h"
+#include "chrome/browser/chrome_thread.h"
+#include "chrome/browser/extensions/image_loading_tracker.h"
+#include "chrome/common/chrome_paths.h"
+#include "chrome/common/json_value_serializer.h"
+#include "chrome/common/notification_service.h"
+#include "chrome/common/notification_type.h"
+#include "gfx/size.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+class ImageLoadingTrackerTest : public testing::Test,
+                                public ImageLoadingTracker::Observer {
+ public:
+  ImageLoadingTrackerTest()
+      : image_loaded_count_(0),
+        quit_in_image_loaded_(false),
+        ui_thread_(ChromeThread::UI, &ui_loop_),
+        file_thread_(ChromeThread::FILE),
+        io_thread_(ChromeThread::IO) {
+  }
+
+  virtual void OnImageLoaded(SkBitmap* image, ExtensionResource resource,
+                             int index) {
+    image_loaded_count_++;
+    if (quit_in_image_loaded_)
+      MessageLoop::current()->Quit();
+    if (image)
+      image_ = *image;
+    else
+      image_.reset();
+  }
+
+  void WaitForImageLoad() {
+    quit_in_image_loaded_ = true;
+    MessageLoop::current()->Run();
+    quit_in_image_loaded_ = false;
+  }
+
+  int image_loaded_count() {
+    int result = image_loaded_count_;
+    image_loaded_count_ = 0;
+    return result;
+  }
+
+  Extension* CreateExtension() {
+    // Create and load an extension.
+    FilePath test_file;
+    if (!PathService::Get(chrome::DIR_TEST_DATA, &test_file)) {
+      EXPECT_FALSE(true);
+      return NULL;
+    }
+    test_file = test_file.AppendASCII("extensions")
+                         .AppendASCII("image_loading_tracker");
+    std::string error;
+    JSONFileValueSerializer serializer(test_file.AppendASCII("app.json"));
+    scoped_ptr<DictionaryValue> valid_value(
+        static_cast<DictionaryValue*>(serializer.Deserialize(&error)));
+    EXPECT_EQ("", error);
+    if (error != "")
+      return NULL;
+
+    EXPECT_TRUE(valid_value.get());
+    if (!valid_value.get())
+      return NULL;
+
+    scoped_ptr<Extension> extension(new Extension(test_file));
+    if (!extension->InitFromValue(*valid_value, false, &error))
+      return NULL;
+
+    return extension.release();
+  }
+
+  SkBitmap image_;
+
+ private:
+  virtual void SetUp() {
+    file_thread_.Start();
+    io_thread_.Start();
+  }
+
+  int image_loaded_count_;
+  bool quit_in_image_loaded_;
+  MessageLoop ui_loop_;
+  ChromeThread ui_thread_;
+  ChromeThread file_thread_;
+  ChromeThread io_thread_;
+};
+
+// Tests asking ImageLoadingTracker to cache pushes the result to the Extension.
+TEST_F(ImageLoadingTrackerTest, Cache) {
+  scoped_ptr<Extension> extension(CreateExtension());
+  ASSERT_TRUE(extension.get() != NULL);
+
+  ExtensionResource image_resource =
+      extension->GetIconPath(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),
+                   ImageLoadingTracker::CACHE);
+
+  // The image isn't cached, so we should not have received notification.
+  EXPECT_EQ(0, image_loaded_count());
+
+  WaitForImageLoad();
+
+  // We should have gotten the image.
+  EXPECT_EQ(1, image_loaded_count());
+
+  // Check that the image was loaded.
+  EXPECT_EQ(Extension::EXTENSION_ICON_SMALLISH, image_.width());
+
+  // The image should be cached in the Extension.
+  EXPECT_TRUE(extension->HasCachedImage(image_resource));
+
+  // Make sure the image is in the extension.
+  EXPECT_EQ(Extension::EXTENSION_ICON_SMALLISH,
+            extension->GetCachedImage(image_resource).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),
+                   ImageLoadingTracker::CACHE);
+  // We should have gotten the image.
+  EXPECT_EQ(1, image_loaded_count());
+
+  // Check that the image was loaded.
+  EXPECT_EQ(Extension::EXTENSION_ICON_SMALLISH, image_.width());
+}
+
+// Tests deleting an extension while waiting for the image to load doesn't cause
+// problems.
+TEST_F(ImageLoadingTrackerTest, DeleteExtensionWhileWaitingForCache) {
+  scoped_ptr<Extension> extension(CreateExtension());
+  ASSERT_TRUE(extension.get() != NULL);
+
+  ExtensionResource image_resource =
+      extension->GetIconPath(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),
+                   ImageLoadingTracker::CACHE);
+
+  // The image isn't cached, so we should not have received notification.
+  EXPECT_EQ(0, image_loaded_count());
+
+  // Send out notification the extension was uninstalled.
+  NotificationService::current()->Notify(
+      NotificationType::EXTENSION_UNLOADED,
+      NotificationService::AllSources(),
+      Details<Extension>(extension.get()));
+
+  // Chuck the image, that way if anyone tries to access it we should crash or
+  // get valgrind errors.
+  extension.release();
+
+  WaitForImageLoad();
+
+  // Even though we deleted the extension, we should still get the image.
+  // We should still have gotten the image.
+  EXPECT_EQ(1, image_loaded_count());
+
+  // Check that the image was loaded.
+  EXPECT_EQ(Extension::EXTENSION_ICON_SMALLISH, image_.width());
+}
diff --git a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
index d177fab..d680251 100644
--- a/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
+++ b/chrome/browser/gtk/browser_actions_toolbar_gtk.cc
@@ -91,9 +91,10 @@ class BrowserActionButton : public NotificationObserver,
     // changed at runtime, so we can load this now and cache it.
     std::string path = extension_->browser_action()->default_icon_path();
     if (!path.empty()) {
-      tracker_.LoadImage(extension_->GetResource(path),
-          gfx::Size(Extension::kBrowserActionIconMaxSize,
-                    Extension::kBrowserActionIconMaxSize));
+      tracker_.LoadImage(extension_, extension_->GetResource(path),
+                         gfx::Size(Extension::kBrowserActionIconMaxSize,
+                                   Extension::kBrowserActionIconMaxSize),
+                         ImageLoadingTracker::DONT_CACHE);
     }
 
     g_signal_connect(button_.get(), "button-press-event",
diff --git a/chrome/browser/gtk/location_bar_view_gtk.cc b/chrome/browser/gtk/location_bar_view_gtk.cc
index bab7259..6115d57 100644
--- a/chrome/browser/gtk/location_bar_view_gtk.cc
+++ b/chrome/browser/gtk/location_bar_view_gtk.cc
@@ -1087,9 +1087,10 @@ LocationBarViewGtk::PageActionViewGtk::PageActionViewGtk(
 
   for (std::vector<std::string>::iterator iter = icon_paths.begin();
        iter != icon_paths.end(); ++iter) {
-    tracker_.LoadImage(extension->GetResource(*iter),
+    tracker_.LoadImage(extension, extension->GetResource(*iter),
                        gfx::Size(Extension::kPageActionIconMaxSize,
-                                 Extension::kPageActionIconMaxSize));
+                                 Extension::kPageActionIconMaxSize),
+                       ImageLoadingTracker::DONT_CACHE);
   }
 }
 
diff --git a/chrome/browser/views/browser_actions_container.cc b/chrome/browser/views/browser_actions_container.cc
index 7ccaee8..6c8aaa0 100644
--- a/chrome/browser/views/browser_actions_container.cc
+++ b/chrome/browser/views/browser_actions_container.cc
@@ -120,9 +120,10 @@ BrowserActionButton::BrowserActionButton(Extension* extension,
   // will crash. But since we know that ImageLoadingTracker is asynchronous,
   // this should be OK. And doing this in the constructor means that we don't
   // have to protect against it getting done multiple times.
-  tracker_.LoadImage(extension->GetResource(relative_path),
+  tracker_.LoadImage(extension, extension->GetResource(relative_path),
                      gfx::Size(Extension::kBrowserActionIconMaxSize,
-                               Extension::kBrowserActionIconMaxSize));
+                               Extension::kBrowserActionIconMaxSize),
+                     ImageLoadingTracker::DONT_CACHE);
 }
 
 void BrowserActionButton::Destroy() {
diff --git a/chrome/browser/views/infobars/extension_infobar.cc b/chrome/browser/views/infobars/extension_infobar.cc
index 2ae4b35..e58a017 100644
--- a/chrome/browser/views/infobars/extension_infobar.cc
+++ b/chrome/browser/views/infobars/extension_infobar.cc
@@ -140,12 +140,14 @@ void ExtensionInfoBar::SetupIconAndMenu() {
   AddChildView(menu_);
 
   ExtensionResource icon_resource;
-  Extension::Icons size = delegate_->extension_host()->extension()->
-      GetIconPathAllowLargerSize(&icon_resource,
-                                 Extension::EXTENSION_ICON_BITTY);
+  Extension* extension = delegate_->extension_host()->extension();
+  Extension::Icons size =
+      extension->GetIconPathAllowLargerSize(&icon_resource,
+                                            Extension::EXTENSION_ICON_BITTY);
   if (!icon_resource.relative_path().empty()) {
     // Create a tracker to load the image. It will report back on OnImageLoaded.
-    tracker_.LoadImage(icon_resource, gfx::Size(size, size));
+    tracker_.LoadImage(extension, icon_resource, gfx::Size(size, size),
+                       ImageLoadingTracker::DONT_CACHE);
   } else {
     OnImageLoaded(NULL, icon_resource, 0);  // |image|, |index|.
   }
diff --git a/chrome/browser/views/location_bar_view.cc b/chrome/browser/views/location_bar_view.cc
index 1d9ced9..250f1ef 100644
--- a/chrome/browser/views/location_bar_view.cc
+++ b/chrome/browser/views/location_bar_view.cc
@@ -1261,9 +1261,10 @@ LocationBarView::PageActionImageView::PageActionImageView(
 
   for (std::vector<std::string>::iterator iter = icon_paths.begin();
        iter != icon_paths.end(); ++iter) {
-    tracker_.LoadImage(extension->GetResource(*iter),
+    tracker_.LoadImage(extension, extension->GetResource(*iter),
                        gfx::Size(Extension::kPageActionIconMaxSize,
-                                 Extension::kPageActionIconMaxSize));
+                                 Extension::kPageActionIconMaxSize),
+                       ImageLoadingTracker::DONT_CACHE);
   }
 }
 
diff --git a/chrome/chrome_tests.gypi b/chrome/chrome_tests.gypi
index 28dbacb..74f327a 100644
--- a/chrome/chrome_tests.gypi
+++ b/chrome/chrome_tests.gypi
@@ -745,6 +745,7 @@
         'browser/extensions/extensions_service_unittest.cc',
         'browser/extensions/extensions_service_unittest.h',
         'browser/extensions/file_reader_unittest.cc',
+        'browser/extensions/image_loading_tracker_unittest.cc',
         'browser/extensions/sandboxed_extension_unpacker_unittest.cc',
         'browser/extensions/user_script_listener_unittest.cc',
         'browser/extensions/user_script_master_unittest.cc',
diff --git a/chrome/common/extensions/extension.cc b/chrome/common/extensions/extension.cc
index 98a2f84..7deff53 100644
--- a/chrome/common/extensions/extension.cc
+++ b/chrome/common/extensions/extension.cc
@@ -1474,6 +1474,28 @@ void Extension::SetBackgroundPageReady() {
       NotificationService::NoDetails());
 }
 
+void Extension::SetCachedImage(const ExtensionResource& source,
+                               const SkBitmap& image) {
+  DCHECK(source.extension_root() == path());  // The resource must come from
+                                              // this extension.
+  image_cache_[source.relative_path()] = image;
+
+}
+bool Extension::HasCachedImage(const ExtensionResource& source) {
+  DCHECK(source.extension_root() == path());  // The resource must come from
+                                              // this extension.
+  return image_cache_.find(source.relative_path()) != image_cache_.end();
+}
+
+SkBitmap Extension::GetCachedImage(const ExtensionResource& source) {
+  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;
+}
+
 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 0129858..f4c797e 100644
--- a/chrome/common/extensions/extension.h
+++ b/chrome/common/extensions/extension.h
@@ -21,6 +21,7 @@
 #include "chrome/common/extensions/url_pattern.h"
 #include "googleurl/src/gurl.h"
 #include "testing/gtest/include/gtest/gtest_prod.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 
 // Represents a Chrome extension.
 class Extension {
@@ -323,7 +324,16 @@ class Extension {
   bool being_upgraded() const { return being_upgraded_; }
   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.
+  void SetCachedImage(const ExtensionResource& source,
+                      const SkBitmap& image);
+  bool HasCachedImage(const ExtensionResource& source);
+  SkBitmap GetCachedImage(const ExtensionResource& source);
+
  private:
+  typedef std::map<FilePath, SkBitmap> ImageCache;
+
   // Helper method that loads a UserScript object from a
   // dictionary in the content_script list of the manifest.
   bool LoadUserScriptHelper(const DictionaryValue* content_script,
@@ -478,6 +488,9 @@ class Extension {
   // The type of container to launch into.
   LaunchContainer launch_container_;
 
+  // Cached images for this extension. This maps from the relative_path of the
+  // resource to the cached image.
+  ImageCache image_cache_;
 
   // Runtime data:
 
diff --git a/chrome/common/extensions/extension_extent.cc b/chrome/common/extensions/extension_extent.cc
index 310673e..3fbb994 100644
--- a/chrome/common/extensions/extension_extent.cc
+++ b/chrome/common/extensions/extension_extent.cc
@@ -6,7 +6,7 @@
 
 #include "base/string_util.h"
 
-bool ExtensionExtent::ContainsURL(const GURL& url) {
+bool ExtensionExtent::ContainsURL(const GURL& url) const {
   if (!url.is_valid())
     return false;
 
diff --git a/chrome/common/extensions/extension_extent.h b/chrome/common/extensions/extension_extent.h
index 76670ab..187303d 100644
--- a/chrome/common/extensions/extension_extent.h
+++ b/chrome/common/extensions/extension_extent.h
@@ -20,7 +20,7 @@ class ExtensionExtent {
   const GURL& origin() const { return origin_; }
   void set_origin(const GURL& val) { origin_ = val; }
 
-  bool ContainsURL(const GURL& url);
+  bool ContainsURL(const GURL& url) const;
 
  private:
   // The security origin (scheme+host+port) of the extent.
diff --git a/chrome/test/data/extensions/image_loading_tracker/16.png b/chrome/test/data/extensions/image_loading_tracker/16.png
new file mode 100644
index 0000000..d145964
--- /dev/null
+++ b/chrome/test/data/extensions/image_loading_tracker/16.png
diff --git a/chrome/test/data/extensions/image_loading_tracker/24.png b/chrome/test/data/extensions/image_loading_tracker/24.png
new file mode 100644
index 0000000..7148782
--- /dev/null
+++ b/chrome/test/data/extensions/image_loading_tracker/24.png
diff --git a/chrome/test/data/extensions/image_loading_tracker/app.json b/chrome/test/data/extensions/image_loading_tracker/app.json
new file mode 100644
index 0000000..9e25a1e
--- /dev/null
+++ b/chrome/test/data/extensions/image_loading_tracker/app.json
@@ -0,0 +1,6 @@
+{
+  "name": "test",
+  "version": "1",
+  "icons": { "16": "16.png",
+             "24": "24.png" }
+}