From 6d41026f1b3dc910c9d34ab89993a280dc9679cf Mon Sep 17 00:00:00 2001 From: Bryce Lee Date: Tue, 28 Feb 2017 15:30:17 -0800 Subject: [PATCH] Clean up closing apps list when clearing anAppWindowToken's task. Previously it was possible for an AppWindowToken to be removed while on the closing apps list, used in transition animations. During these transitions, the visibility of the token is modified. Since visibility relies on the WindowContainer parent, a NullPointerException would occur. This changelist addresses the issue by making sure to remove any AppWindowToken from this list when its task is set to null. Change-Id: Id9234822b228f4658f04d42ac0fe7b49ded6f5a1 Fixes: 35352214 Test: manual (primarily code inspection) --- .../wm/AppWindowContainerController.java | 3 +- .../com/android/server/wm/AppWindowToken.java | 54 +++++++++++++++++-- .../wm/DockedStackDividerController.java | 2 +- .../core/java/com/android/server/wm/Task.java | 5 +- .../android/server/wm/TaskSnapshotCache.java | 4 +- .../server/wm/TaskSnapshotController.java | 5 +- .../server/wm/TaskSnapshotSurface.java | 8 +-- .../java/com/android/server/wm/TaskStack.java | 2 +- .../server/wm/WindowManagerService.java | 16 +++--- .../com/android/server/wm/WindowState.java | 6 +-- .../server/wm/WindowSurfacePlacer.java | 2 +- .../wm/AppWindowContainerControllerTests.java | 11 ++-- .../server/wm/DisplayContentTests.java | 2 +- .../server/wm/TaskSnapshotControllerTest.java | 2 +- .../android/server/wm/WindowTestsBase.java | 2 +- 15 files changed, 86 insertions(+), 38 deletions(-) diff --git a/services/core/java/com/android/server/wm/AppWindowContainerController.java b/services/core/java/com/android/server/wm/AppWindowContainerController.java index 3a86874b1f4e1..266ab4cfde0bc 100644 --- a/services/core/java/com/android/server/wm/AppWindowContainerController.java +++ b/services/core/java/com/android/server/wm/AppWindowContainerController.java @@ -525,7 +525,8 @@ public class AppWindowContainerController private boolean createSnapshot() { final TaskSnapshot snapshot = mService.mTaskSnapshotController.getSnapshot( - mContainer.mTask.mTaskId, mContainer.mTask.mUserId, false /* restoreFromDisk */); + mContainer.getTask().mTaskId, mContainer.getTask().mUserId, + false /* restoreFromDisk */); if (snapshot == null) { return false; diff --git a/services/core/java/com/android/server/wm/AppWindowToken.java b/services/core/java/com/android/server/wm/AppWindowToken.java index e67c91e7a454d..994f38d5a94ae 100644 --- a/services/core/java/com/android/server/wm/AppWindowToken.java +++ b/services/core/java/com/android/server/wm/AppWindowToken.java @@ -85,13 +85,17 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree final boolean mVoiceInteraction; // TODO: Use getParent instead? - Task mTask; + private Task mTask; /** @see WindowContainer#fillsParent() */ private boolean mFillsParent; boolean layoutConfigChanges; boolean mShowForAllUsers; int mTargetSdk; + // Flag set while reparenting to prevent actions normally triggered by an individual parent + // change. + private boolean mReparenting; + // The input dispatching timeout for this application token in nanoseconds. long mInputDispatchingTimeoutNanos; @@ -426,10 +430,7 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree void removeIfPossible() { mIsExiting = false; removeAllWindowsIfPossible(); - if (mTask != null) { - mTask.mStack.mExitingAppTokens.remove(this); - removeImmediately(); - } + removeImmediately(); } @Override @@ -483,6 +484,7 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree removed = true; stopFreezingScreen(true, true); + if (mService.mFocusedApp == this) { if (DEBUG_FOCUS_LIGHT) Slog.v(TAG_WM, "Removing focused app token:" + this); mService.mFocusedApp = null; @@ -664,6 +666,37 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree allDrawnExcludingSaved = false; } + Task getTask() { + return mTask; + } + + /** + * Sets the associated task, cleaning up dependencies when unset. + */ + void setTask(Task task) { + // Note: the following code assumes that the previous task's stack is the same as the + // new task's stack. + if (!mReparenting && mTask != null && mTask.mStack != null) { + mTask.mStack.mExitingAppTokens.remove(this); + } + + mTask = task; + } + + @Override + void onParentSet() { + super.onParentSet(); + + // When the associated task is {@code null}, the {@link AppWindowToken} can no longer + // access visual elements like the {@link DisplayContent}. We must remove any associations + // such as animations. + if (!mReparenting && mTask == null) { + // It is possible we have been marked as a closing app earlier. We must remove ourselves + // from this list so we do not participate in any future animations. + mService.mClosingApps.remove(this); + } + } + void postWindowRemoveStartingWindowCleanup(WindowState win) { // TODO: Something smells about the code below...Is there a better way? if (startingWindow == win) { @@ -866,13 +899,24 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree throw new IllegalArgumentException( "window token=" + this + " already child of task=" + mTask); } + + if (mTask.mStack != task.mStack) { + throw new IllegalArgumentException( + "window token=" + this + " current task=" + mTask + + " belongs to a different stack than " + task); + } + if (DEBUG_ADD_REMOVE) Slog.i(TAG, "reParentWindowToken: removing window token=" + this + " from task=" + mTask); final DisplayContent prevDisplayContent = getDisplayContent(); + mReparenting = true; + getParent().removeChild(this); task.addChild(this, position); + mReparenting = false; + // Relayout display(s). final DisplayContent displayContent = task.getDisplayContent(); displayContent.setLayoutNeeded(); diff --git a/services/core/java/com/android/server/wm/DockedStackDividerController.java b/services/core/java/com/android/server/wm/DockedStackDividerController.java index 75a79fd0b55fa..aa8e377427670 100644 --- a/services/core/java/com/android/server/wm/DockedStackDividerController.java +++ b/services/core/java/com/android/server/wm/DockedStackDividerController.java @@ -588,7 +588,7 @@ public class DockedStackDividerController implements DimLayerUser { private boolean containsAppInDockedStack(ArraySet apps) { for (int i = apps.size() - 1; i >= 0; i--) { final AppWindowToken token = apps.valueAt(i); - if (token.mTask != null && token.mTask.mStack.mStackId == DOCKED_STACK_ID) { + if (token.getTask() != null && token.getTask().mStack.mStackId == DOCKED_STACK_ID) { return true; } } diff --git a/services/core/java/com/android/server/wm/Task.java b/services/core/java/com/android/server/wm/Task.java index da5fcf301c983..07eb88dc9ad7b 100644 --- a/services/core/java/com/android/server/wm/Task.java +++ b/services/core/java/com/android/server/wm/Task.java @@ -133,7 +133,7 @@ class Task extends WindowContainer implements DimLayer.DimLayerU void addChild(AppWindowToken wtoken, int position) { position = getAdjustedAddPosition(position); super.addChild(wtoken, position); - wtoken.mTask = this; + wtoken.setTask(this); mDeferRemoval = false; } @@ -244,7 +244,8 @@ class Task extends WindowContainer implements DimLayer.DimLayerU removeIfPossible(); } } - token.mTask = null; + + token.setTask(null /*task*/); } void setSendingToBottom(boolean toBottom) { diff --git a/services/core/java/com/android/server/wm/TaskSnapshotCache.java b/services/core/java/com/android/server/wm/TaskSnapshotCache.java index 601bf28b7ed88..40283360dcfc1 100644 --- a/services/core/java/com/android/server/wm/TaskSnapshotCache.java +++ b/services/core/java/com/android/server/wm/TaskSnapshotCache.java @@ -106,8 +106,8 @@ class TaskSnapshotCache { if (taskId != null) { removeRunningEntry(taskId); } - if (wtoken.mTask != null) { - mRetrievalCache.remove(wtoken.mTask.mTaskId); + if (wtoken.getTask() != null) { + mRetrievalCache.remove(wtoken.getTask().mTaskId); } } diff --git a/services/core/java/com/android/server/wm/TaskSnapshotController.java b/services/core/java/com/android/server/wm/TaskSnapshotController.java index 5041138a77a3d..5995bba2c4ff6 100644 --- a/services/core/java/com/android/server/wm/TaskSnapshotController.java +++ b/services/core/java/com/android/server/wm/TaskSnapshotController.java @@ -141,11 +141,12 @@ class TaskSnapshotController { outClosingTasks.clear(); for (int i = closingApps.size() - 1; i >= 0; i--) { final AppWindowToken atoken = closingApps.valueAt(i); + final Task task = atoken.getTask(); // If the task of the app is not visible anymore, it means no other app in that task // is opening. Thus, the task is closing. - if (atoken.mTask != null && !atoken.mTask.isVisible()) { - outClosingTasks.add(closingApps.valueAt(i).mTask); + if (task != null && !task.isVisible()) { + outClosingTasks.add(task); } } } diff --git a/services/core/java/com/android/server/wm/TaskSnapshotSurface.java b/services/core/java/com/android/server/wm/TaskSnapshotSurface.java index 9f5241226d09e..9f34bd788f2e0 100644 --- a/services/core/java/com/android/server/wm/TaskSnapshotSurface.java +++ b/services/core/java/com/android/server/wm/TaskSnapshotSurface.java @@ -96,9 +96,11 @@ class TaskSnapshotSurface implements StartingSurface { // TODO: Inherit behavior whether to draw behind status bar/nav bar. layoutParams.systemUiVisibility = View.SYSTEM_UI_FLAG_LAYOUT_FULLSCREEN | View.SYSTEM_UI_FLAG_LAYOUT_HIDE_NAVIGATION; - layoutParams.setTitle(String.format(TITLE_FORMAT, token.mTask.mTaskId)); - if (token.mTask != null) { - final TaskDescription taskDescription = token.mTask.getTaskDescription(); + final Task task = token.getTask(); + if (task != null) { + layoutParams.setTitle(String.format(TITLE_FORMAT,task.mTaskId)); + + final TaskDescription taskDescription = task.getTaskDescription(); if (taskDescription != null) { fillBackgroundColor = taskDescription.getBackgroundColor(); } diff --git a/services/core/java/com/android/server/wm/TaskStack.java b/services/core/java/com/android/server/wm/TaskStack.java index b79ea71f36192..6cc2efb78526f 100644 --- a/services/core/java/com/android/server/wm/TaskStack.java +++ b/services/core/java/com/android/server/wm/TaskStack.java @@ -644,7 +644,7 @@ public class TaskStack extends WindowContainer implements DimLayer.DimLaye } for (int appNdx = mExitingAppTokens.size() - 1; appNdx >= 0; --appNdx) { final AppWindowToken wtoken = mExitingAppTokens.get(appNdx); - if (wtoken.mTask == task) { + if (wtoken.getTask() == task) { wtoken.mIsExiting = false; mExitingAppTokens.remove(appNdx); } diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index eb10f0c2978c1..eb3a2d15a8550 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -1445,9 +1445,9 @@ public class WindowManagerService extends IWindowManager.Stub if (displayContent.isDefaultDisplay) { final DisplayInfo displayInfo = displayContent.getDisplayInfo(); final Rect taskBounds; - if (atoken != null && atoken.mTask != null) { + if (atoken != null && atoken.getTask() != null) { taskBounds = mTmpRect; - atoken.mTask.getBounds(mTmpRect); + atoken.getTask().getBounds(mTmpRect); } else { taskBounds = null; } @@ -2292,7 +2292,7 @@ public class WindowManagerService extends IWindowManager.Stub // is running. Trace.traceBegin(Trace.TRACE_TAG_WINDOW_MANAGER, "WM#applyAnimationLocked"); if (okToDisplay()) { - final DisplayContent displayContent = atoken.mTask.getDisplayContent(); + final DisplayContent displayContent = atoken.getTask().getDisplayContent(); final DisplayInfo displayInfo = displayContent.getDisplayInfo(); final int width = displayInfo.appWidth; final int height = displayInfo.appHeight; @@ -2333,7 +2333,7 @@ public class WindowManagerService extends IWindowManager.Stub final Configuration displayConfig = displayContent.getConfiguration(); Animation a = mAppTransition.loadAnimation(lp, transit, enter, displayConfig.uiMode, displayConfig.orientation, frame, displayFrame, insets, surfaceInsets, - isVoiceInteraction, freeform, atoken.mTask.mTaskId); + isVoiceInteraction, freeform, atoken.getTask().mTaskId); if (a != null) { if (DEBUG_ANIM) logWithStack(TAG, "Loaded animation " + a + " for " + atoken); final int containingWidth = frame.width(); @@ -2553,8 +2553,8 @@ public class WindowManagerService extends IWindowManager.Stub } void setFocusTaskRegionLocked(AppWindowToken previousFocus) { - final Task focusedTask = mFocusedApp != null ? mFocusedApp.mTask : null; - final Task previousTask = previousFocus != null ? previousFocus.mTask : null; + final Task focusedTask = mFocusedApp != null ? mFocusedApp.getTask() : null; + final Task previousTask = previousFocus != null ? previousFocus.getTask() : null; final DisplayContent focusedDisplayContent = focusedTask != null ? focusedTask.getDisplayContent() : null; final DisplayContent previousDisplayContent = @@ -5085,8 +5085,8 @@ public class WindowManagerService extends IWindowManager.Stub // Also don't use mInputMethodTarget's stack, because some window with FLAG_NOT_FOCUSABLE // and FLAG_ALT_FOCUSABLE_IM flags both set might be set to IME target so they're moved // to make room for IME, but the window is not the focused window that's taking input. - return (mFocusedApp != null && mFocusedApp.mTask != null) ? - mFocusedApp.mTask.mStack : null; + return (mFocusedApp != null && mFocusedApp.getTask() != null) ? + mFocusedApp.getTask().mStack : null; } public boolean detectSafeMode() { diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index 14f14c5116716..98bef45ff98cf 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -1202,7 +1202,7 @@ class WindowState extends WindowContainer implements WindowManagerP } Task getTask() { - return mAppToken != null ? mAppToken.mTask : null; + return mAppToken != null ? mAppToken.getTask() : null; } TaskStack getStack() { @@ -2378,8 +2378,8 @@ class WindowState extends WindowContainer implements WindowManagerP /** @return true if this window desires touch events. */ boolean canReceiveTouchInput() { - return mAppToken != null && mAppToken.mTask != null - && mAppToken.mTask.mStack.shouldIgnoreInput(); + return mAppToken != null && mAppToken.getTask() != null + && mAppToken.getTask().mStack.shouldIgnoreInput(); } @Override diff --git a/services/core/java/com/android/server/wm/WindowSurfacePlacer.java b/services/core/java/com/android/server/wm/WindowSurfacePlacer.java index 897d5b86d3d9d..f247ebe62f8ce 100644 --- a/services/core/java/com/android/server/wm/WindowSurfacePlacer.java +++ b/services/core/java/com/android/server/wm/WindowSurfacePlacer.java @@ -651,7 +651,7 @@ class WindowSurfacePlacer { if (openingAppAnimator == null || openingAppAnimator.animation == null) { return; } - final int taskId = appToken.mTask.mTaskId; + final int taskId = appToken.getTask().mTaskId; Bitmap thumbnailHeader = mService.mAppTransition.getAppTransitionThumbnailHeader(taskId); if (thumbnailHeader == null || thumbnailHeader.getConfig() == Bitmap.Config.ALPHA_8) { if (DEBUG_APP_TRANSITIONS) Slog.d(TAG, "No thumbnail header bitmap for: " + taskId); diff --git a/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java b/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java index 04e55836a7069..2ccaefc4512e5 100644 --- a/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java +++ b/services/tests/servicestests/src/com/android/server/wm/AppWindowContainerControllerTests.java @@ -138,19 +138,18 @@ public class AppWindowContainerControllerTests extends WindowTestsBase { @Test public void testReparent() throws Exception { + final StackWindowController stackController = + createStackControllerOnDisplay(sDisplayContent); final TestTaskWindowContainerController taskController1 = - new TestTaskWindowContainerController( - createStackControllerOnDisplay(sDisplayContent)); + new TestTaskWindowContainerController(stackController); final TestAppWindowContainerController appWindowController1 = createAppWindowController( taskController1); final TestTaskWindowContainerController taskController2 = - new TestTaskWindowContainerController( - createStackControllerOnDisplay(sDisplayContent)); + new TestTaskWindowContainerController(stackController); final TestAppWindowContainerController appWindowController2 = createAppWindowController( taskController2); final TestTaskWindowContainerController taskController3 = - new TestTaskWindowContainerController( - createStackControllerOnDisplay(sDisplayContent)); + new TestTaskWindowContainerController(stackController); try { appWindowController1.reparent(taskController1, 0); diff --git a/services/tests/servicestests/src/com/android/server/wm/DisplayContentTests.java b/services/tests/servicestests/src/com/android/server/wm/DisplayContentTests.java index 73ad7c241396e..e589bc77d6415 100644 --- a/services/tests/servicestests/src/com/android/server/wm/DisplayContentTests.java +++ b/services/tests/servicestests/src/com/android/server/wm/DisplayContentTests.java @@ -54,7 +54,7 @@ public class DisplayContentTests extends WindowTestsBase { sDisplayContent, "exiting app"); final AppWindowToken exitingAppToken = exitingAppWindow.mAppToken; exitingAppToken.mIsExiting = true; - exitingAppToken.mTask.mStack.mExitingAppTokens.add(exitingAppToken); + exitingAppToken.getTask().mStack.mExitingAppTokens.add(exitingAppToken); assertForAllWindowsOrder(Arrays.asList( sWallpaperWindow, diff --git a/services/tests/servicestests/src/com/android/server/wm/TaskSnapshotControllerTest.java b/services/tests/servicestests/src/com/android/server/wm/TaskSnapshotControllerTest.java index 5dff9973f734e..58d277b274e81 100644 --- a/services/tests/servicestests/src/com/android/server/wm/TaskSnapshotControllerTest.java +++ b/services/tests/servicestests/src/com/android/server/wm/TaskSnapshotControllerTest.java @@ -49,7 +49,7 @@ public class TaskSnapshotControllerTest extends WindowTestsBase { final ArraySet closingTasks = new ArraySet<>(); sWm.mTaskSnapshotController.getClosingTasks(closingApps, closingTasks); assertEquals(1, closingTasks.size()); - assertEquals(closingWindow.mAppToken.mTask, closingTasks.valueAt(0)); + assertEquals(closingWindow.mAppToken.getTask(), closingTasks.valueAt(0)); } @Test diff --git a/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java b/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java index 83926654884be..440362d5c1b4c 100644 --- a/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java +++ b/services/tests/servicestests/src/com/android/server/wm/WindowTestsBase.java @@ -167,7 +167,7 @@ class WindowTestsBase { final TestTaskWindowContainerController taskController = new TestTaskWindowContainerController(stackController); TestAppWindowToken appWinToken = new TestAppWindowToken(sDisplayContent); - appWinToken.mTask = taskController.mContainer; + appWinToken.setTask(taskController.mContainer); final WindowState win = createWindow(null, TYPE_BASE_APPLICATION, name); win.mAppToken = appWinToken; return win;