Change GpuDataManager to use Observer notifications rather than callbacks. This eliminates

use of legacy callbacks and generally simplifies the implementation. Also migrate one instance
of NewRunnableMethod to base::Bind().

BUG=98478
Review URL: http://codereview.chromium.org/8390018

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@108189 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/ui/webui/flash_ui.cc

@@ -10,7 +10,6 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
-#include "base/callback_old.h"
 #include "base/i18n/time_formatting.h"
 #include "base/memory/weak_ptr.h"
 #include "base/string_number_conversions.h"
@@ -66,10 +65,11 @@ const int kTimeout = 8 * 1000;  // 8 seconds.
 
 // The handler for JavaScript messages for the about:flags page.
 class FlashDOMHandler : public WebUIMessageHandler,
-                        public CrashUploadList::Delegate {
+                        public CrashUploadList::Delegate,
+                        public GpuDataManager::Observer {
  public:
   FlashDOMHandler();
-  virtual ~FlashDOMHandler() {}
+  virtual ~FlashDOMHandler();
 
   // WebUIMessageHandler implementation.
   virtual void RegisterMessages() OVERRIDE;
@@ -77,12 +77,12 @@ class FlashDOMHandler : public WebUIMessageHandler,
   // CrashUploadList::Delegate implementation.
   virtual void OnCrashListAvailable() OVERRIDE;
 
+  // GpuDataManager::Observer implementation.
+  virtual void OnGpuInfoUpdate() OVERRIDE;
+
   // Callback for the "requestFlashInfo" message.
   void HandleRequestFlashInfo(const ListValue* args);
 
-  // Callback for the GPU information update.
-  void OnGpuInfoUpdate();
-
   // Callback for the Flash plugin information.
   void OnGotPlugins(const std::vector<webkit::WebPluginInfo>& plugins);
 
@@ -103,7 +103,6 @@ class FlashDOMHandler : public WebUIMessageHandler,
 
   // GPU variables.
   GpuDataManager* gpu_data_manager_;
-  Callback0::Type* gpu_info_update_callback_;
 
   // Crash list.
   scoped_refptr<CrashUploadList> upload_list_;
@@ -135,9 +134,7 @@ FlashDOMHandler::FlashDOMHandler()
 
   // Watch for changes in GPUInfo.
   gpu_data_manager_ = GpuDataManager::GetInstance();
-  gpu_info_update_callback_ =
-      NewCallback(this, &FlashDOMHandler::OnGpuInfoUpdate);
-  gpu_data_manager_->AddGpuInfoUpdateCallback(gpu_info_update_callback_);
+  gpu_data_manager_->AddObserver(this);
 
   // Tell GpuDataManager it should have full GpuInfo. If the
   // GPU process has not run yet, this will trigger its launch.
@@ -157,6 +154,10 @@ FlashDOMHandler::FlashDOMHandler()
                  this, &FlashDOMHandler::OnTimeout);
 }
 
