From 04b8bdf0c82206a9053ef705c1ab8a2fabff51ab Mon Sep 17 00:00:00 2001
From: Chavi Weingarten <chaviw@google.com>
Date: Wed, 12 Apr 2023 17:21:05 +0000
Subject: [PATCH] Ensure transaction is applied if added after SSG is complete

If a caller adds a Transaction to a SSG after its already complete, make
sure to immediately apply the Transaction and log a warning. It's better
for the Transaction to get applied instead of waiting around forever.

Also cleaning up some logs in SSG

Test: SurfaceSyncGroupTests
Bug: 272189296
Change-Id: I6810c5e0c20f3fd5c7d8bde16f24bc05a0495b4f
---
 .../java/android/window/SurfaceSyncGroup.java | 22 ++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/core/java/android/window/SurfaceSyncGroup.java b/core/java/android/window/SurfaceSyncGroup.java
index 7f99fb7e7815f..b3d5124882398 100644
--- a/core/java/android/window/SurfaceSyncGroup.java
+++ b/core/java/android/window/SurfaceSyncGroup.java
@@ -120,6 +120,7 @@ public final class SurfaceSyncGroup {
     private static HandlerThread sHandlerThread;
     private Handler mHandler;
 
+    @GuardedBy("mLock")
     private boolean mTimeoutAdded;
 
     private static boolean isLocalBinder(IBinder binder) {
@@ -234,6 +235,9 @@ public final class SurfaceSyncGroup {
      * SurfaceSyncGroup have completed their sync.
      */
     public void markSyncReady() {
+        if (DEBUG) {
+            Log.d(TAG, "markSyncReady " + mName);
+        }
         if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {
             Trace.traceBegin(Trace.TRACE_TAG_VIEW, "markSyncReady " + mName);
         }
@@ -456,7 +460,15 @@ public final class SurfaceSyncGroup {
      */
     public void addTransaction(@NonNull Transaction transaction) {
         synchronized (mLock) {
-            mTransaction.merge(transaction);
+            // If the caller tries to add a transaction to a completed SSG, just apply the
+            // transaction immediately since there's nothing to wait on.
+            if (mFinished) {
+                Log.w(TAG, "Adding transaction to a completed SurfaceSyncGroup(" + mName + "). "
+                        + " Applying immediately");
+                transaction.apply();
+            } else {
+                mTransaction.merge(transaction);
+            }
         }
     }
 
@@ -509,7 +521,7 @@ public final class SurfaceSyncGroup {
 
     private boolean addLocalSync(ISurfaceSyncGroup childSyncToken, boolean parentSyncGroupMerge) {
         if (DEBUG) {
-            Log.d(TAG, "Adding local sync " + mName);
+            Log.d(TAG, "Adding local sync to " + mName);
         }
 
         SurfaceSyncGroup childSurfaceSyncGroup = getSurfaceSyncGroup(childSyncToken);
@@ -540,7 +552,7 @@ public final class SurfaceSyncGroup {
     private void setTransactionCallbackFromParent(ISurfaceSyncGroup parentSyncGroup,
             ITransactionReadyCallback transactionReadyCallback) {
         if (DEBUG) {
-            Log.d(TAG, "setTransactionCallbackFromParent " + mName);
+            Log.d(TAG, "setTransactionCallbackFromParent for child " + mName);
         }
 
         if (Trace.isTagEnabled(Trace.TRACE_TAG_VIEW)) {
@@ -677,7 +689,7 @@ public final class SurfaceSyncGroup {
      */
     public ITransactionReadyCallback createTransactionReadyCallback(boolean parentSyncGroupMerge) {
         if (DEBUG) {
-            Log.d(TAG, "createTransactionReadyCallback " + mName);
+            Log.d(TAG, "createTransactionReadyCallback as part of " + mName);
         }
         ITransactionReadyCallback transactionReadyCallback =
                 new ITransactionReadyCallback.Stub() {
@@ -780,7 +792,7 @@ public final class SurfaceSyncGroup {
 
         Runnable runnable = () -> {
             Log.e(TAG, "Failed to receive transaction ready in " + TRANSACTION_READY_TIMEOUT
-                    + "ms. Marking SurfaceSyncGroup as ready " + mName);
+                    + "ms. Marking SurfaceSyncGroup(" + mName + ") as ready");
             // Clear out any pending syncs in case the other syncs can't complete or timeout due to
             // a crash.
             synchronized (mLock) {