Refactor GpuDataManagerImpl to make it thread-safe, now and forever.

content/browser/gpu/gpu_data_manager_impl_private.cc

Index: content/browser/gpu/gpu_data_manager_impl_private.cc
===================================================================
--- content/browser/gpu/gpu_data_manager_impl_private.cc	(revision 199087)
+++ content/browser/gpu/gpu_data_manager_impl_private.cc	(working copy)
@@ -1,8 +1,8 @@
-// Copyright (c) 2012 The Chromium Authors. All rights reserved.
+// Copyright (c) 2013 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 "content/browser/gpu/gpu_data_manager_impl.h"
+#include "content/browser/gpu/gpu_data_manager_impl_private.h"
 
 #if defined(OS_MACOSX)
 #include <ApplicationServices/ApplicationServices.h>
@@ -19,7 +19,6 @@
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_piece.h"
 #include "base/sys_info.h"
-#include "base/values.h"
 #include "base/version.h"
 #include "content/browser/gpu/gpu_process_host.h"
 #include "content/browser/gpu/gpu_util.h"
@@ -102,17 +101,7 @@
 
 }  // namespace anonymous
 
-// static
-GpuDataManager* GpuDataManager::GetInstance() {
-  return GpuDataManagerImpl::GetInstance();
-}
-
-// static
-GpuDataManagerImpl* GpuDataManagerImpl::GetInstance() {
-  return Singleton<GpuDataManagerImpl>::get();
-}
-
-void GpuDataManagerImpl::InitializeForTesting(
+void GpuDataManagerImplPrivate::InitializeForTesting(
     const std::string& gpu_blacklist_json,
     const GPUInfo& gpu_info) {
   // This function is for testing only, so disable histograms.
@@ -121,7 +110,7 @@
   InitializeImpl(gpu_blacklist_json, std::string(), std::string(), gpu_info);
 }
 
-bool GpuDataManagerImpl::IsFeatureBlacklisted(int feature) const {
+bool GpuDataManagerImplPrivate::IsFeatureBlacklisted(int feature) const {
   if (use_swiftshader_) {
     // Skia's software rendering is probably more efficient than going through
     // software emulation of the GPU, so use that.
@@ -133,42 +122,23 @@
   return (blacklisted_features_.count(feature) == 1);
 }
 
-size_t GpuDataManagerImpl::GetBlacklistedFeatureCount() const {
+size_t GpuDataManagerImplPrivate::GetBlacklistedFeatureCount() const {
   if (use_swiftshader_)
     return 1;
   return blacklisted_features_.size();
 }
 
-void GpuDataManagerImpl::AddGpuSwitchCallback(
-    const GpuSwitchCallback& callback) {
-  gpu_switch_callbacks_.push_back(callback);
+GPUInfo GpuDataManagerImplPrivate::GetGPUInfo() const {
+  return gpu_info_;
 }
 
-void GpuDataManagerImpl::RemoveGpuSwitchCallback(
-    const GpuSwitchCallback& callback) {
-  for (size_t i = 0; i < gpu_switch_callbacks_.size(); i++) {
-    if (gpu_switch_callbacks_[i].Equals(callback)) {
-      gpu_switch_callbacks_.erase(gpu_switch_callbacks_.begin() + i);
-      return;
-    }
-  }
-}
-
-GPUInfo GpuDataManagerImpl::GetGPUInfo() const {
-  GPUInfo gpu_info;
-  {
-    base::AutoLock auto_lock(gpu_info_lock_);
-    gpu_info = gpu_info_;
-  }
-  return gpu_info;
-}
-
-void GpuDataManagerImpl::GetGpuProcessHandles(
-    const GetGpuProcessHandlesCallback& callback) const {
+void GpuDataManagerImplPrivate::GetGpuProcessHandles(
+    const GpuDataManager::GetGpuProcessHandlesCallback& callback) const {
   GpuProcessHost::GetProcessHandles(callback);
 }
 
-bool GpuDataManagerImpl::GpuAccessAllowed(std::string* reason) const {
+bool GpuDataManagerImplPrivate::GpuAccessAllowed(
+    std::string* reason) const {
   if (use_swiftshader_)
     return true;
 
@@ -220,7 +190,7 @@
   return true;
 }
 
-void GpuDataManagerImpl::RequestCompleteGpuInfoIfNeeded() {
+void GpuDataManagerImplPrivate::RequestCompleteGpuInfoIfNeeded() {
   if (complete_gpu_info_already_requested_ || gpu_info_.finalized)
     return;
   complete_gpu_info_already_requested_ = true;
@@ -235,35 +205,39 @@
       new GpuMsg_CollectGraphicsInfo());
 }
 
-bool GpuDataManagerImpl::IsCompleteGpuInfoAvailable() const {
+bool GpuDataManagerImplPrivate::IsCompleteGpuInfoAvailable() const {
   return gpu_info_.finalized;
 }
 
-void GpuDataManagerImpl::RequestVideoMemoryUsageStatsUpdate() const {
+void GpuDataManagerImplPrivate::RequestVideoMemoryUsageStatsUpdate() const {
   GpuProcessHost::SendOnIO(
       GpuProcessHost::GPU_PROCESS_KIND_SANDBOXED,
       CAUSE_FOR_GPU_LAUNCH_NO_LAUNCH,
       new GpuMsg_GetVideoMemoryUsageStats());
 }
 
-bool GpuDataManagerImpl::ShouldUseSwiftShader() const {
+bool GpuDataManagerImplPrivate::ShouldUseSwiftShader() const {
   return use_swiftshader_;
 }
 
-void GpuDataManagerImpl::RegisterSwiftShaderPath(const base::FilePath& path) {
+void GpuDataManagerImplPrivate::RegisterSwiftShaderPath(
+    const base::FilePath& path) {
   swiftshader_path_ = path;
   EnableSwiftShaderIfNecessary();
 }
 
-void GpuDataManagerImpl::AddObserver(GpuDataManagerObserver* observer) {
+void GpuDataManagerImplPrivate::AddObserver(GpuDataManagerObserver* observer) {
+  GpuDataManagerImpl::UnlockedSession session(owner_);
   observer_list_->AddObserver(observer);
 }
 
-void GpuDataManagerImpl::RemoveObserver(GpuDataManagerObserver* observer) {
+void GpuDataManagerImplPrivate::RemoveObserver(
+    GpuDataManagerObserver* observer) {
+  GpuDataManagerImpl::UnlockedSession session(owner_);
   observer_list_->RemoveObserver(observer);
 }
 
-void GpuDataManagerImpl::UnblockDomainFrom3DAPIs(const GURL& url) {
+void GpuDataManagerImplPrivate::UnblockDomainFrom3DAPIs(const GURL& url) {
   // This method must do two things:
   //
   //  1. If the specific domain is blocked, then unblock it.
@@ -278,36 +252,32 @@
   // will become difficult to explain.
   std::string domain = GetDomainFromURL(url);
 
-  base::AutoLock auto_lock(gpu_info_lock_);
   blocked_domains_.erase(domain);
   timestamps_of_gpu_resets_.clear();
 }
 
-void GpuDataManagerImpl::DisableGpuWatchdog() {
+void GpuDataManagerImplPrivate::DisableGpuWatchdog() {
   GpuProcessHost::SendOnIO(
       GpuProcessHost::GPU_PROCESS_KIND_SANDBOXED,
       CAUSE_FOR_GPU_LAUNCH_NO_LAUNCH,
       new GpuMsg_DisableWatchdog);
 }
 
-void GpuDataManagerImpl::SetGLStrings(const std::string& gl_vendor,
-                                      const std::string& gl_renderer,
-                                      const std::string& gl_version) {
+void GpuDataManagerImplPrivate::SetGLStrings(const std::string& gl_vendor,
+                                             const std::string& gl_renderer,
+                                             const std::string& gl_version) {
   if (gl_vendor.empty() && gl_renderer.empty() && gl_version.empty())
     return;
 
-  GPUInfo gpu_info;
-  {
-    base::AutoLock auto_lock(gpu_info_lock_);
-    // If GPUInfo already got GL strings, do nothing.  This is for the rare
-    // situation where GPU process collected GL strings before this call.
-    if (!gpu_info_.gl_vendor.empty() ||
-        !gpu_info_.gl_renderer.empty() ||
-        !gpu_info_.gl_version_string.empty())
-      return;
-    gpu_info = gpu_info_;
-  }
+  // If GPUInfo already got GL strings, do nothing.  This is for the rare
+  // situation where GPU process collected GL strings before this call.
+  if (!gpu_info_.gl_vendor.empty() ||
+      !gpu_info_.gl_renderer.empty() ||
+      !gpu_info_.gl_version_string.empty())
+    return;
 
+  GPUInfo gpu_info = gpu_info_;
+
   gpu_info.gl_vendor = gl_vendor;
   gpu_info.gl_renderer = gl_renderer;
   gpu_info.gl_version_string = gl_version;
@@ -319,19 +289,17 @@
   UpdatePreliminaryBlacklistedFeatures();
 }
 
-void GpuDataManagerImpl::GetGLStrings(std::string* gl_vendor,
-                                      std::string* gl_renderer,
-                                      std::string* gl_version) {
+void GpuDataManagerImplPrivate::GetGLStrings(std::string* gl_vendor,
+                                             std::string* gl_renderer,
+                                             std::string* gl_version) {
   DCHECK(gl_vendor && gl_renderer && gl_version);
 
-  base::AutoLock auto_lock(gpu_info_lock_);
   *gl_vendor = gpu_info_.gl_vendor;
   *gl_renderer = gpu_info_.gl_renderer;
   *gl_version = gpu_info_.gl_version_string;
 }
 
-
-void GpuDataManagerImpl::Initialize() {
+void GpuDataManagerImplPrivate::Initialize() {
   TRACE_EVENT0("startup", "GpuDataManagerImpl::Initialize");
   CommandLine* command_line = CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(switches::kSkipGpuDataLoading))
@@ -383,25 +351,20 @@
   }
 }
 
-void GpuDataManagerImpl::UpdateGpuInfo(const GPUInfo& gpu_info) {
+void GpuDataManagerImplPrivate::UpdateGpuInfo(const GPUInfo& gpu_info) {
   // No further update of gpu_info if falling back to SwiftShader.
   if (use_swiftshader_)
     return;
 
-  GPUInfo my_gpu_info;
-  {
-    base::AutoLock auto_lock(gpu_info_lock_);
-    gpu_info_collector::MergeGPUInfo(&gpu_info_, gpu_info);
-    complete_gpu_info_already_requested_ =
-        complete_gpu_info_already_requested_ || gpu_info_.finalized;
-    my_gpu_info = gpu_info_;
-  }
+  gpu_info_collector::MergeGPUInfo(&gpu_info_, gpu_info);
+  complete_gpu_info_already_requested_ =
+      complete_gpu_info_already_requested_ || gpu_info_.finalized;
 
-  GetContentClient()->SetGpuInfo(my_gpu_info);
+  GetContentClient()->SetGpuInfo(gpu_info_);
 
   if (gpu_blacklist_) {
     std::set<int> features = gpu_blacklist_->MakeDecision(
-        GpuControlList::kOsAny, std::string(), my_gpu_info);
+        GpuControlList::kOsAny, std::string(), gpu_info_);
     if (update_histograms_)
       UpdateStats(gpu_blacklist_.get(), features);
 
@@ -409,7 +372,7 @@
   }
   if (gpu_switching_list_) {
     std::set<int> option = gpu_switching_list_->MakeDecision(
-        GpuControlList::kOsAny, std::string(), my_gpu_info);
+        GpuControlList::kOsAny, std::string(), gpu_info_);
     if (option.size() == 1) {
       // Blacklist decision should not overwrite commandline switch from users.
       CommandLine* command_line = CommandLine::ForCurrentProcess();
@@ -419,19 +382,20 @@
   }
   if (gpu_driver_bug_list_)
     gpu_driver_bugs_ = gpu_driver_bug_list_->MakeDecision(
-        GpuControlList::kOsAny, std::string(), my_gpu_info);
+        GpuControlList::kOsAny, std::string(), gpu_info_);
 
   // We have to update GpuFeatureType before notify all the observers.
   NotifyGpuInfoUpdate();
 }
 
-void GpuDataManagerImpl::UpdateVideoMemoryUsageStats(
+void GpuDataManagerImplPrivate::UpdateVideoMemoryUsageStats(
     const GPUVideoMemoryUsageStats& video_memory_usage_stats) {
+  GpuDataManagerImpl::UnlockedSession session(owner_);
   observer_list_->Notify(&GpuDataManagerObserver::OnVideoMemoryUsageStatsUpdate,
                          video_memory_usage_stats);
 }
 
-void GpuDataManagerImpl::AppendRendererCommandLine(
+void GpuDataManagerImplPrivate::AppendRendererCommandLine(
     CommandLine* command_line) const {
   DCHECK(command_line);
 
@@ -459,7 +423,7 @@
     command_line->AppendSwitch(switches::kDisableFlashFullscreen3d);
 }
 
-void GpuDataManagerImpl::AppendGpuCommandLine(
+void GpuDataManagerImplPrivate::AppendGpuCommandLine(
     CommandLine* command_line) const {
   DCHECK(command_line);
 
@@ -527,32 +491,29 @@
   }
 #endif
 
-  {
-    base::AutoLock auto_lock(gpu_info_lock_);
-    if (gpu_info_.optimus)
-      command_line->AppendSwitch(switches::kReduceGpuSandbox);
-    if (gpu_info_.amd_switchable) {
-      // The image transport surface currently doesn't work with AMD Dynamic
-      // Switchable graphics.
-      command_line->AppendSwitch(switches::kReduceGpuSandbox);
-      command_line->AppendSwitch(switches::kDisableImageTransportSurface);
-    }
-    // Pass GPU and driver information to GPU process. We try to avoid full GPU
-    // info collection at GPU process startup, but we need gpu vendor_id,
-    // device_id, driver_vendor, driver_version for deciding whether we need to
-    // collect full info (on Linux) and for crash reporting purpose.
-    command_line->AppendSwitchASCII(switches::kGpuVendorID,
-        base::StringPrintf("0x%04x", gpu_info_.gpu.vendor_id));
-    command_line->AppendSwitchASCII(switches::kGpuDeviceID,
-        base::StringPrintf("0x%04x", gpu_info_.gpu.device_id));
-    command_line->AppendSwitchASCII(switches::kGpuDriverVendor,
-        gpu_info_.driver_vendor);
-    command_line->AppendSwitchASCII(switches::kGpuDriverVersion,
-        gpu_info_.driver_version);
+  if (gpu_info_.optimus)
+    command_line->AppendSwitch(switches::kReduceGpuSandbox);
+  if (gpu_info_.amd_switchable) {
+    // The image transport surface currently doesn't work with AMD Dynamic
+    // Switchable graphics.
+    command_line->AppendSwitch(switches::kReduceGpuSandbox);
+    command_line->AppendSwitch(switches::kDisableImageTransportSurface);
   }
+  // Pass GPU and driver information to GPU process. We try to avoid full GPU
+  // info collection at GPU process startup, but we need gpu vendor_id,
+  // device_id, driver_vendor, driver_version for deciding whether we need to
+  // collect full info (on Linux) and for crash reporting purpose.
+  command_line->AppendSwitchASCII(switches::kGpuVendorID,
+      base::StringPrintf("0x%04x", gpu_info_.gpu.vendor_id));
+  command_line->AppendSwitchASCII(switches::kGpuDeviceID,
+      base::StringPrintf("0x%04x", gpu_info_.gpu.device_id));
+  command_line->AppendSwitchASCII(switches::kGpuDriverVendor,
+      gpu_info_.driver_vendor);
+  command_line->AppendSwitchASCII(switches::kGpuDriverVersion,
+      gpu_info_.driver_version);
 }
 
-void GpuDataManagerImpl::AppendPluginCommandLine(
+void GpuDataManagerImplPrivate::AppendPluginCommandLine(
     CommandLine* command_line) const {
   DCHECK(command_line);
 
@@ -571,7 +532,8 @@
 #endif
 }
 
-void GpuDataManagerImpl::UpdateRendererWebPrefs(WebPreferences* prefs) const {
+void GpuDataManagerImplPrivate::UpdateRendererWebPrefs(
+    WebPreferences* prefs) const {
   DCHECK(prefs);
 
   if (IsFeatureBlacklisted(GPU_FEATURE_TYPE_ACCELERATED_COMPOSITING))
@@ -607,13 +569,13 @@
   }
 }
 
-GpuSwitchingOption GpuDataManagerImpl::GetGpuSwitchingOption() const {
+GpuSwitchingOption GpuDataManagerImplPrivate::GetGpuSwitchingOption() const {
   if (!ui::GpuSwitchingManager::GetInstance()->SupportsDualGpus())
     return GPU_SWITCHING_OPTION_UNKNOWN;
   return gpu_switching_;
 }
 
-void GpuDataManagerImpl::DisableHardwareAcceleration() {
+void GpuDataManagerImplPrivate::DisableHardwareAcceleration() {
   card_blacklisted_ = true;
 
   for (int i = 0; i < NUMBER_OF_GPU_FEATURE_TYPES; ++i)
@@ -623,22 +585,21 @@
   NotifyGpuInfoUpdate();
 }
 
-std::string GpuDataManagerImpl::GetBlacklistVersion() const {
+std::string GpuDataManagerImplPrivate::GetBlacklistVersion() const {
   if (gpu_blacklist_)
     return gpu_blacklist_->version();
   return "0";
 }
 
-base::ListValue* GpuDataManagerImpl::GetBlacklistReasons() const {
+base::ListValue* GpuDataManagerImplPrivate::GetBlacklistReasons() const {
   ListValue* reasons = new ListValue();
   if (gpu_blacklist_)
     gpu_blacklist_->GetReasons(reasons);
   return reasons;
 }
 
-void GpuDataManagerImpl::AddLogMessage(
+void GpuDataManagerImplPrivate::AddLogMessage(
     int level, const std::string& header, const std::string& message) {
-  base::AutoLock auto_lock(log_messages_lock_);
   DictionaryValue* dict = new DictionaryValue();
   dict->SetInteger("level", level);
   dict->SetString("header", header);
@@ -646,37 +607,41 @@
   log_messages_.Append(dict);
 }
 
-void GpuDataManagerImpl::ProcessCrashed(base::TerminationStatus exit_code) {
+void GpuDataManagerImplPrivate::ProcessCrashed(
+    base::TerminationStatus exit_code) {
   if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
-    BrowserThread::PostTask(BrowserThread::UI,
-                            FROM_HERE,
-                            base::Bind(&GpuDataManagerImpl::ProcessCrashed,
-                                       base::Unretained(this),
-                                       exit_code));
+    if (owner_) {
+      // Unretained is ok, because it's posted to UI thread, the thread
+      // where the singleton GpuDataManagerImpl lives until the end.
+      BrowserThread::PostTask(
+          BrowserThread::UI,
+          FROM_HERE,
+          base::Bind(&GpuDataManagerImpl::ProcessCrashed,
+                     base::Unretained(owner_),
+                     exit_code));
+    }
     return;
   }
-  observer_list_->Notify(&GpuDataManagerObserver::OnGpuProcessCrashed,
-                         exit_code);
+  {

[Ken Russell (switch to Gerrit), 2013/05/10 23:59:19]

This is a little hard to read. Instead of the explicit "return" above, consider
writing this as a proper if/else.

+    GpuDataManagerImpl::UnlockedSession session(owner_);
+    observer_list_->Notify(
+        &GpuDataManagerObserver::OnGpuProcessCrashed, exit_code);
+  }
 }
 
-base::ListValue* GpuDataManagerImpl::GetLogMessages() const {
+base::ListValue* GpuDataManagerImplPrivate::GetLogMessages() const {
   base::ListValue* value;
-  {
-    base::AutoLock auto_lock(log_messages_lock_);
-    value = log_messages_.DeepCopy();
-  }
+  value = log_messages_.DeepCopy();
   return value;
 }
 
-void GpuDataManagerImpl::HandleGpuSwitch() {
-  complete_gpu_info_already_requested_ = false;
-  gpu_info_.finalized = false;
-  for (size_t i = 0; i < gpu_switch_callbacks_.size(); ++i)
-    gpu_switch_callbacks_[i].Run();
+void GpuDataManagerImplPrivate::HandleGpuSwitch() {
+  GpuDataManagerImpl::UnlockedSession session(owner_);
+  observer_list_->Notify(&GpuDataManagerObserver::OnGpuSwitching);
 }
 
 #if defined(OS_WIN)
-bool GpuDataManagerImpl::IsUsingAcceleratedSurface() const {
+bool GpuDataManagerImplPrivate::IsUsingAcceleratedSurface() const {
   if (base::win::GetVersion() < base::win::VERSION_VISTA)
     return false;
 
@@ -691,33 +656,43 @@
 }
 #endif
 
-void GpuDataManagerImpl::BlockDomainFrom3DAPIs(
-    const GURL& url, DomainGuilt guilt) {
+void GpuDataManagerImplPrivate::BlockDomainFrom3DAPIs(
+    const GURL& url, GpuDataManagerImpl::DomainGuilt guilt) {
   BlockDomainFrom3DAPIsAtTime(url, guilt, base::Time::Now());
 }
 
-bool GpuDataManagerImpl::Are3DAPIsBlocked(const GURL& url,
-                                          int render_process_id,
-                                          int render_view_id,
-                                          ThreeDAPIType requester) {
+bool GpuDataManagerImplPrivate::Are3DAPIsBlocked(const GURL& url,
+                                                 int render_process_id,
+                                                 int render_view_id,
+                                                 ThreeDAPIType requester) {
   bool blocked = Are3DAPIsBlockedAtTime(url, base::Time::Now()) !=
       GpuDataManagerImpl::DOMAIN_BLOCK_STATUS_NOT_BLOCKED;
-  if (blocked) {
+  if (blocked && owner_) {
+    // Unretained is ok, because it's posted to UI thread, the thread
+    // where the singleton GpuDataManagerImpl lives until the end.
     BrowserThread::PostTask(
         BrowserThread::UI, FROM_HERE,
         base::Bind(&GpuDataManagerImpl::Notify3DAPIBlocked,
-                   base::Unretained(this), url, render_process_id,
+                   base::Unretained(owner_), url, render_process_id,
                    render_view_id, requester));
   }
 
   return blocked;
 }
 
-void GpuDataManagerImpl::DisableDomainBlockingFor3DAPIsForTesting() {
+void GpuDataManagerImplPrivate::DisableDomainBlockingFor3DAPIsForTesting() {
   domain_blocking_enabled_ = false;
 }
 
-GpuDataManagerImpl::GpuDataManagerImpl()
+// static
+GpuDataManagerImplPrivate* GpuDataManagerImplPrivate::Create(
+    GpuDataManagerImpl* owner) {
+  GpuDataManagerImplPrivate* rt = new GpuDataManagerImplPrivate();
+  rt->owner_ = owner;
+  return rt;
+}
+
+GpuDataManagerImplPrivate::GpuDataManagerImplPrivate()
     : complete_gpu_info_already_requested_(false),
       gpu_switching_(GPU_SWITCHING_OPTION_AUTOMATIC),
       observer_list_(new GpuDataManagerObserverList),
@@ -725,7 +700,8 @@
       card_blacklisted_(false),
       update_histograms_(true),
       window_count_(0),
-      domain_blocking_enabled_(true) {
+      domain_blocking_enabled_(true),
+      owner_(NULL) {
   CommandLine* command_line = CommandLine::ForCurrentProcess();
   if (command_line->HasSwitch(switches::kDisableAcceleratedCompositing)) {
     command_line->AppendSwitch(switches::kDisableAccelerated2dCanvas);
@@ -746,13 +722,13 @@
 #endif  // OS_MACOSX
 }
 
-GpuDataManagerImpl::~GpuDataManagerImpl() {
+GpuDataManagerImplPrivate::~GpuDataManagerImplPrivate() {
 #if defined(OS_MACOSX)
   CGDisplayRemoveReconfigurationCallback(DisplayReconfigCallback, this);
 #endif
 }
 
-void GpuDataManagerImpl::InitializeImpl(
+void GpuDataManagerImplPrivate::InitializeImpl(
     const std::string& gpu_blacklist_json,
     const std::string& gpu_switching_list_json,
     const std::string& gpu_driver_bug_list_json,
@@ -780,16 +756,13 @@
         GpuControlList::kCurrentOsOnly);
   }
 
-  {
-    base::AutoLock auto_lock(gpu_info_lock_);
-    gpu_info_ = gpu_info;
-  }
+  gpu_info_ = gpu_info;
   UpdateGpuInfo(gpu_info);
   UpdateGpuSwitchingManager(gpu_info);
   UpdatePreliminaryBlacklistedFeatures();
 }
 
-void GpuDataManagerImpl::UpdateBlacklistedFeatures(
+void GpuDataManagerImplPrivate::UpdateBlacklistedFeatures(
     const std::set<int>& features) {
   CommandLine* command_line = CommandLine::ForCurrentProcess();
   blacklisted_features_ = features;
@@ -808,11 +781,12 @@
   EnableSwiftShaderIfNecessary();
 }
 
-void GpuDataManagerImpl::UpdatePreliminaryBlacklistedFeatures() {
+void GpuDataManagerImplPrivate::UpdatePreliminaryBlacklistedFeatures() {
   preliminary_blacklisted_features_ = blacklisted_features_;
 }
 
-void GpuDataManagerImpl::UpdateGpuSwitchingManager(const GPUInfo& gpu_info) {
+void GpuDataManagerImplPrivate::UpdateGpuSwitchingManager(
+    const GPUInfo& gpu_info) {
   ui::GpuSwitchingManager::GetInstance()->SetGpuCount(
       gpu_info.secondary_gpus.size() + 1);
 
@@ -831,11 +805,11 @@
   }
 }
 
-void GpuDataManagerImpl::NotifyGpuInfoUpdate() {
+void GpuDataManagerImplPrivate::NotifyGpuInfoUpdate() {
   observer_list_->Notify(&GpuDataManagerObserver::OnGpuInfoUpdate);
 }
 
-void GpuDataManagerImpl::EnableSwiftShaderIfNecessary() {
+void GpuDataManagerImplPrivate::EnableSwiftShaderIfNecessary() {
   if (!GpuAccessAllowed(NULL) ||
       blacklisted_features_.count(GPU_FEATURE_TYPE_WEBGL)) {
     if (!swiftshader_path_.empty() &&
@@ -845,7 +819,8 @@
   }
 }
 
-std::string GpuDataManagerImpl::GetDomainFromURL(const GURL& url) const {
+std::string GpuDataManagerImplPrivate::GetDomainFromURL(
+    const GURL& url) const {
   // For the moment, we just use the host, or its IP address, as the
   // entry in the set, rather than trying to figure out the top-level
   // domain. This does mean that a.foo.com and b.foo.com will be
@@ -859,43 +834,41 @@
   return url.host();
 }
 
-void GpuDataManagerImpl::BlockDomainFrom3DAPIsAtTime(
-    const GURL& url, DomainGuilt guilt, base::Time at_time) {
+void GpuDataManagerImplPrivate::BlockDomainFrom3DAPIsAtTime(
+    const GURL& url,
+    GpuDataManagerImpl::DomainGuilt guilt,
+    base::Time at_time) {
   if (!domain_blocking_enabled_)
     return;
 
   std::string domain = GetDomainFromURL(url);
 
-  base::AutoLock auto_lock(gpu_info_lock_);
   DomainBlockEntry& entry = blocked_domains_[domain];
   entry.last_guilt = guilt;
   timestamps_of_gpu_resets_.push_back(at_time);
 }
 
 GpuDataManagerImpl::DomainBlockStatus
-GpuDataManagerImpl::Are3DAPIsBlockedAtTime(
+GpuDataManagerImplPrivate::Are3DAPIsBlockedAtTime(
     const GURL& url, base::Time at_time) const {
   if (!domain_blocking_enabled_)
-    return DOMAIN_BLOCK_STATUS_NOT_BLOCKED;
+    return GpuDataManagerImpl::DOMAIN_BLOCK_STATUS_NOT_BLOCKED;
 
   // Note: adjusting the policies in this code will almost certainly
   // require adjusting the associated unit tests.
   std::string domain = GetDomainFromURL(url);
 
-  base::AutoLock auto_lock(gpu_info_lock_);
-  {
-    DomainBlockMap::const_iterator iter = blocked_domains_.find(domain);
-    if (iter != blocked_domains_.end()) {
-      // Err on the side of caution, and assume that if a particular
-      // domain shows up in the block map, it's there for a good
-      // reason and don't let its presence there automatically expire.
+  DomainBlockMap::const_iterator iter = blocked_domains_.find(domain);
+  if (iter != blocked_domains_.end()) {
+    // Err on the side of caution, and assume that if a particular
+    // domain shows up in the block map, it's there for a good
+    // reason and don't let its presence there automatically expire.
 
-      UMA_HISTOGRAM_ENUMERATION("GPU.BlockStatusForClient3DAPIs",
-                                BLOCK_STATUS_SPECIFIC_DOMAIN_BLOCKED,
-                                BLOCK_STATUS_MAX);
+    UMA_HISTOGRAM_ENUMERATION("GPU.BlockStatusForClient3DAPIs",
+                              BLOCK_STATUS_SPECIFIC_DOMAIN_BLOCKED,
+                              BLOCK_STATUS_MAX);
 
-      return DOMAIN_BLOCK_STATUS_BLOCKED;
-    }
+    return GpuDataManagerImpl::DOMAIN_BLOCK_STATUS_BLOCKED;
   }
 
   // Look at the timestamps of the recent GPU resets to see if there are
@@ -905,47 +878,51 @@
   //
   // TODO(kbr): make this pay attention to the TDR thresholds in the
   // Windows registry, but make sure it continues to be testable.
-  std::list<base::Time>::iterator iter = timestamps_of_gpu_resets_.begin();
-  int num_resets_within_timeframe = 0;
-  while (iter != timestamps_of_gpu_resets_.end()) {
-    base::Time time = *iter;
-    base::TimeDelta delta_t = at_time - time;
+  {

[Ken Russell (switch to Gerrit), 2013/05/10 23:59:19]

This block doesn't seem necessary.

+    std::list<base::Time>::iterator iter = timestamps_of_gpu_resets_.begin();
+    int num_resets_within_timeframe = 0;
+    while (iter != timestamps_of_gpu_resets_.end()) {
+      base::Time time = *iter;
+      base::TimeDelta delta_t = at_time - time;
 
-    // If this entry has "expired", just remove it.
-    if (delta_t.InMilliseconds() > kBlockAllDomainsMs) {
-      iter = timestamps_of_gpu_resets_.erase(iter);
-      continue;
+      // If this entry has "expired", just remove it.
+      if (delta_t.InMilliseconds() > kBlockAllDomainsMs) {
+        iter = timestamps_of_gpu_resets_.erase(iter);
+        continue;
+      }
+
+      ++num_resets_within_timeframe;
+      ++iter;
     }
 
-    ++num_resets_within_timeframe;
-    ++iter;
-  }
+    if (num_resets_within_timeframe >= kNumResetsWithinDuration) {
+      UMA_HISTOGRAM_ENUMERATION("GPU.BlockStatusForClient3DAPIs",
+                                BLOCK_STATUS_ALL_DOMAINS_BLOCKED,
+                                BLOCK_STATUS_MAX);
 
-  if (num_resets_within_timeframe >= kNumResetsWithinDuration) {
-    UMA_HISTOGRAM_ENUMERATION("GPU.BlockStatusForClient3DAPIs",
-                              BLOCK_STATUS_ALL_DOMAINS_BLOCKED,
-                              BLOCK_STATUS_MAX);
-
-    return DOMAIN_BLOCK_STATUS_ALL_DOMAINS_BLOCKED;
+      return GpuDataManagerImpl::DOMAIN_BLOCK_STATUS_ALL_DOMAINS_BLOCKED;
+    }
   }
 
   UMA_HISTOGRAM_ENUMERATION("GPU.BlockStatusForClient3DAPIs",
                             BLOCK_STATUS_NOT_BLOCKED,
                             BLOCK_STATUS_MAX);
 
-  return DOMAIN_BLOCK_STATUS_NOT_BLOCKED;
+  return GpuDataManagerImpl::DOMAIN_BLOCK_STATUS_NOT_BLOCKED;
 }
 
-int64 GpuDataManagerImpl::GetBlockAllDomainsDurationInMs() const {
+int64 GpuDataManagerImplPrivate::GetBlockAllDomainsDurationInMs() const {
   return kBlockAllDomainsMs;
 }
 
-void GpuDataManagerImpl::Notify3DAPIBlocked(const GURL& url,
-                                            int render_process_id,
-                                            int render_view_id,
-                                            ThreeDAPIType requester) {
+void GpuDataManagerImplPrivate::Notify3DAPIBlocked(const GURL& url,
+                                                   int render_process_id,
+                                                   int render_view_id,
+                                                   ThreeDAPIType requester) {
+  GpuDataManagerImpl::UnlockedSession session(owner_);
   observer_list_->Notify(&GpuDataManagerObserver::DidBlock3DAPIs,
                          url, render_process_id, render_view_id, requester);
 }
 
 }  // namespace content
+