When we talked about this today, I suggested making TabletPowerButtonController implement DisplayConfigurator::Observer and watch for OnDisplayModeChanged or OnDisplayModeChangeFailed to ignore power button events after forcing the display on or off. That was just speculation, though, and I'm not sure whether it will work or cause other problems.

Picking this up. Could we do something simpler here, like ignore PowerButtonDisplayController::SetDisplayOff if last SetDisplayOff is called within 500ms? I think it could fix the problem.

I'm a bit anxious about doing things like that, just because it feels like there's potential for us to get it wrong and end up in the wrong state. Maybe I'm misunderstanding, though.

Instead of skipping the SetDisplayOff call, I think it'd be safer to for us to e.g. ignore power button presses entirely if they come in after Chrome has asked powerd to turn the display back on but before it's actually been turned on.

Even simpler (and perhaps safer), maybe we could just ignore button presses received <~500 ms after a button press turned the display on. We may already be effectively doing this (and more) with kIgnoreRepeatedButtonUpMs, for what it's worth.

OK. I agree with problem-specific fix here. Just ignore repeated SetDisplayOff call may be risky, though I didn't see any yet.

Paste one repro case from email threads: "it was confusing that display didn't turn on when I clicked on a key and then I pressed the power button which probably told it to turn off after a couple of seconds." from Omri.

I think I find the actual reason: screen may still be off for some time after powerd sending brightness changed signal for some devices. (caroline seems having a bigger issue).

We rely on PowerButtonDisplayController::screen_state() to determine if a power button release should set display off: https://cs.chromium.org/chromium/src/ash/system/power/tablet_power_button_controller.cc?sq=package:chromium&l=198. I propose when BrightnessChanged signal is received and brightness level is not zero, start a 500ms timer and on timeout set screen_state_ to ScreenState::ON. This is somewhat making brightness changed signal synced with actual screen on state.

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

commit 229d4c0ec1d731c234da782b558127df01f0b2e5
Author: Qiang Xu <warx@chromium.org>
Date: Thu Sep 21 02:38:25 2017

cros: ignore power button forcing off when display is turning on

context:
For some devices, after stopping forcing off, we may leave an interval that
brightness non-zero level signal is received but screen is not yet turned on.
If a power button pressed-and-released action happens before screen is actually
turned on, user may have a "screen-turned-on-and-then-turned-off-immediately"
situation.

changes:
Ignore power button forcing off when display is turning on. This is done by
(1) set a delay (currently set to 500ms) since last screen state changed time
(2) if power button pressed is within this delay, ignore forcing off for
its release.

immediate follow power button press-and-release". Also added test coverage.

Bug:  735225 
Test: test on caroline with "when display forced off, press a key and then
Change-Id: Ie0ff224fc6b9f4317db9e4fa778698881bb03aa9
Reviewed-on: https://chromium-review.googlesource.com/669839
Commit-Queue: Qiang(Joe) Xu <warx@chromium.org>
Reviewed-by: Dan Erat <derat@chromium.org>
Cr-Commit-Position: refs/heads/master@{#503331}
[modify] https://crrev.com/229d4c0ec1d731c234da782b558127df01f0b2e5/ash/system/power/power_button_display_controller.cc
[modify] https://crrev.com/229d4c0ec1d731c234da782b558127df01f0b2e5/ash/system/power/power_button_display_controller.h
[modify] https://crrev.com/229d4c0ec1d731c234da782b558127df01f0b2e5/ash/system/power/power_event_observer.cc
[modify] https://crrev.com/229d4c0ec1d731c234da782b558127df01f0b2e5/ash/system/power/power_event_observer.h
[modify] https://crrev.com/229d4c0ec1d731c234da782b558127df01f0b2e5/ash/system/power/tablet_power_button_controller.cc
[modify] https://crrev.com/229d4c0ec1d731c234da782b558127df01f0b2e5/ash/system/power/tablet_power_button_controller.h
[modify] https://crrev.com/229d4c0ec1d731c234da782b558127df01f0b2e5/ash/system/power/tablet_power_button_controller_unittest.cc
[modify] https://crrev.com/229d4c0ec1d731c234da782b558127df01f0b2e5/ash/wm/lock_state_controller_unittest.cc
[modify] https://crrev.com/229d4c0ec1d731c234da782b558127df01f0b2e5/ash/wm/power_button_controller.cc
[modify] https://crrev.com/229d4c0ec1d731c234da782b558127df01f0b2e5/ash/wm/power_button_controller.h