From f42fbb92cff86d4ef344178fe570874fa3d9f39a Mon Sep 17 00:00:00 2001 From: Andreas Miko Date: Tue, 27 Jun 2023 14:21:33 +0000 Subject: [PATCH 1/2] Call finishedCallback after transitioning away from DREAM Bug: b/288279203 Test: atest DreamingToLockscreenTransitionViewModelTest Change-Id: I7e1a8a5fd6ca02e33622c3e106f4025662da1f33 --- .../keyguard/KeyguardViewMediator.java | 20 ++++++ .../KeyguardTransitionInteractor.kt | 8 ++- ...DreamingToLockscreenTransitionViewModel.kt | 2 +- .../keyguard/KeyguardViewMediatorTest.java | 2 + ...mingToLockscreenTransitionViewModelTest.kt | 61 +++++++++++++++---- 5 files changed, 79 insertions(+), 14 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java index bc41ab3156df1..783517d6f7846 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java +++ b/packages/SystemUI/src/com/android/systemui/keyguard/KeyguardViewMediator.java @@ -139,6 +139,7 @@ import com.android.systemui.flags.FeatureFlags; import com.android.systemui.flags.Flags; import com.android.systemui.flags.SystemPropertiesHelper; import com.android.systemui.keyguard.dagger.KeyguardModule; +import com.android.systemui.keyguard.shared.model.TransitionStep; import com.android.systemui.keyguard.ui.viewmodel.DreamingToLockscreenTransitionViewModel; import com.android.systemui.log.SessionTracker; import com.android.systemui.navigationbar.NavigationModeController; @@ -538,6 +539,8 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, private CentralSurfaces mCentralSurfaces; + private IRemoteAnimationFinishedCallback mUnoccludeFromDreamFinishedCallback; + private final DeviceConfig.OnPropertiesChangedListener mOnPropertiesChangedListener = new DeviceConfig.OnPropertiesChangedListener() { @Override @@ -1165,6 +1168,7 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, getRemoteSurfaceAlphaApplier().accept(0.0f); mDreamingToLockscreenTransitionViewModel.get() .startTransition(); + mUnoccludeFromDreamFinishedCallback = finishedCallback; return; } @@ -1244,6 +1248,19 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, }; } + private Consumer getFinishedCallbackConsumer() { + return (TransitionStep step) -> { + if (mUnoccludeFromDreamFinishedCallback == null) return; + try { + mUnoccludeFromDreamFinishedCallback.onAnimationFinished(); + mUnoccludeFromDreamFinishedCallback = null; + } catch (RemoteException e) { + Log.e(TAG, "Wasn't able to callback", e); + } + mInteractionJankMonitor.end(CUJ_LOCKSCREEN_OCCLUSION); + }; + } + private DeviceConfigProxy mDeviceConfig; private DozeParameters mDozeParameters; @@ -1503,6 +1520,9 @@ public class KeyguardViewMediator implements CoreStartable, Dumpable, collectFlow(viewRootImpl.getView(), mDreamingToLockscreenTransitionViewModel.get().getDreamOverlayAlpha(), getRemoteSurfaceAlphaApplier(), mMainDispatcher); + collectFlow(viewRootImpl.getView(), + mDreamingToLockscreenTransitionViewModel.get().getTransitionEnded(), + getFinishedCallbackConsumer(), mMainDispatcher); } } // Most services aren't available until the system reaches the ready state, so we diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/KeyguardTransitionInteractor.kt b/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/KeyguardTransitionInteractor.kt index 42f12f82d9a7d..41a81a83d06d3 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/KeyguardTransitionInteractor.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/domain/interactor/KeyguardTransitionInteractor.kt @@ -49,11 +49,15 @@ constructor( ) { /** (any)->GONE transition information */ val anyStateToGoneTransition: Flow = - repository.transitions.filter { step -> step.to == KeyguardState.GONE } + repository.transitions.filter { step -> step.to == GONE } /** (any)->AOD transition information */ val anyStateToAodTransition: Flow = - repository.transitions.filter { step -> step.to == KeyguardState.AOD } + repository.transitions.filter { step -> step.to == AOD } + + /** DREAMING->(any) transition information. */ + val fromDreamingTransition: Flow = + repository.transitions.filter { step -> step.from == DREAMING } /** AOD->LOCKSCREEN transition information. */ val aodToLockscreenTransition: Flow = repository.transition(AOD, LOCKSCREEN) diff --git a/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/DreamingToLockscreenTransitionViewModel.kt b/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/DreamingToLockscreenTransitionViewModel.kt index 9ca4bd62b6fe6..e24d326850e0d 100644 --- a/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/DreamingToLockscreenTransitionViewModel.kt +++ b/packages/SystemUI/src/com/android/systemui/keyguard/ui/viewmodel/DreamingToLockscreenTransitionViewModel.kt @@ -48,7 +48,7 @@ constructor( ) val transitionEnded = - keyguardTransitionInteractor.dreamingToLockscreenTransition.filter { step -> + keyguardTransitionInteractor.fromDreamingTransition.filter { step -> step.transitionState == TransitionState.FINISHED || step.transitionState == TransitionState.CANCELED } 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 c280538f2d30a..7a501a85714e2 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java +++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/KeyguardViewMediatorTest.java @@ -186,6 +186,8 @@ public class KeyguardViewMediatorTest extends SysuiTestCase { when(mStatusBarKeyguardViewManager.getViewRootImpl()).thenReturn(testViewRoot); when(mDreamingToLockscreenTransitionViewModel.getDreamOverlayAlpha()) .thenReturn(mock(Flow.class)); + when(mDreamingToLockscreenTransitionViewModel.getTransitionEnded()) + .thenReturn(mock(Flow.class)); mNotificationShadeWindowController = new NotificationShadeWindowControllerImpl(mContext, mWindowManager, mActivityManager, mDozeParameters, mStatusBarStateController, mConfigurationController, mViewMediator, mKeyguardBypassController, diff --git a/packages/SystemUI/tests/src/com/android/systemui/keyguard/ui/viewmodel/DreamingToLockscreenTransitionViewModelTest.kt b/packages/SystemUI/tests/src/com/android/systemui/keyguard/ui/viewmodel/DreamingToLockscreenTransitionViewModelTest.kt index a3413466d62ea..ab994b72a45b9 100644 --- a/packages/SystemUI/tests/src/com/android/systemui/keyguard/ui/viewmodel/DreamingToLockscreenTransitionViewModelTest.kt +++ b/packages/SystemUI/tests/src/com/android/systemui/keyguard/ui/viewmodel/DreamingToLockscreenTransitionViewModelTest.kt @@ -22,8 +22,16 @@ import com.android.systemui.RoboPilotTest import com.android.systemui.SysuiTestCase import com.android.systemui.keyguard.data.repository.FakeKeyguardTransitionRepository import com.android.systemui.keyguard.domain.interactor.KeyguardTransitionInteractor -import com.android.systemui.keyguard.shared.model.KeyguardState +import com.android.systemui.keyguard.shared.model.KeyguardState.AOD +import com.android.systemui.keyguard.shared.model.KeyguardState.DOZING +import com.android.systemui.keyguard.shared.model.KeyguardState.DREAMING +import com.android.systemui.keyguard.shared.model.KeyguardState.GONE +import com.android.systemui.keyguard.shared.model.KeyguardState.LOCKSCREEN import com.android.systemui.keyguard.shared.model.TransitionState +import com.android.systemui.keyguard.shared.model.TransitionState.CANCELED +import com.android.systemui.keyguard.shared.model.TransitionState.FINISHED +import com.android.systemui.keyguard.shared.model.TransitionState.RUNNING +import com.android.systemui.keyguard.shared.model.TransitionState.STARTED import com.android.systemui.keyguard.shared.model.TransitionStep import com.android.systemui.util.mockito.mock import com.google.common.collect.Range @@ -60,7 +68,7 @@ class DreamingToLockscreenTransitionViewModelTest : SysuiTestCase() { val job = underTest.dreamOverlayTranslationY(pixels).onEach { values.add(it) }.launchIn(this) - repository.sendTransitionStep(step(0f, TransitionState.STARTED)) + repository.sendTransitionStep(step(0f, STARTED)) repository.sendTransitionStep(step(0f)) repository.sendTransitionStep(step(0.3f)) repository.sendTransitionStep(step(0.5f)) @@ -82,7 +90,7 @@ class DreamingToLockscreenTransitionViewModelTest : SysuiTestCase() { val job = underTest.dreamOverlayAlpha.onEach { values.add(it) }.launchIn(this) // Should start running here... - repository.sendTransitionStep(step(0f, TransitionState.STARTED)) + repository.sendTransitionStep(step(0f, STARTED)) repository.sendTransitionStep(step(0f)) repository.sendTransitionStep(step(0.1f)) repository.sendTransitionStep(step(0.5f)) @@ -104,7 +112,7 @@ class DreamingToLockscreenTransitionViewModelTest : SysuiTestCase() { val job = underTest.lockscreenAlpha.onEach { values.add(it) }.launchIn(this) - repository.sendTransitionStep(step(0f, TransitionState.STARTED)) + repository.sendTransitionStep(step(0f, STARTED)) repository.sendTransitionStep(step(0f)) repository.sendTransitionStep(step(0.1f)) repository.sendTransitionStep(step(0.2f)) @@ -126,7 +134,7 @@ class DreamingToLockscreenTransitionViewModelTest : SysuiTestCase() { val job = underTest.lockscreenTranslationY(pixels).onEach { values.add(it) }.launchIn(this) - repository.sendTransitionStep(step(0f, TransitionState.STARTED)) + repository.sendTransitionStep(step(0f, STARTED)) repository.sendTransitionStep(step(0f)) repository.sendTransitionStep(step(0.3f)) repository.sendTransitionStep(step(0.5f)) @@ -138,13 +146,44 @@ class DreamingToLockscreenTransitionViewModelTest : SysuiTestCase() { job.cancel() } - private fun step( - value: Float, - state: TransitionState = TransitionState.RUNNING - ): TransitionStep { + @Test + fun transitionEnded() = + runTest(UnconfinedTestDispatcher()) { + val values = mutableListOf() + + val job = underTest.transitionEnded.onEach { values.add(it) }.launchIn(this) + + repository.sendTransitionStep(TransitionStep(DOZING, DREAMING, 0.0f, STARTED)) + repository.sendTransitionStep(TransitionStep(DOZING, DREAMING, 1.0f, FINISHED)) + + repository.sendTransitionStep(TransitionStep(DREAMING, LOCKSCREEN, 0.0f, STARTED)) + repository.sendTransitionStep(TransitionStep(DREAMING, LOCKSCREEN, 0.1f, RUNNING)) + repository.sendTransitionStep(TransitionStep(DREAMING, LOCKSCREEN, 1.0f, FINISHED)) + + repository.sendTransitionStep(TransitionStep(LOCKSCREEN, DREAMING, 0.0f, STARTED)) + repository.sendTransitionStep(TransitionStep(LOCKSCREEN, DREAMING, 0.5f, RUNNING)) + repository.sendTransitionStep(TransitionStep(LOCKSCREEN, DREAMING, 1.0f, FINISHED)) + + repository.sendTransitionStep(TransitionStep(DREAMING, GONE, 0.0f, STARTED)) + repository.sendTransitionStep(TransitionStep(DREAMING, GONE, 0.5f, RUNNING)) + repository.sendTransitionStep(TransitionStep(DREAMING, GONE, 1.0f, CANCELED)) + + repository.sendTransitionStep(TransitionStep(DREAMING, AOD, 0.0f, STARTED)) + repository.sendTransitionStep(TransitionStep(DREAMING, AOD, 1.0f, FINISHED)) + + assertThat(values.size).isEqualTo(3) + values.forEach { + assertThat(it.transitionState == FINISHED || it.transitionState == CANCELED) + .isTrue() + } + + job.cancel() + } + + private fun step(value: Float, state: TransitionState = RUNNING): TransitionStep { return TransitionStep( - from = KeyguardState.DREAMING, - to = KeyguardState.LOCKSCREEN, + from = DREAMING, + to = LOCKSCREEN, value = value, transitionState = state, ownerName = "DreamingToLockscreenTransitionViewModelTest" From 7e77854b33afaf4209878fde86fe48f5d8c902b1 Mon Sep 17 00:00:00 2001 From: Robin Lee Date: Wed, 7 Jun 2023 08:42:23 +0200 Subject: [PATCH 2/2] All keyguard occludes go in-band with transitions Previously the logic was as follows: - If keyguard is locked, send occlude in a transition - And send a direct update after all transitions finish - If keyguard is not locked, send occlude directly This had a race if some activity decided to occlude the keyguard and request its dismissal at the same time, since an occlude transition could be generated followed by a direct update. The direct update is applied instantly but the occlude transition is not so it could run later and since there is no sequencing between the two the keyguard service could still end up confused. Now although we still send the direct updates, they wait to go out until SystemUI has reported to us that it finished handling all the active transitions. This prevents racing ahead of enqueued transitions. To prevent racing behind future transitions we rely on the guarantees about ordering of oneway RPCs on the same IBinder. Test: atest CtsWindowManagerDeviceTestCases:KeyguardTests Fix: 284811687 Change-Id: I6c369c922170a5c9aab96a5060089c1b6fb8fd85 --- .../server/policy/PhoneWindowManager.java | 8 ++-- .../server/policy/WindowManagerPolicy.java | 2 +- .../android/server/wm/KeyguardController.java | 37 +++++++++++++------ .../com/android/server/wm/Transition.java | 3 +- .../server/wm/TestWindowManagerPolicy.java | 2 +- 5 files changed, 33 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/policy/PhoneWindowManager.java b/services/core/java/com/android/server/policy/PhoneWindowManager.java index 3132c5d713e3f..cbded8931d934 100644 --- a/services/core/java/com/android/server/policy/PhoneWindowManager.java +++ b/services/core/java/com/android/server/policy/PhoneWindowManager.java @@ -3570,19 +3570,17 @@ public class PhoneWindowManager implements WindowManagerPolicy { } @Override - public void onKeyguardOccludedChangedLw(boolean occluded, boolean waitAppTransition) { - if (mKeyguardDelegate != null && waitAppTransition) { + public void onKeyguardOccludedChangedLw(boolean occluded) { + if (mKeyguardDelegate != null) { mPendingKeyguardOccluded = occluded; mKeyguardOccludedChanged = true; - } else { - setKeyguardOccludedLw(occluded); } } @Override public int applyKeyguardOcclusionChange() { if (DEBUG_KEYGUARD) Slog.d(TAG, "transition/occluded commit occluded=" - + mPendingKeyguardOccluded); + + mPendingKeyguardOccluded + " changed=" + mKeyguardOccludedChanged); // TODO(b/276433230): Explicitly save before/after for occlude state in each // Transition so we don't need to update SysUI every time. diff --git a/services/core/java/com/android/server/policy/WindowManagerPolicy.java b/services/core/java/com/android/server/policy/WindowManagerPolicy.java index 887f9461bdcef..03a7bd3b68b32 100644 --- a/services/core/java/com/android/server/policy/WindowManagerPolicy.java +++ b/services/core/java/com/android/server/policy/WindowManagerPolicy.java @@ -169,7 +169,7 @@ public interface WindowManagerPolicy extends WindowManagerPolicyConstants { * * @param occluded Whether Keyguard is currently occluded or not. */ - void onKeyguardOccludedChangedLw(boolean occluded, boolean waitAppTransition); + void onKeyguardOccludedChangedLw(boolean occluded); /** * Commit any queued changes to keyguard occlude status that had been deferred during the diff --git a/services/core/java/com/android/server/wm/KeyguardController.java b/services/core/java/com/android/server/wm/KeyguardController.java index ad9c3b2742673..83fd725aafdf5 100644 --- a/services/core/java/com/android/server/wm/KeyguardController.java +++ b/services/core/java/com/android/server/wm/KeyguardController.java @@ -418,13 +418,17 @@ class KeyguardController { return; } - final boolean waitAppTransition = isKeyguardLocked(displayId); - mWindowManager.mPolicy.onKeyguardOccludedChangedLw(isDisplayOccluded(DEFAULT_DISPLAY), - waitAppTransition); - if (waitAppTransition) { - mService.deferWindowLayout(); - try { - if (isDisplayOccluded(DEFAULT_DISPLAY)) { + final TransitionController tc = mRootWindowContainer.mTransitionController; + + final boolean occluded = isDisplayOccluded(displayId); + final boolean performTransition = isKeyguardLocked(displayId); + final boolean executeTransition = performTransition && !tc.isCollecting(); + + mWindowManager.mPolicy.onKeyguardOccludedChangedLw(occluded); + mService.deferWindowLayout(); + try { + if (isKeyguardLocked(displayId)) { + if (occluded) { mRootWindowContainer.getDefaultDisplay().requestTransitionAndLegacyPrepare( TRANSIT_KEYGUARD_OCCLUDE, TRANSIT_FLAG_KEYGUARD_OCCLUDING, @@ -434,11 +438,19 @@ class KeyguardController { TRANSIT_KEYGUARD_UNOCCLUDE, TRANSIT_FLAG_KEYGUARD_UNOCCLUDING); } - updateKeyguardSleepToken(DEFAULT_DISPLAY); - mWindowManager.executeAppTransition(); - } finally { - mService.continueWindowLayout(); + } else { + if (tc.inTransition()) { + tc.mStateValidators.add(mWindowManager.mPolicy::applyKeyguardOcclusionChange); + } else { + mWindowManager.mPolicy.applyKeyguardOcclusionChange(); + } } + updateKeyguardSleepToken(displayId); + if (performTransition && executeTransition) { + mWindowManager.executeAppTransition(); + } + } finally { + mService.continueWindowLayout(); } } @@ -485,6 +497,9 @@ class KeyguardController { } } + /** + * @return true if Keyguard is occluded or the device is dreaming. + */ boolean isDisplayOccluded(int displayId) { return getDisplayState(displayId).mOccluded; } diff --git a/services/core/java/com/android/server/wm/Transition.java b/services/core/java/com/android/server/wm/Transition.java index b7ef5bbf13bd3..4519abf724b59 100644 --- a/services/core/java/com/android/server/wm/Transition.java +++ b/services/core/java/com/android/server/wm/Transition.java @@ -27,6 +27,7 @@ import static android.os.Trace.TRACE_TAG_WINDOW_MANAGER; import static android.view.Display.DEFAULT_DISPLAY; import static android.view.Display.INVALID_DISPLAY; import static android.view.WindowManager.INPUT_CONSUMER_RECENTS_ANIMATION; +import static android.view.WindowManager.KEYGUARD_VISIBILITY_TRANSIT_FLAGS; import static android.view.WindowManager.LayoutParams.ROTATION_ANIMATION_SEAMLESS; import static android.view.WindowManager.LayoutParams.ROTATION_ANIMATION_UNSPECIFIED; import static android.view.WindowManager.LayoutParams.TYPE_APPLICATION_STARTING; @@ -2649,7 +2650,7 @@ class Transition implements BLASTSyncEngine.TransactionReadyListener { } private void validateKeyguardOcclusion() { - if ((mFlags & TRANSIT_FLAG_KEYGUARD_LOCKED) != 0) { + if ((mFlags & KEYGUARD_VISIBILITY_TRANSIT_FLAGS) != 0) { mController.mStateValidators.add( 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 adf3f3976f38d..bd111ada85504 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TestWindowManagerPolicy.java +++ b/services/tests/wmtests/src/com/android/server/wm/TestWindowManagerPolicy.java @@ -233,7 +233,7 @@ class TestWindowManagerPolicy implements WindowManagerPolicy { } @Override - public void onKeyguardOccludedChangedLw(boolean occluded, boolean waitAppTransition) { + public void onKeyguardOccludedChangedLw(boolean occluded) { } public void setSafeMode(boolean safeMode) {