Eliminate all UI thread decoding of extension images....

chrome/browser/extensions/image_loading_tracker.h

Index: chrome/browser/extensions/image_loading_tracker.h
===================================================================
--- chrome/browser/extensions/image_loading_tracker.h	(revision 42362)
+++ chrome/browser/extensions/image_loading_tracker.h	(working copy)
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// 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.
 
@@ -15,68 +15,45 @@
 }
 
 // The views need to load their icons asynchronously but might be deleted before
-// the images have loaded. This class 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.
-// Consider abstracting out a FilePathProvider (ExtensionResource) and moving
-// back to chrome/browser/ if other subsystems want to use it.
-class ImageLoadingTracker
-  : public base::RefCountedThreadSafe<ImageLoadingTracker> {
+// 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.

[Aaron Boodman, 2010/03/23 19:10:55]

It would be good to have an example usage here.

+class ImageLoadingTracker {
  public:
   class Observer {
    public:
     // 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.

[Aaron Boodman, 2010/03/23 19:10:55]

You should make it clear that if the image could not be loaded for some reason
it might be NULL.

-    virtual void OnImageLoaded(SkBitmap* image, size_t index) = 0;
+    virtual void OnImageLoaded(SkBitmap* image, int index) = 0;
   };
 
-  ImageLoadingTracker(Observer* observer, size_t image_count)
-    : observer_(observer), image_count_(image_count), posted_count_(0) {
-    AddRef();  // We hold on to a reference to ourself to make sure we don't
-               // get deleted until we get a response from image loading (see
-               // ImageLoadingDone).
-  }
+  explicit ImageLoadingTracker(Observer* observer);
+  ~ImageLoadingTracker();
 
-  // If there are remaining images to be loaded, the observing object should
-  // call this method on its destruction, so that the tracker will not attempt
-  // to make any more callbacks to it.
-  void StopTrackingImageLoad() {
-    observer_ = NULL;
-  }
-
-  // Specify image resource to load.  This method must be called a number of
-  // times equal to the |image_count| arugment to the constructor.  Calling it
-  // any more or less than that is an error. If the loaded image is larger than
-  // |max_size| it will be resized to those dimensions.
+  // Specify image resource to load. If the loaded image is larger than
+  // |max_size| it will be resized to those dimensions. |index| will be returned
+  // back to the caller through OnImageLoaded.
   void PostLoadImageTask(const ExtensionResource& resource,
-                         const gfx::Size& max_size);
+                         const gfx::Size& max_size,
+                         int index);

[Aaron Boodman, 2010/03/23 19:10:55]

Recommend removing this param and changing OnImageLoaded(SkBitmap*, int index)
to OnImageLoaded(ExtensionResource, SkBitmap*, index).

ImageTrackingLoader can keep a counter it increments to provide index, which
allows the existing callers that rely on index to keep working.

But I think that ExtensionResource is a more useful thing for the client to get
in the callback in general.

 
  private:
-  class LoadImageTask;
+  class ImageLoader;
 
-  friend class base::RefCountedThreadSafe<ImageLoadingTracker>;
-
-  ~ImageLoadingTracker() {}
-
-  // When an image has finished loaded and scaled 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.
+  // 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.
   // |image| may be null if the file failed to decode.
-  void OnImageLoaded(SkBitmap* image, size_t index);
+  void OnImageLoaded(SkBitmap* image, int index);
 
   // The view that is waiting for the image to load.
   Observer* observer_;
 
-  // The number of images this ImageTracker should keep track of.  This is
-  // decremented as each image finishes loading, and the tracker will delete
-  // itself when it reaches zero.
-  size_t image_count_;
+  // The object responsible for loading the image on the File thread.
+  scoped_refptr<ImageLoader> loader_;
 
-  // The number of tasks that have been posted so far.
-  size_t posted_count_;
-
   DISALLOW_COPY_AND_ASSIGN(ImageLoadingTracker);
 };