From f0d3d3c06dd71453457035914a41872421f2932d Mon Sep 17 00:00:00 2001 From: Robert Carr Date: Thu, 6 Jul 2017 15:28:34 -0700 Subject: [PATCH] SurfaceView: Avoid initializing Surface from an invalid SurfaceControl. In a recent CL we introduced a call to Surface#createFrom, in order to recreate the Surface object from the underlying SurfaceControl, as a workaround to emulate when it was parcelled over binder in the past. However this is causing BufferQueue abandoned errors when stopping and resuming some applications. To understand them, we need to revisit the SurfaceView destruction process when handling onStop. First mWindowStopped will be set to true (SurfaceView#windowStopped), and we should then enter updateSurface. Our requested visibility will now be false and so we emit the Surface destroyed callbacks. Notice in the finally block in mUpdateSurface, we will release mSurface, but we will NOT null mSurfaceControl. Inline documentation explains why. In the case that the activity is not actually being destroyed, it's possible that we may not get a dispatchDetachedFromWindow. This means that we will not null mSurfaceControl. Now if the activity is un-stopped and we re-enter updateSurface we encounter a problem state. "creating" will be set to false since mSurfaceControl != null, however mSurfaceControl will not point to a valid surface. Prior to the introduction of the #createFrom call, this unwanted state didn't cause any problems. Because mSurface was released back in the finally block as we were stopping we now fall out of the mSurface.isValid() block in updateSurface. As we reach the finally block again, we would now set mSurfaceControl=null since the app was no longer stopped. Later when we reach updateSurface again (which tends to happen quite often) it will now be null and we will correctly set creating=true, create a valid SurfaceControl, and move along happily. However following, the introduction of this Surface#createFrom call we will now reinitialize the Surface from an invalid underlying SurfaceControl. This means we will enter the mSurface.isValid block, but will proceed to emit an invalid Surface to the client in the callbacks. We avoid this state by making creating=true even if SurfaceControl=non-null when the calculated visibility changes from invisible to visible. Bug: 63251745 Test: Manual of app from bug and apps from previous related bugs. go/wm-smoke. Additional manual testing of many SV apps. Change-Id: Icc32a34cac239d65267da705cc23feb23e1ceb67 (cherry picked from commit 7c67b7d097bbf4a32e7b97689f97139a4b71007b) --- core/java/android/view/SurfaceView.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java index b035b7fd53ed..a19f05c6ae38 100644 --- a/core/java/android/view/SurfaceView.java +++ b/core/java/android/view/SurfaceView.java @@ -491,10 +491,10 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb if (myHeight <= 0) myHeight = getHeight(); final boolean formatChanged = mFormat != mRequestedFormat; - final boolean creating = (mSurfaceControl == null || formatChanged) + final boolean visibleChanged = mVisible != mRequestedVisible; + final boolean creating = (mSurfaceControl == null || formatChanged || visibleChanged) && mRequestedVisible; final boolean sizeChanged = mSurfaceWidth != myWidth || mSurfaceHeight != myHeight; - final boolean visibleChanged = mVisible != mRequestedVisible; final boolean windowVisibleChanged = mWindowVisibility != mLastWindowVisibility; boolean redrawNeeded = false; -- 2.11.0