New issue
Advanced search Search tips

Issue 894192 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 17
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug



Sign in to add a comment

Power menu sometimes automatically proceeds to power off

Project Member Reported by zalcorn@google.com, Oct 10

Issue description

Chrome Version       : 71.0.3567.0

What steps will reproduce the problem?
1. Press power button long enough to show power menu on nocturne
2. See that Power off has default focus
3. Watch device unexpectedly begin power down

I can reproduce this very reliably on most recent canary build. The power off animation doesn't begin until after the default focus ring appears. I can catch a video if helpful.


 
Components: OS>Kernel>Power
I've seen this too. My guess at the time was that powerd's reporting of the button-up event might've been delayed for some reason such that Chrome thought that the power button was held for longer than it actually was.

Zach, after triggering this, please log back in after rebooting and collect debug logs via chrome://net-internals/#chromeos so we can see if they provide any clues.
Thanks. Here's the relevant part of powerd.PREVIOUS:

[1010/133105:INFO:daemon.cc(473)] Power button down
[1010/133105:INFO:main.cc(247)] Launching "sync"
[1010/133105:INFO:daemon.cc(473)] Power button up
[1010/133108:INFO:daemon.cc(972)] Got RequestShutdown message from :1.23 with reason user-request (UI request from ash: power button)

We don't have sub-second timestamps, but that at least suggests that the power button down and up D-Bus signals were emitted within a single second -- the "Power button down" and "Power button up" messages are logged after we've emitted signals to notify Chrome.

So that makes me think that either there was jank on the Chrome UI thread that delayed its receipt of the button-up signal, or there's some other bug in Chrome's power button code that results in it shutting down despite seeing the button-up signal.
Labels: Restrict-View-Google
Gotcha - Min, any idea if something in the Chrome code could be causing this?
Labels: -Restrict-View-Google
Please don't add Restrict-View-Google to bugs except when it's necessary.
I am going to take a look of the code. But it's hard for me to repro this on my device (tried several times on meowth).
I can sometimes repro this on eve in tablet mode. I found even the button up event is received, but the animation hasn't been cancelled. I will going to do more investigation and trying to find a fix.
Status: Started (was: Assigned)
Found the root cause, going to upload the fix cl. This should only happened in tablet mode.
Project Member

Comment 10 by bugdroid1@chromium.org, Oct 15

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

commit 3a8fc6341bebc496fd7ac2344c8314fb5b3d82b2
Author: Min Chen <minch@google.com>
Date: Mon Oct 15 19:55:29 2018

Cancel the menu animation if it is partial shown in tablet mode too.

We used a callback SetShowMenuAnimationDone to start the
|pre_shutdown_timer_| if the PowerButtonMenuScreenView has been full
shown (opacity reaches its final opacity), but didn't stop it if
UseTabletBehavior is true when power button up.

Then if menu animation has been started
(power_button_menu_timer_ run out of) but not finished
(pre_shutdown_timer_ hasn't been started) and UseTabletBehavior is true
when power button up. We did nothing in this case. This will cause the
|pre_shutdown_timer_| to be started and finally trigger power off when
the menu shown animation finished.

This cl tried to fix the issue by calling ScheduleShowHideAnimation(false)
too if UseTabletBehavior. This will cancel the menu shown animation to make
sure its opacity will not achieve its final shown opacity then the callback
SetShowMenuAnimationDone will not be called.
Turn screen off too in this case.

Bug:  894192 
Change-Id: Id744597e53229384fd01279e489d9099c83be67b
Reviewed-on: https://chromium-review.googlesource.com/c/1277914
Commit-Queue: Min Chen <minch@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599724}
[modify] https://crrev.com/3a8fc6341bebc496fd7ac2344c8314fb5b3d82b2/ash/system/power/power_button_controller.cc
[modify] https://crrev.com/3a8fc6341bebc496fd7ac2344c8314fb5b3d82b2/ash/system/power/power_button_controller_test_api.cc
[modify] https://crrev.com/3a8fc6341bebc496fd7ac2344c8314fb5b3d82b2/ash/system/power/power_button_controller_test_api.h
[modify] https://crrev.com/3a8fc6341bebc496fd7ac2344c8314fb5b3d82b2/ash/system/power/power_button_controller_unittest.cc

Labels: Merge-Request-71
Add Merge-Request-71.
Project Member

