Add support for multiple icon sizes for Mac platform apps

chrome/browser/extensions/image_loading_tracker.cc

Index: chrome/browser/extensions/image_loading_tracker.cc
diff --git a/chrome/browser/extensions/image_loading_tracker.cc b/chrome/browser/extensions/image_loading_tracker.cc
index ca60dba5e6af4762d7c3f010a5b256e2c9bf6a69..f8a20b608aacfe75c276c0373f400601dbba1608 100644
--- a/chrome/browser/extensions/image_loading_tracker.cc
+++ b/chrome/browser/extensions/image_loading_tracker.cc
@@ -13,12 +13,26 @@
 #include "content/public/browser/notification_service.h"
 #include "skia/ext/image_operations.h"
 #include "third_party/skia/include/core/SkBitmap.h"
+#include "ui/gfx/image/image.h"
 #include "webkit/glue/image_decoder.h"
 
 using content::BrowserThread;
 
 ImageLoadingTracker::Observer::~Observer() {}
 
+ImageLoadingTracker::ImageInfo::ImageInfo(
+    const ExtensionResource resource, gfx::Size max_size)
+    : resource(resource), max_size(max_size) {
+}
+
+ImageLoadingTracker::PendingLoadInfo::PendingLoadInfo()
+  : extension(NULL),
+    pending_count(0) {
+}
+
+ImageLoadingTracker::PendingLoadInfo::~PendingLoadInfo() {
+}
+
 ////////////////////////////////////////////////////////////////////////////////
 // ImageLoadingTracker::ImageLoader
 
@@ -139,31 +153,46 @@ void ImageLoadingTracker::LoadImage(const Extension* extension,
                                     const ExtensionResource& resource,
                                     const gfx::Size& max_size,
                                     CacheParam cache) {
-  // If we don't have a path we don't need to do any further work, just respond
-  // back.
+  std::vector<ImageInfo> info_list;
+  info_list.push_back(ImageInfo(resource, max_size));
+  LoadImages(extension, info_list, cache);
+}
+
+void ImageLoadingTracker::LoadImages(const Extension* extension,
+                                     const std::vector<ImageInfo>& info_list,
+                                     CacheParam cache) {
+  PendingLoadInfo load_info;
+  load_info.extension = extension;
+  load_info.cache = cache;
+  load_info.extension_id = extension->id();
+  load_info.pending_count = info_list.size();
   int id = next_id_++;
-  if (resource.relative_path().empty()) {
-    OnImageLoaded(NULL, resource, max_size, id);
-    return;
-  }
+  load_map_[id] = load_info;
+
+  for (std::vector<ImageInfo>::const_iterator it = info_list.begin();
+       it != info_list.end(); ++it) {
+    // If we don't have a path we don't need to do any further work, just
+    // respond back.
+    if (it->resource.relative_path().empty()) {
+      OnImageLoaded(NULL, it->resource, it->max_size, id);
+      continue;
+    }
 
-  DCHECK(extension->path() == resource.extension_root());
+    DCHECK(extension->path() == it->resource.extension_root());
 
-  // See if the extension has the image already.
-  if (extension->HasCachedImage(resource, max_size)) {
-    SkBitmap image = extension->GetCachedImage(resource, max_size);
-    OnImageLoaded(&image, resource, max_size, id);
-    return;
-  }
-
-  if (cache == CACHE)
-    load_map_[id] = extension;
+    // See if the extension has the image already.
+    if (extension->HasCachedImage(it->resource, it->max_size)) {
+      SkBitmap image = extension->GetCachedImage(it->resource, it->max_size);
+      OnImageLoaded(&image, it->resource, it->max_size, id);
+      continue;
+    }
 
-  // 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, id);
+    // Instruct the ImageLoader to load this on the File thread. LoadImage does
+    // not block.
+    if (!loader_)
+      loader_ = new ImageLoader(this);
+    loader_->LoadImage(it->resource, it->max_size, id);
+  }
 }
 
 void ImageLoadingTracker::OnImageLoaded(
@@ -171,14 +200,40 @@ void ImageLoadingTracker::OnImageLoaded(
     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(),
-                              original_size);
-    load_map_.erase(i);
+  LoadMap::iterator it = load_map_.find(id);
+  if (it == load_map_.end())
+    return;
+
+  PendingLoadInfo* info = &it->second;
+
+  // Save the pending results.
+  DCHECK(info->pending_count > 0);
+  info->pending_count--;
+  if (image)
+    info->bitmaps.push_back(*image);
+
+  // Add to the extension's image cache if requested.
+  if (info->cache == CACHE && info->extension &&
+      !info->extension->HasCachedImage(resource, original_size)) {
+    info->extension->SetCachedImage(resource, image ? *image : SkBitmap(),
+                                    original_size);
   }
 
-  observer_->OnImageLoaded(image, resource, id);
+  // If all pending images are done report back.
+  if (info->pending_count == 0) {
+    if (info->bitmaps.size() > 0) {
+      std::vector<const SkBitmap*> bitmaps;
+      for (std::vector<SkBitmap>::const_iterator it = info->bitmaps.begin();
+           it != info->bitmaps.end(); ++it) {
+        bitmaps.push_back(new SkBitmap(*it));

[Mihai Parparita -not on Chrome, 2012/02/23 00:19:41]

Why make a new SkBitmap when you could just use a pointer to the instance you
already have via the iterator?

[sail, 2012/02/23 02:54:27]

The gfx::Image constructor takes ownership of the bitmaps. Added comment.

+      }
+      gfx::Image gfx_image(bitmaps);
+      observer_->OnImageLoaded(&gfx_image, info->extension_id, id);
+    } else {
+      observer_->OnImageLoaded(NULL, info->extension_id, id);
+    }
+    load_map_.erase(it);
+  }
 }
 
 void ImageLoadingTracker::Observe(int type,
@@ -189,12 +244,13 @@ void ImageLoadingTracker::Observe(int type,
   const Extension* extension =
       content::Details<UnloadedExtensionInfo>(details)->extension;
 
-  // 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();) {
-    if (i->second == extension)
-      load_map_.erase(i++);
-    else
-      ++i;
+  // Remove reference to this extension from all pending load entries. This

[Mihai Parparita -not on Chrome, 2012/02/23 00:19:41]

Why not remove the map entry? The check at line 204 will fail, and any loads
that complete will be ignored.

[sail, 2012/02/23 02:54:27]

This doesn't match the old behavior. In the old code any loads that complete
still fire the OnImageLoaded() callback.

+  // 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) {
+    PendingLoadInfo* info = &i->second;
+    if (info->extension == extension) {
+      info->extension = NULL;
+      info->cache = DONT_CACHE;
+    }
   }
 }