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

Issue 747019 link

Starred by 1 user

Issue metadata

Status: Archived
Owner:
Closed: Aug 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 647412



Sign in to add a comment

mash: Migrate logout button code from SystemTrayDelgateChromeOS into ash

Project Member Reported by jamescook@chromium.org, Jul 20 2017

Issue description

c/b/ui/ash/system_tray_delegate_chromeos.cc has code to support some kiosk features around logout:

* Show a red button in system tray labeled "logout" which shows a dialog
* Make the dialog close after a specific amount of time
* Exit the session when all windows are closed

These features are controlled by enterprise policy via prefs.

For go/mustash I'm trying to eliminate SystemTrayDelegate and eliminate code that synchronously calls into ash (since browser and ash are separate processes). Ash is capable of observing for pref changes over mojo, so I'm going to try making it observe the prefs directly.

Things are complicated by multiprofile support. The code in SystemTrayDelegateChromeOS changes its pref observers when the user profile changes.

bartfab, do you know offhand if this feature is supposed to work with multiprofile? It seems like it is targeted at kiosks and I suspect multiprofile isn't supported there.

 
Blocking: 647412
This feature is typically used in Public Sessions only, but its implementation is intentionally orthogonal to session type and multiprofile - it should work in all cases. In case of multiprofile, the primary user's prefs/policy control the button.
Project Member

Comment 3 by bugdroid1@chromium.org, Jul 31 2017

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

commit 8532dbc1799380b044b950372e23296259d267e0
Author: James Cook <jamescook@chromium.org>
Date: Mon Jul 31 21:54:51 2017

mash: Connect to the correct user profile pref service on startup

Previously the pref service was connected to the temporary login screen
profile prefs. Now it connects to the user profile on login and after
user switch.

Rename RegisterPrefs() to RegisterProfilePrefs() in ash to emphasize
that these are profile-associated and not local state.

Add ShellObserver notification of PrefService change. The lifetime is
tricky because the old PrefService must continue to exist so shell
observers can unregister their pref observers. Also, there is a window
using user switch where the user pref service is null.

This is based on https://chromium-review.googlesource.com/c/580115
and is a prerequisite for moving some system tray code out of chrome
into ash.

Bug:  747019 ,  721961 , 736169
Test: added to ash_unittests
Change-Id: I186e55c1d88e0e2052b9cd76a490e26804e4b6d3
Reviewed-on: https://chromium-review.googlesource.com/590356
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490804}
[modify] https://crrev.com/8532dbc1799380b044b950372e23296259d267e0/ash/shell.cc
[modify] https://crrev.com/8532dbc1799380b044b950372e23296259d267e0/ash/shell.h
[modify] https://crrev.com/8532dbc1799380b044b950372e23296259d267e0/ash/shell_observer.h
[modify] https://crrev.com/8532dbc1799380b044b950372e23296259d267e0/ash/shell_unittest.cc
[modify] https://crrev.com/8532dbc1799380b044b950372e23296259d267e0/ash/system/night_light/night_light_controller.cc
[modify] https://crrev.com/8532dbc1799380b044b950372e23296259d267e0/ash/system/night_light/night_light_controller.h
[modify] https://crrev.com/8532dbc1799380b044b950372e23296259d267e0/ash/system/night_light/night_light_controller_unittest.cc
[modify] https://crrev.com/8532dbc1799380b044b950372e23296259d267e0/ash/system/night_light/tray_night_light_unittest.cc
[modify] https://crrev.com/8532dbc1799380b044b950372e23296259d267e0/chrome/browser/chromeos/preferences.cc
[modify] https://crrev.com/8532dbc1799380b044b950372e23296259d267e0/chrome/browser/prefs/active_profile_pref_service.cc
[modify] https://crrev.com/8532dbc1799380b044b950372e23296259d267e0/chrome/browser/prefs/active_profile_pref_service.h

Project Member

Comment 4 by bugdroid1@chromium.org, Aug 1 2017

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

commit dc04112c020870fc087be4694e0b55494f4c270a
Author: James Cook <jamescook@chromium.org>
Date: Tue Aug 01 14:03:22 2017

cros: Support status area logout button under mash

The button is only shown when enabled by enterprise policy prefs,
which are observed in chrome in SystemTrayDelegateChromeOS. For
go/mustash this code cannot directly call into ash because it
runs in a different process. Migrate the prefs observation into ash.

Add unit tests for LogoutButtonTray.

Remove some unused pref declarations and fix some USE_AURA vs.
OS_CHROMEOS confusion.

