Release SurfaceView surface if viewrootimpl surface is not valid

When the window visiblity changes, SurfaceView gets notified to
release its surface. If an app relayout happens before this
notification, the ViewRootImpl surface will be null and the
SurfaceView will not release it surface.

Fixes: 158469622
Test: go/wm-smoke
Test: repro steps from bug
Test: play with SurfaceView apps
Change-Id: Ia4d5948fd229b2c77700c91691b54561d84290bb

1 files changed, 35 insertions, 28 deletions

diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java
index 57f91ed3c0ae..c098fae11b8c 100644
--- a/core/java/android/view/SurfaceView.java
+++ b/core/java/android/view/SurfaceView.java
@@ -893,12 +893,15 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
             }
             return;
         }
-        ViewRootImpl viewRoot = getViewRootImpl();
-        if (viewRoot == null || viewRoot.mSurface == null || !viewRoot.mSurface.isValid()) {
-            if (DEBUG) {
-                Log.d(TAG, System.identityHashCode(this)
-                        + " updateSurface: no valid surface");
-            }
+        final ViewRootImpl viewRoot = getViewRootImpl();
+
+        if (viewRoot == null) {
+            return;
+        }
+
+        if (viewRoot.mSurface == null || !viewRoot.mSurface.isValid()) {
+            notifySurfaceDestroyed();
+            releaseSurfaces();
             return;
         }
 
@@ -1109,28 +1112,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
                     final boolean surfaceChanged = creating;
                     if (mSurfaceCreated && (surfaceChanged || (!visible && visibleChanged))) {
                         mSurfaceCreated = false;
-                        if (mSurface.isValid()) {
-                            if (DEBUG) Log.i(TAG, System.identityHashCode(this) + " "
-                                    + "visibleChanged -- surfaceDestroyed");
-                            callbacks = getSurfaceCallbacks();
-                            for (SurfaceHolder.Callback c : callbacks) {
-                                c.surfaceDestroyed(mSurfaceHolder);
-                            }
-                            // Since Android N the same surface may be reused and given to us
-                            // again by the system server at a later point. However
-                            // as we didn't do this in previous releases, clients weren't
-                            // necessarily required to clean up properly in
-                            // surfaceDestroyed. This leads to problems for example when
-                            // clients don't destroy their EGL context, and try
-                            // and create a new one on the same surface following reuse.
-                            // Since there is no valid use of the surface in-between
-                            // surfaceDestroyed and surfaceCreated, we force a disconnect,
-                            // so the next connect will always work if we end up reusing
-                            // the surface.
-                            if (mSurface.isValid()) {
-                                mSurface.forceScopedDisconnect();
-                            }
-                        }
+                        notifySurfaceDestroyed();
                     }
 
                     if (creating) {
@@ -1786,6 +1768,31 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
         }
     }
 
+    private void notifySurfaceDestroyed() {
+        if (mSurface.isValid()) {
+            if (DEBUG) Log.i(TAG, System.identityHashCode(this) + " "
+                    + "surfaceDestroyed");
+            SurfaceHolder.Callback[] callbacks = getSurfaceCallbacks();
+            for (SurfaceHolder.Callback c : callbacks) {
+                c.surfaceDestroyed(mSurfaceHolder);
+            }
+            // Since Android N the same surface may be reused and given to us
+            // again by the system server at a later point. However
+            // as we didn't do this in previous releases, clients weren't
+            // necessarily required to clean up properly in
+            // surfaceDestroyed. This leads to problems for example when
+            // clients don't destroy their EGL context, and try
+            // and create a new one on the same surface following reuse.
+            // Since there is no valid use of the surface in-between
+            // surfaceDestroyed and surfaceCreated, we force a disconnect,
+            // so the next connect will always work if we end up reusing
+            // the surface.
+            if (mSurface.isValid()) {
+                mSurface.forceScopedDisconnect();
+            }
+        }
+    }
+
     /**
      * Wrapper of accessibility embedded connection for embedded view hierarchy.
      */