Only load theme images for the scale factors that use them

chrome/browser/themes/browser_theme_pack.cc

 // Copyright (c) 2012 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.
 
 #include "chrome/browser/themes/browser_theme_pack.h"
 
 #include <limits>
 
 #include "base/memory/ref_counted_memory.h"
 #include "base/memory/scoped_ptr.h"
@@ skipping 425 matching lines @@
 }
 
 // Shifts an image's HSL values. The caller is responsible for deleting
 // the returned image.
 gfx::Image CreateHSLShiftedImage(const gfx::Image& image,
                                  const color_utils::HSL& hsl_shift) {
   const gfx::ImageSkia* src_image = image.ToImageSkia();
   return gfx::Image(gfx::ImageSkiaOperations::CreateHSLShiftedImage(
       *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 {
  public:
   explicit ThemeImageSource(const gfx::ImageSkia& source) : source_(source) {
   }
   virtual ~ThemeImageSource() {}
 
   virtual gfx::ImageSkiaRep GetImageForScale(
       ui::ScaleFactor scale_factor) OVERRIDE {
     if (source_.HasRepresentation(scale_factor))
       return source_.GetRepresentation(scale_factor);
     const gfx::ImageSkiaRep& rep_100p =
         source_.GetRepresentation(ui::SCALE_FACTOR_100P);
     float scale = ui::GetScaleFactorScale(scale_factor);
     gfx::Size size(rep_100p.GetWidth() * scale, rep_100p.GetHeight() * scale);
     SkBitmap resized_bitmap;
     resized_bitmap.setConfig(SkBitmap::kARGB_8888_Config, size.width(),

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

You should be able to invoke CreateLowQualityResizedBitmap() here too

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

Done.

                              size.height());
     if (!resized_bitmap.allocPixels())
       SK_CRASH();
     resized_bitmap.eraseARGB(0, 0, 0, 0);
     SkCanvas canvas(resized_bitmap);
     SkRect resized_bounds = RectToSkRect(gfx::Rect(size));
     // Note(oshima): The following scaling code doesn't work with
     // a mask image.
     canvas.drawBitmapRect(rep_100p.sk_bitmap(), NULL, resized_bounds);
     return gfx::ImageSkiaRep(resized_bitmap, scale_factor);
   }
 
  private:
   const gfx::ImageSkia source_;
 
   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,
                            const gfx::ImageSkia& overlay,
                            const color_utils::HSL& hsl_shift,
                            int vertical_offset)
       : gfx::CanvasImageSource(image_to_tint.size(), false),
         image_to_tint_(image_to_tint),
         overlay_(overlay),
         hsl_shift_(hsl_shift),
@@ skipping 249 matching lines @@
 gfx::Image BrowserThemePack::GetImageNamed(int idr_id) {
   int prs_id = GetPersistentIDByIDR(idr_id);
   if (prs_id == -1)
     return gfx::Image();
 
   // Check if the image is cached.
   ImageCache::const_iterator image_iter = images_on_ui_thread_.find(prs_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();
 }
 
 base::RefCountedMemory* BrowserThemePack::GetRawData(
     int idr_id,
     ui::ScaleFactor scale_factor) const {
   base::RefCountedMemory* memory = NULL;
   int prs_id = GetPersistentIDByIDR(idr_id);
   int raw_id = GetRawIDByPersistentID(prs_id, scale_factor);
 
@@ skipping 678 matching lines @@
     int raw_id = GetRawIDByPersistentID(prs_id, scale_factors_[i]);
     if (image_memory_.find(raw_id) == image_memory_.end()) {
       image_missing = true;
       break;
     }
   }
   if (!image_missing)
     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;
   }
 
   // Fill in all missing scale factors by scaling the available bitmap.
   for (size_t i = 0; i < scale_factors_.size(); ++i) {
     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,
                                            &bitmap_data)) {
       NOTREACHED() << "Unable to encode theme image for prs_id="
                    << prs_id << " for scale_factor=" << scale_factors_[i];
       break;
     }
     image_memory_[scaled_raw_id] =
         base::RefCountedBytes::TakeVector(&bitmap_data);
   }
 }