Switch from ITransactionCompletedListener to IBinder keys
IBinder remains consistent across multiple transactions when passed
cross-process to SF. However, interface_cast'ing it to an
ITransactionCompletedListener results in different underlying objects.
Switch to using IBinder as the keys for listener callbacks.
Bug: 139731321
Test: build, boot, SurfaceFlinger_test
Change-Id: I9bea76664087020eb43ceec258b5ecede0faaea5
diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index 12deaf0..dc161b7 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -93,7 +93,7 @@
if (data.writeVectorSize(listenerCallbacks) == NO_ERROR) {
for (const auto& [listener, callbackIds] : listenerCallbacks) {
- data.writeStrongBinder(IInterface::asBinder(listener));
+ data.writeStrongBinder(listener);
data.writeInt64Vector(callbackIds);
}
}
@@ -1042,8 +1042,7 @@
std::vector<ListenerCallbacks> listenerCallbacks;
int32_t listenersSize = data.readInt32();
for (int32_t i = 0; i < listenersSize; i++) {
- auto listener =
- interface_cast<ITransactionCompletedListener>(data.readStrongBinder());
+ auto listener = data.readStrongBinder();
std::vector<CallbackId> callbackIds;
data.readInt64Vector(&callbackIds);
listenerCallbacks.emplace_back(listener, callbackIds);
diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp
index 74cd4f1..acda600 100644
--- a/libs/gui/ITransactionCompletedListener.cpp
+++ b/libs/gui/ITransactionCompletedListener.cpp
@@ -151,7 +151,7 @@
return NO_ERROR;
}
-ListenerStats ListenerStats::createEmpty(const sp<ITransactionCompletedListener>& listener,
+ListenerStats ListenerStats::createEmpty(const sp<IBinder>& listener,
const std::unordered_set<CallbackId>& callbackIds) {
ListenerStats listenerStats;
listenerStats.listener = listener;
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index e6b1beb..3d074e1 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -548,7 +548,7 @@
continue;
}
- listenerCallbacks.emplace_back(listener, std::move(callbackIds));
+ listenerCallbacks.emplace_back(IInterface::asBinder(listener), std::move(callbackIds));
// If the listener has any SurfaceControls set on this Transaction update the surface state
for (const auto& surfaceControl : surfaceControls) {
diff --git a/libs/gui/include/gui/ITransactionCompletedListener.h b/libs/gui/include/gui/ITransactionCompletedListener.h
index cbfd365..178ca2d 100644
--- a/libs/gui/include/gui/ITransactionCompletedListener.h
+++ b/libs/gui/include/gui/ITransactionCompletedListener.h
@@ -31,6 +31,7 @@
namespace android {
class ITransactionCompletedListener;
+class ListenerCallbacks;
using CallbackId = int64_t;
@@ -72,10 +73,10 @@
status_t writeToParcel(Parcel* output) const override;
status_t readFromParcel(const Parcel* input) override;
- static ListenerStats createEmpty(const sp<ITransactionCompletedListener>& listener,
+ static ListenerStats createEmpty(const sp<IBinder>& listener,
const std::unordered_set<CallbackId>& callbackIds);
- sp<ITransactionCompletedListener> listener;
+ sp<IBinder> listener;
std::vector<TransactionStats> transactionStats;
};
@@ -97,13 +98,11 @@
class ListenerCallbacks {
public:
- ListenerCallbacks(const sp<ITransactionCompletedListener>& listener,
- const std::unordered_set<CallbackId>& callbacks)
+ ListenerCallbacks(const sp<IBinder>& listener, const std::unordered_set<CallbackId>& callbacks)
: transactionCompletedListener(listener),
callbackIds(callbacks.begin(), callbacks.end()) {}
- ListenerCallbacks(const sp<ITransactionCompletedListener>& listener,
- const std::vector<CallbackId>& ids)
+ ListenerCallbacks(const sp<IBinder>& listener, const std::vector<CallbackId>& ids)
: transactionCompletedListener(listener), callbackIds(ids) {}
bool operator==(const ListenerCallbacks& rhs) const {
@@ -116,8 +115,42 @@
return callbackIds.front() == rhs.callbackIds.front();
}
- sp<ITransactionCompletedListener> transactionCompletedListener;
+ sp<IBinder> transactionCompletedListener;
std::vector<CallbackId> callbackIds;
};
+struct IListenerHash {
+ std::size_t operator()(const sp<IBinder>& strongPointer) const {
+ return std::hash<IBinder*>{}(strongPointer.get());
+ }
+};
+
+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());
+ }
+};
+
+struct ListenerCallbacksHash {
+ std::size_t HashCombine(size_t value1, size_t value2) const {
+ return value1 ^ (value2 + 0x9e3779b9 + (value1 << 6) + (value1 >> 2));
+ }
+
+ std::size_t operator()(const ListenerCallbacks& listenerCallbacks) const {
+ struct IListenerHash listenerHasher;
+ struct CallbackIdsHash callbackIdsHasher;
+
+ std::size_t listenerHash = listenerHasher(listenerCallbacks.transactionCompletedListener);
+ std::size_t callbackIdsHash = callbackIdsHasher(listenerCallbacks.callbackIds);
+
+ return HashCombine(listenerHash, callbackIdsHash);
+ }
+};
+
} // namespace android