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

content/browser/gpu/gpu_data_manager_impl_private.h

Index: content/browser/gpu/gpu_data_manager_impl_private.h
===================================================================
--- content/browser/gpu/gpu_data_manager_impl_private.h	(revision 199087)
+++ content/browser/gpu/gpu_data_manager_impl_private.h	(working copy)
@@ -1,9 +1,9 @@
-// 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_
+#ifndef CONTENT_BROWSER_GPU_GPU_DATA_MANAGER_IMPL_PRIVATE_H_
+#define CONTENT_BROWSER_GPU_GPU_DATA_MANAGER_IMPL_PRIVATE_H_
 
 #include <list>
 #include <map>
@@ -11,117 +11,62 @@
 #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/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_data_manager_impl.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"
-#include "content/public/common/gpu_switching_option.h"
-#include "content/public/common/three_d_api_types.h"
 
-class CommandLine;
-class GURL;
-struct WebPreferences;
-
 namespace content {
 
-class CONTENT_EXPORT GpuDataManagerImpl
-    : public NON_EXPORTED_BASE(GpuDataManager) {
+class CONTENT_EXPORT GpuDataManagerImplPrivate {

[Ken Russell (switch to Gerrit), 2013/05/09 20:21:16]

Does this class really need to be CONTENT_EXPORT?

Also, is there any way to guard against accidentally subclassing it?

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

Removed.
Make constructor private and add a Create() function.

  public:
-  // Indicates the guilt level of a domain which caused a GPU reset.
-  // If a domain is 100% known to be guilty of resetting the GPU, then
-  // it will generally not cause other domains' use of 3D APIs to be
-  // blocked, unless system stability would be compromised.
-  enum DomainGuilt {
-    DOMAIN_GUILT_KNOWN,
-    DOMAIN_GUILT_UNKNOWN
-  };
-
-  // Indicates the reason that access to a given client API (like
-  // WebGL or Pepper 3D) was blocked or not. This state is distinct
-  // from blacklisting of an entire feature.
-  enum DomainBlockStatus {
-    DOMAIN_BLOCK_STATUS_BLOCKED,
-    DOMAIN_BLOCK_STATUS_ALL_DOMAINS_BLOCKED,
-    DOMAIN_BLOCK_STATUS_NOT_BLOCKED
-  };
-
-  // Getter for the singleton. This will return NULL on failure.
-  static GpuDataManagerImpl* GetInstance();
-
-  // GpuDataManager implementation.
-  virtual void InitializeForTesting(
+  void InitializeForTesting(
       const std::string& gpu_blacklist_json,
-      const GPUInfo& gpu_info) OVERRIDE;
-  virtual bool IsFeatureBlacklisted(int feature) const OVERRIDE;
-  virtual GPUInfo GetGPUInfo() const OVERRIDE;
-  virtual void GetGpuProcessHandles(
-      const GetGpuProcessHandlesCallback& callback) const OVERRIDE;
-  virtual bool GpuAccessAllowed(std::string* reason) const OVERRIDE;
-  virtual void RequestCompleteGpuInfoIfNeeded() OVERRIDE;
-  virtual bool IsCompleteGpuInfoAvailable() const OVERRIDE;
-  virtual void RequestVideoMemoryUsageStatsUpdate() const OVERRIDE;
-  virtual bool ShouldUseSwiftShader() const OVERRIDE;
-  virtual void RegisterSwiftShaderPath(const base::FilePath& path) OVERRIDE;
-  virtual void AddObserver(GpuDataManagerObserver* observer) OVERRIDE;
-  virtual void RemoveObserver(GpuDataManagerObserver* observer) OVERRIDE;
-  virtual void UnblockDomainFrom3DAPIs(const GURL& url) OVERRIDE;
-  virtual void DisableGpuWatchdog() OVERRIDE;
-  virtual void SetGLStrings(const std::string& gl_vendor,
-                            const std::string& gl_renderer,
-                            const std::string& gl_version) OVERRIDE;
-  virtual void GetGLStrings(std::string* gl_vendor,
-                            std::string* gl_renderer,
-                            std::string* gl_version) OVERRIDE;
-  virtual void DisableHardwareAcceleration() OVERRIDE;
+      const GPUInfo& gpu_info);
+  bool IsFeatureBlacklisted(int feature) const;
+  GPUInfo GetGPUInfo() const;
+  void GetGpuProcessHandles(
+      const GpuDataManager::GetGpuProcessHandlesCallback& callback) const;
+  bool GpuAccessAllowed(std::string* reason) const;
+  void RequestCompleteGpuInfoIfNeeded();
+  bool IsCompleteGpuInfoAvailable() const;
+  void RequestVideoMemoryUsageStatsUpdate() const;
+  bool ShouldUseSwiftShader() const;
+  void RegisterSwiftShaderPath(const base::FilePath& path);
+  void AddObserver(GpuDataManagerObserver* observer);
+  void RemoveObserver(GpuDataManagerObserver* observer);
+  void UnblockDomainFrom3DAPIs(const GURL& url);
+  void DisableGpuWatchdog();
+  void SetGLStrings(const std::string& gl_vendor,
+                    const std::string& gl_renderer,
+                    const std::string& gl_version);
+  void GetGLStrings(std::string* gl_vendor,
+                    std::string* gl_renderer,
+                    std::string* gl_version);
+  void DisableHardwareAcceleration();
 
-  // This collects preliminary GPU info, load GpuBlacklist, and compute the
-  // preliminary blacklisted features; it should only be called at browser
-  // startup time in UI thread before the IO restriction is turned on.
   void Initialize();
 
-  // Only update if the current GPUInfo is not finalized.  If blacklist is
-  // loaded, run through blacklist and update blacklisted features.
   void UpdateGpuInfo(const GPUInfo& gpu_info);
 
   void UpdateVideoMemoryUsageStats(
       const GPUVideoMemoryUsageStats& video_memory_usage_stats);
 
-  // Insert disable-feature switches corresponding to preliminary gpu feature
-  // flags into the renderer process command line.
   void AppendRendererCommandLine(CommandLine* command_line) const;
 
-  // Insert switches into gpu process command line: kUseGL,
-  // kDisableGLMultisampling.
   void AppendGpuCommandLine(CommandLine* command_line) const;
 
-  // Insert switches into plugin process command line:
-  // kDisableCoreAnimationPlugins.
   void AppendPluginCommandLine(CommandLine* command_line) const;
 
-  // Update WebPreferences for renderer based on blacklisting decisions.
   void UpdateRendererWebPrefs(WebPreferences* prefs) const;
 
   GpuSwitchingOption GetGpuSwitchingOption() const;
 
   std::string GetBlacklistVersion() const;
 
-  // Returns the reasons for the latest run of blacklisting decisions.
-  // For the structure of returned value, see documentation for
-  // GpuBlacklist::GetBlacklistedReasons().
-  // Caller is responsible to release the returned value.
   base::ListValue* GetBlacklistReasons() const;
 
   void AddLogMessage(int level,
@@ -130,11 +75,8 @@
 
   void ProcessCrashed(base::TerminationStatus exit_code);
 
-  // Returns a new copy of the ListValue.  Caller is responsible to release
-  // the returned value.
   base::ListValue* GetLogMessages() const;
 
-  // Called when switching gpu.
   void HandleGpuSwitch();
 
 #if defined(OS_WIN)
@@ -143,47 +85,27 @@
   bool IsUsingAcceleratedSurface() const;
 #endif
 
-  // Maintenance of domains requiring explicit user permission before
-  // using client-facing 3D APIs (WebGL, Pepper 3D), either because
-  // the domain has caused the GPU to reset, or because too many GPU
-  // resets have been observed globally recently, and system stability
-  // might be compromised.
-  //
-  // The given URL may be a partial URL (including at least the host)
-  // or a full URL to a page.
-  //
-  // Note that the unblocking API must be part of the content API
-  // because it is called from Chrome side code.
-  void BlockDomainFrom3DAPIs(const GURL& url, DomainGuilt guilt);
+  void BlockDomainFrom3DAPIs(
+      const GURL& url, GpuDataManagerImpl::DomainGuilt guilt);
   bool Are3DAPIsBlocked(const GURL& url,
                         int render_process_id,
                         int render_view_id,
                         ThreeDAPIType requester);
 
-  // Disables domain blocking for 3D APIs. For use only in tests.
   void DisableDomainBlockingFor3DAPIsForTesting();
 
-  // Get number of features being blacklisted.
   size_t GetBlacklistedFeatureCount() const;
 
   typedef base::Callback<void()> GpuSwitchCallback;
 
-  // Add and remove gpu switch callback.
   void AddGpuSwitchCallback(const GpuSwitchCallback& callback);
   void RemoveGpuSwitchCallback(const GpuSwitchCallback& callback);
 
- private:
-  struct DomainBlockEntry {
-    DomainGuilt last_guilt;
-  };
+  GpuDataManagerImplPrivate();
+  virtual ~GpuDataManagerImplPrivate();
 
-  typedef std::map<std::string, DomainBlockEntry> DomainBlockMap;
-
-  typedef ObserverListThreadSafe<GpuDataManagerObserver>
-      GpuDataManagerObserverList;
-
+ private:
   friend class GpuDataManagerImplTest;
-  friend struct DefaultSingletonTraits<GpuDataManagerImpl>;
 
   FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest, GpuSideBlacklisting);
   FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest, GpuSideExceptions);
@@ -209,9 +131,15 @@
   FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest, GpuDriverBugListSingle);
   FRIEND_TEST_ALL_PREFIXES(GpuDataManagerImplTest, GpuDriverBugListMultiple);
 
-  GpuDataManagerImpl();
-  virtual ~GpuDataManagerImpl();
+  struct DomainBlockEntry {
+    GpuDataManagerImpl::DomainGuilt last_guilt;
+  };
 
+  typedef std::map<std::string, DomainBlockEntry> DomainBlockMap;
+
+  typedef ObserverListThreadSafe<GpuDataManagerObserver>
+      GpuDataManagerObserverList;
+
   void InitializeImpl(const std::string& gpu_blacklist_json,
                       const std::string& gpu_switching_list_json,
                       const std::string& gpu_driver_bug_list_json,
@@ -238,9 +166,10 @@
 
   // 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(
+  void BlockDomainFrom3DAPIsAtTime(const GURL& url,
+                                   GpuDataManagerImpl::DomainGuilt guilt,
+                                   base::Time at_time);
+  GpuDataManagerImpl::DomainBlockStatus Are3DAPIsBlockedAtTime(
       const GURL& url, base::Time at_time) const;
   int64 GetBlockAllDomainsDurationInMs() const;
 
@@ -259,7 +188,6 @@
   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_;
@@ -268,7 +196,6 @@
   const scoped_refptr<GpuDataManagerObserverList> observer_list_;
 
   ListValue log_messages_;
-  mutable base::Lock log_messages_lock_;
 
   bool use_swiftshader_;
 
@@ -291,9 +218,10 @@
 
   std::vector<GpuSwitchCallback> gpu_switch_callbacks_;
 
-  DISALLOW_COPY_AND_ASSIGN(GpuDataManagerImpl);
+  DISALLOW_COPY_AND_ASSIGN(GpuDataManagerImplPrivate);
 };
 
 }  // namespace content
 
-#endif  // CONTENT_BROWSER_GPU_GPU_DATA_MANAGER_IMPL_H_
+#endif  // CONTENT_BROWSER_GPU_GPU_DATA_MANAGER_IMPL_PRIVATE_H_
+