From 7021370af78e185beb489cf0fba0ee19bd372c4f Mon Sep 17 00:00:00 2001 From: Evan Rosky Date: Tue, 5 Nov 2019 10:26:24 -0800 Subject: [PATCH] Remove some hard-coded fullscreen-specific hierarchy logic There were a couple issues where evaluation was hard-coded: 1. detecting window movement was always relative to the display. Because animations are moving to be more "leashed", this detection method can cause double-animations. 2. The Task-level configuration resolution was hard-coded to assume that null-bounds would inherit from the display. This breaks down when we try to influence parent configs (eg. Stack or, in the future, parent Tasks) and causes the app to receive configs that don't match its window size. For (1) this CL changes WindowState to only animation movement if the windows have moved relative to the root-windowstate's parent. This works because movements above Activity-level are done by WM and have animations already (eg. transitions or task-org leashes) while movements below activity-level are triggered by app (eg. relayout) and don't have intrinsic animations. For (2) this CL adjusts the task-level resolution to always calculate relative to its parent, even with null bounds. Bug: 133381692 Bug: 133381284 Test: Added to TaskRecordTests and updated WindowFrameTests Change-Id: I172ca65465bbfdfe5e2022c5a4bfb6b6f0140b24 --- .../com/android/server/wm/ActivityStack.java | 3 -- .../android/server/wm/DisplayRotation.java | 2 +- .../core/java/com/android/server/wm/Task.java | 45 ++++++++++++------- .../server/wm/TaskLaunchParamsModifier.java | 3 +- .../com/android/server/wm/WindowFrames.java | 10 +++++ .../com/android/server/wm/WindowState.java | 27 +++++++++-- .../android/server/wm/TaskRecordTests.java | 23 +++++++++- .../android/server/wm/WindowFrameTests.java | 16 +++++++ 8 files changed, 102 insertions(+), 27 deletions(-) diff --git a/services/core/java/com/android/server/wm/ActivityStack.java b/services/core/java/com/android/server/wm/ActivityStack.java index 91c909d9eba89..afcde9524d31f 100644 --- a/services/core/java/com/android/server/wm/ActivityStack.java +++ b/services/core/java/com/android/server/wm/ActivityStack.java @@ -421,8 +421,6 @@ class ActivityStack extends WindowContainer implements BoundsAnimationTarg /** Stores the override windowing-mode from before a transient mode change (eg. split) */ private int mRestoreOverrideWindowingMode = WINDOWING_MODE_UNDEFINED; - private final ActivityOptions mTmpOptions = ActivityOptions.makeBasic(); - /** List for processing through a set of activities */ private final ArrayList mTmpActivities = new ArrayList<>(); @@ -749,7 +747,6 @@ class ActivityStack extends WindowContainer implements BoundsAnimationTarg // Leaving a transient mode. Interpret UNDEFINED as "restore" windowingMode = mRestoreOverrideWindowingMode; } - mTmpOptions.setLaunchWindowingMode(windowingMode); // Need to make sure windowing mode is supported. If we in the process of creating the stack // no need to resolve the windowing mode again as it is already resolved to the right mode. diff --git a/services/core/java/com/android/server/wm/DisplayRotation.java b/services/core/java/com/android/server/wm/DisplayRotation.java index 0c68084151c7a..c76d03cf27cb3 100644 --- a/services/core/java/com/android/server/wm/DisplayRotation.java +++ b/services/core/java/com/android/server/wm/DisplayRotation.java @@ -541,8 +541,8 @@ public class DisplayRotation { } mService.mH.removeCallbacks(mDisplayRotationHandlerTimeout); mIsWaitingForRemoteRotation = false; - mService.mAtmService.applyContainerTransaction(t); mDisplayContent.sendNewConfiguration(); + mService.mAtmService.applyContainerTransaction(t); } } diff --git a/services/core/java/com/android/server/wm/Task.java b/services/core/java/com/android/server/wm/Task.java index b229a1d0a8e7a..8992be29efed6 100644 --- a/services/core/java/com/android/server/wm/Task.java +++ b/services/core/java/com/android/server/wm/Task.java @@ -60,9 +60,7 @@ import static com.android.server.EventLogTags.WM_TASK_CREATED; import static com.android.server.EventLogTags.WM_TASK_REMOVED; import static com.android.server.am.TaskRecordProto.ACTIVITIES; import static com.android.server.am.TaskRecordProto.ACTIVITY_TYPE; -import static com.android.server.am.TaskRecordProto.BOUNDS; import static com.android.server.am.TaskRecordProto.FULLSCREEN; -import static com.android.server.am.TaskRecordProto.ID; import static com.android.server.am.TaskRecordProto.LAST_NON_FULLSCREEN_BOUNDS; import static com.android.server.am.TaskRecordProto.MIN_HEIGHT; import static com.android.server.am.TaskRecordProto.MIN_WIDTH; @@ -89,10 +87,8 @@ import static com.android.server.wm.ActivityTaskManagerDebugConfig.TAG_WITH_CLAS import static com.android.server.wm.DragResizeMode.DRAG_RESIZE_MODE_DOCKED_DIVIDER; import static com.android.server.wm.ProtoLogGroup.WM_DEBUG_ADD_REMOVE; import static com.android.server.wm.TaskProto.APP_WINDOW_TOKENS; -import static com.android.server.wm.TaskProto.BOUNDS; import static com.android.server.wm.TaskProto.DISPLAYED_BOUNDS; import static com.android.server.wm.TaskProto.FILLS_PARENT; -import static com.android.server.wm.TaskProto.ID; import static com.android.server.wm.TaskProto.SURFACE_HEIGHT; import static com.android.server.wm.TaskProto.SURFACE_WIDTH; import static com.android.server.wm.TaskProto.WINDOW_CONTAINER; @@ -314,6 +310,7 @@ class Task extends WindowContainer implements ConfigurationConta private final Rect mTmpNonDecorBounds = new Rect(); private final Rect mTmpBounds = new Rect(); private final Rect mTmpInsets = new Rect(); + private final Rect mTmpFullBounds = new Rect(); // Last non-fullscreen bounds the task was launched in or resized to. // The information is persisted and used to determine the appropriate stack to launch the @@ -1673,7 +1670,9 @@ class Task extends WindowContainer implements ConfigurationConta } void adjustForMinimalTaskDimensions(Rect bounds, Rect previousBounds) { - if (bounds == null) { + final Rect parentBounds = getParent() != null ? getParent().getBounds() : null; + if (bounds == null + || (bounds.isEmpty() && (parentBounds == null || parentBounds.isEmpty()))) { return; } int minWidth = mMinWidth; @@ -1697,6 +1696,14 @@ class Task extends WindowContainer implements ConfigurationConta minHeight = defaultMinSize; } } + if (bounds.isEmpty()) { + // If inheriting parent bounds, check if parent bounds adhere to minimum size. If they + // do, we can just skip. + if (parentBounds.width() >= minWidth && parentBounds.height() >= minHeight) { + return; + } + bounds.set(parentBounds); + } final boolean adjustWidth = minWidth > bounds.width(); final boolean adjustHeight = minHeight > bounds.height(); if (!(adjustWidth || adjustHeight)) { @@ -1948,10 +1955,19 @@ class Task extends WindowContainer implements ConfigurationConta } density *= DisplayMetrics.DENSITY_DEFAULT_SCALE; - final Rect bounds = inOutConfig.windowConfiguration.getBounds(); + final Rect resolvedBounds = inOutConfig.windowConfiguration.getBounds(); + if (resolvedBounds == null) { + mTmpFullBounds.setEmpty(); + } else { + mTmpFullBounds.set(resolvedBounds); + } + if (mTmpFullBounds.isEmpty()) { + mTmpFullBounds.set(parentConfig.windowConfiguration.getBounds()); + } + Rect outAppBounds = inOutConfig.windowConfiguration.getAppBounds(); if (outAppBounds == null || outAppBounds.isEmpty()) { - inOutConfig.windowConfiguration.setAppBounds(bounds); + inOutConfig.windowConfiguration.setAppBounds(mTmpFullBounds); outAppBounds = inOutConfig.windowConfiguration.getAppBounds(); } // Non-null compatibility insets means the activity prefers to keep its original size, so @@ -1974,7 +1990,7 @@ class Task extends WindowContainer implements ConfigurationConta // area, i.e. the screen area without the system bars. // The non decor inset are areas that could never be removed in Honeycomb. See // {@link WindowManagerPolicy#getNonDecorInsetsLw}. - calculateInsetFrames(mTmpNonDecorBounds, mTmpStableBounds, bounds, di); + calculateInsetFrames(mTmpNonDecorBounds, mTmpStableBounds, mTmpFullBounds, di); } else { // Apply the given non-decor and stable insets to calculate the corresponding bounds // for screen size of configuration. @@ -1983,8 +1999,8 @@ class Task extends WindowContainer implements ConfigurationConta rotation = parentConfig.windowConfiguration.getRotation(); } if (rotation != ROTATION_UNDEFINED && compatInsets != null) { - mTmpNonDecorBounds.set(bounds); - mTmpStableBounds.set(bounds); + mTmpNonDecorBounds.set(mTmpFullBounds); + mTmpStableBounds.set(mTmpFullBounds); compatInsets.getDisplayBoundsByRotation(mTmpBounds, rotation); intersectWithInsetsIfFits(mTmpNonDecorBounds, mTmpBounds, compatInsets.mNonDecorInsets[rotation]); @@ -2016,13 +2032,13 @@ class Task extends WindowContainer implements ConfigurationConta if (WindowConfiguration.isFloating(windowingMode)) { // For floating tasks, calculate the smallest width from the bounds of the task inOutConfig.smallestScreenWidthDp = (int) ( - Math.min(bounds.width(), bounds.height()) / density); + Math.min(mTmpFullBounds.width(), mTmpFullBounds.height()) / density); } else if (WindowConfiguration.isSplitScreenWindowingMode(windowingMode)) { // Iterating across all screen orientations, and return the minimum of the task // width taking into account that the bounds might change because the snap // algorithm snaps to a different value inOutConfig.smallestScreenWidthDp = - getSmallestScreenWidthDpForDockedBounds(bounds); + getSmallestScreenWidthDpForDockedBounds(mTmpFullBounds); } // otherwise, it will just inherit } @@ -2067,11 +2083,6 @@ class Task extends WindowContainer implements ConfigurationConta newParentConfig.orientation); } - if (outOverrideBounds.isEmpty()) { - // If the task fills the parent, just inherit all the other configs from parent. - return; - } - adjustForMinimalTaskDimensions(outOverrideBounds, mTmpBounds); if (windowingMode == WINDOWING_MODE_FREEFORM) { // by policy, make sure the window remains within parent somewhere diff --git a/services/core/java/com/android/server/wm/TaskLaunchParamsModifier.java b/services/core/java/com/android/server/wm/TaskLaunchParamsModifier.java index d7bc072dc69ac..8ad8972937cd2 100644 --- a/services/core/java/com/android/server/wm/TaskLaunchParamsModifier.java +++ b/services/core/java/com/android/server/wm/TaskLaunchParamsModifier.java @@ -284,7 +284,8 @@ class TaskLaunchParamsModifier implements LaunchParamsModifier { && outParams.mBounds.isEmpty() && source.getDisplayId() == display.mDisplayId) { // Set bounds to be not very far from source activity. - cascadeBounds(source.getBounds(), display, outParams.mBounds); + cascadeBounds(source.getConfiguration().windowConfiguration.getBounds(), + display, outParams.mBounds); } getTaskBounds(root, display, layout, resolvedMode, hasInitialBounds, outParams.mBounds); } diff --git a/services/core/java/com/android/server/wm/WindowFrames.java b/services/core/java/com/android/server/wm/WindowFrames.java index 84fcfbd43d320..bafa38c7f4a75 100644 --- a/services/core/java/com/android/server/wm/WindowFrames.java +++ b/services/core/java/com/android/server/wm/WindowFrames.java @@ -106,6 +106,16 @@ public class WindowFrames { */ final Rect mLastFrame = new Rect(); + /** + * mFrame but relative to the parent container. + */ + final Rect mRelFrame = new Rect(); + + /** + * mLastFrame but relative to the parent container + */ + final Rect mLastRelFrame = new Rect(); + private boolean mFrameSizeChanged = false; // Frame that is scaled to the application's coordinate space when in diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java index f98e307488474..db0f3bc139bc7 100644 --- a/services/core/java/com/android/server/wm/WindowState.java +++ b/services/core/java/com/android/server/wm/WindowState.java @@ -212,7 +212,6 @@ import com.android.internal.util.ToBooleanFunction; import com.android.server.policy.WindowManagerPolicy; import com.android.server.protolog.common.ProtoLog; import com.android.server.wm.LocalAnimationAdapter.AnimationSpec; -import com.android.server.wm.utils.InsetUtils; import com.android.server.wm.utils.WmDisplayCutout; import java.io.PrintWriter; @@ -1129,6 +1128,22 @@ class WindowState extends WindowContainer implements WindowManagerP } } + // Calculate relative frame + mWindowFrames.mRelFrame.set(mWindowFrames.mFrame); + WindowContainer parent = getParent(); + int parentLeft = 0; + int parentTop = 0; + if (mIsChildWindow) { + parentLeft = ((WindowState) parent).mWindowFrames.mFrame.left; + parentTop = ((WindowState) parent).mWindowFrames.mFrame.top; + } else if (parent != null) { + final Rect parentBounds = parent.getDisplayedBounds(); + parentLeft = parentBounds.left; + parentTop = parentBounds.top; + } + mWindowFrames.mRelFrame.offsetTo(mWindowFrames.mFrame.left - parentLeft, + mWindowFrames.mFrame.top - parentTop); + if (DEBUG_LAYOUT || DEBUG) { Slog.v(TAG, "Resolving (mRequestedWidth=" + mRequestedWidth + ", mRequestedheight=" @@ -1154,6 +1169,11 @@ class WindowState extends WindowContainer implements WindowManagerP return mWindowFrames.mFrame; } + /** Accessor for testing */ + Rect getRelativeFrameLw() { + return mWindowFrames.mRelFrame; + } + @Override public Rect getDisplayFrameLw() { return mWindowFrames.mDisplayFrame; @@ -1289,6 +1309,7 @@ class WindowState extends WindowContainer implements WindowManagerP // We update mLastFrame always rather than in the conditional with the last inset // variables, because mFrameSizeChanged only tracks the width and height changing. mWindowFrames.mLastFrame.set(mWindowFrames.mFrame); + mWindowFrames.mLastRelFrame.set(mWindowFrames.mRelFrame); if (didFrameInsetsChange || winAnimator.mSurfaceResized @@ -1859,8 +1880,8 @@ class WindowState extends WindowContainer implements WindowManagerP private boolean hasMoved() { return mHasSurface && (mWindowFrames.hasContentChanged() || mMovedByResize) && !mAnimatingExit - && (mWindowFrames.mFrame.top != mWindowFrames.mLastFrame.top - || mWindowFrames.mFrame.left != mWindowFrames.mLastFrame.left) + && (mWindowFrames.mRelFrame.top != mWindowFrames.mLastRelFrame.top + || mWindowFrames.mRelFrame.left != mWindowFrames.mLastRelFrame.left) && (!mIsChildWindow || !getParentWindow().hasMoved()); } diff --git a/services/tests/wmtests/src/com/android/server/wm/TaskRecordTests.java b/services/tests/wmtests/src/com/android/server/wm/TaskRecordTests.java index faa9f11f97bf3..fa433f03726bf 100644 --- a/services/tests/wmtests/src/com/android/server/wm/TaskRecordTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/TaskRecordTests.java @@ -45,6 +45,7 @@ import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.sameInstance; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; @@ -161,12 +162,12 @@ public class TaskRecordTests extends ActivityTestsBase { assertTrue(task.returnsToHomeStack()); } - /** Ensures that empty bounds are not propagated to the configuration. */ + /** Ensures that empty bounds cause appBounds to inherit from parent. */ @Test public void testAppBounds_EmptyBounds() { final Rect emptyBounds = new Rect(); testStackBoundsConfiguration(WINDOWING_MODE_FULLSCREEN, mParentBounds, emptyBounds, - null /*ExpectedBounds*/); + mParentBounds); } /** Ensures that bounds on freeform stacks are not clipped. */ @@ -415,6 +416,24 @@ public class TaskRecordTests extends ActivityTestsBase { assertEquals(Configuration.ORIENTATION_LANDSCAPE, inOutConfig.orientation); } + @Test + public void testComputeNestedConfigResourceOverrides() { + final Task task = new TaskBuilder(mSupervisor).build(); + assertTrue(task.getResolvedOverrideBounds().isEmpty()); + int origScreenH = task.getConfiguration().screenHeightDp; + Configuration stackConfig = new Configuration(); + stackConfig.setTo(task.getStack().getRequestedOverrideConfiguration()); + stackConfig.windowConfiguration.setWindowingMode(WINDOWING_MODE_FREEFORM); + + // Set bounds on stack (not task) and verify that the task resource configuration changes + // despite it's override bounds being empty. + Rect bounds = new Rect(task.getStack().getBounds()); + bounds.bottom = (int) (bounds.bottom * 0.6f); + stackConfig.windowConfiguration.setBounds(bounds); + task.getStack().onRequestedOverrideConfigurationChanged(stackConfig); + assertNotEquals(origScreenH, task.getConfiguration().screenHeightDp); + } + /** Ensures that the alias intent won't have target component resolved. */ @Test public void testTaskIntentActivityAlias() { diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowFrameTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowFrameTests.java index 0b8b6a1f23ecf..7be05a39cbde5 100644 --- a/services/tests/wmtests/src/com/android/server/wm/WindowFrameTests.java +++ b/services/tests/wmtests/src/com/android/server/wm/WindowFrameTests.java @@ -96,6 +96,10 @@ public class WindowFrameTests extends WindowTestsBase { assertRect(w.getFrameLw(), left, top, right, bottom); } + private void assertRelFrame(WindowState w, int left, int top, int right, int bottom) { + assertRect(w.getRelativeFrameLw(), left, top, right, bottom); + } + private void assertContentFrame(WindowState w, Rect expectedRect) { assertRect(w.getContentFrameLw(), expectedRect.left, expectedRect.top, expectedRect.right, expectedRect.bottom); @@ -153,6 +157,7 @@ public class WindowFrameTests extends WindowTestsBase { w.getWindowFrames().setFrames(pf, df, cf, vf, dcf, sf); w.computeFrameLw(); assertFrame(w, 0, 0, 1000, 1000); + assertRelFrame(w, 0, 0, 1000, 1000); assertContentInset(w, 0, topContentInset, 0, bottomContentInset); assertVisibleInset(w, 0, topVisibleInset, 0, bottomVisibleInset); assertStableInset(w, leftStableInset, 0, rightStableInset, 0); @@ -167,6 +172,7 @@ public class WindowFrameTests extends WindowTestsBase { w.mRequestedHeight = 100; w.computeFrameLw(); assertFrame(w, 100, 100, 200, 200); + assertRelFrame(w, 100, 100, 200, 200); assertContentInset(w, 0, 0, 0, 0); // In this case the frames are shrunk to the window frame. assertContentFrame(w, w.getFrameLw()); @@ -189,6 +195,7 @@ public class WindowFrameTests extends WindowTestsBase { w.getWindowFrames().setFrames(pf, pf, pf, pf, pf, pf); w.computeFrameLw(); assertFrame(w, 0, 0, 1000, 1000); + assertRelFrame(w, 0, 0, 1000, 1000); // It can select various widths and heights within the bounds. // Strangely the window attribute width is ignored for normal windows @@ -242,15 +249,18 @@ public class WindowFrameTests extends WindowTestsBase { w.mAttrs.gravity = Gravity.RIGHT | Gravity.TOP; w.computeFrameLw(); assertFrame(w, 700, 0, 1000, 300); + assertRelFrame(w, 700, 0, 1000, 300); w.mAttrs.gravity = Gravity.RIGHT | Gravity.BOTTOM; w.computeFrameLw(); assertFrame(w, 700, 700, 1000, 1000); + assertRelFrame(w, 700, 700, 1000, 1000); // Window specified x and y are interpreted as offsets in the opposite // direction of gravity w.mAttrs.x = 100; w.mAttrs.y = 100; w.computeFrameLw(); assertFrame(w, 600, 600, 900, 900); + assertRelFrame(w, 600, 600, 900, 900); } @Test @@ -279,6 +289,8 @@ public class WindowFrameTests extends WindowTestsBase { // For non fullscreen tasks the containing frame is based off the // task bounds not the parent frame. assertEquals(resolvedTaskBounds, w.getFrameLw()); + assertEquals(0, w.getRelativeFrameLw().left); + assertEquals(0, w.getRelativeFrameLw().top); assertContentFrame(w, resolvedTaskBounds); assertContentInset(w, 0, 0, 0, 0); @@ -290,6 +302,8 @@ public class WindowFrameTests extends WindowTestsBase { windowFrames.setFrames(pf, pf, cf, cf, pf, cf); w.computeFrameLw(); assertEquals(resolvedTaskBounds, w.getFrameLw()); + assertEquals(0, w.getRelativeFrameLw().left); + assertEquals(0, w.getRelativeFrameLw().top); int contentInsetRight = resolvedTaskBounds.right - cfRight; int contentInsetBottom = resolvedTaskBounds.bottom - cfBottom; assertContentInset(w, 0, 0, contentInsetRight, contentInsetBottom); @@ -309,6 +323,8 @@ public class WindowFrameTests extends WindowTestsBase { windowFrames.setFrames(pf, pf, cf, cf, pf, cf); w.computeFrameLw(); assertEquals(resolvedTaskBounds, w.getFrameLw()); + assertEquals(0, w.getRelativeFrameLw().left); + assertEquals(0, w.getRelativeFrameLw().top); contentInsetRight = insetRight - cfRight; contentInsetBottom = insetBottom - cfBottom; assertContentInset(w, 0, 0, contentInsetRight, contentInsetBottom);