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

1 files changed, 10 insertions, 142 deletions

diff --git a/services/surfaceflinger/TransactionCallbackInvoker.cpp b/services/surfaceflinger/TransactionCallbackInvoker.cpp
index 6af69f0ef2..4b12a2640b 100644
--- a/services/surfaceflinger/TransactionCallbackInvoker.cpp
+++ b/services/surfaceflinger/TransactionCallbackInvoker.cpp
@@ -49,121 +49,25 @@ static bool containsOnCommitCallbacks(const std::vector<CallbackId>& callbacks)
     return !callbacks.empty() && callbacks.front().type == CallbackId::Type::ON_COMMIT;
 }
 
-TransactionCallbackInvoker::~TransactionCallbackInvoker() {
-    {
-        std::lock_guard lock(mMutex);
-        for (const auto& [listener, transactionStats] : mCompletedTransactions) {
-            listener->unlinkToDeath(mDeathRecipient);
-        }
-    }
-}
-
-status_t TransactionCallbackInvoker::startRegistration(const ListenerCallbacks& listenerCallbacks) {
-    std::lock_guard lock(mMutex);
-
-    auto [itr, inserted] = mRegisteringTransactions.insert(listenerCallbacks);
+void TransactionCallbackInvoker::addEmptyTransaction(const ListenerCallbacks& listenerCallbacks) {
     auto& [listener, callbackIds] = listenerCallbacks;
-
-    if (inserted) {
-        if (mCompletedTransactions.count(listener) == 0) {
-            status_t err = listener->linkToDeath(mDeathRecipient);
-            if (err != NO_ERROR) {
-                ALOGE("cannot add callback because linkToDeath failed, err: %d", err);
-                return err;
-            }
-        }
-        auto& transactionStatsDeque = mCompletedTransactions[listener];
-        transactionStatsDeque.emplace_back(callbackIds);
-    }
-
-    return NO_ERROR;
-}
-
-status_t TransactionCallbackInvoker::endRegistration(const ListenerCallbacks& listenerCallbacks) {
-    std::lock_guard lock(mMutex);
-
-    auto itr = mRegisteringTransactions.find(listenerCallbacks);
-    if (itr == mRegisteringTransactions.end()) {
-        ALOGE("cannot end a registration that does not exist");
-        return BAD_VALUE;
-    }
-
-    mRegisteringTransactions.erase(itr);
-
-    return NO_ERROR;
-}
-
-bool TransactionCallbackInvoker::isRegisteringTransaction(
-        const sp<IBinder>& transactionListener, const std::vector<CallbackId>& callbackIds) {
-    ListenerCallbacks listenerCallbacks(transactionListener, callbackIds);
-
-    auto itr = mRegisteringTransactions.find(listenerCallbacks);
-    return itr != mRegisteringTransactions.end();
-}
-
-status_t TransactionCallbackInvoker::registerPendingCallbackHandle(
-        const sp<CallbackHandle>& handle) {
-    std::lock_guard lock(mMutex);
-
-    // If we can't find the transaction stats something has gone wrong. The client should call
-    // startRegistration before trying to register a pending callback handle.
-    TransactionStats* transactionStats;
-    status_t err = findTransactionStats(handle->listener, handle->callbackIds, &transactionStats);
-    if (err != NO_ERROR) {
-        ALOGE("cannot find transaction stats");
-        return err;
-    }
-
-    mPendingTransactions[handle->listener][handle->callbackIds]++;
-    return NO_ERROR;
-}
-
-status_t TransactionCallbackInvoker::finalizeCallbackHandle(const sp<CallbackHandle>& handle,
-                                                            const std::vector<JankData>& jankData) {
-    auto listener = mPendingTransactions.find(handle->listener);
-    if (listener != mPendingTransactions.end()) {
-        auto& pendingCallbacks = listener->second;
-        auto pendingCallback = pendingCallbacks.find(handle->callbackIds);
-
-        if (pendingCallback != pendingCallbacks.end()) {
-            auto& pendingCount = pendingCallback->second;
-
-            // Decrease the pending count for this listener
-            if (--pendingCount == 0) {
-                pendingCallbacks.erase(pendingCallback);
-            }
-        } else {
-            ALOGW("there are more latched callbacks than there were registered callbacks");
-        }
-        if (listener->second.size() == 0) {
-            mPendingTransactions.erase(listener);
-        }
-    } else {
-        ALOGW("cannot find listener in mPendingTransactions");
-    }
-
-    status_t err = addCallbackHandle(handle, jankData);
-    if (err != NO_ERROR) {
-        ALOGE("could not add callback handle");
-        return err;
-    }
-    return NO_ERROR;
+    auto& transactionStatsDeque = mCompletedTransactions[listener];
+    transactionStatsDeque.emplace_back(callbackIds);
 }
 
-status_t TransactionCallbackInvoker::finalizeOnCommitCallbackHandles(
+status_t TransactionCallbackInvoker::addOnCommitCallbackHandles(
         const std::deque<sp<CallbackHandle>>& handles,
         std::deque<sp<CallbackHandle>>& outRemainingHandles) {
     if (handles.empty()) {
         return NO_ERROR;
     }
-    std::lock_guard lock(mMutex);
     const std::vector<JankData>& jankData = std::vector<JankData>();
     for (const auto& handle : handles) {
         if (!containsOnCommitCallbacks(handle->callbackIds)) {
             outRemainingHandles.push_back(handle);
             continue;
         }
-        status_t err = finalizeCallbackHandle(handle, jankData);
+        status_t err = addCallbackHandle(handle, jankData);
         if (err != NO_ERROR) {
             return err;
         }
@@ -172,14 +76,13 @@ status_t TransactionCallbackInvoker::finalizeOnCommitCallbackHandles(
     return NO_ERROR;
 }
 
-status_t TransactionCallbackInvoker::finalizePendingCallbackHandles(
+status_t TransactionCallbackInvoker::addCallbackHandles(
         const std::deque<sp<CallbackHandle>>& handles, const std::vector<JankData>& jankData) {
     if (handles.empty()) {
         return NO_ERROR;
     }
-    std::lock_guard lock(mMutex);
     for (const auto& handle : handles) {
-        status_t err = finalizeCallbackHandle(handle, jankData);
+        status_t err = addCallbackHandle(handle, jankData);
         if (err != NO_ERROR) {
             return err;
         }
@@ -190,8 +93,6 @@ status_t TransactionCallbackInvoker::finalizePendingCallbackHandles(
 
 status_t TransactionCallbackInvoker::registerUnpresentedCallbackHandle(
         const sp<CallbackHandle>& handle) {
-    std::lock_guard lock(mMutex);
-
     return addCallbackHandle(handle, std::vector<JankData>());
 }
 
@@ -208,9 +109,8 @@ status_t TransactionCallbackInvoker::findTransactionStats(
             return NO_ERROR;
         }
     }
-
-    ALOGE("could not find transaction stats");
-    return BAD_VALUE;
+    *outTransactionStats = &transactionStatsDeque.emplace_back(callbackIds);
+    return NO_ERROR;
 }
 
 status_t TransactionCallbackInvoker::addCallbackHandle(const sp<CallbackHandle>& handle,
@@ -244,13 +144,10 @@ status_t TransactionCallbackInvoker::addCallbackHandle(const sp<CallbackHandle>&
 }
 
 void TransactionCallbackInvoker::addPresentFence(const sp<Fence>& presentFence) {
-    std::lock_guard<std::mutex> lock(mMutex);
     mPresentFence = presentFence;
 }
 
 void TransactionCallbackInvoker::sendCallbacks() {
-    std::lock_guard lock(mMutex);
-
     // For each listener
     auto completedTransactionsItr = mCompletedTransactions.begin();
     while (completedTransactionsItr != mCompletedTransactions.end()) {
@@ -263,27 +160,9 @@ void TransactionCallbackInvoker::sendCallbacks() {
         while (transactionStatsItr != transactionStatsDeque.end()) {
             auto& transactionStats = *transactionStatsItr;
 
-            // If this transaction is still registering, it is not safe to send a callback
-            // because there could be surface controls that haven't been added to
-            // transaction stats or mPendingTransactions.
-            if (isRegisteringTransaction(listener, transactionStats.callbackIds)) {
-                break;
-            }
-
-            // If we are still waiting on the callback handles for this transaction, stop
-            // here because all transaction callbacks for the same listener must come in order
-            auto pendingTransactions = mPendingTransactions.find(listener);
-            if (pendingTransactions != mPendingTransactions.end() &&
-                pendingTransactions->second.count(transactionStats.callbackIds) != 0) {
-                break;
-            }
-
             // If the transaction has been latched
             if (transactionStats.latchTime >= 0 &&
                 !containsOnCommitCallbacks(transactionStats.callbackIds)) {
-                if (!mPresentFence) {
-                    break;
-                }
                 transactionStats.presentFence = mPresentFence;
             }
 
@@ -303,20 +182,9 @@ void TransactionCallbackInvoker::sendCallbacks() {
                 // we get pointers that compare unequal in the SF process.
                 interface_cast<ITransactionCompletedListener>(listenerStats.listener)
                         ->onTransactionCompleted(listenerStats);
-                if (transactionStatsDeque.empty()) {
-                    listener->unlinkToDeath(mDeathRecipient);
-                    completedTransactionsItr =
-                            mCompletedTransactions.erase(completedTransactionsItr);
-                } else {
-                    completedTransactionsItr++;
-                }
-            } else {
-                completedTransactionsItr =
-                        mCompletedTransactions.erase(completedTransactionsItr);
             }
-        } else {
-            completedTransactionsItr++;
         }
+        completedTransactionsItr++;
     }
 
     if (mPresentFence) {