Merge changes from topic "sc-destruction-cleanup"

* changes:
  SurfaceControl: Provide Transaction#remove() method.
  Replace SurfaceControl#destroy with #remove
  SurfaceControl: Fix release

20 files changed, 65 insertions, 47 deletions

diff --git a/core/java/android/view/SurfaceControl.java b/core/java/android/view/SurfaceControl.java
index c0a4028c0301..8028715219cc 100644
--- a/core/java/android/view/SurfaceControl.java
+++ b/core/java/android/view/SurfaceControl.java
@@ -852,12 +852,11 @@ public final class SurfaceControl implements Parcelable {
     }
 
     /**
-     * Free all server-side state associated with this surface and
-     * release this object's reference.  This method can only be
-     * called from the process that created the service.
+     * Release the local resources like {@link #release} but also
+     * remove the Surface from the screen.
      * @hide
      */
-    public void destroy() {
+    public void remove() {
         if (mNativeObject != 0) {
             nativeDestroy(mNativeObject);
             mNativeObject = 0;
@@ -2467,5 +2466,23 @@ public final class SurfaceControl implements Parcelable {
             nativeMergeTransaction(mNativeObject, other.mNativeObject);
             return this;
         }
+
+        /**
+         * Equivalent to reparent with a null parent, in that it removes
+         * the SurfaceControl from the scene, but it also releases
+         * the local resources (by calling {@link SurfaceControl#release})
+         * after this method returns, {@link SurfaceControl#isValid} will return
+         * false for the argument.
+         *
+         * @param sc The surface to remove and release.
+         * @return This transaction
+         * @hide
+         */
+        @NonNull
+        public Transaction remove(@NonNull SurfaceControl sc) {
+            reparent(sc, null);
+            sc.release();
+            return this;
+        }
     }
 }
diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java
index cd5207c50d1d..9f0800f11721 100644
--- a/core/java/android/view/SurfaceView.java
+++ b/core/java/android/view/SurfaceView.java
@@ -334,7 +334,7 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
 
         updateSurface();
         if (mSurfaceControl != null) {
-            mSurfaceControl.destroy();
+            mSurfaceControl.remove();
         }
         mSurfaceControl = null;
 
@@ -502,11 +502,11 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
 
     private void releaseSurfaces() {
         if (mSurfaceControl != null) {
-            mSurfaceControl.destroy();
+            mSurfaceControl.remove();
             mSurfaceControl = null;
         }
         if (mBackgroundControl != null) {
-            mBackgroundControl.destroy();
+            mBackgroundControl.remove();
             mBackgroundControl = null;
         }
     }
@@ -816,7 +816,7 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
         }
 
         if (mDeferredDestroySurfaceControl != null) {
-            mDeferredDestroySurfaceControl.destroy();
+            mDeferredDestroySurfaceControl.remove();
             mDeferredDestroySurfaceControl = null;
         }
 
diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java
index 67cca56bbb8a..156972f6ecbb 100644
--- a/core/java/android/view/ViewRootImpl.java
+++ b/core/java/android/view/ViewRootImpl.java
@@ -1603,7 +1603,7 @@ public final class ViewRootImpl implements ViewParent,
         mSurfaceSession = null;
 
         if (mBoundsSurfaceControl != null) {
-            mBoundsSurfaceControl.destroy();
+            mBoundsSurfaceControl.remove();
             mBoundsSurface.release();
             mBoundsSurfaceControl = null;
         }
