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

Commit 267c6c21 authored by Tiger's avatar Tiger
Browse files

Re-land: Execute afterPrepareSurfacesRunnables after committing... 

... the transaction

Some of the runnables will assume the transcation has been committed by
the surface flinger before getting run. For example, the leashes of
insets need to be setup before dispatching to the client. If the order
is not ensured, the transaction sent from the client might be executed
earlier than the one sent from system server which might override the
state of the leash unexpectedly.

This CL executes the runnables when the transaction has been committed.

Fix: 298018626
Fix: 356783416
Flag: EXEMPT bugfix
Test: InsetsPolicyTest InsetsStateControllerTest
      ImeInsetsSourceProviderTest CtsWindowManagerDeviceInsets
      InputMethodStatsTest KeyboardVisibilityControlTest
      MultiDisplayImeTests
Change-Id: I0ee18b75a657eba658561f6f4ace07c59bec64ae
parent 42452428

services/core/java/com/android/server/wm/InsetsSourceProvider.java

+27 −4
Original line number Diff line number Diff line
@@ -556,11 +556,37 @@ class InsetsSourceProvider { 
        }

        mControl = new InsetsSourceControl(mSource.getId(), mSource.getType(), leash,

                initiallyVisible, surfacePosition, getInsetsHint());

        mStateController.notifySurfaceTransactionReady(this, getSurfaceTransactionId(leash), true);



        ProtoLog.d(WM_DEBUG_WINDOW_INSETS,

                "InsetsSource Control %s for target %s", mControl, mControlTarget);

    }



    private long getSurfaceTransactionId(SurfaceControl leash) {

        // Here returns mNativeObject (long) as the ID instead of the leash itself so that

        // InsetsStateController won't keep referencing the leash unexpectedly.

        return leash != null ? leash.mNativeObject : 0;

    }



    /**

     * This is called when the surface transaction of the leash initialization has been committed.

     *

     * @param id Indicates which transaction is committed so that stale callbacks can be dropped.

     */

    void onSurfaceTransactionCommitted(long id) {

        if (mIsLeashReadyForDispatching) {

            return;

        }

        if (mControl == null) {

            return;

        }

        if (id != getSurfaceTransactionId(mControl.getLeash())) {

            return;

        }

        mIsLeashReadyForDispatching = true;

        mStateController.notifySurfaceTransactionReady(this, 0, false);

    }



    void startSeamlessRotation() {

        if (!mSeamlessRotating) {

            mSeamlessRotating = true;
@@ -582,10 +608,6 @@ class InsetsSourceProvider { 
        return true;

    }



    void onSurfaceTransactionApplied() {

        mIsLeashReadyForDispatching = true;

    }



    void setClientVisible(boolean clientVisible) {

        if (mClientVisible == clientVisible) {

            return;
@@ -799,6 +821,7 @@ class InsetsSourceProvider { 
        public void onAnimationCancelled(SurfaceControl animationLeash) {

            if (mAdapter == this) {

                mStateController.notifyControlRevoked(mControlTarget, InsetsSourceProvider.this);

                mStateController.notifySurfaceTransactionReady(InsetsSourceProvider.this, 0, false);

                mControl = null;

                mControlTarget = null;

                mAdapter = null;

services/core/java/com/android/server/wm/InsetsStateController.java

+23 −3
Original line number Diff line number Diff line
@@ -34,6 +34,7 @@ import android.os.Trace; 
import android.util.ArrayMap;

import android.util.ArraySet;

import android.util.SparseArray;

import android.util.SparseLongArray;

import android.util.proto.ProtoOutputStream;

import android.view.InsetsSource;

import android.view.InsetsSourceControl;
@@ -59,6 +60,7 @@ class InsetsStateController { 
    private final DisplayContent mDisplayContent;



    private final SparseArray<InsetsSourceProvider> mProviders = new SparseArray<>();

    private final SparseLongArray mSurfaceTransactionIds = new SparseLongArray();

    private final ArrayMap<InsetsControlTarget, ArrayList<InsetsSourceProvider>>

            mControlTargetProvidersMap = new ArrayMap<>();

    private final SparseArray<InsetsControlTarget> mIdControlTargetMap = new SparseArray<>();
@@ -376,14 +378,32 @@ class InsetsStateController { 
        }

    }



    void notifySurfaceTransactionReady(InsetsSourceProvider provider, long id, boolean ready) {

        if (ready) {

            mSurfaceTransactionIds.put(provider.getSource().getId(), id);

        } else {

            mSurfaceTransactionIds.delete(provider.getSource().getId());

        }

    }



    private void notifyPendingInsetsControlChanged() {

        if (mPendingControlChanged.isEmpty()) {

            return;

        }

        final int size = mSurfaceTransactionIds.size();

        final SparseLongArray surfaceTransactionIds = new SparseLongArray(size);

        for (int i = 0; i < size; i++) {

            surfaceTransactionIds.append(

                    mSurfaceTransactionIds.keyAt(i), mSurfaceTransactionIds.valueAt(i));

        }

        mDisplayContent.mWmService.mAnimator.addAfterPrepareSurfacesRunnable(() -> {

            for (int i = mProviders.size() - 1; i >= 0; i--) {

                final InsetsSourceProvider provider = mProviders.valueAt(i);

                provider.onSurfaceTransactionApplied();

            for (int i = 0; i < size; i++) {

                final int sourceId = surfaceTransactionIds.keyAt(i);

                final InsetsSourceProvider provider = mProviders.get(sourceId);

                if (provider == null) {

                    continue;

                }

                provider.onSurfaceTransactionCommitted(surfaceTransactionIds.valueAt(i));

            }

            final ArraySet<InsetsControlTarget> newControlTargets = new ArraySet<>();

            int displayId = mDisplayContent.getDisplayId();

services/core/java/com/android/server/wm/WindowAnimator.java

+19 −28
Original line number Diff line number Diff line
@@ -27,6 +27,7 @@ import static com.android.server.wm.WindowManagerDebugConfig.TAG_WITH_CLASS_NAME 
import static com.android.server.wm.WindowManagerDebugConfig.TAG_WM;



import android.content.Context;

import android.os.HandlerExecutor;

import android.os.Trace;

import android.util.Slog;

import android.util.TimeUtils;
@@ -68,6 +69,8 @@ public class WindowAnimator { 


    private Choreographer mChoreographer;



    private final HandlerExecutor mExecutor;



    /**

     * Indicates whether we have an animation frame callback scheduled, which will happen at

     * vsync-app and then schedule the animation tick at the right time (vsync-sf).
@@ -79,8 +82,7 @@ public class WindowAnimator { 
     * A list of runnable that need to be run after {@link WindowContainer#prepareSurfaces} is

     * executed and the corresponding transaction is closed and applied.

     */

    private final ArrayList<Runnable> mAfterPrepareSurfacesRunnables = new ArrayList<>();

    private boolean mInExecuteAfterPrepareSurfacesRunnables;

    private ArrayList<Runnable> mAfterPrepareSurfacesRunnables = new ArrayList<>();



    private final SurfaceControl.Transaction mTransaction;
@@ -91,6 +93,7 @@ public class WindowAnimator { 
        mTransaction = service.mTransactionFactory.get();

        service.mAnimationHandler.runWithScissors(

                () -> mChoreographer = Choreographer.getSfInstance(), 0 /* timeout */);

        mExecutor = new HandlerExecutor(service.mAnimationHandler);



        mAnimationFrameCallback = frameTimeNs -> {

            synchronized (mService.mGlobalLock) {
@@ -198,6 +201,19 @@ public class WindowAnimator { 
            updateRunningExpensiveAnimationsLegacy();

        }



        final ArrayList<Runnable> afterPrepareSurfacesRunnables = mAfterPrepareSurfacesRunnables;

        if (!afterPrepareSurfacesRunnables.isEmpty()) {

            mAfterPrepareSurfacesRunnables = new ArrayList<>();

            mTransaction.addTransactionCommittedListener(mExecutor, () -> {

                synchronized (mService.mGlobalLock) {

                    // Traverse in order they were added.

                    for (int i = 0, size = afterPrepareSurfacesRunnables.size(); i < size; i++) {

                        afterPrepareSurfacesRunnables.get(i).run();

                    }

                    afterPrepareSurfacesRunnables.clear();

                }

            });

        }

        Trace.traceBegin(Trace.TRACE_TAG_WINDOW_MANAGER, "applyTransaction");

        mTransaction.apply();

        Trace.traceEnd(Trace.TRACE_TAG_WINDOW_MANAGER);
@@ -205,7 +221,6 @@ public class WindowAnimator { 
        ProtoLog.i(WM_SHOW_TRANSACTIONS, "<<< CLOSE TRANSACTION animate");



        mService.mAtmService.mTaskOrganizerController.dispatchPendingEvents();

        executeAfterPrepareSurfacesRunnables();



        if (DEBUG_WINDOW_TRACE) {

            Slog.i(TAG, "!!! animate: exit"
@@ -287,34 +302,10 @@ public class WindowAnimator { 


    /**

     * Adds a runnable to be executed after {@link WindowContainer#prepareSurfaces} is called and

     * the corresponding transaction is closed and applied.

     * the corresponding transaction is closed, applied, and committed.

     */

    void addAfterPrepareSurfacesRunnable(Runnable r) {

        // If runnables are already being handled in executeAfterPrepareSurfacesRunnable, then just

        // immediately execute the runnable passed in.

        if (mInExecuteAfterPrepareSurfacesRunnables) {

            r.run();

            return;

        }



        mAfterPrepareSurfacesRunnables.add(r);

        scheduleAnimation();

    }



    void executeAfterPrepareSurfacesRunnables() {



        // Don't even think about to start recursing!

        if (mInExecuteAfterPrepareSurfacesRunnables) {

            return;

        }

        mInExecuteAfterPrepareSurfacesRunnables = true;



        // Traverse in order they were added.

        final int size = mAfterPrepareSurfacesRunnables.size();

        for (int i = 0; i < size; i++) {

            mAfterPrepareSurfacesRunnables.get(i).run();

        }

        mAfterPrepareSurfacesRunnables.clear();

        mInExecuteAfterPrepareSurfacesRunnables = false;

    }

}

services/tests/servicestests/utils/com/android/server/testutils/StubTransaction.java

+10 −0
Original line number Diff line number Diff line
@@ -40,12 +40,19 @@ import java.util.concurrent.Executor; 
public class StubTransaction extends SurfaceControl.Transaction {



    private HashSet<Runnable> mWindowInfosReportedListeners = new HashSet<>();

    private HashSet<SurfaceControl.TransactionCommittedListener> mTransactionCommittedListeners =

            new HashSet<>();



    @Override

    public void apply() {

        for (Runnable listener : mWindowInfosReportedListeners) {

            listener.run();

        }

        for (SurfaceControl.TransactionCommittedListener listener

                : mTransactionCommittedListeners) {

            listener.onTransactionCommitted();

        }

        mTransactionCommittedListeners.clear();

    }



    @Override
@@ -239,6 +246,9 @@ public class StubTransaction extends SurfaceControl.Transaction { 
    @Override

    public SurfaceControl.Transaction addTransactionCommittedListener(Executor executor,

            SurfaceControl.TransactionCommittedListener listener) {

        SurfaceControl.TransactionCommittedListener listenerInner =

                () -> executor.execute(listener::onTransactionCommitted);

        mTransactionCommittedListeners.add(listenerInner);

        return this;

    }

services/tests/wmtests/src/com/android/server/wm/AppTransitionControllerTest.java

+13 −12
Original line number Diff line number Diff line
@@ -94,6 +94,7 @@ public class AppTransitionControllerTest extends WindowTestsBase { 
    public void setUp() throws Exception {

        assumeFalse(WindowManagerService.sEnableShellTransitions);

        mAppTransitionController = new AppTransitionController(mWm, mDisplayContent);

        mWm.mAnimator.ready();

    }



    @Test
@@ -855,7 +856,7 @@ public class AppTransitionControllerTest extends WindowTestsBase { 


        // Prepare and start transition.

        prepareAndTriggerAppTransition(activity, null /* closingActivity */, taskFragment);

        mWm.mAnimator.executeAfterPrepareSurfacesRunnables();

        waitUntilWindowAnimatorIdle();



        // Animation run by the remote handler.

        assertTrue(remoteAnimationRunner.isAnimationStarted());
@@ -886,7 +887,7 @@ public class AppTransitionControllerTest extends WindowTestsBase { 
        // Prepare and start transition.

        prepareAndTriggerAppTransition(openingActivity, closingActivity,

                null /* changingTaskFragment */);

        mWm.mAnimator.executeAfterPrepareSurfacesRunnables();

        waitUntilWindowAnimatorIdle();



        // Animation is not run by the remote handler because the activity is filling the Task.

        assertFalse(remoteAnimationRunner.isAnimationStarted());
@@ -921,7 +922,7 @@ public class AppTransitionControllerTest extends WindowTestsBase { 
        // Prepare and start transition.

        prepareAndTriggerAppTransition(openingActivity, closingActivity,

                null /* changingTaskFragment */);

        mWm.mAnimator.executeAfterPrepareSurfacesRunnables();

        waitUntilWindowAnimatorIdle();



        // Animation run by the remote handler.

        assertTrue(remoteAnimationRunner.isAnimationStarted());
@@ -946,7 +947,7 @@ public class AppTransitionControllerTest extends WindowTestsBase { 


        // Prepare and start transition.

        prepareAndTriggerAppTransition(openingActivity, closingActivity, taskFragment);

        mWm.mAnimator.executeAfterPrepareSurfacesRunnables();

        waitUntilWindowAnimatorIdle();



        // Animation run by the remote handler.

        assertTrue(remoteAnimationRunner.isAnimationStarted());
@@ -973,7 +974,7 @@ public class AppTransitionControllerTest extends WindowTestsBase { 


        // Prepare and start transition.

        prepareAndTriggerAppTransition(openingActivity, closingActivity, taskFragment1);

        mWm.mAnimator.executeAfterPrepareSurfacesRunnables();

        waitUntilWindowAnimatorIdle();



        // Animation run by the remote handler.

        assertTrue(remoteAnimationRunner.isAnimationStarted());
@@ -997,7 +998,7 @@ public class AppTransitionControllerTest extends WindowTestsBase { 


        // Prepare and start transition.

        prepareAndTriggerAppTransition(openingActivity, closingActivity, taskFragment);

        mWm.mAnimator.executeAfterPrepareSurfacesRunnables();

        waitUntilWindowAnimatorIdle();



        // Animation not run by the remote handler.

        assertFalse(remoteAnimationRunner.isAnimationStarted());
@@ -1024,7 +1025,7 @@ public class AppTransitionControllerTest extends WindowTestsBase { 


        // Prepare and start transition.

        prepareAndTriggerAppTransition(openingActivity, closingActivity, taskFragment);

        mWm.mAnimator.executeAfterPrepareSurfacesRunnables();

        waitUntilWindowAnimatorIdle();



        // Animation should not run by the remote handler when there are non-embedded activities of

        // different UID.
@@ -1051,7 +1052,7 @@ public class AppTransitionControllerTest extends WindowTestsBase { 


        // Prepare and start transition.

        prepareAndTriggerAppTransition(activity, null /* closingActivity */, taskFragment);

        mWm.mAnimator.executeAfterPrepareSurfacesRunnables();

        waitUntilWindowAnimatorIdle();



        // Animation should not run by the remote handler when there is wallpaper in the transition.

        assertFalse(remoteAnimationRunner.isAnimationStarted());
@@ -1085,7 +1086,7 @@ public class AppTransitionControllerTest extends WindowTestsBase { 


        // Prepare and start transition.

        prepareAndTriggerAppTransition(activity1, null /* closingActivity */, taskFragment);

        mWm.mAnimator.executeAfterPrepareSurfacesRunnables();

        waitUntilWindowAnimatorIdle();



        // The animation will be animated remotely by client and all activities are input disabled

        // for untrusted animation.
@@ -1136,7 +1137,7 @@ public class AppTransitionControllerTest extends WindowTestsBase { 


        // Prepare and start transition.

        prepareAndTriggerAppTransition(activity, null /* closingActivity */, taskFragment);

        mWm.mAnimator.executeAfterPrepareSurfacesRunnables();

        waitUntilWindowAnimatorIdle();



        // The animation will be animated remotely by client and all activities are input disabled

        // for untrusted animation.
@@ -1178,7 +1179,7 @@ public class AppTransitionControllerTest extends WindowTestsBase { 


        // Prepare and start transition.

        prepareAndTriggerAppTransition(activity, null /* closingActivity */, taskFragment);

        mWm.mAnimator.executeAfterPrepareSurfacesRunnables();

        waitUntilWindowAnimatorIdle();



        // The animation will be animated remotely by client, but input should not be dropped for

        // fully trusted.