First fix to minimize copying of image data.

chrome/browser/browser_theme_provider.cc

 // Copyright (c) 2009 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/browser_theme_provider.h"
 
 #include "app/gfx/codec/png_codec.h"
 #include "app/gfx/skbitmap_operations.h"
 #include "base/file_util.h"
 #include "base/stl_util-inl.h"
@@ skipping 429 matching lines @@
   // SaveThemeBitmap describing the process by which an original image is
   // tagged.
   ImageMap::const_iterator images_iter = images_.find(id);
   ResourceNameMap::const_iterator names_iter = resource_names_.find(id);
   if ((images_iter == images_.end()) || (names_iter == resource_names_.end()))
     return false;
   return !EndsWith(UTF8ToWide(images_iter->second),
                    UTF8ToWide(names_iter->second), false);
 }
 
-bool BrowserThemeProvider::GetRawData(
-    int id,
-    std::vector<unsigned char>* raw_data) const {
+RefCountedMemory* BrowserThemeProvider::GetRawData(int id) const {
   // Check to see whether we should substitute some images.
   int ntp_alternate;
   GetDisplayProperty(NTP_LOGO_ALTERNATE, &ntp_alternate);
   if (id == IDR_PRODUCT_LOGO && ntp_alternate != 0)
     id = IDR_PRODUCT_LOGO_WHITE;
 
   RawDataMap::const_iterator data_iter = raw_data_.find(id);
   if (data_iter != raw_data_.end()) {
-    *raw_data = data_iter->second;
-    return true;
+    return data_iter->second;
   }

[brettw, 2009/10/16 23:03:12]

You should also remove the {} for this conditional to match the other ones.

 
-  if (!ReadThemeFileData(id, raw_data) &&
-      !rb_.LoadImageResourceBytes(id, raw_data))
-    return false;
+  RefCountedMemory* data = ReadThemeFileData(id);
+  if (!data)
+    data = rb_.LoadImageResourceBytes(id);
+  if (!data)
+    return NULL;
 
-  raw_data_[id] = *raw_data;
-  return true;
+  raw_data_[id] = data;
+  return data;
 }
 
 void BrowserThemeProvider::SetTheme(Extension* extension) {
   // Clear our image cache.
   ClearCaches();
 
   DCHECK(extension);
   DCHECK(extension->IsTheme());
   SetImageData(extension->GetThemeImages(),
                extension->path());
@@ skipping 34 matching lines @@
   ClearAllThemeData();
   NotifyThemeChanged();
   UserMetrics::RecordAction(L"Themes_Reset", profile_);
 }
 
 std::string BrowserThemeProvider::GetThemeID() const {
   std::wstring id = profile_->GetPrefs()->GetString(prefs::kCurrentThemeID);
   return WideToUTF8(id);
 }
 
-bool BrowserThemeProvider::ReadThemeFileData(
-    int id, std::vector<unsigned char>* raw_data) const {
+RefCountedMemory* BrowserThemeProvider::ReadThemeFileData(int id) const {
   ImageMap::const_iterator images_iter = images_.find(id);
   if (images_iter != images_.end()) {
     // First check to see if we have a registered theme extension and whether
     // it can handle this resource.
 #if defined(OS_WIN)
     FilePath path = FilePath(UTF8ToWide(images_iter->second));
 #else
     FilePath path = FilePath(images_iter->second);
 #endif
     if (!path.empty()) {
       net::FileStream file;
       int flags = base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ;
       if (file.Open(path, flags) == net::OK) {
         int64 avail = file.Available();
         if (avail > 0 && avail < INT_MAX) {
           size_t size = static_cast<size_t>(avail);
-          raw_data->resize(size);
-          char* data = reinterpret_cast<char*>(&(raw_data->front()));
+          std::vector<unsigned char> raw_data;
+          raw_data.resize(size);
+          char* data = reinterpret_cast<char*>(&(raw_data.front()));
           if (file.ReadUntilComplete(data, size) == avail)
-            return true;
+            return RefCountedBytes::TakeVector(&raw_data);
         }
       }
     }
   }
 
-  return false;
+  return NULL;
 }
 
 // static
 std::string BrowserThemeProvider::AlignmentToString(int alignment) {
   // Convert from an AlignmentProperty back into a string.
   std::string vertical_string;
   std::string horizontal_string;
 
   if (alignment & BrowserThemeProvider::ALIGN_TOP)
     vertical_string = kAlignmentTop;
@@ skipping 233 matching lines @@
                   Source<BrowserThemeProvider>(this),
                   NotificationService::NoDetails());
 }
 
 SkBitmap* BrowserThemeProvider::LoadThemeBitmap(int id) const {
   DCHECK(CalledOnValidThread());
 
   if (!themeable_images.count(id))
     return NULL;
 
-  // Attempt to find the image in our theme bundle.
-  std::vector<unsigned char> raw_data, png_data;
-  if (ReadThemeFileData(id, &raw_data)) {
+  scoped_refptr<RefCountedMemory> raw_data;
+
+  // We special case images related to the NTP so we first try raw data. Why?
+  // Because the DOMUI stuff uses that interface and otherwise we would be
+  // loading big images twice. Ouch. So either we prime the cache for when
+  // DOMUIThemeSource requests our image, or we take advantage of the already
+  // loaded data, saving a trip to disk.

[brettw, 2009/10/16 23:03:12]

In person, I asked for some clarification of the caching behaviors of the two
functions since why one is used in some cases wasn't clear to me.

+  if (id == IDR_THEME_NTP_BACKGROUND)
+    raw_data = GetRawData(id);
+
+  if (!raw_data)
+    raw_data = ReadThemeFileData(id);
+
+  if (raw_data) {
+    std::vector<unsigned char> png_data;
+
     // Decode the PNG.
     int image_width = 0;
     int image_height = 0;
 
-    if (!gfx::PNGCodec::Decode(&raw_data.front(), raw_data.size(),
+    if (!gfx::PNGCodec::Decode(raw_data->front(), raw_data->size(),
                                gfx::PNGCodec::FORMAT_BGRA, &png_data,
                                &image_width, &image_height)) {
       NOTREACHED() << "Unable to decode theme image resource " << id;
       return NULL;
     }
 
     return gfx::PNGCodec::CreateSkBitmapFromBGRAFormat(png_data,
                                                        image_width,
                                                        image_height);
   } else {
@@ skipping 470 matching lines @@
 }
 
 bool BrowserThemeProvider::ShouldTintFrames() const {
   return (HasCustomImage(IDR_THEME_FRAME) ||
       tints_.count(GetTintKey(TINT_BACKGROUND_TAB)) ||
       tints_.count(GetTintKey(TINT_FRAME)) ||
       tints_.count(GetTintKey(TINT_FRAME_INACTIVE)) ||
       tints_.count(GetTintKey(TINT_FRAME_INCOGNITO)) ||
       tints_.count(GetTintKey(TINT_FRAME_INCOGNITO_INACTIVE)));
 }