sessiond: the press that wakes the panel must not also blank it
Press the power button on a sleeping phone: the screen comes on, the lock screen appears, and it goes black again. One physical press produces two reports and the second cannot see what the first did. note_input fires on the DOWN edge — correctly, a finger on the button is a user present — which takes the device out of Locked; the executor lights the panel and reports it back through set_panel before the finger is off the button. apply_gesture then resolves the tap on the UP edge, reads panel_on as true, and takes the lock-then-blank branch. The press that woke the screen is read as a press to blank it. The panel state is latched on the DOWN edge and the gesture is decided from that. The level is unreadable by the time the gesture resolves; only the edge is still true. Sixth edge-versus-level bug here, after locked_ack, ChargeRate, bootBloomActive, hasLoginctl and the compositor's dormant OR in disclosure_locked. The existing dark-panel test passed throughout because it never simulated the executor's report landing between the two edges. The new one does, and fails without the latch.
This commit is contained in:
parent
266af98a99
commit
8c7567e419
1 changed files with 77 additions and 1 deletions
|
|
@ -1337,6 +1337,22 @@ pub struct DeviceStateMachine {
|
|||
/// Per-button gesture recognition. Keyed rather than a field per button so
|
||||
/// adding volume costs nothing and binds nothing.
|
||||
buttons: std::collections::HashMap<Button, ButtonRecognizer>,
|
||||
/// Whether the panel was dark when the current press began.
|
||||
///
|
||||
/// Latched on the DOWN edge because **the press itself changes the
|
||||
/// answer.** `note_input` fires on that same edge — correctly, a finger on
|
||||
/// the button is a user present — which takes the device out of `Locked`,
|
||||
/// and the executor lights the panel and reports it back through
|
||||
/// [`Self::set_panel`]. By the time the UP edge resolves the tap,
|
||||
/// `panel_on` is already true, so the press that woke the screen is read as
|
||||
/// a press to blank it: wake, lock screen, black.
|
||||
///
|
||||
/// One physical press, two reports, and the second cannot see what the
|
||||
/// first did. The level is unreadable by then; only the edge is true. That
|
||||
/// makes this the sixth edge-versus-level bug here, after `locked_ack`,
|
||||
/// `ChargeRate`, `bootBloomActive`, `hasLoginctl` and the dormant OR in the
|
||||
/// compositor's `disclosure_locked`.
|
||||
press_began_dark: bool,
|
||||
/// True once a blank has been asked for, so we ask exactly once per wake
|
||||
/// instead of every tick.
|
||||
blank_requested: bool,
|
||||
|
|
@ -1451,6 +1467,9 @@ impl DeviceStateMachine {
|
|||
started_at: Instant::now(),
|
||||
buttons: std::collections::HashMap::new(),
|
||||
blank_requested: false,
|
||||
// Boot comes up lit, so no press is in flight and the latch would
|
||||
// only ever be read after a real DOWN edge has set it.
|
||||
press_began_dark: false,
|
||||
dimmed: false,
|
||||
pending_blank: None,
|
||||
brightness_before_dim: None,
|
||||
|
|
@ -1512,6 +1531,12 @@ impl DeviceStateMachine {
|
|||
edge: ButtonEdge,
|
||||
now: Instant,
|
||||
) -> Vec<Action> {
|
||||
// Before anything below can change it. `note_input` on this same edge
|
||||
// is what lights the panel, so this is the last moment the answer is
|
||||
// still about the screen the user actually pressed against.
|
||||
if edge == ButtonEdge::Down {
|
||||
self.press_began_dark = !self.panel_on;
|
||||
}
|
||||
let rec = self.buttons.entry(button).or_default();
|
||||
let gesture = match edge {
|
||||
ButtonEdge::Down => {
|
||||
|
|
@ -1587,7 +1612,9 @@ impl DeviceStateMachine {
|
|||
if button != Button::Power || gesture != ButtonGesture::Tap {
|
||||
return Vec::new();
|
||||
}
|
||||
if !self.panel_on {
|
||||
// The panel as it was when the press began, not as it is now. See
|
||||
// `press_began_dark`: this same press already woke it.
|
||||
if self.press_began_dark {
|
||||
// Dark: the tap is a wake, and a power-button wake is never vetoed.
|
||||
//
|
||||
// `Unblank` *then* `Restore`, and the order is the whole of it: the
|
||||
|
|
@ -3045,6 +3072,55 @@ mod tests {
|
|||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_press_that_woke_the_panel_does_not_then_blank_it() {
|
||||
// The regression Casey hit: press to wake, the lock screen appears, and
|
||||
// the screen goes black again. One press, two reports.
|
||||
//
|
||||
// `note_input` on the DOWN edge takes the device out of Locked and the
|
||||
// executor lights the panel, reporting it back through `set_panel`
|
||||
// before the finger is even off the button. The test above never
|
||||
// simulated that report, which is why it passed while the phone failed.
|
||||
let (mut sm, t0) = unlocked_and_lit();
|
||||
sm.set_panel(false);
|
||||
sm.button_edge_at(Button::Power, ButtonEdge::Down, t0);
|
||||
// The wake this very press caused, landing between the edges.
|
||||
sm.set_panel(true);
|
||||
|
||||
let mut actions =
|
||||
sm.button_edge_at(Button::Power, ButtonEdge::Up, t0 + Duration::from_millis(80));
|
||||
if actions.is_empty() {
|
||||
actions = sm.tick_at(t0 + Duration::from_secs(2));
|
||||
}
|
||||
assert!(
|
||||
!actions.iter().any(|a| matches!(a, Action::Blank)),
|
||||
"the press that woke the panel must not also blank it: {actions:?}"
|
||||
);
|
||||
assert!(
|
||||
!actions.iter().any(|a| matches!(a, Action::Lock)),
|
||||
"and it must not lock in order to blank: {actions:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_press_against_a_lit_panel_still_blanks_it() {
|
||||
// The other half — without this the fix would simply disable the power
|
||||
// button's only binding.
|
||||
let (mut sm, t0) = unlocked_and_lit();
|
||||
sm.button_edge_at(Button::Power, ButtonEdge::Down, t0);
|
||||
let mut actions =
|
||||
sm.button_edge_at(Button::Power, ButtonEdge::Up, t0 + Duration::from_millis(80));
|
||||
if actions.is_empty() {
|
||||
actions = sm.tick_at(t0 + Duration::from_secs(2));
|
||||
}
|
||||
assert!(
|
||||
actions
|
||||
.iter()
|
||||
.any(|a| matches!(a, Action::Lock | Action::Blank)),
|
||||
"a tap on a lit panel is still lock-then-blank: {actions:?}"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn asking_for_the_state_the_panel_is_already_in_does_nothing() {
|
||||
let (mut sm, _t0) = unlocked_and_lit();
|
||||
|
|
|
|||
Loading…
Reference in a new issue