From 25c92d9205a066bb45caf2cd66abf02f83bc94e3 Mon Sep 17 00:00:00 2001 From: Evan Rosky Date: Tue, 8 Feb 2022 17:10:54 -0800 Subject: [PATCH] Fix defer-display-removal logic WindowAnimator.animate repeatedly calls into handleCompleteDeferredRemoval() which wasn't checking the same conditions as removeIfPossible() (which is the function that actually sets mDeferredRemoval). This meant that removeImmediately() would be called early at times and break CTS tests. Similarly, moved the onDisplayRemoved call in keyguard controller to removeImmediately() so that it wouldn't get called prematurely. Also fix a NPE in DefaultTransitionHandler Bug: 183993924 Test: atest MultiDisplayPolicyTests (with shell-transitions) Change-Id: I73b10a290a3b2be6ea4d978190863cc6ce62ef8f --- .../transition/DefaultTransitionHandler.java | 4 +- .../com/android/server/wm/DisplayContent.java | 19 +++--- .../com/android/server/wm/Transition.java | 62 +++++++++++-------- .../server/wm/TransitionController.java | 11 ++++ 4 files changed, 62 insertions(+), 34 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultTransitionHandler.java b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultTransitionHandler.java index 34d98ee87dcc5..5af15300d10bb 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultTransitionHandler.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/transition/DefaultTransitionHandler.java @@ -419,8 +419,8 @@ public class DefaultTransitionHandler implements Transitions.TransitionHandler { // hasRoundedCorners is currently only enabled for tasks final Context displayContext = mDisplayController.getDisplayContext(change.getTaskInfo().displayId); - cornerRadius = - ScreenDecorationsUtils.getWindowCornerRadius(displayContext); + cornerRadius = displayContext == null ? 0 + : ScreenDecorationsUtils.getWindowCornerRadius(displayContext); } else { cornerRadius = 0; } diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index 2c58d92c6f150..0018a1c41c1a6 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -3127,12 +3127,16 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp mDisplayPolicy.switchUser(); } - @Override - void removeIfPossible() { - if (isAnimating(TRANSITION | PARENTS) + private boolean shouldDeferRemoval() { + return isAnimating(TRANSITION | PARENTS) // isAnimating is a legacy transition query and will be removed, so also add a // check for whether this is in a shell-transition when not using legacy. - || mTransitionController.inTransition()) { + || mTransitionController.isTransitionOnDisplay(this); + } + + @Override + void removeIfPossible() { + if (shouldDeferRemoval()) { mDeferredRemoval = true; return; } @@ -3161,6 +3165,8 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp mInputMonitor.onDisplayRemoved(); mWmService.mDisplayNotificationController.dispatchDisplayRemoved(this); mWmService.mAccessibilityController.onDisplayRemoved(mDisplayId); + mRootWindowContainer.mTaskSupervisor + .getKeyguardController().onDisplayRemoved(mDisplayId); } finally { mDisplayReady = false; } @@ -3174,7 +3180,8 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp /** Returns true if a removal action is still being deferred. */ @Override boolean handleCompleteDeferredRemoval() { - final boolean stillDeferringRemoval = super.handleCompleteDeferredRemoval(); + final boolean stillDeferringRemoval = + super.handleCompleteDeferredRemoval() || shouldDeferRemoval(); if (!stillDeferringRemoval && mDeferredRemoval) { removeImmediately(); @@ -5915,8 +5922,6 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp forAllRootTasks(t -> {t.removeIfPossible("releaseSelfIfNeeded");}); } else if (getTopRootTask() == null) { removeIfPossible(); - mRootWindowContainer.mTaskSupervisor - .getKeyguardController().onDisplayRemoved(mDisplayId); } } diff --git a/services/core/java/com/android/server/wm/Transition.java b/services/core/java/com/android/server/wm/Transition.java index 26871c3dc76aa..ab724a3f141b0 100644 --- a/services/core/java/com/android/server/wm/Transition.java +++ b/services/core/java/com/android/server/wm/Transition.java @@ -158,8 +158,8 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe /** The final animation targets derived from participants after promotion. */ private ArrayList mTargets; - /** The main display running this transition. */ - private DisplayContent mTargetDisplay; + /** The displays that this transition is running on. */ + private final ArrayList mTargetDisplays = new ArrayList<>(); /** * Set of participating windowtokens (activity/wallpaper) which are visible at the end of @@ -209,6 +209,10 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe return mTransientLaunches != null && mTransientLaunches.contains(activity); } + boolean isOnDisplay(@NonNull DisplayContent dc) { + return mTargetDisplays.contains(dc); + } + void setSeamlessRotation(@NonNull WindowContainer wc) { final ChangeInfo info = mChanges.get(wc); if (info == null) return; @@ -280,6 +284,9 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe mChanges.put(wc, info); } mParticipants.add(wc); + if (wc.getDisplayContent() != null && !mTargetDisplays.contains(wc.getDisplayContent())) { + mTargetDisplays.add(wc.getDisplayContent()); + } if (info.mShowWallpaper) { // Collect the wallpaper token (for isWallpaper(wc)) so it is part of the sync set. final WindowState wallpaper = @@ -516,15 +523,23 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe dc.getInputMonitor().setActiveRecents(null /* activity */, null /* layer */); } - final AsyncRotationController asyncRotationController = - mTargetDisplay.getAsyncRotationController(); - if (asyncRotationController != null) { - asyncRotationController.onTransitionFinished(); - } - // Transient-launch activities cannot be IME target (WindowState#canBeImeTarget), - // so re-compute in case the IME target is changed after transition. - if (mTransientLaunches != null) { - mTargetDisplay.computeImeTarget(true /* updateImeTarget */); + for (int i = 0; i < mTargetDisplays.size(); ++i) { + final DisplayContent dc = mTargetDisplays.get(i); + final AsyncRotationController asyncRotationController = dc.getAsyncRotationController(); + if (asyncRotationController != null) { + asyncRotationController.onTransitionFinished(); + } + if (mTransientLaunches != null) { + // Transient-launch activities cannot be IME target (WindowState#canBeImeTarget), + // so re-compute in case the IME target is changed after transition. + for (int t = 0; t < mTransientLaunches.size(); ++t) { + if (mTransientLaunches.valueAt(t).getDisplayContent() == dc) { + dc.computeImeTarget(true /* updateImeTarget */); + break; + } + } + } + dc.handleCompleteDeferredRemoval(); } } @@ -555,19 +570,13 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe Slog.e(TAG, "Unexpected Sync ID " + syncId + ". Expected " + mSyncId); return; } - boolean hasWallpaper = false; - DisplayContent dc = null; - for (int i = mParticipants.size() - 1; i >= 0; --i) { - final WindowContainer wc = mParticipants.valueAt(i); - if (dc == null && wc.mDisplayContent != null) { - dc = wc.mDisplayContent; - } - if (!hasWallpaper && isWallpaper(wc)) { - hasWallpaper = true; - } + if (mTargetDisplays.isEmpty()) { + mTargetDisplays.add(mController.mAtm.mRootWindowContainer.getDefaultDisplay()); } - if (dc == null) dc = mController.mAtm.mRootWindowContainer.getDefaultDisplay(); - mTargetDisplay = dc; + // While there can be multiple DC's involved. For now, we just use the first one as + // the "primary" one for most things. Eventually, this will need to change, but, for the + // time being, we don't have full cross-display transitions so it isn't a problem. + final DisplayContent dc = mTargetDisplays.get(0); if (mState == STATE_ABORT) { mController.abort(this); @@ -577,8 +586,11 @@ class Transition extends Binder implements BLASTSyncEngine.TransactionReadyListe return; } // Ensure that wallpaper visibility is updated with the latest wallpaper target. - if (hasWallpaper) { - dc.mWallpaperController.adjustWallpaperWindows(); + for (int i = mParticipants.size() - 1; i >= 0; --i) { + final WindowContainer wc = mParticipants.valueAt(i); + if (isWallpaper(wc) && wc.getDisplayContent() != null) { + wc.getDisplayContent().mWallpaperController.adjustWallpaperWindows(); + } } mState = STATE_PLAYING; diff --git a/services/core/java/com/android/server/wm/TransitionController.java b/services/core/java/com/android/server/wm/TransitionController.java index 3d9d824fe075c..c267cbacdf245 100644 --- a/services/core/java/com/android/server/wm/TransitionController.java +++ b/services/core/java/com/android/server/wm/TransitionController.java @@ -256,6 +256,17 @@ class TransitionController { return false; } + /** @return {@code true} if wc is in a participant subtree */ + boolean isTransitionOnDisplay(@NonNull DisplayContent dc) { + if (mCollectingTransition != null && mCollectingTransition.isOnDisplay(dc)) { + return true; + } + for (int i = mPlayingTransitions.size() - 1; i >= 0; --i) { + if (mPlayingTransitions.get(i).isOnDisplay(dc)) return true; + } + return false; + } + /** * @return {@code true} if {@param ar} is part of a transient-launch activity in an active * transition.