From c66f0ae2e992cbaac2c8e30d25ca48656f6da3dc Mon Sep 17 00:00:00 2001
From: Josh Tsuji <tsuji@google.com>
Date: Mon, 28 Mar 2022 13:16:29 -0400
Subject: [PATCH] Don't short-circuit setting StatusBarState if the upcoming
 state differs.

This signals that a status bar state transition has been interrupted,
and we're re-setting the status bar state before the upcoming state
could be applied. This means that some components (such as the shade)
could be partially showing the upcoming state still. In these cases, we
should proactively re-apply the current state.

Test: atest SystemUITests
Fixes: 221099026
Change-Id: I8429b8ffbc5cb15f7be8394339cff80cb93d5068
---
 .../StatusBarStateControllerImpl.java         | 13 +++++-
 .../StatusBarStateControllerImplTest.kt       | 40 +++++++++++++++++++
 2 files changed, 52 insertions(+), 1 deletion(-)

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateControllerImpl.java
index 19f59f76f8a57..93103e2c3f497 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateControllerImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/StatusBarStateControllerImpl.java
@@ -174,10 +174,21 @@ public class StatusBarStateControllerImpl implements
         if (state > MAX_STATE || state < MIN_STATE) {
             throw new IllegalArgumentException("Invalid state " + state);
         }
-        if (!force && state == mState) {
+
+        // Unless we're explicitly asked to force the state change, don't apply the new state if
+        // it's identical to both the current and upcoming states, since that should not be
+        // necessary.
+        if (!force && state == mState && state == mUpcomingState) {
             return false;
         }
 
+        if (state != mUpcomingState) {
+            Log.d(TAG, "setState: requested state " + StatusBarState.toString(state)
+                    + "!= upcomingState: " + StatusBarState.toString(mUpcomingState) + ". "
+                    + "This usually means the status bar state transition was interrupted before "
+                    + "the upcoming state could be applied.");
+        }
+
         // Record the to-be mState and mLastState
         recordHistoricalState(state /* newState */, mState /* lastState */, false);
 
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/StatusBarStateControllerImplTest.kt b/packages/SystemUI/tests/src/com/android/systemui/statusbar/StatusBarStateControllerImplTest.kt
index 6abdea5d92301..bc1abe62c7ffe 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/StatusBarStateControllerImplTest.kt
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/StatusBarStateControllerImplTest.kt
@@ -25,6 +25,8 @@ import com.android.systemui.SysuiTestCase
 import com.android.systemui.dump.DumpManager
 import com.android.systemui.plugins.statusbar.StatusBarStateController
 import org.junit.Assert.assertEquals
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
 import org.junit.Before
 import org.junit.Test
 import org.junit.runner.RunWith
@@ -87,4 +89,42 @@ class StatusBarStateControllerImplTest : SysuiTestCase() {
         controller.setDozeAmount(0.5f, false /* animated */)
         verify(listener).onDozeAmountChanged(eq(0.5f), anyFloat())
     }
+
+    @Test
+    fun testSetState_appliesState_sameStateButDifferentUpcomingState() {
+        controller.state = StatusBarState.SHADE
+        controller.setUpcomingState(StatusBarState.KEYGUARD)
+
+        assertEquals(controller.state, StatusBarState.SHADE)
+
+        // We should return true (state change was applied) despite going from SHADE to SHADE, since
+        // the upcoming state was set to KEYGUARD.
+        assertTrue(controller.setState(StatusBarState.SHADE))
+    }
+
+    @Test
+    fun testSetState_appliesState_differentStateEqualToUpcomingState() {
+        controller.state = StatusBarState.SHADE
+        controller.setUpcomingState(StatusBarState.KEYGUARD)
+
+        assertEquals(controller.state, StatusBarState.SHADE)
+
+        // Make sure we apply a SHADE -> KEYGUARD state change when the upcoming state is KEYGUARD.
+        assertTrue(controller.setState(StatusBarState.KEYGUARD))
+    }
+
+    @Test
+    fun testSetState_doesNotApplyState_currentAndUpcomingStatesSame() {
+        controller.state = StatusBarState.SHADE
+        controller.setUpcomingState(StatusBarState.SHADE)
+
+        assertEquals(controller.state, StatusBarState.SHADE)
+
+        // We're going from SHADE -> SHADE, and the upcoming state is also SHADE, this should not do
+        // anything.
+        assertFalse(controller.setState(StatusBarState.SHADE))
+
+        // Double check that we can still force it to happen.
+        assertTrue(controller.setState(StatusBarState.SHADE, true /* force */))
+    }
 }