From aea60d2f55dc71ec05c4bd2636e25f2dd7d0bf12 Mon Sep 17 00:00:00 2001 From: Bryce Lee Date: Wed, 31 Jan 2018 14:42:15 -0800 Subject: [PATCH] Do not return stacks to AM that are marked for deferred removal. When the Activity Manager informs the Window Manager that a stack has been removed, it can opt to hold onto the stack (such as when an animation is in progress). This can lead to inconsistencies where the Window Manager can report a stack back to the Activity Manager that has since been removed. This changelist addresses one such call point in WindowManagerService#setNewDisplayOverrideConfiguration. Deferred removed displays are now filtered out of the returned list. Change-Id: I5f7aad9296cec8bd56e933a71553f9cd40579378 Fixes: 71548119 Test: atest FrameworksServicesTests:com.android.server.wm.RootWindowContainerTests --- .../com/android/server/wm/DisplayContent.java | 4 +- .../server/wm/RootWindowContainer.java | 51 ++++++++++++------- .../server/wm/WindowManagerService.java | 4 +- .../server/wm/RootWindowContainerTests.java | 50 ++++++++++++++++++ 4 files changed, 85 insertions(+), 24 deletions(-) create mode 100644 services/tests/servicestests/src/com/android/server/wm/RootWindowContainerTests.java diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java index 7674b5e9ed2c0..651908be1483a 100644 --- a/services/core/java/com/android/server/wm/DisplayContent.java +++ b/services/core/java/com/android/server/wm/DisplayContent.java @@ -1542,11 +1542,11 @@ class DisplayContent extends WindowContainer changedStackList) { + void updateStackBoundsAfterConfigChange(@NonNull List changedStackList) { for (int i = mTaskStackContainers.getChildCount() - 1; i >= 0; --i) { final TaskStack stack = mTaskStackContainers.getChildAt(i); if (stack.updateBoundsAfterConfigChange()) { - changedStackList.add(stack.mStackId); + changedStackList.add(stack); } } diff --git a/services/core/java/com/android/server/wm/RootWindowContainer.java b/services/core/java/com/android/server/wm/RootWindowContainer.java index deed7f17e4e66..768cec140282f 100644 --- a/services/core/java/com/android/server/wm/RootWindowContainer.java +++ b/services/core/java/com/android/server/wm/RootWindowContainer.java @@ -47,6 +47,7 @@ import com.android.server.EventLogTags; import java.io.FileDescriptor; import java.io.PrintWriter; import java.util.ArrayList; +import java.util.List; import java.util.function.Consumer; import static android.app.AppOpsManager.MODE_ALLOWED; @@ -126,7 +127,8 @@ class RootWindowContainer extends WindowContainer { boolean mOrientationChangeComplete = true; boolean mWallpaperActionPending = false; - private final ArrayList mChangedStackList = new ArrayList(); + private final ArrayList mTmpStackList = new ArrayList(); + private final ArrayList mTmpStackIds = new ArrayList<>(); // State for the RemoteSurfaceTrace system used in testing. If this is enabled SurfaceControl // instances will be replaced with an instance that writes a binary representation of all @@ -333,7 +335,8 @@ class RootWindowContainer extends WindowContainer { /** * Set new display override config and return array of ids of stacks that were changed during - * update. If called for the default display, global configuration will also be updated. + * update. If called for the default display, global configuration will also be updated. Stacks + * that are marked for deferred removal are excluded from the returned array. */ int[] setDisplayOverrideConfigurationIfNeeded(Configuration newConfiguration, int displayId) { final DisplayContent displayContent = getDisplayContent(displayId); @@ -346,24 +349,42 @@ class RootWindowContainer extends WindowContainer { if (!configChanged) { return null; } + displayContent.onOverrideConfigurationChanged(newConfiguration); + mTmpStackList.clear(); if (displayId == DEFAULT_DISPLAY) { // Override configuration of the default display duplicates global config. In this case // we also want to update the global config. - return setGlobalConfigurationIfNeeded(newConfiguration); + setGlobalConfigurationIfNeeded(newConfiguration, mTmpStackList); } else { - return updateStackBoundsAfterConfigChange(displayId); + updateStackBoundsAfterConfigChange(displayId, mTmpStackList); } + + mTmpStackIds.clear(); + final int stackCount = mTmpStackList.size(); + + for (int i = 0; i < stackCount; ++i) { + final TaskStack stack = mTmpStackList.get(i); + + // We only include stacks that are not marked for removal as they do not exist outside + // of WindowManager at this point. + if (!stack.mDeferRemoval) { + mTmpStackIds.add(stack.mStackId); + } + } + + return mTmpStackIds.isEmpty() ? null : ArrayUtils.convertToIntArray(mTmpStackIds); } - private int[] setGlobalConfigurationIfNeeded(Configuration newConfiguration) { + private void setGlobalConfigurationIfNeeded(Configuration newConfiguration, + List changedStacks) { final boolean configChanged = getConfiguration().diff(newConfiguration) != 0; if (!configChanged) { - return null; + return; } onConfigurationChanged(newConfiguration); - return updateStackBoundsAfterConfigChange(); + updateStackBoundsAfterConfigChange(changedStacks); } @Override @@ -378,26 +399,18 @@ class RootWindowContainer extends WindowContainer { * Callback used to trigger bounds update after configuration change and get ids of stacks whose * bounds were updated. */ - private int[] updateStackBoundsAfterConfigChange() { - mChangedStackList.clear(); - + private void updateStackBoundsAfterConfigChange(List changedStacks) { final int numDisplays = mChildren.size(); for (int i = 0; i < numDisplays; ++i) { final DisplayContent dc = mChildren.get(i); - dc.updateStackBoundsAfterConfigChange(mChangedStackList); + dc.updateStackBoundsAfterConfigChange(changedStacks); } - - return mChangedStackList.isEmpty() ? null : ArrayUtils.convertToIntArray(mChangedStackList); } /** Same as {@link #updateStackBoundsAfterConfigChange()} but only for a specific display. */ - private int[] updateStackBoundsAfterConfigChange(int displayId) { - mChangedStackList.clear(); - + private void updateStackBoundsAfterConfigChange(int displayId, List changedStacks) { final DisplayContent dc = getDisplayContent(displayId); - dc.updateStackBoundsAfterConfigChange(mChangedStackList); - - return mChangedStackList.isEmpty() ? null : ArrayUtils.convertToIntArray(mChangedStackList); + dc.updateStackBoundsAfterConfigChange(changedStacks); } private void prepareFreezingTaskBounds() { diff --git a/services/core/java/com/android/server/wm/WindowManagerService.java b/services/core/java/com/android/server/wm/WindowManagerService.java index 066e4e6a8c676..d565a6a7a4761 100644 --- a/services/core/java/com/android/server/wm/WindowManagerService.java +++ b/services/core/java/com/android/server/wm/WindowManagerService.java @@ -24,8 +24,6 @@ import static android.Manifest.permission.RESTRICTED_VR_ACCESS; import static android.app.ActivityManager.SPLIT_SCREEN_CREATE_MODE_TOP_OR_LEFT; import static android.app.AppOpsManager.OP_SYSTEM_ALERT_WINDOW; import static android.app.StatusBarManager.DISABLE_MASK; -import static android.app.WindowConfiguration.ACTIVITY_TYPE_HOME; -import static android.app.WindowConfiguration.WINDOWING_MODE_SPLIT_SCREEN_PRIMARY; import static android.app.admin.DevicePolicyManager.ACTION_DEVICE_POLICY_MANAGER_STATE_CHANGED; import static android.content.Intent.ACTION_USER_REMOVED; import static android.content.Intent.EXTRA_USER_HANDLE; @@ -125,7 +123,6 @@ import android.app.ActivityThread; import android.app.AppOpsManager; import android.app.IActivityManager; import android.app.IAssistDataReceiver; -import android.app.WindowConfiguration; import android.content.BroadcastReceiver; import android.content.ContentResolver; import android.content.Context; @@ -2466,6 +2463,7 @@ public class WindowManagerService extends IWindowManager.Stub mWaitingForConfig = false; mLastFinishedFreezeSource = "new-config"; } + return mRoot.setDisplayOverrideConfigurationIfNeeded(overrideConfig, displayId); } } diff --git a/services/tests/servicestests/src/com/android/server/wm/RootWindowContainerTests.java b/services/tests/servicestests/src/com/android/server/wm/RootWindowContainerTests.java new file mode 100644 index 0000000000000..51b019a4d61ea --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/wm/RootWindowContainerTests.java @@ -0,0 +1,50 @@ +package com.android.server.wm; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import android.content.res.Configuration; +import android.graphics.Rect; + +import android.platform.test.annotations.Presubmit; +import android.support.test.filters.SmallTest; +import android.support.test.runner.AndroidJUnit4; + +import static org.junit.Assert.assertTrue; + +/** + * Tests for the {@link RootWindowContainer} class. + * + * Build/Install/Run: + * atest FrameworksServicesTests:com.android.server.wm.RootWindowContainerTests + */ +@SmallTest +@Presubmit +@RunWith(AndroidJUnit4.class) +public class RootWindowContainerTests extends WindowTestsBase { + @Test + public void testSetDisplayOverrideConfigurationIfNeeded() throws Exception { + // Add first stack we expect to be updated with configuration change. + final TaskStack stack = createTaskStackOnDisplay(mDisplayContent); + stack.getOverrideConfiguration().windowConfiguration.setBounds(new Rect(0, 0, 5, 5)); + + // Add second task that will be set for deferred removal that should not be returned + // with the configuration change. + final TaskStack deferredDeletedStack = createTaskStackOnDisplay(mDisplayContent); + deferredDeletedStack.getOverrideConfiguration().windowConfiguration.setBounds( + new Rect(0, 0, 5, 5)); + deferredDeletedStack.mDeferRemoval = true; + + final Configuration override = new Configuration( + mDisplayContent.getOverrideConfiguration()); + override.windowConfiguration.setBounds(new Rect(0, 0, 10, 10)); + + // Set display override. + final int[] results = sWm.mRoot.setDisplayOverrideConfigurationIfNeeded(override, + mDisplayContent.getDisplayId()); + + // Ensure only first stack is returned. + assertTrue(results.length == 1); + assertTrue(results[0] == stack.mStackId); + } +}