SystemMonitor refactoring: move power state monitor into a separate class called PowerMonitor

base/power_monitor/power_monitor.h

Index: base/power_monitor/power_monitor.h
diff --git a/base/system_monitor/system_monitor.h b/base/power_monitor/power_monitor.h
similarity index 32%
copy from base/system_monitor/system_monitor.h
copy to base/power_monitor/power_monitor.h
index ea95da89ccbe9fb6635b8ae905b6415ac977ca24..86a5a13d2d90552489265142e504b0cad7920916 100644
--- a/base/system_monitor/system_monitor.h
+++ b/base/power_monitor/power_monitor.h
@@ -2,19 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef BASE_SYSTEM_MONITOR_SYSTEM_MONITOR_H_
-#define BASE_SYSTEM_MONITOR_SYSTEM_MONITOR_H_
-
-#include <map>
-#include <string>
-#include <vector>
+#ifndef BASE_POWER_MONITOR_POWER_MONITOR_H_
+#define BASE_POWER_MONITOR_POWER_MONITOR_H_
 
 #include "base/base_export.h"
 #include "base/basictypes.h"
-#include "base/file_path.h"
-#include "base/string16.h"
-#include "base/synchronization/lock.h"
-#include "build/build_config.h"
+#include "base/memory/scoped_ptr.h"
+#include "base/power_monitor/power_observer.h"
+#include "base/observer_list_threadsafe.h"
 
 // Windows HiRes timers drain the battery faster so we need to know the battery
 // status.  This isn't true for other platforms.
@@ -24,26 +19,19 @@
 #undef ENABLE_BATTERY_MONITORING
 #endif  // !OS_WIN
 
-#include "base/observer_list_threadsafe.h"
 #if defined(ENABLE_BATTERY_MONITORING)
 #include "base/timer.h"
 #endif  // defined(ENABLE_BATTERY_MONITORING)
 
