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

Commit 1cf47ef3 authored by Evan Rosky's avatar Evan Rosky
Browse files

Prevent double-applying transactions 

With BLAST, transactions contain buffers. Since we pass the transaction
between processes, its possible to commit the transaction twice. If the
transaction contains buffers, it will confuse surfaceflinger.

Account for this by clearing transactions when they are sent to remotes.
The merge case is non-trivial, though, because the remote may not
actually merge. So, for that, we clear the transaction only when it
is merged.

Fixing this revealed a bug in an edge-case triggered by tapltests, so this
fixes that as well.

Bug: 238328090
Test: atest TaplTestsLauncher3
Change-Id: If30b0bd4e8fad4658365826a25eac38dc06af4c7
parent aec1a526

libs/WindowManager/Shell/src/com/android/wm/shell/transition/OneShotRemoteHandler.java

+7 −0
Original line number Diff line number Diff line
@@ -99,6 +99,8 @@ public class OneShotRemoteHandler implements Transitions.TransitionHandler { 
                        + " during unit tests");

            }

            mRemote.getRemoteTransition().startAnimation(transition, info, startTransaction, cb);

            // assume that remote will apply the start transaction.

            startTransaction.clear();

        } catch (RemoteException e) {

            Log.e(Transitions.TAG, "Error running remote transition.", e);

            if (mRemote.asBinder() != null) {
@@ -120,6 +122,11 @@ public class OneShotRemoteHandler implements Transitions.TransitionHandler { 
            @Override

            public void onTransitionFinished(WindowContainerTransaction wct,

                    SurfaceControl.Transaction sct) {

                // We have merged, since we sent the transaction over binder, the one in this

                // process won't be cleared if the remote applied it. We don't actually know if the

                // remote applied the transaction, but applying twice will break surfaceflinger

                // so just assume the worst-case and clear the local transaction.

                t.clear();

                mMainExecutor.execute(

                        () -> finishCallback.onTransitionFinished(wct, null /* wctCB */));

            }

libs/WindowManager/Shell/src/com/android/wm/shell/transition/RemoteTransitionHandler.java

+7 −0
Original line number Diff line number Diff line
@@ -139,6 +139,8 @@ public class RemoteTransitionHandler implements Transitions.TransitionHandler { 
                        + " during unit tests");

            }

            remote.getRemoteTransition().startAnimation(transition, info, startTransaction, cb);

            // assume that remote will apply the start transaction.

            startTransaction.clear();

        } catch (RemoteException e) {

            Log.e(Transitions.TAG, "Error running remote transition.", e);

            unhandleDeath(remote.asBinder(), finishCallback);
@@ -162,6 +164,11 @@ public class RemoteTransitionHandler implements Transitions.TransitionHandler { 
            @Override

            public void onTransitionFinished(WindowContainerTransaction wct,

                    SurfaceControl.Transaction sct) {

                // We have merged, since we sent the transaction over binder, the one in this

                // process won't be cleared if the remote applied it. We don't actually know if the

                // remote applied the transaction, but applying twice will break surfaceflinger

                // so just assume the worst-case and clear the local transaction.

                t.clear();

                mMainExecutor.execute(() -> {

                    if (!mRequestedRemotes.containsKey(mergeTarget)) {

                        Log.e(TAG, "Merged transition finished after it's mergeTarget (the "

packages/SystemUI/shared/src/com/android/systemui/shared/system/RemoteTransitionCompat.java

+23 −0
Original line number Diff line number Diff line
@@ -224,6 +224,7 @@ public class RemoteTransitionCompat implements Parcelable { 
        private WindowContainerToken mRecentsTask = null;

        private TransitionInfo mInfo = null;

        private ArrayList<SurfaceControl> mOpeningLeashes = null;

        private boolean mOpeningHome = false;

        private ArrayMap<SurfaceControl, SurfaceControl> mLeashMap = null;

        private PictureInPictureSurfaceTransaction mPipTransaction = null;

        private IBinder mTransition = null;
@@ -321,6 +322,7 @@ public class RemoteTransitionCompat implements Parcelable { 
            }

            final int layer = mInfo.getChanges().size() * 3;

            mOpeningLeashes = new ArrayList<>();

            mOpeningHome = cancelRecents;

            final RemoteAnimationTargetCompat[] targets =

                    new RemoteAnimationTargetCompat[openingTasks.size()];

            for (int i = 0; i < openingTasks.size(); ++i) {
@@ -406,6 +408,26 @@ public class RemoteTransitionCompat implements Parcelable { 
                if (!mKeyguardLocked && mRecentsTask != null) {

                    wct.restoreTransientOrder(mRecentsTask);

                }

            } else if (toHome && mOpeningHome && mPausingTasks != null) {

                // Special situaition where 3p launcher was changed during recents (this happens

                // during tapltests...). Here we get both "return to home" AND "home opening".

                // This is basically going home, but we have to restore recents order and also

                // treat the home "pausing" task properly.

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

                    final TransitionInfo.Change change = mInfo.getChange(mPausingTasks.get(i));

                    final ActivityManager.RunningTaskInfo taskInfo = change.getTaskInfo();

                    if (taskInfo.topActivityType == ACTIVITY_TYPE_HOME) {

                        // Treat as opening (see above)

                        wct.reorder(mPausingTasks.get(i), true /* onTop */);

                        t.show(mInfo.getChange(mPausingTasks.get(i)).getLeash());

                    } else {

                        // Treat as hiding (see below)

                        t.hide(mInfo.getChange(mPausingTasks.get(i)).getLeash());

                    }

                }

                if (!mKeyguardLocked && mRecentsTask != null) {

                    wct.restoreTransientOrder(mRecentsTask);

                }

            } else {

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

                    if (!sendUserLeaveHint) {
@@ -444,6 +466,7 @@ public class RemoteTransitionCompat implements Parcelable { 
            mPausingTasks = null;

            mInfo = null;

            mOpeningLeashes = null;

            mOpeningHome = false;

            mLeashMap = null;

            mTransition = null;

        }