Theme suppliers: Avoid all Image copying.

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 daf8a176b7d55c7af41499619066bff99957c6c4..e91228f4f97fe1f3995bb761a9bceaf1be934d4d 100644
--- a/chrome/browser/themes/browser_theme_pack.cc
+++ b/chrome/browser/themes/browser_theme_pack.cc
@@ -9,6 +9,7 @@
 
 #include <limits>
 #include <memory>
+#include <utility>
 
 #include "base/files/file.h"
 #include "base/macros.h"
@@ -620,33 +621,36 @@ scoped_refptr<BrowserThemePack> BrowserThemePack::BuildFromExtension(
       &file_paths);
   pack->BuildSourceImagesArray(file_paths);
 
-  if (!pack->LoadRawBitmapsTo(file_paths, &pack->images_on_ui_thread_))
+  // Create images for use on the FILE thread (which is where these Image
+  // objects will live).
+  if (!pack->LoadRawBitmapsTo(file_paths, &pack->images_on_file_thread_))
     return NULL;
 
-  pack->CreateImages(&pack->images_on_ui_thread_);
+  pack->CreateImages(&pack->images_on_file_thread_);
 
   // Make sure the |images_on_file_thread_| has bitmaps for supported
   // scale factors before passing to FILE thread.
-  pack->images_on_file_thread_ = pack->images_on_ui_thread_;
-  for (ImageCache::iterator it = pack->images_on_file_thread_.begin();
-       it != pack->images_on_file_thread_.end(); ++it) {
+  for (auto& item : pack->images_on_file_thread_) {
     gfx::ImageSkia* image_skia =
-        const_cast<gfx::ImageSkia*>(it->second.ToImageSkia());
+        const_cast<gfx::ImageSkia*>(item.second.ToImageSkia());
     image_skia->MakeThreadSafe();
   }
 
+  // Copy the images over for use on the UI thread. To avoid sharing
+  // thread-unsafe Image objects across threads, new Image objects are created.
+  //
   // Set ThemeImageSource on |images_on_ui_thread_| to resample the source
   // image if a caller of BrowserThemePack::GetImageNamed() requests an
   // ImageSkiaRep for a scale factor not specified by the theme author.
   // Callers of BrowserThemePack::GetImageNamed() to be able to retrieve
   // ImageSkiaReps for all supported scale factors.
-  for (ImageCache::iterator it = pack->images_on_ui_thread_.begin();
-       it != pack->images_on_ui_thread_.end(); ++it) {
-    const gfx::ImageSkia source_image_skia = it->second.AsImageSkia();
+  for (auto& item : pack->images_on_file_thread_) {
+    const gfx::ImageSkia source_image_skia = item.second.AsImageSkia();
     ThemeImageSource* source = new ThemeImageSource(source_image_skia);
     // image_skia takes ownership of source.
     gfx::ImageSkia image_skia(source, source_image_skia.size());
-    it->second = gfx::Image(image_skia);
+    pack->images_on_ui_thread_.insert(
+        std::make_pair(item.first, gfx::Image(image_skia)));
   }
 
   // Generate raw images (for new-tab-page attribution and background) for
@@ -814,10 +818,10 @@ bool BrowserThemePack::GetDisplayProperty(int id, int* result) const {
   return false;
 }
 
-gfx::Image BrowserThemePack::GetImageNamed(int idr_id) {
+const gfx::Image& BrowserThemePack::GetImageNamed(int idr_id) {
   int prs_id = GetPersistentIDByIDR(idr_id);
   if (prs_id == -1)
-    return gfx::Image();
+    return empty_image();
 
   // Check if the image is cached.
   ImageCache::const_iterator image_iter = images_on_ui_thread_.find(prs_id);
@@ -834,12 +838,12 @@ gfx::Image BrowserThemePack::GetImageNamed(int idr_id) {
   if (!png_map.empty()) {
     gfx::ImageSkia image_skia(new ThemeImagePngSource(png_map), 1.0f);
     // |image_skia| takes ownership of ThemeImagePngSource.
-    gfx::Image ret = gfx::Image(image_skia);
-    images_on_ui_thread_[prs_id] = ret;
-    return ret;
+    auto result = images_on_ui_thread_.insert(
+        std::make_pair(prs_id, gfx::Image(image_skia)));
+    return result.first->second;
   }
 
-  return gfx::Image();
+  return empty_image();
 }
 
 base::RefCountedMemory* BrowserThemePack::GetRawData(
@@ -1289,7 +1293,7 @@ void BrowserThemePack::CreateFrameImages(ImageCache* images) const {
 
   for (size_t i = 0; i < arraysize(kFrameTintMap); ++i) {
     int prs_id = kFrameTintMap[i].key;
-    gfx::Image frame;
+    const gfx::Image* frame = &empty_image();
     // If there's no frame image provided for the specified id, then load
     // the default provided frame. If that's not provided, skip this whole
     // thing and just use the default images.
@@ -1311,27 +1315,22 @@ void BrowserThemePack::CreateFrameImages(ImageCache* images) const {
       }
     }
     if (images->count(prs_id)) {
-      frame = (*images)[prs_id];
+      frame = &(*images)[prs_id];
     } else if (prs_base_id != prs_id && images->count(prs_base_id)) {
-      frame = (*images)[prs_base_id];
-    } else if (prs_base_id == PRS_THEME_FRAME_OVERLAY) {
-      if (images->count(PRS_THEME_FRAME)) {
-        // If there is no theme overlay, don't tint the default frame,
-        // because it will overwrite the custom frame image when we cache and
-        // reload from disk.
-        frame = gfx::Image();
-      }
-    } else {
-      // If the theme doesn't specify an image, then apply the tint to
-      // the default frame.
-      frame = rb.GetImageNamed(IDR_THEME_FRAME);
+      frame = &(*images)[prs_base_id];
+    } else if (prs_base_id != PRS_THEME_FRAME_OVERLAY) {
+      // If there is no theme overlay, don't tint the default frame,
+      // because it will overwrite the custom frame image when we cache and
+      // reload from disk. If the theme doesn't specify an image, then apply the
+      // tint to the default frame.
+      frame = &rb.GetImageNamed(IDR_THEME_FRAME);
     }
-    if (!frame.IsEmpty()) {
+    if (!frame->IsEmpty()) {
       temp_output[prs_id] = CreateHSLShiftedImage(
-          frame, GetTintInternal(kFrameTintMap[i].value));
+          *frame, GetTintInternal(kFrameTintMap[i].value));
     }
   }
-  MergeImageCaches(temp_output, images);
+  MergeImageCaches(std::move(temp_output), images);
 }
 
 void BrowserThemePack::CreateTintedButtons(
@@ -1384,7 +1383,7 @@ void BrowserThemePack::CreateTabBackgroundImages(ImageCache* images) const {
           image_to_tint.size()));
     }
   }
-  MergeImageCaches(temp_output, images);
+  MergeImageCaches(std::move(temp_output), images);
 }
 
 void BrowserThemePack::RepackImages(const ImageCache& images,
@@ -1415,12 +1414,10 @@ void BrowserThemePack::RepackImages(const ImageCache& images,
   }
 }
 
-void BrowserThemePack::MergeImageCaches(
-    const ImageCache& source, ImageCache* destination) const {
-  for (ImageCache::const_iterator it = source.begin(); it != source.end();
-       ++it) {
-    (*destination)[it->first] = it->second;
-  }
+void BrowserThemePack::MergeImageCaches(ImageCache source,
+                                        ImageCache* destination) const {
+  for (auto& item : source)
+    (*destination)[item.first] = std::move(item.second);
 }
 
 void BrowserThemePack::AddRawImagesTo(const RawImages& images,