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

content/browser/gpu/gpu_data_manager_impl.h

Index: content/browser/gpu/gpu_data_manager_impl.h
===================================================================
--- content/browser/gpu/gpu_data_manager_impl.h	(revision 199087)
+++ content/browser/gpu/gpu_data_manager_impl.h	(working copy)
@@ -1,30 +1,22 @@
-// 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.
 
 #ifndef CONTENT_BROWSER_GPU_GPU_DATA_MANAGER_IMPL_H_
 #define CONTENT_BROWSER_GPU_GPU_DATA_MANAGER_IMPL_H_
 
-#include <list>
-#include <map>
-#include <set>
 #include <string>
-#include <vector>
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/files/file_path.h"
 #include "base/gtest_prod_util.h"
-#include "base/memory/ref_counted.h"
+#include "base/memory/scoped_ptr.h"
 #include "base/memory/singleton.h"
-#include "base/observer_list_threadsafe.h"
 #include "base/process_util.h"
 #include "base/synchronization/lock.h"
 #include "base/time.h"
 #include "base/values.h"
-#include "content/browser/gpu/gpu_blacklist.h"
-#include "content/browser/gpu/gpu_driver_bug_list.h"
-#include "content/browser/gpu/gpu_switching_list.h"
 #include "content/public/browser/gpu_data_manager.h"
 #include "content/public/common/gpu_info.h"
 #include "content/public/common/gpu_memory_stats.h"
@@ -37,6 +29,8 @@
 
 namespace content {
 
+class GpuDataManagerImplPrivate;
+
 class CONTENT_EXPORT GpuDataManagerImpl
     : public NON_EXPORTED_BASE(GpuDataManager) {
  public:
@@ -173,124 +167,14 @@
   void RemoveGpuSwitchCallback(const GpuSwitchCallback& callback);
 
  private:
-  struct DomainBlockEntry {
-    DomainGuilt last_guilt;
-  };
-
-  typedef std::map<std::string, DomainBlockEntry> DomainBlockMap;
-
-  typedef ObserverListThreadSafe<GpuDataManagerObserver>
-      GpuDataManagerObserverList;
-
-  friend class GpuDataManagerImplTest;
   friend struct DefaultSingletonTraits<GpuDataManagerImpl>;
 
-  FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest, GpuSideBlacklisting);
-  FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest, GpuSideExceptions);
-  FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest,
-                           DisableHardwareAcceleration);
-  FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest, SwiftShaderRendering);
-  FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest, SwiftShaderRendering2);
-  FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest, GpuInfoUpdate);
-  FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest,
-                           NoGpuInfoUpdateWithSwiftShader);
-  FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest,
-                           GPUVideoMemoryUsageStatsUpdate);
-  FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest,
-                           BlockAllDomainsFrom3DAPIs);
-  FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest,
-                           UnblockGuiltyDomainFrom3DAPIs);
-  FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest,
-                           UnblockDomainOfUnknownGuiltFrom3DAPIs);
-  FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest,
-                           UnblockOtherDomainFrom3DAPIs);
-  FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest,
-                           UnblockThisDomainFrom3DAPIs);
-  FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest, GpuDriverBugListSingle);
-  FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest, GpuDriverBugListMultiple);
-
   GpuDataManagerImpl();
   virtual ~GpuDataManagerImpl();
 
-  void InitializeImpl(const std::string& gpu_blacklist_json,
-                      const std::string& gpu_switching_list_json,
-                      const std::string& gpu_driver_bug_list_json,
-                      const GPUInfo& gpu_info);
+  scoped_ptr<GpuDataManagerImplPrivate> private_;
+  mutable base::Lock lock_;

[piman, 2013/05/09 20:18:23]

nit: it's nice to put the lock above the members it protects, for 2 reasons:
1- it gives good symmetry with the call sites, where you take the lock
(autolock) before accessing the variable
2- it ensures the lock outlives the member, which can sometimes avoid subtle
bugs.

[Zhenyao Mo, 2013/05/09 23:35:12]

Done.

 
-  void UpdateBlacklistedFeatures(const std::set<int>& features);
-
-  // This should only be called once at initialization time, when preliminary
-  // gpu info is collected.
-  void UpdatePreliminaryBlacklistedFeatures();
-
-  // Update the GPU switching status.
-  // This should only be called once at initialization time.
-  void UpdateGpuSwitchingManager(const GPUInfo& gpu_info);
-
-  // Notify all observers whenever there is a GPU info update.
-  void NotifyGpuInfoUpdate();
-
-  // Try to switch to SwiftShader rendering, if possible and necessary.
-  void EnableSwiftShaderIfNecessary();
-
-  // Helper to extract the domain from a given URL.
-  std::string GetDomainFromURL(const GURL& url) const;
-
-  // Implementation functions for blocking of 3D graphics APIs, used
-  // for unit testing.
-  void BlockDomainFrom3DAPIsAtTime(
-      const GURL& url, DomainGuilt guilt, base::Time at_time);
-  DomainBlockStatus Are3DAPIsBlockedAtTime(
-      const GURL& url, base::Time at_time) const;
-  int64 GetBlockAllDomainsDurationInMs() const;
-
-  void Notify3DAPIBlocked(const GURL& url,
-                          int render_process_id,
-                          int render_view_id,
-                          ThreeDAPIType requester);
-
-  bool complete_gpu_info_already_requested_;
-
-  std::set<int> blacklisted_features_;
-  std::set<int> preliminary_blacklisted_features_;
-
-  GpuSwitchingOption gpu_switching_;
-
-  std::set<int> gpu_driver_bugs_;
-
-  GPUInfo gpu_info_;
-  mutable base::Lock gpu_info_lock_;
-
-  scoped_ptr<GpuBlacklist> gpu_blacklist_;
-  scoped_ptr<GpuSwitchingList> gpu_switching_list_;
-  scoped_ptr<GpuDriverBugList> gpu_driver_bug_list_;
-
-  const scoped_refptr<GpuDataManagerObserverList> observer_list_;
-
-  ListValue log_messages_;
-  mutable base::Lock log_messages_lock_;
-
-  bool use_swiftshader_;
-
-  base::FilePath swiftshader_path_;
-
-  // Current card force-blacklisted due to GPU crashes, or disabled through
-  // the --disable-gpu commandline switch.
-  bool card_blacklisted_;
-
-  // We disable histogram stuff in testing, especially in unit tests because
-  // they cause random failures.
-  bool update_histograms_;
-
-  // Number of currently open windows, to be used in gpu memory allocation.
-  int window_count_;
-
-  DomainBlockMap blocked_domains_;
-  mutable std::list<base::Time> timestamps_of_gpu_resets_;
-  bool domain_blocking_enabled_;
-
-  std::vector<GpuSwitchCallback> gpu_switch_callbacks_;
-
   DISALLOW_COPY_AND_ASSIGN(GpuDataManagerImpl);
 };