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

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);
 

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 */))
+    }
 }