Add a class to replace ImageLoadingTracker with a nicer API.

chrome/browser/extensions/image_loader.h

Index: chrome/browser/extensions/image_loader.h
diff --git a/chrome/browser/extensions/image_loading_tracker.h b/chrome/browser/extensions/image_loader.h
similarity index 38%
copy from chrome/browser/extensions/image_loading_tracker.h
copy to chrome/browser/extensions/image_loader.h
index 5eefcd1840f2a66637a06a13204562ae1f476b04..273682122ba76ea037857929743e9aa06f735e42 100644
--- a/chrome/browser/extensions/image_loading_tracker.h
+++ b/chrome/browser/extensions/image_loader.h
@@ -2,71 +2,47 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef CHROME_BROWSER_EXTENSIONS_IMAGE_LOADING_TRACKER_H_
-#define CHROME_BROWSER_EXTENSIONS_IMAGE_LOADING_TRACKER_H_
+#ifndef CHROME_BROWSER_EXTENSIONS_IMAGE_LOADER_H_
+#define CHROME_BROWSER_EXTENSIONS_IMAGE_LOADER_H_
 
 #include <map>
-#include <string>
-#include <vector>
+#include <set>
 
-#include "base/compiler_specific.h"
+#include "base/callback_forward.h"
 #include "base/gtest_prod_util.h"
-#include "base/memory/ref_counted.h"
-#include "chrome/common/extensions/extension_icon_set.h"
 #include "chrome/common/extensions/extension_resource.h"
 #include "content/public/browser/notification_observer.h"
 #include "content/public/browser/notification_registrar.h"
+#include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/base/layout.h"
-#include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/size.h"
 
-class SkBitmap;
+template <typename T> struct DefaultSingletonTraits;
 
-namespace extensions {
-class Extension;
-}
+FORWARD_DECLARE_TEST(ImageLoaderTest, Cache);
+FORWARD_DECLARE_TEST(ImageLoaderTest, DeleteExtensionWhileWaitingForCache);
+FORWARD_DECLARE_TEST(ImageLoaderTest, MultipleImages);
 
 namespace gfx {
 class Image;
 }
 
