From a91b6439ba457a33cd09d498cb96d1b02e1832e7 Mon Sep 17 00:00:00 2001 From: Sebastian Keller Date: Wed, 16 Mar 2022 22:36:49 +0100 Subject: [PATCH] overview: Validate transitions of the `shown` state There have been several bugs in the past that caused invalid transitions of the `shown` state, such as going from `showing` to `showing`. These cause consecutive emissions of the `showing` signal, which can confuse other classes such as the search controller which connects to the stage `key-press-event` on showing and disconnects again on `hiding`. Having two consecutive `showing` signals will cause it to connect twice, and only disconnect once when hiding the overview again. This will lead to key presses getting repeated in the search until the session is restarted. Because there is no obvious connection to how and when this issue got triggered, this now adds some validation code that only allows valid transitions and throws an error otherwise so we get a backtrace of the code actually causing the problem. This does not fix the issue(s) causing the invalid state transitions, it only adds a way of tracking them down. Related: https://gitlab.gnome.org/GNOME/gnome-shell/-/issues/4651 Part-of: --- js/ui/overview.js | 60 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 51 insertions(+), 9 deletions(-) diff --git a/js/ui/overview.js b/js/ui/overview.js index 34343a019..4966cdb69 100644 --- a/js/ui/overview.js +++ b/js/ui/overview.js @@ -111,6 +111,34 @@ class OverviewActor extends St.BoxLayout { } }); +const OverviewShownState = { + HIDDEN: 'HIDDEN', + HIDING: 'HIDING', + SHOWING: 'SHOWING', + SHOWN: 'SHOWN', +}; + +const OVERVIEW_SHOWN_TRANSITIONS = { + [OverviewShownState.HIDDEN]: { + signal: 'hidden', + allowedTransitions: [OverviewShownState.SHOWING], + }, + [OverviewShownState.HIDING]: { + signal: 'hiding', + allowedTransitions: + [OverviewShownState.HIDDEN, OverviewShownState.SHOWING], + }, + [OverviewShownState.SHOWING]: { + signal: 'showing', + allowedTransitions: + [OverviewShownState.SHOWN, OverviewShownState.HIDING], + }, + [OverviewShownState.SHOWN]: { + signal: 'shown', + allowedTransitions: [OverviewShownState.HIDING], + }, +}; + var Overview = class extends Signals.EventEmitter { constructor() { super(); @@ -162,6 +190,7 @@ var Overview = class extends Signals.EventEmitter { this._modal = false; // have a modal grab this._animationInProgress = false; this._visibleTarget = false; + this._shownState = OverviewShownState.HIDDEN; // During transitions, we raise this to the top to avoid having the overview // area be reactive; it causes too many issues such as double clicks on @@ -258,6 +287,19 @@ var Overview = class extends Signals.EventEmitter { this._shellInfo.setMessage(text, options); } + _changeShownState(state) { + const {allowedTransitions} = + OVERVIEW_SHOWN_TRANSITIONS[this._shownState]; + + if (!allowedTransitions.includes(state)) { + throw new Error('Invalid overview shown transition from ' + + `${this._shownState} to ${state}`); + } + + this._shownState = state; + this.emit(OVERVIEW_SHOWN_TRANSITIONS[state].signal); + } + _onDragBegin() { this._inXdndDrag = true; @@ -370,7 +412,7 @@ var Overview = class extends Signals.EventEmitter { Main.layoutManager.overviewGroup.set_child_above_sibling( this._coverPane, null); this._coverPane.show(); - this.emit('showing'); + this._changeShownState(OverviewShownState.SHOWING); Main.layoutManager.showOverview(); this._syncGrab(); @@ -384,7 +426,7 @@ var Overview = class extends Signals.EventEmitter { if (endProgress === 0) { this._shown = false; this._visibleTarget = false; - this.emit('hiding'); + this._changeShownState(OverviewShownState.HIDING); Main.panel.style = `transition-duration: ${duration}ms;`; onComplete = () => this._hideDone(); } else { @@ -523,7 +565,7 @@ var Overview = class extends Signals.EventEmitter { this._coverPane.show(); this._overview.prepareToEnterOverview(); - this.emit('showing'); + this._changeShownState(OverviewShownState.SHOWING); this._overview.animateToOverview(state, () => this._showDone()); } @@ -531,7 +573,7 @@ var Overview = class extends Signals.EventEmitter { this._animationInProgress = false; this._coverPane.hide(); - this.emit('shown'); + this._changeShownState(OverviewShownState.SHOWN); // Handle any calls to hide* while we were showing if (!this._shown) this._animateNotVisible(); @@ -577,7 +619,7 @@ var Overview = class extends Signals.EventEmitter { this._coverPane.show(); this._overview.prepareToLeaveOverview(); - this.emit('hiding'); + this._changeShownState(OverviewShownState.HIDING); this._overview.animateFromOverview(() => this._hideDone()); } @@ -592,11 +634,11 @@ var Overview = class extends Signals.EventEmitter { // Handle any calls to show* while we were hiding if (this._shown) { - this.emit('hidden'); + this._changeShownState(OverviewShownState.HIDDEN); this._animateVisible(OverviewControls.ControlsState.WINDOW_PICKER); } else { Main.layoutManager.hideOverview(); - this.emit('hidden'); + this._changeShownState(OverviewShownState.HIDDEN); } Main.panel.style = null; @@ -636,7 +678,7 @@ var Overview = class extends Signals.EventEmitter { Meta.disable_unredirect_for_display(global.display); - this.emit('showing'); + this._changeShownState(OverviewShownState.SHOWING); this._overview.runStartupAnimation(() => { if (!this._syncGrab()) { @@ -645,7 +687,7 @@ var Overview = class extends Signals.EventEmitter { } Main.panel.style = null; - this.emit('shown'); + this._changeShownState(OverviewShownState.SHOWN); callback(); }); }