From 5af7d62e27ff7db2b185482b842df5ebda19d46c Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Tue, 17 Mar 2020 12:04:20 -0700 Subject: [PATCH 1/2] SurfaceView: Release SurfacePackage when done We release the SurfaceControl assosciated with a Surface package when accepting a new SurfacePackage, or at time of detached-from-window this way we don't rely on the finalize method. Bug: 149591513 Test: Existing tests pass Change-Id: Ic0f7259394836ee094ed49db73b5986b778b450f --- .../android/view/SurfaceControlViewHost.java | 5 ++++- core/java/android/view/SurfaceView.java | 16 ++++++++++++++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/core/java/android/view/SurfaceControlViewHost.java b/core/java/android/view/SurfaceControlViewHost.java index b587fbe247672..e8fc1ea6e8cf2 100644 --- a/core/java/android/view/SurfaceControlViewHost.java +++ b/core/java/android/view/SurfaceControlViewHost.java @@ -98,7 +98,10 @@ public class SurfaceControlViewHost { } /** - * Release the SurfaceControl associated with the SurfacePackage. + * Release the {@link SurfaceControl} associated with this package. + * It's not necessary to call this if you pass the package to + * {@link SurfaceView#setChildSurfacePackage} as {@link SurfaceView} will + * take ownership in that case. */ public void release() { if (mSurfaceControl != null) { diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java index 1f7c3504560f0..3e1e3939d570f 100644 --- a/core/java/android/view/SurfaceView.java +++ b/core/java/android/view/SurfaceView.java @@ -496,8 +496,17 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall updateSurface(); releaseSurfaces(); - mHaveFrame = false; + // We don't release this as part of releaseSurfaces as + // that is also called on transient visibility changes. We can't + // recreate this Surface, so only release it when we are fully + // detached. + if (mSurfacePackage != null) { + mSurfacePackage.release(); + mSurfacePackage = null; + } + + mHaveFrame = false; super.onDetachedFromWindow(); } @@ -1546,7 +1555,9 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall * Display the view-hierarchy embedded within a {@link SurfaceControlViewHost.SurfacePackage} * within this SurfaceView. If this SurfaceView is above it's host Surface (see * {@link #setZOrderOnTop} then the embedded Surface hierarchy will be able to receive - * input. + * input. This will take ownership of the SurfaceControl contained inside the SurfacePackage + * and free the caller of the obligation to call + * {@link SurfaceControlViewHost.SurfacePackage#release}. * * @param p The SurfacePackage to embed. */ @@ -1556,6 +1567,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall mSurfacePackage.getSurfaceControl() : null; if (mSurfaceControl != null && lastSc != null) { mTmpTransaction.reparent(lastSc, null).apply(); + mSurfacePackage.release(); } else if (mSurfaceControl != null) { reparentSurfacePackage(mTmpTransaction, p); mTmpTransaction.apply(); From bb7964d6452c27e57a4195cb67c26b15d37f1d1a Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Wed, 18 Mar 2020 14:24:38 -0700 Subject: [PATCH 2/2] SurfaceControlViewHost: Call die instead of dispatchDetached Seems more appropriate as this more mirrors the normal release path. I think everything gets release eventually either way. Bug: 149591513 Test: Existing tests pass Change-Id: I6f21d2723e06dbae4e09421efc352c179115bc40 --- core/java/android/view/SurfaceControlViewHost.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/java/android/view/SurfaceControlViewHost.java b/core/java/android/view/SurfaceControlViewHost.java index e8fc1ea6e8cf2..cd22ad6151b82 100644 --- a/core/java/android/view/SurfaceControlViewHost.java +++ b/core/java/android/view/SurfaceControlViewHost.java @@ -233,7 +233,7 @@ public class SurfaceControlViewHost { * and render the object unusable. */ public void release() { - mViewRoot.dispatchDetachedFromWindow(); + mViewRoot.die(false /* immediate */); mSurfaceControl.release(); }