Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3e93b262 authored by Charles Chen's avatar Charles Chen
Browse files

Update SplitContainer for non-finishing containers

If an app is split with an Activity from another process
by Activity Embedding feature, and is launched twice
with different orientations, there's a chance that
the app still shows in previous orientation at the second
launch even if the orientation is changed.

The root cause is that, the app goes back to background
before the secondary container receives its TaskFragmentInfo.

Also, there's no pendingActivities and #onActivityPostCreated
won't be invoked, and also #updateContainer to change its
layout, because the Activity is from other process.

Moreover, in #onTaskFragmentParentInfoChanged, updateContainer
is skipped because secondary container is empty(No pending activities,
and hasn't received its TaskFragmentInfo).

This CL fixes the issue by verifying if one or both containers are
finished instead.

fixes: 228906392
Test: atest SplitControllerTest
Test: manual - reproducible steps in bug

Change-Id: Ifba4b56c9225089776835aeeef7c4a13249b55bf
parent 12873579
Loading
Loading
Loading
Loading
+7 −7
Original line number Diff line number Diff line
@@ -619,14 +619,13 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
        }
        final List<SplitContainer> splitContainers = mTaskContainers.get(container.getTaskId())
                .mSplitContainers;
        if (splitContainers == null
                || splitContainer != splitContainers.get(splitContainers.size() - 1)) {
        if (splitContainer != splitContainers.get(splitContainers.size() - 1)) {
            // Skip position update - it isn't the topmost split.
            return;
        }
        if (splitContainer.getPrimaryContainer().isEmpty()
                || splitContainer.getSecondaryContainer().isEmpty()) {
            // Skip position update - one or both containers are empty.
        if (splitContainer.getPrimaryContainer().isFinished()
                || splitContainer.getSecondaryContainer().isFinished()) {
            // Skip position update - one or both containers are finished.
            return;
        }
        if (dismissPlaceholderIfNecessary(splitContainer)) {
@@ -643,7 +642,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
    private SplitContainer getActiveSplitForContainer(@NonNull TaskFragmentContainer container) {
        final List<SplitContainer> splitContainers = mTaskContainers.get(container.getTaskId())
                .mSplitContainers;
        if (splitContainers == null) {
        if (splitContainers.isEmpty()) {
            return null;
        }
        for (int i = splitContainers.size() - 1; i >= 0; i--) {
@@ -721,7 +720,8 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen
        return true;
    }

    private boolean dismissPlaceholderIfNecessary(@NonNull SplitContainer splitContainer) {
    @VisibleForTesting
    boolean dismissPlaceholderIfNecessary(@NonNull SplitContainer splitContainer) {
        if (!splitContainer.isPlaceholderContainer()) {
            return false;
        }
+2 −0
Original line number Diff line number Diff line
@@ -47,9 +47,11 @@ class TaskContainer {
    private int mWindowingMode = WINDOWING_MODE_UNDEFINED;

    /** Active TaskFragments in this Task. */
    @NonNull
    final List<TaskFragmentContainer> mContainers = new ArrayList<>();

    /** Active split pairs in this Task. */
    @NonNull
    final List<SplitContainer> mSplitContainers = new ArrayList<>();

    /**
+4 −3
Original line number Diff line number Diff line
@@ -319,14 +319,15 @@ class TaskFragmentContainer {
    private String toString(boolean includeContainersToFinishOnExit) {
        return "TaskFragmentContainer{"
                + " token=" + mToken
                + " info=" + mInfo
                + " topNonFinishingActivity=" + getTopNonFinishingActivity()
                + " runningActivityCount=" + getRunningActivityCount()
                + " isFinished=" + mIsFinished
                + " lastRequestedBounds=" + mLastRequestedBounds
                + " pendingAppearedActivities=" + mPendingAppearedActivities
                + (includeContainersToFinishOnExit ? " containersToFinishOnExit="
                + containersToFinishOnExitToString() : "")
                + " activitiesToFinishOnExit=" + mActivitiesToFinishOnExit
                + " isFinished=" + mIsFinished
                + " lastRequestedBounds=" + mLastRequestedBounds
                + " info=" + mInfo
                + "}";
    }

+78 −0
Original line number Diff line number Diff line
@@ -25,6 +25,8 @@ import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertThrows;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
@@ -35,6 +37,7 @@ import android.content.res.Resources;
import android.graphics.Rect;
import android.platform.test.annotations.Presubmit;
import android.window.TaskFragmentInfo;
import android.window.WindowContainerTransaction;

import androidx.test.ext.junit.runners.AndroidJUnit4;
import androidx.test.filters.SmallTest;
@@ -45,6 +48,8 @@ import org.junit.runner.RunWith;
import org.mockito.Mock;
import org.mockito.MockitoAnnotations;

import java.util.List;

/**
 * Test class for {@link SplitController}.
 *
@@ -64,6 +69,8 @@ public class SplitControllerTest {
    private Resources mActivityResources;
    @Mock
    private TaskFragmentInfo mInfo;
    @Mock
    private WindowContainerTransaction mTransaction;
    private SplitController mSplitController;
    private SplitPresenter mSplitPresenter;

@@ -140,4 +147,75 @@ public class SplitControllerTest {
        assertNotNull(taskContainer);
        assertEquals(TASK_BOUNDS, taskContainer.getTaskBounds());
    }

    @Test
    public void testUpdateContainer() {
        // Make SplitController#launchPlaceholderIfNecessary(TaskFragmentContainer) return true
        // and verify if shouldContainerBeExpanded() not called.
        final TaskFragmentContainer tf = mSplitController.newContainer(mActivity, TASK_ID);
        spyOn(tf);
        doReturn(mActivity).when(tf).getTopNonFinishingActivity();
        doReturn(true).when(tf).isEmpty();
        doReturn(true).when(mSplitController).launchPlaceholderIfNecessary(mActivity);
        doNothing().when(mSplitPresenter).updateSplitContainer(any(), any(), any());

        mSplitController.updateContainer(mTransaction, tf);

        verify(mSplitController, never()).shouldContainerBeExpanded(any());

        // Verify if tf should be expanded, getTopActiveContainer() won't be called
        doReturn(null).when(tf).getTopNonFinishingActivity();
        doReturn(true).when(mSplitController).shouldContainerBeExpanded(tf);

        mSplitController.updateContainer(mTransaction, tf);

        verify(mSplitController, never()).getTopActiveContainer(TASK_ID);

        // Verify if tf is not in split, dismissPlaceholderIfNecessary won't be called.
        doReturn(false).when(mSplitController).shouldContainerBeExpanded(tf);

        mSplitController.updateContainer(mTransaction, tf);

        verify(mSplitController, never()).dismissPlaceholderIfNecessary(any());

        // Verify if tf is not in the top splitContainer,
        final SplitContainer splitContainer = mock(SplitContainer.class);
        doReturn(tf).when(splitContainer).getPrimaryContainer();
        doReturn(tf).when(splitContainer).getSecondaryContainer();
        final List<SplitContainer> splitContainers =
                mSplitController.getTaskContainer(TASK_ID).mSplitContainers;
        splitContainers.add(splitContainer);
        // Add a mock SplitContainer on top of splitContainer
        splitContainers.add(1, mock(SplitContainer.class));

        mSplitController.updateContainer(mTransaction, tf);

        verify(mSplitController, never()).dismissPlaceholderIfNecessary(any());

        // Verify if one or both containers in the top SplitContainer are finished,
        // dismissPlaceholder() won't be called.
        splitContainers.remove(1);
        doReturn(true).when(tf).isFinished();

        mSplitController.updateContainer(mTransaction, tf);

        verify(mSplitController, never()).dismissPlaceholderIfNecessary(any());

        // Verify if placeholder should be dismissed, updateSplitContainer() won't be called.
        doReturn(false).when(tf).isFinished();
        doReturn(true).when(mSplitController)
                .dismissPlaceholderIfNecessary(splitContainer);

        mSplitController.updateContainer(mTransaction, tf);

        verify(mSplitPresenter, never()).updateSplitContainer(any(), any(), any());

        // Verify if the top active split is updated if both of its containers are not finished.
        doReturn(false).when(mSplitController)
                        .dismissPlaceholderIfNecessary(splitContainer);

        mSplitController.updateContainer(mTransaction, tf);

        verify(mSplitPresenter).updateSplitContainer(eq(splitContainer), eq(tf), eq(mTransaction));
    }
}