Only load theme images for the scale factors that use them

chrome/browser/themes/browser_theme_pack.cc

Index: chrome/browser/themes/browser_theme_pack.cc
diff --git a/chrome/browser/themes/browser_theme_pack.cc b/chrome/browser/themes/browser_theme_pack.cc
index 3a8fb68df6d7987446647d55572ff96063c37d1f..2d8a8e1ec58e67ecbd1cba429cb21dbeace3f0e4 100644
--- a/chrome/browser/themes/browser_theme_pack.cc
+++ b/chrome/browser/themes/browser_theme_pack.cc
@@ -444,6 +444,27 @@ gfx::Image CreateHSLShiftedImage(const gfx::Image& image,
       *src_image, hsl_shift));
 }
 

[pkotwicz, 2013/06/19 16:20:32]

Nit: Add a comment for this method

[sschmitz, 2013/06/19 21:56:52]

Done.

+SkBitmap CreateLowQualityResizedBitmap(const SkBitmap& available_bitmap,

[pkotwicz, 2013/06/19 16:20:32]

Nit: Name this |source_bitmap|

[sschmitz, 2013/06/19 21:56:52]

Done.

+                                       ui::ScaleFactor available_scale_factor,
+                                       ui::ScaleFactor desired_scale_factor) {
+  gfx::Size scaled_size = gfx::ToCeiledSize(
+      gfx::ScaleSize(gfx::Size(available_bitmap.width(),
+                               available_bitmap.height()),
+                     ui::GetScaleFactorScale(desired_scale_factor) /
+                     ui::GetScaleFactorScale(available_scale_factor)));
+  SkBitmap scaled_bitmap;
+  scaled_bitmap.setConfig(SkBitmap::kARGB_8888_Config,
+                          scaled_size.width(),
+                          scaled_size.height());
+  if (!scaled_bitmap.allocPixels())
+    SK_CRASH();
+  scaled_bitmap.eraseARGB(0, 0, 0, 0);
+  SkCanvas canvas(scaled_bitmap);
+  SkRect scaled_bounds = RectToSkRect(gfx::Rect(scaled_size));
+  canvas.drawBitmapRect(available_bitmap, NULL, scaled_bounds);
+  return scaled_bitmap;
+}
+
 // A ImageSkiaSource that scales 100P image to the target scale factor
 // if the ImageSkiaRep for the target scale factor isn't available.
 class ThemeImageSource: public gfx::ImageSkiaSource {
@@ -480,6 +501,89 @@ class ThemeImageSource: public gfx::ImageSkiaSource {
   DISALLOW_COPY_AND_ASSIGN(ThemeImageSource);
 };
 
+// An ImageSkiaSource that delays decoding PNG data into bitmaps until
+// needed. Missing data for a scale factor is computed by scaling data for an
+// available scale factor. Computed bitmaps are stored for future look up.
+class ThemeImagePngSource : public gfx::ImageSkiaSource {
+ public:
+  ThemeImagePngSource() {}

[pkotwicz, 2013/06/19 16:20:32]

Nit: Could you pass std::map<ui::ScaleFactor, base::RefCountedMemory> in the
constructor? (That would allow you to get rid of AddPng() )

[sschmitz, 2013/06/19 21:56:52]

Done.

+
+  virtual ~ThemeImagePngSource() {}
+
+  void AddPng(scoped_refptr<base::RefCountedMemory> png_memory,
+              ui::ScaleFactor scale_factor) {
+    png_map_[scale_factor] = png_memory;
+  }
+
+ private:
+  virtual gfx::ImageSkiaRep GetImageForScale(
+      ui::ScaleFactor scale_factor) OVERRIDE {
+    // Look up the bitmap for |scale factor| in the bitmap map. If found
+    // return it.
+    BitmapMap::const_iterator exact_bitmap_it = bitmap_map_.find(scale_factor);

[pkotwicz, 2013/06/19 16:20:32]

Technically, this check here is unnecessary because GetImageForScale() will only
be called once per scale factor.

[sschmitz, 2013/06/19 21:56:52]

It feel safer to keep it. Then we're less coupled to how this class is used.

+    if (exact_bitmap_it != bitmap_map_.end())
+      return gfx::ImageSkiaRep(exact_bitmap_it->second, scale_factor);
+
+    // Look up the raw PNG data for |scale_factor| in the png map. If found,
+    // decode it, store the result in the bitmap map and return it.
+    PngMap::const_iterator exact_png_it = png_map_.find(scale_factor);
+    if (exact_png_it != png_map_.end()) {
+      SkBitmap bitmap;
+      if (!gfx::PNGCodec::Decode(exact_png_it->second->front(),
+                                 exact_png_it->second->size(),
+                                 &bitmap)) {
+        NOTREACHED();
+        return gfx::ImageSkiaRep();
+      }
+      bitmap_map_[scale_factor] = bitmap;
+      return gfx::ImageSkiaRep(bitmap, scale_factor);
+    }
+
+    // Find an available PNG for another scale factor. We want to use the
+    // highest available scale factor.
+    PngMap::const_iterator available_png_it = png_map_.end();
+    for (PngMap::const_iterator png_it = png_map_.begin();
+         png_it != png_map_.end(); ++png_it) {
+      if (available_png_it == png_map_.end() ||
+          ui::GetScaleFactorScale(png_it->first) >
+          ui::GetScaleFactorScale(available_png_it->first)) {
+        available_png_it = png_it;
+      }
+    }
+    if (available_png_it == png_map_.end())
+      return gfx::ImageSkiaRep();
+

[pkotwicz, 2013/06/19 16:23:28]

Shouldn't you check if there is already a bitmap in |bitmap_map_|?

[sschmitz, 2013/06/19 21:56:52]

Yes. Done.

+    // Decode the available png and store the result in the bitmap map.
+    SkBitmap available_bitmap;
+    if (!gfx::PNGCodec::Decode(available_png_it->second->front(),
+                               available_png_it->second->size(),
+                               &available_bitmap)) {
+      NOTREACHED();
+      return gfx::ImageSkiaRep();
+    }
+    ui::ScaleFactor available_scale_factor = available_png_it->first;
+    bitmap_map_[available_scale_factor] = available_bitmap;
+
+    // Scale the available bitmap to the desired scale factor, store the result
+    // in the bitmap map and return it.
+    SkBitmap scaled_bitmap = CreateLowQualityResizedBitmap(
+        available_bitmap,
+        available_scale_factor,
+        scale_factor);
+    bitmap_map_[scale_factor] = scaled_bitmap;
+    return gfx::ImageSkiaRep(scaled_bitmap, scale_factor);
+  }
+
+  typedef std::map<ui::ScaleFactor,
+                   scoped_refptr<base::RefCountedMemory> > PngMap;
+  PngMap png_map_;
+
+  typedef std::map<ui::ScaleFactor, SkBitmap> BitmapMap;
+  BitmapMap bitmap_map_;
+
+  DISALLOW_COPY_AND_ASSIGN(ThemeImagePngSource);
+};
+
 class TabBackgroundImageSource: public gfx::CanvasImageSource {
  public:
   TabBackgroundImageSource(const gfx::ImageSkia& image_to_tint,
@@ -749,34 +853,24 @@ gfx::Image BrowserThemePack::GetImageNamed(int idr_id) {
   if (image_iter != images_on_ui_thread_.end())
     return image_iter->second;
 
-  // TODO(pkotwicz): Do something better than loading the bitmaps
-  // for all the scale factors associated with |idr_id|.
-  // See crbug.com/243831.
-  gfx::ImageSkia source_image_skia;
+  ThemeImagePngSource* png_source = NULL;
   for (size_t i = 0; i < scale_factors_.size(); ++i) {
     scoped_refptr<base::RefCountedMemory> memory =
         GetRawData(idr_id, scale_factors_[i]);
     if (memory.get()) {
-      // Decode the PNG.
-      SkBitmap bitmap;
-      if (!gfx::PNGCodec::Decode(memory->front(), memory->size(),
-                                 &bitmap)) {
-        NOTREACHED() << "Unable to decode theme image resource " << idr_id
-                     << " from saved DataPack.";
-        continue;
-      }
-      source_image_skia.AddRepresentation(
-          gfx::ImageSkiaRep(bitmap, scale_factors_[i]));
+      if (!png_source)
+        png_source = new ThemeImagePngSource;
+      png_source->AddPng(memory, scale_factors_[i]);
     }
   }
-
-  if (!source_image_skia.isNull()) {
-    ThemeImageSource* source = new ThemeImageSource(source_image_skia);
-    gfx::ImageSkia image_skia(source, source_image_skia.size());
+  if (png_source) {
+    // |image_skia| owns png_source.
+    gfx::ImageSkia image_skia(png_source, ui::SCALE_FACTOR_100P);
     gfx::Image ret = gfx::Image(image_skia);
     images_on_ui_thread_[prs_id] = ret;
     return ret;
   }
+
   return gfx::Image();
 }
 
@@ -1475,29 +1569,29 @@ void BrowserThemePack::GenerateRawImageForAllSupportedScales(int prs_id) {
     return;
 
   // Find available scale factor with highest scale.
-  ui::ScaleFactor available = ui::SCALE_FACTOR_NONE;
+  ui::ScaleFactor available_scale_factor = ui::SCALE_FACTOR_NONE;
   for (size_t i = 0; i < scale_factors_.size(); ++i) {
     int raw_id = GetRawIDByPersistentID(prs_id, scale_factors_[i]);
-    if ((available == ui::SCALE_FACTOR_NONE ||
+    if ((available_scale_factor == ui::SCALE_FACTOR_NONE ||
          (ui::GetScaleFactorScale(scale_factors_[i]) >
-          ui::GetScaleFactorScale(available))) &&
+          ui::GetScaleFactorScale(available_scale_factor))) &&
         image_memory_.find(raw_id) != image_memory_.end()) {
-      available = scale_factors_[i];
+      available_scale_factor = scale_factors_[i];
     }
   }
   // If no scale factor is available, we're done.
-  if (available == ui::SCALE_FACTOR_NONE)
+  if (available_scale_factor == ui::SCALE_FACTOR_NONE)
     return;
 
   // Get bitmap for the available scale factor.
-  int available_raw_id = GetRawIDByPersistentID(prs_id, available);
+  int available_raw_id = GetRawIDByPersistentID(prs_id, available_scale_factor);
   RawImages::const_iterator it = image_memory_.find(available_raw_id);
   SkBitmap available_bitmap;
   if (!gfx::PNGCodec::Decode(it->second->front(),
                              it->second->size(),
                              &available_bitmap)) {
     NOTREACHED() << "Unable to decode theme image for prs_id="
-                 << prs_id << " for scale_factor=" << available;
+                 << prs_id << " for scale_factor=" << available_scale_factor;
     return;
   }
 
@@ -1506,21 +1600,10 @@ void BrowserThemePack::GenerateRawImageForAllSupportedScales(int prs_id) {
     int scaled_raw_id = GetRawIDByPersistentID(prs_id, scale_factors_[i]);
     if (image_memory_.find(scaled_raw_id) != image_memory_.end())
       continue;
-    gfx::Size scaled_size = gfx::ToCeiledSize(
-        gfx::ScaleSize(gfx::Size(available_bitmap.width(),
-                                 available_bitmap.height()),
-                       ui::GetScaleFactorScale(scale_factors_[i]) /
-                       ui::GetScaleFactorScale(available)));
-    SkBitmap scaled_bitmap;
-    scaled_bitmap.setConfig(SkBitmap::kARGB_8888_Config,
-                            scaled_size.width(),
-                            scaled_size.height());
-    if (!scaled_bitmap.allocPixels())
-      SK_CRASH();
-    scaled_bitmap.eraseARGB(0, 0, 0, 0);
-    SkCanvas canvas(scaled_bitmap);
-    SkRect scaled_bounds = RectToSkRect(gfx::Rect(scaled_size));
-    canvas.drawBitmapRect(available_bitmap, NULL, scaled_bounds);
+    SkBitmap scaled_bitmap =
+        CreateLowQualityResizedBitmap(available_bitmap,
+                                      available_scale_factor,
+                                      scale_factors_[i]);
     std::vector<unsigned char> bitmap_data;
     if (!gfx::PNGCodec::EncodeBGRASkBitmap(scaled_bitmap,
                                            false,