Tracing showed that the resource bundle lock was held for long periods of tim...

chrome/common/resource_bundle.cc

 // Copyright (c) 2006-2008 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/common/resource_bundle.h"
 
 #include <atlbase.h>
 
 #include "base/file_util.h"
 #include "base/gfx/png_decoder.h"
@@ skipping 128 matching lines @@
     NOTREACHED() << "Unable to decode theme resource " << resource_id;
     return NULL;
   }
 
   return PNGDecoder::CreateSkBitmapFromBGRAFormat(png_data,
                                                   image_width,
                                                   image_height);
 }
 
 SkBitmap* ResourceBundle::GetBitmapNamed(int resource_id) {
-  AutoLock lock_scope(lock_);
-
-  SkImageMap::const_iterator found = skia_images_.find(resource_id);
-  SkBitmap* bitmap = NULL;
-
-  // If not found load and store the image
-  if (found == skia_images_.end()) {
-    // Try to load the bitmap from the theme dll.
-    if (theme_dll_)
-      bitmap = LoadBitmap(theme_dll_, resource_id);
-    // We did not find the bitmap in the theme DLL, try the current one.
-    if (!bitmap)
-      bitmap = LoadBitmap(_AtlBaseModule.GetModuleInstance(), resource_id);
-    skia_images_[resource_id] = bitmap;
-  } else {
-    bitmap = found->second;
+  // Check to see if we already have the Skia image in the cache.
+  {
+    AutoLock lock_scope(lock_);
+    SkImageMap::const_iterator found = skia_images_.find(resource_id);
+    if (found != skia_images_.end())
+      return found->second;
   }
 
-  // This bitmap will be returned when a bitmap is requested that can not be
-  // found. The data inside is lazily initialized, so users must lock and
-  static SkBitmap* empty_bitmap = NULL;
+  scoped_ptr<SkBitmap> bitmap;
 
-  // Handle the case where loading the bitmap failed.
-  if (!bitmap) {
+  // Try to load the bitmap from the theme dll.
+  if (theme_dll_)
+    bitmap.reset(LoadBitmap(theme_dll_, resource_id));
+
+  // If we did not find the bitmap in the theme DLL, try the current one.
+  if (!bitmap.get())
+    bitmap.reset(LoadBitmap(_AtlBaseModule.GetModuleInstance(), resource_id));
+
+  // We loaded successfully.  Cache the Skia version of the bitmap.
+  if (bitmap.get()) {
+    AutoLock lock_scope(lock_);
+
+    // Another thread raced us, and has already cached the skia image.
+    if (skia_images_.count(resource_id))
+      return skia_images_[resource_id];
+
+    skia_images_[resource_id] = bitmap.get();
+    return bitmap.release();
+  }
+
+  // We failed to retrieve the bitmap, show a debugging red square.
+  {
     LOG(WARNING) << "Unable to load bitmap with id " << resource_id;
-    NOTREACHED(); // Want to assert in debug mode.
+    NOTREACHED();  // Want to assert in debug mode.
+
+    AutoLock lock_scope(lock_);  // Guard empty_bitmap initialization.
+
+    static SkBitmap* empty_bitmap = NULL;
     if (!empty_bitmap) {
-      // The placeholder bitmap is bright red so people notice the problem.     
-      empty_bitmap = new SkBitmap();    
+      // The placeholder bitmap is bright red so people notice the problem.
+      // This bitmap will be leaked, but this code should never be hit.
+      empty_bitmap = new SkBitmap();
       empty_bitmap->setConfig(SkBitmap::kARGB_8888_Config, 32, 32);     
       empty_bitmap->allocPixels();      
       empty_bitmap->eraseARGB(255, 255, 0, 0);
     }
     return empty_bitmap;
   }
-  return bitmap;
 }
 
 bool ResourceBundle::LoadImageResourceBytes(int resource_id,
                                             vector<unsigned char>* bytes) {
   return LoadModuleResourceBytes(theme_dll_, resource_id, bytes);
 }
 
 bool ResourceBundle::LoadDataResourceBytes(int resource_id,
                                            vector<unsigned char>* bytes) {
   return LoadModuleResourceBytes(_AtlBaseModule.GetModuleInstance(),
@@ skipping 114 matching lines @@
       return *medium_bold_font_;
     case LargeFont:
       return *large_font_;
     case WebFont:
       return *web_font_;
     default:
       return *base_font_;
   }
 }