From e87a4dcd85dac5d4d4ca2aab63c66641d01b0f87 Mon Sep 17 00:00:00 2001
From: Josh Tsuji <tsuji@google.com>
Date: Tue, 10 Jan 2023 21:58:05 +0200
Subject: [PATCH] Explicitly re-show rather than reset keyguard if we're not
 interactive.

Showing the keyguard while not interactive should only happen
during race conditions involving locking and unlocking
simultaneously, where we want to make sure we end up locked.
"Resetting" is a short-circuit to just reset the lockscreen
views if we are asked to show while already showing. If we
are asked to show keyguard while not interactive, even if we
think we're already showing, we should not short-circuit and
instead fully re-show the keyguard to ensure we end up in the
correct state.

Fixes: 265025247
Test: mash power/unlock repeatedly until hitting this case
Test: atest KeyguardViewMediatorTest
Change-Id: Ibb80fdd3d292c4cf05203ea89bfcc113481830d6
---
 .../keyguard/KeyguardViewMediator.java        | 22 +++++++++----
 .../keyguard/KeyguardViewMediatorTest.java    | 33 +++++++++++++++++++
 2 files changed, 49 insertions(+), 6 deletions(-)

diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java
index 8aada1f12b0c3..6eeb8c3018eab 100644
--- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java
+++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java
@@ -1907,13 +1907,23 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable,
             return;
         }
 
-        // if the keyguard is already showing, don't bother. check flags in both files
-        // to account for the hiding animation which results in a delay and discrepancy
-        // between flags
+        // If the keyguard is already showing, see if we don't need to bother re-showing it. Check
+        // flags in both files to account for the hiding animation which results in a delay and
+        // discrepancy between flags.
         if (mShowing && mKeyguardStateController.isShowing()) {
-            if (DEBUG) Log.d(TAG, "doKeyguard: not showing because it is already showing");
-            resetStateLocked();
-            return;
+            if (mPM.isInteractive()) {
+                // It's already showing, and we're not trying to show it while the screen is off.
+                // We can simply reset all of the views.
+                if (DEBUG) Log.d(TAG, "doKeyguard: not showing because it is already showing");
+                resetStateLocked();
+                return;
+            } else {
+                // We are trying to show the keyguard while the screen is off - this results from
+                // race conditions involving locking while unlocking. Don't short-circuit here and
+                // ensure the keyguard is fully re-shown.
+                Log.e(TAG,
+                        "doKeyguard: already showing, but re-showing since we're not interactive");
+            }
         }
 
         // In split system user mode, we never unlock system user.
diff --git a/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java b/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java
index 804960dc3b181..196221fc7f58a 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java
@@ -29,6 +29,7 @@ import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.anyBoolean;
 import static org.mockito.ArgumentMatchers.anyInt;
 import static org.mockito.Mockito.atLeast;
+import static org.mockito.Mockito.clearInvocations;
 import static org.mockito.Mockito.eq;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.never;
@@ -369,6 +370,38 @@ public class KeyguardViewMediatorTest extends SysuiTestCase {
         assertTrue(mViewMediator.isAnimatingBetweenKeyguardAndSurfaceBehind());
     }
 
+    @Test
+    @TestableLooper.RunWithLooper(setAsMainLooper = true)
+    public void testDoKeyguardWhileInteractive_resets() {
+        mViewMediator.setShowingLocked(true);
+        when(mKeyguardStateController.isShowing()).thenReturn(true);
+        TestableLooper.get(this).processAllMessages();
+
+        when(mPowerManager.isInteractive()).thenReturn(true);
+
+        mViewMediator.onSystemReady();
+        TestableLooper.get(this).processAllMessages();
+
+        assertTrue(mViewMediator.isShowingAndNotOccluded());
+        verify(mStatusBarKeyguardViewManager).reset(anyBoolean());
+    }
+
+    @Test
+    @TestableLooper.RunWithLooper(setAsMainLooper = true)
+    public void testDoKeyguardWhileNotInteractive_showsInsteadOfResetting() {
+        mViewMediator.setShowingLocked(true);
+        when(mKeyguardStateController.isShowing()).thenReturn(true);
+        TestableLooper.get(this).processAllMessages();
+
+        when(mPowerManager.isInteractive()).thenReturn(false);
+
+        mViewMediator.onSystemReady();
+        TestableLooper.get(this).processAllMessages();
+
+        assertTrue(mViewMediator.isShowingAndNotOccluded());
+        verify(mStatusBarKeyguardViewManager, never()).reset(anyBoolean());
+    }
+
     private void createAndStartViewMediator() {
         mViewMediator = new KeyguardViewMediator(
                 mContext,