-// The views need to load their icons asynchronously but might be deleted before
-// the images have loaded. This class encapsulates a loader class that stays
-// alive while the request is in progress (manages its own lifetime) and keeps
-// track of whether the view still cares about the icon loading.
+namespace extensions {
+
+class Extension;
+
+// This class is responsible for asynchronously loading extension images and
+// calling a callback when an image is loaded.
+// The views need to load their icons asynchronously might be deleted before
+// the images have loaded. If you pass your callback using a weak_ptr, this
+// will make sure the callback won't be called after the view is deleted.
 //
-// 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(extension, resource, max_size, false);
-// ... and wait for OnImageLoaded to be called back on you with a pointer to the
-// ImageSkia 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.
+// NOTE: if the image is available already, the callback is called immediately
+// from the call to LoadImageAsync. In other words, by the time LoadImageAsync
+// returns the callback has been notified.

[Aaron Boodman, 2012/10/17 17:15:13]

I think "reentrant" is the right word here?

 //
-class ImageLoadingTracker : public content::NotificationObserver {
+class ImageLoader : public content::NotificationObserver {
  public:
-  enum CacheParam {
-    CACHE,
-    DONT_CACHE
-  };
-
-  class Observer {
-   public:
-    // Will be called when the image with the given index has loaded.
-    // |image| can be empty if a valid image was not found or it failed to
-    // decode. |extension_id| is the ID of the extension the images are loaded
-    // from.  |index| represents the index of the image just loaded (starts at 0
-    // and increments every time LoadImage is called).
-    virtual void OnImageLoaded(const gfx::Image& image,
-                               const std::string& extension_id,
-                               int index) = 0;
-
-   protected:
-    virtual ~Observer();
-  };
-
   // Information about a singe image representation to load from an extension
   // resource.
   struct ImageRepresentation {
@@ -78,7 +54,7 @@ class ImageLoadingTracker : public content::NotificationObserver {
     };
 
     ImageRepresentation(const ExtensionResource& resource,
-                        ResizeCondition resize_method,
+                        ResizeCondition resize_condition,
                         const gfx::Size& desired_size,
                         ui::ScaleFactor scale_factor);
     ~ImageRepresentation();
@@ -86,7 +62,7 @@ class ImageLoadingTracker : public content::NotificationObserver {
     // Extension resource to load.
     ExtensionResource resource;
 
-    ResizeCondition resize_method;
+    ResizeCondition resize_condition;
 
     // When |resize_method| is ALWAYS_RESIZE or when the loaded image is larger
     // than |desired_size| it will be resized to these dimensions.
@@ -96,91 +72,116 @@ class ImageLoadingTracker : public content::NotificationObserver {
     ui::ScaleFactor scale_factor;
   };
 
-  explicit ImageLoadingTracker(Observer* observer);
-  virtual ~ImageLoadingTracker();
+  static ImageLoader* GetInstance();
+
+  // Checks whether image is a component extension resource. Returns false
+  // if a given |resource| does not have a corresponding image in bundled
+  // resources. Otherwise fills |resource_id|.
+  static bool IsComponentExtensionResource(
+      const extensions::Extension* extension,
+      const FilePath& resource_path,
+      int* resource_id);
 
   // Specify image resource to load. If the loaded image is larger than
   // |max_size| it will be resized to those dimensions. IMPORTANT NOTE: this
-  // function may call back your observer synchronously (ie before it returns)
+  // function may call back your callback synchronously (ie before it returns)
   // if the image was found in the cache.
   // Note this method loads a raw bitmap from the resource. All sizes given are
   // assumed to be in pixels.
-  void LoadImage(const extensions::Extension* extension,
-                 const ExtensionResource& resource,
-                 const gfx::Size& max_size,
-                 CacheParam cache);
+  void LoadImageAsync(const extensions::Extension* extension,
+                      const ExtensionResource& resource,
+                      const gfx::Size& max_size,
+                      const base::Callback<void(const gfx::Image&)>& callback);
 
   // Same as LoadImage() above except it loads multiple images from the same
   // extension. This is used to load multiple resolutions of the same image
   // type.
-  void LoadImages(const extensions::Extension* extension,
-                  const std::vector<ImageRepresentation>& info_list,
-                  CacheParam cache);
-
-  // Returns the ID used for the next image that is loaded. That is, the return
-  // value from this method corresponds to the int that is passed to
-  // OnImageLoaded() the next time LoadImage() is invoked.
-  int next_id() const { return next_id_; }
+  void LoadImagesAsync(const extensions::Extension* extension,
+                       const std::vector<ImageRepresentation>& info_list,
+                       const base::Callback<void(const gfx::Image&)>& callback);
 
-  // Checks whether image is a component extension resource. Returns false
-  // if a given |resource| does not have a corresponding image in bundled
-  // resources. Otherwise fills |resource_id|.
-  static bool IsComponentExtensionResource(
-      const extensions::Extension* extension,
-      const FilePath& resource_path,
-      int* resource_id);
+  void SetCachedImage(const ExtensionResource& source,
+                      const SkBitmap& image,
+                      const gfx::Size& original_size) const;
 
  private:
-  // Information for pending resource load operation for one or more image
-  // representations.
+  friend struct DefaultSingletonTraits<ImageLoader>;
+
+  // 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;
+
   struct PendingLoadInfo {
     PendingLoadInfo();
     ~PendingLoadInfo();
 
-    const extensions::Extension* extension;
-    // This is cached separate from |extension| in case the extension is
-    // unloaded.
     std::string extension_id;
-    CacheParam cache;
-    size_t pending_count;
-    gfx::ImageSkia image_skia;
+    bool save_to_cache;
   };
 
-  // Maps an integer identifying a load request to a PendingLoadInfo.
-  typedef std::map<int, PendingLoadInfo> LoadMap;
+  typedef std::set<PendingLoadInfo*> LoadSet;
+
+  struct LoadResult {
+    LoadResult(const SkBitmap& bitmap,
+               const gfx::Size& original_size,
+               const ImageRepresentation& image_representation);
+    ~LoadResult();
+
+    SkBitmap bitmap;
+    gfx::Size original_size;
+    ImageRepresentation image_representation;
+  };
 
-  class ImageLoader;
+  ImageLoader();
+  virtual ~ImageLoader();
 
-  // Called on the calling thread when the bitmap finishes loading.
-  // |bitmap| may be null if the image file failed to decode.
-  void OnBitmapLoaded(const SkBitmap* bitmap,
-                      const ImageRepresentation& image_info,
-                      const gfx::Size& original_size,
-                      int id,
-                      bool should_cache);
+  void LoadImagesOnBlockingPool(
+      const std::vector<ImageRepresentation>& info_list,
+      const std::vector<SkBitmap>& bitmaps,
+      PendingLoadInfo* load_info,
+      const base::Callback<void(const gfx::Image&)>& callback);
+
+  void ReplyBack(
+      const std::vector<LoadResult>& load_result,
+      PendingLoadInfo* load_info,
+      const base::Callback<void(const gfx::Image&)>& callback);
 
   // content::NotificationObserver method. If an extension is uninstalled while
-  // we're waiting for the image we remove the entry from load_map_.
+  // we're waiting for the image we make sure it won't be cached.
   virtual void Observe(int type,
                        const content::NotificationSource& source,
                        const content::NotificationDetails& details) OVERRIDE;
 
-  // The view that is waiting for the image to load.
-  Observer* observer_;
+  void SetCachedImageImpl(const std::string& extension_id,
+                          const ImageCacheKey& key,
+                          const SkBitmap& image);
+
+  const SkBitmap* GetCachedImage(const ExtensionResource& source,
+                                 const gfx::Size& max_size) const;
 
-  // ID to use for next image requested. This is an ever increasing integer.
-  int next_id_;
+  bool ShouldCacheImage(const ImageRepresentation& image_info) const;
+  void SetImageSizesToCache(const std::set<int> image_sizes_);
 
-  // The object responsible for loading the image on the File thread.
-  scoped_refptr<ImageLoader> loader_;
+  // Cached images for extensions. This should only be touched on the UI thread.
+  std::map<std::string, ImageCache> image_cache_;
 
-  // Information for each LoadImage request is cached here. The integer
-  // identifies the id assigned to the request.
-  LoadMap load_map_;
+  // Set of all pending image loads that should be cached unless the extension
+  // they are loaded from gets unloaded before the load finishes.
+  LoadSet pending_;
+
+  // Sizes of images to cache
+  std::set<int> image_sizes_to_cache_;
 
   content::NotificationRegistrar registrar_;
 
-  DISALLOW_COPY_AND_ASSIGN(ImageLoadingTracker);
+  FRIEND_TEST_ALL_PREFIXES(::ImageLoaderTest, Cache);
+  FRIEND_TEST_ALL_PREFIXES(::ImageLoaderTest,
+                           DeleteExtensionWhileWaitingForCache);
+  FRIEND_TEST_ALL_PREFIXES(::ImageLoaderTest, MultipleImages);
 };
 
-#endif  // CHROME_BROWSER_EXTENSIONS_IMAGE_LOADING_TRACKER_H_
+}  // namespace extensions
+
+#endif  // CHROME_BROWSER_EXTENSIONS_IMAGE_LOADER_H_