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

Commit 7b246acb authored by Riddle Hsu's avatar Riddle Hsu
Browse files

Ensure order of transaction for surface reparent 

This avoids the case:
(1) Pending transaction reparents a window to an animation leash.
    Though it may come from some places that use pending transaction,
    such as applyStartingWindowAnimationa and WS#startAnimation,
    even if the callers use getSyncTransaction, it may still return
    pending transaction if there is no sync state.
(2) Sync transaction restores the window to original parent.
(3) Sync transaction applies in shell.
(4) Pending transaction applies in core (on vsync).

The expected result should be that the window still stay in its
original parent. But the order will cause the window to be put in
the removed animation leash.

So this change makes sure the pending transaction is applied before
(3) if surface animator uses pending transaction to reparent/remove.

Bug: 313329824
Bug: 384659191
Flag: EXEMPT bugfix
Test: TransitionTests#testTransitionVisibleChange

Change-Id: Id35577ace8f1d83a8244bc54e6b899284ba6bfe0
parent c05c4e0e

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

+9 −0
Original line number Diff line number Diff line
@@ -270,6 +270,11 @@ class BLASTSyncEngine { 
                    () -> callback.onCommitted(new SurfaceControl.Transaction()));

            mHandler.postDelayed(callback, BLAST_TIMEOUT_DURATION);



            if (mWm.mAnimator.mPendingState == WindowAnimator.PENDING_STATE_NEED_APPLY) {

                // Applies pending transaction before onTransactionReady to ensure the order with

                // sync transaction. This is unlikely to happen unless animator thread is slow.

                mWm.mAnimator.applyPendingTransaction();

            }

            Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "onTransactionReady");

            mListener.onTransactionReady(mSyncId, merged);

            Trace.traceEnd(TRACE_TAG_WINDOW_MANAGER);
@@ -353,6 +358,10 @@ class BLASTSyncEngine { 
                        + " for non-sync " + wc);

                wc.mSyncGroup = null;

            }

            if (mWm.mAnimator.mPendingState == WindowAnimator.PENDING_STATE_HAS_CHANGES

                    && wc.mSyncState != WindowContainer.SYNC_STATE_NONE) {

                mWm.mAnimator.mPendingState = WindowAnimator.PENDING_STATE_NEED_APPLY;

            }

            if (mReady) {

                mWm.mWindowPlacerLocked.requestTraversal();

            }

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

