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

Commit 71c0761d authored by Jiaming Liu's avatar Jiaming Liu
Browse files

[Divider] Fix divider flicker for cold launch cross-app embedding

The flicker happens because the divider is drawn before the open
transition is completed. When the TaskFragment and the decor surface are
created in the same WCT, we get the callback that the decor surface is
created, and all the other conditions are met to draw the divider. But
we don't have a good signal about whether the transition is completed
and when the secondary container content is drawn. This is especially
significant for cross-app cold launch because it is slow. The flicker
also has a small chance to happen for same-app launch.

This change moves the decor surface visibility operations into post
transition callback to ensure that the decor surface presentation
changes deterministically happen after the transition is completed.

Bug: 343973599
Test: atest TaskFragmentOrganizerControllerTest TaskTests and manual
Change-Id: Id8525d44817f2f530cbeafe458d02ed39a81af0d
parent 4c363457
Loading
Loading
Loading
Loading
+46 −11
Original line number Diff line number Diff line
@@ -3739,7 +3739,13 @@ class Task extends TaskFragment {
        return !isOwnActivity && !isTrustedTaskFragment;
    }

    void setDecorSurfaceBoosted(
    /**
     * Sets the requested boosted state for the decor surface.
     *
     * The caller must call {@link #commitDecorSurfaceBoostedState()} to ensure that the change is
     * applied.
     */
    void requestDecorSurfaceBoosted(
            @NonNull TaskFragment ownerTaskFragment,
            boolean isBoosted,
            @Nullable SurfaceControl.Transaction clientTransaction) {
@@ -3747,9 +3753,17 @@ class Task extends TaskFragment {
                || mDecorSurfaceContainer.mOwnerTaskFragment != ownerTaskFragment) {
            return;
        }
        mDecorSurfaceContainer.setBoosted(isBoosted, clientTransaction);
        // scheduleAnimation() is called inside assignChildLayers(), which ensures that child
        // surface visibility is updated with prepareSurfaces()
        mDecorSurfaceContainer.requestBoosted(isBoosted, clientTransaction);
    }

    void commitDecorSurfaceBoostedState() {
        if (mDecorSurfaceContainer == null) {
            return;
        }
        mDecorSurfaceContainer.commitBoostedState();

        // assignChildLayers() calls scheduleAnimation(), which calls prepareSurfaces()
        // to ensure child surface visibility.
        assignChildLayers();
    }

@@ -6787,11 +6801,11 @@ class Task extends TaskFragment {
     * Associates the decor surface with the given TF, or create one if there
     * isn't one in the Task yet. The surface will be removed with the TF,
     * and become invisible if the TF is invisible. */
    void moveOrCreateDecorSurfaceFor(TaskFragment taskFragment) {
    void moveOrCreateDecorSurfaceFor(TaskFragment taskFragment, boolean visible) {
        if (mDecorSurfaceContainer != null) {
            mDecorSurfaceContainer.mOwnerTaskFragment = taskFragment;
        } else {
            mDecorSurfaceContainer = new DecorSurfaceContainer(taskFragment);
            mDecorSurfaceContainer = new DecorSurfaceContainer(taskFragment, visible);
            assignChildLayers();
            sendTaskFragmentParentInfoChangedIfNeeded();
        }
@@ -6810,6 +6824,13 @@ class Task extends TaskFragment {
        return mDecorSurfaceContainer != null ? mDecorSurfaceContainer.mDecorSurface : null;
    }

    void setDecorSurfaceVisible(@NonNull SurfaceControl.Transaction t) {
        if (mDecorSurfaceContainer == null) {
            return;
        }
        t.show(mDecorSurfaceContainer.mDecorSurface);
    }

    /**
     * A class managing the decor surface.
     *
@@ -6849,12 +6870,13 @@ class Task extends TaskFragment {
        @NonNull TaskFragment mOwnerTaskFragment;

        private boolean mIsBoosted;
        private boolean mIsBoostedRequested;

        // The surface transactions that will be applied when the layer is reassigned.
        @NonNull private final List<SurfaceControl.Transaction> mPendingClientTransactions =
                new ArrayList<>();

        private DecorSurfaceContainer(@NonNull TaskFragment initialOwner) {
        private DecorSurfaceContainer(@NonNull TaskFragment initialOwner, boolean visible) {
            mOwnerTaskFragment = initialOwner;
            mContainerSurface = makeSurface().setContainerLayer()
                    .setParent(mSurfaceControl)
@@ -6867,23 +6889,36 @@ class Task extends TaskFragment {
            mDecorSurface = makeSurface()
                    .setParent(mContainerSurface)
                    .setName(mSurfaceControl + " - decor surface")
                    .setHidden(false)
                    .setHidden(!visible)
                    .setCallsite("Task.DecorSurfaceContainer")
                    .build();
        }

        private void setBoosted(
        /**
         * Sets the requested boosted state. The state is not applied until
         * {@link commitBoostedState} is called.
         */
        private void requestBoosted(
                boolean isBoosted, @Nullable SurfaceControl.Transaction clientTransaction) {
            mIsBoosted = isBoosted;
            // The client transaction will be applied together with the next assignLayer.
            mIsBoostedRequested = isBoosted;
            // The client transaction will be applied together with the next commitBoostedState.
            if (clientTransaction != null) {
                mPendingClientTransactions.add(clientTransaction);
            }
        }

        /** Applies the last requested boosted state. */
        private void commitBoostedState() {
            mIsBoosted = mIsBoostedRequested;
            applyPendingClientTransactions(getSyncTransaction());
        }

        private void assignLayer(@NonNull SurfaceControl.Transaction t, int layer) {
            t.setLayer(mContainerSurface, layer);
            t.setVisibility(mContainerSurface, mOwnerTaskFragment.isVisible() || mIsBoosted);
        }

        private void applyPendingClientTransactions(@NonNull SurfaceControl.Transaction t) {
            for (int i = 0; i < mPendingClientTransactions.size(); i++) {
                t.merge(mPendingClientTransactions.get(i));
            }
+44 −17
Original line number Diff line number Diff line
@@ -1594,11 +1594,31 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
                break;
            }
            case OP_TYPE_CREATE_OR_MOVE_TASK_FRAGMENT_DECOR_SURFACE: {
                taskFragment.getTask().moveOrCreateDecorSurfaceFor(taskFragment);
                final Task task = taskFragment.getTask();
                if (task == null) {
                    break;
                }
                // If any TaskFragment in the Task is collected by the transition, we make the decor
                // surface visible in sync with the TaskFragment transition. Otherwise, we make the
                // decor surface visible immediately.
                final TaskFragment syncTaskFragment = transition != null
                        ? task.getTaskFragment(transition.mParticipants::contains)
                        : null;

                if (syncTaskFragment != null) {
                    task.moveOrCreateDecorSurfaceFor(taskFragment, false /* visible */);
                    task.setDecorSurfaceVisible(syncTaskFragment.getSyncTransaction());
                } else {
                    task.moveOrCreateDecorSurfaceFor(taskFragment, true /* visible */);
                }
                break;
            }
            case OP_TYPE_REMOVE_TASK_FRAGMENT_DECOR_SURFACE: {
                taskFragment.getTask().removeDecorSurface();
                final Task task = taskFragment.getTask();
                if (task == null) {
                    break;
                }
                task.removeDecorSurface();
                break;
            }
            case OP_TYPE_SET_DIM_ON_TASK: {
@@ -1626,21 +1646,15 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
                        clientTransaction.sanitize(caller.mPid, caller.mUid);
                    }

                    if (transition != null) {
                        // The decor surface boost/unboost must happen after the transition is
                        // completed. Otherwise, the decor surface could be moved before Shell
                        // completes the transition, causing flicker.
                        transition.addTransitionEndedListener(() ->
                                task.setDecorSurfaceBoosted(
                                        taskFragment,
                                        operation.getBooleanValue() /* isBoosted */,
                                        clientTransaction));
                    } else {
                        task.setDecorSurfaceBoosted(
                    task.requestDecorSurfaceBoosted(
                            taskFragment,
                            operation.getBooleanValue() /* isBoosted */,
                            clientTransaction);
                    }

                    // The decor surface boost/unboost must be applied after the transition is
                    // completed. Otherwise, the decor surface could be moved before Shell completes
                    // the transition, causing flicker.
                    runAfterTransition(transition, task::commitDecorSurfaceBoostedState);
                }
                break;
            }
@@ -1653,6 +1667,19 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
        return effects;
    }

    /**
     * Executes the provided {@code runnable} after the {@code transition}. If the
     * {@code transition} is {@code null}, the {@code runnable} is executed immediately.
     */
    private static void runAfterTransition(
            @Nullable Transition transition, @NonNull Runnable runnable) {
        if (transition == null) {
            runnable.run();
        } else {
            transition.addTransitionEndedListener(runnable);
        }
    }

    private boolean validateTaskFragmentOperation(
            @NonNull WindowContainerTransaction.HierarchyOp hop,
            @Nullable IBinder errorCallbackToken, @Nullable ITaskFragmentOrganizer organizer) {
+1 −1
Original line number Diff line number Diff line
@@ -1902,7 +1902,7 @@ public class TaskFragmentOrganizerControllerTest extends WindowTestsBase {

        assertApplyTransactionAllowed(mTransaction);

        verify(task).moveOrCreateDecorSurfaceFor(tf);
        verify(task).moveOrCreateDecorSurfaceFor(tf, true /* visible */);
    }

    @Test
+10 −8
Original line number Diff line number Diff line
@@ -1695,7 +1695,7 @@ public class TaskTests extends WindowTestsBase {

        // Decor surface should be created.
        clearInvocations(task);
        task.moveOrCreateDecorSurfaceFor(fragment);
        task.moveOrCreateDecorSurfaceFor(fragment, true /* visible */);

        assertNotNull(task.mDecorSurfaceContainer);
        assertNotNull(task.getDecorSurface());
@@ -1722,14 +1722,14 @@ public class TaskTests extends WindowTestsBase {
        final TaskFragment fragment2 = createTaskFragmentWithEmbeddedActivity(task, organizer);
        doNothing().when(task).sendTaskFragmentParentInfoChangedIfNeeded();

        task.moveOrCreateDecorSurfaceFor(fragment1);
        task.moveOrCreateDecorSurfaceFor(fragment1, true /* visible */);

        assertNotNull(task.mDecorSurfaceContainer);
        assertNotNull(task.getDecorSurface());
        assertEquals(fragment1, task.mDecorSurfaceContainer.mOwnerTaskFragment);

        // Transfer ownership
        task.moveOrCreateDecorSurfaceFor(fragment2);
        task.moveOrCreateDecorSurfaceFor(fragment2, true /* visible */);

        assertNotNull(task.mDecorSurfaceContainer);
        assertNotNull(task.getDecorSurface());
@@ -1775,7 +1775,7 @@ public class TaskTests extends WindowTestsBase {
        doReturn(true).when(fragment2).isAllowedToBeEmbeddedInTrustedMode();
        doReturn(true).when(fragment1).isVisible();

        task.moveOrCreateDecorSurfaceFor(fragment1);
        task.moveOrCreateDecorSurfaceFor(fragment1, true /* visible */);
        task.assignChildLayers(t);

        verify(unembeddedActivity).assignLayer(t, 0);
@@ -1880,7 +1880,7 @@ public class TaskTests extends WindowTestsBase {
        doReturn(false).when(fragment2).isAllowedToBeEmbeddedInTrustedMode();
        doReturn(true).when(fragment1).isVisible();

        task.moveOrCreateDecorSurfaceFor(fragment1);
        task.moveOrCreateDecorSurfaceFor(fragment1, true /* visible */);

        clearInvocations(t);
        clearInvocations(unembeddedActivity);
@@ -1889,7 +1889,8 @@ public class TaskTests extends WindowTestsBase {

        // The decor surface should be placed above all the windows when boosted and the cover
        // surface should show.
        task.setDecorSurfaceBoosted(fragment1, true /* isBoosted */, clientTransaction);
        task.requestDecorSurfaceBoosted(fragment1, true /* isBoosted */, clientTransaction);
        task.commitDecorSurfaceBoostedState();

        verify(unembeddedActivity).assignLayer(t, 0);
        verify(fragment1).assignLayer(t, 1);
@@ -1906,8 +1907,9 @@ public class TaskTests extends WindowTestsBase {

        // The decor surface should be placed just above the owner TaskFragment and the cover
        // surface should hide.
        task.moveOrCreateDecorSurfaceFor(fragment1);
        task.setDecorSurfaceBoosted(fragment1, false /* isBoosted */, clientTransaction);
        task.moveOrCreateDecorSurfaceFor(fragment1, true /* visible */);
        task.requestDecorSurfaceBoosted(fragment1, false /* isBoosted */, clientTransaction);
        task.commitDecorSurfaceBoostedState();

        verify(unembeddedActivity).assignLayer(t, 0);
        verify(fragment1).assignLayer(t, 1);