From d600d57da59244af8f94145558debe7f1acad998 Mon Sep 17 00:00:00 2001
From: Marissa Wall <marissaw@google.com>
Date: Tue, 26 Mar 2019 15:38:50 -0700
Subject: blast: in order no-op transaction callbacks

Transactions callbacks were being sent as soon as they were ready
instead of in order. To fix this, keep a deque of Transaction
callbacks and do not send a callback until all the callbacks
before it have been sent.

Bug: 128519264
Test: Transaction_test

Change-Id: Ia363b3aca85bc1cd71d0fd915de79b44f786e09f
---
 .../surfaceflinger/TransactionCompletedThread.h    | 51 ++++++++++++++--------
 1 file changed, 34 insertions(+), 17 deletions(-)

(limited to 'services/surfaceflinger/TransactionCompletedThread.h')

diff --git a/services/surfaceflinger/TransactionCompletedThread.h b/services/surfaceflinger/TransactionCompletedThread.h
index 88808fd0da..09feb75d1a 100644
--- a/services/surfaceflinger/TransactionCompletedThread.h
+++ b/services/surfaceflinger/TransactionCompletedThread.h
@@ -30,6 +30,18 @@
 
 namespace android {
 
+struct CallbackIdsHash {
+    // CallbackId vectors have several properties that let us get away with this simple hash.
+    // 1) CallbackIds are never 0 so if something has gone wrong and our CallbackId vector is
+    // empty we can still hash 0.
+    // 2) CallbackId vectors for the same listener either are identical or contain none of the
+    // same members. It is sufficient to just check the first CallbackId in the vectors. If
+    // they match, they are the same. If they do not match, they are not the same.
+    std::size_t operator()(const std::vector<CallbackId> callbackIds) const {
+        return std::hash<CallbackId>{}((callbackIds.empty()) ? 0 : callbackIds.front());
+    }
+};
+
 class CallbackHandle : public RefBase {
 public:
     CallbackHandle(const sp<ITransactionCompletedListener>& transactionListener,
@@ -51,23 +63,24 @@ public:
 
     void run();
 
+    // Adds listener and callbackIds in case there are no SurfaceControls that are supposed
+    // to be included in the callback. This functions should be call before attempting to add any
+    // callback handles.
+    status_t addCallback(const sp<ITransactionCompletedListener>& transactionListener,
+                         const std::vector<CallbackId>& callbackIds);
+
     // Informs the TransactionCompletedThread that there is a Transaction with a CallbackHandle
     // that needs to be latched and presented this frame. This function should be called once the
     // layer has received the CallbackHandle so the TransactionCompletedThread knows not to send
     // a callback for that Listener/Transaction pair until that CallbackHandle has been latched and
     // presented.
-    void registerPendingCallbackHandle(const sp<CallbackHandle>& handle);
+    status_t registerPendingCallbackHandle(const sp<CallbackHandle>& handle);
     // Notifies the TransactionCompletedThread that a pending CallbackHandle has been presented.
-    void addPresentedCallbackHandles(const std::deque<sp<CallbackHandle>>& handles);
+    status_t addPresentedCallbackHandles(const std::deque<sp<CallbackHandle>>& handles);
 
     // Adds the Transaction CallbackHandle from a layer that does not need to be relatched and
     // presented this frame.
-    void addUnpresentedCallbackHandle(const sp<CallbackHandle>& handle);
-
-    // Adds listener and callbackIds in case there are no SurfaceControls that are supposed
-    // to be included in the callback.
-    void addCallback(const sp<ITransactionCompletedListener>& transactionListener,
-                     const std::vector<CallbackId>& callbackIds);
+    status_t addUnpresentedCallbackHandle(const sp<CallbackHandle>& handle);
 
     void addPresentFence(const sp<Fence>& presentFence);
 
@@ -76,9 +89,11 @@ public:
 private:
     void threadMain();
 
+    status_t findTransactionStats(const sp<ITransactionCompletedListener>& listener,
+                                  const std::vector<CallbackId>& callbackIds,
+                                  TransactionStats** outTransactionStats) REQUIRES(mMutex);
+
     status_t addCallbackHandle(const sp<CallbackHandle>& handle) REQUIRES(mMutex);
-    status_t addCallbackLocked(const sp<ITransactionCompletedListener>& transactionListener,
-                               const std::vector<CallbackId>& callbackIds) REQUIRES(mMutex);
 
     class ThreadDeathRecipient : public IBinder::DeathRecipient {
     public:
@@ -91,9 +106,10 @@ private:
     };
     sp<ThreadDeathRecipient> mDeathRecipient;
 
-    struct IBinderHash {
-        std::size_t operator()(const sp<IBinder>& strongPointer) const {
-            return std::hash<IBinder*>{}(strongPointer.get());
+    struct ITransactionCompletedListenerHash {
+        std::size_t operator()(const sp<ITransactionCompletedListener>& listener) const {
+            return std::hash<IBinder*>{}((listener) ? IInterface::asBinder(listener).get()
+                                                    : nullptr);
         }
     };
 
@@ -106,12 +122,13 @@ private:
     std::condition_variable_any mConditionVariable;
 
     std::unordered_map<
-            sp<IBinder /*listener*/>,
+            sp<ITransactionCompletedListener>,
             std::unordered_map<std::vector<CallbackId>, uint32_t /*count*/, CallbackIdsHash>,
-            IBinderHash>
+            ITransactionCompletedListenerHash>
             mPendingTransactions GUARDED_BY(mMutex);
-    std::unordered_map<sp<IBinder /*listener*/>, ListenerStats, IBinderHash> mListenerStats
-            GUARDED_BY(mMutex);
+    std::unordered_map<sp<ITransactionCompletedListener>, std::deque<TransactionStats>,
+                       ITransactionCompletedListenerHash>
+            mCompletedTransactions GUARDED_BY(mMutex);
 
     bool mRunning GUARDED_BY(mMutex) = false;
     bool mKeepRunning GUARDED_BY(mMutex) = true;
-- 
cgit v1.2.3-59-g8ed1b