From 402b440f3222f4a7a41c8917b486681bb4278ed2 Mon Sep 17 00:00:00 2001 From: Riddle Hsu Date: Tue, 6 Nov 2018 17:23:15 +0800 Subject: [PATCH] Fix unable to cleanup a removed display Assume a display supports system decoration and allows reparenting stacks to another display when the display is removed. If there exists a standard type finishing activity during removing the display, the activity will be reparented and found the original stack is empty, then home is started again on the removed display. This change combines the condition ActivityDisplay.mRemoved into ActivityStack.isAttached, and defers the resume during finishing or reparenting, so the stacks in the removing display won't be focusable and resumed. Also keep original z-order of the reparented stacks and reduce unnecessary resume, visibility update for each reparented stack. Bug: 119093839 Test: atest ActivityDisplayTests#testNotResumeHomeStackOnRemovingDisplay Change-Id: I7dea2582ce8bd4aa4a18e1d5d721bfb1d9027c6a --- .../android/server/wm/ActivityDisplay.java | 50 +++++++++++++------ .../com/android/server/wm/ActivityStack.java | 12 ++++- .../server/wm/ActivityStackSupervisor.java | 6 +-- .../server/wm/ActivityDisplayTests.java | 39 +++++++++++++++ 4 files changed, 88 insertions(+), 19 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityDisplay.java b/services/core/java/com/android/server/wm/ActivityDisplay.java index f8c4ca219280c..8d5380c710686 100644 --- a/services/core/java/com/android/server/wm/ActivityDisplay.java +++ b/services/core/java/com/android/server/wm/ActivityDisplay.java @@ -1010,29 +1010,50 @@ class ActivityDisplay extends ConfigurationContainer void remove() { final boolean destroyContentOnRemoval = shouldDestroyContentOnRemove(); + ActivityStack lastReparentedStack = null; + mPreferredTopFocusableStack = null; // Stacks could be reparented from the removed display to other display. While // reparenting the last stack of the removed display, the remove display is ready to be // released (no more ActivityStack). But, we cannot release it at that moment or the // related WindowContainer and WindowContainerController will also be removed. So, we // set display as removed after reparenting stack finished. - for (int i = mStacks.size() - 1; i >= 0; --i) { - final ActivityStack stack = mStacks.get(i); - // Always finish non-standard type stacks. - if (destroyContentOnRemoval || !stack.isActivityTypeStandardOrUndefined()) { - stack.finishAllActivitiesLocked(true /* immediately */); - } else { - // If default display is in split-window mode, set windowing mode of the stack to - // split-screen secondary. Otherwise, set the windowing mode to undefined by - // default to let stack inherited the windowing mode from the new display. - int windowingMode = mSupervisor.getDefaultDisplay().hasSplitScreenPrimaryStack() - ? WINDOWING_MODE_SPLIT_SCREEN_SECONDARY : WINDOWING_MODE_UNDEFINED; - mSupervisor.moveStackToDisplayLocked(stack.mStackId, DEFAULT_DISPLAY, true); - stack.setWindowingMode(windowingMode); + final ActivityDisplay toDisplay = mSupervisor.getDefaultDisplay(); + mSupervisor.beginDeferResume(); + try { + int numStacks = mStacks.size(); + // Keep the order from bottom to top. + for (int stackNdx = 0; stackNdx < numStacks; stackNdx++) { + final ActivityStack stack = mStacks.get(stackNdx); + // Always finish non-standard type stacks. + if (destroyContentOnRemoval || !stack.isActivityTypeStandardOrUndefined()) { + stack.finishAllActivitiesLocked(true /* immediately */); + } else { + // If default display is in split-window mode, set windowing mode of the stack + // to split-screen secondary. Otherwise, set the windowing mode to undefined by + // default to let stack inherited the windowing mode from the new display. + final int windowingMode = toDisplay.hasSplitScreenPrimaryStack() + ? WINDOWING_MODE_SPLIT_SCREEN_SECONDARY + : WINDOWING_MODE_UNDEFINED; + stack.reparent(toDisplay, true /* onTop */, true /* displayRemoved */); + stack.setWindowingMode(windowingMode); + lastReparentedStack = stack; + } + // Stacks may be removed from this display. Ensure each stack will be processed and + // the loop will end. + stackNdx -= numStacks - mStacks.size(); + numStacks = mStacks.size(); } + } finally { + mSupervisor.endDeferResume(); } mRemoved = true; + // Only update focus/visibility for the last one because there may be many stacks are + // reparented and the intermediate states are unnecessary. + if (lastReparentedStack != null) { + lastReparentedStack.postReparent(); + } releaseSelfIfNeeded(); mSupervisor.getKeyguardController().onDisplayRemoved(mDisplayId); @@ -1071,7 +1092,8 @@ class ActivityDisplay extends ConfigurationContainer || mDisplayId == mSupervisor.mService.mVr2dDisplayId; } - private boolean shouldDestroyContentOnRemove() { + @VisibleForTesting + boolean shouldDestroyContentOnRemove() { return mDisplay.getRemoveMode() == REMOVE_MODE_DESTROY_CONTENT; } diff --git a/services/core/java/com/android/server/wm/ActivityStack.java b/services/core/java/com/android/server/wm/ActivityStack.java index a8b4a9d43ebbc..44a62e2066d9a 100644 --- a/services/core/java/com/android/server/wm/ActivityStack.java +++ b/services/core/java/com/android/server/wm/ActivityStack.java @@ -729,7 +729,7 @@ class ActivityStack extends ConfigurationContai } /** Adds the stack to specified display and calls WindowManager to do the same. */ - void reparent(ActivityDisplay activityDisplay, boolean onTop) { + void reparent(ActivityDisplay activityDisplay, boolean onTop, boolean displayRemoved) { // TODO: We should probably resolve the windowing mode for the stack on the new display here // so that it end up in a compatible mode in the new display. e.g. split-screen secondary. removeFromDisplay(); @@ -738,6 +738,13 @@ class ActivityStack extends ConfigurationContai mTmpRect2.setEmpty(); mWindowContainerController.reparent(activityDisplay.mDisplayId, mTmpRect2, onTop); postAddToDisplay(activityDisplay, mTmpRect2.isEmpty() ? null : mTmpRect2, onTop); + if (!displayRemoved) { + postReparent(); + } + } + + /** Resume next focusable stack after reparenting to another display. */ + void postReparent() { adjustFocusToNextFocusableStack("reparent", true /* allowFocusSelf */); mStackSupervisor.resumeFocusedStacksTopActivitiesLocked(); // Update visibility of activities before notifying WM. This way it won't try to resize @@ -1151,7 +1158,8 @@ class ActivityStack extends ConfigurationContai } final boolean isAttached() { - return getParent() != null; + final ActivityDisplay display = getDisplay(); + return display != null && !display.isRemoved(); } /** diff --git a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java index 77b331e6c4a9c..a0b0360ef15e2 100644 --- a/services/core/java/com/android/server/wm/ActivityStackSupervisor.java +++ b/services/core/java/com/android/server/wm/ActivityStackSupervisor.java @@ -3172,7 +3172,7 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D + " to its current displayId=" + displayId); } - stack.reparent(activityDisplay, onTop); + stack.reparent(activityDisplay, onTop, false /* displayRemoved */); // TODO(multi-display): resize stacks properly if moved from split-screen. } @@ -4562,14 +4562,14 @@ public class ActivityStackSupervisor extends ConfigurationContainer implements D /** * Begin deferring resume to avoid duplicate resumes in one pass. */ - private void beginDeferResume() { + void beginDeferResume() { mDeferResumeCount++; } /** * End deferring resume and determine if resume can be called. */ - private void endDeferResume() { + void endDeferResume() { mDeferResumeCount--; } diff --git a/services/tests/wmtests/src/com/android/server/wm/ActivityDisplayTests.java b/services/tests/wmtests/src/com/android/server/wm/ActivityDisplayTests.java index d3f1a0a537604..7a9c8dc27e7d7 100644 --- a/services/tests/wmtests/src/com/android/server/wm/ActivityDisplayTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/ActivityDisplayTests.java @@ -16,6 +16,7 @@ package com.android.server.wm; +import static android.app.WindowConfiguration.ACTIVITY_TYPE_HOME; import static android.app.WindowConfiguration.ACTIVITY_TYPE_STANDARD; import static android.app.WindowConfiguration.WINDOWING_MODE_FULLSCREEN; import static android.app.WindowConfiguration.WINDOWING_MODE_PINNED; @@ -27,7 +28,12 @@ import static com.android.server.wm.ActivityStackSupervisor.ON_TOP; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.reset; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; import android.platform.test.annotations.Presubmit; @@ -120,6 +126,39 @@ public class ActivityDisplayTests extends ActivityTestsBase { assertTrue(stack2.isFocusedStackOnDisplay()); } + /** + * Verifies {@link ActivityDisplay#remove} should not resume home stack on the removing display. + */ + @Test + public void testNotResumeHomeStackOnRemovingDisplay() { + // Create a display which supports system decoration and allows reparenting stacks to + // another display when the display is removed. + final ActivityDisplay display = spy(createNewActivityDisplay()); + doReturn(false).when(display).shouldDestroyContentOnRemove(); + doReturn(true).when(display).supportsSystemDecorations(); + mSupervisor.addChild(display, ActivityDisplay.POSITION_TOP); + + // Put home stack on the display. + final ActivityStack homeStack = display.createStack( + WINDOWING_MODE_FULLSCREEN, ACTIVITY_TYPE_HOME, ON_TOP); + final TaskRecord task = new TaskBuilder(mSupervisor).setStack(homeStack).build(); + new ActivityBuilder(mService).setTask(task).build(); + display.removeChild(homeStack); + final ActivityStack spiedHomeStack = spy(homeStack); + display.addChild(spiedHomeStack, ActivityDisplay.POSITION_TOP); + reset(spiedHomeStack); + + // Put a finishing standard activity which will be reparented. + final ActivityStack stack = createFullscreenStackWithSimpleActivityAt(display); + stack.topRunningActivityLocked().makeFinishingLocked(); + + display.remove(); + + // The removed display should have no focused stack and its home stack should never resume. + assertNull(display.getFocusedStack()); + verify(spiedHomeStack, never()).resumeTopActivityUncheckedLocked(any(), any()); + } + private ActivityStack createFullscreenStackWithSimpleActivityAt(ActivityDisplay display) { final ActivityStack fullscreenStack = display.createStack( WINDOWING_MODE_FULLSCREEN, ACTIVITY_TYPE_STANDARD, ON_TOP);