Merge "Don't clear mUsingBLASTSyncTransaction until sending callback" into rvc-dev am: 259309f1d3

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/base/+/11854634

Change-Id: Ieb7cbb1fd3087ab8b24926027d39d843df6b3ca2

services/core/java/com/android/server/wm/BLASTSyncEngine.java

@@ -16,17 +16,19 @@
 
 package com.android.server.wm;
 
-import android.view.SurfaceControl;
+import android.util.ArrayMap;
+import android.util.ArraySet;
 
-import java.util.HashMap;
+import java.util.Set;
 
 /**
- * Utility class for collecting and merging transactions from various sources asynchronously.
+ * Utility class for collecting WindowContainers that will merge transactions.
  * For example to use to synchronously resize all the children of a window container
  *   1. Open a new sync set, and pass the listener that will be invoked
  *        int id startSyncSet(TransactionReadyListener)
  *      the returned ID will be eventually passed to the TransactionReadyListener in combination
- *      with the prepared transaction. You also use it to refer to the operation in future steps.
+ *      with a set of WindowContainers that are ready, meaning onTransactionReady was called for
+ *      those WindowContainers. You also use it to refer to the operation in future steps.
  *   2. Ask each child to participate:
  *       addToSyncSet(int id, WindowContainer wc)
  *      if the child thinks it will be affected by a configuration change (a.k.a. has a visible
@@ -38,35 +40,37 @@ import java.util.HashMap;
  *       setReady(int id)
  *   5. If there were no sub windows anywhere in the hierarchy to wait on, then
  *      transactionReady is immediately invoked, otherwise all the windows are poked
- *      to redraw and to deliver a buffer to WMS#finishDrawing.
+ *      to redraw and to deliver a buffer to {@link WindowState#finishDrawing}.
- *      Once all this drawing is complete the combined transaction of all the buffers
+ *      Once all this drawing is complete the WindowContainer that's ready will be added to the
- *      and pending transaction hierarchy changes will be delivered to the TransactionReadyListener
+ *      set of ready WindowContainers. When the final onTransactionReady is called, it will merge
+ *      the transactions of the all the WindowContainers and will be delivered to the
+ *      TransactionReadyListener
  */
 class BLASTSyncEngine {
     private static final String TAG = "BLASTSyncEngine";
 
     interface TransactionReadyListener {
-        void onTransactionReady(int mSyncId, SurfaceControl.Transaction mergedTransaction);
+        void onTransactionReady(int mSyncId, Set<WindowContainer> windowContainersReady);
     };
 
     // Holds state associated with a single synchronous set of operations.
     class SyncState implements TransactionReadyListener {
         int mSyncId;
-        SurfaceControl.Transaction mMergedTransaction;
         int mRemainingTransactions;
         TransactionReadyListener mListener;
         boolean mReady = false;
+        Set<WindowContainer> mWindowContainersReady = new ArraySet<>();
 
         private void tryFinish() {
             if (mRemainingTransactions == 0 && mReady) {
-                mListener.onTransactionReady(mSyncId, mMergedTransaction);
+                mListener.onTransactionReady(mSyncId, mWindowContainersReady);
                 mPendingSyncs.remove(mSyncId);
             }
         }
 
-        public void onTransactionReady(int mSyncId, SurfaceControl.Transaction mergedTransaction) {
+        public void onTransactionReady(int mSyncId, Set<WindowContainer> windowContainersReady) {
             mRemainingTransactions--;
-            mMergedTransaction.merge(mergedTransaction);
+            mWindowContainersReady.addAll(windowContainersReady);
             tryFinish();
         }
 
@@ -86,14 +90,13 @@ class BLASTSyncEngine {
         SyncState(TransactionReadyListener l, int id) {
             mListener = l;
             mSyncId = id;
-            mMergedTransaction = new SurfaceControl.Transaction();
             mRemainingTransactions = 0;
         }
     };
 
-    int mNextSyncId = 0;
+    private int mNextSyncId = 0;
 
-    final HashMap<Integer, SyncState> mPendingSyncs = new HashMap();
+    private final ArrayMap<Integer, SyncState> mPendingSyncs = new ArrayMap<>();
 
     BLASTSyncEngine() {
     }

services/core/java/com/android/server/wm/WindowContainer.java

@@ -93,6 +93,7 @@ import java.lang.ref.WeakReference;
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.LinkedList;
+import java.util.Set;
 import java.util.function.Consumer;
 import java.util.function.Function;
 import java.util.function.Predicate;
@@ -2685,11 +2686,13 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
     }
 
     @Override
-    public void onTransactionReady(int mSyncId, SurfaceControl.Transaction mergedTransaction) {
+    public void onTransactionReady(int mSyncId, Set<WindowContainer> windowContainersReady) {
-        mergedTransaction.merge(mBLASTSyncTransaction);
+        if (mWaitingListener == null) {
-        mUsingBLASTSyncTransaction = false;
+            return;
+        }
 
-        mWaitingListener.onTransactionReady(mWaitingSyncId, mergedTransaction);
+        windowContainersReady.add(this);
+        mWaitingListener.onTransactionReady(mWaitingSyncId, windowContainersReady);
 
         mWaitingListener = null;
         mWaitingSyncId = -1;
@@ -2740,4 +2743,9 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
     boolean useBLASTSync() {
         return mUsingBLASTSyncTransaction;
     }
+
+    void mergeBlastSyncTransaction(Transaction t) {
+        t.merge(mBLASTSyncTransaction);
+        mUsingBLASTSyncTransaction = false;
+    }
 }

services/core/java/com/android/server/wm/WindowOrganizerController.java

@@ -44,6 +44,7 @@ import android.window.IWindowOrganizerController;
 import android.window.WindowContainerToken;
 import android.window.WindowContainerTransaction;
 
+import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.util.function.pooled.PooledConsumer;
 import com.android.internal.util.function.pooled.PooledLambda;
 
@@ -51,6 +52,7 @@ import java.util.HashMap;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 /**
  * Server side implementation for the interface for organizing windows
@@ -142,7 +144,7 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
                         // operations so we don't end up splitting effects between the WM
                         // pending transaction and the BLASTSync transaction.
                         if (syncId >= 0) {
-                            mBLASTSyncEngine.addToSyncSet(syncId, wc);
+                            addToSyncSet(syncId, wc);
                         }
 
                         int containerEffect = applyWindowContainerChange(wc, entry.getValue());
@@ -164,7 +166,7 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
                             continue;
                         }
                         if (syncId >= 0) {
-                            mBLASTSyncEngine.addToSyncSet(syncId, wc);
+                            addToSyncSet(syncId, wc);
                         }
                         effects |= sanitizeAndApplyHierarchyOp(wc, hop);
                     }
@@ -396,21 +398,33 @@ class WindowOrganizerController extends IWindowOrganizerController.Stub
         return mDisplayAreaOrganizerController;
     }
 
+    @VisibleForTesting
     int startSyncWithOrganizer(IWindowContainerTransactionCallback callback) {
         int id = mBLASTSyncEngine.startSyncSet(this);
         mTransactionCallbacksByPendingSyncId.put(id, callback);
         return id;
     }
 
+    @VisibleForTesting
     void setSyncReady(int id) {
         mBLASTSyncEngine.setReady(id);
     }
 
+    @VisibleForTesting
+    void addToSyncSet(int syncId, WindowContainer wc) {
+        mBLASTSyncEngine.addToSyncSet(syncId, wc);
+    }
+
     @Override
-    public void onTransactionReady(int mSyncId, SurfaceControl.Transaction mergedTransaction) {
+    public void onTransactionReady(int mSyncId, Set<WindowContainer> windowContainersReady) {
         final IWindowContainerTransactionCallback callback =
                 mTransactionCallbacksByPendingSyncId.get(mSyncId);
 
+        SurfaceControl.Transaction mergedTransaction = new SurfaceControl.Transaction();
+        for (WindowContainer container : windowContainersReady) {
+            container.mergeBlastSyncTransaction(mergedTransaction);
+        }
+
         try {
             callback.onTransactionReady(mSyncId, mergedTransaction);
         } catch (RemoteException e) {

services/core/java/com/android/server/wm/WindowState.java

@@ -242,8 +242,10 @@ import com.android.server.wm.utils.WmDisplayCutout;
 import java.io.PrintWriter;
 import java.lang.ref.WeakReference;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Comparator;
 import java.util.List;
+import java.util.Set;
 import java.util.function.Predicate;
 
 /** A window in the window manager. */
@@ -655,6 +657,15 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
 
     private static final StringBuilder sTmpSB = new StringBuilder();
 
+    /**
+     * Whether the next surfacePlacement call should notify that the blast sync is ready.
+     * This is set to true when {@link #finishDrawing(Transaction)} is called so
+     * {@link #onTransactionReady(int, Set)} is called after the next surfacePlacement. This allows
+     * Transactions to get flushed into the syncTransaction before notifying {@link BLASTSyncEngine}
+     * that this WindowState is ready.
+     */
+    private boolean mNotifyBlastOnSurfacePlacement;
+
     /**
      * Compares two window sub-layers and returns -1 if the first is lesser than the second in terms
      * of z-order and 1 otherwise.
@@ -5346,6 +5357,7 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
         updateFrameRateSelectionPriorityIfNeeded();
 
         mWinAnimator.prepareSurfaceLocked(true);
+        notifyBlastSyncTransaction();
         super.prepareSurfaces();
     }
 
@@ -5837,6 +5849,17 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
             mBLASTSyncTransaction.merge(postDrawTransaction);
         }
 
+        mNotifyBlastOnSurfacePlacement = true;
+        return mWinAnimator.finishDrawingLocked(null);
+    }
+
+    @VisibleForTesting
+    void notifyBlastSyncTransaction() {
+        if (!mNotifyBlastOnSurfacePlacement || mWaitingListener == null) {
+            mNotifyBlastOnSurfacePlacement = false;
+            return;
+        }
+
         // If localSyncId is >0 then we are syncing with children and will
         // invoke transaction ready from our own #transactionReady callback
         // we just need to signal our side of the sync (setReady). But if we
@@ -5844,15 +5867,14 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
         // be invoked and we need to invoke it ourself.
         if (mLocalSyncId >= 0) {
             mBLASTSyncEngine.setReady(mLocalSyncId);
-            return mWinAnimator.finishDrawingLocked(null);
+            return;
         }
 
-        mWaitingListener.onTransactionReady(mWaitingSyncId, mBLASTSyncTransaction);
+        mWaitingListener.onTransactionReady(mWaitingSyncId,  Collections.singleton(this));
-        mUsingBLASTSyncTransaction = false;
 
         mWaitingSyncId = 0;
         mWaitingListener = null;
-        return mWinAnimator.finishDrawingLocked(null);
+        mNotifyBlastOnSurfacePlacement = false;
     }
 
     private boolean requestResizeForBlastSync() {

services/tests/wmtests/src/com/android/server/wm/WindowOrganizerTests.java

@@ -67,6 +67,7 @@ import android.util.Rational;
 import android.view.Display;
 import android.view.SurfaceControl;
 import android.window.ITaskOrganizer;
+import android.window.IWindowContainerTransactionCallback;
 import android.window.WindowContainerTransaction;
 
 import androidx.test.filters.SmallTest;
@@ -728,7 +729,7 @@ public class WindowOrganizerTests extends WindowTestsBase {
         // We should be rejected from the second sync since we are already
         // in one.
         assertEquals(false, bse.addToSyncSet(id2, task));
-        w.finishDrawing(null);
+        finishAndNotifyDrawing(w);
         assertEquals(true, bse.addToSyncSet(id2, task));
         bse.setReady(id2);
     }
@@ -752,7 +753,7 @@ public class WindowOrganizerTests extends WindowTestsBase {
         // Since we have a window we have to wait for it to draw to finish sync.
         verify(transactionListener, never())
             .onTransactionReady(anyInt(), any());
-        w.finishDrawing(null);
+        finishAndNotifyDrawing(w);
         verify(transactionListener)
             .onTransactionReady(anyInt(), any());
     }
@@ -820,14 +821,14 @@ public class WindowOrganizerTests extends WindowTestsBase {
         int id = bse.startSyncSet(transactionListener);
         assertEquals(true, bse.addToSyncSet(id, task));
         bse.setReady(id);
-        w.finishDrawing(null);
+        finishAndNotifyDrawing(w);
 
         // Since we have a child window we still shouldn't be done.
         verify(transactionListener, never())
             .onTransactionReady(anyInt(), any());
         reset(transactionListener);
 
-        child.finishDrawing(null);
+        finishAndNotifyDrawing(child);
         // Ah finally! Done
         verify(transactionListener)
                 .onTransactionReady(anyInt(), any());
@@ -979,4 +980,42 @@ public class WindowOrganizerTests extends WindowTestsBase {
                 new IRequestFinishCallback.Default());
         verify(organizer, times(1)).onBackPressedOnTaskRoot(any());
     }
+
+    @Test
+    public void testBLASTCallbackWithMultipleWindows() throws Exception {
+        final ActivityStack stackController = createStack();
+        final Task task = createTask(stackController);
+        final ITaskOrganizer organizer = registerMockOrganizer();
+        final WindowState w1 = createAppWindow(task, TYPE_APPLICATION, "Enlightened Window 1");
+        final WindowState w2 = createAppWindow(task, TYPE_APPLICATION, "Enlightened Window 2");
+        makeWindowVisible(w1);
+        makeWindowVisible(w2);
+
+        IWindowContainerTransactionCallback mockCallback =
+                mock(IWindowContainerTransactionCallback.class);
+        int id = mWm.mAtmService.mWindowOrganizerController.startSyncWithOrganizer(mockCallback);
+
+        mWm.mAtmService.mWindowOrganizerController.addToSyncSet(id, task);
+        mWm.mAtmService.mWindowOrganizerController.setSyncReady(id);
+
+        // Since we have a window we have to wait for it to draw to finish sync.
+        verify(mockCallback, never()).onTransactionReady(anyInt(), any());
+        assertTrue(w1.useBLASTSync());
+        assertTrue(w2.useBLASTSync());
+        finishAndNotifyDrawing(w1);
+
+        // Even though one Window finished drawing, both windows should still be using blast sync
+        assertTrue(w1.useBLASTSync());
+        assertTrue(w2.useBLASTSync());
+
+        finishAndNotifyDrawing(w2);
+        verify(mockCallback).onTransactionReady(anyInt(), any());
+        assertFalse(w1.useBLASTSync());
+        assertFalse(w2.useBLASTSync());
+    }
+
+    private void finishAndNotifyDrawing(WindowState ws) {
+        ws.finishDrawing(null);
+        ws.notifyBlastSyncTransaction();
+    }
 }