New issue
Advanced search Search tips
Note: Color blocks (like █ or █) mean that a user may not be available. Tooltip shows the reason.

Issue 745006 link

Starred by 1 user
Status: Fixed
Owner:
█ xiaoyinh@chromium.org
Last visit > 30 days ago
Closed: Jul 24
Cc:
xiy...@chromium.org
r...@chromium.org
jamescook@chromium.org
jdufault@chromium.org
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug



Sign in to add a comment

views based lockscreen is shown in mash config on desktop

Project Member Reported by xiaoyinh@chromium.org, Jul 17 2017 
I just run mash on the desktop, and click on the lock button won't start the lock screen.
Looking at the code, clicking on the lock button will send request to session manager: https://cs.chromium.org/chromium/src/ash/system/tiles/tiles_default_view.cc?rcl=8d8f44b58664f8e29248bfe502b0ae1503a690c4&l=134

On desktop build, this goes to SessionManagerClientStubImpl here https://cs.chromium.org/chromium/src/chromeos/dbus/session_manager_client.cc?rcl=d4fa4f78601e7ab27b0ce4b1e65e3bfb3187d12e&l=906.

ScreenLockObserver is initialized and set itself to the delegate of SessionManagerClient. https://cs.chromium.org/chromium/src/chrome/browser/chromeos/login/lock/screen_locker.cc?rcl=8d8f44b58664f8e29248bfe502b0ae1503a690c4&l=118

In classic Ash, it will eventually call into ScreenLockObserver::LockScreenForStub.

In Mash config, SessionManagerClient couldn't find the delegate to lock the screen.
+jamescook, is this because DBusThreadManager::Get()->GetSessionManagerClient() returns different clients for chrome and ash process? And the ScreenLockObserver only set itself as delegate to SessionManagerClient in chrome process?

Comment 1 by jamescook@chromium.org, Jul 17 2017 

Yes, under mash code in chrome browser runs in a separate process than code in ash. Therefore there are two DBusThreadManagers and two SessionManagerClients (one in each process).

If you need stub behavior on desktop you'll either need to have separate stubs in each process or have the code in ash invoke a mojo method to tell the browser to do something. See session_controller.mojom and lock_screen.mojom

Hope that helps.
Project Member

Comment 2 by sheriffbot@chromium.org, Jul 18

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 3 by jamescook@chromium.org, Jul 23

Labels: -Hotlist-Recharge-Cold
This is still an issue with ToT.

Comment 4 by xiy...@chromium.org, Jul 24 

I have a CL to play with lock screen on devbox before.

https://chromium-review.googlesource.com/c/chromium/src/+/1148654
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 24 

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

commit f650f0752a280fc8b532eab473dc346589d3cb20
Author: Xiyuan Xia <xiyuan@chromium.org>
Date: Tue Jul 24 18:14:36 2018

mash: Allow lock on devbox.

Lock screen via SessionController so that it also works on
dev box.

Bug:  745006 
Change-Id: Ic269926063741be5d876dcbc9188eee7da12604f
Reviewed-on: https://chromium-review.googlesource.com/1148654
Commit-Queue: Xiyuan Xia <xiyuan@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#577609}
[modify] https://crrev.com/f650f0752a280fc8b532eab473dc346589d3cb20/ash/system/tiles/tiles_default_view.cc
[modify] https://crrev.com/f650f0752a280fc8b532eab473dc346589d3cb20/ash/system/unified/unified_system_tray_controller.cc

Comment 6 by xiy...@chromium.org, Jul 24

Status: Fixed (was: Untriaged)
►

Sign in to add a comment