From 2e09cb3188a29e4a8911833312a88035d5470142 Mon Sep 17 00:00:00 2001
From: chaviw <chaviw@google.com>
Date: Wed, 13 Apr 2022 10:14:03 -0500
Subject: [PATCH] Set mSyncBuffer to true when VRI did not initiate the sync
 request

VRI can initiate syncs, but they can also happen from other places when
using SurfaceSyncer. When VRI initiates the sync, sometimes the buffer
needs to be synced. Other times, we just want to know the buffer has
drawn, but don't actually need to sync the buffer.

When a sync is initiated from outside VRI, using SurfaceSyncer, we
always want to sync the buffer. This change ensures that if the sync was
started from an outside request, it will set mSyncBuffer to true.

An additional part to clarify the sync logic, renamed mLastSyncId to
mSyncId and renamed isInSync to isInLocalSync to clarify that the
mSyncId only represents VRI initiated syncs.

Test: Internet Dialog syncs buffer
Fixes: 229098223
Change-Id: I99e9e6341a487d59c33a33a61e442be7909518c2
---
 core/java/android/view/SurfaceView.java  |  2 +-
 core/java/android/view/ViewRootImpl.java | 40 ++++++++++++++----------
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java
index ed57136b1d35e..f2bf819522d22 100644
--- a/core/java/android/view/SurfaceView.java
+++ b/core/java/android/view/SurfaceView.java
@@ -967,7 +967,7 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
                 final boolean redrawNeeded = sizeChanged || creating || hintChanged
                         || (mVisible && !mDrawFinished);
                 boolean shouldSyncBuffer =
-                        redrawNeeded && viewRoot.wasRelayoutRequested() && viewRoot.isInSync();
+                        redrawNeeded && viewRoot.wasRelayoutRequested() && viewRoot.isInLocalSync();
                 SyncBufferTransactionCallback syncBufferTransactionCallback = null;
                 if (shouldSyncBuffer) {
                     syncBufferTransactionCallback = new SyncBufferTransactionCallback();
diff --git a/core/java/android/view/ViewRootImpl.java b/core/java/android/view/ViewRootImpl.java
index a3d0bf79416ba..754f55ba4a838 100644
--- a/core/java/android/view/ViewRootImpl.java
+++ b/core/java/android/view/ViewRootImpl.java
@@ -319,6 +319,8 @@ public final class ViewRootImpl implements ViewParent,
      */
     private static final int SCROLL_CAPTURE_REQUEST_TIMEOUT_MILLIS = 2500;
 
+    private static final int UNSET_SYNC_ID = -1;
+
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)
     static final ThreadLocal<HandlerActionQueue> sRunQueues = new ThreadLocal<HandlerActionQueue>();
 
@@ -815,7 +817,7 @@ public final class ViewRootImpl implements ViewParent,
     }
 
     private final SurfaceSyncer mSurfaceSyncer = new SurfaceSyncer();
-    private int mLastSyncId = -1;
+    private int mSyncId = UNSET_SYNC_ID;
     private SurfaceSyncer.SyncBufferCallback mSyncBufferCallback;
     private int mNumSyncsInProgress = 0;
 
@@ -3462,21 +3464,21 @@ public final class ViewRootImpl implements ViewParent,
             mReportNextDraw = false;
             mSyncBufferCallback = null;
             mSyncBuffer = false;
-            if (mLastSyncId != -1) {
-                mSurfaceSyncer.markSyncReady(mLastSyncId);
-                mLastSyncId = -1;
+            if (isInLocalSync()) {
+                mSurfaceSyncer.markSyncReady(mSyncId);
+                mSyncId = UNSET_SYNC_ID;
             }
         }
     }
 
     private void createSyncIfNeeded() {
         // Started a sync already or there's nothing needing to sync
-        if (mLastSyncId != -1 || !mReportNextDraw) {
+        if (isInLocalSync() || !mReportNextDraw) {
             return;
         }
 
         final int seqId = mSyncSeqId;
-        mLastSyncId = mSurfaceSyncer.setupSync(transaction -> {
+        mSyncId = mSurfaceSyncer.setupSync(transaction -> {
             // Callback will be invoked on executor thread so post to main thread.
             mHandler.postAtFrontOfQueue(() -> {
                 mSurfaceChangedTransaction.merge(transaction);
@@ -3484,9 +3486,9 @@ public final class ViewRootImpl implements ViewParent,
             });
         });
         if (DEBUG_BLAST) {
-            Log.d(mTag, "Setup new sync id=" + mLastSyncId);
+            Log.d(mTag, "Setup new sync id=" + mSyncId);
         }
-        mSurfaceSyncer.addToSync(mLastSyncId, mSyncTarget);
+        mSurfaceSyncer.addToSync(mSyncId, mSyncTarget);
     }
 
     private void notifyContentCatpureEvents() {
@@ -4122,17 +4124,19 @@ public final class ViewRootImpl implements ViewParent,
         return mAttachInfo.mThreadedRenderer != null && mAttachInfo.mThreadedRenderer.isEnabled();
     }
 
-    boolean addToSync(SurfaceSyncer.SyncTarget syncable) {
-        if (mLastSyncId == -1) {
-            return false;
+    void addToSync(SurfaceSyncer.SyncTarget syncable) {
+        if (!isInLocalSync()) {
+            return;
         }
-        mSurfaceSyncer.addToSync(mLastSyncId, syncable);
-        return true;
+        mSurfaceSyncer.addToSync(mSyncId, syncable);
     }
 
-
-    public boolean isInSync() {
-        return mLastSyncId != -1;
+    /**
+     * This VRI is currently in the middle of a sync request, but specifically one initiated from
+     * within VRI.
+     */
+    public boolean isInLocalSync() {
+        return mSyncId != UNSET_SYNC_ID;
     }
 
     private void addFrameCommitCallbackIfNeeded() {
@@ -10889,6 +10893,10 @@ public final class ViewRootImpl implements ViewParent,
 
     private void readyToSync(SurfaceSyncer.SyncBufferCallback syncBufferCallback) {
         mNumSyncsInProgress++;
+        if (!isInLocalSync()) {
+            // Always sync the buffer if the sync request did not come from VRI.
+            mSyncBuffer = true;
+        }
         if (mAttachInfo.mThreadedRenderer != null) {
             HardwareRenderer.setRtAnimationsEnabled(false);
         }