From 6b1b599f30959d7029a6001740fdf0e753555285 Mon Sep 17 00:00:00 2001
From: chaviw <chaviw@google.com>
Date: Thu, 4 Nov 2021 11:16:12 -0500
Subject: [PATCH] Call applyTransactionOnDraw when removing SV

When SV is detached, we will try to clean up the SurfaceControls.
However, we want the clean up to be synced with the main app window.
This is to ensure we don't remove SV's SC before the
app window draws in the hole punched area.

Instead of relying on postionLost to synchronize with the main app
window, we can use applyTransactionOnDraw to ensure the next frame will
merge the remove transaction. Therefore, we no longer have to call
remove in positionLost since it's already handled elsewhere.

Test: No flicker when removing SV
Bug: 199860472
Change-Id: I93a91c45c862949e53ee9fd1b4a3b10404fba6bb
---
 core/java/android/view/SurfaceView.java  | 29 ++++++------------------
 core/java/android/view/ViewRootImpl.java |  8 ++++---
 2 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java
index 856dfe53dfac6..e8725f00ffdff 100644
--- a/core/java/android/view/SurfaceView.java
+++ b/core/java/android/view/SurfaceView.java
@@ -162,8 +162,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
 
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
     boolean mIsCreating = false;
-    private volatile boolean mRtHandlingPositionUpdates = false;
-    private volatile boolean mRtReleaseSurfaces = false;
 
     private final ViewTreeObserver.OnScrollChangedListener mScrollChangedListener =
             this::updateSurface;
@@ -909,13 +907,14 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
                 mBlastBufferQueue = null;
             }
 
-            if (mRtHandlingPositionUpdates) {
-                mRtReleaseSurfaces = true;
-                return;
+            ViewRootImpl viewRoot = getViewRootImpl();
+            Transaction transaction = new Transaction();
+            releaseSurfaces(transaction);
+            if (viewRoot != null) {
+                viewRoot.applyTransactionOnDraw(transaction);
+            } else {
+                transaction.apply();
             }
-
-            releaseSurfaces(mTmpTransaction);
-            mTmpTransaction.apply();
         }
     }
 
@@ -1468,15 +1467,6 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
                 if (mSurfaceControl == null) {
                     return;
                 }
-                // TODO: This is teensy bit racey in that a brand new SurfaceView moving on
-                // its 2nd frame if RenderThread is running slowly could potentially see
-                // this as false, enter the branch, get pre-empted, then this comes along
-                // and reports a new position, then the UI thread resumes and reports
-                // its position. This could therefore be de-sync'd in that interval, but
-                // the synchronization would violate the rule that RT must never block
-                // on the UI thread which would open up potential deadlocks. The risk of
-                // a single-frame desync is therefore preferable for now.
-                mRtHandlingPositionUpdates = true;
             }
             if (mRTLastReportedPosition.left == left
                     && mRTLastReportedPosition.top == top
@@ -1552,12 +1542,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
                     return;
                 }
                 mRtTransaction.hide(mSurfaceControl);
-                if (mRtReleaseSurfaces) {
-                    mRtReleaseSurfaces = false;
-                    releaseSurfaces(mRtTransaction);
-                }
                 applyOrMergeTransaction(mRtTransaction, frameNumber);
-                mRtHandlingPositionUpdates = false;
             }
         }
     }
diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java
index 65d0e497a5b17..2d7388fda2de1 100644
--- a/core/java/android/view/ViewRootImpl.java
+++ b/core/java/android/view/ViewRootImpl.java
@@ -10507,9 +10507,11 @@ public final class ViewRootImpl implements ViewParent,
 
     @Override
     public boolean applyTransactionOnDraw(@NonNull SurfaceControl.Transaction t) {
-        registerRtFrameCallback(frame -> {
-            mergeWithNextTransaction(t, frame);
-        });
+        if (mRemoved) {
+            t.apply();
+        } else {
+            registerRtFrameCallback(frame -> mergeWithNextTransaction(t, frame));
+        }
         return true;
     }