Change GpuDataManager to use Observer notifications rather than callbacks

content/browser/gpu/gpu_data_manager.h

 // Copyright (c) 2011 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_H_
 #define CONTENT_BROWSER_GPU_GPU_DATA_MANAGER_H_
 #pragma once
 
 #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"
 #include "content/common/content_export.h"
 #include "content/common/gpu/gpu_feature_flags.h"
 #include "content/public/common/gpu_info.h"
 
 class CommandLine;
 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();
 
   // Requests complete GPUinfo if it has not already been requested
   void RequestCompleteGpuInfoIfNeeded();
 
   // Only update if the current GPUInfo is not finalized.
   void UpdateGpuInfo(const content::GPUInfo& gpu_info);
 
   const content::GPUInfo& gpu_info() const;
@@ skipping 38 matching lines @@
   GpuFeatureFlags GetGpuFeatureFlags();
 
   // This indicator might change because we could collect more GPU info or
   // because the GPU blacklist could be updated.
   // If this returns false, any further GPU access, including launching GPU
   // process, establish GPU channel, and GPU info collection, should be
   // blocked.
   // 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.
   void AppendRendererCommandLine(CommandLine* command_line);
 
   // Inserting switches into gpu process command-line: kUseGL,
   // kDisableGLMultisampling.
   void AppendGpuCommandLine(CommandLine* command_line);
 
   // Gives ownership of the built-in blacklist.  This is always called on the
@@ skipping 49 matching lines @@
     bool disable_accelerated_compositing_;
     bool disable_accelerated_layers_;
     bool disable_experimental_webgl_;
     bool disable_gl_multisampling_;
 
     bool ignore_gpu_blacklist_;
 
     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();
   virtual ~GpuDataManager();
 
   void Initialize();
 
   // Check if we should go ahead and use gpu blacklist.
   // If not, return NULL; otherwise, update and return the current list.
   GpuBlacklist* GetGpuBlacklist() const;
 
   // If flags hasn't been set and GPUInfo is available, run through blacklist
   // 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();
 
   // Merges the second GPUInfo object with the first.
   // If it's the same GPU, i.e., device id and vendor id are the same, then
   // copy over the fields that are not set yet and ignore the rest.
   // If it's a different GPU, then reset and copy over everything.
   // Return true if something changes that may affect blacklisting; currently
   // they are device_id, vendor_id, driver_vendor, driver_version, driver_date,
   // and gl_renderer.
   static bool Merge(content::GPUInfo* object, const content::GPUInfo& other);
 
   bool complete_gpu_info_already_requested_;
 
   GpuFeatureFlags gpu_feature_flags_;
   GpuFeatureFlags preliminary_gpu_feature_flags_;
 
   UserFlags user_flags_;
 
   content::GPUInfo gpu_info_;
   mutable base::Lock gpu_info_lock_;
 
   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_;
 
   DISALLOW_COPY_AND_ASSIGN(GpuDataManager);
 };
 
 DISABLE_RUNNABLE_METHOD_REFCOUNT(GpuDataManager);

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

Assuming it's not safe to delete yet, add a TODO to remove.

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

Removed, I overlooked this in my prior CL.

 
 #endif  // CONTENT_BROWSER_GPU_GPU_DATA_MANAGER_H_