From c45e1c1ed5cb66aa0ad660b86b7b95aa0b1faf49 Mon Sep 17 00:00:00 2001 From: Chris Li Date: Wed, 27 Apr 2022 23:09:09 +0800 Subject: [PATCH] Don't launch placeholder for bottom TaskFragment when waiting for top 1. When the top TaskFragment just appears, we should treat it as active even if it is empty, because we are expecting an activity to be launched into it soon. Before that happens, we don't want to launch placeholder for the activity behind. 2. When a TaskFragment is appeared as empty, set a timeout to remove it instead of waiting forever in case the activtiy start failed. Fix: 230540531 Bug: 219683160 Test: atest WMJetpackUnitTests:SplitControllerTest Test: atest WMJetpackUnitTests:TaskFragmentContainerTest Change-Id: Ie05e379bf898b67912a849ad84f9fe7708e289c6 --- .../extensions/embedding/SplitController.java | 34 +++++++-- .../embedding/TaskFragmentContainer.java | 44 +++++++++++- .../JetpackTaskFragmentOrganizerTest.java | 10 ++- .../embedding/SplitControllerTest.java | 41 +++++++++-- .../embedding/TaskContainerTest.java | 14 +++- .../embedding/TaskFragmentContainerTest.java | 70 ++++++++++++++++++- 6 files changed, 197 insertions(+), 16 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 8115eafe66683..82f8a131ae2ae 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java @@ -81,9 +81,12 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen @NonNull private Consumer> mEmbeddingCallback; private final List mLastReportedSplitStates = new ArrayList<>(); + private final Handler mHandler; public SplitController() { - mPresenter = new SplitPresenter(new MainThreadExecutor(), this); + final MainThreadExecutor executor = new MainThreadExecutor(); + mHandler = executor.mHandler; + mPresenter = new SplitPresenter(executor, this); ActivityThread activityThread = ActivityThread.currentActivityThread(); // Register a callback to be notified about activities being created. activityThread.getApplication().registerActivityLifecycleCallbacks( @@ -167,11 +170,13 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen // to fullscreen. cleanupForEnterPip(wct, container); mPresenter.cleanupContainer(container, false /* shouldFinishDependent */, wct); - } else { + } else if (taskFragmentInfo.isTaskClearedForReuse()) { // Do not finish the dependents if this TaskFragment was cleared due to launching // activity in the Task. - final boolean shouldFinishDependent = !taskFragmentInfo.isTaskClearedForReuse(); - mPresenter.cleanupContainer(container, shouldFinishDependent, wct); + mPresenter.cleanupContainer(container, false /* shouldFinishDependent */, wct); + } else if (!container.isWaitingActivityAppear()) { + // Do not finish the container before the expected activity appear until timeout. + mPresenter.cleanupContainer(container, true /* shouldFinishDependent */, wct); } } else if (wasInPip && isInPip) { // No update until exit PIP. @@ -417,6 +422,14 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen launchPlaceholderIfNecessary(activity); } + /** + * Called when we have been waiting too long for the TaskFragment to become non-empty after + * creation. + */ + void onTaskFragmentAppearEmptyTimeout(@NonNull TaskFragmentContainer container) { + mPresenter.cleanupContainer(container, false /* shouldFinishDependent */); + } + /** * Returns a container that this activity is registered with. An activity can only belong to one * container, or no container at all. @@ -452,7 +465,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen if (activityInTask == null) { throw new IllegalArgumentException("activityInTask must not be null,"); } - final TaskFragmentContainer container = new TaskFragmentContainer(activity, taskId); + final TaskFragmentContainer container = new TaskFragmentContainer(activity, taskId, this); if (!mTaskContainers.contains(taskId)) { mTaskContainers.put(taskId, new TaskContainer(taskId)); } @@ -590,7 +603,12 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen } for (int i = taskContainer.mContainers.size() - 1; i >= 0; i--) { final TaskFragmentContainer container = taskContainer.mContainers.get(i); - if (!container.isFinished() && container.getRunningActivityCount() > 0) { + if (!container.isFinished() && (container.getRunningActivityCount() > 0 + // We may be waiting for the top TaskFragment to become non-empty after + // creation. In that case, we don't want to treat the TaskFragment below it as + // top active, otherwise it may incorrectly launch placeholder on top of the + // pending TaskFragment. + || container.isWaitingActivityAppear())) { return container; } } @@ -920,6 +938,10 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen return mTaskContainers.get(taskId); } + Handler getHandler() { + return mHandler; + } + /** * Returns {@code true} if an Activity with the provided component name should always be * expanded to occupy full task bounds. Such activity must not be put in a split. 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 03f38ed80fb4b..35981d3af948f 100644 --- a/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java +++ b/libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/TaskFragmentContainer.java @@ -30,6 +30,8 @@ import android.os.IBinder; import android.window.TaskFragmentInfo; import android.window.WindowContainerTransaction; +import com.android.internal.annotations.VisibleForTesting; + import java.util.ArrayList; import java.util.Iterator; import java.util.List; @@ -39,6 +41,11 @@ import java.util.List; * on the server side. */ class TaskFragmentContainer { + private static final int APPEAR_EMPTY_TIMEOUT_MS = 3000; + + @NonNull + private final SplitController mController; + /** * Client-created token that uniquely identifies the task fragment container instance. */ @@ -51,7 +58,8 @@ class TaskFragmentContainer { /** * Server-provided task fragment information. */ - private TaskFragmentInfo mInfo; + @VisibleForTesting + TaskFragmentInfo mInfo; /** * Activities that are being reparented or being started to this container, but haven't been @@ -80,11 +88,21 @@ class TaskFragmentContainer { @WindowingMode private int mLastRequestedWindowingMode = WINDOWING_MODE_UNDEFINED; + /** + * When the TaskFragment has appeared in server, but is empty, we should remove the TaskFragment + * if it is still empty after the timeout. + */ + @VisibleForTesting + @Nullable + Runnable mAppearEmptyTimeout; + /** * Creates a container with an existing activity that will be re-parented to it in a window * container transaction. */ - TaskFragmentContainer(@Nullable Activity activity, int taskId) { + TaskFragmentContainer(@Nullable Activity activity, int taskId, + @NonNull SplitController controller) { + mController = controller; mToken = new Binder("TaskFragmentContainer"); if (taskId == INVALID_TASK_ID) { throw new IllegalArgumentException("Invalid Task id"); @@ -155,12 +173,30 @@ class TaskFragmentContainer { return count; } + /** Whether we are waiting for the TaskFragment to appear and become non-empty. */ + boolean isWaitingActivityAppear() { + return !mIsFinished && (mInfo == null || mAppearEmptyTimeout != null); + } + @Nullable TaskFragmentInfo getInfo() { return mInfo; } void setInfo(@NonNull TaskFragmentInfo info) { + if (!mIsFinished && mInfo == null && info.isEmpty()) { + // onTaskFragmentAppeared with empty info. We will remove the TaskFragment if it is + // still empty after timeout. + mAppearEmptyTimeout = () -> { + mAppearEmptyTimeout = null; + mController.onTaskFragmentAppearEmptyTimeout(this); + }; + mController.getHandler().postDelayed(mAppearEmptyTimeout, APPEAR_EMPTY_TIMEOUT_MS); + } else if (mAppearEmptyTimeout != null && !info.isEmpty()) { + mController.getHandler().removeCallbacks(mAppearEmptyTimeout); + mAppearEmptyTimeout = null; + } + mInfo = info; if (mInfo == null || mPendingAppearedActivities.isEmpty()) { return; @@ -234,6 +270,10 @@ class TaskFragmentContainer { @NonNull WindowContainerTransaction wct, @NonNull SplitController controller) { if (!mIsFinished) { mIsFinished = true; + if (mAppearEmptyTimeout != null) { + mController.getHandler().removeCallbacks(mAppearEmptyTimeout); + mAppearEmptyTimeout = null; + } finishActivities(shouldFinishDependent, presenter, wct, controller); } diff --git a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/JetpackTaskFragmentOrganizerTest.java b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/JetpackTaskFragmentOrganizerTest.java index 1f12c44841597..7aa47ef11cb15 100644 --- a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/JetpackTaskFragmentOrganizerTest.java +++ b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/JetpackTaskFragmentOrganizerTest.java @@ -18,6 +18,7 @@ package androidx.window.extensions.embedding; import static android.app.WindowConfiguration.WINDOWING_MODE_UNDEFINED; +import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn; import static com.android.dx.mockito.inline.extended.ExtendedMockito.spyOn; import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify; @@ -29,6 +30,7 @@ import static org.mockito.Mockito.never; import android.content.res.Configuration; import android.graphics.Point; +import android.os.Handler; import android.platform.test.annotations.Presubmit; import android.window.TaskFragmentInfo; import android.window.WindowContainerToken; @@ -61,6 +63,10 @@ public class JetpackTaskFragmentOrganizerTest { private WindowContainerTransaction mTransaction; @Mock private JetpackTaskFragmentOrganizer.TaskFragmentCallback mCallback; + @Mock + private SplitController mSplitController; + @Mock + private Handler mHandler; private JetpackTaskFragmentOrganizer mOrganizer; @Before @@ -69,6 +75,7 @@ public class JetpackTaskFragmentOrganizerTest { mOrganizer = new JetpackTaskFragmentOrganizer(Runnable::run, mCallback); mOrganizer.registerOrganizer(); spyOn(mOrganizer); + doReturn(mHandler).when(mSplitController).getHandler(); } @Test @@ -106,7 +113,8 @@ public class JetpackTaskFragmentOrganizerTest { @Test public void testExpandTaskFragment() { - final TaskFragmentContainer container = new TaskFragmentContainer(null, TASK_ID); + final TaskFragmentContainer container = new TaskFragmentContainer(null /* activity */, + TASK_ID, mSplitController); final TaskFragmentInfo info = createMockInfo(container); mOrganizer.mFragmentInfos.put(container.getTaskFragmentToken(), info); container.setInfo(info); diff --git a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java index 32c61eaa6b50d..983208c974a1f 100644 --- a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java +++ b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java @@ -35,6 +35,7 @@ import android.app.Activity; import android.content.res.Configuration; import android.content.res.Resources; import android.graphics.Rect; +import android.os.Handler; import android.platform.test.annotations.Presubmit; import android.window.TaskFragmentInfo; import android.window.WindowContainerTransaction; @@ -48,6 +49,7 @@ import org.junit.runner.RunWith; import org.mockito.Mock; import org.mockito.MockitoAnnotations; +import java.util.ArrayList; import java.util.List; /** @@ -71,6 +73,9 @@ public class SplitControllerTest { private TaskFragmentInfo mInfo; @Mock private WindowContainerTransaction mTransaction; + @Mock + private Handler mHandler; + private SplitController mSplitController; private SplitPresenter mSplitPresenter; @@ -86,6 +91,7 @@ public class SplitControllerTest { activityConfig.windowConfiguration.setMaxBounds(TASK_BOUNDS); doReturn(mActivityResources).when(mActivity).getResources(); doReturn(activityConfig).when(mActivityResources).getConfiguration(); + doReturn(mHandler).when(mSplitController).getHandler(); } @Test @@ -94,28 +100,45 @@ public class SplitControllerTest { // tf3 is finished so is not active. TaskFragmentContainer tf3 = mock(TaskFragmentContainer.class); doReturn(true).when(tf3).isFinished(); + doReturn(false).when(tf3).isWaitingActivityAppear(); // tf2 has running activity so is active. TaskFragmentContainer tf2 = mock(TaskFragmentContainer.class); doReturn(1).when(tf2).getRunningActivityCount(); // tf1 has no running activity so is not active. - TaskFragmentContainer tf1 = new TaskFragmentContainer(null, TASK_ID); + TaskFragmentContainer tf1 = new TaskFragmentContainer(null /* activity */, TASK_ID, + mSplitController); - taskContainer.mContainers.add(tf3); - taskContainer.mContainers.add(tf2); taskContainer.mContainers.add(tf1); + taskContainer.mContainers.add(tf2); + taskContainer.mContainers.add(tf3); mSplitController.mTaskContainers.put(TASK_ID, taskContainer); assertWithMessage("Must return tf2 because tf3 is not active.") .that(mSplitController.getTopActiveContainer(TASK_ID)).isEqualTo(tf2); - taskContainer.mContainers.remove(tf1); + taskContainer.mContainers.remove(tf3); assertWithMessage("Must return tf2 because tf2 has running activity.") .that(mSplitController.getTopActiveContainer(TASK_ID)).isEqualTo(tf2); taskContainer.mContainers.remove(tf2); - assertWithMessage("Must return null because tf1 has no running activity.") + assertWithMessage("Must return tf because we are waiting for tf1 to appear.") + .that(mSplitController.getTopActiveContainer(TASK_ID)).isEqualTo(tf1); + + final TaskFragmentInfo info = mock(TaskFragmentInfo.class); + doReturn(new ArrayList<>()).when(info).getActivities(); + doReturn(true).when(info).isEmpty(); + tf1.setInfo(info); + + assertWithMessage("Must return tf because we are waiting for tf1 to become non-empty after" + + " creation.") + .that(mSplitController.getTopActiveContainer(TASK_ID)).isEqualTo(tf1); + + doReturn(false).when(info).isEmpty(); + tf1.setInfo(info); + + assertWithMessage("Must return null because tf1 becomes empty.") .that(mSplitController.getTopActiveContainer(TASK_ID)).isNull(); } @@ -132,6 +155,14 @@ public class SplitControllerTest { verify(mActivity, never()).finish(); } + @Test + public void testOnTaskFragmentAppearEmptyTimeout() { + final TaskFragmentContainer tf = mSplitController.newContainer(mActivity, TASK_ID); + mSplitController.onTaskFragmentAppearEmptyTimeout(tf); + + verify(mSplitPresenter).cleanupContainer(tf, false /* shouldFinishDependent */); + } + @Test public void testNewContainer() { // Must pass in a valid activity. diff --git a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskContainerTest.java b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskContainerTest.java index c7feb7e59de30..c40bab8d9ae34 100644 --- a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskContainerTest.java +++ b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskContainerTest.java @@ -32,8 +32,11 @@ import android.platform.test.annotations.Presubmit; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.SmallTest; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.MockitoAnnotations; /** * Test class for {@link TaskContainer}. @@ -48,6 +51,14 @@ public class TaskContainerTest { private static final int TASK_ID = 10; private static final Rect TASK_BOUNDS = new Rect(0, 0, 600, 1200); + @Mock + private SplitController mController; + + @Before + public void setup() { + MockitoAnnotations.initMocks(this); + } + @Test public void testIsTaskBoundsInitialized() { final TaskContainer taskContainer = new TaskContainer(TASK_ID); @@ -126,7 +137,8 @@ public class TaskContainerTest { assertTrue(taskContainer.isEmpty()); - final TaskFragmentContainer tf = new TaskFragmentContainer(null, TASK_ID); + final TaskFragmentContainer tf = new TaskFragmentContainer(null /* activity */, TASK_ID, + mController); taskContainer.mContainers.add(tf); assertFalse(taskContainer.isEmpty()); diff --git a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskFragmentContainerTest.java b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskFragmentContainerTest.java index 97896c2c0a57c..d80f2b939bf05 100644 --- a/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskFragmentContainerTest.java +++ b/libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/TaskFragmentContainerTest.java @@ -18,12 +18,18 @@ package androidx.window.extensions.embedding; import static com.android.dx.mockito.inline.extended.ExtendedMockito.verify; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import android.app.Activity; +import android.os.Handler; import android.platform.test.annotations.Presubmit; import android.window.TaskFragmentInfo; import android.window.WindowContainerTransaction; @@ -59,15 +65,19 @@ public class TaskFragmentContainerTest { private Activity mActivity; @Mock private TaskFragmentInfo mInfo; + @Mock + private Handler mHandler; @Before public void setup() { MockitoAnnotations.initMocks(this); + doReturn(mHandler).when(mController).getHandler(); } @Test public void testFinish() { - final TaskFragmentContainer container = new TaskFragmentContainer(mActivity, TASK_ID); + final TaskFragmentContainer container = new TaskFragmentContainer(mActivity, TASK_ID, + mController); final WindowContainerTransaction wct = new WindowContainerTransaction(); // Only remove the activity, but not clear the reference until appeared. @@ -94,4 +104,62 @@ public class TaskFragmentContainerTest { verify(mPresenter).deleteTaskFragment(wct, container.getTaskFragmentToken()); verify(mController).removeContainer(container); } + + @Test + public void testIsWaitingActivityAppear() { + final TaskFragmentContainer container = new TaskFragmentContainer(null /* activity */, + TASK_ID, mController); + + assertTrue(container.isWaitingActivityAppear()); + + final TaskFragmentInfo info = mock(TaskFragmentInfo.class); + doReturn(new ArrayList<>()).when(info).getActivities(); + doReturn(true).when(info).isEmpty(); + container.setInfo(info); + + assertTrue(container.isWaitingActivityAppear()); + + doReturn(false).when(info).isEmpty(); + container.setInfo(info); + + assertFalse(container.isWaitingActivityAppear()); + } + + @Test + public void testAppearEmptyTimeout() { + final TaskFragmentContainer container = new TaskFragmentContainer(null /* activity */, + TASK_ID, mController); + + assertNull(container.mAppearEmptyTimeout); + + // Not set if it is not appeared empty. + final TaskFragmentInfo info = mock(TaskFragmentInfo.class); + doReturn(new ArrayList<>()).when(info).getActivities(); + doReturn(false).when(info).isEmpty(); + container.setInfo(info); + + assertNull(container.mAppearEmptyTimeout); + + // Set timeout if the first info set is empty. + container.mInfo = null; + doReturn(true).when(info).isEmpty(); + container.setInfo(info); + + assertNotNull(container.mAppearEmptyTimeout); + + // Remove timeout after the container becomes non-empty. + doReturn(false).when(info).isEmpty(); + container.setInfo(info); + + assertNull(container.mAppearEmptyTimeout); + + // Running the timeout will call into SplitController.onTaskFragmentAppearEmptyTimeout. + container.mInfo = null; + doReturn(true).when(info).isEmpty(); + container.setInfo(info); + container.mAppearEmptyTimeout.run(); + + assertNull(container.mAppearEmptyTimeout); + verify(mController).onTaskFragmentAppearEmptyTimeout(container); + } }