commit 3d1047b26c81b8f83e3e420f95fc47dd004266b7
author Robert Carr <racarr@google.com> Mon Sep 20 18:22:32 2021 -0700
committer Robert Carr <racarr@google.com> Wed Sep 29 16:20:04 2021 -0700
tree 1467f9a07652a9f288d5a360aa4372e32d0c52f0
parent 2d5b070bcfe8c632edb7bbd947f2dc45255fa342

Simplify TransactionCallbackInvoker

The old code was written around the old threading model, and had
to deal with callback registration coming from multiple binder
threads that could interleave with sendCallbacks. The old code
likewise had to deal with cases where a Transaction would apply
but not "latch" in this frame. Both of these cases are removed
by queued transactions, and so we can rework the model to be much
simpler. This is the first step towards another simplification
used to rework the frame queuing model.

Test: Existing tests pass
Bug: 201436596
Change-Id: I6983221ce66bb31c5da1122bde176ab4465556eb

services/surfaceflinger/SurfaceFlinger.cpp

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 88715e3..5d47ee8 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2158,10 +2158,11 @@
 
 bool SurfaceFlinger::handleMessageInvalidate() {
     ATRACE_CALL();
+        // Send on commit callbacks
+    mTransactionCallbackInvoker.sendCallbacks();
+
     bool refreshNeeded = handlePageFlip();
 
-    // Send on commit callbacks
-    mTransactionCallbackInvoker.sendCallbacks();
 
     if (mVisibleRegionsDirty) {
         computeLayerBounds();
@@ -2352,6 +2353,7 @@
 
     mTransactionCallbackInvoker.addPresentFence(mPreviousPresentFences[0].fence);
     mTransactionCallbackInvoker.sendCallbacks();
+    mTransactionCallbackInvoker.clearCompletedTransactions();
 
     if (display && display->isInternal() && display->getPowerMode() == hal::PowerMode::ON &&
         mPreviousPresentFences[0].fenceTime->isValid()) {
@@ -3735,8 +3737,7 @@
     // that listeners with SurfaceControls will start registration during setClientStateLocked
     // below.
     for (const auto& listener : listenerCallbacks) {
-        mTransactionCallbackInvoker.startRegistration(listener);
-        mTransactionCallbackInvoker.endRegistration(listener);
+        mTransactionCallbackInvoker.addEmptyTransaction(listener);
     }
 
     std::unordered_set<ListenerCallbacks, ListenerCallbacksHash> listenerCallbacksWithSurfaces;
@@ -3754,10 +3755,6 @@
         }
     }
 
-    for (const auto& listenerCallback : listenerCallbacksWithSurfaces) {
-        mTransactionCallbackInvoker.endRegistration(listenerCallback);
-    }
-
     // If the state doesn't require a traversal and there are callbacks, send them now
     if (!(clientStateFlags & eTraversalNeeded) && hasListenerCallbacks) {
         mTransactionCallbackInvoker.sendCallbacks();
@@ -3887,14 +3884,12 @@
 
         ListenerCallbacks onCommitCallbacks = listener.filter(CallbackId::Type::ON_COMMIT);
         if (!onCommitCallbacks.callbackIds.empty()) {
-            mTransactionCallbackInvoker.startRegistration(onCommitCallbacks);
             filteredListeners.push_back(onCommitCallbacks);
             outListenerCallbacks.insert(onCommitCallbacks);
         }
 
         ListenerCallbacks onCompleteCallbacks = listener.filter(CallbackId::Type::ON_COMPLETE);
         if (!onCompleteCallbacks.callbackIds.empty()) {
-            mTransactionCallbackInvoker.startRegistration(onCompleteCallbacks);
             filteredListeners.push_back(onCompleteCallbacks);
             outListenerCallbacks.insert(onCompleteCallbacks);
         }