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

Commit 0b5729fe authored by Annie Lin's avatar Annie Lin
Browse files

[Divider] Fix flicker when dragging to dismiss secondary container. 

Previously, the surface decor was removed too early in
expandTaskFragment() when there are containers to finish. It should instead be done in TYPE_TASK_FRAGMENT_VANISHED after the task fragment is gone.

Before: https://screencast.googleplex.com/cast/NTgyOTIyNTkyNjg4NTM3NnxmY2YxNDA3MC0yYQ
After: https://screencast.googleplex.com/cast/NTU4MTIyMTQwMzgyMDAzMnw4ZjUwYzkyYi1lOA
Bug: 343509560
Flag: com.android.window.flags.activity_embedding_interactive_divider_flag
Test: atest DividerPresenterTest SplitControllerTest
Change-Id: If507a09791b5b29724625c6b2bc24c492f4b3495
parent f25716a4

libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/DividerPresenter.java

+25 −1
Original line number Diff line number Diff line
@@ -169,6 +169,11 @@ class DividerPresenter implements View.OnTouchListener { 
    @GuardedBy("mLock")

    private int mDividerPosition;



    /** Indicates if there are containers to be finished since the divider has appeared. */

    @GuardedBy("mLock")

    @VisibleForTesting

    private boolean mHasContainersToFinish = false;



    DividerPresenter(int taskId, @NonNull DragEventCallback dragEventCallback,

            @NonNull Executor callbackExecutor) {

        mTaskId = taskId;
@@ -180,7 +185,8 @@ class DividerPresenter implements View.OnTouchListener { 
    void updateDivider(

            @NonNull WindowContainerTransaction wct,

            @NonNull TaskFragmentParentInfo parentInfo,

            @Nullable SplitContainer topSplitContainer) {

            @Nullable SplitContainer topSplitContainer,

            boolean isTaskFragmentVanished) {

        if (!Flags.activityEmbeddingInteractiveDividerFlag()) {

            return;

        }
@@ -188,6 +194,18 @@ class DividerPresenter implements View.OnTouchListener { 
        synchronized (mLock) {

            // Clean up the decor surface if top SplitContainer is null.

            if (topSplitContainer == null) {

                // Check if there are containers to finish but the TaskFragment hasn't vanished yet.

                // Don't remove the decor surface and divider if so as the removal should happen in

                // a following step when the TaskFragment has vanished. This ensures that the decor

                // surface is removed only after the resulting Activity is ready to be shown,

                // otherwise there may be flicker.

                if (mHasContainersToFinish) {

                    if (isTaskFragmentVanished) {

                        setHasContainersToFinish(false);

                    } else {

                        return;

                    }

                }

                removeDecorSurfaceAndDivider(wct);

                return;

            }
@@ -868,6 +886,12 @@ class DividerPresenter implements View.OnTouchListener { 
        }

    }



    void setHasContainersToFinish(boolean hasContainersToFinish) {

        synchronized (mLock) {

            mHasContainersToFinish = hasContainersToFinish;

        }

    }



    private static boolean isDraggingToFullscreenAllowed(

            @NonNull DividerAttributes dividerAttributes) {

        // TODO(b/293654166) Use DividerAttributes.isDraggingToFullscreenAllowed when extension is

libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitController.java

+18 −7
Original line number Diff line number Diff line
@@ -673,7 +673,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen 
                        break;

                    case TYPE_TASK_FRAGMENT_VANISHED:

                        mPresenter.removeTaskFragmentInfo(info);

                        onTaskFragmentVanished(wct, info);

                        onTaskFragmentVanished(wct, info, taskId);

                        break;

                    case TYPE_TASK_FRAGMENT_PARENT_INFO_CHANGED:

                        onTaskFragmentParentInfoChanged(wct, taskId,
@@ -834,7 +834,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen 
    @VisibleForTesting

    @GuardedBy("mLock")

    void onTaskFragmentVanished(@NonNull WindowContainerTransaction wct,

            @NonNull TaskFragmentInfo taskFragmentInfo) {

            @NonNull TaskFragmentInfo taskFragmentInfo, int taskId) {

        final TaskFragmentContainer container = getContainer(taskFragmentInfo.getFragmentToken());

        if (container != null) {

            // Cleanup if the TaskFragment vanished is not requested by the organizer.
@@ -843,6 +843,11 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen 
            updateContainersInTaskIfVisible(wct, container.getTaskId());

        }

        cleanupTaskFragment(taskFragmentInfo.getFragmentToken());

        final TaskContainer taskContainer = getTaskContainer(taskId);

        if (taskContainer != null) {

            // Update the divider to clean up any decor surfaces.

            updateDivider(wct, taskContainer, true /* isTaskFragmentVanished */);

        }

    }



    /**
@@ -884,7 +889,7 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen 
        // The divider need to be updated even if shouldUpdateContainer is false, because the decor

        // surface may change in TaskFragmentParentInfo, which requires divider update but not

        // container update.

        updateDivider(wct, taskContainer);

        updateDivider(wct, taskContainer, false /* isTaskFragmentVanished */);



        // If the last direct activity of the host task is dismissed and there's an always-on-top

        // overlay container in the task, the overlay container should also be dismissed.
@@ -3257,12 +3262,15 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen 
    }



    @GuardedBy("mLock")

    void updateDivider(

            @NonNull WindowContainerTransaction wct, @NonNull TaskContainer taskContainer) {

    void updateDivider(@NonNull WindowContainerTransaction wct,

            @NonNull TaskContainer taskContainer, boolean isTaskFragmentVanished) {

        final DividerPresenter dividerPresenter = mDividerPresenters.get(taskContainer.getTaskId());

        final TaskFragmentParentInfo parentInfo = taskContainer.getTaskFragmentParentInfo();

        final SplitContainer topSplitContainer = taskContainer.getTopNonFinishingSplitContainer();

        if (dividerPresenter != null) {

            dividerPresenter.updateDivider(

                wct, parentInfo, taskContainer.getTopNonFinishingSplitContainer());

                    wct, parentInfo, topSplitContainer, isTaskFragmentVanished);

        }

    }



    @Override
@@ -3292,6 +3300,9 @@ public class SplitController implements JetpackTaskFragmentOrganizer.TaskFragmen 
                final List<TaskFragmentContainer> containersToFinish = new ArrayList<>();

                taskContainer.updateTopSplitContainerForDivider(

                        dividerPresenter, containersToFinish);

                if (!containersToFinish.isEmpty()) {

                    dividerPresenter.setHasContainersToFinish(true);

                }

                for (final TaskFragmentContainer container : containersToFinish) {

                    mPresenter.cleanupContainer(wct, container, false /* shouldFinishDependent */);

                }

libs/WindowManager/Jetpack/src/androidx/window/extensions/embedding/SplitPresenter.java

+3 −2
Original line number Diff line number Diff line
@@ -374,7 +374,7 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer { 
        updateTaskFragmentWindowingModeIfRegistered(wct, secondaryContainer, windowingMode);

        updateAnimationParams(wct, primaryContainer.getTaskFragmentToken(), splitAttributes);

        updateAnimationParams(wct, secondaryContainer.getTaskFragmentToken(), splitAttributes);

        mController.updateDivider(wct, taskContainer);

        mController.updateDivider(wct, taskContainer, false /* isTaskFragmentVanished */);

    }



    private void setAdjacentTaskFragments(@NonNull WindowContainerTransaction wct,
@@ -757,7 +757,8 @@ class SplitPresenter extends JetpackTaskFragmentOrganizer { 
    void expandTaskFragment(@NonNull WindowContainerTransaction wct,

            @NonNull TaskFragmentContainer container) {

        super.expandTaskFragment(wct, container);

        mController.updateDivider(wct, container.getTaskContainer());

        mController.updateDivider(

                wct, container.getTaskContainer(), false /* isTaskFragmentVanished */);

    }



    static boolean shouldShowSplit(@NonNull SplitContainer splitContainer) {

libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/DividerPresenterTest.java

+45 −5
Original line number Diff line number Diff line
@@ -82,6 +82,7 @@ import java.util.concurrent.Executor; 
 */

@Presubmit

@SmallTest

@SuppressWarnings("GuardedBy")

@RunWith(AndroidJUnit4.class)

public class DividerPresenterTest {

    @Rule
@@ -186,7 +187,8 @@ public class DividerPresenterTest { 
        mDividerPresenter.updateDivider(

                mTransaction,

                mParentInfo,

                mSplitContainer);

                mSplitContainer,

                false /* isTaskFragmentVanished */);



        assertNotEquals(mProperties, mDividerPresenter.mProperties);

        verify(mRenderer).update();
@@ -206,7 +208,8 @@ public class DividerPresenterTest { 
        mDividerPresenter.updateDivider(

                mTransaction,

                mParentInfo,

                mSplitContainer);

                mSplitContainer,

                false /* isTaskFragmentVanished */);



        assertNotEquals(mProperties, mDividerPresenter.mProperties);

        verify(mRenderer).update();
@@ -222,7 +225,8 @@ public class DividerPresenterTest { 
        mDividerPresenter.updateDivider(

                mTransaction,

                mParentInfo,

                mSplitContainer);

                mSplitContainer,

                false /* isTaskFragmentVanished */);



        assertEquals(mProperties, mDividerPresenter.mProperties);

        verify(mRenderer, never()).update();
@@ -234,7 +238,42 @@ public class DividerPresenterTest { 
        mDividerPresenter.updateDivider(

                mTransaction,

                mParentInfo,

                null /* splitContainer */);

                null /* splitContainer */,

                false /* isTaskFragmentVanished */);

        final TaskFragmentOperation taskFragmentOperation = new TaskFragmentOperation.Builder(

                OP_TYPE_REMOVE_TASK_FRAGMENT_DECOR_SURFACE)

                .build();



        verify(mTransaction).addTaskFragmentOperation(

                mPrimaryContainerToken, taskFragmentOperation);

        verify(mRenderer).release();

        assertNull(mDividerPresenter.mRenderer);

        assertNull(mDividerPresenter.mProperties);

        assertNull(mDividerPresenter.mDecorSurfaceOwner);

    }



    @Test

    public void testUpdateDivider_noChangeWhenHasContainersToFinishButTaskFragmentNotVanished() {

        mDividerPresenter.setHasContainersToFinish(true);

        mDividerPresenter.updateDivider(

                mTransaction,

                mParentInfo,

                null /* splitContainer */,

                false /* isTaskFragmentVanished */);



        assertEquals(mProperties, mDividerPresenter.mProperties);

        verify(mRenderer, never()).update();

        verify(mTransaction, never()).addTaskFragmentOperation(any(), any());

    }



    @Test

    public void testUpdateDivider_dividerRemovedWhenHasContainersToFinishAndTaskFragmentVanished() {

        mDividerPresenter.setHasContainersToFinish(true);

        mDividerPresenter.updateDivider(

                mTransaction,

                mParentInfo,

                null /* splitContainer */,

                true /* isTaskFragmentVanished */);

        final TaskFragmentOperation taskFragmentOperation = new TaskFragmentOperation.Builder(

                OP_TYPE_REMOVE_TASK_FRAGMENT_DECOR_SURFACE)

                .build();
@@ -254,7 +293,8 @@ public class DividerPresenterTest { 
        mDividerPresenter.updateDivider(

                mTransaction,

                mParentInfo,

                mSplitContainer);

                mSplitContainer,

                false /* isTaskFragmentVanished */);

        final TaskFragmentOperation taskFragmentOperation = new TaskFragmentOperation.Builder(

                OP_TYPE_REMOVE_TASK_FRAGMENT_DECOR_SURFACE)

                .build();

libs/WindowManager/Jetpack/tests/unittest/src/androidx/window/extensions/embedding/SplitControllerTest.java

+4 −2
Original line number Diff line number Diff line
@@ -200,12 +200,14 @@ public class SplitControllerTest { 
    public void testOnTaskFragmentVanished() {

        final TaskFragmentContainer tf = createTfContainer(mSplitController, mActivity);

        doReturn(tf.getTaskFragmentToken()).when(mInfo).getFragmentToken();

        doReturn(createTestTaskContainer()).when(mSplitController).getTaskContainer(TASK_ID);



        // The TaskFragment has been removed in the server, we only need to cleanup the reference.

        mSplitController.onTaskFragmentVanished(mTransaction, mInfo);

        mSplitController.onTaskFragmentVanished(mTransaction, mInfo, TASK_ID);



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

        verify(mSplitController).removeContainer(tf);

        verify(mSplitController).updateDivider(any(), any(), anyBoolean());

        verify(mTransaction, never()).finishActivity(any());

    }
@@ -1152,7 +1154,7 @@ public class SplitControllerTest { 
                .setTaskFragmentInfo(info));

        mSplitController.onTransactionReady(transaction);



        verify(mSplitController).onTaskFragmentVanished(any(), eq(info));

        verify(mSplitController).onTaskFragmentVanished(any(), eq(info), anyInt());

        verify(mSplitPresenter).onTransactionHandled(eq(transaction.getTransactionToken()), any(),

                anyInt(), anyBoolean());

    }