chromeos: Fix pixelated icons in app list and launcher (part 2)

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 ea84c34fe5dc55d490b8af4ea2a5971c38ecc3e1..a64d692a847be4a917523d60dffdd22e7b426417 100644
--- a/chrome/browser/extensions/image_loading_tracker.cc
+++ b/chrome/browser/extensions/image_loading_tracker.cc
@@ -13,6 +13,7 @@
 #include "chrome/common/chrome_notification_types.h"
 #include "chrome/common/extensions/extension.h"
 #include "chrome/common/extensions/extension_constants.h"
+#include "chrome/common/extensions/extension_icon_set.h"
 #include "chrome/common/extensions/extension_resource.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/notification_service.h"
@@ -21,8 +22,8 @@
 #include "skia/ext/image_operations.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "ui/gfx/image/image.h"
-#include "ui/gfx/image/image_skia.h"
 #include "ui/gfx/image/image_skia_rep.h"
+#include "ui/gfx/screen.h"
 #include "webkit/glue/image_decoder.h"
 
 using content::BrowserThread;
@@ -37,8 +38,12 @@ ImageLoadingTracker::Observer::~Observer() {}
 // ImageLoadingTracker::ImageInfo
 
 ImageLoadingTracker::ImageInfo::ImageInfo(
-    const ExtensionResource& resource, gfx::Size max_size)
-    : resource(resource), max_size(max_size) {
+    const ExtensionResource& resource,
+    const gfx::Size& max_size,
+    ui::ScaleFactor scale_factor)
+    : resource(resource),
+      max_size(max_size),
+      scale_factor(scale_factor) {
 }
 
 ImageLoadingTracker::ImageInfo::~ImageInfo() {
@@ -76,26 +81,21 @@ class ImageLoadingTracker::ImageLoader
   }
 
   // Instructs the loader to load a task on the File thread.
-  void LoadImage(const ExtensionResource& resource,
-                 const gfx::Size& max_size,
-                 int id) {
+  void LoadImage(const ImageInfo& image_info, int id) {
     DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::FILE));
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
-        base::Bind(&ImageLoader::LoadOnFileThread, this, resource,
-                   max_size, id));
+        base::Bind(&ImageLoader::LoadOnFileThread, this, image_info, id));
   }
 
-  void LoadOnFileThread(const ExtensionResource& resource,
-                        const gfx::Size& max_size,
-                        int id) {
+  void LoadOnFileThread(const ImageInfo& image_info, int id) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
     // Read the file from disk.
     std::string file_contents;
-    FilePath path = resource.GetFilePath();
+    FilePath path = image_info.resource.GetFilePath();
     if (path.empty() || !file_util::ReadFileToString(path, &file_contents)) {
-      ReportBack(NULL, resource, gfx::Size(), id);
+      ReportBack(NULL, image_info, gfx::Size(), id);
       return;
     }
 
@@ -112,61 +112,57 @@ class ImageLoadingTracker::ImageLoader
     // Chrome is therefore decoding images here that were generated by Chrome.
     *decoded = decoder.Decode(data, file_contents.length());
     if (decoded->empty()) {
-      ReportBack(NULL, resource, gfx::Size(), id);
+      ReportBack(NULL, image_info, gfx::Size(), id);
       return;  // Unable to decode.
     }
 
     gfx::Size original_size(decoded->width(), decoded->height());
 
-    if (decoded->width() > max_size.width() ||
-        decoded->height() > max_size.height()) {
+    if (decoded->width() > image_info.max_size.width() ||
+        decoded->height() > image_info.max_size.height()) {
       // The bitmap is too big, re-sample.
       *decoded = skia::ImageOperations::Resize(
           *decoded, skia::ImageOperations::RESIZE_LANCZOS3,
-          max_size.width(), max_size.height());
+          image_info.max_size.width(), image_info.max_size.height());
     }
 
-    ReportBack(decoded.release(), resource, original_size, id);
+    ReportBack(decoded.release(), image_info, original_size, id);
   }
 
   // Instructs the loader to load a resource on the File thread.