+FlashDOMHandler::~FlashDOMHandler() {
+  gpu_data_manager_->RemoveObserver(this);
+}
+
 void FlashDOMHandler::RegisterMessages() {
   web_ui_->RegisterMessageCallback("requestFlashInfo",
       base::Bind(&FlashDOMHandler::HandleRequestFlashInfo,

chrome/browser/ui/webui/gpu_internals_ui.cc

@@ -8,7 +8,6 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
-#include "base/callback_old.h"
 #include "base/command_line.h"
 #include "base/string_number_conversions.h"
 #include "base/stringprintf.h"
@@ -44,7 +43,8 @@ ChromeWebUIDataSource* CreateGpuHTMLSource() {
 // this class's methods are expected to run on the UI thread.
 class GpuMessageHandler
     : public WebUIMessageHandler,
-      public base::SupportsWeakPtr<GpuMessageHandler> {
+      public base::SupportsWeakPtr<GpuMessageHandler>,
+      public GpuDataManager::Observer {
  public:
   GpuMessageHandler();
   virtual ~GpuMessageHandler();
@@ -53,6 +53,9 @@ class GpuMessageHandler
   virtual WebUIMessageHandler* Attach(WebUI* web_ui) OVERRIDE;
   virtual void RegisterMessages() OVERRIDE;
 
+  // GpuDataManager::Observer implementation.
+  virtual void OnGpuInfoUpdate() OVERRIDE;
+
   // Messages
   void OnBrowserBridgeInitialized(const ListValue* list);
   void OnCallAsync(const ListValue* list);
@@ -61,9 +64,6 @@ class GpuMessageHandler
   Value* OnRequestClientInfo(const ListValue* list);
   Value* OnRequestLogMessages(const ListValue* list);
 
-  // Callbacks.
-  void OnGpuInfoUpdate();
-
   // Executes the javascript function |function_name| in the renderer, passing
   // it the argument |value|.
   void CallJavascriptFunction(const std::wstring& function_name,
@@ -73,8 +73,6 @@ class GpuMessageHandler
   // Cache the Singleton for efficiency.
   GpuDataManager* gpu_data_manager_;
 
-  Callback0::Type* gpu_info_update_callback_;
-
   DISALLOW_COPY_AND_ASSIGN(GpuMessageHandler);
 };
 
@@ -84,17 +82,13 @@ class GpuMessageHandler
 //
 ////////////////////////////////////////////////////////////////////////////////
 
-GpuMessageHandler::GpuMessageHandler()
-  : gpu_info_update_callback_(NULL) {
+GpuMessageHandler::GpuMessageHandler() {
   gpu_data_manager_ = GpuDataManager::GetInstance();
   DCHECK(gpu_data_manager_);
 }
 
 GpuMessageHandler::~GpuMessageHandler() {
-  if (gpu_info_update_callback_) {
-    gpu_data_manager_->RemoveGpuInfoUpdateCallback(gpu_info_update_callback_);
-    delete gpu_info_update_callback_;
-  }
+  gpu_data_manager_->RemoveObserver(this);
 }
 
 WebUIMessageHandler* GpuMessageHandler::Attach(WebUI* web_ui) {
@@ -165,12 +159,8 @@ void GpuMessageHandler::OnCallAsync(const ListValue* args) {
 void GpuMessageHandler::OnBrowserBridgeInitialized(const ListValue* args) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  DCHECK(!gpu_info_update_callback_);
-
   // Watch for changes in GPUInfo
-  gpu_info_update_callback_ =
-      NewCallback(this, &GpuMessageHandler::OnGpuInfoUpdate);
-  gpu_data_manager_->AddGpuInfoUpdateCallback(gpu_info_update_callback_);
+  gpu_data_manager_->AddObserver(this);
 
   // Tell GpuDataManager it should have full GpuInfo. If the
   // Gpu process has not run yet, this will trigger its launch.

chrome/browser/ui/webui/tracing_ui.cc

@@ -8,7 +8,6 @@
 
 #include "base/bind.h"
 #include "base/bind_helpers.h"
-#include "base/callback_old.h"
 #include "base/command_line.h"
 #include "base/file_util.h"
 #include "base/memory/scoped_ptr.h"
@@ -48,7 +47,8 @@ class TracingMessageHandler
     : public WebUIMessageHandler,
       public SelectFileDialog::Listener,
       public base::SupportsWeakPtr<TracingMessageHandler>,
-      public TraceSubscriber {
+      public TraceSubscriber,
+      public GpuDataManager::Observer {
  public:
   TracingMessageHandler();
   virtual ~TracingMessageHandler();
@@ -66,6 +66,9 @@ class TracingMessageHandler
   virtual void OnTraceDataCollected(const std::string& trace_fragment);
   virtual void OnTraceBufferPercentFullReply(float percent_full);
 
+  // GpuDataManager::Observer implementation.
+  virtual void OnGpuInfoUpdate() OVERRIDE;
+
   // Messages.
   void OnTracingControllerInitialized(const ListValue* list);
   void OnBeginTracing(const ListValue* list);
@@ -74,9 +77,6 @@ class TracingMessageHandler
   void OnLoadTraceFile(const ListValue* list);
   void OnSaveTraceFile(const ListValue* list);
 
-  // Callbacks.
-  void OnGpuInfoUpdate();
-
   // Callbacks.
   void LoadTraceFileComplete(std::string* file_contents);
   void SaveTraceFileComplete();
@@ -98,9 +98,6 @@ class TracingMessageHandler
   // Cache the Singleton for efficiency.
   GpuDataManager* gpu_data_manager_;
 
-  // Callback called when the GPU info is updated.
-  Callback0::Type* gpu_info_update_callback_;
-
   DISALLOW_COPY_AND_ASSIGN(TracingMessageHandler);
 };
 
@@ -138,17 +135,13 @@ class TaskProxy : public base::RefCountedThreadSafe<TaskProxy> {
 
 TracingMessageHandler::TracingMessageHandler()
   : select_trace_file_dialog_type_(SelectFileDialog::SELECT_NONE),
-    trace_enabled_(false),
-    gpu_info_update_callback_(NULL) {
+    trace_enabled_(false) {
   gpu_data_manager_ = GpuDataManager::GetInstance();
   DCHECK(gpu_data_manager_);
 }
 
 TracingMessageHandler::~TracingMessageHandler() {
-  if (gpu_info_update_callback_) {
-    gpu_data_manager_->RemoveGpuInfoUpdateCallback(gpu_info_update_callback_);
-    delete gpu_info_update_callback_;
-  }
+  gpu_data_manager_->RemoveObserver(this);
 
   if (select_trace_file_dialog_)
     select_trace_file_dialog_->ListenerDestroyed();
@@ -190,12 +183,8 @@ void TracingMessageHandler::OnTracingControllerInitialized(
     const ListValue* args) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
 
-  DCHECK(!gpu_info_update_callback_);
-
   // Watch for changes in GPUInfo
-  gpu_info_update_callback_ =
-      NewCallback(this, &TracingMessageHandler::OnGpuInfoUpdate);
-  gpu_data_manager_->AddGpuInfoUpdateCallback(gpu_info_update_callback_);
+  gpu_data_manager_->AddObserver(this);
 
   // Tell GpuDataManager it should have full GpuInfo. If the
   // Gpu process has not run yet, this will trigger its launch.

content/browser/gpu/gpu_data_manager.cc

@@ -8,6 +8,8 @@
 #include <CoreGraphics/CGDisplayConfiguration.h>
 #endif
 
+#include "base/bind.h"
+#include "base/bind_helpers.h"
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/string_number_conversions.h"
@@ -190,7 +192,8 @@ void GpuDataManager::UserFlags::ApplyPolicies() {
 }
 
 GpuDataManager::GpuDataManager()
-    : complete_gpu_info_already_requested_(false) {
+    : complete_gpu_info_already_requested_(false),
+      observer_list_(new GpuDataManagerObserverList) {
   Initialize();
 }
 
@@ -244,7 +247,7 @@ void GpuDataManager::UpdateGpuInfo(const content::GPUInfo& gpu_info) {
       return;
   }
 
-  RunGpuInfoUpdateCallbacks();
+  NotifyGpuInfoUpdate();
 
   {
     base::AutoLock auto_lock(gpu_info_lock_);
@@ -412,20 +415,12 @@ bool GpuDataManager::GpuAccessAllowed() {
   return (gpu_feature_flags_.flags() & mask) == 0;
 }
 
-void GpuDataManager::AddGpuInfoUpdateCallback(Callback0::Type* callback) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  gpu_info_update_callbacks_.insert(callback);
+void GpuDataManager::AddObserver(Observer* observer) {
+  observer_list_->AddObserver(observer);
 }
 
-bool GpuDataManager::RemoveGpuInfoUpdateCallback(Callback0::Type* callback) {
-  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
-  std::set<Callback0::Type*>::iterator i =
-      gpu_info_update_callbacks_.find(callback);
-  if (i != gpu_info_update_callbacks_.end()) {
-    gpu_info_update_callbacks_.erase(i);
-    return true;
-  }
-  return false;
+void GpuDataManager::RemoveObserver(Observer* observer) {
+  observer_list_->RemoveObserver(observer);
 }
 
 void GpuDataManager::AppendRendererCommandLine(
@@ -572,23 +567,16 @@ DictionaryValue* GpuDataManager::GpuInfoAsDictionaryValue() const {
   return info;
 }
 
-void GpuDataManager::RunGpuInfoUpdateCallbacks() {
-  if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-        NewRunnableMethod(this, &GpuDataManager::RunGpuInfoUpdateCallbacks));
-    return;
-  }
-
-  std::set<Callback0::Type*>::iterator i = gpu_info_update_callbacks_.begin();
-  for (; i != gpu_info_update_callbacks_.end(); ++i) {
-    (*i)->Run();
-  }
+void GpuDataManager::NotifyGpuInfoUpdate() {
+  observer_list_->Notify(&GpuDataManager::Observer::OnGpuInfoUpdate);
 }
 
 void GpuDataManager::UpdateGpuFeatureFlags() {
   if (!BrowserThread::CurrentlyOn(BrowserThread::UI)) {
-    BrowserThread::PostTask(BrowserThread::UI, FROM_HERE,
-        NewRunnableMethod(this, &GpuDataManager::UpdateGpuFeatureFlags));
+    BrowserThread::PostTask(
+        BrowserThread::UI, FROM_HERE,
+        base::Bind(&GpuDataManager::UpdateGpuFeatureFlags,
+                   base::Unretained(this)));
     return;
   }
 
@@ -607,7 +595,7 @@ void GpuDataManager::UpdateGpuFeatureFlags() {
   }
 
   // Notify clients that GpuInfo state has changed
-  RunGpuInfoUpdateCallbacks();
+  NotifyGpuInfoUpdate();
 
   uint32 flags = gpu_feature_flags_.flags();
   uint32 max_entry_id = gpu_blacklist->max_entry_id();

content/browser/gpu/gpu_data_manager.h

@@ -9,9 +9,9 @@
 #include <set>
 #include <string>
 
-#include "base/callback_old.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/singleton.h"
+#include "base/observer_list_threadsafe.h"
 #include "base/synchronization/lock.h"
 #include "base/task.h"
 #include "base/values.h"
@@ -24,6 +24,17 @@ class GpuBlacklist;
 
 class CONTENT_EXPORT GpuDataManager {
  public:
+  // Observers can register themselves via GpuDataManager::AddObserver, and
+  // can un-register with GpuDataManager::RemoveObserver.
+  class Observer {
+   public:
+    // Called for any observers whenever there is a GPU info update.
+    virtual void OnGpuInfoUpdate() = 0;
+
+   protected:
+    virtual ~Observer() {}
+  };
+
   // Getter for the singleton. This will return NULL on failure.
   static GpuDataManager* GetInstance();
 
@@ -82,12 +93,15 @@ class CONTENT_EXPORT GpuDataManager {
   // Can be called on any thread.
   bool GpuAccessAllowed();
 
-  // Add a callback.
-  void AddGpuInfoUpdateCallback(Callback0::Type* callback);
+  // Registers |observer|. The thread on which this is called is the thread
+  // on which |observer| will be called back with notifications. |observer|
+  // must not be NULL.
+  void AddObserver(Observer* observer);
 
-  // Remove a callback.
-  // Returns true if removed, or false if it was not found.
-  bool RemoveGpuInfoUpdateCallback(Callback0::Type* callback);
+  // Unregisters |observer| from receiving notifications. This must be called
+  // on the same thread on which AddObserver() was called. |observer|
+  // must not be NULL.
+  void RemoveObserver(Observer* observer);
 
   // Inserting disable-feature switches into renderer process command-line
   // in correspondance to preliminary gpu feature flags.
@@ -160,6 +174,9 @@ class CONTENT_EXPORT GpuDataManager {
     std::string use_gl_;
   };
 
+  typedef ObserverListThreadSafe<GpuDataManager::Observer>
+      GpuDataManagerObserverList;
+
   friend struct DefaultSingletonTraits<GpuDataManager>;
 
   GpuDataManager();
@@ -175,8 +192,8 @@ class CONTENT_EXPORT GpuDataManager {
   // and compute the flags.
   void UpdateGpuFeatureFlags();
 
-  // Call all callbacks.
-  void RunGpuInfoUpdateCallbacks();
+  // Notify all observers whenever there is a GPU info update.
+  void NotifyGpuInfoUpdate();
 
   // If use-gl switch is osmesa or any, return true.
   bool UseGLIsOSMesaOrAny();
@@ -206,14 +223,12 @@ class CONTENT_EXPORT GpuDataManager {
 
   scoped_ptr<GpuBlacklist> gpu_blacklist_;
 
-  // Map of callbacks.
-  std::set<Callback0::Type*> gpu_info_update_callbacks_;
+  // Observers.
+  const scoped_refptr<GpuDataManagerObserverList> observer_list_;
 
   ListValue log_messages_;
 
   DISALLOW_COPY_AND_ASSIGN(GpuDataManager);
 };
 
-DISABLE_RUNNABLE_METHOD_REFCOUNT(GpuDataManager);
-
 #endif  // CONTENT_BROWSER_GPU_GPU_DATA_MANAGER_H_