Remove CounterRotator surface in finishTransaction

Before, the surface will be removed, and the children will be reparented
to root in a separate transaction when animation is done. That would
cause a flicker because the reparented children may be above the
rotation activity.

Bug: 206094140
Test: manually test expanding PIP
Change-Id: I0f16b7590d4bd0f4c377158d555cb94f28f134cd

libs/WindowManager/Shell/src/com/android/wm/shell/pip/PipTransition.java

@@ -155,7 +155,8 @@ public class PipTransition extends PipTransitionController {
 
             switch (type) {
                 case TRANSIT_EXIT_PIP:
-                    startExitAnimation(info, startTransaction, finishCallback, exitPipChange);
+                    startExitAnimation(info, startTransaction, finishTransaction, finishCallback,
+                            exitPipChange);
                     break;
                 case TRANSIT_EXIT_PIP_TO_SPLIT:
                     startExitToSplitAnimation(info, startTransaction, finishTransaction,
@@ -283,6 +284,7 @@ public class PipTransition extends PipTransitionController {
 
     private void startExitAnimation(@NonNull TransitionInfo info,
             @NonNull SurfaceControl.Transaction startTransaction,
+            @NonNull SurfaceControl.Transaction finishTransaction,
             @NonNull Transitions.TransitionFinishCallback finishCallback,
             @NonNull TransitionInfo.Change pipChange) {
         TransitionInfo.Change displayRotationChange = null;
@@ -298,8 +300,8 @@ public class PipTransition extends PipTransitionController {
 
         if (displayRotationChange != null) {
             // Exiting PIP to fullscreen with orientation change.
-            startExpandAndRotationAnimation(info, startTransaction, finishCallback,
-                    displayRotationChange, pipChange);
+            startExpandAndRotationAnimation(info, startTransaction, finishTransaction,
+                    finishCallback, displayRotationChange, pipChange);
             return;
         }
 
@@ -322,6 +324,7 @@ public class PipTransition extends PipTransitionController {
 
     private void startExpandAndRotationAnimation(@NonNull TransitionInfo info,
             @NonNull SurfaceControl.Transaction startTransaction,
+            @NonNull SurfaceControl.Transaction finishTransaction,
             @NonNull Transitions.TransitionFinishCallback finishCallback,
             @NonNull TransitionInfo.Change displayRotationChange,
             @NonNull TransitionInfo.Change pipChange) {
@@ -335,7 +338,6 @@ public class PipTransition extends PipTransitionController {
         rotator.handleClosingChanges(info, startTransaction, rotateDelta, displayW, displayH);
 
         mFinishCallback = (wct, wctCB) -> {
-            rotator.cleanUp();
             mPipOrganizer.onExitPipFinished(pipChange.getTaskInfo());
             finishCallback.onTransitionFinished(wct, wctCB);
         };
@@ -366,6 +368,7 @@ public class PipTransition extends PipTransitionController {
                 endBounds, startBounds, new Rect(), degree, x, y, true /* isExpanding */,
                 pipRotateDelta == ROTATION_270 /* clockwise */);
         startTransaction.apply();
+        rotator.cleanUp(finishTransaction);
 
         // Expand and rotate the pip window to fullscreen.
         final PipAnimationController.PipTransitionAnimator animator =

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

@@ -35,13 +35,11 @@ import java.util.List;
  */
 public class CounterRotatorHelper {
     private final ArrayMap<WindowContainerToken, CounterRotator> mRotatorMap = new ArrayMap<>();
-    private SurfaceControl mRootLeash;
 
     /** Puts the surface controls of closing changes to counter-rotated surfaces. */
     public void handleClosingChanges(@NonNull TransitionInfo info,
             @NonNull SurfaceControl.Transaction startTransaction,
             int rotateDelta, int displayW, int displayH) {
-        mRootLeash = info.getRootLeash();
         final List<TransitionInfo.Change> changes = info.getChanges();
         final int numChanges = changes.size();
         for (int i = numChanges - 1; i >= 0; --i) {
@@ -71,10 +69,15 @@ public class CounterRotatorHelper {
         }
     }
 
-    /** Restores to the original state, i.e. reparent back to transition root. */
-    public void cleanUp() {
+    /**
+     * Removes the counter rotation surface in the finish transaction. No need to reparent the
+     * children as the finish transaction should have already taken care of that.
+     *
+     * This can only be called after startTransaction for {@link #handleClosingChanges} is applied.
+     */
+    public void cleanUp(@NonNull SurfaceControl.Transaction finishTransaction) {
         for (int i = mRotatorMap.size() - 1; i >= 0; --i) {
-            mRotatorMap.valueAt(i).cleanUp(mRootLeash);
+            mRotatorMap.valueAt(i).cleanUp(finishTransaction);
         }
         mRotatorMap.clear();
     }

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

@@ -282,7 +282,6 @@ public class DefaultTransitionHandler implements Transitions.TransitionHandler {
         final Runnable onAnimFinish = () -> {
             if (!animations.isEmpty()) return;
 
-            rotator.cleanUp();
             if (mRotationAnimation != null) {
                 mRotationAnimation.kill();
                 mRotationAnimation = null;
@@ -382,6 +381,7 @@ public class DefaultTransitionHandler implements Transitions.TransitionHandler {
         }
 
         startTransaction.apply();
+        rotator.cleanUp(finishTransaction);
         TransitionMetrics.getInstance().reportAnimationStart(transition);
         // run finish now in-case there are no animations
         onAnimFinish.run();

libs/WindowManager/Shell/src/com/android/wm/shell/util/CounterRotator.java

@@ -18,14 +18,11 @@ package com.android.wm.shell.util;
 
 import android.view.SurfaceControl;
 
-import java.util.ArrayList;
-
 /**
  * Utility class that takes care of counter-rotating surfaces during a transition animation.
  */
 public class CounterRotator {
-    SurfaceControl mSurface = null;
-    ArrayList<SurfaceControl> mRotateChildren = null;
+    private SurfaceControl mSurface = null;
 
     /** Gets the surface with the counter-rotation. */
     public SurfaceControl getSurface() {
@@ -41,7 +38,6 @@ public class CounterRotator {
     public void setup(SurfaceControl.Transaction t, SurfaceControl parent, int rotateDelta,
             float displayW, float displayH) {
         if (rotateDelta == 0) return;
-        mRotateChildren = new ArrayList<>();
         // We want to counter-rotate, so subtract from 4
         rotateDelta = 4 - (rotateDelta + 4) % 4;
         mSurface = new SurfaceControl.Builder()
@@ -64,24 +60,19 @@ public class CounterRotator {
     }
 
     /**
-     * Add a surface that needs to be counter-rotate.
+     * Adds a surface that needs to be counter-rotate.
      */
     public void addChild(SurfaceControl.Transaction t, SurfaceControl child) {
         if (mSurface == null) return;
         t.reparent(child, mSurface);
-        mRotateChildren.add(child);
     }
 
     /**
-     * Clean-up. This undoes any reparenting and effectively stops the counter-rotation.
+     * Clean-up. Since finishTransaction should reset all change leashes, we only need to remove the
+     * counter rotation surface.
      */
-    public void cleanUp(SurfaceControl rootLeash) {
+    public void cleanUp(SurfaceControl.Transaction finishTransaction) {
         if (mSurface == null) return;
-        SurfaceControl.Transaction t = new SurfaceControl.Transaction();
-        for (int i = mRotateChildren.size() - 1; i >= 0; --i) {
-            t.reparent(mRotateChildren.get(i), rootLeash);
-        }
-        t.remove(mSurface);
-        t.apply();
+        finishTransaction.remove(mSurface);
     }
 }

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

@@ -205,8 +205,10 @@ public class RemoteAnimationAdapterCompat {
                     @Override
                     @SuppressLint("NewApi")
                     public void run() {
-                        counterLauncher.cleanUp(info.getRootLeash());
-                        counterWallpaper.cleanUp(info.getRootLeash());
+                        final SurfaceControl.Transaction finishTransaction =
+                                new SurfaceControl.Transaction();
+                        counterLauncher.cleanUp(finishTransaction);
+                        counterWallpaper.cleanUp(finishTransaction);
                         // Release surface references now. This is apparently to free GPU memory
                         // while doing quick operations (eg. during CTS).
                         for (int i = info.getChanges().size() - 1; i >= 0; --i) {
@@ -216,7 +218,7 @@ public class RemoteAnimationAdapterCompat {
                             leashMap.valueAt(i).release();
                         }
                         try {
-                            finishCallback.onTransitionFinished(null /* wct */, null /* sct */);
+                            finishCallback.onTransitionFinished(null /* wct */, finishTransaction);
                         } catch (RemoteException e) {
                             Log.e("ActivityOptionsCompat", "Failed to call app controlled animation"
                                     + " finished callback", e);