From 6a08d323313851f08caa5b2555d612747d2d9095 Mon Sep 17 00:00:00 2001
From: Robert Carr <racarr@google.com>
Date: Tue, 19 May 2020 10:21:55 -0700
Subject: [PATCH] BLASTSyncEngine: Avoid overlapping syncs.

WindowState is missing a condition in prepareForSync that
WindowContainer already has about not joining a new sync when
an old one is in progress. We fix this by extracting the code
in-to a shared method.

Bug: 156758771
Test: WindowOrganizerTests
Change-Id: Ia7e9ba6b1263ccce27e7245b332669763104ae94
---
 .../android/server/wm/WindowContainer.java    | 15 +++++++++----
 .../com/android/server/wm/WindowState.java    | 12 ++++++----
 .../server/wm/WindowOrganizerTests.java       | 22 +++++++++++++++++++
 3 files changed, 41 insertions(+), 8 deletions(-)

diff --git a/services/core/java/com/android/server/wm/WindowContainer.java b/services/core/java/com/android/server/wm/WindowContainer.java
index e2023aeac3dc3..89fa907b4c584 100644
--- a/services/core/java/com/android/server/wm/WindowContainer.java
+++ b/services/core/java/com/android/server/wm/WindowContainer.java
@@ -2616,10 +2616,8 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
         return willSync;
     }
 
-    boolean prepareForSync(BLASTSyncEngine.TransactionReadyListener waitingListener,
-            int waitingId) {
-        boolean willSync = true;
-
+    boolean setPendingListener(BLASTSyncEngine.TransactionReadyListener waitingListener,
+        int waitingId) {
         // If we are invisible, no need to sync, likewise if we are already engaged in a sync,
         // we can't support overlapping syncs on a single container yet.
         if (!isVisible() || mWaitingListener != null) {
@@ -2630,6 +2628,15 @@ class WindowContainer<E extends WindowContainer> extends ConfigurationContainer<
         // Make sure to set these before we call setReady in case the sync was a no-op
         mWaitingSyncId = waitingId;
         mWaitingListener = waitingListener;
+        return true;
+    }
+
+    boolean prepareForSync(BLASTSyncEngine.TransactionReadyListener waitingListener,
+            int waitingId) {
+        boolean willSync = setPendingListener(waitingListener, waitingId);
+        if (!willSync) {
+            return false;
+        }
 
         int localId = mBLASTSyncEngine.startSyncSet(this);
         willSync |= addChildrenToSyncSet(localId);
diff --git a/services/core/java/com/android/server/wm/WindowState.java b/services/core/java/com/android/server/wm/WindowState.java
index e925ce5c2dac8..1c7946dc040e3 100644
--- a/services/core/java/com/android/server/wm/WindowState.java
+++ b/services/core/java/com/android/server/wm/WindowState.java
@@ -5707,16 +5707,20 @@ class WindowState extends WindowContainer<WindowState> implements WindowManagerP
     @Override
     boolean prepareForSync(BLASTSyncEngine.TransactionReadyListener waitingListener,
             int waitingId) {
-        if (!isVisible()) {
+        boolean willSync = setPendingListener(waitingListener, waitingId);
+        if (!willSync) {
             return false;
         }
-        mWaitingListener = waitingListener;
-        mWaitingSyncId = waitingId;
-        mUsingBLASTSyncTransaction = true;
 
         mLocalSyncId = mBLASTSyncEngine.startSyncSet(this);
         addChildrenToSyncSet(mLocalSyncId);
 
+        // In the WindowContainer implementation we immediately mark ready
+        // since a generic WindowContainer only needs to wait for its
+        // children to finish and is immediately ready from its own
+        // perspective but at the WindowState level we need to wait for ourselves
+        // to draw even if the children draw first our don't need to sync, so we omit
+        // the set ready call until later in finishDrawing()
         mWmService.mH.removeMessages(WINDOW_STATE_BLAST_SYNC_TIMEOUT, this);
         mWmService.mH.sendNewMessageDelayed(WINDOW_STATE_BLAST_SYNC_TIMEOUT, this,
             BLAST_TIMEOUT_DURATION);
diff --git a/services/tests/wmtests/src/com/android/server/wm/WindowOrganizerTests.java b/services/tests/wmtests/src/com/android/server/wm/WindowOrganizerTests.java
index fdc5c7bf0ce14..71dabc56719bc 100644
--- a/services/tests/wmtests/src/com/android/server/wm/WindowOrganizerTests.java
+++ b/services/tests/wmtests/src/com/android/server/wm/WindowOrganizerTests.java
@@ -757,6 +757,28 @@ public class WindowOrganizerTests extends WindowTestsBase {
             .onTransactionReady(anyInt(), any());
     }
 
+    @Test
+    public void testBLASTCallbackNoDoubleAdd() {
+        final ActivityStack stackController1 = createStack();
+        final Task task = createTask(stackController1);
+        final ITaskOrganizer organizer = registerMockOrganizer();
+        final WindowState w = createAppWindow(task, TYPE_APPLICATION, "Enlightened Window");
+        makeWindowVisible(w);
+
+        BLASTSyncEngine bse = new BLASTSyncEngine();
+
+        BLASTSyncEngine.TransactionReadyListener transactionListener =
+                mock(BLASTSyncEngine.TransactionReadyListener.class);
+
+        int id = bse.startSyncSet(transactionListener);
+        assertTrue(bse.addToSyncSet(id, w));
+        assertFalse(bse.addToSyncSet(id, w));
+
+        // Clean-up
+        bse.setReady(id);
+    }
+
+
     @Test
     public void testBLASTCallbackWithInvisibleWindow() {
         final ActivityStack stackController1 = createStack();