chromeos: Request screen lock directly from session manager.

chromeos/dbus/session_manager_client.cc

Index: chromeos/dbus/session_manager_client.cc
diff --git a/chromeos/dbus/session_manager_client.cc b/chromeos/dbus/session_manager_client.cc
index 6c40353cbece0b57d9d3e6e5e37f09b2f0cbba8f..2feb79ab5e51b5ccfe1a0da36fcbcdba35076fa6 100644
--- a/chromeos/dbus/session_manager_client.cc
+++ b/chromeos/dbus/session_manager_client.cc
@@ -6,6 +6,7 @@
 
 #include "base/bind.h"
 #include "base/callback.h"
+#include "base/metrics/histogram.h"
 #include "base/string_util.h"
 #include "dbus/bus.h"
 #include "dbus/message.h"
@@ -18,14 +19,30 @@ namespace chromeos {
 // The SessionManagerClient implementation used in production.
 class SessionManagerClientImpl : public SessionManagerClient {
  public:
+  enum LockScreensState {

[Daniel Erat, 2012/07/03 22:44:27]

Rob, are these metrics still useful?  If not, I can delete a bunch of this code.

[flackr, 2012/07/04 12:55:52]

We still have to get lid and idle notifications from powerd right? It would be
useful to know if that path was failing.

[Daniel Erat, 2012/07/09 16:01:42]

Screen locking can be initiated by either Chrome or powerd; in both cases, the
process that wants the screen to be locked calls a method on the session manager
asking it to lock the screen*, which then emits a signal telling Chrome to lock,
after which Chrome calls a method on powerd telling it that the screen has been
locked.  Which part of the path are you hoping to cover with the metrics?  The
Chrome->session_manager->Chrome part of the path is pretty simple (or at least
will be, after this change is in).

* Chrome currently calls it indirectly via powerd; this is the bit that I'm
cleaning up here.

[flackr, 2012/07/09 18:19:57]

Thanks for clarifying, I don't think the current metrics are useful anymore
then. The original problem was the chrome -> power_manager -> chrome path was
failing.

+    LOCK_SCREEN_REQUESTED,  // Lock screen is requested.
+    LOCK_SCREEN_REQUEST_SUCCEEDED,  // Method call succeeded.
+    LOCK_SCREEN_REQUEST_FAILED,  // Method call failed.
+    LOCK_SCREEN_FINISHED,  // Signal is received.
+    NUM_LOCK_SCREEN_STATES
+  };
+
+  enum UnlockScreensState {
+    UNLOCK_SCREEN_REQUESTED,  // Unlock screen is requested.
+    UNLOCK_SCREEN_REQUEST_SUCCEEDED,  // Method call succeeded.
+    UNLOCK_SCREEN_REQUEST_FAILED,  // Method call failed.
+    UNLOCK_SCREEN_FINISHED,  // Signal is received.
+    NUM_UNLOCK_SCREEN_STATES
+  };
+
   explicit SessionManagerClientImpl(dbus::Bus* bus)
       : session_manager_proxy_(NULL),
+        screen_locked_(false),
         weak_ptr_factory_(this) {
     session_manager_proxy_ = bus->GetObjectProxy(
         login_manager::kSessionManagerServiceName,
         dbus::ObjectPath(login_manager::kSessionManagerServicePath));
 
-    // Monitor the D-Bus signal for owner key changes.
     session_manager_proxy_->ConnectToSignal(
         chromium::kChromiumInterface,
         chromium::kOwnerKeySetSignal,
@@ -34,7 +51,6 @@ class SessionManagerClientImpl : public SessionManagerClient {
         base::Bind(&SessionManagerClientImpl::SignalConnected,
                    weak_ptr_factory_.GetWeakPtr()));
 
-    // Monitor the D-Bus signal for property changes.
     session_manager_proxy_->ConnectToSignal(
         chromium::kChromiumInterface,
         chromium::kPropertyChangeCompleteSignal,
@@ -42,46 +58,50 @@ class SessionManagerClientImpl : public SessionManagerClient {
                    weak_ptr_factory_.GetWeakPtr()),
         base::Bind(&SessionManagerClientImpl::SignalConnected,
                    weak_ptr_factory_.GetWeakPtr()));
+
+    session_manager_proxy_->ConnectToSignal(
+        chromium::kChromiumInterface,
+        chromium::kLockScreenSignal,
+        base::Bind(&SessionManagerClientImpl::ScreenLockReceived,
+                   weak_ptr_factory_.GetWeakPtr()),
+        base::Bind(&SessionManagerClientImpl::SignalConnected,
+                   weak_ptr_factory_.GetWeakPtr()));
+
+    session_manager_proxy_->ConnectToSignal(
+        chromium::kChromiumInterface,
+        chromium::kUnlockScreenSignal,
+        base::Bind(&SessionManagerClientImpl::ScreenUnlockReceived,
+                   weak_ptr_factory_.GetWeakPtr()),
+        base::Bind(&SessionManagerClientImpl::SignalConnected,
+                   weak_ptr_factory_.GetWeakPtr()));
   }
 
   virtual ~SessionManagerClientImpl() {
   }
 
-  // SessionManagerClient override.
+  // SessionManagerClient overrides:
   virtual void AddObserver(Observer* observer) OVERRIDE {
     observers_.AddObserver(observer);
   }
 
-  // SessionManagerClient override.
   virtual void RemoveObserver(Observer* observer) OVERRIDE {
     observers_.RemoveObserver(observer);
   }
 
-  // SessionManagerClient override.
+  virtual bool HasObserver(Observer* observer) OVERRIDE {
+    return observers_.HasObserver(observer);
+  }
+
   virtual void EmitLoginPromptReady() OVERRIDE {
-    dbus::MethodCall method_call(
-        login_manager::kSessionManagerInterface,
+    SimpleMethodCallToSessionManager(
         login_manager::kSessionManagerEmitLoginPromptReady);
-    session_manager_proxy_->CallMethod(
-        &method_call,
-        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-        base::Bind(&SessionManagerClientImpl::OnEmitLoginPromptReady,
-                   weak_ptr_factory_.GetWeakPtr()));
   }
 
-  // SessionManagerClient override.
   virtual void EmitLoginPromptVisible() OVERRIDE {
-    dbus::MethodCall method_call(
-        login_manager::kSessionManagerInterface,
+    SimpleMethodCallToSessionManager(
         login_manager::kSessionManagerEmitLoginPromptVisible);
-    session_manager_proxy_->CallMethod(
-        &method_call,
-        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-        base::Bind(&SessionManagerClientImpl::OnEmitLoginPromptVisible,
-                   weak_ptr_factory_.GetWeakPtr()));
   }
 
-  // SessionManagerClient override.
   virtual void RestartJob(int pid, const std::string& command_line) OVERRIDE {
     dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
                                  login_manager::kSessionManagerRestartJob);
@@ -95,18 +115,10 @@ class SessionManagerClientImpl : public SessionManagerClient {
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
-  // SessionManagerClient override.
   virtual void RestartEntd() OVERRIDE {
-    dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
-                                 login_manager::kSessionManagerRestartEntd);
-    session_manager_proxy_->CallMethod(
-        &method_call,
-        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-        base::Bind(&SessionManagerClientImpl::OnRestartEntd,
-                   weak_ptr_factory_.GetWeakPtr()));
+    SimpleMethodCallToSessionManager(login_manager::kSessionManagerRestartEntd);
   }
 
-  // SessionManagerClient override.
   virtual void StartSession(const std::string& user_email) OVERRIDE {
     dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
                                  login_manager::kSessionManagerStartSession);
@@ -120,7 +132,6 @@ class SessionManagerClientImpl : public SessionManagerClient {
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
-  // SessionManagerClient override.
   virtual void StopSession() OVERRIDE {
     dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
                                  login_manager::kSessionManagerStopSession);
@@ -133,28 +144,58 @@ class SessionManagerClientImpl : public SessionManagerClient {
                    weak_ptr_factory_.GetWeakPtr()));
   }
 
-  // SessionManagerClient override.
+  virtual void RequestLockScreen() OVERRIDE {
+    UMA_HISTOGRAM_ENUMERATION("LockScreen.LockScreenPath",
+                              LOCK_SCREEN_REQUESTED,
+                              NUM_LOCK_SCREEN_STATES);
+    dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
+                                 login_manager::kSessionManagerLockScreen);
+    session_manager_proxy_->CallMethodWithErrorCallback(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&SessionManagerClientImpl::OnRequestLockScreenSuccess,
+                   weak_ptr_factory_.GetWeakPtr()),
+        base::Bind(&SessionManagerClientImpl::OnRequestLockScreenError,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+
+  virtual void RequestUnlockScreen() OVERRIDE {
+    UMA_HISTOGRAM_ENUMERATION("LockScreen.UnlockScreenPath",
+                              UNLOCK_SCREEN_REQUESTED,
+                              NUM_UNLOCK_SCREEN_STATES);
+    dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
+                                 login_manager::kSessionManagerUnlockScreen);
+    session_manager_proxy_->CallMethodWithErrorCallback(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&SessionManagerClientImpl::OnRequestUnlockScreenSuccess,
+                   weak_ptr_factory_.GetWeakPtr()),
+        base::Bind(&SessionManagerClientImpl::OnRequestUnlockScreenError,
+                   weak_ptr_factory_.GetWeakPtr()));
+  }
+
+  virtual bool GetIsScreenLocked() OVERRIDE {
+    return screen_locked_;
+  }
+
   virtual void RetrieveDevicePolicy(
       const RetrievePolicyCallback& callback) OVERRIDE {
     CallRetrievePolicy(login_manager::kSessionManagerRetrievePolicy,
                        callback);
   }
 
-  // SessionManagerClient override.
   virtual void RetrieveUserPolicy(
       const RetrievePolicyCallback& callback) OVERRIDE {
     CallRetrievePolicy(login_manager::kSessionManagerRetrieveUserPolicy,
                        callback);
   }
 
-  // SessionManagerClient override.
   virtual void StoreDevicePolicy(const std::string& policy_blob,
                                  const StorePolicyCallback& callback) OVERRIDE {
     CallStorePolicy(login_manager::kSessionManagerStorePolicy,
                     policy_blob, callback);
   }
 
-  // SessionManagerClient override.
   virtual void StoreUserPolicy(const std::string& policy_blob,
                                const StorePolicyCallback& callback) OVERRIDE {
     CallStorePolicy(login_manager::kSessionManagerStoreUserPolicy,
@@ -162,6 +203,17 @@ class SessionManagerClientImpl : public SessionManagerClient {
   }
 
  private:
+  // Makes a method call to the session manager with no arguments and no
+  // response.
+  void SimpleMethodCallToSessionManager(const std::string& method_name) {
+    dbus::MethodCall method_call(login_manager::kSessionManagerInterface,
+                                 method_name);
+    session_manager_proxy_->CallMethod(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        dbus::ObjectProxy::EmptyResponseCallback());
+  }
+
   // Helper for Retrieve{User,Device}Policy.
   virtual void CallRetrievePolicy(const std::string& method_name,
                                   const RetrievePolicyCallback& callback) {
@@ -195,20 +247,6 @@ class SessionManagerClientImpl : public SessionManagerClient {
                    callback));
   }
 
-  // Called when kSessionManagerEmitLoginPromptReady method is complete.
-  void OnEmitLoginPromptReady(dbus::Response* response) {
-    LOG_IF(ERROR, !response)
-        << "Failed to call "
-        << login_manager::kSessionManagerEmitLoginPromptReady;
-  }
-
-  // Called when kSessionManagerEmitLoginPromptVisible method is complete.
-  void OnEmitLoginPromptVisible(dbus::Response* response) {
-    LOG_IF(ERROR, !response)
-        << "Failed to call "
-        << login_manager::kSessionManagerEmitLoginPromptVisible;
-  }
-
   // Called when kSessionManagerRestartJob method is complete.
   void OnRestartJob(dbus::Response* response) {
     LOG_IF(ERROR, !response)
@@ -216,13 +254,6 @@ class SessionManagerClientImpl : public SessionManagerClient {
         << login_manager::kSessionManagerRestartJob;
   }
 
-  // Called when kSessionManagerRestartEntd method is complete.
-  void OnRestartEntd(dbus::Response* response) {
-    LOG_IF(ERROR, !response)
-        << "Failed to call "
-        << login_manager::kSessionManagerRestartEntd;
-  }
-
   // Called when kSessionManagerStartSession method is complete.
   void OnStartSession(dbus::Response* response) {
     LOG_IF(ERROR, !response)
@@ -300,6 +331,27 @@ class SessionManagerClientImpl : public SessionManagerClient {
     FOR_EACH_OBSERVER(Observer, observers_, PropertyChangeComplete(success));
   }
 
+  void ScreenLockReceived(dbus::Signal* signal) {
+    // TODO(flackr): This warning is actually a signal that things are working
+    // as expected. As per http://crbug.com/126217, this will help determine
+    // if the problem is with dbus or in chrome.
+    LOG(WARNING) << "LockScreen signal received from session manager.";
+    screen_locked_ = true;
+    UMA_HISTOGRAM_ENUMERATION("LockScreen.LockScreenPath",
+                              LOCK_SCREEN_FINISHED,
+                              NUM_LOCK_SCREEN_STATES);
+    FOR_EACH_OBSERVER(Observer, observers_, LockScreen());
+  }
+
+  void ScreenUnlockReceived(dbus::Signal* signal) {
+    screen_locked_ = false;
+    UMA_HISTOGRAM_ENUMERATION("LockScreen.UnlockScreenPath",
+                              UNLOCK_SCREEN_FINISHED,
+                              NUM_UNLOCK_SCREEN_STATES);
+    FOR_EACH_OBSERVER(Observer, observers_, UnlockScreen());
+  }
+
+
   // Called when the object is connected to the signal.
   void SignalConnected(const std::string& interface_name,
                        const std::string& signal_name,
@@ -307,8 +359,51 @@ class SessionManagerClientImpl : public SessionManagerClient {
     LOG_IF(ERROR, !success) << "Failed to connect to " << signal_name;
   }
 
+  void OnRequestLockScreenSuccess(dbus::Response* response) {
+    UMA_HISTOGRAM_ENUMERATION("LockScreen.LockScreenPath",
+                              LOCK_SCREEN_REQUEST_SUCCEEDED,
+                              NUM_LOCK_SCREEN_STATES);
+  }
+
+  void OnRequestLockScreenError(dbus::ErrorResponse* error_response) {
+    if (error_response) {
+      dbus::MessageReader reader(error_response);
+      std::string error_message;
+      reader.PopString(&error_message);
+      LOG(ERROR) << "Failed to call "
+                 << login_manager::kSessionManagerLockScreen
+                 << error_response->GetErrorName()
+                 << ": " << error_message;
+    }
+    UMA_HISTOGRAM_ENUMERATION("LockScreen.LockScreenPath",
+                              LOCK_SCREEN_REQUEST_FAILED,
+                              NUM_LOCK_SCREEN_STATES);
+  }
+
+  void OnRequestUnlockScreenSuccess(dbus::Response* response) {
+    UMA_HISTOGRAM_ENUMERATION("LockScreen.UnlockScreenPath",
+                              UNLOCK_SCREEN_REQUEST_SUCCEEDED,
+                              NUM_UNLOCK_SCREEN_STATES);
+  }
+
+  void OnRequestUnlockScreenError(dbus::ErrorResponse* error_response) {
+    if (error_response) {
+      dbus::MessageReader reader(error_response);
+      std::string error_message;
+      reader.PopString(&error_message);
+      LOG(ERROR) << "Failed to call "
+                 << login_manager::kSessionManagerUnlockScreen
+                 << error_response->GetErrorName()
+                 << ": " << error_message;
+    }
+    UMA_HISTOGRAM_ENUMERATION("LockScreen.UnlockScreenPath",
+                              UNLOCK_SCREEN_REQUEST_FAILED,
+                              NUM_UNLOCK_SCREEN_STATES);
+  }
+
   dbus::ObjectProxy* session_manager_proxy_;
   ObserverList<Observer> observers_;
+  bool screen_locked_;
   base::WeakPtrFactory<SessionManagerClientImpl> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SessionManagerClientImpl);
@@ -320,12 +415,16 @@ class SessionManagerClientStubImpl : public SessionManagerClient {
   // SessionManagerClient overrides.
   virtual void AddObserver(Observer* observer) OVERRIDE {}
   virtual void RemoveObserver(Observer* observer) OVERRIDE {}
+  virtual bool HasObserver(Observer* observer) OVERRIDE { return false; }
   virtual void EmitLoginPromptReady() OVERRIDE {}
   virtual void EmitLoginPromptVisible() OVERRIDE {}
   virtual void RestartJob(int pid, const std::string& command_line) OVERRIDE {}
   virtual void RestartEntd() OVERRIDE {}
   virtual void StartSession(const std::string& user_email) OVERRIDE {}
   virtual void StopSession() OVERRIDE {}
+  virtual void RequestLockScreen() OVERRIDE {}
+  virtual void RequestUnlockScreen() OVERRIDE {}
+  virtual bool GetIsScreenLocked() OVERRIDE { return false; }
   virtual void RetrieveDevicePolicy(
       const RetrievePolicyCallback& callback) OVERRIDE {
     callback.Run("");