Clean up tracking of layers with buffers

The function to check if the transaction is ready to be applied
should not modify any state. Instead keep track of layers with
ready to be presented outside the check.

Bug: 184063141
Test: atest SurfaceFlinger_test ASurfaceControlTest
Change-Id: I671eabb36195b11c66a1aeefadc78c3d194caeca
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index f7a2669..74cb486 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3319,7 +3319,7 @@
     // states) around outside the scope of the lock
     std::vector<const TransactionState> transactions;
     // Layer handles that have transactions with buffers that are ready to be applied.
-    std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>> pendingBuffers;
+    std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>> bufferLayersReadyToPresent;
     {
         Mutex::Autolock _l(mStateLock);
         {
@@ -3335,10 +3335,13 @@
                                                        transaction.isAutoTimestamp,
                                                        transaction.desiredPresentTime,
                                                        transaction.originUid, transaction.states,
-                                                       pendingBuffers)) {
+                                                       bufferLayersReadyToPresent)) {
                         setTransactionFlags(eTransactionFlushNeeded);
                         break;
                     }
+                    transaction.traverseStatesWithBuffers([&](const layer_state_t& state) {
+                        bufferLayersReadyToPresent.insert(state.surface);
+                    });
                     transactions.emplace_back(std::move(transaction));
                     transactionQueue.pop();
                 }
@@ -3359,14 +3362,17 @@
                 auto& transaction = mTransactionQueue.front();
                 bool pendingTransactions = mPendingTransactionQueues.find(transaction.applyToken) !=
                         mPendingTransactionQueues.end();
-                if (!transactionIsReadyToBeApplied(transaction.frameTimelineInfo,
+                if (pendingTransactions ||
+                    !transactionIsReadyToBeApplied(transaction.frameTimelineInfo,
                                                    transaction.isAutoTimestamp,
                                                    transaction.desiredPresentTime,
                                                    transaction.originUid, transaction.states,
-                                                   pendingBuffers) ||
-                    pendingTransactions) {
+                                                   bufferLayersReadyToPresent)) {
                     mPendingTransactionQueues[transaction.applyToken].push(std::move(transaction));
                 } else {
+                    transaction.traverseStatesWithBuffers([&](const layer_state_t& state) {
+                        bufferLayersReadyToPresent.insert(state.surface);
+                    });
                     transactions.emplace_back(std::move(transaction));
                 }
                 mTransactionQueue.pop();
@@ -3421,28 +3427,28 @@
 bool SurfaceFlinger::transactionIsReadyToBeApplied(
         const FrameTimelineInfo& info, bool isAutoTimestamp, int64_t desiredPresentTime,
         uid_t originUid, const Vector<ComposerState>& states,
-        std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>>& pendingBuffers) {
+        const std::unordered_set<sp<IBinder>, ISurfaceComposer::SpHash<IBinder>>&
+                bufferLayersReadyToPresent) const {
     ATRACE_CALL();
     const nsecs_t expectedPresentTime = mExpectedPresentTime.load();
-    bool ready = true;
     // Do not present if the desiredPresentTime has not passed unless it is more than one second
     // in the future. We ignore timestamps more than 1 second in the future for stability reasons.
     if (!isAutoTimestamp && desiredPresentTime >= expectedPresentTime &&
         desiredPresentTime < expectedPresentTime + s2ns(1)) {
         ATRACE_NAME("not current");
-        ready = false;
+        return false;
     }
 
     if (!mScheduler->isVsyncValid(expectedPresentTime, originUid)) {
         ATRACE_NAME("!isVsyncValid");
-        ready = false;
+        return false;
     }
 
     // If the client didn't specify desiredPresentTime, use the vsyncId to determine the expected
     // present time of this transaction.
     if (isAutoTimestamp && frameIsEarly(expectedPresentTime, info.vsyncId)) {
         ATRACE_NAME("frameIsEarly");
-        ready = false;
+        return false;
     }
 
     for (const ComposerState& state : states) {
@@ -3451,7 +3457,7 @@
         if (acquireFenceChanged && s.acquireFence &&
             s.acquireFence->getStatus() == Fence::Status::Unsignaled) {
             ATRACE_NAME("fence unsignaled");
-            ready = false;
+            return false;
         }
 
         sp<Layer> layer = nullptr;
@@ -3470,15 +3476,15 @@
         if (s.hasBufferChanges()) {
             // If backpressure is enabled and we already have a buffer to commit, keep the
             // transaction in the queue.
-            const bool hasPendingBuffer = pendingBuffers.find(s.surface) != pendingBuffers.end();
+            const bool hasPendingBuffer =
+                    bufferLayersReadyToPresent.find(s.surface) != bufferLayersReadyToPresent.end();
             if (layer->backpressureEnabled() && hasPendingBuffer && isAutoTimestamp) {
                 ATRACE_NAME("hasPendingBuffer");
-                ready = false;
+                return false;
             }
-            pendingBuffers.insert(s.surface);
         }
     }
-    return ready;
+    return true;
 }
 
 void SurfaceFlinger::queueTransaction(TransactionState& state) {
@@ -3548,13 +3554,6 @@
         const std::vector<ListenerCallbacks>& listenerCallbacks, uint64_t transactionId) {
     ATRACE_CALL();
 
-    // Check for incoming buffer updates and increment the pending buffer count.
-    for (const auto& state : states) {
-        if (state.state.hasBufferChanges() && (state.state.surface)) {
-            mBufferCountTracker.increment(state.state.surface->localBinder());
-        }
-    }
-
     uint32_t permissions =
             callingThreadHasUnscopedSurfaceFlingerAccess() ? Permission::ACCESS_SURFACE_FLINGER : 0;
     // Avoid checking for rotation permissions if the caller already has ACCESS_SURFACE_FLINGER
@@ -3583,6 +3582,11 @@
                            permissions,        hasListenerCallbacks,
                            listenerCallbacks,  originPid,
                            originUid,          transactionId};
+
+    // Check for incoming buffer updates and increment the pending buffer count.
+    state.traverseStatesWithBuffers([&](const layer_state_t& state) {
+        mBufferCountTracker.increment(state.surface->localBinder());
+    });
     queueTransaction(state);
 
     // Check the pending state to make sure the transaction is synchronous.
@@ -6320,7 +6324,7 @@
     return fromHandleLocked(handle);
 }
 
-wp<Layer> SurfaceFlinger::fromHandleLocked(const sp<IBinder>& handle) {
+wp<Layer> SurfaceFlinger::fromHandleLocked(const sp<IBinder>& handle) const {
     BBinder* b = nullptr;
     if (handle) {
         b = handle->localBinder();
@@ -6562,6 +6566,15 @@
     return NO_ERROR;
 }
 
+void SurfaceFlinger::TransactionState::traverseStatesWithBuffers(
+        std::function<void(const layer_state_t&)> visitor) {
+    for (const auto& state : states) {
+        if (state.state.hasBufferChanges() && (state.state.surface)) {
+            visitor(state.state);
+        }
+    }
+}
+
 } // namespace android
 
 #if defined(__gl_h_)