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

Issue 793112 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Jan 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug
Team-Accessibility



Sign in to add a comment

Make chrome.accessibilityPrivate.darkenScreen work in mash

Project Member Reported by derat@chromium.org, Dec 7 2017

Issue description

For  issue 760730 , I'm updating the chrome.accessibilityPrivate.darkenScreen API to turn the screen off using ash::BacklightsForcedOffSetter (added for issue 767711) rather than calling powerd's SetBacklightsForcedOff D-Bus method directly. (The problems with the current approach are described in  issue 760730 .)

Since the API lives in Chrome, this won't work in mash without adding a new way for Chrome to call into ash via Mojo. It's possible that it would make sense to instead move BacklightsForcedOffSetter (or something like it) into the device service, since that's probably where a lot of other power-management-related code will live in the future. Issue 644348 is tracking mash powerd stuff.
 
I don't think there is anyone working on the device service now (I think it got deprioritized in favor of identity service). As such, adding a mojo call to ash to do this sounds like the right thing to do to me.

Alternately, if powerd could handle multiple dbus clients for this method, we could let both chrome and ash talk to it.

Comment 2 by derat@chromium.org, Dec 13 2017

I think it makes sense to keep this in ash. Display configuration / power-settings is already a complicated dance between powerd and Chrome, and having multiple processes providing input into powerd will probably make things even harder to understand and debug.
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 13 2017

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

commit 477c56da2daf91ce8cb1f8abacad62e70f73bde7
Author: Daniel Erat <derat@chromium.org>
Date: Wed Dec 13 21:42:54 2017

chromeos: Make darkenScreen a11y API work with other callers

Update the chrome.accessibilityPrivate.darkenScreen API to
use ash::BacklightsForcedOffSetter to force backlights off
rather than calling powerd's SetBacklightsForcedOff D-Bus
method directly.

Otherwise, the backlights can get into an unexpected state
due to ChromeVox and ash's power button code both calling
the D-Bus method, which was only designed for a single
caller.

      Search+Shift+F7 darkens the screen and Search+F7
      undarkens it

Bug:  760730 ,  793112 
Test: enabled ChromeVox with Ctrl+Alt+Z and checked that
Change-Id: I6e5d2f11746ce7ae2e34987a413f2655747e9532
Reviewed-on: https://chromium-review.googlesource.com/815762
Reviewed-by: David Tseng <dtseng@chromium.org>
Commit-Queue: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#523890}
[modify] https://crrev.com/477c56da2daf91ce8cb1f8abacad62e70f73bde7/ash/shell.h
[modify] https://crrev.com/477c56da2daf91ce8cb1f8abacad62e70f73bde7/chrome/browser/accessibility/accessibility_extension_api.cc
[modify] https://crrev.com/477c56da2daf91ce8cb1f8abacad62e70f73bde7/chrome/browser/chromeos/accessibility/accessibility_manager.cc
[modify] https://crrev.com/477c56da2daf91ce8cb1f8abacad62e70f73bde7/chrome/browser/chromeos/accessibility/accessibility_manager.h

Comment 4 by warx@chromium.org, Jan 8 2018

Owner: warx@chromium.org
Status: Assigned (was: Untriaged)
Based on the discussion we have, I would implement the chrome/ call ash controller's SetDarkenScreen mojo API.
Project Member

Comment 5 by bugdroid1@chromium.org, Jan 9 2018

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

commit 4e5853d387094c47b6146f4a5ad5da43db22fe61
Author: Qiang Xu <warx@chromium.org>
Date: Tue Jan 09 08:36:07 2018

mash: make SetDarkenScreen mojo call

changes:
chrome.accessibilityPrivate.darkenScreen needs to call
ash::BacklightsForcedOffSetter. This CL defines SetDarkenScreen mojo
call for this chrome-to-ash call.

TBR=dtseng@chromium.org

Search+Shift+F7 darkens the screen and Search+F7 undarkens it.

Bug:  793112 
Test: enabled ChromeVox with Ctrl+Alt+Z and checked that
Change-Id: I22219e11cae258f6aba9f0820783f37a262455db
Reviewed-on: https://chromium-review.googlesource.com/853753
Commit-Queue: Qiang(Joe) Xu <warx@chromium.org>
Reviewed-by: Qiang(Joe) Xu <warx@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Cr-Commit-Position: refs/heads/master@{#527938}
[modify] https://crrev.com/4e5853d387094c47b6146f4a5ad5da43db22fe61/ash/accessibility/accessibility_controller.cc
[modify] https://crrev.com/4e5853d387094c47b6146f4a5ad5da43db22fe61/ash/accessibility/accessibility_controller.h
[modify] https://crrev.com/4e5853d387094c47b6146f4a5ad5da43db22fe61/ash/accessibility/accessibility_controller_unittest.cc
[modify] https://crrev.com/4e5853d387094c47b6146f4a5ad5da43db22fe61/ash/public/interfaces/accessibility_controller.mojom
[modify] https://crrev.com/4e5853d387094c47b6146f4a5ad5da43db22fe61/chrome/browser/chromeos/accessibility/accessibility_manager.cc
[modify] https://crrev.com/4e5853d387094c47b6146f4a5ad5da43db22fe61/chrome/browser/chromeos/accessibility/accessibility_manager.h
[modify] https://crrev.com/4e5853d387094c47b6146f4a5ad5da43db22fe61/chrome/browser/ui/ash/accessibility/accessibility_controller_client_unittest.cc

Comment 6 by warx@chromium.org, Jan 9 2018

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

Sign in to add a comment