Implement support for monitor suspend.

chromeos/dbus/power_manager_client.cc

Index: chromeos/dbus/power_manager_client.cc
diff --git a/chromeos/dbus/power_manager_client.cc b/chromeos/dbus/power_manager_client.cc
index 4dd191f0b6c663b6f84079e1c8d222d600675fa7..5eefc62250aa6acba6916628780b5b18532ee176 100644
--- a/chromeos/dbus/power_manager_client.cc
+++ b/chromeos/dbus/power_manager_client.cc
@@ -27,11 +27,14 @@
 
 namespace chromeos {
 
+const int kSuspendDelayTimeoutMs = 5000;
+
 // The PowerManagerClient implementation used in production.
 class PowerManagerClientImpl : public PowerManagerClient {
  public:
   explicit PowerManagerClientImpl(dbus::Bus* bus)
       : power_manager_proxy_(NULL),
+        has_delay_id_(false),

[Daniel Erat, 2013/01/08 00:51:35]

nit: rename to be more descriptive: has_suspend_delay_id_

also initialize suspend_delay_id_ to something like 0 or -1 here (just so it
doesn't contain a random value that could make it hard to investigate bugs
later)

[marcheu, 2013/01/10 01:03:49]

Done.

         weak_ptr_factory_(this) {
     power_manager_proxy_ = bus->GetObjectProxy(
         power_manager::kPowerManagerServiceName,
@@ -96,9 +99,61 @@ class PowerManagerClientImpl : public PowerManagerClient {
                    weak_ptr_factory_.GetWeakPtr()),
         base::Bind(&PowerManagerClientImpl::SignalConnected,
                    weak_ptr_factory_.GetWeakPtr()));
+
+    power_manager_proxy_->ConnectToSignal(
+        power_manager::kPowerManagerInterface,
+        power_manager::kSuspendImminentSignal,
+        base::Bind(
+            &PowerManagerClientImpl::SuspendImminentReceived,
+            weak_ptr_factory_.GetWeakPtr()),
+        base::Bind(&PowerManagerClientImpl::SignalConnected,
+                   weak_ptr_factory_.GetWeakPtr()));
+
+    // Register to powerd for suspend notifications.
+    dbus::MethodCall method_call(
+        power_manager::kPowerManagerInterface,
+    power_manager::kRegisterSuspendDelayMethod);

[Daniel Erat, 2013/01/08 00:51:35]

nit: fix indenting

[marcheu, 2013/01/10 01:03:49]

Done.

+    dbus::MessageWriter writer(&method_call);
+
+    power_manager::RegisterSuspendDelayRequest protobuf_request;
+    base::TimeDelta timeout =
+        base::TimeDelta::FromMilliseconds(kSuspendDelayTimeoutMs);
+    protobuf_request.Clear();

[Daniel Erat, 2013/01/08 00:51:35]

nit: don't need to call Clear() on a new protobuf, i don't think

[marcheu, 2013/01/10 01:03:49]

Done.

+    protobuf_request.set_timeout(timeout.ToInternalValue());
+
+    if (!writer.AppendProtoAsArrayOfBytes(protobuf_request)) {
+      LOG(ERROR) << "Error calling "

[Daniel Erat, 2013/01/08 00:51:35]

nit: change this to "Error constructing message for " <<
power_manager::kRegisterSuspendDelayMethod;

[marcheu, 2013/01/10 01:03:49]

Done.

+                 << power_manager::kRegisterSuspendDelayMethod;
+      return;
+    }
+    power_manager_proxy_->CallMethod(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(
+            &PowerManagerClientImpl::OnRegisterSuspendDelayReply,
+            weak_ptr_factory_.GetWeakPtr()));
   }
 
   virtual ~PowerManagerClientImpl() {
+    // Unregister from powerd for suspend notifications.
+    if (!has_delay_id_) {
+      LOG(ERROR) << "Destroying PowerManagerClientImpl without a delay_id_";
+      return;

[Daniel Erat, 2013/01/08 00:51:35]

i'm pretty sure you don't want to bail out early from the d'tor like this --
what if someone else adds something unrelated to it later?  i'd do:

  if (has_delay_id_) {
    dbus::MethodCall ...
  }

instead.  if you want to log an error if registration fails, it'd be better to
do it in response to the RegisterSuspendDelay call failing.

[marcheu, 2013/01/10 01:03:49]

Done.

+    }
+
+    dbus::MethodCall method_call(
+        power_manager::kPowerManagerInterface,
+    power_manager::kUnregisterSuspendDelayMethod);
+    dbus::MessageWriter writer(&method_call);
+
+    power_manager::UnregisterSuspendDelayRequest protobuf;
+    protobuf.Clear();
+    protobuf.set_delay_id(delay_id_);
+
+    power_manager_proxy_->CallMethod(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        dbus::ObjectProxy::EmptyResponseCallback());
   }
 
   // PowerManagerClient overrides:
@@ -444,6 +499,25 @@ class PowerManagerClientImpl : public PowerManagerClient {
     callback.Run(percent);
   }
 
+  void OnRegisterSuspendDelayReply(
+      dbus::Response* response) {

[Daniel Erat, 2013/01/08 00:51:35]

nit: unwrap this line

[marcheu, 2013/01/10 01:03:49]

Done.

+    if (!response) {
+      LOG(ERROR) << "Error calling "
+                 << power_manager::kRegisterSuspendDelayMethod;
+      return;
+    }
+    dbus::MessageReader reader(response);
+    power_manager::RegisterSuspendDelayReply protobuf;
+    if (!reader.PopArrayOfBytesAsProto(&protobuf)) {
+      LOG(ERROR) << "Error calling "
+                 << power_manager::kRegisterSuspendDelayMethod
+                 << response->ToString();

[Daniel Erat, 2013/01/08 00:51:35]

nit: change this to be more descriptive, like:

  "Unable to parse reply from " << power_manager::kRegisterSuspendDelayMethod

i don't think that logging the response here is going to be useful; it'll just
be a bunch of binary data

[marcheu, 2013/01/10 01:03:49]

Done.

+      return;
+    }
+    delay_id_ = protobuf.delay_id();
+    has_delay_id_ = true;
+  }
+
   void IdleNotifySignalReceived(dbus::Signal* signal) {
     dbus::MessageReader reader(signal);
     int64 threshold = 0;
@@ -481,6 +555,45 @@ class PowerManagerClientImpl : public PowerManagerClient {
     FOR_EACH_OBSERVER(Observer, observers_, ScreenDimmingRequested(state));
   }
 
+  void SuspendImminentReceived(dbus::Signal* signal) {
+    if (!has_delay_id_) {
+      LOG(ERROR) << "Received unrequested "
+                 << power_manager::kSuspendImminentSignal << " signal";
+      return;
+    }
+
+    dbus::MessageReader reader(signal);
+    power_manager::SuspendImminent protobuf_imminent;
+    if (!reader.PopArrayOfBytesAsProto(&protobuf_imminent)) {
+      LOG(ERROR) << "Unable to decode protocol buffer from "
+                 << power_manager::kSuspendImminentSignal << " signal";
+      return;
+    }
+    int32 suspend_id = protobuf_imminent.suspend_id();
+
+    FOR_EACH_OBSERVER(Observer, observers_, SuspendImminent());
+
+    dbus::MethodCall method_call(
+        power_manager::kPowerManagerInterface,
+    power_manager::kHandleSuspendReadinessMethod);

[Daniel Erat, 2013/01/08 00:51:35]

nit: fix indenting

[marcheu, 2013/01/10 01:03:49]

Done.

+    dbus::MessageWriter writer(&method_call);
+
+    power_manager::SuspendReadinessInfo protobuf_request;
+    protobuf_request.Clear();

[Daniel Erat, 2013/01/08 00:51:35]

nit: don't need Clear() here

[marcheu, 2013/01/10 01:03:49]

Done.

+    protobuf_request.set_delay_id(delay_id_);
+    protobuf_request.set_suspend_id(suspend_id);
+
+    if (!writer.AppendProtoAsArrayOfBytes(protobuf_request)) {
+      LOG(ERROR) << "Error calling "

[Daniel Erat, 2013/01/08 00:51:35]

nit: "Error constructing message for " << ...

[marcheu, 2013/01/10 01:03:49]

Done.

+                 << power_manager::kHandleSuspendReadinessMethod;
+      return;
+    }
+    power_manager_proxy_->CallMethod(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        dbus::ObjectProxy::EmptyResponseCallback());
+  }
+
   void InputEventReceived(dbus::Signal* signal) {
     dbus::MessageReader reader(signal);
     power_manager::InputEvent proto;
@@ -542,6 +655,10 @@ class PowerManagerClientImpl : public PowerManagerClient {
   dbus::ObjectProxy* power_manager_proxy_;
   ObserverList<Observer> observers_;
 
+  // The delay_id_ obtained from the RegisterSuspendDelay request.
+  int32 delay_id_;

[Daniel Erat, 2013/01/08 00:51:35]

nit: rename this to suspend_delay_id_, too

[marcheu, 2013/01/10 01:03:49]

Done.

+  bool has_delay_id_;
+
   // Wall time from the latest signal telling us that the system was about to
   // suspend to memory.
   base::Time last_suspend_wall_time_;