-  void LoadResource(const ExtensionResource& resource,
-                    const gfx::Size& max_size,
-                    int id,
-                    int resource_id) {
+  void LoadResource(const ImageInfo& image_info, int id, int resource_id) {
     DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::FILE));
     BrowserThread::PostTask(
         BrowserThread::FILE, FROM_HERE,
-        base::Bind(&ImageLoader::LoadResourceOnFileThread, this, resource,
-                   max_size, id, resource_id));
+        base::Bind(&ImageLoader::LoadResourceOnFileThread, this, image_info,
+                   id, resource_id));
   }
 
-  void LoadResourceOnFileThread(const ExtensionResource& resource,
-                                const gfx::Size& max_size,
+  void LoadResourceOnFileThread(const ImageInfo& image_info,
                                 int id,
                                 int resource_id) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
     SkBitmap* image = ExtensionIconSource::LoadImageByResourceId(
         resource_id);
-    ReportBack(image, resource, max_size, id);
+    ReportBack(image, image_info, image_info.max_size, id);
   }
 
-  void ReportBack(SkBitmap* image, const ExtensionResource& resource,
+  void ReportBack(SkBitmap* image, const ImageInfo& image_info,
                   const gfx::Size& original_size, int id) {
     DCHECK(BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
     BrowserThread::PostTask(
         callback_thread_id_, FROM_HERE,
         base::Bind(&ImageLoader::ReportOnUIThread, this,
-                   image, resource, original_size, id));
+                   image, image_info, original_size, id));
   }
 
-  void ReportOnUIThread(SkBitmap* image, const ExtensionResource& resource,
+  void ReportOnUIThread(SkBitmap* image, const ImageInfo& image_info,
                         const gfx::Size& original_size, int id) {
     DCHECK(!BrowserThread::CurrentlyOn(BrowserThread::FILE));
 
     if (tracker_)
-      tracker_->OnImageLoaded(image, resource, original_size, id, true);
+      tracker_->OnImageLoaded(image, image_info, original_size, id, true);
 
     delete image;
   }
@@ -207,7 +203,34 @@ void ImageLoadingTracker::LoadImage(const Extension* extension,
                                     const gfx::Size& max_size,
                                     CacheParam cache) {
   std::vector<ImageInfo> info_list;
-  info_list.push_back(ImageInfo(resource, max_size));
+  info_list.push_back(ImageInfo(resource, max_size, ui::SCALE_FACTOR_NONE));
+  LoadImages(extension, info_list, cache);
+}
+
+void ImageLoadingTracker::LoadDIPImage(
+    const extensions::Extension* extension,
+    int preferred_dip_size,
+    gfx::Size max_dip_size,
+    CacheParam cache) {
+  if (!gfx::Screen::IsDIPEnabled()) {
+    LoadImage(extension,
+              extension->GetIconResource(preferred_dip_size,
+                                         ExtensionIconSet::MATCH_BIGGER),
+              max_dip_size,
+              cache);
+    return;
+  }

[oshima, 2012/07/11 21:09:33]

instead of having different code path, won't this work?

const ui::ScaleFactor scale_factor =
 gfx::Screen::IsDIPEnabled() ? ui::SCALE_FACTOR_200P : ui::SCALE_FACTOR_100P ?

[xiyuan, 2012/07/13 18:35:46]

Done.

+
+  // Always use a hi-res resource when DIP is enabled so that it is safe to
+  // complete hand off the image to the caller.
+  const ui::ScaleFactor scale_factor = ui::SCALE_FACTOR_200P;
+  const float scale = ui::GetScaleFactorScale(scale_factor);
+  std::vector<ImageInfo> info_list;
+  info_list.push_back(ImageInfo(
+      extension->GetIconResource(static_cast<int>(preferred_dip_size * scale),
+                                 ExtensionIconSet::MATCH_BIGGER),
+      max_dip_size.Scale(scale),
+      scale_factor));
   LoadImages(extension, info_list, cache);
 }
 
@@ -228,22 +251,19 @@ void ImageLoadingTracker::LoadImages(const Extension* extension,
     if (load_info.extension_id == extension_misc::kWebStoreAppId) {
       if (!loader_)
         loader_ = new ImageLoader(this);
-      loader_->LoadResource(it->resource, it->max_size, id, IDR_WEBSTORE_ICON);
+      loader_->LoadResource(*it, id, IDR_WEBSTORE_ICON);
       continue;
     } else if (load_info.extension_id == extension_misc::kChromeAppId) {
       if (!loader_)
         loader_ = new ImageLoader(this);
-      loader_->LoadResource(it->resource,
-                            it->max_size,
-                            id,
-                            IDR_PRODUCT_LOGO_128);
+      loader_->LoadResource(*it, id, IDR_PRODUCT_LOGO_128);
       continue;
     }
 
     // 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, false);
+      OnImageLoaded(NULL, *it, it->max_size, id, false);
       continue;
     }
 
@@ -252,7 +272,7 @@ void ImageLoadingTracker::LoadImages(const Extension* 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, false);
+      OnImageLoaded(&image, *it, it->max_size, id, false);
       continue;
     }
 
