From 624aa970dfb1a641703521dab084a82bd9d6b4f5 Mon Sep 17 00:00:00 2001 From: Maryam Dehaini Date: Tue, 21 Feb 2023 13:48:07 -0800 Subject: [PATCH] Resizing to invalid bounds causes task to jump to fullscreen When resizing the task to a negative width/height, the task becomes fullscreen. This change stops shell from sending invalid bound changes (changes where width/height is less than minimum task width/minimum task height) to core. Bug: 269660668 Test: Flashing device and manual testing Change-Id: Ie0f133be03d3201cc4f6b266bd7d27af624571e4 --- .../CaptionWindowDecorViewModel.java | 2 +- .../DesktopModeWindowDecorViewModel.java | 3 +- .../wm/shell/windowdecor/TaskPositioner.java | 40 ++- .../shell/windowdecor/TaskPositionerTest.kt | 247 ++++++++++++++++++ 4 files changed, 287 insertions(+), 5 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/CaptionWindowDecorViewModel.java b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/CaptionWindowDecorViewModel.java index 2981f5e160f6f..9224b3cbd798a 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/CaptionWindowDecorViewModel.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/CaptionWindowDecorViewModel.java @@ -174,7 +174,7 @@ public class CaptionWindowDecorViewModel implements WindowDecorViewModel { mWindowDecorByTaskId.put(taskInfo.taskId, windowDecoration); final TaskPositioner taskPositioner = - new TaskPositioner(mTaskOrganizer, windowDecoration); + new TaskPositioner(mTaskOrganizer, windowDecoration, mDisplayController); final CaptionTouchEventListener touchEventListener = new CaptionTouchEventListener(taskInfo, taskPositioner); windowDecoration.setCaptionListeners(touchEventListener, touchEventListener); diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/DesktopModeWindowDecorViewModel.java b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/DesktopModeWindowDecorViewModel.java index db6cbdc5176d3..1124ce61e736a 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/DesktopModeWindowDecorViewModel.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/DesktopModeWindowDecorViewModel.java @@ -539,7 +539,8 @@ public class DesktopModeWindowDecorViewModel implements WindowDecorViewModel { mWindowDecorByTaskId.put(taskInfo.taskId, windowDecoration); final TaskPositioner taskPositioner = - new TaskPositioner(mTaskOrganizer, windowDecoration, mDragStartListener); + new TaskPositioner(mTaskOrganizer, windowDecoration, mDisplayController, + mDragStartListener); final DesktopModeTouchEventListener touchEventListener = new DesktopModeTouchEventListener(taskInfo, taskPositioner); windowDecoration.setCaptionListeners(touchEventListener, touchEventListener); diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/TaskPositioner.java b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/TaskPositioner.java index d3f92277bf039..a3d364a0068e9 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/TaskPositioner.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/windowdecor/TaskPositioner.java @@ -19,9 +19,11 @@ package com.android.wm.shell.windowdecor; import android.annotation.IntDef; import android.graphics.PointF; import android.graphics.Rect; +import android.util.DisplayMetrics; import android.window.WindowContainerTransaction; import com.android.wm.shell.ShellTaskOrganizer; +import com.android.wm.shell.common.DisplayController; class TaskPositioner implements DragPositioningCallback { @@ -35,6 +37,7 @@ class TaskPositioner implements DragPositioningCallback { static final int CTRL_TYPE_BOTTOM = 8; private final ShellTaskOrganizer mTaskOrganizer; + private final DisplayController mDisplayController; private final WindowDecoration mWindowDecoration; private final Rect mTaskBoundsAtDragStart = new Rect(); @@ -45,14 +48,16 @@ class TaskPositioner implements DragPositioningCallback { private int mCtrlType; private DragStartListener mDragStartListener; - TaskPositioner(ShellTaskOrganizer taskOrganizer, WindowDecoration windowDecoration) { - this(taskOrganizer, windowDecoration, dragStartListener -> {}); + TaskPositioner(ShellTaskOrganizer taskOrganizer, WindowDecoration windowDecoration, + DisplayController displayController) { + this(taskOrganizer, windowDecoration, displayController, dragStartListener -> {}); } TaskPositioner(ShellTaskOrganizer taskOrganizer, WindowDecoration windowDecoration, - DragStartListener dragStartListener) { + DisplayController displayController, DragStartListener dragStartListener) { mTaskOrganizer = taskOrganizer; mWindowDecoration = windowDecoration; + mDisplayController = displayController; mDragStartListener = dragStartListener; } @@ -128,15 +133,44 @@ class TaskPositioner implements DragPositioningCallback { mRepositionTaskBounds.offset((int) deltaX, (int) deltaY); } + // If width or height are negative or less than the minimum width or height, revert the + // respective bounds to use previous bound dimensions. + if (mRepositionTaskBounds.width() < getMinWidth()) { + mRepositionTaskBounds.right = oldRight; + mRepositionTaskBounds.left = oldLeft; + } + if (mRepositionTaskBounds.height() < getMinHeight()) { + mRepositionTaskBounds.top = oldTop; + mRepositionTaskBounds.bottom = oldBottom; + } + // If there are no changes to the bounds after checking new bounds against minimum width + // and height, do not set bounds and return false if (oldLeft == mRepositionTaskBounds.left && oldTop == mRepositionTaskBounds.top && oldRight == mRepositionTaskBounds.right && oldBottom == mRepositionTaskBounds.bottom) { return false; } + wct.setBounds(mWindowDecoration.mTaskInfo.token, mRepositionTaskBounds); return true; } + private float getMinWidth() { + return mWindowDecoration.mTaskInfo.minWidth < 0 ? getDefaultMinSize() + : mWindowDecoration.mTaskInfo.minWidth; + } + + private float getMinHeight() { + return mWindowDecoration.mTaskInfo.minHeight < 0 ? getDefaultMinSize() + : mWindowDecoration.mTaskInfo.minHeight; + } + + private float getDefaultMinSize() { + float density = mDisplayController.getDisplayLayout(mWindowDecoration.mTaskInfo.displayId) + .densityDpi() * DisplayMetrics.DENSITY_DEFAULT_SCALE; + return mWindowDecoration.mTaskInfo.defaultMinSize * density; + } + interface DragStartListener { /** * Inform the implementing class that a drag resize has started diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/TaskPositionerTest.kt b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/TaskPositionerTest.kt index f185a8a807191..8f66f4e7e47ba 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/TaskPositionerTest.kt +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/windowdecor/TaskPositionerTest.kt @@ -8,6 +8,8 @@ import android.testing.AndroidTestingRunner import android.window.WindowContainerToken import android.window.WindowContainerTransaction.Change.CHANGE_DRAG_RESIZING import androidx.test.filters.SmallTest +import com.android.wm.shell.common.DisplayController +import com.android.wm.shell.common.DisplayLayout import com.android.wm.shell.ShellTaskOrganizer import com.android.wm.shell.ShellTestCase import com.android.wm.shell.windowdecor.TaskPositioner.CTRL_TYPE_RIGHT @@ -45,6 +47,11 @@ class TaskPositionerTest : ShellTestCase() { @Mock private lateinit var taskBinder: IBinder + @Mock + private lateinit var mockDisplayController: DisplayController + @Mock + private lateinit var mockDisplayLayout: DisplayLayout + private lateinit var taskPositioner: TaskPositioner @Before @@ -54,12 +61,21 @@ class TaskPositionerTest : ShellTestCase() { taskPositioner = TaskPositioner( mockShellTaskOrganizer, mockWindowDecoration, + mockDisplayController, mockDragStartListener ) + `when`(taskToken.asBinder()).thenReturn(taskBinder) + `when`(mockDisplayController.getDisplayLayout(DISPLAY_ID)).thenReturn(mockDisplayLayout) + `when`(mockDisplayLayout.densityDpi()).thenReturn(DENSITY_DPI) + mockWindowDecoration.mTaskInfo = ActivityManager.RunningTaskInfo().apply { taskId = TASK_ID token = taskToken + minWidth = MIN_WIDTH + minHeight = MIN_HEIGHT + defaultMinSize = DEFAULT_MIN + displayId = DISPLAY_ID configuration.windowConfiguration.bounds = STARTING_BOUNDS } } @@ -209,8 +225,239 @@ class TaskPositionerTest : ShellTestCase() { }) } + @Test + fun testDragResize_resize_setBoundsDoesNotChangeHeightWhenLessThanMin() { + taskPositioner.onDragPositioningStart( + CTRL_TYPE_RIGHT or CTRL_TYPE_TOP, // Resize right and top + STARTING_BOUNDS.right.toFloat(), + STARTING_BOUNDS.top.toFloat() + ) + + // Resize to width of 95px and height of 5px with min width of 10px + val newX = STARTING_BOUNDS.right.toFloat() - 5 + val newY = STARTING_BOUNDS.top.toFloat() + 95 + taskPositioner.onDragPositioningMove( + newX, + newY + ) + + taskPositioner.onDragPositioningEnd(newX, newY) + + verify(mockShellTaskOrganizer).applyTransaction(argThat { wct -> + return@argThat wct.changes.any { (token, change) -> + token == taskBinder && + ((change.windowSetMask and WindowConfiguration.WINDOW_CONFIG_BOUNDS) + != 0) && change.configuration.windowConfiguration.bounds.top == + STARTING_BOUNDS.top && + change.configuration.windowConfiguration.bounds.bottom == + STARTING_BOUNDS.bottom + } + }) + } + + @Test + fun testDragResize_resize_setBoundsDoesNotChangeWidthWhenLessThanMin() { + taskPositioner.onDragPositioningStart( + CTRL_TYPE_RIGHT or CTRL_TYPE_TOP, // Resize right and top + STARTING_BOUNDS.right.toFloat(), + STARTING_BOUNDS.top.toFloat() + ) + + // Resize to height of 95px and width of 5px with min width of 10px + val newX = STARTING_BOUNDS.right.toFloat() - 95 + val newY = STARTING_BOUNDS.top.toFloat() + 5 + taskPositioner.onDragPositioningMove( + newX, + newY + ) + + taskPositioner.onDragPositioningEnd(newX, newY) + + verify(mockShellTaskOrganizer).applyTransaction(argThat { wct -> + return@argThat wct.changes.any { (token, change) -> + token == taskBinder && + ((change.windowSetMask and WindowConfiguration.WINDOW_CONFIG_BOUNDS) + != 0) && change.configuration.windowConfiguration.bounds.right == + STARTING_BOUNDS.right && + change.configuration.windowConfiguration.bounds.left == + STARTING_BOUNDS.left + } + }) + } + + @Test + fun testDragResize_resize_setBoundsDoesNotChangeHeightWhenNegative() { + taskPositioner.onDragPositioningStart( + CTRL_TYPE_RIGHT or CTRL_TYPE_TOP, // Resize right and top + STARTING_BOUNDS.right.toFloat(), + STARTING_BOUNDS.top.toFloat() + ) + + // Resize to height of -5px and width of 95px + val newX = STARTING_BOUNDS.right.toFloat() - 5 + val newY = STARTING_BOUNDS.top.toFloat() + 105 + taskPositioner.onDragPositioningMove( + newX, + newY + ) + + taskPositioner.onDragPositioningEnd(newX, newY) + + verify(mockShellTaskOrganizer).applyTransaction(argThat { wct -> + return@argThat wct.changes.any { (token, change) -> + token == taskBinder && + ((change.windowSetMask and WindowConfiguration.WINDOW_CONFIG_BOUNDS) + != 0) && change.configuration.windowConfiguration.bounds.top == + STARTING_BOUNDS.top && + change.configuration.windowConfiguration.bounds.bottom == + STARTING_BOUNDS.bottom + } + }) + } + + @Test + fun testDragResize_resize_setBoundsDoesNotChangeWidthWhenNegative() { + taskPositioner.onDragPositioningStart( + CTRL_TYPE_RIGHT or CTRL_TYPE_TOP, // Resize right and top + STARTING_BOUNDS.right.toFloat(), + STARTING_BOUNDS.top.toFloat() + ) + + // Resize to width of -5px and height of 95px + val newX = STARTING_BOUNDS.right.toFloat() - 105 + val newY = STARTING_BOUNDS.top.toFloat() + 5 + taskPositioner.onDragPositioningMove( + newX, + newY + ) + + taskPositioner.onDragPositioningEnd(newX, newY) + + verify(mockShellTaskOrganizer).applyTransaction(argThat { wct -> + return@argThat wct.changes.any { (token, change) -> + token == taskBinder && + ((change.windowSetMask and WindowConfiguration.WINDOW_CONFIG_BOUNDS) + != 0) && change.configuration.windowConfiguration.bounds.right == + STARTING_BOUNDS.right && + change.configuration.windowConfiguration.bounds.left == + STARTING_BOUNDS.left + } + }) + } + + @Test + fun testDragResize_resize_setBoundsRunsWhenResizeBoundsValid() { + taskPositioner.onDragPositioningStart( + CTRL_TYPE_RIGHT or CTRL_TYPE_TOP, // Resize right and top + STARTING_BOUNDS.right.toFloat(), + STARTING_BOUNDS.top.toFloat() + ) + + // Shrink to height 20px and width 20px with both min height/width equal to 10px + val newX = STARTING_BOUNDS.right.toFloat() - 80 + val newY = STARTING_BOUNDS.top.toFloat() + 80 + taskPositioner.onDragPositioningMove( + newX, + newY + ) + + taskPositioner.onDragPositioningEnd(newX, newY) + + verify(mockShellTaskOrganizer).applyTransaction(argThat { wct -> + return@argThat wct.changes.any { (token, change) -> + token == taskBinder && + ((change.windowSetMask and WindowConfiguration.WINDOW_CONFIG_BOUNDS) != 0) + } + }) + } + + @Test + fun testDragResize_resize_setBoundsDoesNotRunWithNegativeHeightAndWidth() { + taskPositioner.onDragPositioningStart( + CTRL_TYPE_RIGHT or CTRL_TYPE_TOP, // Resize right and top + STARTING_BOUNDS.right.toFloat(), + STARTING_BOUNDS.top.toFloat() + ) + + // Shrink to height 5px and width 5px with both min height/width equal to 10px + val newX = STARTING_BOUNDS.right.toFloat() - 95 + val newY = STARTING_BOUNDS.top.toFloat() + 95 + taskPositioner.onDragPositioningMove( + newX, + newY + ) + + taskPositioner.onDragPositioningEnd(newX, newY) + + verify(mockShellTaskOrganizer, never()).applyTransaction(argThat { wct -> + return@argThat wct.changes.any { (token, change) -> + token == taskBinder && + ((change.windowSetMask and WindowConfiguration.WINDOW_CONFIG_BOUNDS) != 0) + } + }) + } + + @Test + fun testDragResize_resize_useDefaultMinWhenMinWidthInvalid() { + mockWindowDecoration.mTaskInfo.minWidth = -1 + + taskPositioner.onDragPositioningStart( + CTRL_TYPE_RIGHT or CTRL_TYPE_TOP, // Resize right and top + STARTING_BOUNDS.right.toFloat(), + STARTING_BOUNDS.top.toFloat() + ) + + // Shrink to width and height of 3px with invalid minWidth = -1 and defaultMinSize = 5px + val newX = STARTING_BOUNDS.right.toFloat() - 97 + val newY = STARTING_BOUNDS.top.toFloat() + 97 + taskPositioner.onDragPositioningMove( + newX, + newY + ) + + taskPositioner.onDragPositioningEnd(newX, newY) + + verify(mockShellTaskOrganizer, never()).applyTransaction(argThat { wct -> + return@argThat wct.changes.any { (token, change) -> + token == taskBinder && + ((change.windowSetMask and WindowConfiguration.WINDOW_CONFIG_BOUNDS) != 0) + } + }) + } + + @Test + fun testDragResize_resize_useMinWidthWhenValid() { + taskPositioner.onDragPositioningStart( + CTRL_TYPE_RIGHT or CTRL_TYPE_TOP, // Resize right and top + STARTING_BOUNDS.right.toFloat(), + STARTING_BOUNDS.top.toFloat() + ) + + // Shrink to width and height of 7px with valid minWidth = 10px and defaultMinSize = 5px + val newX = STARTING_BOUNDS.right.toFloat() - 93 + val newY = STARTING_BOUNDS.top.toFloat() + 93 + taskPositioner.onDragPositioningMove( + newX, + newY + ) + + taskPositioner.onDragPositioningEnd(newX, newY) + + verify(mockShellTaskOrganizer, never()).applyTransaction(argThat { wct -> + return@argThat wct.changes.any { (token, change) -> + token == taskBinder && + ((change.windowSetMask and WindowConfiguration.WINDOW_CONFIG_BOUNDS) != 0) + } + }) + } + companion object { private const val TASK_ID = 5 + private const val MIN_WIDTH = 10 + private const val MIN_HEIGHT = 10 + private const val DENSITY_DPI = 20 + private const val DEFAULT_MIN = 40 + private const val DISPLAY_ID = 1 private val STARTING_BOUNDS = Rect(0, 0, 100, 100) } }