Only send surfaces to Listener that registered or applied transaction
We must not leak surface controls to processes that shouldn't know about
them. With this change, we limit the listeners that receive a callback
for a surface control to those that 1) registered the surface control
for callback or 2) received and merged a transaction containing that surface
control to apply
Bug: 139439952
Test: build, boot, IPC_test, SurfaceFlinger_test, libsurfaceflinger_unittest
Change-Id: I4eccc3e72d60729c2f3aa7788db0c5c39fbf46b7
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 02690b0..6576be2 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3100,9 +3100,9 @@
transactions.push_back(transaction);
applyTransactionState(transaction.states, transaction.displays, transaction.flags,
mPendingInputWindowCommands, transaction.desiredPresentTime,
- transaction.buffer, transaction.callback,
- transaction.postTime, transaction.privileged,
- /*isMainThread*/ true);
+ transaction.buffer, transaction.postTime,
+ transaction.privileged, transaction.hasListenerCallbacks,
+ transaction.listenerCallbacks, /*isMainThread*/ true);
transactionQueue.pop();
flushedATransaction = true;
}
@@ -3149,13 +3149,11 @@
return true;
}
-void SurfaceFlinger::setTransactionState(const Vector<ComposerState>& states,
- const Vector<DisplayState>& displays, uint32_t flags,
- const sp<IBinder>& applyToken,
- const InputWindowCommands& inputWindowCommands,
- int64_t desiredPresentTime,
- const client_cache_t& uncacheBuffer,
- const std::vector<ListenerCallbacks>& listenerCallbacks) {
+void SurfaceFlinger::setTransactionState(
+ const Vector<ComposerState>& states, const Vector<DisplayState>& displays, uint32_t flags,
+ const sp<IBinder>& applyToken, const InputWindowCommands& inputWindowCommands,
+ int64_t desiredPresentTime, const client_cache_t& uncacheBuffer, bool hasListenerCallbacks,
+ const std::vector<ListenerCallbacks>& listenerCallbacks) {
ATRACE_CALL();
const int64_t postTime = systemTime();
@@ -3185,24 +3183,23 @@
if (itr != mTransactionQueues.end() || !transactionIsReadyToBeApplied(
desiredPresentTime, false /* useCachedExpectedPresentTime */, states)) {
mTransactionQueues[applyToken].emplace(states, displays, flags, desiredPresentTime,
- uncacheBuffer, listenerCallbacks, postTime,
- privileged);
+ uncacheBuffer, postTime, privileged,
+ hasListenerCallbacks, listenerCallbacks);
setTransactionFlags(eTransactionFlushNeeded);
return;
}
applyTransactionState(states, displays, flags, inputWindowCommands, desiredPresentTime,
- uncacheBuffer, listenerCallbacks, postTime, privileged);
+ uncacheBuffer, postTime, privileged, hasListenerCallbacks,
+ listenerCallbacks);
}
-void SurfaceFlinger::applyTransactionState(const Vector<ComposerState>& states,
- const Vector<DisplayState>& displays, uint32_t flags,
- const InputWindowCommands& inputWindowCommands,
- const int64_t desiredPresentTime,
- const client_cache_t& uncacheBuffer,
- const std::vector<ListenerCallbacks>& listenerCallbacks,
- const int64_t postTime, bool privileged,
- bool isMainThread) {
+void SurfaceFlinger::applyTransactionState(
+ const Vector<ComposerState>& states, const Vector<DisplayState>& displays, uint32_t flags,
+ const InputWindowCommands& inputWindowCommands, const int64_t desiredPresentTime,
+ const client_cache_t& uncacheBuffer, const int64_t postTime, bool privileged,
+ bool hasListenerCallbacks, const std::vector<ListenerCallbacks>& listenerCallbacks,
+ bool isMainThread) {
uint32_t transactionFlags = 0;
if (flags & eAnimation) {
@@ -3225,28 +3222,27 @@
transactionFlags |= setDisplayStateLocked(display);
}
- // In case the client has sent a Transaction that should receive callbacks but without any
- // SurfaceControls that should be included in the callback, send the listener and callbackIds
- // to the callback thread so it can send an empty callback
- if (!listenerCallbacks.empty()) {
- mTransactionCompletedThread.run();
- }
- for (const auto& listenerCallback : listenerCallbacks) {
- mTransactionCompletedThread.startRegistration(listenerCallback);
+ // start and end registration for listeners w/ no surface so they can get their callback. Note
+ // that listeners with SurfaceControls will start registration during setClientStateLocked
+ // below.
+ for (const auto& listener : listenerCallbacks) {
+ mTransactionCompletedThread.startRegistration(listener);
+ mTransactionCompletedThread.endRegistration(listener);
}
+ std::unordered_set<ListenerCallbacks, ListenerCallbacksHash> listenerCallbacksWithSurfaces;
uint32_t clientStateFlags = 0;
for (const ComposerState& state : states) {
- clientStateFlags |= setClientStateLocked(state, desiredPresentTime, listenerCallbacks,
- postTime, privileged);
+ clientStateFlags |= setClientStateLocked(state, desiredPresentTime, postTime, privileged,
+ listenerCallbacksWithSurfaces);
}
- for (const auto& listenerCallback : listenerCallbacks) {
+ for (const auto& listenerCallback : listenerCallbacksWithSurfaces) {
mTransactionCompletedThread.endRegistration(listenerCallback);
}
// If the state doesn't require a traversal and there are callbacks, send them now
- if (!(clientStateFlags & eTraversalNeeded) && !listenerCallbacks.empty()) {
+ if (!(clientStateFlags & eTraversalNeeded) && hasListenerCallbacks) {
mTransactionCompletedThread.sendCallbacks();
}
transactionFlags |= clientStateFlags;
@@ -3374,17 +3370,23 @@
}
uint32_t SurfaceFlinger::setClientStateLocked(
- const ComposerState& composerState, int64_t desiredPresentTime,
- const std::vector<ListenerCallbacks>& listenerCallbacks, int64_t postTime,
- bool privileged) {
+ const ComposerState& composerState, int64_t desiredPresentTime, int64_t postTime,
+ bool privileged,
+ std::unordered_set<ListenerCallbacks, ListenerCallbacksHash>& listenerCallbacks) {
const layer_state_t& s = composerState.state;
+ for (auto& listener : s.listeners) {
+ // note that startRegistration will not re-register if the listener has
+ // already be registered for a prior surface control
+ mTransactionCompletedThread.startRegistration(listener);
+ listenerCallbacks.insert(listener);
+ }
+
sp<Layer> layer(fromHandle(s.surface));
if (layer == nullptr) {
- for (auto& listenerCallback : listenerCallbacks) {
+ for (auto& [listener, callbackIds] : s.listeners) {
mTransactionCompletedThread.registerUnpresentedCallbackHandle(
- new CallbackHandle(listenerCallback.transactionCompletedListener,
- listenerCallback.callbackIds, s.surface));
+ new CallbackHandle(listener, callbackIds, s.surface));
}
return 0;
}
@@ -3607,8 +3609,8 @@
}
}
std::vector<sp<CallbackHandle>> callbackHandles;
- if ((what & layer_state_t::eHasListenerCallbacksChanged) && (!listenerCallbacks.empty())) {
- for (const auto& [listener, callbackIds] : listenerCallbacks) {
+ if ((what & layer_state_t::eHasListenerCallbacksChanged) && (!s.listeners.empty())) {
+ for (auto& [listener, callbackIds] : s.listeners) {
callbackHandles.emplace_back(new CallbackHandle(listener, callbackIds, s.surface));
}
}
@@ -3890,7 +3892,8 @@
d.width = 0;
d.height = 0;
displays.add(d);
- setTransactionState(state, displays, 0, nullptr, mPendingInputWindowCommands, -1, {}, {});
+ setTransactionState(state, displays, 0, nullptr, mPendingInputWindowCommands, -1, {}, false,
+ {});
setPowerModeInternal(display, HWC_POWER_MODE_NORMAL);