From 140a05907d905e38aa5690320d26c90ca29bf772 Mon Sep 17 00:00:00 2001
From: Robin Lee <rgl@google.com>
Date: Fri, 31 Mar 2023 14:29:24 +0200
Subject: [PATCH] Run keyguard occlusion update after transitions

Earlier code was calling applyKeyguardOcclusionChange as soon as the
transition starts, which is not very effective because some of the
transitions only update keyguard occluded state at the end of the
animation and race behind this call.

If we wait for all transitions to finish playing before validating
keyguard occlusion, the last word always comes from
applyKeyguardOcclusionChange which synchronises the source of truth
in KeyguardController over to SystemUI.

Test: atest android.server.wm.KeyguardTests
Bug: 275650364
Change-Id: I016b72b87baadb63f3f5f83112901315f29d2f0a
---
 .../server/policy/PhoneWindowManager.java     | 17 ++++++++------
 .../com/android/server/wm/Transition.java     | 23 +++++++------------
 2 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/services/core/java/com/android/server/policy/PhoneWindowManager.java b/services/core/java/com/android/server/policy/PhoneWindowManager.java
index 2e05fd0c6bf77..8165958bd4ef7 100644
--- a/services/core/java/com/android/server/policy/PhoneWindowManager.java
+++ b/services/core/java/com/android/server/policy/PhoneWindowManager.java
@@ -3573,14 +3573,16 @@ public class PhoneWindowManager implements WindowManagerPolicy {
 
     @Override
     public int applyKeyguardOcclusionChange() {
-        if (mKeyguardOccludedChanged) {
-            if (DEBUG_KEYGUARD) Slog.d(TAG, "transition/occluded changed occluded="
-                    + mPendingKeyguardOccluded);
-            if (setKeyguardOccludedLw(mPendingKeyguardOccluded)) {
-                return FINISH_LAYOUT_REDO_LAYOUT | FINISH_LAYOUT_REDO_WALLPAPER;
-            }
+        if (DEBUG_KEYGUARD) Slog.d(TAG, "transition/occluded commit occluded="
+                + mPendingKeyguardOccluded);
+
+        // TODO(b/276433230): Explicitly save before/after for occlude state in each
+        // Transition so we don't need to update SysUI every time.
+        if (setKeyguardOccludedLw(mPendingKeyguardOccluded)) {
+            return FINISH_LAYOUT_REDO_LAYOUT | FINISH_LAYOUT_REDO_WALLPAPER;
+        } else {
+            return 0;
         }
-        return 0;
     }
 
     /**
@@ -3858,6 +3860,7 @@ public class PhoneWindowManager implements WindowManagerPolicy {
     private boolean setKeyguardOccludedLw(boolean isOccluded) {
         if (DEBUG_KEYGUARD) Slog.d(TAG, "setKeyguardOccluded occluded=" + isOccluded);
         mKeyguardOccludedChanged = false;
+        mPendingKeyguardOccluded = isOccluded;
         mKeyguardDelegate.setOccluded(isOccluded, true /* notify */);
         return mKeyguardDelegate.isShowing();
     }
diff --git a/services/core/java/com/android/server/wm/Transition.java b/services/core/java/com/android/server/wm/Transition.java
index c6ba60026cdf4..88fc43a62ed4e 100644
--- a/services/core/java/com/android/server/wm/Transition.java
+++ b/services/core/java/com/android/server/wm/Transition.java
@@ -1019,6 +1019,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
             dc.removeImeSurfaceImmediately();
             dc.handleCompleteDeferredRemoval();
         }
+        validateKeyguardOcclusion();
         validateVisibility();
 
         mState = STATE_FINISHED;
@@ -1175,8 +1176,6 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
             if (mRecentsDisplayId != INVALID_DISPLAY) break;
         }
 
-        handleNonAppWindowsInTransition(mType, mFlags);
-
         // The callback is only populated for custom activity-level client animations
         sendRemoteCallback(mClientAnimationStartCallback);
 
@@ -1480,19 +1479,6 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
         }
     }
 
-    private void handleNonAppWindowsInTransition(
-            @TransitionType int transit, @TransitionFlags int flags) {
-        if ((flags & TRANSIT_FLAG_KEYGUARD_LOCKED) != 0) {
-            // If the occlusion changed but the transition isn't an occlude/unocclude transition,
-            // then we have to notify KeyguardService directly. This can happen if there is
-            // another ongoing transition when the app changes occlusion OR if the app dies or
-            // is killed. Both of these are common during tests.
-            if (transit != TRANSIT_KEYGUARD_OCCLUDE && transit != TRANSIT_KEYGUARD_UNOCCLUDE) {
-                mController.mAtm.mWindowManager.mPolicy.applyKeyguardOcclusionChange();
-            }
-        }
-    }
-
     private void reportStartReasonsToLogger() {
         // Record transition start in metrics logger. We just assume everything is "DRAWN"
         // at this point since splash-screen is a presentation (shell) detail.
@@ -2185,6 +2171,13 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener {
         return mainWin.getAttrs().rotationAnimation;
     }
 
+    private void validateKeyguardOcclusion() {
+        if ((mFlags & TRANSIT_FLAG_KEYGUARD_LOCKED) != 0) {
+            mController.mStateValidators.add(
+                mController.mAtm.mWindowManager.mPolicy::applyKeyguardOcclusionChange);
+        }
+    }
+
     private void validateVisibility() {
         for (int i = mTargets.size() - 1; i >= 0; --i) {
             if (reduceMode(mTargets.get(i).mReadyMode) != TRANSIT_CLOSE) {