From 52d8053880b1be999ebc75730333b2d9ea326fb4 Mon Sep 17 00:00:00 2001 From: Robin Lee Date: Thu, 16 Mar 2023 19:24:01 +0100 Subject: [PATCH] Hold onto occluded change until it's committed When we're delaying commit of setKeyguardOccluded due to a transition taking over instead to synchronise occlude status more nicely, we hold onto the final occlude status so we can be eventually consistent even if the systemui restarts or gets something wrong. The "notify" parameter in setKeyguardOccludedLw was risky because when false it could lead to SystemUI not being informed of an update, but the system server still considering its job done because the flag for pending updates was cleared anyway. This is a speculative fix to help the attached issue resolved itself whenever it happens. Test: atest android.server.wm.KeyguardTests Bug: 269892931 Change-Id: I7d3ac020db34d8a15d18b25de07231bda83cbcf0 --- .../server/policy/PhoneWindowManager.java | 24 +++++++++---------- .../server/policy/WindowManagerPolicy.java | 6 ++--- .../com/android/server/wm/Transition.java | 6 ++--- .../server/wm/TestWindowManagerPolicy.java | 2 +- 4 files changed, 18 insertions(+), 20 deletions(-) diff --git a/services/core/java/com/android/server/policy/PhoneWindowManager.java b/services/core/java/com/android/server/policy/PhoneWindowManager.java index 3eeafeb52b4b3..08a6481d71451 100644 --- a/services/core/java/com/android/server/policy/PhoneWindowManager.java +++ b/services/core/java/com/android/server/policy/PhoneWindowManager.java @@ -3555,16 +3555,16 @@ public class PhoneWindowManager implements WindowManagerPolicy { mPendingKeyguardOccluded = occluded; mKeyguardOccludedChanged = true; } else { - setKeyguardOccludedLw(occluded, true /* notify */); + setKeyguardOccludedLw(occluded); } } @Override - public int applyKeyguardOcclusionChange(boolean notify) { + public int applyKeyguardOcclusionChange() { if (mKeyguardOccludedChanged) { if (DEBUG_KEYGUARD) Slog.d(TAG, "transition/occluded changed occluded=" + mPendingKeyguardOccluded); - if (setKeyguardOccludedLw(mPendingKeyguardOccluded, notify)) { + if (setKeyguardOccludedLw(mPendingKeyguardOccluded)) { return FINISH_LAYOUT_REDO_LAYOUT | FINISH_LAYOUT_REDO_WALLPAPER; } } @@ -3583,8 +3583,10 @@ public class PhoneWindowManager implements WindowManagerPolicy { */ private int handleTransitionForKeyguardLw(boolean startKeyguardExitAnimation, boolean notifyOccluded) { - final int redoLayout = applyKeyguardOcclusionChange(notifyOccluded); - if (redoLayout != 0) return redoLayout; + if (notifyOccluded) { + final int redoLayout = applyKeyguardOcclusionChange(); + if (redoLayout != 0) return redoLayout; + } if (startKeyguardExitAnimation) { if (DEBUG_KEYGUARD) Slog.d(TAG, "Starting keyguard exit animation"); startKeyguardExitAnimation(SystemClock.uptimeMillis()); @@ -3835,20 +3837,16 @@ public class PhoneWindowManager implements WindowManagerPolicy { } /** - * Updates the occluded state of the Keyguard. + * Updates the occluded state of the Keyguard immediately via + * {@link com.android.internal.policy.IKeyguardService}. * * @param isOccluded Whether the Keyguard is occluded by another window. - * @param notify Notify keyguard occlude status change immediately via - * {@link com.android.internal.policy.IKeyguardService}. * @return Whether the flags have changed and we have to redo the layout. */ - private boolean setKeyguardOccludedLw(boolean isOccluded, boolean notify) { + private boolean setKeyguardOccludedLw(boolean isOccluded) { if (DEBUG_KEYGUARD) Slog.d(TAG, "setKeyguardOccluded occluded=" + isOccluded); mKeyguardOccludedChanged = false; - if (isKeyguardOccluded() == isOccluded) { - return false; - } - mKeyguardDelegate.setOccluded(isOccluded, notify); + mKeyguardDelegate.setOccluded(isOccluded, true /* notify */); return mKeyguardDelegate.isShowing(); } diff --git a/services/core/java/com/android/server/policy/WindowManagerPolicy.java b/services/core/java/com/android/server/policy/WindowManagerPolicy.java index 3c4dbf2479344..5d558e9c1e2fb 100644 --- a/services/core/java/com/android/server/policy/WindowManagerPolicy.java +++ b/services/core/java/com/android/server/policy/WindowManagerPolicy.java @@ -170,10 +170,10 @@ public interface WindowManagerPolicy extends WindowManagerPolicyConstants { void onKeyguardOccludedChangedLw(boolean occluded, boolean waitAppTransition); /** - * @param notify {@code true} if the status change should be immediately notified via - * {@link com.android.internal.policy.IKeyguardService} + * Commit any queued changes to keyguard occlude status that had been deferred during the + * start of an animation or transition. */ - int applyKeyguardOcclusionChange(boolean notify); + int applyKeyguardOcclusionChange(); /** * Interface to the Window Manager state associated with a particular diff --git a/services/core/java/com/android/server/wm/Transition.java b/services/core/java/com/android/server/wm/Transition.java index 873a83d5527ee..e82a25ed8f9e4 100644 --- a/services/core/java/com/android/server/wm/Transition.java +++ b/services/core/java/com/android/server/wm/Transition.java @@ -1485,9 +1485,9 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { // 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. - final boolean notify = !(transit == TRANSIT_KEYGUARD_OCCLUDE - || transit == TRANSIT_KEYGUARD_UNOCCLUDE); - mController.mAtm.mWindowManager.mPolicy.applyKeyguardOcclusionChange(notify); + if (transit != TRANSIT_KEYGUARD_OCCLUDE && transit != TRANSIT_KEYGUARD_UNOCCLUDE) { + mController.mAtm.mWindowManager.mPolicy.applyKeyguardOcclusionChange(); + } } } diff --git a/services/tests/wmtests/src/com/android/server/wm/TestWindowManagerPolicy.java b/services/tests/wmtests/src/com/android/server/wm/TestWindowManagerPolicy.java index 1d0715ab04601..2a2641edb6374 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TestWindowManagerPolicy.java +++ b/services/tests/wmtests/src/com/android/server/wm/TestWindowManagerPolicy.java @@ -314,7 +314,7 @@ class TestWindowManagerPolicy implements WindowManagerPolicy { } @Override - public int applyKeyguardOcclusionChange(boolean notify) { + public int applyKeyguardOcclusionChange() { return 0; }