New issue
Advanced search Search tips

Issue 678988 link

Starred by 2 users
Status: Archived
Owner:
xiy...@chromium.org
Closed: Apr 2017
Cc:
derat@chromium.org
mustash-bugs@google.com
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 678705
issue 679450



Sign in to add a comment

mash: chrome ScreenLocker / LoginLockStateNotifier should not depend on ash::Shell

Project Member Reported by jamescook@chromium.org, Jan 6 2017 
chrome can't directly access ash::Shell under mash.

These are used to inform ash of lock state, trigger screen lock animations, and inform ash of login state changes (!).

For fishfood of mustash, we can skip the animations. However, ash still needs to know about lock and login state.

These need to either switch to new mojo interfaces or use the existing SessionController mojom interface.

Comment 1 by xiy...@chromium.org, Apr 12 2017

Cc: -xiy...@chromium.org
Owner: xiy...@chromium.org
Status: Started (was: Untriaged)

Comment 2 by xiy...@chromium.org, Apr 12 2017

Blocking: 679450
Project Member

Comment 3 by bugdroid1@chromium.org, Apr 18 2017 

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

commit 8d74137498475f0cf8ab7b31082a578ad64de504
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Tue Apr 18 22:19:41 2017

mash: Decommission LoginLockStateNotifier

- Move app terminating notification from LoginLockStateNotifier into
  SessionControllerClient;
- Add a SessionController::NotifyAppTerminating mojo call to pass
  the notification to ash;
- Decommission LoginLockStateNotifier;

BUG= 678988 

Change-Id: I497938cf7f218377f0ecad2084e09cfa2bb24ed6
Reviewed-on: https://chromium-review.googlesource.com/478055
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#465391}
[modify] https://crrev.com/8d74137498475f0cf8ab7b31082a578ad64de504/ash/public/interfaces/session_controller.mojom
[modify] https://crrev.com/8d74137498475f0cf8ab7b31082a578ad64de504/ash/session/session_controller.cc
[modify] https://crrev.com/8d74137498475f0cf8ab7b31082a578ad64de504/ash/session/session_controller.h
[modify] https://crrev.com/8d74137498475f0cf8ab7b31082a578ad64de504/ash/session/session_state_observer.h
[modify] https://crrev.com/8d74137498475f0cf8ab7b31082a578ad64de504/ash/shell.cc
[modify] https://crrev.com/8d74137498475f0cf8ab7b31082a578ad64de504/ash/shell.h
[modify] https://crrev.com/8d74137498475f0cf8ab7b31082a578ad64de504/ash/test/ash_test_base.cc
[modify] https://crrev.com/8d74137498475f0cf8ab7b31082a578ad64de504/ash/wm/video_detector_unittest.cc
[modify] https://crrev.com/8d74137498475f0cf8ab7b31082a578ad64de504/chrome/browser/chromeos/BUILD.gn
[modify] https://crrev.com/8d74137498475f0cf8ab7b31082a578ad64de504/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/8d74137498475f0cf8ab7b31082a578ad64de504/chrome/browser/chromeos/chrome_browser_main_chromeos.h
[delete] https://crrev.com/2501a834815e3a9e460051bd98f1eb49c7357e7b/chrome/browser/chromeos/power/login_lock_state_notifier.cc
[delete] https://crrev.com/2501a834815e3a9e460051bd98f1eb49c7357e7b/chrome/browser/chromeos/power/login_lock_state_notifier.h
[modify] https://crrev.com/8d74137498475f0cf8ab7b31082a578ad64de504/chrome/browser/ui/ash/session_controller_client.cc
[modify] https://crrev.com/8d74137498475f0cf8ab7b31082a578ad64de504/chrome/browser/ui/ash/session_controller_client.h
Project Member

Comment 4 by bugdroid1@chromium.org, Apr 20 2017 

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

commit 7f80c700a7f188fc1aa30713c4a486138f957c69
Author: xiyuan <xiyuan@chromium.org>
Date: Thu Apr 20 22:10:32 2017

cros: Use SessionController for lock starting code

- Add SessionController::StartLock to start locking on ash
  side and returns when post lock animation is finished and
  ash is fully locked, or aborted when ash exits before the
  lock code goes thread;
- Update ScreenLocker/WebUIScreenLocker to use StartLock instead
  of ash::LockStateController to start locking and observe
  lock animation;

BUG= 678988 

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

