+derat, this is a tracking bug for our Q1 OKRs. Can you and jonross sort out who is going to own screen locking and/or collapse this bug into other ones?

So this is currently blocked on issue 644350 where the dbus SessionManagerClient has not transitioned to work cross-process.

The related SessionManagerClient::Observer could be transitioned to a mojo interface, to allow for the login screen to connect. However I'm not sure how that would work with current dbus mojo support.

derat@ thoughts?

I'm not familiar with where the screen lock code currently (or will) live in mustash. As I understand it, both the mash and browser processes have D-Bus connections -- is the login screen going to be somewhere else? If you can point me toward a doc that describes the eventual goal, it'd be much appreciated!

I thought the trickier part of this may be moving chromeos::ScreenLockServiceProvider, which receives screen-lock D-Bus requests from session_manager, outside of the org.chromium.LibCrosService D-Bus interface. I started on some of the work needed to enable that (see  issue 692246 ).

There's also the "small" matter of moving the screen-locking code outside of c/b/chromeos/login (or is all of that going to continue to live inside of the browser?).

+xiyuan, jamescook

I'm not sure who the authoritative source should be of screen lock state. Having ash maintain the state would be ideal and where we want to go in the long term, but I don't know what the dependencies are like right now. 

I don't have much to add here. I agree ash is the right long-term place to maintain the state, but I suspect there are heavy dependencies on browser code / login code for the lock screen.

Adding  issue 648964  as a blocker, as xiyuan@ was working on large updates to SessionStateDelegate, including login unification.

If we have the unified location for performing the login/lock actions, the browser login screen code can migrate to using that.

I can give it a crack after the SessionStateDelegate -> SessionController (mojo based) work is done in  issue 648964 . 

Currently, the lock state is exposed to ash in a couple of ways:
- via ShellObserver, there are OnLockStateChanged and OnLoginStateChanged
- via SessionStateDelegate, there is IsScreenLocked

ShellObserver::OnLoginStateChanged and SessionStateDelegate::IsScreenLocked is based on session_manager::SessionState. ShellObserver::OnLockStateChanged is based on NOTIFICATION_SCREEN_LOCK_STATE_CHANGED sent from ScreenLocker.

After switching to SessionController, we should consolidate them into observing SessionState of SessionController, e.g. via SessionStateObserver::SessionStateChanged

 Issue 648962  has been merged into this issue.

ScreenLocker has references to ash::Shell for lock_state_controller and wallpaper_controller. This would not work in mash.

 Issue 704269  has been merged into this issue.

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/233f4e71bd49e79e477ef3ba814d53e8036e3c50

commit 233f4e71bd49e79e477ef3ba814d53e8036e3c50
Author: xiyuan <xiyuan@chromium.org>
Date: Thu Apr 06 06:59:56 2017

ash: Merge LoginStatus update code path

- Add LoginStatusChanged to SessionStateObserver;
- Make SessionController fire LoginStatusChanged;
- Shell call UpdateAfterLoginStatusChange on LoginStatusChanged;
- Remove Shell::OnLoginStateChanged and notify ShellObserver in
  Shell::LoginStatusChanged;
- Remove UpdateAfterLoginStatusChange calls from SystemTrayDelegateChromeOS
  and TestSessionControllerClient;
- Remove Shell::OnLoginStateChanged calls from LoginLockStateNotifier
  since LoginStatusChanged should cover that;
- Fix ash_shell_with_content that SessionController is not bound
  to its client;

BUG=701193, 679450 

Review-Url: https://codereview.chromium.org/2797743002
Cr-Commit-Position: refs/heads/master@{#462378}

[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/ash/common/session/session_controller.cc
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/ash/common/session/session_controller.h
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/ash/common/session/session_controller_unittest.cc
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/ash/common/session/session_state_observer.h
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/ash/common/system/user/tray_user.cc
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/ash/common/test/test_session_controller_client.cc
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/ash/mus/system_tray_delegate_mus.cc
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/ash/shell.cc
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/ash/shell.h
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/ash/shell/content/client/shell_browser_main_parts.cc
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/ash/shell/example_session_controller_client.cc
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/ash/wm/lock_state_controller.cc
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/ash/wm/lock_state_controller.h
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/chrome/browser/chromeos/power/login_lock_state_notifier.cc
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/chrome/browser/chromeos/system/tray_accessibility_browsertest.cc
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/chrome/browser/ui/ash/session_controller_client.cc
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/chrome/browser/ui/ash/session_controller_client.h
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/chrome/browser/ui/ash/system_tray_delegate_chromeos.cc
[modify] https://crrev.com/233f4e71bd49e79e477ef3ba814d53e8036e3c50/chrome/browser/ui/ash/system_tray_delegate_chromeos.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9

commit 7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9
Author: xiyuan <xiyuan@chromium.org>
Date: Mon Apr 10 16:49:10 2017

mash: Run pre-unlock animation via SessionController

- Replace LockStateController::OnLockScreenHide with mojo call
  SessionController.RunUnlockAnimation so that it works on mash;
- Implement lock state change detection in SessionController
  and remove relevant part in LoginLockStateNotifier that needs
  to talk directly to Shell;

BUG= 679450 

Review-Url: https://codereview.chromium.org/2801333002
Cr-Commit-Position: refs/heads/master@{#463295}

[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/ash/public/interfaces/session_controller.mojom
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/ash/session/session_controller.cc
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/ash/session/session_controller.h
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/ash/session/session_controller_unittest.cc
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/ash/session/session_state_observer.h
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/ash/shell.cc
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/ash/shell.h
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/ash/shell_unittest.cc
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/ash/test/ash_test_base.cc
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/ash/wm/lock_state_controller.cc
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/ash/wm/lock_state_controller.h
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/ash/wm/system_modal_container_layout_manager_unittest.cc
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/chrome/browser/chromeos/login/lock/screen_locker.cc
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/chrome/browser/chromeos/power/login_lock_state_notifier.cc
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/chrome/browser/notifications/login_state_notification_blocker_chromeos_unittest.cc
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/chrome/browser/ui/ash/session_controller_client.cc
[modify] https://crrev.com/7ebbf7f4312a35d83beb00e2bb66e57bf229d5f9/chrome/browser/ui/ash/session_controller_client.h

SessionController allows mash screen lock to run on old code path and ash does not need to talk to session_manager via D-Bus directly. Remove a couple of dependent issues.