From 34486ad942d30a71042a2d29b1c20b4df40635af Mon Sep 17 00:00:00 2001
From: Vishnu Nair <vishnun@google.com>
Date: Wed, 10 Jun 2020 18:14:20 -0700
Subject: [PATCH] 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
---
 core/java/android/view/SurfaceView.java | 63 ++++++++++++++-----------
 1 file changed, 35 insertions(+), 28 deletions(-)

diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java
index 57f91ed3c0ae3..c098fae11b8c6 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.
      */