Bug:  747019 
Test: ash_unittests
Change-Id: Ifae5fedd163d0f3f9913369538856f13ba37c332
Reviewed-on: https://chromium-review.googlesource.com/590154
Reviewed-by: Thiemo Nagel <tnagel@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Commit-Queue: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/heads/master@{#490982}
[modify] https://crrev.com/dc04112c020870fc087be4694e0b55494f4c270a/ash/BUILD.gn
[modify] https://crrev.com/dc04112c020870fc087be4694e0b55494f4c270a/ash/public/cpp/ash_pref_names.cc
[modify] https://crrev.com/dc04112c020870fc087be4694e0b55494f4c270a/ash/public/cpp/ash_pref_names.h
[modify] https://crrev.com/dc04112c020870fc087be4694e0b55494f4c270a/ash/shell.cc
[delete] https://crrev.com/ebc21ea05638561d60da231a0e52e50d712520e5/ash/system/session/logout_button_observer.h
[modify] https://crrev.com/dc04112c020870fc087be4694e0b55494f4c270a/ash/system/session/logout_button_tray.cc
[modify] https://crrev.com/dc04112c020870fc087be4694e0b55494f4c270a/ash/system/session/logout_button_tray.h
[add] https://crrev.com/dc04112c020870fc087be4694e0b55494f4c270a/ash/system/session/logout_button_tray_unittest.cc
[modify] https://crrev.com/dc04112c020870fc087be4694e0b55494f4c270a/ash/system/tray/system_tray_notifier.cc
[modify] https://crrev.com/dc04112c020870fc087be4694e0b55494f4c270a/ash/system/tray/system_tray_notifier.h
[modify] https://crrev.com/dc04112c020870fc087be4694e0b55494f4c270a/chrome/browser/policy/configuration_policy_handler_list_factory.cc
[modify] https://crrev.com/dc04112c020870fc087be4694e0b55494f4c270a/chrome/browser/ui/ash/chrome_launcher_prefs.cc
[modify] https://crrev.com/dc04112c020870fc087be4694e0b55494f4c270a/chrome/browser/ui/ash/system_tray_delegate_chromeos.cc
[modify] https://crrev.com/dc04112c020870fc087be4694e0b55494f4c270a/chrome/browser/ui/ash/system_tray_delegate_chromeos.h
[modify] https://crrev.com/dc04112c020870fc087be4694e0b55494f4c270a/chrome/common/pref_names.cc
[modify] https://crrev.com/dc04112c020870fc087be4694e0b55494f4c270a/chrome/common/pref_names.h

Status: Fixed (was: Started)
Project Member

Comment 6 by bugdroid1@chromium.org, Aug 2 2017

Labels: merge-merged-3163
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2b2cd05166ee2fa3399d8351a65bc3599a8c5122

commit 2b2cd05166ee2fa3399d8351a65bc3599a8c5122
Author: James Cook <jamescook@chromium.org>
Date: Wed Aug 02 01:10:24 2017

M61 backport: mash: Connect to the correct user profile pref service on startup

Previously the pref service was connected to the temporary login screen
profile prefs. Now it connects to the user profile on login and after
user switch.

Rename RegisterPrefs() to RegisterProfilePrefs() in ash to emphasize
that these are profile-associated and not local state.

Add ShellObserver notification of PrefService change. The lifetime is
tricky because the old PrefService must continue to exist so shell
observers can unregister their pref observers. Also, there is a window
using user switch where the user pref service is null.

This is based on https://chromium-review.googlesource.com/c/580115
and is a prerequisite for moving some system tray code out of chrome
into ash.

TBR=jamescook@chromium.org

(cherry picked from commit 8532dbc1799380b044b950372e23296259d267e0)

Bug:  747019 ,  721961 , 736169
Test: added to ash_unittests
Change-Id: I186e55c1d88e0e2052b9cd76a490e26804e4b6d3
Reviewed-on: https://chromium-review.googlesource.com/590356
Commit-Queue: James Cook <jamescook@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Sam McNally <sammc@chromium.org>
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#490804}
Reviewed-on: https://chromium-review.googlesource.com/597313
Reviewed-by: James Cook <jamescook@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#227}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify] https://crrev.com/2b2cd05166ee2fa3399d8351a65bc3599a8c5122/ash/shell.cc
[modify] https://crrev.com/2b2cd05166ee2fa3399d8351a65bc3599a8c5122/ash/shell.h
[modify] https://crrev.com/2b2cd05166ee2fa3399d8351a65bc3599a8c5122/ash/shell_observer.h
[modify] https://crrev.com/2b2cd05166ee2fa3399d8351a65bc3599a8c5122/ash/shell_unittest.cc
[modify] https://crrev.com/2b2cd05166ee2fa3399d8351a65bc3599a8c5122/ash/system/night_light/night_light_controller.cc
[modify] https://crrev.com/2b2cd05166ee2fa3399d8351a65bc3599a8c5122/ash/system/night_light/night_light_controller.h
[modify] https://crrev.com/2b2cd05166ee2fa3399d8351a65bc3599a8c5122/ash/system/night_light/night_light_controller_unittest.cc
[modify] https://crrev.com/2b2cd05166ee2fa3399d8351a65bc3599a8c5122/ash/system/night_light/tray_night_light_unittest.cc
[modify] https://crrev.com/2b2cd05166ee2fa3399d8351a65bc3599a8c5122/chrome/browser/chromeos/preferences.cc
[modify] https://crrev.com/2b2cd05166ee2fa3399d8351a65bc3599a8c5122/chrome/browser/prefs/active_profile_pref_service.cc
[modify] https://crrev.com/2b2cd05166ee2fa3399d8351a65bc3599a8c5122/chrome/browser/prefs/active_profile_pref_service.h

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

Status: Archived (was: Fixed)
Components: -Internals>MUS Internals>Services>WindowService

Sign in to add a comment