-#if defined(OS_MACOSX) && !defined(OS_IOS)
-#include <IOKit/pwr_mgt/IOPMLib.h>
-#include <IOKit/IOMessage.h>
-#endif  // OS_MACOSX && !OS_IOS
-
 #if defined(OS_IOS)
 #include <objc/runtime.h>
 #endif  // OS_IOS
 
 namespace base {
 
-// Class for monitoring various system-related subsystems
-// such as power management, network status, etc.
-// TODO(mbelshe):  Add support beyond just power management.
-class BASE_EXPORT SystemMonitor {
+// A class used to monitor the power state change and notify the observers about
+// the change event.
+class BASE_EXPORT PowerMonitor {
  public:
   // Normalized list of power events.
   enum PowerEvent {
@@ -52,39 +40,51 @@ class BASE_EXPORT SystemMonitor {
     RESUME_EVENT        // The system is being resumed.
   };
 
-  // Type of devices whose change need to be monitored, such as add/remove.
-  enum DeviceType {
-    DEVTYPE_AUDIO_CAPTURE,  // Audio capture device, e.g., microphone.
-    DEVTYPE_VIDEO_CAPTURE,  // Video capture device, e.g., webcam.
-    DEVTYPE_UNKNOWN,  // Other devices.
+  // A helper class to wrap the process of power event in PowerMonitor. The
+  // power event source is allowed to notify the PowerMonitor's observers via
+  // ProcessPowerEvent methods of Signaler instance.
+  //
+  // The reason why not making ProcessPowerEvent as the public method of
+  // PowerMonitor instead is to avoid layering violation for the friendship
+  // declaration of PowerMonitor, e.g. making views::HWNDMessageHandler as a
+  // friend of base::PowerMonitor violates the layering rule.
+  class BASE_EXPORT Signaler {
+   public:
+#if defined(OS_WIN)
+    void ProcessWmPowerBroadcastMessage(int event) {
+      CHECK(monitor_);

[vandebo (ex-Chrome), 2012/10/29 18:04:38]

These checks don't do anything.

[Hongbo Min, 2012/10/30 14:33:47]

Use if stmt instead.

+      monitor_->ProcessWmPowerBroadcastMessage(event);
+    }
+#endif
+    void ProcessPowerEvent(PowerMonitor::PowerEvent event) {
+      CHECK(monitor_);
+      monitor_->ProcessPowerEvent(event);
+    }
+
+   private:
+    friend class PowerMonitor;
+    // Only allows PowerMonitor to create it.
+    Signaler(PowerMonitor* monitor) : monitor_(monitor) {}
+    ~Signaler() {}

[vandebo (ex-Chrome), 2012/10/29 18:04:38]

Don't inline the implementation, but it in the implementation file.

[Hongbo Min, 2012/10/30 14:33:47]

Done.

+
+    PowerMonitor* monitor_;
   };
 
-  struct BASE_EXPORT RemovableStorageInfo {
-    RemovableStorageInfo();
-    RemovableStorageInfo(const std::string& id,
-                         const string16& device_name,
-                         const FilePath::StringType& device_location);
+  PowerMonitor();
+  ~PowerMonitor();
 
-    // Unique device id - persists between device attachments.
-    std::string device_id;
+  // Return the singleton instance in the application wide.
+  static PowerMonitor* Get();
 
-    // Human readable removable storage device name.
-    string16 name;
-
-    // Current attached removable storage device location.
-    FilePath::StringType location;
-  };
-
-  // Create SystemMonitor. Only one SystemMonitor instance per application
-  // is allowed.
-  SystemMonitor();
-  ~SystemMonitor();
-
-  // Get the application-wide SystemMonitor (if not present, returns NULL).
-  static SystemMonitor* Get();
+  // Return the |signaler_| pointer at the first time it gets called, otherwise

[vandebo (ex-Chrome), 2012/10/29 18:04:38]

nit: The first call to this method will return an instance of Signaler which the
caller can use to signal power events.  All subsequent calls will return NULL.

[Hongbo Min, 2012/10/30 14:33:47]

Done.

+  // NULL is returned. It allows the power event source to have a chance to
+  // store the |signaler_| pointer of PowerMonitor for use in future.
+  //
+  // Note that we assume there is at most only one power event source.
+  PowerMonitor::Signaler* GetSignalerOnce();
 
 #if defined(OS_MACOSX)
-  // Allocate system resources needed by the SystemMonitor class.
+  // Allocate system resources needed by the PowerMonitor class.
   //
   // This function must be called before instantiating an instance of the class
   // and before the Sandbox is initialized.
@@ -95,74 +95,26 @@ class BASE_EXPORT SystemMonitor {
 #endif  // OS_IOS
 #endif  // OS_MACOSX
 
-  // Returns information for attached removable storage.
-  std::vector<RemovableStorageInfo> GetAttachedRemovableStorage() const;
-
-  //
-  // Power-related APIs
-  //
+  // 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);
 
-  // Is the computer currently on battery power.
-  // Can be called on any thread.
+  // Is the computer currently on battery power. Can be called on any thread.
   bool BatteryPower() const {
     // Using a lock here is not necessary for just a bool.
     return battery_in_use_;
   }
 
-  // Callbacks will be called on the thread which creates the SystemMonitor.
-  // During the callback, Add/RemoveObserver will block until the callbacks
-  // are finished. Observers should implement quick callback functions; if
-  // lengthy operations are needed, the observer should take care to invoke
-  // the operation on an appropriate thread.
-  class BASE_EXPORT PowerObserver {
-   public:
-    // Notification of a change in power status of the computer, such
-    // as from switching between battery and A/C power.
-    virtual void OnPowerStateChange(bool on_battery_power) {}
-
-    // Notification that the system is suspending.
-    virtual void OnSuspend() {}
-
-    // Notification that the system is resuming.
-    virtual void OnResume() {}
-
-   protected:
-    virtual ~PowerObserver() {}
-  };
-
-  class BASE_EXPORT DevicesChangedObserver {
-   public:
-    // Notification that the devices connected to the system have changed.
-    // This is only implemented on Windows currently.
-    virtual void OnDevicesChanged(DeviceType device_type) {}
-
-    // When a removable storage device is attached or detached, one of these
-    // two events is triggered.
-    virtual void OnRemovableStorageAttached(
-        const std::string& id,
-        const string16& name,
-        const FilePath::StringType& location) {}
-    virtual void OnRemovableStorageDetached(const std::string& id) {}
-
-   protected:
-    virtual ~DevicesChangedObserver() {}
-  };
-
-  // Add a new observer.
-  // Can be called from any thread.
-  // Must not be called from within a notification callback.
-  void AddPowerObserver(PowerObserver* obs);
-  void AddDevicesChangedObserver(DevicesChangedObserver* obs);
-
-  // Remove an existing observer.
-  // Can be called from any thread.
-  // Must not be called from within a notification callback.
-  void RemovePowerObserver(PowerObserver* obs);
-  void RemoveDevicesChangedObserver(DevicesChangedObserver* obs);
+ private:
+  friend class PowerMonitorTest;
+  friend class Signaler;
 
-  // The ProcessFoo() style methods are a broken pattern and should not
-  // be copied. Any significant addition to this class is blocked on
-  // refactoring to improve the state of affairs. See http://crbug.com/149059
+#if defined(OS_MACOSX)
+  void PlatformInit();
+  void PlatformDestroy();
+#endif
 
 #if defined(OS_WIN)
   // Windows-specific handling of a WM_POWERBROADCAST message.
@@ -172,23 +124,7 @@ class BASE_EXPORT SystemMonitor {
 #endif
 
   // Cross-platform handling of a power event.
-  void ProcessPowerMessage(PowerEvent event_id);
-
-  // Cross-platform handling of a device change event.
-  void ProcessDevicesChanged(DeviceType device_type);
-  void ProcessRemovableStorageAttached(const std::string& id,
-                                       const string16& name,
-                                       const FilePath::StringType& location);
-  void ProcessRemovableStorageDetached(const std::string& id);
-
- private:
-  // Mapping of unique device id to device info tuple.
-  typedef std::map<std::string, RemovableStorageInfo> RemovableStorageMap;
-
-#if defined(OS_MACOSX)
-  void PlatformInit();
-  void PlatformDestroy();
-#endif
+  void ProcessPowerEvent(PowerEvent event_id);
 
   // Platform-specific method to check whether the system is currently
   // running on battery power.  Returns true if running on batteries,
@@ -199,39 +135,30 @@ class BASE_EXPORT SystemMonitor {
   // status has changed.
   void BatteryCheck();
 
-  // Functions to trigger notifications.
-  void NotifyDevicesChanged(DeviceType device_type);
-  void NotifyRemovableStorageAttached(const std::string& id,
-                                      const string16& name,
-                                      const FilePath::StringType& location);
-  void NotifyRemovableStorageDetached(const std::string& id);
   void NotifyPowerStateChange();
   void NotifySuspend();
   void NotifyResume();
 
-  scoped_refptr<ObserverListThreadSafe<PowerObserver> > power_observer_list_;
-  scoped_refptr<ObserverListThreadSafe<DevicesChangedObserver> >
-      devices_changed_observer_list_;
-  bool battery_in_use_;
-  bool suspended_;
-
-#if defined(ENABLE_BATTERY_MONITORING)
-  base::OneShotTimer<SystemMonitor> delayed_battery_check_;
-#endif
-
 #if defined(OS_IOS)
   // Holds pointers to system event notification observers.
   std::vector<id> notification_observers_;
 #endif
 
-  // For manipulating removable_storage_map_ structure.
-  mutable base::Lock removable_storage_lock_;
-  // Map of all the attached removable storage devices.
-  RemovableStorageMap removable_storage_map_;
+#if defined(ENABLE_BATTERY_MONITORING)
+  base::OneShotTimer<PowerMonitor> delayed_battery_check_;
+#endif
+
+  scoped_refptr<ObserverListThreadSafe<PowerObserver> > observers_;
+  bool battery_in_use_;
+  bool suspended_;
+
+  // The Signaler instance owned by PowerMonitor, but might be referenced by
+  // other object and used in the whole life time of the program.
+  PowerMonitor::Signaler* signaler_;
 
-  DISALLOW_COPY_AND_ASSIGN(SystemMonitor);
+  DISALLOW_COPY_AND_ASSIGN(PowerMonitor);
 };
 
 }  // namespace base
 
-#endif  // BASE_SYSTEM_MONITOR_SYSTEM_MONITOR_H_
+#endif  // BASE_POWER_MONITOR_POWER_MONITOR_H_