From 603462e0731027f665b574aa6bef93fc78d63ecb Mon Sep 17 00:00:00 2001 From: Evan Rosky Date: Mon, 14 Jun 2021 13:57:55 -0700 Subject: [PATCH] Fix some bugs with display iteration/removal ordering There were loops whose body were removing elements during loop iteration. This CL separates some of those to avoid out-of-bounds errors. Also fixes an issue where navbar was using a non-visual context. Bug: 183993924 Test: atest MultiDisplaySystemDecorationTests Change-Id: I90eb229e1739e18e2b9b9347212cc52c5b48c30e --- .../NavigationBarController.java | 5 ++-- .../com/android/server/wm/DisplayContent.java | 28 +++++++++++++------ .../server/wm/RootWindowContainer.java | 10 +++---- .../core/java/com/android/server/wm/Task.java | 3 +- .../com/android/server/wm/WindowToken.java | 5 ++++ 5 files changed, 33 insertions(+), 18 deletions(-) diff --git a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarController.java b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarController.java index aa5964bf5d3b8..96c8c16309e8e 100644 --- a/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarController.java +++ b/packages/SystemUI/src/com/android/systemui/navigationbar/NavigationBarController.java @@ -17,6 +17,7 @@ package com.android.systemui.navigationbar; import static android.view.Display.DEFAULT_DISPLAY; +import static android.view.WindowManager.LayoutParams.TYPE_NAVIGATION_BAR; import static android.view.WindowManagerPolicyConstants.NAV_BAR_MODE_3BUTTON; import android.content.Context; @@ -351,9 +352,7 @@ public class NavigationBarController implements Callbacks, Log.w(TAG, "Cannot get WindowManager."); return; } - final Context context = isOnDefaultDisplay - ? mContext - : mContext.createDisplayContext(display); + final Context context = mContext.createWindowContext(display, TYPE_NAVIGATION_BAR, null); NavigationBar navBar = new NavigationBar(context, mWindowManager, mAssistManagerLazy, diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index 5c86a85892e4e..2c4f74ad85a68 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -696,6 +696,8 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp // well and thus won't change the top resumed / focused record boolean mDontMoveToTop; + private final ArrayList mTmpActivityList = new ArrayList<>(); + private final Consumer mUpdateWindowsForAnimator = w -> { WindowStateAnimator winAnimator = w.mWinAnimator; final ActivityRecord activity = w.mActivityRecord; @@ -2485,7 +2487,7 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp @Override boolean isVisibleRequested() { - return isVisible(); + return isVisible() && !mRemoved && !mRemoving; } @Override @@ -4508,6 +4510,8 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp } } + // clear first just in case. + mTmpActivityList.clear(); // Time to remove any exiting applications? forAllRootTasks(task -> { final ArrayList activities = task.mExitingActivities; @@ -4515,16 +4519,24 @@ class DisplayContent extends RootDisplayArea implements WindowManagerPolicy.Disp final ActivityRecord activity = activities.get(j); if (!activity.hasVisible && !mDisplayContent.mClosingApps.contains(activity) && (!activity.mIsExiting || activity.isEmpty())) { - // Make sure there is no animation running on this activity, so any windows - // associated with it will be removed as soon as their animations are - // complete. - cancelAnimation(); - ProtoLog.v(WM_DEBUG_ADD_REMOVE, - "performLayout: Activity exiting now removed %s", activity); - activity.removeIfPossible(); + mTmpActivityList.add(activity); } } }); + if (!mTmpActivityList.isEmpty()) { + // Make sure there is no animation running on this activity, so any windows + // associated with it will be removed as soon as their animations are + // complete. + cancelAnimation(); + } + for (int i = 0; i < mTmpActivityList.size(); ++i) { + final ActivityRecord activity = mTmpActivityList.get(i); + ProtoLog.v(WM_DEBUG_ADD_REMOVE, + "performLayout: Activity exiting now removed %s", activity); + activity.removeIfPossible(); + } + // Clear afterwards so we don't hold references. + mTmpActivityList.clear(); } @Override diff --git a/services/core/java/com/android/server/wm/RootWindowContainer.java b/services/core/java/com/android/server/wm/RootWindowContainer.java index c2b9796a3d940..03cc7b6189c7c 100644 --- a/services/core/java/com/android/server/wm/RootWindowContainer.java +++ b/services/core/java/com/android/server/wm/RootWindowContainer.java @@ -71,7 +71,6 @@ import static com.android.server.wm.ActivityTaskSupervisor.ON_TOP; import static com.android.server.wm.ActivityTaskSupervisor.PRESERVE_WINDOWS; import static com.android.server.wm.ActivityTaskSupervisor.dumpHistoryList; import static com.android.server.wm.ActivityTaskSupervisor.printThisActivity; -import static com.android.server.wm.RecentsAnimationController.REORDER_KEEP_IN_PLACE; import static com.android.server.wm.RootWindowContainerProto.IS_HOME_RECENTS_COMPONENT; import static com.android.server.wm.RootWindowContainerProto.KEYGUARD_CONTROLLER; import static com.android.server.wm.RootWindowContainerProto.WINDOW_CONTAINER; @@ -817,8 +816,7 @@ class RootWindowContainer extends WindowContainer } // Initialize state of exiting tokens. - final int numDisplays = mChildren.size(); - for (int displayNdx = 0; displayNdx < numDisplays; ++displayNdx) { + for (int displayNdx = 0; displayNdx < mChildren.size(); ++displayNdx) { final DisplayContent displayContent = mChildren.get(displayNdx); displayContent.setExitingTokensHasVisible(false); } @@ -867,7 +865,7 @@ class RootWindowContainer extends WindowContainer recentsAnimationController.checkAnimationReady(defaultDisplay.mWallpaperController); } - for (int displayNdx = 0; displayNdx < numDisplays; ++displayNdx) { + for (int displayNdx = 0; displayNdx < mChildren.size(); ++displayNdx) { final DisplayContent displayContent = mChildren.get(displayNdx); if (displayContent.mWallpaperMayChange) { if (DEBUG_WALLPAPER_LIGHT) Slog.v(TAG, "Wallpaper may change! Adjusting"); @@ -929,12 +927,12 @@ class RootWindowContainer extends WindowContainer } // Time to remove any exiting tokens? - for (int displayNdx = 0; displayNdx < numDisplays; ++displayNdx) { + for (int displayNdx = mChildren.size() - 1; displayNdx >= 0; --displayNdx) { final DisplayContent displayContent = mChildren.get(displayNdx); displayContent.removeExistingTokensIfPossible(); } - for (int displayNdx = 0; displayNdx < numDisplays; ++displayNdx) { + for (int displayNdx = 0; displayNdx < mChildren.size(); ++displayNdx) { final DisplayContent displayContent = mChildren.get(displayNdx); if (displayContent.pendingLayoutChanges != 0) { displayContent.setLayoutNeeded(); diff --git a/services/core/java/com/android/server/wm/Task.java b/services/core/java/com/android/server/wm/Task.java index a6bf520561acf..9a6e3a800e656 100644 --- a/services/core/java/com/android/server/wm/Task.java +++ b/services/core/java/com/android/server/wm/Task.java @@ -6053,7 +6053,8 @@ class Task extends TaskFragment { /** Returns true if a removal action is still being deferred. */ boolean handleCompleteDeferredRemoval() { - if (isAnimating(TRANSITION | CHILDREN)) { + if (isAnimating(TRANSITION | CHILDREN) + || mAtmService.getTransitionController().inTransition(this)) { return true; } diff --git a/services/core/java/com/android/server/wm/WindowToken.java b/services/core/java/com/android/server/wm/WindowToken.java index 26241421cef92..fa32be363ffea 100644 --- a/services/core/java/com/android/server/wm/WindowToken.java +++ b/services/core/java/com/android/server/wm/WindowToken.java @@ -231,6 +231,11 @@ class WindowToken extends WindowContainer { ProtoLog.w(WM_DEBUG_WINDOW_MOVEMENT, "removeAllWindowsIfPossible: removing win=%s", win); win.removeIfPossible(); + if (i > mChildren.size()) { + // It's possible for removeIfPossible to delete siblings (for example if it is a + // starting window, it will perform operations on the ActivityRecord). + i = mChildren.size(); + } } }