From d72d7bf73583013fdf582b51af3901a58dfe4fe3 Mon Sep 17 00:00:00 2001 From: Mady Mellor Date: Thu, 27 Jul 2023 16:46:52 -0700 Subject: Release TaskView after the task has been removed in Bubbles When the bubble expanded view gets cleaned up, we could still be animating task view / the task removal, so instead of releasing TaskView there (which clears the task from TaskViewTransitions which caused issues with animating the removal), we release the TaskView in the callback we get for the task removal. Additionally fix up the code in TaskViewTaskController where onTaskRemovalStarted is called so that if TaskView#release is called from onTaskRemoval, then the taskLeash, taskToken, taskInfo won't be null. Test: atest TaskViewTest Test: manual - expand a android messages bubble - tap on the manage menu > dismiss bubble => observe that the animation is normal and the surface isn't left on screen (repeat test a couple of times, isn't a 100% repro). Bug: 293399673 Change-Id: I3b29a7c358f7fe9f79aa99bdd00a8e3ec35cfd69 --- .../wm/shell/bubbles/BubbleExpandedView.java | 21 ++++++---- .../wm/shell/taskview/TaskViewTaskController.java | 49 ++++++++++------------ .../android/wm/shell/taskview/TaskViewTest.java | 25 +++++++++++ 3 files changed, 58 insertions(+), 37 deletions(-) diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleExpandedView.java b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleExpandedView.java index 67185655f2d2..e6986012dd88 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleExpandedView.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/bubbles/BubbleExpandedView.java @@ -312,9 +312,13 @@ public class BubbleExpandedView extends LinearLayout { + " bubble=" + getBubbleKey()); } if (mBubble != null) { - // Must post because this is called from a binder thread. - post(() -> mController.removeBubble( - mBubble.getKey(), Bubbles.DISMISS_TASK_FINISHED)); + mController.removeBubble(mBubble.getKey(), Bubbles.DISMISS_TASK_FINISHED); + } + if (mTaskView != null) { + // Release the surface + mTaskView.release(); + removeView(mTaskView); + mTaskView = null; } } @@ -1058,8 +1062,10 @@ public class BubbleExpandedView extends LinearLayout { } /** - * Cleans up anything related to the task and {@code TaskView}. If this view should be reused - * after this method is called, then + * Cleans up anything related to the task. The TaskView itself is released after the task + * has been removed. + * + * If this view should be reused after this method is called, then * {@link #initialize(BubbleController, BubbleStackView, boolean)} must be invoked first. */ public void cleanUpExpandedState() { @@ -1081,10 +1087,7 @@ public class BubbleExpandedView extends LinearLayout { } } if (mTaskView != null) { - // Release the surface & other task view related things - mTaskView.release(); - removeView(mTaskView); - mTaskView = null; + mTaskView.setVisibility(GONE); } } diff --git a/libs/WindowManager/Shell/src/com/android/wm/shell/taskview/TaskViewTaskController.java b/libs/WindowManager/Shell/src/com/android/wm/shell/taskview/TaskViewTaskController.java index 064af04cbc4e..a743e99d6954 100644 --- a/libs/WindowManager/Shell/src/com/android/wm/shell/taskview/TaskViewTaskController.java +++ b/libs/WindowManager/Shell/src/com/android/wm/shell/taskview/TaskViewTaskController.java @@ -317,18 +317,12 @@ public class TaskViewTaskController implements ShellTaskOrganizer.TaskListener { // we know about -- so leave clean-up here even if shell transitions are enabled. if (mTaskToken == null || !mTaskToken.equals(taskInfo.token)) return; - if (mListener != null) { - final int taskId = taskInfo.taskId; - mListenerExecutor.execute(() -> { - mListener.onTaskRemovalStarted(taskId); - }); - } - mTaskOrganizer.setInterceptBackPressedOnTaskRoot(mTaskToken, false); + final SurfaceControl taskLeash = mTaskLeash; + handleAndNotifyTaskRemoval(mTaskInfo); // Unparent the task when this surface is destroyed - mTransaction.reparent(mTaskLeash, null).apply(); + mTransaction.reparent(taskLeash, null).apply(); resetTaskInfo(); - mTaskViewBase.onTaskVanished(taskInfo); } @Override @@ -498,6 +492,20 @@ public class TaskViewTaskController implements ShellTaskOrganizer.TaskListener { } } + /** Notifies listeners of a task being removed and stops intercepting back presses on it. */ + private void handleAndNotifyTaskRemoval(ActivityManager.RunningTaskInfo taskInfo) { + if (taskInfo != null) { + if (mListener != null) { + final int taskId = taskInfo.taskId; + mListenerExecutor.execute(() -> { + mListener.onTaskRemovalStarted(taskId); + }); + } + mTaskViewBase.onTaskVanished(taskInfo); + mTaskOrganizer.setInterceptBackPressedOnTaskRoot(taskInfo.token, false); + } + } + /** Returns the task info for the task in the TaskView. */ @Nullable public ActivityManager.RunningTaskInfo getTaskInfo() { @@ -523,18 +531,12 @@ public class TaskViewTaskController implements ShellTaskOrganizer.TaskListener { */ void cleanUpPendingTask() { if (mPendingInfo != null) { - if (mListener != null) { - final int taskId = mPendingInfo.taskId; - mListenerExecutor.execute(() -> { - mListener.onTaskRemovalStarted(taskId); - }); - } - mTaskViewBase.onTaskVanished(mPendingInfo); - mTaskOrganizer.setInterceptBackPressedOnTaskRoot(mPendingInfo.token, false); + final ActivityManager.RunningTaskInfo pendingInfo = mPendingInfo; + handleAndNotifyTaskRemoval(pendingInfo); // Make sure the task is removed WindowContainerTransaction wct = new WindowContainerTransaction(); - wct.removeTask(mPendingInfo.token); + wct.removeTask(pendingInfo.token); mTaskViewTransitions.closeTaskView(wct, this); } resetTaskInfo(); @@ -559,16 +561,7 @@ public class TaskViewTaskController implements ShellTaskOrganizer.TaskListener { * is used instead. */ void prepareCloseAnimation() { - if (mTaskToken != null) { - if (mListener != null) { - final int taskId = mTaskInfo.taskId; - mListenerExecutor.execute(() -> { - mListener.onTaskRemovalStarted(taskId); - }); - } - mTaskViewBase.onTaskVanished(mTaskInfo); - mTaskOrganizer.setInterceptBackPressedOnTaskRoot(mTaskToken, false); - } + handleAndNotifyTaskRemoval(mTaskInfo); resetTaskInfo(); } diff --git a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/taskview/TaskViewTest.java b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/taskview/TaskViewTest.java index 50435a02a44d..d098d332a376 100644 --- a/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/taskview/TaskViewTest.java +++ b/libs/WindowManager/Shell/tests/unittest/src/com/android/wm/shell/taskview/TaskViewTest.java @@ -607,4 +607,29 @@ public class TaskViewTest extends ShellTestCase { verify(mTaskViewTaskController).applyCaptionInsetsIfNeeded(); verify(mOrganizer).applyTransaction(any()); } + + @Test + public void testReleaseInOnTaskRemoval_noNPE() { + mTaskViewTaskController = spy(new TaskViewTaskController(mContext, mOrganizer, + mTaskViewTransitions, mSyncQueue)); + mTaskView = new TaskView(mContext, mTaskViewTaskController); + mTaskView.setListener(mExecutor, new TaskView.Listener() { + @Override + public void onTaskRemovalStarted(int taskId) { + mTaskView.release(); + } + }); + + WindowContainerTransaction wct = new WindowContainerTransaction(); + mTaskViewTaskController.prepareOpenAnimation(true /* newTask */, + new SurfaceControl.Transaction(), new SurfaceControl.Transaction(), mTaskInfo, + mLeash, wct); + mTaskView.surfaceCreated(mock(SurfaceHolder.class)); + + assertThat(mTaskViewTaskController.getTaskInfo()).isEqualTo(mTaskInfo); + + mTaskViewTaskController.prepareCloseAnimation(); + + assertThat(mTaskViewTaskController.getTaskInfo()).isNull(); + } } -- cgit v1.2.3-59-g8ed1b