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

Issue 807861 link

Starred by 4 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

powerd suspends immediately after locking screen due to inactivity

Project Member Reported by derat@chromium.org, Feb 1 2018

Issue description

(forked from  issue 807511 )

When auto screen-lock is enabled, the device is on battery power, and inactivity delays are lengthened due to earlier user activity, powerd can request that the device be suspended immediately after the screen is automatically locked due to inactivity, which is a bit weird. I think that we've had this behavior since the short lock screen inactivity delays landed in M41 (early 2015, see  issue 190499 ).

From powerd.LATEST in https://listnr.corp.google.com/report/84996901327:

[0129/103819:INFO:state_controller.cc(437)] Scaling delays due to user activity while screen was dimmed or soon after it was turned off
...
[0129/132154:INFO:daemon.cc(1443)] Received updated external policy: ac_dim=7m ac_screen_off=7m30s ac_lock=7m40s ac_idle_warn=0s ac_idle=30m battery_dim=5m battery_screen_off=5m30s battery_lock
=5m40s battery_idle_warn=0s battery_idle=6m30s ac_idle=suspend battery_idle=suspend lid_closed=suspend use_audio=1 use_video=1 presentation_factor=2.0 user_activity_factor=2.0 wait_for_initial_
user_activity=0 force_nonzero_brightness_for_user_activity=1 (Prefs)
[0129/132154:INFO:state_controller.cc(855)] Updated settings: dim=10m screen_off=10m30s lock=10m40s idle_warn=0s idle=11m30s (suspend) lid_closed=suspend use_audio=1 use_video=1
...
[0129/134624:INFO:state_controller.cc(89)] Dimming screen after 10m
[0129/134624:INFO:internal_backlight_controller.cc(677)] Setting brightness to 6 (14.1949%) over 200 ms
[0129/134625:INFO:daemon.cc(782)] On battery at 91% (displayed as 93%), 5.684/6.271Ah at 0.435A, 11h25m until empty (11h22m until shutdown)
[0129/134654:INFO:state_controller.cc(89)] Turning screen off after 10m30s
[0129/134654:INFO:internal_backlight_controller.cc(677)] Setting brightness to 0 (0%) over 200 ms
[0129/134654:INFO:display_power_setter.cc(81)] Asking DisplayService to turn all displays off
[0129/134655:INFO:daemon.cc(1425)] Chrome is using normal display mode
[0129/134655:INFO:daemon.cc(782)] On battery at 91% (displayed as 93%), 5.681/6.271Ah at 0.446A, 11h30m20s until empty (11h27m20s until shutdown)
[0129/134704:INFO:state_controller.cc(89)] Locking screen after 10m40s
[0129/134704:INFO:daemon.cc(1443)] Received updated external policy: ac_dim=30s ac_screen_off=40s ac_lock=50s ac_idle_warn=0s ac_idle=30m battery_dim=30s battery_screen_off=40s battery_lock=50s battery_idle_warn=0s battery_idle=6m30s ac_idle=suspend battery_idle=suspend lid_closed=suspend use_audio=1 use_video=1 presentation_factor=2.0 user_activity_factor=2.0 wait_for_initial_user_activity=0 force_nonzero_brightness_for_user_activity=1 (Prefs)
[0129/134704:INFO:state_controller.cc(855)] Updated settings: dim=1m screen_off=1m10s lock=1m20s idle_warn=0s idle=7m (suspend) lid_closed=suspend use_audio=1 use_video=1
[0129/134704:INFO:state_controller.cc(996)] Ready to perform idle action (suspend) after 10m29s

The 5m/5m30s/5m40s/6m30s battery delays are extended to 10m/10m30s/10m40s/11m30s due to earlier user activity (see https://chromium.googlesource.com/chromiumos/platform2/+/master/power_manager/docs/inactivity_delays.md for more info). At 10m40s, we lock the screen as expected, which results in Chrome sending powerd an updated policy with battery delays of 30s/40s/50s/6m30s, again as expected. When powerd extends the delays this time, they just go to 1m/1m10s/1m20s/7m. We're past the seven-minute mark, so we start suspending immediately.

This is expected given the way that inactivity delays work, but the interaction between the shortened dim/off delays while the lock screen is shown and the delay-doubling logic is weird and unexpected.
 

Comment 1 by derat@chromium.org, Feb 1 2018

Cc: derat@chromium.org
 Issue 807850  has been merged into this issue.
"At 10m40s, we lock the screen as expected, which results in Chrome sending powerd an updated policy with battery delays of 30s/40s/50s/6m30s, again as expected."

Why do we do this? My memory is super hazy on that part. The screen is locked now, but it is still the same user's session. Why do we not keep the delays that were in effect during the session? It is not like we expose separate controls of "delays on the lock screen" anywhere.

Comment 3 by derat@chromium.org, Feb 7 2018

> It is not like we expose separate controls of "delays on the lock screen" anywhere.

Well...  https://crbug.com/190499  :-P

(I don't think they were ever connected to policies.)
Thanks for helping my memory :). I can think of a couple of ways out of this:
1) Add more settings and policies for the lock screen. I think this will be truly impossible for regular humans to keep track of.
2) Set lock screen timeouts to the same as in-session timeouts (essentially reverting to pre- bug 190499 ).
3) Make the shorter lock screen timeouts only apply if the lock screen was triggered by the user.
4) Add more corner case code.

For enterprise, my preference would be (2), followed by (3). I presume (3) would work for consumer as well while (2) would be a bad idea as it reopens an old bug.

Comment 5 by derat@chromium.org, Feb 7 2018

Thanks for enumerating the options. I'm probably going to look at 4, but I won't make you review it. :-P
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 29 2018

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

commit f22d1295d84f4365a69d4c30823eddd9134a4c98
Author: Daniel Erat <derat@chromium.org>
Date: Thu Mar 29 20:40:18 2018

chromeos: Defer delay change after idle screen lock.

Avoid a subtle issue in the PowerPrefs class where we can
unexpectedly suspend immediately after automatically locking
the screen as a result of switching to the set of shorter
while-locked inactivity delays. Instead, we hold off on
using the shorter delays until the screen has been turned
back on or manually turned off.

Bug:  807861 
Change-Id: Ic5a6b74381c67666b7aed94d60d487eb77004658
Reviewed-on: https://chromium-review.googlesource.com/986461
Commit-Queue: Dan Erat <derat@chromium.org>
Reviewed-by: Toni Barzic <tbarzic@chromium.org>
Cr-Commit-Position: refs/heads/master@{#546938}
[modify] https://crrev.com/f22d1295d84f4365a69d4c30823eddd9134a4c98/chrome/browser/chromeos/chrome_browser_main_chromeos.cc
[modify] https://crrev.com/f22d1295d84f4365a69d4c30823eddd9134a4c98/chrome/browser/chromeos/power/power_prefs.cc
[modify] https://crrev.com/f22d1295d84f4365a69d4c30823eddd9134a4c98/chrome/browser/chromeos/power/power_prefs.h
[modify] https://crrev.com/f22d1295d84f4365a69d4c30823eddd9134a4c98/chrome/browser/chromeos/power/power_prefs_unittest.cc

Comment 7 by derat@chromium.org, Mar 29 2018

Status: Fixed (was: Assigned)
(The change in #6 essentially implements option 3 from #4.)
 Issue 871018  has been merged into this issue.

Sign in to add a comment