Change GpuDataManager to use Observer notifications rather than callbacks

content/browser/gpu/gpu_data_manager.h

Index: content/browser/gpu/gpu_data_manager.h
===================================================================
--- content/browser/gpu/gpu_data_manager.h	(revision 107010)
+++ content/browser/gpu/gpu_data_manager.h	(working copy)
@@ -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 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,13 @@
   // 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.
+  void AddObserver(Observer* observer);

[James Hawkins, 2011/10/26 00:21:29]

|observer| must not be NULL?  Here and RemoveObserver().

[csilv, 2011/10/26 01:35:31]

Wouldn't observer != NULL be assumed?  ;-)  Done.

[James Hawkins, 2011/10/26 01:45:28]

I understand where you're coming from, but I've recently gotten into the habit
of always documenting pointer variable requirements. For example, what if I'm
trying to use this in a situation where a pointer passed in may be NULL?  What
would happen in that case?  Documenting it just removes any possible ambiguity.

 
-  // 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

[James Hawkins, 2011/10/26 00:21:29]

Single space between comments.

[csilv, 2011/10/26 01:35:31]

Done.

+  // on the same thread on which AddObserver() was called.
+  void RemoveObserver(Observer* observer);
 
   // Inserting disable-feature switches into renderer process command-line
   // in correspondance to preliminary gpu feature flags.
@@ -157,6 +169,9 @@
     std::string use_gl_;
   };
 
+  typedef ObserverListThreadSafe<GpuDataManager::Observer>

[James Hawkins, 2011/10/26 00:21:29]

Document var.

[James Hawkins, 2011/10/26 01:32:16]

Oops, nm.

+      GpuDataManagerObserverList;
+
   friend struct DefaultSingletonTraits<GpuDataManager>;
 
   GpuDataManager();
@@ -172,8 +187,8 @@
   // 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();
@@ -199,8 +214,8 @@
 
   scoped_ptr<GpuBlacklist> gpu_blacklist_;
 
-  // Map of callbacks.
-  std::set<Callback0::Type*> gpu_info_update_callbacks_;
+  // Observers.
+  const scoped_refptr<GpuDataManagerObserverList> observer_list_;

[James Hawkins, 2011/10/26 00:21:29]

Why const?

[csilv, 2011/10/26 01:35:31]

observer_list_ is initialized in the constructor and we don't need or use any
non-const methods.  This seems like the correct thing to do (and it's common in
our codebase.)  Any reason why I wouldn't want to use const here?

[James Hawkins, 2011/10/26 01:45:28]

No, was just curious.

 
   ListValue log_messages_;