Use ImageLoader instead of ImageLoadingTracker (Part 5)

* Remove usage of tracker from ExtensionIconManager
* Ensure Profile instance is passed everywhere needed
* Update unit tests

BUG=163929


Review URL: https://chromiumcodereview.appspot.com/11877009

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

9 files changed, 52 insertions, 39 deletions

diff --git a/chrome/browser/extensions/api/omnibox/omnibox_api.cc b/chrome/browser/extensions/api/omnibox/omnibox_api.cc
index 806997f..748c539 100644
--- a/chrome/browser/extensions/api/omnibox/omnibox_api.cc
+++ b/chrome/browser/extensions/api/omnibox/omnibox_api.cc
@@ -129,7 +129,8 @@ void ExtensionOmniboxEventRouter::OnInputCancelled(
 }
 
 OmniboxAPI::OmniboxAPI(Profile* profile)
-    : url_service_(TemplateURLServiceFactory::GetForProfile(profile)) {
+    : profile_(profile),
+      url_service_(TemplateURLServiceFactory::GetForProfile(profile)) {
   ManifestHandler::Register(extension_manifest_keys::kOmnibox,
                             new OmniboxHandler);
   registrar_.Add(this, chrome::NOTIFICATION_EXTENSION_LOADED,
@@ -173,8 +174,8 @@ void OmniboxAPI::Observe(int type,
     const std::string& keyword = OmniboxInfo::GetKeyword(extension);
     if (!keyword.empty()) {
       // Load the omnibox icon so it will be ready to display in the URL bar.
-      omnibox_popup_icon_manager_.LoadIcon(extension);
-      omnibox_icon_manager_.LoadIcon(extension);
+      omnibox_popup_icon_manager_.LoadIcon(profile_, extension);
+      omnibox_icon_manager_.LoadIcon(profile_, extension);
 
       if (url_service_) {
         url_service_->Load();
diff --git a/chrome/browser/extensions/api/omnibox/omnibox_api.h b/chrome/browser/extensions/api/omnibox/omnibox_api.h
index ea4cde6..2685790 100644
--- a/chrome/browser/extensions/api/omnibox/omnibox_api.h
+++ b/chrome/browser/extensions/api/omnibox/omnibox_api.h
@@ -113,6 +113,8 @@ class OmniboxAPI : public ProfileKeyedAPI,
   }
   static const bool kServiceRedirectedInIncognito = true;
 
+  Profile* profile_;
+
   TemplateURLService* url_service_;
 
   // List of extensions waiting for the TemplateURLService to Load to
diff --git a/chrome/browser/extensions/extension_icon_manager.cc b/chrome/browser/extensions/extension_icon_manager.cc
index 63250dc..404b2e5 100644
--- a/chrome/browser/extensions/extension_icon_manager.cc
+++ b/chrome/browser/extensions/extension_icon_manager.cc
@@ -4,8 +4,10 @@
 
 #include "chrome/browser/extensions/extension_icon_manager.h"
 
+#include "base/bind.h"
 #include "base/logging.h"
 #include "base/stl_util.h"
+#include "chrome/browser/extensions/image_loader.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
 #include "chrome/common/extensions/extension_icon_set.h"
@@ -42,24 +44,28 @@ static SkBitmap ApplyPadding(const SkBitmap& source,
 }  // namespace
 
 ExtensionIconManager::ExtensionIconManager()
-    : ALLOW_THIS_IN_INITIALIZER_LIST(image_tracker_(this)),
-      monochrome_(false) {
+    : monochrome_(false),
+      ALLOW_THIS_IN_INITIALIZER_LIST(weak_ptr_factory_(this))  {
 }
 
 ExtensionIconManager::~ExtensionIconManager() {
 }
 
-void ExtensionIconManager::LoadIcon(const extensions::Extension* extension) {
+void ExtensionIconManager::LoadIcon(Profile* profile,
+                                    const extensions::Extension* extension) {
   ExtensionResource icon_resource = extension->GetIconResource(
       extension_misc::EXTENSION_ICON_BITTY, ExtensionIconSet::MATCH_BIGGER);
   if (!icon_resource.extension_root().empty()) {
     // Insert into pending_icons_ first because LoadImage can call us back
     // synchronously if the image is already cached.
     pending_icons_.insert(extension->id());
-    image_tracker_.LoadImage(extension,
-                             icon_resource,
-                             gfx::Size(gfx::kFaviconSize, gfx::kFaviconSize),
-                             ImageLoadingTracker::CACHE);
+    extensions::ImageLoader* loader = extensions::ImageLoader::Get(profile);
+    loader->LoadImageAsync(extension, icon_resource,
+                           gfx::Size(gfx::kFaviconSize, gfx::kFaviconSize),
+                           base::Bind(
+                               &ExtensionIconManager::OnImageLoaded,
+                               weak_ptr_factory_.GetWeakPtr(),
+                               extension->id()));
   }
 }
 
@@ -82,9 +88,8 @@ void ExtensionIconManager::RemoveIcon(const std::string& extension_id) {
   pending_icons_.erase(extension_id);
 }
 
-void ExtensionIconManager::OnImageLoaded(const gfx::Image& image,
-                                         const std::string& extension_id,
-                                         int index) {
+void ExtensionIconManager::OnImageLoaded(const std::string& extension_id,
+                                         const gfx::Image& image) {
   if (image.IsEmpty())
     return;
 
diff --git a/chrome/browser/extensions/extension_icon_manager.h b/chrome/browser/extensions/extension_icon_manager.h
index 614648e..59f48a2 100644
--- a/chrome/browser/extensions/extension_icon_manager.h
+++ b/chrome/browser/extensions/extension_icon_manager.h
@@ -10,21 +10,27 @@
 #include <string>
 
 #include "base/basictypes.h"
-#include "chrome/browser/extensions/image_loading_tracker.h"
+#include "base/memory/weak_ptr.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/insets.h"
 
+class Profile;
+
 namespace extensions {
 class Extension;
 }
 
-class ExtensionIconManager : public ImageLoadingTracker::Observer {
+namespace gfx {
+class Image;
+}
+
+class ExtensionIconManager {
  public:
   ExtensionIconManager();
   virtual ~ExtensionIconManager();
 
   // Start loading the icon for the given extension.
-  void LoadIcon(const extensions::Extension* extension);
+  void LoadIcon(Profile* profile, const extensions::Extension* extension);
 
   // 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
@@ -34,14 +40,13 @@ class ExtensionIconManager : public ImageLoadingTracker::Observer {
   // Removes the extension's icon from memory.
   void RemoveIcon(const std::string& extension_id);
 
-  // Implements the ImageLoadingTracker::Observer interface.
-  virtual void OnImageLoaded(const gfx::Image& image,
-                             const std::string& extension_id,
-                             int index) OVERRIDE;
-
   void set_monochrome(bool value) { monochrome_ = value; }
   void set_padding(const gfx::Insets& value) { padding_ = value; }
 
+ protected:
+  virtual void OnImageLoaded(const std::string& extension_id,
+                             const gfx::Image& image);
+
  private:
   // Makes sure we've done one-time initialization of the default extension icon
   // default_icon_.
@@ -51,9 +56,6 @@ class ExtensionIconManager : public ImageLoadingTracker::Observer {
   // coloring.
   SkBitmap ApplyTransforms(const SkBitmap& src);
 
-  // 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> icons_;
 
@@ -69,6 +71,8 @@ class ExtensionIconManager : public ImageLoadingTracker::Observer {
   // Specifies the amount of empty padding to place around the icon.
   gfx::Insets padding_;
 
+  base::WeakPtrFactory<ExtensionIconManager> weak_ptr_factory_;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionIconManager);
 };
 
diff --git a/chrome/browser/extensions/extension_icon_manager_unittest.cc b/chrome/browser/extensions/extension_icon_manager_unittest.cc
index dc280c6..fcefadd 100644
--- a/chrome/browser/extensions/extension_icon_manager_unittest.cc
+++ b/chrome/browser/extensions/extension_icon_manager_unittest.cc
@@ -10,6 +10,7 @@
 #include "chrome/common/chrome_paths.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_resource.h"
+#include "chrome/test/base/testing_profile.h"
 #include "content/public/test/test_browser_thread.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "ui/gfx/skia_util.h"
@@ -77,10 +78,9 @@ class TestIconManager : public ExtensionIconManager {
   // Implements the ImageLoadingTracker::Observer interface, and calls through
   // to the base class' implementation. Then it lets the test know that an
   // image load was observed.
-  virtual void OnImageLoaded(const gfx::Image& image,
-                             const std::string& extension_id,
-                             int index) OVERRIDE {
-    ExtensionIconManager::OnImageLoaded(image, extension_id, index);
+  virtual void OnImageLoaded(const std::string& extension_id,
+                             const gfx::Image& image) OVERRIDE {
+    ExtensionIconManager::OnImageLoaded(extension_id, image);
     test_->ImageLoadObserved();
   }
 
@@ -101,6 +101,7 @@ SkBitmap GetDefaultIcon() {
 
 // Tests loading an icon for an extension, removing it, then re-loading it.
 TEST_F(ExtensionIconManagerTest, LoadRemoveLoad) {
+  scoped_ptr<Profile> profile(new TestingProfile());
   SkBitmap default_icon = GetDefaultIcon();
 
   FilePath test_dir;
@@ -121,7 +122,7 @@ TEST_F(ExtensionIconManagerTest, LoadRemoveLoad) {
   TestIconManager icon_manager(this);
 
   // Load the icon and grab the bitmap.
-  icon_manager.LoadIcon(extension.get());
+  icon_manager.LoadIcon(profile.get(), extension.get());
   WaitForImageLoad();
   SkBitmap first_icon = icon_manager.GetIcon(extension->id());
   EXPECT_FALSE(gfx::BitmapsAreEqual(first_icon, default_icon));
@@ -131,7 +132,7 @@ TEST_F(ExtensionIconManagerTest, LoadRemoveLoad) {
 
   // Now re-load the icon - we should get the same result bitmap (and not the
   // default icon).
-  icon_manager.LoadIcon(extension.get());
+  icon_manager.LoadIcon(profile.get(), extension.get());
   WaitForImageLoad();
   SkBitmap second_icon = icon_manager.GetIcon(extension->id());
   EXPECT_FALSE(gfx::BitmapsAreEqual(second_icon, default_icon));
@@ -160,9 +161,10 @@ TEST_F(ExtensionIconManagerTest, LoadComponentExtensionResource) {
       Extension::NO_FLAGS, &error));
   ASSERT_TRUE(extension.get());
 
+  scoped_ptr<Profile> profile(new TestingProfile());
   TestIconManager icon_manager(this);
   // Load the icon and grab the bitmap.
-  icon_manager.LoadIcon(extension.get());
+  icon_manager.LoadIcon(profile.get(), extension.get());
   WaitForImageLoad();
   SkBitmap first_icon = icon_manager.GetIcon(extension->id());
   EXPECT_FALSE(gfx::BitmapsAreEqual(first_icon, default_icon));
@@ -172,7 +174,7 @@ TEST_F(ExtensionIconManagerTest, LoadComponentExtensionResource) {
 
   // Now re-load the icon - we should get the same result bitmap (and not the
   // default icon).
-  icon_manager.LoadIcon(extension.get());
+  icon_manager.LoadIcon(profile.get(), extension.get());
   WaitForImageLoad();
   SkBitmap second_icon = icon_manager.GetIcon(extension->id());
   EXPECT_FALSE(gfx::BitmapsAreEqual(second_icon, default_icon));
diff --git a/chrome/browser/extensions/extension_web_ui.cc b/chrome/browser/extensions/extension_web_ui.cc
index cdb516467..019e3db 100644
--- a/chrome/browser/extensions/extension_web_ui.cc
+++ b/chrome/browser/extensions/extension_web_ui.cc
@@ -35,6 +35,7 @@
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/codec/png_codec.h"
 #include "ui/gfx/favicon_size.h"
+#include "ui/gfx/image/image_skia.h"
 
 using content::WebContents;
 using extensions::Extension;
diff --git a/chrome/browser/extensions/image_loading_tracker.h b/chrome/browser/extensions/image_loading_tracker.h
index 43591d4..54e84f5 100644
--- a/chrome/browser/extensions/image_loading_tracker.h
+++ b/chrome/browser/extensions/image_loading_tracker.h
@@ -131,7 +131,6 @@ class ImageLoadingTracker : public content::NotificationObserver {
   // usage of this class, so once this list is empty this class can and
   // should be removed.
   friend class CreateChromeApplicationShortcutView;
-  friend class ExtensionIconManager;
   friend class ExtensionInfoBar;
   friend class ExtensionInfoBarGtk;
   friend class InfobarBridge;
diff --git a/chrome/browser/extensions/menu_manager.cc b/chrome/browser/extensions/menu_manager.cc
index 077c564..ed423e4 100644
--- a/chrome/browser/extensions/menu_manager.cc
+++ b/chrome/browser/extensions/menu_manager.cc
@@ -351,7 +351,7 @@ bool MenuManager::AddContextItem(
 
   // If this is the first item for this extension, start loading its icon.
   if (first_item)
-    icon_manager_.LoadIcon(extension);
+    icon_manager_.LoadIcon(profile_, extension);
 
   return true;
 }
diff --git a/chrome/browser/ui/webui/ntp/favicon_webui_handler.cc b/chrome/browser/ui/webui/ntp/favicon_webui_handler.cc
index f41fd50..92f66b9 100644
--- a/chrome/browser/ui/webui/ntp/favicon_webui_handler.cc
+++ b/chrome/browser/ui/webui/ntp/favicon_webui_handler.cc
@@ -53,10 +53,9 @@ class ExtensionIconColorManager : public ExtensionIconManager {
         handler_(handler) {}
   virtual ~ExtensionIconColorManager() {}
 
-  virtual void OnImageLoaded(const gfx::Image& image,
-                             const std::string& extension_id,
-                             int index) OVERRIDE {
-    ExtensionIconManager::OnImageLoaded(image, extension_id, index);
+  virtual void OnImageLoaded(const std::string& extension_id,
+                             const gfx::Image& image) OVERRIDE {
+    ExtensionIconManager::OnImageLoaded(extension_id, image);
     handler_->NotifyAppIconReady(extension_id);
   }
 
@@ -152,7 +151,7 @@ void FaviconWebUIHandler::HandleGetAppIconDominantColor(
       extension_id, false);
   if (!extension)
     return;
-  app_icon_color_manager_->LoadIcon(extension);
+  app_icon_color_manager_->LoadIcon(extension_service->profile(), extension);
 }
 
 void FaviconWebUIHandler::NotifyAppIconReady(const std::string& extension_id) {