Attempting to resolve a race condition with PowerMonitor

base/power_monitor/power_monitor.h

Index: base/power_monitor/power_monitor.h
diff --git a/base/power_monitor/power_monitor.h b/base/power_monitor/power_monitor.h
index 4acb3bf122dba93633a1201631063810ef3e9d5d..d00b7211398e207bbaeaa4a0299fdfe2d1d261cd 100644
--- a/base/power_monitor/power_monitor.h
+++ b/base/power_monitor/power_monitor.h
@@ -19,27 +19,31 @@ class PowerMonitorSource;
 // the change event.
 class BASE_EXPORT PowerMonitor {
  public:
-  // Takes ownership of |source|.
-  explicit PowerMonitor(scoped_ptr<PowerMonitorSource> source);
+  static PowerMonitor* Create(scoped_ptr<PowerMonitorSource> source);

[Ken Russell (switch to Gerrit), 2014/02/28 02:02:42]

I find the pattern here confusing where PowerMonitor is implicitly a singleton,
but there's still a factory method for creating one. This was a problem in the
earlier code too.

Perhaps a better paradigm would be to have static Initialize and Shutdown
methods, replacing Create and the destructor. Then the constructor and
destructor can both be private.

The class could expose just static AddObserverThreadSafe,
RemoveObserverThreadSave, and IsOnBatteryPower methods, and do all locking and
fetching of the global instance internally. They could call private instance
methods.

Would that be feasible?

[jam, 2014/02/28 17:05:18]

+1

   ~PowerMonitor();
 
   // Get the process-wide PowerMonitor (if not present, returns NULL).
   static PowerMonitor* Get();
 
-  // Add and remove an observer.
-  // Can be called from any thread.
-  // Must not be called from within a notification callback.
-  void AddObserver(PowerObserver* observer);
-  void RemoveObserver(PowerObserver* observer);
+  static bool AddObserverThreadSafe(PowerObserver* observer);
+  static bool RemoveObserverThreadSafe(PowerObserver* observer);
 
   // Is the computer currently on battery power.
   bool IsOnBatteryPower();
 
  private:
+  // Takes ownership of |source|.
+  explicit PowerMonitor(scoped_ptr<PowerMonitorSource> source);
   friend class PowerMonitorSource;
 
   PowerMonitorSource* Source();
 
+  // Add and remove an observer.
+  // Can be called from any thread.
+  // Must not be called from within a notification callback.
+  void AddObserver(PowerObserver* observer);
+  void RemoveObserver(PowerObserver* observer);
+
   void NotifyPowerStateChange(bool battery_in_use);
   void NotifySuspend();
   void NotifyResume();