@@ -263,9 +283,9 @@ void ImageLoadingTracker::LoadImages(const Extension* extension,
 
     int resource_id;
     if (IsComponentExtensionResource(extension, it->resource, resource_id))
-      loader_->LoadResource(it->resource, it->max_size, id, resource_id);
+      loader_->LoadResource(*it, id, resource_id);
     else
-      loader_->LoadImage(it->resource, it->max_size, id);
+      loader_->LoadImage(*it, id);
   }
 }
 
@@ -295,7 +315,7 @@ bool ImageLoadingTracker::IsComponentExtensionResource(
 
 void ImageLoadingTracker::OnImageLoaded(
     SkBitmap* image,
-    const ExtensionResource& resource,
+    const ImageInfo& image_info,
     const gfx::Size& original_size,
     int id,
     bool should_cache) {
@@ -306,33 +326,25 @@ void ImageLoadingTracker::OnImageLoaded(
   // Save the pending results.
   DCHECK(info->pending_count > 0);
   info->pending_count--;
-  if (image)
-    info->bitmaps.push_back(*image);
+  if (image) {
+    info->images.AddRepresentation(gfx::ImageSkiaRep(*image,
+        image_info.scale_factor));
+  }
 
   // Add to the extension's image cache if requested.
   DCHECK(info->cache != CACHE || info->extension);
   if (should_cache && info->cache == CACHE  &&
-      !info->extension->HasCachedImage(resource, original_size)) {
-    info->extension->SetCachedImage(resource, image ? *image : SkBitmap(),
+      !info->extension->HasCachedImage(image_info.resource, original_size)) {
+    info->extension->SetCachedImage(image_info.resource,
+                                    image ? *image : SkBitmap(),
                                     original_size);
   }
 
   // If all pending images are done then report back.
   if (info->pending_count == 0) {
-    gfx::Image image;
+    gfx::Image image(info->images);
     std::string extension_id = info->extension_id;
 
-    if (info->bitmaps.size() > 0) {
-      gfx::ImageSkia image_skia;
-      for (std::vector<SkBitmap>::const_iterator it = info->bitmaps.begin();
-           it != info->bitmaps.end(); ++it) {
-        // TODO(pkotwicz): Do something better but ONLY when DIP is enabled.
-        image_skia.AddRepresentation(
-            gfx::ImageSkiaRep(*it, ui::SCALE_FACTOR_100P));
-      }
-      image = gfx::Image(image_skia);
-    }
-
     load_map_.erase(load_map_it);
 
     // ImageLoadingTracker might be deleted after the callback so don't