+10 −0
Original line number Diff line number Diff line
@@ -200,6 +200,7 @@ public class SurfaceAnimator { 
            }

            mSnapshot.startAnimation(t, snapshotAnim, type);

        }

        setAnimatorPendingState(t);

    }



    void startAnimation(Transaction t, AnimationAdapter anim, boolean hidden,
@@ -208,6 +209,14 @@ public class SurfaceAnimator { 
                null /* animationCancelledCallback */, null /* snapshotAnim */, null /* freezer */);

    }



    /** Indicates that there are surface operations in the pending transaction. */

    private void setAnimatorPendingState(Transaction t) {

        if (mService.mAnimator.mPendingState == WindowAnimator.PENDING_STATE_NONE

                && t == mAnimatable.getPendingTransaction()) {

            mService.mAnimator.mPendingState = WindowAnimator.PENDING_STATE_HAS_CHANGES;

        }

    }



    /** Returns whether it is currently running an animation. */

    boolean isAnimating() {

        return mAnimation != null;
@@ -357,6 +366,7 @@ public class SurfaceAnimator { 
        final boolean scheduleAnim = removeLeash(t, mAnimatable, leash, destroyLeash);

        mAnimationFinished = false;

        if (scheduleAnim) {

            setAnimatorPendingState(t);

            mService.scheduleAnimationLocked();

        }

    }

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

+36 −2
Original line number Diff line number Diff line
@@ -26,6 +26,7 @@ import static com.android.server.wm.WindowManagerDebugConfig.DEBUG_WINDOW_TRACE; 
import static com.android.server.wm.WindowManagerDebugConfig.TAG_WITH_CLASS_NAME;

import static com.android.server.wm.WindowManagerDebugConfig.TAG_WM;



import android.annotation.IntDef;

import android.content.Context;

import android.os.HandlerExecutor;

import android.os.Trace;
@@ -38,6 +39,8 @@ import com.android.internal.protolog.ProtoLog; 
import com.android.server.policy.WindowManagerPolicy;



import java.io.PrintWriter;

import java.lang.annotation.Retention;

import java.lang.annotation.RetentionPolicy;

import java.util.ArrayList;



/**
@@ -86,6 +89,25 @@ public class WindowAnimator { 


    private final SurfaceControl.Transaction mTransaction;



    /** The pending transaction is applied. */

    static final int PENDING_STATE_NONE = 0;

    /** There are some (significant) operations set to the pending transaction. */

    static final int PENDING_STATE_HAS_CHANGES = 1;

    /** The pending transaction needs to be applied before sending sync transaction to shell. */

    static final int PENDING_STATE_NEED_APPLY = 2;



    @IntDef(prefix = { "PENDING_STATE_" }, value = {

            PENDING_STATE_NONE,

            PENDING_STATE_HAS_CHANGES,

            PENDING_STATE_NEED_APPLY,

    })

    @Retention(RetentionPolicy.SOURCE)

    @interface PendingState {}



    /** The global state of pending transaction. */

    @PendingState

    int mPendingState;



    WindowAnimator(final WindowManagerService service) {

        mService = service;

        mContext = service.mContext;
@@ -217,6 +239,7 @@ public class WindowAnimator { 
        Trace.traceBegin(Trace.TRACE_TAG_WINDOW_MANAGER, "applyTransaction");

        mTransaction.apply();

        Trace.traceEnd(Trace.TRACE_TAG_WINDOW_MANAGER);

        mPendingState = PENDING_STATE_NONE;

        mService.mWindowTracing.logState("WindowAnimator");

        ProtoLog.i(WM_SHOW_TRANSACTIONS, "<<< CLOSE TRANSACTION animate");
@@ -296,8 +319,19 @@ public class WindowAnimator { 
        return mAnimationFrameCallbackScheduled;

    }



    Choreographer getChoreographer() {

        return mChoreographer;

    void applyPendingTransaction() {

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

        mPendingState = PENDING_STATE_NONE;

        final int numDisplays = mService.mRoot.getChildCount();

        if (numDisplays == 1) {

            mService.mRoot.getChildAt(0).getPendingTransaction().apply();

        } else {

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

                mTransaction.merge(mService.mRoot.getChildAt(i).getPendingTransaction());

            }

            mTransaction.apply();

        }

        Trace.traceEnd(Trace.TRACE_TAG_WINDOW_MANAGER);

    }



    /**

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

+10 −1
Original line number Diff line number Diff line
@@ -2272,15 +2272,24 @@ public class TransitionTests extends WindowTestsBase { 
            public void cleanUp(SurfaceControl.Transaction t) {

            }

        });

        assertEquals(WindowAnimator.PENDING_STATE_NONE, mWm.mAnimator.mPendingState);

        app.startAnimation(app.getPendingTransaction(), mock(AnimationAdapter.class),

                false /* hidden */, SurfaceAnimator.ANIMATION_TYPE_WINDOW_ANIMATION);

        assertEquals(WindowAnimator.PENDING_STATE_HAS_CHANGES, mWm.mAnimator.mPendingState);



        final Task task = app.getTask();

        transition.collect(task);

        assertEquals(WindowAnimator.PENDING_STATE_NEED_APPLY, mWm.mAnimator.mPendingState);

        final Rect bounds = new Rect(task.getBounds());

        Configuration c = new Configuration(task.getRequestedOverrideConfiguration());

        bounds.inset(10, 10);

        c.windowConfiguration.setBounds(bounds);

        task.onRequestedOverrideConfigurationChanged(c);

        assertTrue(freezeCalls.contains(task));

        transition.abort();



        transition.start();

        mWm.mSyncEngine.abort(transition.getSyncId());

        assertEquals(WindowAnimator.PENDING_STATE_NONE, mWm.mAnimator.mPendingState);

    }



    @Test