From 275aba2deb2a9b7069e6ea66327dd6ca7854d6aa Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Thu, 19 Nov 2020 19:28:38 +0800 Subject: [PATCH] Move handleCompleteDeferredRemoval to animator thread The method may remove window hierarchy element in random timing (usually depends on animation). If the caller indirectly calls to performSurfacePlacement inside a loop that also removes element, the loop may encounter index out of bound exception. It is hard for the caller to aware how to iterate window containers safely. Because performSurfacePlacement always calls WindowAnimator scheduleAnimationLocked, the movement of invocation should not miss the original cases for deferred removal. It also reduces unnecessary invocations up to 50% because animator won't execute multiple times if it is scheduled, while performSurfacePlacement calls each time. And the partial cost may be moved out from binder thread. Bug: 173270358 Test: CtsWindowManagerDeviceTestCases Change-Id: If9a45403e7bbedb2073f8e6f6e90934af0c4c29e --- .../core/java/com/android/server/wm/RootWindowContainer.java | 3 --- services/core/java/com/android/server/wm/WindowAnimator.java | 3 +++ services/core/java/com/android/server/wm/WindowContainer.java | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/services/core/java/com/android/server/wm/RootWindowContainer.java b/services/core/java/com/android/server/wm/RootWindowContainer.java index 72cd32f8d0579..14316fa681361 100644 --- a/services/core/java/com/android/server/wm/RootWindowContainer.java +++ b/services/core/java/com/android/server/wm/RootWindowContainer.java @@ -994,9 +994,6 @@ class RootWindowContainer extends WindowContainer } } - // Remove all deferred displays stacks, tasks, and activities. - handleCompleteDeferredRemoval(); - forAllDisplays(dc -> { dc.getInputMonitor().updateInputWindowsLw(true /*force*/); dc.updateSystemGestureExclusion(); diff --git a/services/core/java/com/android/server/wm/WindowAnimator.java b/services/core/java/com/android/server/wm/WindowAnimator.java index 9d0bac9dd2905..9d07304aa2b96 100644 --- a/services/core/java/com/android/server/wm/WindowAnimator.java +++ b/services/core/java/com/android/server/wm/WindowAnimator.java @@ -143,6 +143,9 @@ public class WindowAnimator { ProtoLog.i(WM_SHOW_TRANSACTIONS, ">>> OPEN TRANSACTION animate"); mService.openSurfaceTransaction(); try { + // Remove all deferred displays, tasks, and activities. + mService.mRoot.handleCompleteDeferredRemoval(); + final AccessibilityController accessibilityController = mService.mAccessibilityController; final int numDisplays = mDisplayContentsAnimators.size(); diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java index dd5afdd4f2b97..6cd6a57eedd49 100644 --- a/services/core/java/com/android/server/wm/WindowContainer.java +++ b/services/core/java/com/android/server/wm/WindowContainer.java @@ -1060,7 +1060,7 @@ class WindowContainer extends ConfigurationContainer< // descendant. E.g. if a display is pending to be removed because it contains an // activity with {@link ActivityRecord#mIsExiting} is true, the display may be // removed when completing the removal of the last activity from - // {@link ActivityRecord#checkCompleteDeferredRemoval}. + // {@link ActivityRecord#handleCompleteDeferredRemoval}. return false; } }