Add dark resume methods to PowerManagerClient

chromeos/dbus/power_manager_client.cc

 // 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 "chromeos/dbus/power_manager_client.h"
 
 #include <algorithm>
 
 #include "base/bind.h"
 #include "base/callback.h"
@@ skipping 30 matching lines @@
 const char kSuspendDelayDescription[] = "chrome";
 
 // The PowerManagerClient implementation used in production.
 class PowerManagerClientImpl : public PowerManagerClient {
  public:
   PowerManagerClientImpl()
       : origin_thread_id_(base::PlatformThread::CurrentId()),
         power_manager_proxy_(NULL),
         suspend_delay_id_(-1),
         has_suspend_delay_id_(false),
+        dark_suspend_delay_id_(-1),
+        has_dark_suspend_delay_id_(false),
         pending_suspend_id_(-1),
         suspend_is_pending_(false),
+        suspending_from_dark_resume_(false),
         num_pending_suspend_readiness_callbacks_(0),
         last_is_projecting_(false),
         weak_ptr_factory_(this) {}
 
   virtual ~PowerManagerClientImpl() {
     // Here we should unregister suspend notifications from powerd,
     // however:
     // - The lifetime of the PowerManagerClientImpl can extend past that of
     //   the objectproxy,
     // - power_manager can already detect that the client is gone and
@@ skipping 215 matching lines @@
     power_manager_proxy_->ConnectToSignal(
         power_manager::kPowerManagerInterface,
         power_manager::kSuspendDoneSignal,
         base::Bind(&PowerManagerClientImpl::SuspendDoneReceived,
                    weak_ptr_factory_.GetWeakPtr()),
         base::Bind(&PowerManagerClientImpl::SignalConnected,
                    weak_ptr_factory_.GetWeakPtr()));
 
     power_manager_proxy_->ConnectToSignal(
         power_manager::kPowerManagerInterface,
+        power_manager::kDarkSuspendImminentSignal,
+        base::Bind(
+            &PowerManagerClientImpl::DarkSuspendImminentReceived,
+            weak_ptr_factory_.GetWeakPtr()),
+        base::Bind(&PowerManagerClientImpl::SignalConnected,
+                   weak_ptr_factory_.GetWeakPtr()));
+
+    power_manager_proxy_->ConnectToSignal(
+        power_manager::kPowerManagerInterface,
         power_manager::kIdleActionImminentSignal,
         base::Bind(
             &PowerManagerClientImpl::IdleActionImminentReceived,
             weak_ptr_factory_.GetWeakPtr()),
         base::Bind(&PowerManagerClientImpl::SignalConnected,
                    weak_ptr_factory_.GetWeakPtr()));
 
     power_manager_proxy_->ConnectToSignal(
         power_manager::kPowerManagerInterface,
         power_manager::kIdleActionDeferredSignal,
         base::Bind(
             &PowerManagerClientImpl::IdleActionDeferredReceived,
             weak_ptr_factory_.GetWeakPtr()),
         base::Bind(&PowerManagerClientImpl::SignalConnected,
                    weak_ptr_factory_.GetWeakPtr()));
 
-    RegisterSuspendDelay();
+    RegisterSuspendDelays();
   }
 
  private:
   // Returns true if the current thread is the origin thread.
   bool OnOriginThread() {
     return base::PlatformThread::CurrentId() == origin_thread_id_;
   }
 
   // Called when a dbus signal is initially connected.
   void SignalConnected(const std::string& interface_name,
@@ skipping 14 matching lines @@
   }
 
   void NameOwnerChangedReceived(const std::string& old_owner,
                                 const std::string& new_owner) {
     VLOG(1) << "Power manager restarted (old owner was "
             << (old_owner.empty() ? "[none]" : old_owner.c_str())
             << ", new owner is "
             << (new_owner.empty() ? "[none]" : new_owner.c_str()) << ")";
     if (!new_owner.empty()) {
       VLOG(1) << "Sending initial state to power manager";
-      RegisterSuspendDelay();
+      RegisterSuspendDelays();
       SetIsProjecting(last_is_projecting_);
       FOR_EACH_OBSERVER(Observer, observers_, PowerManagerRestarted());
     }
   }
 
   void BrightnessChangedReceived(dbus::Signal* signal) {
     dbus::MessageReader reader(signal);
     int32 brightness_level = 0;
     bool user_initiated = 0;
     if (!(reader.PopInt32(&brightness_level) &&
@@ skipping 67 matching lines @@
       return;
     }
     dbus::MessageReader reader(response);
     double percent = 0.0;
     if (!reader.PopDouble(&percent))
       LOG(ERROR) << "Error reading response from powerd: "
                  << response->ToString();
     callback.Run(percent);
   }
 
-  void OnRegisterSuspendDelayReply(dbus::Response* response) {
+  void OnRegisterSuspendDelayReply(bool dark_suspend,
+                                   dbus::Response* response) {
     if (!response) {
-      LOG(ERROR) << "Error calling "
-                 << power_manager::kRegisterSuspendDelayMethod;
+      LOG(ERROR) << "Error calling " << response->GetMember();
       return;
     }
 
     dbus::MessageReader reader(response);
     power_manager::RegisterSuspendDelayReply protobuf;
     if (!reader.PopArrayOfBytesAsProto(&protobuf)) {
-      LOG(ERROR) << "Unable to parse reply from "
-                 << power_manager::kRegisterSuspendDelayMethod;
+      LOG(ERROR) << "Unable to parse reply from " << response->GetMember();
       return;
     }
 
-    suspend_delay_id_ = protobuf.delay_id();
-    has_suspend_delay_id_ = true;
-    VLOG(1) << "Registered suspend delay " << suspend_delay_id_;
+    if (dark_suspend) {
+      dark_suspend_delay_id_ = protobuf.delay_id();
+      has_dark_suspend_delay_id_ = true;
+      VLOG(1) << "Registered dark suspend delay " << dark_suspend_delay_id_;
+    } else {
+      suspend_delay_id_ = protobuf.delay_id();
+      has_suspend_delay_id_ = true;
+      VLOG(1) << "Registered suspend delay " << suspend_delay_id_;
+    }
   }
 
   void SuspendImminentReceived(dbus::Signal* signal) {
-    if (!has_suspend_delay_id_) {
-      LOG(ERROR) << "Received unrequested "
-                 << power_manager::kSuspendImminentSignal << " signal";
+    SuspendImminentCommon(false, signal);
+  }
+
+  void DarkSuspendImminentReceived(dbus::Signal* signal) {

[Daniel Erat, 2014/08/15 23:49:31]

can you just eliminate these and bind true or false to the shared method and use
that as the callback?

[Chirantan Ekbote, 2014/08/18 17:49:08]

sure.  will fix.

+    SuspendImminentCommon(true, signal);
+  }
+
+  void SuspendImminentCommon(bool in_dark_resume,

[Daniel Erat, 2014/08/15 23:49:31]

nit: HandleSuspendImminent()?

+                             dbus::Signal* signal) {
+    std::string signal_name = signal->GetMember();
+    if ((in_dark_resume && !has_dark_suspend_delay_id_) ||
+        (!in_dark_resume && !has_suspend_delay_id_)) {
+      LOG(ERROR) << "Received unrequested " << signal_name << " signal";
       return;
     }
 
     dbus::MessageReader reader(signal);
     power_manager::SuspendImminent proto;
     if (!reader.PopArrayOfBytesAsProto(&proto)) {
-      LOG(ERROR) << "Unable to decode protocol buffer from "
-                 << power_manager::kSuspendImminentSignal << " signal";
+      LOG(ERROR) << "Unable to decode protocol buffer from " << signal_name
+                 << " signal";
       return;
     }
 
-    VLOG(1) << "Got " << power_manager::kSuspendImminentSignal << " signal "
-            << "announcing suspend attempt " << proto.suspend_id();
-    if (suspend_is_pending_) {
-      LOG(WARNING) << "Got " << power_manager::kSuspendImminentSignal
-                   << " signal about pending suspend attempt "
-                   << proto.suspend_id() << " while still waiting "
-                   << "on attempt " << pending_suspend_id_;
+    VLOG(1) << "Got " << signal_name << " signal announcing suspend attempt "
+            << proto.suspend_id();
+
+    // If a previous suspend is pending from the same state we are currently in
+    // (fully powered on or in dark resume), then something's gone a little
+    // wonky.
+    if (suspend_is_pending_ &&
+        suspending_from_dark_resume_ == in_dark_resume) {
+      LOG(WARNING) << "Got " << signal_name << " signal about pending suspend "
+                   << "attempt " << proto.suspend_id() << " while still "
+                   << "waiting on attempt " << pending_suspend_id_;

[Daniel Erat, 2014/08/15 23:49:31]

i'm trying to think of when this could happen -- when someone within chrome
registers an async delay and then waits too long to call it, resulting in the
delay timing out and powerd suspending anyway?

in general, does suspend_is_pending_ being true here indicate a problem?

[Chirantan Ekbote, 2014/08/18 17:49:08]

Yes, that's the situation in which this could happen.  I don't _think_ that it's
a big problem if suspend_is_pending_ is true here but I guess it depends on what
the delays are doing.  From the PowerManagerClient's perspective, we would let
any outstanding delays continue running and when they run the suspend readiness
callback we would just ignore it.

     }
 
     pending_suspend_id_ = proto.suspend_id();
     suspend_is_pending_ = true;
+    suspending_from_dark_resume_ = in_dark_resume;
     num_pending_suspend_readiness_callbacks_ = 0;
-    FOR_EACH_OBSERVER(Observer, observers_, SuspendImminent());
+    if (suspending_from_dark_resume_) {

[Daniel Erat, 2014/08/15 23:49:31]

nit: omit curly brackets

+      FOR_EACH_OBSERVER(Observer, observers_, DarkSuspendImminent());
+    } else {
+      FOR_EACH_OBSERVER(Observer, observers_, SuspendImminent());
+    }
     MaybeReportSuspendReadiness();
   }
 
   void SuspendDoneReceived(dbus::Signal* signal) {
     dbus::MessageReader reader(signal);
     power_manager::SuspendDone proto;
     if (!reader.PopArrayOfBytesAsProto(&proto)) {
       LOG(ERROR) << "Unable to decode protocol buffer from "
                  << power_manager::kSuspendDoneSignal << " signal";
       return;
@@ skipping 64 matching lines @@
       case power_manager::InputEvent_Type_LID_CLOSED: {
         bool open =
             (proto.type() == power_manager::InputEvent_Type_LID_OPEN);
         FOR_EACH_OBSERVER(PowerManagerClient::Observer, observers_,
                           LidEventReceived(open, timestamp));
         break;
       }
     }
   }
 
-  // Registers a suspend delay with the power manager.  This is usually
-  // only called at startup, but if the power manager restarts, we need to
-  // create a new delay.
-  void RegisterSuspendDelay() {
+  // Registers suspend delays with the power manager.  This is usually only
+  // called at startup, but if the power manager restarts, we need to create new
+  // delays.
+  void RegisterSuspendDelays() {
     // Throw out any old delay that was registered.
     suspend_delay_id_ = -1;
     has_suspend_delay_id_ = false;
+    dark_suspend_delay_id_ = -1;
+    has_dark_suspend_delay_id_ = false;
 
     dbus::MethodCall method_call(
         power_manager::kPowerManagerInterface,
         power_manager::kRegisterSuspendDelayMethod);
     dbus::MessageWriter writer(&method_call);
+    dbus::MethodCall dark_method_call(
+        power_manager::kPowerManagerInterface,
+        power_manager::kRegisterDarkSuspendDelayMethod);
+    dbus::MessageWriter dark_writer(&dark_method_call);
 
     power_manager::RegisterSuspendDelayRequest protobuf_request;
     base::TimeDelta timeout =
         base::TimeDelta::FromMilliseconds(kSuspendDelayTimeoutMs);
     protobuf_request.set_timeout(timeout.ToInternalValue());
     protobuf_request.set_description(kSuspendDelayDescription);
 
     if (!writer.AppendProtoAsArrayOfBytes(protobuf_request)) {
       LOG(ERROR) << "Error constructing message for "
                  << power_manager::kRegisterSuspendDelayMethod;
       return;
     }
+    if (!dark_writer.AppendProtoAsArrayOfBytes(protobuf_request)) {
+      LOG(ERROR) << "Error constructing message for "
+                 << power_manager::kRegisterDarkSuspendDelayMethod;
+      return;
+    }
     power_manager_proxy_->CallMethod(
         &method_call,
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         base::Bind(
             &PowerManagerClientImpl::OnRegisterSuspendDelayReply,
-            weak_ptr_factory_.GetWeakPtr()));
+            weak_ptr_factory_.GetWeakPtr(), false));
+    power_manager_proxy_->CallMethod(

[Daniel Erat, 2014/08/15 23:49:31]

add a helper method for this to cut down on the duplicated code?

+        &dark_method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(
+            &PowerManagerClientImpl::OnRegisterSuspendDelayReply,
+            weak_ptr_factory_.GetWeakPtr(), true));
   }
 
   // Records the fact that an observer has finished doing asynchronous work
   // that was blocking a pending suspend attempt and possibly reports
   // suspend readiness to powerd.  Called by callbacks returned via
   // GetSuspendReadinessCallback().
   void HandleObserverSuspendReadiness(int32 suspend_id) {
     DCHECK(OnOriginThread());
     if (!suspend_is_pending_ || suspend_id != pending_suspend_id_)
       return;
 
     num_pending_suspend_readiness_callbacks_--;
     MaybeReportSuspendReadiness();
   }
 
   // Reports suspend readiness to powerd if no observers are still holding
   // suspend readiness callbacks.
   void MaybeReportSuspendReadiness() {
     if (!suspend_is_pending_ || num_pending_suspend_readiness_callbacks_ > 0)
       return;
 
+    std::string method_name;
+    int32 delay_id;
+    if (suspending_from_dark_resume_) {
+      method_name = power_manager::kHandleDarkSuspendReadinessMethod;
+      delay_id = dark_suspend_delay_id_;
+    } else {
+      method_name = power_manager::kHandleSuspendReadinessMethod;
+      delay_id = suspend_delay_id_;
+    }
+
     dbus::MethodCall method_call(
-        power_manager::kPowerManagerInterface,
-        power_manager::kHandleSuspendReadinessMethod);
+        power_manager::kPowerManagerInterface, method_name);
     dbus::MessageWriter writer(&method_call);
 
-    VLOG(1) << "Announcing readiness of suspend delay " << suspend_delay_id_
+    VLOG(1) << "Announcing readiness of suspend delay " << delay_id
             << " for suspend attempt " << pending_suspend_id_;
     power_manager::SuspendReadinessInfo protobuf_request;
-    protobuf_request.set_delay_id(suspend_delay_id_);
+    protobuf_request.set_delay_id(delay_id);
     protobuf_request.set_suspend_id(pending_suspend_id_);
 
     pending_suspend_id_ = -1;
     suspend_is_pending_ = false;
 
     if (!writer.AppendProtoAsArrayOfBytes(protobuf_request)) {
-      LOG(ERROR) << "Error constructing message for "
-                 << power_manager::kHandleSuspendReadinessMethod;
+      LOG(ERROR) << "Error constructing message for " << method_name;
       return;
     }
     power_manager_proxy_->CallMethod(
         &method_call,
         dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
         dbus::ObjectProxy::EmptyResponseCallback());
   }
 
   // Origin thread (i.e. the UI thread in production).
   base::PlatformThreadId origin_thread_id_;
 
   dbus::ObjectProxy* power_manager_proxy_;
   ObserverList<Observer> observers_;
 
   // The delay_id_ obtained from the RegisterSuspendDelay request.
   int32 suspend_delay_id_;
   bool has_suspend_delay_id_;
 
+  // The delay_id_ obtained from the RegisterDarkSuspendDelay request.
+  int32 dark_suspend_delay_id_;
+  bool has_dark_suspend_delay_id_;
+
   // powerd-supplied ID corresponding to an imminent suspend attempt that is
   // currently being delayed.
   int32 pending_suspend_id_;
   bool suspend_is_pending_;
+  bool suspending_from_dark_resume_;

[Daniel Erat, 2014/08/15 23:49:31]

add a comment clarifying what this means, and that it affects the interpretation
of pending_suspend_id_ and suspend_is_pending_

 
   // Number of callbacks that have been returned by
   // GetSuspendReadinessCallback() during the currently-pending suspend
   // attempt but have not yet been called.
   int num_pending_suspend_readiness_callbacks_;
 
   // Last state passed to SetIsProjecting().
   bool last_is_projecting_;
 
   // Note: This should remain the last member so it'll be destroyed and
@@ skipping 230 matching lines @@
 // static
 PowerManagerClient* PowerManagerClient::Create(
     DBusClientImplementationType type) {
   if (type == REAL_DBUS_CLIENT_IMPLEMENTATION)
     return new PowerManagerClientImpl();
   DCHECK_EQ(STUB_DBUS_CLIENT_IMPLEMENTATION, type);
   return new PowerManagerClientStubImpl();
 }
 
 }  // namespace chromeos