From 29bc5360a8c3e99b864c1b4b7b7bb981ae6d34f1 Mon Sep 17 00:00:00 2001
From: chaviw <chaviw@google.com>
Date: Tue, 7 Sep 2021 16:17:13 -0500
Subject: Store jankListener and surfaceStatListeners by layerId

Both maps were storing the listener using the SurfaceControl handle as
the key. This isn't necessary since we just need some unique identifier
for the layer. Intead, use the layerId which is safer since it won't
force the maps to hold a reference to the object.

Test: JankListener still works
Bug: 196171638
Change-Id: Ie9a2d0357dd73af7ee6f7737650451f7fd6ca6d3
---
 libs/gui/SurfaceComposerClient.cpp | 27 ++++++++++++++++++++-------
 1 file changed, 20 insertions(+), 7 deletions(-)

(limited to 'libs/gui/SurfaceComposerClient.cpp')

diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index dc71b6a212..a2037ac614 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -194,7 +194,7 @@ CallbackId TransactionCompletedListener::addCallbackFunction(
 void TransactionCompletedListener::addJankListener(const sp<JankDataListener>& listener,
                                                    sp<SurfaceControl> surfaceControl) {
     std::lock_guard<std::mutex> lock(mMutex);
-    mJankListeners.insert({surfaceControl->getHandle(), listener});
+    mJankListeners.insert({surfaceControl->getLayerId(), listener});
 }
 
 void TransactionCompletedListener::removeJankListener(const sp<JankDataListener>& listener) {
@@ -223,8 +223,8 @@ void TransactionCompletedListener::removeReleaseBufferCallback(
 void TransactionCompletedListener::addSurfaceStatsListener(void* context, void* cookie,
         sp<SurfaceControl> surfaceControl, SurfaceStatsCallback listener) {
     std::scoped_lock<std::recursive_mutex> lock(mSurfaceStatsListenerMutex);
-    mSurfaceStatsListeners.insert({surfaceControl->getHandle(),
-            SurfaceStatsCallbackEntry(context, cookie, listener)});
+    mSurfaceStatsListeners.insert(
+            {surfaceControl->getLayerId(), SurfaceStatsCallbackEntry(context, cookie, listener)});
 }
 
 void TransactionCompletedListener::removeSurfaceStatsListener(void* context, void* cookie) {
@@ -254,7 +254,7 @@ void TransactionCompletedListener::addSurfaceControlToCallbacks(
 
 void TransactionCompletedListener::onTransactionCompleted(ListenerStats listenerStats) {
     std::unordered_map<CallbackId, CallbackTranslation, CallbackIdHash> callbacksMap;
-    std::multimap<sp<IBinder>, sp<JankDataListener>> jankListenersMap;
+    std::multimap<int32_t, sp<JankDataListener>> jankListenersMap;
     {
         std::lock_guard<std::mutex> lock(mMutex);
 
@@ -352,13 +352,26 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener
             callbackFunction(transactionStats.latchTime, transactionStats.presentFence,
                              surfaceControlStats);
         }
+
         for (const auto& surfaceStats : transactionStats.surfaceStats) {
+            // The callbackMap contains the SurfaceControl object, which we need to look up the
+            // layerId. Since we don't know which callback contains the SurfaceControl, iterate
+            // through all until the SC is found.
+            int32_t layerId = -1;
+            for (auto callbackId : transactionStats.callbackIds) {
+                sp<SurfaceControl> sc =
+                        callbacksMap[callbackId].surfaceControls[surfaceStats.surfaceControl];
+                if (sc != nullptr) {
+                    layerId = sc->getLayerId();
+                    break;
+                }
+            }
+
             {
                 // Acquire surface stats listener lock such that we guarantee that after calling
                 // unregister, there won't be any further callback.
                 std::scoped_lock<std::recursive_mutex> lock(mSurfaceStatsListenerMutex);
-                auto listenerRange = mSurfaceStatsListeners.equal_range(
-                        surfaceStats.surfaceControl);
+                auto listenerRange = mSurfaceStatsListeners.equal_range(layerId);
                 for (auto it = listenerRange.first; it != listenerRange.second; it++) {
                     auto entry = it->second;
                     entry.callback(entry.context, transactionStats.latchTime,
@@ -367,7 +380,7 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener
             }
 
             if (surfaceStats.jankData.empty()) continue;
-            auto jankRange = jankListenersMap.equal_range(surfaceStats.surfaceControl);
+            auto jankRange = jankListenersMap.equal_range(layerId);
             for (auto it = jankRange.first; it != jankRange.second; it++) {
                 it->second->onJankDataAvailable(surfaceStats.jankData);
             }
-- 
cgit v1.2.3-59-g8ed1b