Power menu sometimes automatically proceeds to power off |
|||||||||
Issue descriptionChrome 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.
,
Oct 10
Here's logs from just now: https://drive.google.com/open?id=0B0Qr_v3F1RKDODAyWmwtUHJkSDhCbEk4REtCWXRKRFl5Umd3
,
Oct 10
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.
,
Oct 11
,
Oct 11
Gotcha - Min, any idea if something in the Chrome code could be causing this?
,
Oct 11
Please don't add Restrict-View-Google to bugs except when it's necessary.
,
Oct 11
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).
,
Oct 12
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.
,
Oct 12
Found the root cause, going to upload the fix cl. This should only happened in tablet mode.
,
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
,
Oct 15
Add Merge-Request-71.
,
Oct 16
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
,
Oct 17
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
,
Oct 17
,
Oct 23
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 |
|||||||||
Comment 1 by derat@chromium.org
, Oct 10