[modify] https://crrev.com/7f80c700a7f188fc1aa30713c4a486138f957c69/ash/public/interfaces/session_controller.mojom
[modify] https://crrev.com/7f80c700a7f188fc1aa30713c4a486138f957c69/ash/session/session_controller.cc
[modify] https://crrev.com/7f80c700a7f188fc1aa30713c4a486138f957c69/ash/session/session_controller.h
[modify] https://crrev.com/7f80c700a7f188fc1aa30713c4a486138f957c69/ash/wm/lock_state_controller.cc
[modify] https://crrev.com/7f80c700a7f188fc1aa30713c4a486138f957c69/ash/wm/lock_state_controller.h
[modify] https://crrev.com/7f80c700a7f188fc1aa30713c4a486138f957c69/chrome/browser/chromeos/login/lock/screen_locker.cc
[modify] https://crrev.com/7f80c700a7f188fc1aa30713c4a486138f957c69/chrome/browser/chromeos/login/lock/screen_locker.h
[modify] https://crrev.com/7f80c700a7f188fc1aa30713c4a486138f957c69/chrome/browser/chromeos/login/lock/webui_screen_locker.cc
[modify] https://crrev.com/7f80c700a7f188fc1aa30713c4a486138f957c69/chrome/browser/chromeos/login/lock/webui_screen_locker.h
[modify] https://crrev.com/7f80c700a7f188fc1aa30713c4a486138f957c69/chrome/browser/ui/ash/session_controller_client.cc
[modify] https://crrev.com/7f80c700a7f188fc1aa30713c4a486138f957c69/chrome/browser/ui/ash/session_controller_client.h
Project Member

Comment 6 by bugdroid1@chromium.org, Apr 24 2017 

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

commit 84e360a7f1dddd9ce125ad6f1de090b7d207c569
Author: xiyuan <xiyuan@chromium.org>
Date: Mon Apr 24 23:11:34 2017

mash: Shell ref clean up for screen lock

- Skip manipulating active window state to exit full screen
   http://crbug.com/714677  tracks work for mash;
- Update LockWindow creation code to run on mash;
- Add a SessionController::NotifyChromeLockAnimationsComplete
  to replace the direct ash::Shell PowerEventObserver call;

BUG= 678988 

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

[modify] https://crrev.com/84e360a7f1dddd9ce125ad6f1de090b7d207c569/ash/public/interfaces/session_controller.mojom
[modify] https://crrev.com/84e360a7f1dddd9ce125ad6f1de090b7d207c569/ash/session/session_controller.cc
[modify] https://crrev.com/84e360a7f1dddd9ce125ad6f1de090b7d207c569/ash/session/session_controller.h
[modify] https://crrev.com/84e360a7f1dddd9ce125ad6f1de090b7d207c569/chrome/browser/chromeos/login/lock/screen_locker.cc
[modify] https://crrev.com/84e360a7f1dddd9ce125ad6f1de090b7d207c569/chrome/browser/chromeos/login/lock/webui_screen_locker.cc
[modify] https://crrev.com/84e360a7f1dddd9ce125ad6f1de090b7d207c569/chrome/browser/chromeos/login/lock/webui_screen_locker.h
[modify] https://crrev.com/84e360a7f1dddd9ce125ad6f1de090b7d207c569/chrome/browser/chromeos/login/ui/lock_window.cc
[modify] https://crrev.com/84e360a7f1dddd9ce125ad6f1de090b7d207c569/chrome/browser/ui/ash/session_controller_client.cc
[modify] https://crrev.com/84e360a7f1dddd9ce125ad6f1de090b7d207c569/chrome/browser/ui/ash/session_controller_client.h
[modify] https://crrev.com/84e360a7f1dddd9ce125ad6f1de090b7d207c569/chrome/browser/ui/ash/session_controller_client_unittest.cc

Comment 7 by xiy...@chromium.org, Apr 24 2017

Status: Fixed (was: Started)
No crashes now. Separate issues filed to track
- lock window size ( issue 714804 )
- how to deal with fullscreen window ( issue 714677 ).

Comment 8 by dchan@google.com, May 30 2017

Labels: VerifyIn-60

Comment 9 by dchan@chromium.org, Aug 1 2017

Labels: VerifyIn-61

Comment 10 by dchan@chromium.org, Jan 22 2018

Status: Archived (was: Fixed)

Comment 11 by lafo...@chromium.org, Feb 26 2018

Components: -Internals>MUS Internals>Services>WindowService
►

Sign in to add a comment