From 1c8fcaa8b32a87a43c7051510c0205eeb7fec1c5 Mon Sep 17 00:00:00 2001 From: Charles Chen Date: Thu, 23 Nov 2023 17:02:00 +0800 Subject: Early return for task-start-position-only change This CL makes containers in a task don't update if the task is just moved but not resized. An example is drag-and drop a freeform task. Containers don't need to update because we use relative bounds in WCT operations. Relative bounds won't be changed when a task is moved. Note that we still need to check if the overlay container should be dismissed in case the overlay container is the last child window container of the parent task. Test: atest OverlayPresentationTest Test: atest WMJetpackUnitTests Test: atest PinActivityStackTests Bug: 243518738 Change-Id: Iaa6a48d8145caea42cf984b02cd4a7aafbfe96be --- .../extensions/embedding/SplitController.java | 35 +++++++++++++++++----- .../window/extensions/embedding/TaskContainer.java | 21 ++++++++++++- .../embedding/TaskFragmentContainer.java | 3 ++ .../extensions/embedding/TransactionManager.java | 14 ++++++++- .../embedding/OverlayPresentationTest.java | 23 ++++++++++++++ 5 files changed, 87 insertions(+), 9 deletions(-) diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java index 65597de44255..066f38b61eec 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java @@ -819,14 +819,20 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen Log.e(TAG, "onTaskFragmentParentInfoChanged on empty Task id=" + taskId); return; } + // Checks if container should be updated before apply new parentInfo. + final boolean shouldUpdateContainer = taskContainer.shouldUpdateContainer(parentInfo); taskContainer.updateTaskFragmentParentInfo(parentInfo); if (!taskContainer.isVisible()) { // Don't update containers if the task is not visible. We only update containers when // parentInfo#isVisibleRequested is true. return; } - if (isInPictureInPicture(parentInfo.getConfiguration())) { - // No need to update presentation in PIP until the Task exit PIP. + + // If the last direct activity of the host task is dismissed and the overlay container is + // the only taskFragment, the overlay container should also be dismissed. + dismissOverlayContainerIfNeeded(wct, taskContainer); + + if (!shouldUpdateContainer) { return; } updateContainersInTask(wct, taskContainer); @@ -1947,11 +1953,8 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen void updateOverlayContainer(@NonNull WindowContainerTransaction wct, @NonNull TaskFragmentContainer container) { final TaskContainer taskContainer = container.getTaskContainer(); - // Dismiss the overlay container if it's the only container in the task and there's no - // direct activity in the parent task. - if (taskContainer.getTaskFragmentContainers().size() == 1 - && !taskContainer.hasDirectActivity()) { - container.finish(false /* shouldFinishDependent */, mPresenter, wct, this); + + if (dismissOverlayContainerIfNeeded(wct, taskContainer)) { return; } @@ -1968,6 +1971,24 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen } } + /** Dismisses the overlay container in the {@code taskContainer} if needed. */ + @GuardedBy("mLock") + private boolean dismissOverlayContainerIfNeeded(@NonNull WindowContainerTransaction wct, + @NonNull TaskContainer taskContainer) { + final TaskFragmentContainer overlayContainer = taskContainer.getOverlayContainer(); + if (overlayContainer == null) { + return false; + } + // Dismiss the overlay container if it's the only container in the task and there's no + // direct activity in the parent task. + if (taskContainer.getTaskFragmentContainers().size() == 1 + && !taskContainer.hasDirectActivity()) { + mPresenter.cleanupContainer(wct, overlayContainer, false /* shouldFinishDependant */); + return true; + } + return false; + } + /** * Updates {@link SplitContainer} with the given {@link SplitAttributes} if the * {@link SplitContainer} is the top most and not finished. If passed {@link SplitAttributes} diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskContainer.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskContainer.java index 64ad4faa421d..71195b6df97e 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskContainer.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskContainer.java @@ -137,6 +137,21 @@ class TaskContainer { mHasDirectActivity = info.hasDirectActivity(); } + /** + * Returns {@code true} if the container should be updated with {@code info}. + */ + boolean shouldUpdateContainer(@NonNull TaskFragmentParentInfo info) { + final Configuration configuration = info.getConfiguration(); + + return info.isVisible() + // No need to update presentation in PIP until the Task exit PIP. + && !isInPictureInPicture(configuration) + // If the task properties equals regardless of starting position, don't need to + // update the container. + && (mConfiguration.diffPublicOnly(configuration) != 0 + || mDisplayId != info.getDisplayId()); + } + /** * Returns the windowing mode for the TaskFragments below this Task, which should be split with * other TaskFragments. @@ -161,7 +176,11 @@ class TaskContainer { } boolean isInPictureInPicture() { - return getWindowingMode() == WINDOWING_MODE_PINNED; + return isInPictureInPicture(mConfiguration); + } + + private static boolean isInPictureInPicture(@NonNull Configuration configuration) { + return configuration.windowConfiguration.getWindowingMode() == WINDOWING_MODE_PINNED; } boolean isInMultiWindow() { diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java index 810bded8a7f0..9a3f275e2ea4 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java @@ -611,6 +611,9 @@ class TaskFragmentContainer { * Removes all activities that belong to this process and finishes other containers/activities * configured to finish together. */ + // Suppress GuardedBy warning because lint ask to mark this method as + // @GuardedBy(container.mController.mLock), which is mLock itself + @SuppressWarnings("GuardedBy") @GuardedBy("mController.mLock") void finish(boolean shouldFinishDependent, @NonNull SplitPresenter presenter, @NonNull WindowContainerTransaction wct, @NonNull SplitController controller) { diff --git a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TransactionManager.java b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TransactionManager.java index 396956e56bb5..6624c703f027 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TransactionManager.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TransactionManager.java @@ -77,9 +77,11 @@ class TransactionManager { @NonNull TransactionRecord startNewTransaction(@Nullable IBinder taskFragmentTransactionToken) { if (mCurrentTransaction != null) { + final TransactionRecord lastTransaction = mCurrentTransaction; mCurrentTransaction = null; throw new IllegalStateException( - "The previous transaction has not been applied or aborted,"); + "The previous transaction:" + lastTransaction + " has not been applied or " + + "aborted."); } mCurrentTransaction = new TransactionRecord(taskFragmentTransactionToken); return mCurrentTransaction; @@ -199,5 +201,15 @@ class TransactionManager { ? mOriginType : TASK_FRAGMENT_TRANSIT_CHANGE; } + + @Override + @NonNull + public String toString() { + return TransactionRecord.class.getSimpleName() + "{" + + "token=" + mTaskFragmentTransactionToken + + ", type=" + getTransactionTransitionType() + + ", transaction=" + mTransaction + + "}"; + } } } diff --git a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/OverlayPresentationTest.java b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/OverlayPresentationTest.java index 5ef6a5263f96..bc921010b469 100644 --- a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/OverlayPresentationTest.java +++ b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/OverlayPresentationTest.java @@ -472,6 +472,29 @@ public class OverlayPresentationTest { verify(mSplitPresenter).applyActivityStackAttributes(any(), eq(container), eq(attrs)); } + @Test + public void testOnTaskFragmentParentInfoChanged_positionOnlyChange_earlyReturn() { + final TaskFragmentContainer overlayContainer = createTestOverlayContainer(TASK_ID, "test"); + + final TaskContainer taskContainer = overlayContainer.getTaskContainer(); + spyOn(taskContainer); + final TaskContainer.TaskProperties taskProperties = taskContainer.getTaskProperties(); + final TaskFragmentParentInfo parentInfo = new TaskFragmentParentInfo( + new Configuration(taskProperties.getConfiguration()), taskProperties.getDisplayId(), + true /* visible */, false /* hasDirectActivity */, null /* decorSurface */); + parentInfo.getConfiguration().windowConfiguration.getBounds().offset(10, 10); + + mSplitController.onTaskFragmentParentInfoChanged(mTransaction, TASK_ID, parentInfo); + + // The parent info must be applied to the task container + verify(taskContainer).updateTaskFragmentParentInfo(parentInfo); + verify(mSplitController, never()).updateContainer(any(), any()); + + assertWithMessage("The overlay container must still be dismissed even if " + + "#updateContainer is not called") + .that(taskContainer.getOverlayContainer()).isNull(); + } + /** * A simplified version of {@link SplitController.ActivityStartMonitor * #createOrUpdateOverlayTaskFragmentIfNeeded} -- cgit v1.2.3-59-g8ed1b