Fix SurfaceView background layer leaks

The background layer was parented to the root sv layer but in blast
when recreating the sv surface, we were only cleaning up the blast
surface. This would leak the root container layer and the background
color layer that was parented to it.

Instead with blast enabled, we keep both the root and the blast layer
alive. If the surface needs to be recreated, we only recreate the
BlastBufferQueue adapter. This also simplifies the preserve old surface
logic because we no longer need to track the old surface control. The
blast layer will continue presenting the buffer from the old BBQ.

Test: -1 screen with video, check we are not leaking background layers
Test: go/wm-smoke
Fixes: 175743319
Change-Id: I0f5d57e512bfae4fff9cbcd52f6939103ad3a4b0

1 files changed, 59 insertions, 30 deletions

diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java
index 70ec2d42b59b..6eba83fee48c 100644
--- a/core/java/android/view/SurfaceView.java
+++ b/core/java/android/view/SurfaceView.java
@@ -1083,7 +1083,12 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
 
                 if (creating) {
                     updateOpaqueFlag();
-                    mDeferredDestroySurfaceControl = createSurfaceControls(viewRoot);
+                    final String name = "SurfaceView[" + viewRoot.getTitle().toString() + "]";
+                    if (mUseBlastAdapter) {
+                        createBlastSurfaceControls(viewRoot, name);
+                    } else {
+                        mDeferredDestroySurfaceControl = createSurfaceControls(viewRoot, name);
+                    }
                 } else if (mSurfaceControl == null) {
                     return;
                 }
@@ -1220,53 +1225,77 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
      * out, the old surface can be persevered until the new one has drawn by keeping the reference
      * of the old SurfaceControl alive.
      */
-    private SurfaceControl createSurfaceControls(ViewRootImpl viewRoot) {
-        final String name = "SurfaceView[" + viewRoot.getTitle().toString() + "]";
-
-        SurfaceControl.Builder builder = new SurfaceControl.Builder(mSurfaceSession)
+    private SurfaceControl createSurfaceControls(ViewRootImpl viewRoot, String name) {
+        final SurfaceControl previousSurfaceControl = mSurfaceControl;
+        mSurfaceControl = new SurfaceControl.Builder(mSurfaceSession)
                 .setName(name)
                 .setLocalOwnerView(this)
                 .setParent(viewRoot.getBoundsLayer())
-                .setCallsite("SurfaceView.updateSurface");
+                .setCallsite("SurfaceView.updateSurface")
+                .setBufferSize(mSurfaceWidth, mSurfaceHeight)
+                .setFlags(mSurfaceFlags)
+                .setFormat(mFormat)
+                .build();
+        mBackgroundControl = createBackgroundControl(name);
+        return previousSurfaceControl;
+    }
 
-        final SurfaceControl previousSurfaceControl;
-        if (mUseBlastAdapter) {
-            mSurfaceControl = builder
+    private SurfaceControl createBackgroundControl(String name) {
+        return new SurfaceControl.Builder(mSurfaceSession)
+        .setName("Background for " + name)
+        .setLocalOwnerView(this)
+        .setOpaque(true)
+        .setColorLayer()
+        .setParent(mSurfaceControl)
+        .setCallsite("SurfaceView.updateSurface")
+        .build();
+    }
+
+    // We don't recreate the surface controls but only recreate the adapter. Since the blast layer
+    // is still alive, the old buffers will continue to be presented until replaced by buffers from
+    // the new adapter. This means we do not need to track the old surface control and destroy it
+    // after the client has drawn to avoid any flickers.
+    private void createBlastSurfaceControls(ViewRootImpl viewRoot, String name) {
+        if (mSurfaceControl == null) {
+            mSurfaceControl = new SurfaceControl.Builder(mSurfaceSession)
+                    .setName(name)
+                    .setLocalOwnerView(this)
+                    .setParent(viewRoot.getBoundsLayer())
+                    .setCallsite("SurfaceView.updateSurface")
                     .setContainerLayer()
                     .build();
-            previousSurfaceControl = mBlastSurfaceControl;
+        }
+
+        if (mBlastSurfaceControl == null) {
             mBlastSurfaceControl = new SurfaceControl.Builder(mSurfaceSession)
                     .setName(name + "(BLAST)")
                     .setLocalOwnerView(this)
-                    .setBufferSize(mSurfaceWidth, mSurfaceHeight)
                     .setParent(mSurfaceControl)
                     .setFlags(mSurfaceFlags)
                     .setHidden(false)
                     .setBLASTLayer()
                     .setCallsite("SurfaceView.updateSurface")
                     .build();
-            mBlastBufferQueue = new BLASTBufferQueue(name, mBlastSurfaceControl, mSurfaceWidth,
-                    mSurfaceHeight, mFormat, true /* TODO */);
         } else {
-            previousSurfaceControl = mSurfaceControl;
-            mSurfaceControl = builder
-                    .setBufferSize(mSurfaceWidth, mSurfaceHeight)
-                    .setFlags(mSurfaceFlags)
-                    .setFormat(mFormat)
-                    .build();
-            mBlastSurfaceControl = null;
-            mBlastBufferQueue = null;
+            // update blast layer
+            mTmpTransaction
+                    .setOpaque(mBlastSurfaceControl, (mSurfaceFlags & SurfaceControl.OPAQUE) != 0)
+                    .setSecure(mBlastSurfaceControl, (mSurfaceFlags & SurfaceControl.SECURE) != 0)
+                    .show(mBlastSurfaceControl)
+                    .apply();
         }
-        mBackgroundControl = new SurfaceControl.Builder(mSurfaceSession)
-            .setName("Background for " + name)
-            .setLocalOwnerView(this)
-            .setOpaque(true)
-            .setColorLayer()
-            .setParent(mSurfaceControl)
-            .setCallsite("SurfaceView.updateSurface")
-            .build();
 
-        return previousSurfaceControl;
+        if (mBackgroundControl == null) {
+            mBackgroundControl = createBackgroundControl(name);
+        }
+
+        // Always recreate the IGBP for compatibility. This can be optimized in the future but
+        // the behavior change will need to be gated by SDK version.
+        if (mBlastBufferQueue != null) {
+            mBlastBufferQueue.destroy();
+        }
+        mBlastBufferQueue = new BLASTBufferQueue(name, mBlastSurfaceControl, mSurfaceWidth,
+                mSurfaceHeight, mFormat, true /* TODO */);
     }
 
     private void onDrawFinished() {