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

base/power_monitor/power_monitor.cc

Index: base/power_monitor/power_monitor.cc
diff --git a/base/power_monitor/power_monitor.cc b/base/power_monitor/power_monitor.cc
new file mode 100644
index 0000000000000000000000000000000000000000..8460108862416ff5dfdaf244fa3582df0dba0532
--- /dev/null
+++ b/base/power_monitor/power_monitor.cc
@@ -0,0 +1,124 @@
+// Copyright (c) 2012 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.
+
+#include "base/power_monitor/power_monitor.h"
+
+namespace base {
+
+static PowerMonitor* g_power_monitor = NULL;
+
+#if defined(ENABLE_BATTERY_MONITORING)
+// The amount of time (in ms) to wait before running the initial
+// battery check.
+static int kDelayedBatteryCheckMs = 10 * 1000;
+#endif  // defined(ENABLE_BATTERY_MONITORING)
+
+PowerMonitor::PowerMonitor()
+    : observers_(new ObserverListThreadSafe<PowerObserver>()),
+      battery_in_use_(false),
+      suspended_(false),
+      signaler_(NULL)  {
+  DCHECK(!g_power_monitor);
+  g_power_monitor = this;
+
+  DCHECK(MessageLoop::current());
+#if defined(ENABLE_BATTERY_MONITORING)
+  delayed_battery_check_.Start(FROM_HERE,
+      base::TimeDelta::FromMilliseconds(kDelayedBatteryCheckMs), this,
+      &PowerMonitor::BatteryCheck);
+#endif  // defined(ENABLE_BATTERY_MONITORING)
+
+#if defined(OS_MACOSX)
+  PlatformInit();
+#endif
+}
+
+PowerMonitor::~PowerMonitor() {
+#if defined(OS_MACOSX)
+  PlatformDestroy();
+#endif
+  if (signaler_)
+    delete signaler_;

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

Don't manage lifetimes in the destructor:  If this class owns signaler_, then it
should be in a scoped_ptr<>.  But I don't think this class can own it since the
class that gets it could outlive this class.  In that case, there's no reason to
store it, just store a bool indicating if we've given own out.

Furthermore, since signaler_ can outlive PowerMonitor, signaler_ should
internally call PowerMonitor::Get() to see if it's still alive.

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

You are right.

+  DCHECK_EQ(this, g_power_monitor);
+  g_power_monitor = NULL;
+}
+
+// static
+PowerMonitor* PowerMonitor::Get() {
+  return g_power_monitor;
+}
+
+PowerMonitor::Signaler* PowerMonitor::GetSignalerOnce() {
+  CHECK(signaler_ == NULL);

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

The interface says it will return NULL on the second call, this checks = crash.

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

Sorry for this low level mistake:)

+
+  if (signaler_ == NULL) {
+    // The signaler is allowed to be leaked since the PowerMonitor and Signaler

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

Comment is out of date.

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

Done.

+    // hold a reference to each other.
+    signaler_ = new PowerMonitor::Signaler(this);
+    return signaler_;
+  }
+
+  // Return NULL after the first time it get called.
+  return NULL;
+}
+
+void PowerMonitor::AddObserver(PowerObserver* obs) {
+  observers_->AddObserver(obs);
+}
+
+void PowerMonitor::RemoveObserver(PowerObserver* obs) {
+  observers_->RemoveObserver(obs);
+}
+
+void PowerMonitor::ProcessPowerEvent(PowerEvent event_id) {
+  // Suppress duplicate notifications.  Some platforms may
+  // send multiple notifications of the same event.
+  switch (event_id) {
+    case POWER_STATE_EVENT:
+      {
+        bool on_battery = IsBatteryPower();
+        if (on_battery != battery_in_use_) {
+          battery_in_use_ = on_battery;
+          NotifyPowerStateChange();
+        }
+      }
+      break;
+    case RESUME_EVENT:
+      if (suspended_) {
+        suspended_ = false;
+        NotifyResume();
+      }
+      break;
+    case SUSPEND_EVENT:
+      if (!suspended_) {
+        suspended_ = true;
+        NotifySuspend();
+      }
+      break;
+  }
+}
+
+void PowerMonitor::NotifyPowerStateChange() {
+  DVLOG(1) << "PowerStateChange: " << (BatteryPower() ? "On" : "Off")
+           << " battery";
+  observers_->Notify(&PowerObserver::OnPowerStateChange,
+                               BatteryPower());
+}
+
+void PowerMonitor::NotifySuspend() {
+  DVLOG(1) << "Power Suspending";
+  observers_->Notify(&PowerObserver::OnSuspend);
+}
+
+void PowerMonitor::NotifyResume() {
+  DVLOG(1) << "Power Resuming";
+  observers_->Notify(&PowerObserver::OnResume);
+}
+
+void PowerMonitor::BatteryCheck() {
+  ProcessPowerEvent(PowerMonitor::POWER_STATE_EVENT);
+}
+
+}  // namespace base
+