diff --git a/core/java/android/widget/Magnifier.java b/core/java/android/widget/Magnifier.java
index d5b1a3d69dfe..249f49956256 100644
--- a/core/java/android/widget/Magnifier.java
+++ b/core/java/android/widget/Magnifier.java
@@ -1013,7 +1013,7 @@ public final class Magnifier {
             }
             synchronized (mLock) {
                 mRenderer.destroy();
-                mSurfaceControl.destroy();
+                mSurfaceControl.remove();
                 mSurfaceSession.kill();
                 mHandler.removeCallbacks(mMagnifierUpdater);
                 if (mBitmap != null) {
diff --git a/core/jni/android_view_SurfaceControl.cpp b/core/jni/android_view_SurfaceControl.cpp
index 68be0054ae45..6b8d8b1bb91f 100644
--- a/core/jni/android_view_SurfaceControl.cpp
+++ b/core/jni/android_view_SurfaceControl.cpp
@@ -178,12 +178,13 @@ static jlong nativeCreate(JNIEnv* env, jclass clazz, jobject sessionObj,
 
 static void nativeRelease(JNIEnv* env, jclass clazz, jlong nativeObject) {
     sp<SurfaceControl> ctrl(reinterpret_cast<SurfaceControl *>(nativeObject));
+    ctrl->release();
     ctrl->decStrong((void *)nativeCreate);
 }
 
 static void nativeDestroy(JNIEnv* env, jclass clazz, jlong nativeObject) {
     sp<SurfaceControl> ctrl(reinterpret_cast<SurfaceControl *>(nativeObject));
-    ctrl->clear();
+    ctrl->destroy();
     ctrl->decStrong((void *)nativeCreate);
 }
 
diff --git a/services/core/java/com/android/server/display/ColorFade.java b/services/core/java/com/android/server/display/ColorFade.java
index 2f277e4f2ff5..31b497d001eb 100644
--- a/services/core/java/com/android/server/display/ColorFade.java
+++ b/services/core/java/com/android/server/display/ColorFade.java
@@ -636,7 +636,7 @@ final class ColorFade {
             mSurfaceLayout = null;
             SurfaceControl.openTransaction();
             try {
-                mSurfaceControl.destroy();
+                mSurfaceControl.remove();
                 mSurface.release();
             } finally {
                 SurfaceControl.closeTransaction();
diff --git a/services/core/java/com/android/server/wm/AppWindowThumbnail.java b/services/core/java/com/android/server/wm/AppWindowThumbnail.java
index 6fcc331bf62f..0e14e46e77de 100644
--- a/services/core/java/com/android/server/wm/AppWindowThumbnail.java
+++ b/services/core/java/com/android/server/wm/AppWindowThumbnail.java
@@ -143,7 +143,7 @@ class AppWindowThumbnail implements Animatable {
 
     void destroy() {
         mSurfaceAnimator.cancelAnimation();
-        mSurfaceControl.destroy();
+        mSurfaceControl.remove();
     }
 
     /**
diff --git a/services/core/java/com/android/server/wm/AppWindowToken.java b/services/core/java/com/android/server/wm/AppWindowToken.java
index 3246a8702cd1..2cc85e2076e0 100644
--- a/services/core/java/com/android/server/wm/AppWindowToken.java
+++ b/services/core/java/com/android/server/wm/AppWindowToken.java
@@ -2504,7 +2504,7 @@ class AppWindowToken extends WindowToken implements WindowManagerService.AppFree
     public void onAnimationLeashDestroyed(Transaction t) {
         super.onAnimationLeashDestroyed(t);
         if (mAnimationBoundsLayer != null) {
-            t.reparent(mAnimationBoundsLayer, null);
+            t.remove(mAnimationBoundsLayer);
             mAnimationBoundsLayer = null;
         }
 
diff --git a/services/core/java/com/android/server/wm/BlackFrame.java b/services/core/java/com/android/server/wm/BlackFrame.java
index c90f5bfb7ee0..497e4121f12e 100644
--- a/services/core/java/com/android/server/wm/BlackFrame.java
+++ b/services/core/java/com/android/server/wm/BlackFrame.java
@@ -153,7 +153,7 @@ public class BlackFrame {
                 if (mBlackSurfaces[i] != null) {
                     if (SHOW_TRANSACTIONS || SHOW_SURFACE_ALLOC) Slog.i(TAG_WM,
                             "  BLACK " + mBlackSurfaces[i].surface + ": DESTROY");
-                    mBlackSurfaces[i].surface.destroy();
+                    mBlackSurfaces[i].surface.remove();
                     mBlackSurfaces[i] = null;
                 }
             }
diff --git a/services/core/java/com/android/server/wm/Dimmer.java b/services/core/java/com/android/server/wm/Dimmer.java
index c39060ee1922..1373e1879d14 100644
--- a/services/core/java/com/android/server/wm/Dimmer.java
+++ b/services/core/java/com/android/server/wm/Dimmer.java
@@ -129,7 +129,7 @@ class Dimmer {
             final DimAnimatable dimAnimatable = new DimAnimatable(dimLayer);
             mSurfaceAnimator = new SurfaceAnimator(dimAnimatable, () -> {
                 if (!mDimming) {
-                    dimAnimatable.getPendingTransaction().reparent(mDimLayer, null);
+                    dimAnimatable.getPendingTransaction().remove(mDimLayer);
                 }
             }, mHost.mWmService);
         }
@@ -300,7 +300,7 @@ class Dimmer {
 
         if (!mDimState.mDimming) {
             if (!mDimState.mAnimateExit) {
-                t.reparent(mDimState.mDimLayer, null);
+                t.remove(mDimState.mDimLayer);
             } else {
                 startDimExit(mLastRequestedDimContainer, mDimState.mSurfaceAnimator, t);
             }
diff --git a/services/core/java/com/android/server/wm/DisplayContent.java b/services/core/java/com/android/server/wm/DisplayContent.java
index 7a8fd49361b6..928b57cf050b 100644
--- a/services/core/java/com/android/server/wm/DisplayContent.java
+++ b/services/core/java/com/android/server/wm/DisplayContent.java
@@ -4418,13 +4418,13 @@ class DisplayContent extends WindowContainer<DisplayContent.DisplayChildWindowCo
                         .show(mSplitScreenDividerAnchor);
                 scheduleAnimation();
             } else {
-                mAppAnimationLayer.destroy();
+                mAppAnimationLayer.remove();
                 mAppAnimationLayer = null;
-                mBoostedAppAnimationLayer.destroy();
+                mBoostedAppAnimationLayer.remove();
                 mBoostedAppAnimationLayer = null;
-                mHomeAppAnimationLayer.destroy();
+                mHomeAppAnimationLayer.remove();
                 mHomeAppAnimationLayer = null;
-                mSplitScreenDividerAnchor.destroy();
+                mSplitScreenDividerAnchor.remove();
                 mSplitScreenDividerAnchor = null;
             }
         }
diff --git a/services/core/java/com/android/server/wm/Letterbox.java b/services/core/java/com/android/server/wm/Letterbox.java
index 434084cc1f3d..987492024546 100644
--- a/services/core/java/com/android/server/wm/Letterbox.java
+++ b/services/core/java/com/android/server/wm/Letterbox.java
@@ -107,10 +107,10 @@ public class Letterbox {
         mOuter.setEmpty();
         mInner.setEmpty();
 
-        mTop.destroy();
-        mLeft.destroy();
-        mBottom.destroy();
-        mRight.destroy();
+        mTop.remove();
+        mLeft.remove();
+        mBottom.remove();
+        mRight.remove();
     }
 
     /** Returns whether a call to {@link #applySurfaceChanges} would change the surface. */
@@ -154,9 +154,9 @@ public class Letterbox {
             mSurface.setColor(new float[]{0, 0, 0});
         }
 
-        public void destroy() {
+        public void remove() {
             if (mSurface != null) {
-                mSurface.destroy();
+                mSurface.remove();
                 mSurface = null;
             }
         }
diff --git a/services/core/java/com/android/server/wm/ScreenRotationAnimation.java b/services/core/java/com/android/server/wm/ScreenRotationAnimation.java
index 3947bd47b588..84cd8d1632cf 100644
--- a/services/core/java/com/android/server/wm/ScreenRotationAnimation.java
+++ b/services/core/java/com/android/server/wm/ScreenRotationAnimation.java
@@ -626,7 +626,7 @@ class ScreenRotationAnimation {
             if (SHOW_TRANSACTIONS ||
                     SHOW_SURFACE_ALLOC) Slog.i(TAG_WM,
                             "  FREEZE " + mSurfaceControl + ": DESTROY");
-            mSurfaceControl.destroy();
+            mSurfaceControl.remove();
             mSurfaceControl = null;
         }
         if (mCustomBlackFrame != null) {
diff --git a/services/core/java/com/android/server/wm/SurfaceAnimator.java b/services/core/java/com/android/server/wm/SurfaceAnimator.java
index c600e0f4a657..5ea24518370b 100644
--- a/services/core/java/com/android/server/wm/SurfaceAnimator.java
+++ b/services/core/java/com/android/server/wm/SurfaceAnimator.java
@@ -280,7 +280,7 @@ class SurfaceAnimator {
         }
         mService.mAnimationTransferMap.remove(mAnimation);
         if (mLeash != null && destroyLeash) {
-            t.reparent(mLeash, null);
+            t.remove(mLeash);
             scheduleAnim = true;
         }
         mLeash = null;
diff --git a/services/core/java/com/android/server/wm/TaskStack.java b/services/core/java/com/android/server/wm/TaskStack.java
index 53cd5ea8bf16..7b742fd2e0f3 100644
--- a/services/core/java/com/android/server/wm/TaskStack.java
+++ b/services/core/java/com/android/server/wm/TaskStack.java
@@ -1006,7 +1006,7 @@ public class TaskStack extends WindowContainer<Task> implements
         EventLog.writeEvent(EventLogTags.WM_STACK_REMOVED, mStackId);
 
         if (mAnimationBackgroundSurface != null) {
-            mAnimationBackgroundSurface.destroy();
+            mAnimationBackgroundSurface.remove();
             mAnimationBackgroundSurface = null;
         }
 
diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java
index a054148e26b9..76f080b233a8 100644
--- a/services/core/java/com/android/server/wm/WindowContainer.java
+++ b/services/core/java/com/android/server/wm/WindowContainer.java
@@ -320,7 +320,7 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
         }
 
         if (mSurfaceControl != null) {
-            mPendingTransaction.reparent(mSurfaceControl, null);
+            mPendingTransaction.remove(mSurfaceControl);
 
             // Merge to parent transaction to ensure the transactions on this WindowContainer are
             // applied in native even if WindowContainer is removed.
diff --git a/services/core/java/com/android/server/wm/WindowSurfaceController.java b/services/core/java/com/android/server/wm/WindowSurfaceController.java
index dea3597989be..e796b99f3989 100644
--- a/services/core/java/com/android/server/wm/WindowSurfaceController.java
+++ b/services/core/java/com/android/server/wm/WindowSurfaceController.java
@@ -161,7 +161,7 @@ class WindowSurfaceController {
         }
         try {
             if (mSurfaceControl != null) {
-                mSurfaceControl.destroy();
+                mSurfaceControl.remove();
             }
         } catch (RuntimeException e) {
             Slog.w(TAG, "Error destroying surface in: " + this, e);
diff --git a/services/tests/wmtests/src/com/android/server/wm/AppWindowTokenAnimationTests.java b/services/tests/wmtests/src/com/android/server/wm/AppWindowTokenAnimationTests.java
index ea5ab7bf0621..dd5f32d248b7 100644
--- a/services/tests/wmtests/src/com/android/server/wm/AppWindowTokenAnimationTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/AppWindowTokenAnimationTests.java
@@ -87,8 +87,8 @@ public class AppWindowTokenAnimationTests extends WindowTestsBase {
         verify(mSpec).startAnimation(any(), any(), callbackCaptor.capture());
 
         callbackCaptor.getValue().onAnimationFinished(mSpec);
-        verify(mTransaction).reparent(eq(leash), eq(null));
-        verify(mTransaction).reparent(eq(animationBoundsLayer), eq(null));
+        verify(mTransaction).remove(eq(leash));
+        verify(mTransaction).remove(eq(animationBoundsLayer));
         assertThat(mToken.mNeedsAnimationBoundsLayer).isFalse();
     }
 
@@ -100,8 +100,8 @@ public class AppWindowTokenAnimationTests extends WindowTestsBase {
         final SurfaceControl animationBoundsLayer = mToken.mAnimationBoundsLayer;
 
         mToken.mSurfaceAnimator.cancelAnimation();
-        verify(mTransaction).reparent(eq(leash), eq(null));
-        verify(mTransaction).reparent(eq(animationBoundsLayer), eq(null));
+        verify(mTransaction).remove(eq(leash));
+        verify(mTransaction).remove(eq(animationBoundsLayer));
         assertThat(mToken.mNeedsAnimationBoundsLayer).isFalse();
     }
 
diff --git a/services/tests/wmtests/src/com/android/server/wm/DimmerTests.java b/services/tests/wmtests/src/com/android/server/wm/DimmerTests.java
index f99cd4b18647..5b32fe68feae 100644
--- a/services/tests/wmtests/src/com/android/server/wm/DimmerTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/DimmerTests.java
@@ -166,7 +166,7 @@ public class DimmerTests extends WindowTestsBase {
 
         mDimmer.updateDims(mTransaction, new Rect());
         verify(mTransaction).show(getDimLayer());
-        verify(dimLayer, never()).destroy();
+        verify(dimLayer, never()).remove();
     }
 
     @Test
@@ -212,7 +212,7 @@ public class DimmerTests extends WindowTestsBase {
         mDimmer.updateDims(mTransaction, new Rect());
         verify(mSurfaceAnimatorStarter).startAnimation(any(SurfaceAnimator.class), any(
                 SurfaceControl.Transaction.class), any(AnimationAdapter.class), anyBoolean());
-        verify(mHost.getPendingTransaction()).reparent(dimLayer, null);
+        verify(mHost.getPendingTransaction()).remove(dimLayer);
     }
 
     @Test
@@ -228,7 +228,7 @@ public class DimmerTests extends WindowTestsBase {
 
         mDimmer.updateDims(mTransaction, new Rect());
         verify(mTransaction).show(dimLayer);
-        verify(dimLayer, never()).destroy();
+        verify(dimLayer, never()).remove();
     }
 
     @Test
@@ -269,7 +269,7 @@ public class DimmerTests extends WindowTestsBase {
         mDimmer.updateDims(mTransaction, new Rect());
         verify(mSurfaceAnimatorStarter, never()).startAnimation(any(SurfaceAnimator.class), any(
                 SurfaceControl.Transaction.class), any(AnimationAdapter.class), anyBoolean());
-        verify(mTransaction).reparent(dimLayer, null);
+        verify(mTransaction).remove(dimLayer);
     }
 
     private SurfaceControl getDimLayer() {
diff --git a/services/tests/wmtests/src/com/android/server/wm/SurfaceAnimatorTest.java b/services/tests/wmtests/src/com/android/server/wm/SurfaceAnimatorTest.java
index edb395a17abb..8c32e8cea93c 100644
--- a/services/tests/wmtests/src/com/android/server/wm/SurfaceAnimatorTest.java
+++ b/services/tests/wmtests/src/com/android/server/wm/SurfaceAnimatorTest.java
@@ -96,7 +96,7 @@ public class SurfaceAnimatorTest extends WindowTestsBase {
         callbackCaptor.getValue().onAnimationFinished(mSpec);
         assertNotAnimating(mAnimatable);
         assertTrue(mAnimatable.mFinishedCallbackCalled);
-        verify(mTransaction).reparent(eq(mAnimatable.mLeash), eq(null));
+        verify(mTransaction).remove(eq(mAnimatable.mLeash));
         // TODO: Verify reparenting once we use mPendingTransaction to reparent it back
     }
 
@@ -106,7 +106,7 @@ public class SurfaceAnimatorTest extends WindowTestsBase {
         final SurfaceControl firstLeash = mAnimatable.mLeash;
         mAnimatable.mSurfaceAnimator.startAnimation(mTransaction, mSpec2, true /* hidden */);
 
-        verify(mTransaction).reparent(eq(firstLeash), eq(null));
+        verify(mTransaction).remove(eq(firstLeash));
         assertFalse(mAnimatable.mFinishedCallbackCalled);
 
         final ArgumentCaptor<OnAnimationFinishedCallback> callbackCaptor = ArgumentCaptor.forClass(
@@ -133,7 +133,7 @@ public class SurfaceAnimatorTest extends WindowTestsBase {
         assertNotAnimating(mAnimatable);
         verify(mSpec).onAnimationCancelled(any());
         assertTrue(mAnimatable.mFinishedCallbackCalled);
-        verify(mTransaction).reparent(eq(mAnimatable.mLeash), eq(null));
+        verify(mTransaction).remove(eq(mAnimatable.mLeash));
     }
 
     @Test
@@ -155,7 +155,7 @@ public class SurfaceAnimatorTest extends WindowTestsBase {
         verifyZeroInteractions(mSpec);
         assertNotAnimating(mAnimatable);
         assertTrue(mAnimatable.mFinishedCallbackCalled);
-        verify(mTransaction).reparent(eq(mAnimatable.mLeash), eq(null));
+        verify(mTransaction).remove(eq(mAnimatable.mLeash));
     }
 
     @Test
@@ -171,11 +171,11 @@ public class SurfaceAnimatorTest extends WindowTestsBase {
         assertNotAnimating(mAnimatable);
         assertAnimating(mAnimatable2);
         assertEquals(leash, mAnimatable2.mSurfaceAnimator.mLeash);
-        verify(mTransaction, never()).reparent(eq(leash), eq(null));
+        verify(mTransaction, never()).remove(eq(leash));
         callbackCaptor.getValue().onAnimationFinished(mSpec);
         assertNotAnimating(mAnimatable2);
         assertTrue(mAnimatable2.mFinishedCallbackCalled);
-        verify(mTransaction).reparent(eq(leash), eq(null));
+        verify(mTransaction).remove(eq(leash));
     }
 
     @Test
@@ -197,7 +197,7 @@ public class SurfaceAnimatorTest extends WindowTestsBase {
         mDeferFinishAnimatable.mEndDeferFinishCallback.run();
         assertNotAnimating(mAnimatable2);
         assertTrue(mDeferFinishAnimatable.mFinishedCallbackCalled);
-        verify(mTransaction).reparent(eq(mDeferFinishAnimatable.mLeash), eq(null));
+        verify(mTransaction).remove(eq(mDeferFinishAnimatable.mLeash));
     }
 
     private void assertAnimating(MyAnimatable animatable) {