Comment 12 by sheriffbot@chromium.org, Oct 16

Labels: -Merge-Request-71 Hotlist-Merge-Approved Merge-Approved-71
Your change meets the bar and is auto-approved for M71. Please go ahead and merge the CL to branch 3578 manually. Please contact milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

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

Comment 13 by bugdroid1@chromium.org, Oct 17

Labels: -merge-approved-71 merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2bf7ec6b6f505b132198adc88c47238e1a2dad7b

commit 2bf7ec6b6f505b132198adc88c47238e1a2dad7b
Author: Min Chen <minch@google.com>
Date: Wed Oct 17 00:41:26 2018

[Merge to M71]Cancel the menu animation if it is partial shown in tablet mode too.

We used a callback SetShowMenuAnimationDone to start the
|pre_shutdown_timer_| if the PowerButtonMenuScreenView has been full
shown (opacity reaches its final opacity), but didn't stop it if
UseTabletBehavior is true when power button up.

Then if menu animation has been started
(power_button_menu_timer_ run out of) but not finished
(pre_shutdown_timer_ hasn't been started) and UseTabletBehavior is true
when power button up. We did nothing in this case. This will cause the
|pre_shutdown_timer_| to be started and finally trigger power off when
the menu shown animation finished.

This cl tried to fix the issue by calling ScheduleShowHideAnimation(false)
too if UseTabletBehavior. This will cancel the menu shown animation to make
sure its opacity will not achieve its final shown opacity then the callback
SetShowMenuAnimationDone will not be called.
Turn screen off too in this case.

TBR=minch@chromium.org

(cherry picked from commit 3a8fc6341bebc496fd7ac2344c8314fb5b3d82b2)

Bug:  894192 
Change-Id: Id744597e53229384fd01279e489d9099c83be67b
Reviewed-on: https://chromium-review.googlesource.com/c/1277914
Commit-Queue: Min Chen <minch@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599724}
Reviewed-on: https://chromium-review.googlesource.com/c/1285729
Reviewed-by: Min Chen <minch@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#79}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/2bf7ec6b6f505b132198adc88c47238e1a2dad7b/ash/system/power/power_button_controller.cc
[modify] https://crrev.com/2bf7ec6b6f505b132198adc88c47238e1a2dad7b/ash/system/power/power_button_controller_test_api.cc
[modify] https://crrev.com/2bf7ec6b6f505b132198adc88c47238e1a2dad7b/ash/system/power/power_button_controller_test_api.h
[modify] https://crrev.com/2bf7ec6b6f505b132198adc88c47238e1a2dad7b/ash/system/power/power_button_controller_unittest.cc

Status: Fixed (was: Started)
Labels: Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/2bf7ec6b6f505b132198adc88c47238e1a2dad7b

Commit: 2bf7ec6b6f505b132198adc88c47238e1a2dad7b
Author: minch@google.com
Commiter: minch@chromium.org
Date: 2018-10-17 00:41:26 +0000 UTC

[Merge to M71]Cancel the menu animation if it is partial shown in tablet mode too.

We used a callback SetShowMenuAnimationDone to start the
|pre_shutdown_timer_| if the PowerButtonMenuScreenView has been full
shown (opacity reaches its final opacity), but didn't stop it if
UseTabletBehavior is true when power button up.

Then if menu animation has been started
(power_button_menu_timer_ run out of) but not finished
(pre_shutdown_timer_ hasn't been started) and UseTabletBehavior is true
when power button up. We did nothing in this case. This will cause the
|pre_shutdown_timer_| to be started and finally trigger power off when
the menu shown animation finished.

This cl tried to fix the issue by calling ScheduleShowHideAnimation(false)
too if UseTabletBehavior. This will cancel the menu shown animation to make
sure its opacity will not achieve its final shown opacity then the callback
SetShowMenuAnimationDone will not be called.
Turn screen off too in this case.

TBR=minch@chromium.org

(cherry picked from commit 3a8fc6341bebc496fd7ac2344c8314fb5b3d82b2)

Bug:  894192 
Change-Id: Id744597e53229384fd01279e489d9099c83be67b
Reviewed-on: https://chromium-review.googlesource.com/c/1277914
Commit-Queue: Min Chen <minch@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#599724}
Reviewed-on: https://chromium-review.googlesource.com/c/1285729
Reviewed-by: Min Chen <minch@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#79}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}

Sign in to add a comment