chromeos: Drop session_manager screen lock notifications.

Remove ScreenIsLocked and ScreenIsUnlocked from
SessionManagerClient::Observer. Callers should use
ash::SessionObserver (in ash) or
session_manager::SessionManagerObserver (in the
browser) instead.

Update the following classes to do that:
- ash::PowerEventObserver
- chromeos::ExtensionSystemEventObserver
- TetherService

Bug: 778888
Change-Id: I9792338a8c5cce6f0aeaddb11dad8adfd951683c
Reviewed-on: https://chromium-review.googlesource.com/741262
Commit-Queue: Dan Erat <derat@chromium.org>
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Reviewed-by: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512898}

ash/system/power/power_event_observer.cc

@@ -44,18 +44,16 @@ void OnSuspendDisplaysCompleted(const base::Closure& suspend_callback,
 }  // namespace
 
 PowerEventObserver::PowerEventObserver()
-    : screen_locked_(false), waiting_for_lock_screen_animations_(false) {
+    : session_observer_(this),
+      screen_locked_(Shell::Get()->session_controller()->IsScreenLocked()),
+      waiting_for_lock_screen_animations_(false) {
   chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->AddObserver(
       this);
-  chromeos::DBusThreadManager::Get()->GetSessionManagerClient()->AddObserver(
-      this);
 }
 
 PowerEventObserver::~PowerEventObserver() {
   chromeos::DBusThreadManager::Get()->GetPowerManagerClient()->RemoveObserver(
       this);
-  chromeos::DBusThreadManager::Get()->GetSessionManagerClient()->RemoveObserver(
-      this);
 }
 
 void PowerEventObserver::OnLockAnimationsComplete() {
@@ -142,21 +140,21 @@ void PowerEventObserver::SuspendDone(const base::TimeDelta& sleep_duration) {
   ResumeRenderingRequests();
 }
 
-void PowerEventObserver::ScreenIsLocked() {
-  screen_locked_ = true;
-  waiting_for_lock_screen_animations_ = true;
-
-  // The screen is now locked but the pending suspend, if any, will be blocked
-  // until all the animations have completed.
-  if (!screen_lock_callback_.is_null()) {
-    VLOG(1) << "Screen locked due to suspend";
+void PowerEventObserver::OnLockStateChanged(bool locked) {
+  if (locked) {
+    screen_locked_ = true;
+    waiting_for_lock_screen_animations_ = true;
+
+    // The screen is now locked but the pending suspend, if any, will be blocked
+    // until all the animations have completed.
+    if (!screen_lock_callback_.is_null()) {
+      VLOG(1) << "Screen locked due to suspend";
+    } else {
+      VLOG(1) << "Screen locked without suspend";
+    }
   } else {
-    VLOG(1) << "Screen locked without suspend";
+    screen_locked_ = false;
   }
 }
 
-void PowerEventObserver::ScreenIsUnlocked() {
-  screen_locked_ = false;
-}
-
 }  // namespace ash

ash/system/power/power_event_observer.h

@@ -6,18 +6,18 @@
 #define ASH_SYSTEM_POWER_POWER_EVENT_OBSERVER_H_
 
 #include "ash/ash_export.h"
+#include "ash/session/session_observer.h"
 #include "base/callback.h"
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "chromeos/dbus/power_manager_client.h"
-#include "chromeos/dbus/session_manager_client.h"
 
 namespace ash {
 
 // A class that observes power-management-related events.
 class ASH_EXPORT PowerEventObserver
     : public chromeos::PowerManagerClient::Observer,
-      public chromeos::SessionManagerClient::Observer {
+      public SessionObserver {
  public:
   // This class registers/unregisters itself as an observer in ctor/dtor.
   PowerEventObserver();
@@ -33,9 +33,10 @@ class ASH_EXPORT PowerEventObserver
   void SuspendImminent(power_manager::SuspendImminent::Reason reason) override;
   void SuspendDone(const base::TimeDelta& sleep_duration) override;
 
-  // chromeos::SessionManagerClient::Observer overrides.
-  void ScreenIsLocked() override;
-  void ScreenIsUnlocked() override;
+  // SessionObserver overrides:
+  void OnLockStateChanged(bool locked) override;
+
+  ScopedSessionObserver session_observer_;
 
   // Is the screen currently locked?
   bool screen_locked_;

ash/system/power/power_event_observer_unittest.cc

@@ -77,14 +77,14 @@ TEST_F(PowerEventObserverTest, LockBeforeSuspend) {
 
   // It should run the callback when it hears that the screen is locked and the
   // lock screen animations have completed.
-  observer_->ScreenIsLocked();
+  BlockUserSession(BLOCKED_BY_LOCK_SCREEN);
   observer_->OnLockAnimationsComplete();
   EXPECT_EQ(0, client->GetNumPendingSuspendReadinessCallbacks());
 
   // If the system is already locked, no callback should be requested.
   observer_->SuspendDone(base::TimeDelta());
-  observer_->ScreenIsUnlocked();
-  observer_->ScreenIsLocked();
+  UnblockUserSession();
+  BlockUserSession(BLOCKED_BY_LOCK_SCREEN);
   observer_->OnLockAnimationsComplete();
   observer_->SuspendImminent(power_manager::SuspendImminent_Reason_OTHER);
   EXPECT_EQ(0, client->GetNumPendingSuspendReadinessCallbacks());
@@ -114,7 +114,7 @@ TEST_F(PowerEventObserverTest, SetInvisibleBeforeSuspend) {
   observer_->SuspendImminent(power_manager::SuspendImminent_Reason_OTHER);
   EXPECT_EQ(1, GetNumVisibleCompositors());
 
-  observer_->ScreenIsLocked();
+  BlockUserSession(BLOCKED_BY_LOCK_SCREEN);
   EXPECT_EQ(1, GetNumVisibleCompositors());
 
   observer_->OnLockAnimationsComplete();
@@ -133,7 +133,7 @@ TEST_F(PowerEventObserverTest, CanceledSuspend) {
   EXPECT_EQ(1, GetNumVisibleCompositors());
 
   observer_->SuspendDone(base::TimeDelta());
-  observer_->ScreenIsLocked();
+  BlockUserSession(BLOCKED_BY_LOCK_SCREEN);
   observer_->OnLockAnimationsComplete();
   EXPECT_EQ(1, GetNumVisibleCompositors());
 }
@@ -157,7 +157,7 @@ TEST_F(PowerEventObserverTest, DelayResuspendForLockAnimations) {
   observer_->SuspendImminent(power_manager::SuspendImminent_Reason_OTHER);
   EXPECT_EQ(1, client->GetNumPendingSuspendReadinessCallbacks());
 
-  observer_->ScreenIsLocked();
+  BlockUserSession(BLOCKED_BY_LOCK_SCREEN);
   observer_->SuspendDone(base::TimeDelta());
   observer_->SuspendImminent(power_manager::SuspendImminent_Reason_OTHER);
 

chrome/browser/chromeos/extensions/extension_system_event_observer.cc

@@ -6,17 +6,19 @@
 
 #include "chrome/browser/extensions/api/system_private/system_private_api.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
+#include "components/session_manager/core/session_manager.h"
 
 namespace chromeos {
 
-ExtensionSystemEventObserver::ExtensionSystemEventObserver() {
+ExtensionSystemEventObserver::ExtensionSystemEventObserver()
+    : screen_locked_(session_manager::SessionManager::Get()->IsScreenLocked()) {
   DBusThreadManager::Get()->GetPowerManagerClient()->AddObserver(this);
-  DBusThreadManager::Get()->GetSessionManagerClient()->AddObserver(this);
+  session_manager::SessionManager::Get()->AddObserver(this);
 }
 
 ExtensionSystemEventObserver::~ExtensionSystemEventObserver() {
   DBusThreadManager::Get()->GetPowerManagerClient()->RemoveObserver(this);
-  DBusThreadManager::Get()->GetSessionManagerClient()->RemoveObserver(this);
+  session_manager::SessionManager::Get()->RemoveObserver(this);
 }
 
 void ExtensionSystemEventObserver::BrightnessChanged(int level,
@@ -29,8 +31,11 @@ void ExtensionSystemEventObserver::SuspendDone(
   extensions::DispatchWokeUpEvent();
 }
 
-void ExtensionSystemEventObserver::ScreenIsUnlocked() {
-  extensions::DispatchScreenUnlockedEvent();
+void ExtensionSystemEventObserver::OnSessionStateChanged() {
+  const bool was_locked = screen_locked_;
+  screen_locked_ = session_manager::SessionManager::Get()->IsScreenLocked();
+  if (was_locked && !screen_locked_)
+    extensions::DispatchScreenUnlockedEvent();
 }
 
 }  // namespace chromeos

chrome/browser/chromeos/extensions/extension_system_event_observer.h

@@ -8,13 +8,14 @@
 #include "base/compiler_specific.h"
 #include "base/macros.h"
 #include "chromeos/dbus/power_manager_client.h"
-#include "chromeos/dbus/session_manager_client.h"
+#include "components/session_manager/core/session_manager_observer.h"
 
 namespace chromeos {
 
 // Dispatches extension events in response to system events.
-class ExtensionSystemEventObserver : public PowerManagerClient::Observer,
-                                     public SessionManagerClient::Observer {
+class ExtensionSystemEventObserver
+    : public PowerManagerClient::Observer,
+      public session_manager::SessionManagerObserver {
  public:
   // This class registers/unregisters itself as an observer in ctor/dtor.
   ExtensionSystemEventObserver();
@@ -24,10 +25,12 @@ class ExtensionSystemEventObserver : public PowerManagerClient::Observer,
   void BrightnessChanged(int level, bool user_initiated) override;
   void SuspendDone(const base::TimeDelta& sleep_duration) override;
 
-  // SessionManagerClient::Observer override.
-  void ScreenIsUnlocked() override;
+  // session_manager::SessionManagerObserver override:
+  void OnSessionStateChanged() override;
 
  private:
+  bool screen_locked_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(ExtensionSystemEventObserver);
 };
 

chromeos/dbus/session_manager_client.cc

@@ -727,14 +727,10 @@ class SessionManagerClientImpl : public SessionManagerClient {
 
   void ScreenIsLockedReceived(dbus::Signal* signal) {
     screen_is_locked_ = true;
-    for (auto& observer : observers_)
-      observer.ScreenIsLocked();
   }
 
   void ScreenIsUnlockedReceived(dbus::Signal* signal) {
     screen_is_locked_ = false;
-    for (auto& observer : observers_)
-      observer.ScreenIsUnlocked();
   }
 
   void ArcInstanceStoppedReceived(dbus::Signal* signal) {
@@ -852,7 +848,7 @@ class SessionManagerClientStubImpl : public SessionManagerClient {
   SessionManagerClientStubImpl() = default;
   ~SessionManagerClientStubImpl() override = default;
 
-  // SessionManagerClient overrides
+  // SessionManagerClient overrides:
   void Init(dbus::Bus* bus) override {}
   void SetStubDelegate(StubDelegate* delegate) override {
     delegate_ = delegate;
@@ -881,16 +877,8 @@ class SessionManagerClientStubImpl : public SessionManagerClient {
     if (delegate_)
       delegate_->LockScreenForStub();
   }
-  void NotifyLockScreenShown() override {
-    screen_is_locked_ = true;
-    for (auto& observer : observers_)
-      observer.ScreenIsLocked();
-  }
-  void NotifyLockScreenDismissed() override {
-    screen_is_locked_ = false;
-    for (auto& observer : observers_)
-      observer.ScreenIsUnlocked();
-  }
+  void NotifyLockScreenShown() override { screen_is_locked_ = true; }
+  void NotifyLockScreenDismissed() override { screen_is_locked_ = false; }
   void RetrieveActiveSessions(ActiveSessionsCallback callback) override {}
   void RetrieveDevicePolicy(RetrievePolicyCallback callback) override {
     base::FilePath owner_key_path;

chromeos/dbus/session_manager_client.h

@@ -58,15 +58,6 @@ class CHROMEOS_EXPORT SessionManagerClient : public DBusClient {
     // Called when the property change is complete.
     virtual void PropertyChangeComplete(bool success) {}
 
-    // Called when the session manager announces that the screen has been locked
-    // successfully (i.e. after NotifyLockScreenShown() has been called).
-    virtual void ScreenIsLocked() {}
-
-    // Called when the session manager announces that the screen has been
-    // unlocked successfully (i.e. after NotifyLockScreenDismissed() has
-    // been called).
-    virtual void ScreenIsUnlocked() {}
-
     // Called after EmitLoginPromptVisible is called.
     virtual void EmitLoginPromptVisibleCalled() {}
 
@@ -95,14 +86,14 @@ class CHROMEOS_EXPORT SessionManagerClient : public DBusClient {
   // remains with the caller.
   virtual void SetStubDelegate(StubDelegate* delegate) = 0;
 
-  // Adds and removes the observer.
+  // Adds or removes an observer.
   virtual void AddObserver(Observer* observer) = 0;
   virtual void RemoveObserver(Observer* observer) = 0;
   virtual bool HasObserver(const Observer* observer) const = 0;
 
   // Returns the most recent screen-lock state received from session_manager.
-  // This mirrors the last Observer::ScreenIsLocked() or ScreenIsUnlocked()
-  // call.
+  // This method should only be called by low-level code that is unable to
+  // depend on UI code and get the lock state from it instead.
   virtual bool IsScreenLocked() const = 0;
 
   // Kicks off an attempt to emit the "login-prompt-visible" upstart signal.
@@ -132,13 +123,14 @@ class CHROMEOS_EXPORT SessionManagerClient : public DBusClient {
   // Triggers a TPM firmware update.
   virtual void StartTPMFirmwareUpdate(const std::string& update_mode) = 0;
 
-  // Locks the screen.
+  // Sends a request to lock the screen to session_manager. Locking occurs
+  // asynchronously.
   virtual void RequestLockScreen() = 0;
 
-  // Notifies that the lock screen is shown.
+  // Notifies session_manager that Chrome has shown the lock screen.
   virtual void NotifyLockScreenShown() = 0;
 
-  // Notifies that the lock screen is dismissed.
+  // Notifies session_manager that Chrome has hidden the lock screen.
   virtual void NotifyLockScreenDismissed() = 0;
 
   // Notifies that supervised user creation have started.