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/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index dc161b7..5805797 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -69,7 +69,7 @@
const sp<IBinder>& applyToken,
const InputWindowCommands& commands,
int64_t desiredPresentTime,
- const client_cache_t& uncacheBuffer,
+ const client_cache_t& uncacheBuffer, bool hasListenerCallbacks,
const std::vector<ListenerCallbacks>& listenerCallbacks) {
Parcel data, reply;
data.writeInterfaceToken(ISurfaceComposer::getInterfaceDescriptor());
@@ -90,6 +90,7 @@
data.writeInt64(desiredPresentTime);
data.writeStrongBinder(uncacheBuffer.token.promote());
data.writeUint64(uncacheBuffer.id);
+ data.writeBool(hasListenerCallbacks);
if (data.writeVectorSize(listenerCallbacks) == NO_ERROR) {
for (const auto& [listener, callbackIds] : listenerCallbacks) {
@@ -1039,6 +1040,8 @@
uncachedBuffer.token = data.readStrongBinder();
uncachedBuffer.id = data.readUint64();
+ bool hasListenerCallbacks = data.readBool();
+
std::vector<ListenerCallbacks> listenerCallbacks;
int32_t listenersSize = data.readInt32();
for (int32_t i = 0; i < listenersSize; i++) {
@@ -1047,9 +1050,9 @@
data.readInt64Vector(&callbackIds);
listenerCallbacks.emplace_back(listener, callbackIds);
}
-
setTransactionState(state, displays, stateFlags, applyToken, inputWindowCommands,
- desiredPresentTime, uncachedBuffer, listenerCallbacks);
+ desiredPresentTime, uncachedBuffer, hasListenerCallbacks,
+ listenerCallbacks);
return NO_ERROR;
}
case BOOT_FINISHED: {
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index 523ed1d..e004e95 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -86,7 +86,6 @@
memcpy(output.writeInplace(16 * sizeof(float)),
colorTransform.asArray(), 16 * sizeof(float));
output.writeFloat(cornerRadius);
- output.writeBool(hasListenerCallbacks);
output.writeStrongBinder(cachedBuffer.token.promote());
output.writeUint64(cachedBuffer.id);
output.writeParcelable(metadata);
@@ -95,6 +94,22 @@
output.writeUint32(static_cast<uint32_t>(bgColorDataspace));
output.writeBool(colorSpaceAgnostic);
+ auto err = output.writeVectorSize(listeners);
+ if (err) {
+ return err;
+ }
+
+ for (auto listener : listeners) {
+ err = output.writeStrongBinder(listener.transactionCompletedListener);
+ if (err) {
+ return err;
+ }
+ err = output.writeInt64Vector(listener.callbackIds);
+ if (err) {
+ return err;
+ }
+ }
+
return NO_ERROR;
}
@@ -156,7 +171,6 @@
colorTransform = mat4(static_cast<const float*>(input.readInplace(16 * sizeof(float))));
cornerRadius = input.readFloat();
- hasListenerCallbacks = input.readBool();
cachedBuffer.token = input.readStrongBinder();
cachedBuffer.id = input.readUint64();
input.readParcelable(&metadata);
@@ -165,6 +179,14 @@
bgColorDataspace = static_cast<ui::Dataspace>(input.readUint32());
colorSpaceAgnostic = input.readBool();
+ int32_t numListeners = input.readInt32();
+ listeners.clear();
+ for (int i = 0; i < numListeners; i++) {
+ auto listener = input.readStrongBinder();
+ std::vector<CallbackId> callbackIds;
+ input.readInt64Vector(&callbackIds);
+ listeners.emplace_back(listener, callbackIds);
+ }
return NO_ERROR;
}
@@ -361,7 +383,6 @@
}
if (other.what & eHasListenerCallbacksChanged) {
what |= eHasListenerCallbacksChanged;
- hasListenerCallbacks = other.hasListenerCallbacks;
}
#ifndef NO_INPUT
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 67dd726..6b5021d 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -366,6 +366,33 @@
if (count > parcel->dataSize()) {
return BAD_VALUE;
}
+ std::unordered_map<sp<ITransactionCompletedListener>, CallbackInfo, TCLHash> listenerCallbacks;
+ listenerCallbacks.reserve(count);
+ for (size_t i = 0; i < count; i++) {
+ sp<ITransactionCompletedListener> listener =
+ interface_cast<ITransactionCompletedListener>(parcel->readStrongBinder());
+ size_t numCallbackIds = parcel->readUint32();
+ if (numCallbackIds > parcel->dataSize()) {
+ return BAD_VALUE;
+ }
+ for (size_t j = 0; j < numCallbackIds; j++) {
+ listenerCallbacks[listener].callbackIds.insert(parcel->readInt64());
+ }
+ size_t numSurfaces = parcel->readUint32();
+ if (numSurfaces > parcel->dataSize()) {
+ return BAD_VALUE;
+ }
+ for (size_t j = 0; j < numSurfaces; j++) {
+ sp<SurfaceControl> surface;
+ surface = SurfaceControl::readFromParcel(parcel);
+ listenerCallbacks[listener].surfaceControls.insert(surface);
+ }
+ }
+
+ count = static_cast<size_t>(parcel->readUint32());
+ if (count > parcel->dataSize()) {
+ return BAD_VALUE;
+ }
std::unordered_map<sp<IBinder>, ComposerState, IBinderHash> composerStates;
composerStates.reserve(count);
for (size_t i = 0; i < count; i++) {
@@ -389,10 +416,9 @@
mContainsBuffer = containsBuffer;
mDesiredPresentTime = desiredPresentTime;
mDisplayStates = displayStates;
+ mListenerCallbacks = listenerCallbacks;
mComposerStates = composerStates;
mInputWindowCommands = inputWindowCommands;
- // listener callbacks contain function pointer addresses and may not be safe to parcel.
- mListenerCallbacks.clear();
return NO_ERROR;
}
@@ -408,6 +434,19 @@
displayState.write(*parcel);
}
+ parcel->writeUint32(static_cast<uint32_t>(mListenerCallbacks.size()));
+ for (auto const& [listener, callbackInfo] : mListenerCallbacks) {
+ parcel->writeStrongBinder(ITransactionCompletedListener::asBinder(listener));
+ parcel->writeUint32(static_cast<uint32_t>(callbackInfo.callbackIds.size()));
+ for (auto callbackId : callbackInfo.callbackIds) {
+ parcel->writeInt64(callbackId);
+ }
+ parcel->writeUint32(static_cast<uint32_t>(callbackInfo.surfaceControls.size()));
+ for (auto surfaceControl : callbackInfo.surfaceControls) {
+ surfaceControl->writeToParcel(parcel);
+ }
+ }
+
parcel->writeUint32(static_cast<uint32_t>(mComposerStates.size()));
for (auto const& [surfaceHandle, composerState] : mComposerStates) {
parcel->writeStrongBinder(surfaceHandle);
@@ -441,6 +480,11 @@
mListenerCallbacks[listener].callbackIds.insert(std::make_move_iterator(
callbackIds.begin()),
std::make_move_iterator(callbackIds.end()));
+ // register surface controls for this listener that is merging
+ for (const auto& surfaceControl : surfaceControls) {
+ registerSurfaceControlForCallback(surfaceControl);
+ }
+
mListenerCallbacks[listener]
.surfaceControls.insert(std::make_move_iterator(surfaceControls.begin()),
std::make_move_iterator(surfaceControls.end()));
@@ -479,7 +523,7 @@
composerStates.add(s);
sp<IBinder> applyToken = IInterface::asBinder(TransactionCompletedListener::getIInstance());
- sf->setTransactionState(composerStates, displayStates, 0, applyToken, {}, -1, {}, {});
+ sf->setTransactionState(composerStates, displayStates, 0, applyToken, {}, -1, {}, false, {});
}
void SurfaceComposerClient::doUncacheBufferTransaction(uint64_t cacheId) {
@@ -490,7 +534,7 @@
uncacheBuffer.id = cacheId;
sp<IBinder> applyToken = IInterface::asBinder(TransactionCompletedListener::getIInstance());
- sf->setTransactionState({}, {}, 0, applyToken, {}, -1, uncacheBuffer, {});
+ sf->setTransactionState({}, {}, 0, applyToken, {}, -1, uncacheBuffer, false, {});
}
void SurfaceComposerClient::Transaction::cacheBuffers() {
@@ -539,8 +583,8 @@
sp<ISurfaceComposer> sf(ComposerService::getComposerService());
+ bool hasListenerCallbacks = !mListenerCallbacks.empty();
std::vector<ListenerCallbacks> listenerCallbacks;
-
// For every listener with registered callbacks
for (const auto& [listener, callbackInfo] : mListenerCallbacks) {
auto& [callbackIds, surfaceControls] = callbackInfo;
@@ -548,19 +592,24 @@
continue;
}
- 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) {
- layer_state_t* s = getLayerState(surfaceControl);
- if (!s) {
- ALOGE("failed to get layer state");
- continue;
+ if (surfaceControls.empty()) {
+ listenerCallbacks.emplace_back(IInterface::asBinder(listener), std::move(callbackIds));
+ } else {
+ // If the listener has any SurfaceControls set on this Transaction update the surface
+ // state
+ for (const auto& surfaceControl : surfaceControls) {
+ layer_state_t* s = getLayerState(surfaceControl);
+ if (!s) {
+ ALOGE("failed to get layer state");
+ continue;
+ }
+ std::vector<CallbackId> callbacks(callbackIds.begin(), callbackIds.end());
+ s->what |= layer_state_t::eHasListenerCallbacksChanged;
+ s->listeners.emplace_back(IInterface::asBinder(listener), callbacks);
}
- s->what |= layer_state_t::eHasListenerCallbacksChanged;
- s->hasListenerCallbacks = true;
}
}
+
mListenerCallbacks.clear();
cacheBuffers();
@@ -598,7 +647,7 @@
sf->setTransactionState(composerStates, displayStates, flags, applyToken, mInputWindowCommands,
mDesiredPresentTime,
{} /*uncacheBuffer - only set in doUncacheBufferTransaction*/,
- listenerCallbacks);
+ hasListenerCallbacks, listenerCallbacks);
mInputWindowCommands.clear();
mStatus = NO_ERROR;
return NO_ERROR;
diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h
index c84910b..06be1b3 100644
--- a/libs/gui/include/gui/ISurfaceComposer.h
+++ b/libs/gui/include/gui/ISurfaceComposer.h
@@ -140,7 +140,7 @@
const sp<IBinder>& applyToken,
const InputWindowCommands& inputWindowCommands,
int64_t desiredPresentTime,
- const client_cache_t& uncacheBuffer,
+ const client_cache_t& uncacheBuffer, bool hasListenerCallbacks,
const std::vector<ListenerCallbacks>& listenerCallbacks) = 0;
/* signal that we're done booting.
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index 2eb5492..a49ed52 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -23,6 +23,7 @@
#include <utils/Errors.h>
#include <gui/IGraphicBufferProducer.h>
+#include <gui/ITransactionCompletedListener.h>
#include <math/mat4.h>
#ifndef NO_INPUT
@@ -123,7 +124,6 @@
surfaceDamageRegion(),
api(-1),
colorTransform(mat4()),
- hasListenerCallbacks(false),
bgColorAlpha(0),
bgColorDataspace(ui::Dataspace::UNKNOWN),
colorSpaceAgnostic(false) {
@@ -186,7 +186,6 @@
sp<NativeHandle> sidebandStream;
mat4 colorTransform;
- bool hasListenerCallbacks;
#ifndef NO_INPUT
InputWindowInfo inputInfo;
#endif
@@ -203,6 +202,8 @@
// A color space agnostic layer means the color of this layer can be
// interpreted in any color space.
bool colorSpaceAgnostic;
+
+ std::vector<ListenerCallbacks> listeners;
};
struct ComposerState {
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 8a6920c..15287e2 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -291,6 +291,7 @@
};
class Transaction : public Parcelable {
+ protected:
std::unordered_map<sp<IBinder>, ComposerState, IBinderHash> mComposerStates;
SortedVector<DisplayState > mDisplayStates;
std::unordered_map<sp<ITransactionCompletedListener>, CallbackInfo, TCLHash>
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index 5a121d7..d75b1ca 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -688,6 +688,7 @@
const sp<IBinder>& /*applyToken*/,
const InputWindowCommands& /*inputWindowCommands*/,
int64_t /*desiredPresentTime*/, const client_cache_t& /*cachedBuffer*/,
+ bool /*hasListenerCallbacks*/,
const std::vector<ListenerCallbacks>& /*listenerCallbacks*/) override {
}
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);
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index f220c26..3467de2 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -400,6 +400,7 @@
const sp<IBinder>& applyToken,
const InputWindowCommands& inputWindowCommands,
int64_t desiredPresentTime, const client_cache_t& uncacheBuffer,
+ bool hasListenerCallbacks,
const std::vector<ListenerCallbacks>& listenerCallbacks) override;
void bootFinished() override;
bool authenticateSurfaceTexture(
@@ -568,10 +569,10 @@
const Vector<DisplayState>& displays, uint32_t flags,
const InputWindowCommands& inputWindowCommands,
const int64_t desiredPresentTime,
- const client_cache_t& uncacheBuffer,
+ const client_cache_t& uncacheBuffer, const int64_t postTime,
+ bool privileged, bool hasListenerCallbacks,
const std::vector<ListenerCallbacks>& listenerCallbacks,
- const int64_t postTime, bool privileged, bool isMainThread = false)
- REQUIRES(mStateLock);
+ bool isMainThread = false) REQUIRES(mStateLock);
// Returns true if at least one transaction was flushed
bool flushTransactionQueues();
// Returns true if there is at least one transaction that needs to be flushed
@@ -586,9 +587,11 @@
bool transactionIsReadyToBeApplied(int64_t desiredPresentTime,
bool useCachedExpectedPresentTime,
const Vector<ComposerState>& states);
- uint32_t setClientStateLocked(const ComposerState& composerState, int64_t desiredPresentTime,
- const std::vector<ListenerCallbacks>& listenerCallbacks,
- int64_t postTime, bool privileged) REQUIRES(mStateLock);
+ uint32_t setClientStateLocked(
+ const ComposerState& composerState, int64_t desiredPresentTime, int64_t postTime,
+ bool privileged,
+ std::unordered_set<ListenerCallbacks, ListenerCallbacksHash>& listenerCallbacks)
+ REQUIRES(mStateLock);
uint32_t setDisplayStateLocked(const DisplayState& s) REQUIRES(mStateLock);
uint32_t addInputWindowCommands(const InputWindowCommands& inputWindowCommands)
REQUIRES(mStateLock);
@@ -1024,25 +1027,27 @@
TransactionState(const Vector<ComposerState>& composerStates,
const Vector<DisplayState>& displayStates, uint32_t transactionFlags,
int64_t desiredPresentTime, const client_cache_t& uncacheBuffer,
- const std::vector<ListenerCallbacks>& listenerCallbacks, int64_t postTime,
- bool privileged)
+ int64_t postTime, bool privileged, bool hasListenerCallbacks,
+ std::vector<ListenerCallbacks> listenerCallbacks)
: states(composerStates),
displays(displayStates),
flags(transactionFlags),
desiredPresentTime(desiredPresentTime),
buffer(uncacheBuffer),
- callback(listenerCallbacks),
postTime(postTime),
- privileged(privileged) {}
+ privileged(privileged),
+ hasListenerCallbacks(hasListenerCallbacks),
+ listenerCallbacks(listenerCallbacks) {}
Vector<ComposerState> states;
Vector<DisplayState> displays;
uint32_t flags;
const int64_t desiredPresentTime;
client_cache_t buffer;
- std::vector<ListenerCallbacks> callback;
const int64_t postTime;
bool privileged;
+ bool hasListenerCallbacks;
+ std::vector<ListenerCallbacks> listenerCallbacks;
};
std::unordered_map<sp<IBinder>, std::queue<TransactionState>, IListenerHash> mTransactionQueues;
diff --git a/services/surfaceflinger/TransactionCompletedThread.cpp b/services/surfaceflinger/TransactionCompletedThread.cpp
index 1324f20..92e59d7 100644
--- a/services/surfaceflinger/TransactionCompletedThread.cpp
+++ b/services/surfaceflinger/TransactionCompletedThread.cpp
@@ -75,27 +75,29 @@
}
status_t TransactionCompletedThread::startRegistration(const ListenerCallbacks& listenerCallbacks) {
+ // begin running if not already running
+ run();
std::lock_guard lock(mMutex);
if (!mRunning) {
ALOGE("cannot add callback because the callback thread isn't running");
return BAD_VALUE;
}
+ auto [itr, inserted] = mRegisteringTransactions.insert(listenerCallbacks);
auto& [listener, callbackIds] = listenerCallbacks;
- 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;
+ 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);
}
- mRegisteringTransactions.insert(listenerCallbacks);
-
- auto& transactionStatsDeque = mCompletedTransactions[listener];
- transactionStatsDeque.emplace_back(callbackIds);
-
return NO_ERROR;
}
diff --git a/services/surfaceflinger/tests/Android.bp b/services/surfaceflinger/tests/Android.bp
index 159c2a4..7f960f3 100644
--- a/services/surfaceflinger/tests/Android.bp
+++ b/services/surfaceflinger/tests/Android.bp
@@ -43,7 +43,48 @@
"libui",
"libutils",
]
+}
+cc_defaults {
+ name: "ipc_defaults",
+ cflags: [
+ "-Wall",
+ "-Werror",
+ ],
+}
+
+cc_test {
+ name: "IPC_test",
+ defaults: ["ipc_defaults"],
+ test_suites: ["device-tests"],
+ srcs: [
+ "BufferGenerator.cpp",
+ "IPC_test.cpp",
+ ],
+ cppflags: [
+ "-Wall",
+ "-Werror",
+ "-Wformat",
+ "-Wthread-safety",
+ "-Wunused",
+ "-Wunreachable-code",
+ ],
+ shared_libs: [
+ "libandroid",
+ "libbinder",
+ "libcutils",
+ "libEGL",
+ "libGLESv2",
+ "libgui",
+ "liblayers_proto",
+ "liblog",
+ "libprotobuf-cpp-full",
+ "libtimestats_proto",
+ "libui",
+ "libutils",
+ ],
+ cpp_std: "experimental",
+ gnu_extensions: false,
}
subdirs = [
diff --git a/services/surfaceflinger/tests/IPC_test.cpp b/services/surfaceflinger/tests/IPC_test.cpp
new file mode 100644
index 0000000..8a756a6
--- /dev/null
+++ b/services/surfaceflinger/tests/IPC_test.cpp
@@ -0,0 +1,351 @@
+/*
+ * Copyright (C) 2019 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include <gtest/gtest.h>
+
+#include <binder/IInterface.h>
+#include <binder/IPCThreadState.h>
+#include <binder/IServiceManager.h>
+#include <binder/ProcessState.h>
+
+#include <gui/ISurfaceComposer.h>
+#include <gui/LayerState.h>
+#include <gui/Surface.h>
+#include <gui/SurfaceComposerClient.h>
+
+#include <limits>
+
+#include <ui/DisplayInfo.h>
+
+#include <utils/String8.h>
+
+#include "BufferGenerator.h"
+#include "utils/CallbackUtils.h"
+#include "utils/ColorUtils.h"
+#include "utils/TransactionUtils.h"
+
+namespace android {
+
+namespace test {
+
+using Transaction = SurfaceComposerClient::Transaction;
+using CallbackInfo = SurfaceComposerClient::CallbackInfo;
+using TCLHash = SurfaceComposerClient::TCLHash;
+using android::hardware::graphics::common::V1_1::BufferUsage;
+
+class TransactionHelper : public Transaction {
+public:
+ size_t getNumListeners() { return mListenerCallbacks.size(); }
+
+ std::unordered_map<sp<ITransactionCompletedListener>, CallbackInfo, TCLHash>
+ getListenerCallbacks() {
+ return mListenerCallbacks;
+ }
+};
+
+class IPCTestUtils {
+public:
+ static void waitForCallback(CallbackHelper& helper, const ExpectedResult& expectedResult,
+ bool finalState = false);
+ static status_t getBuffer(sp<GraphicBuffer>* outBuffer, sp<Fence>* outFence);
+};
+
+class IIPCTest : public IInterface {
+public:
+ DECLARE_META_INTERFACE(IPCTest)
+ enum class Tag : uint32_t {
+ SetDeathToken = IBinder::FIRST_CALL_TRANSACTION,
+ InitClient,
+ CreateTransaction,
+ MergeAndApply,
+ VerifyCallbacks,
+ CleanUp,
+ Last,
+ };
+
+ virtual status_t setDeathToken(sp<IBinder>& token) = 0;
+
+ virtual status_t initClient() = 0;
+
+ virtual status_t createTransaction(TransactionHelper* outTransaction, uint32_t width,
+ uint32_t height) = 0;
+
+ virtual status_t mergeAndApply(TransactionHelper transaction) = 0;
+
+ virtual status_t verifyCallbacks() = 0;
+
+ virtual status_t cleanUp() = 0;
+};
+
+class BpIPCTest : public SafeBpInterface<IIPCTest> {
+public:
+ explicit BpIPCTest(const sp<IBinder>& impl) : SafeBpInterface<IIPCTest>(impl, "BpIPCTest") {}
+
+ status_t setDeathToken(sp<IBinder>& token) {
+ return callRemote<decltype(&IIPCTest::setDeathToken)>(Tag::SetDeathToken, token);
+ }
+
+ status_t initClient() { return callRemote<decltype(&IIPCTest::initClient)>(Tag::InitClient); }
+
+ status_t createTransaction(TransactionHelper* transaction, uint32_t width, uint32_t height) {
+ return callRemote<decltype(&IIPCTest::createTransaction)>(Tag::CreateTransaction,
+ transaction, width, height);
+ }
+
+ status_t mergeAndApply(TransactionHelper transaction) {
+ return callRemote<decltype(&IIPCTest::mergeAndApply)>(Tag::MergeAndApply, transaction);
+ }
+
+ status_t verifyCallbacks() {
+ return callRemote<decltype(&IIPCTest::verifyCallbacks)>(Tag::VerifyCallbacks);
+ }
+
+ status_t cleanUp() { return callRemote<decltype(&IIPCTest::cleanUp)>(Tag::CleanUp); }
+};
+
+IMPLEMENT_META_INTERFACE(IPCTest, "android.gfx.tests.IIPCTest")
+
+class onTestDeath : public IBinder::DeathRecipient {
+public:
+ void binderDied(const wp<IBinder>& /*who*/) override {
+ ALOGE("onTestDeath::binderDied, exiting");
+ exit(0);
+ }
+};
+
+sp<onTestDeath> getDeathToken() {
+ static sp<onTestDeath> token = new onTestDeath;
+ return token;
+}
+
+class BnIPCTest : public SafeBnInterface<IIPCTest> {
+public:
+ BnIPCTest() : SafeBnInterface("BnIPCTest") {}
+
+ status_t setDeathToken(sp<IBinder>& token) override {
+ return token->linkToDeath(getDeathToken());
+ }
+
+ status_t initClient() override {
+ mClient = new SurfaceComposerClient;
+ auto err = mClient->initCheck();
+ return err;
+ }
+
+ status_t createTransaction(TransactionHelper* transaction, uint32_t width, uint32_t height) {
+ if (transaction == nullptr) {
+ ALOGE("Error in createTransaction: transaction is nullptr");
+ return BAD_VALUE;
+ }
+ mSurfaceControl = mClient->createSurface(String8("parentProcessSurface"), 0, 0,
+ PIXEL_FORMAT_RGBA_8888,
+ ISurfaceComposerClient::eFXSurfaceBufferState,
+ /*parent*/ nullptr);
+ sp<GraphicBuffer> gb;
+ sp<Fence> fence;
+ int err = IPCTestUtils::getBuffer(&gb, &fence);
+ if (err != NO_ERROR) return err;
+
+ TransactionUtils::fillGraphicBufferColor(gb,
+ {0, 0, static_cast<int32_t>(width),
+ static_cast<int32_t>(height)},
+ Color::RED);
+ transaction->setLayerStack(mSurfaceControl, 0)
+ .setLayer(mSurfaceControl, std::numeric_limits<int32_t>::max())
+ .setFrame(mSurfaceControl, Rect(0, 0, width, height))
+ .setBuffer(mSurfaceControl, gb)
+ .setAcquireFence(mSurfaceControl, fence)
+ .show(mSurfaceControl)
+ .addTransactionCompletedCallback(mCallbackHelper.function,
+ mCallbackHelper.getContext());
+ return NO_ERROR;
+ }
+
+ status_t mergeAndApply(TransactionHelper /*transaction*/) {
+ // transaction.apply();
+ return NO_ERROR;
+ }
+
+ status_t verifyCallbacks() {
+ ExpectedResult expected;
+ expected.addSurface(ExpectedResult::Transaction::PRESENTED, mSurfaceControl);
+ EXPECT_NO_FATAL_FAILURE(IPCTestUtils::waitForCallback(mCallbackHelper, expected, true));
+ return NO_ERROR;
+ }
+
+ status_t cleanUp() {
+ if (mClient) mClient->dispose();
+ mSurfaceControl = nullptr;
+ IPCThreadState::self()->stopProcess();
+ return NO_ERROR;
+ }
+
+ status_t onTransact(uint32_t code, const Parcel& data, Parcel* reply,
+ uint32_t /*flags*/) override {
+ EXPECT_GE(code, IBinder::FIRST_CALL_TRANSACTION);
+ EXPECT_LT(code, static_cast<uint32_t>(IIPCTest::Tag::Last));
+ switch (static_cast<IIPCTest::Tag>(code)) {
+ case IIPCTest::Tag::SetDeathToken:
+ return callLocal(data, reply, &IIPCTest::setDeathToken);
+ case IIPCTest::Tag::InitClient:
+ return callLocal(data, reply, &IIPCTest::initClient);
+ case IIPCTest::Tag::CreateTransaction:
+ return callLocal(data, reply, &IIPCTest::createTransaction);
+ case IIPCTest::Tag::MergeAndApply:
+ return callLocal(data, reply, &IIPCTest::mergeAndApply);
+ case IIPCTest::Tag::VerifyCallbacks:
+ return callLocal(data, reply, &IIPCTest::verifyCallbacks);
+ case IIPCTest::Tag::CleanUp:
+ return callLocal(data, reply, &IIPCTest::cleanUp);
+ default:
+ return UNKNOWN_ERROR;
+ }
+ }
+
+private:
+ sp<SurfaceComposerClient> mClient;
+ sp<SurfaceControl> mSurfaceControl;
+ CallbackHelper mCallbackHelper;
+};
+
+class IPCTest : public ::testing::Test {
+public:
+ IPCTest() : mDeathRecipient(new BBinder), mRemote(initRemoteService()) {
+ ProcessState::self()->startThreadPool();
+ }
+ void SetUp() {
+ mClient = new SurfaceComposerClient;
+ ASSERT_EQ(NO_ERROR, mClient->initCheck());
+
+ mPrimaryDisplay = mClient->getInternalDisplayToken();
+ DisplayInfo info;
+ mClient->getDisplayInfo(mPrimaryDisplay, &info);
+ mDisplayWidth = info.w;
+ mDisplayHeight = info.h;
+
+ Transaction setupTransaction;
+ setupTransaction.setDisplayLayerStack(mPrimaryDisplay, 0);
+ setupTransaction.apply();
+ }
+
+protected:
+ sp<IIPCTest> initRemoteService();
+
+ sp<IBinder> mDeathRecipient;
+ sp<IIPCTest> mRemote;
+ sp<SurfaceComposerClient> mClient;
+ sp<IBinder> mPrimaryDisplay;
+ uint32_t mDisplayWidth;
+ uint32_t mDisplayHeight;
+ sp<SurfaceControl> sc;
+};
+
+status_t IPCTestUtils::getBuffer(sp<GraphicBuffer>* outBuffer, sp<Fence>* outFence) {
+ static BufferGenerator bufferGenerator;
+ return bufferGenerator.get(outBuffer, outFence);
+}
+
+void IPCTestUtils::waitForCallback(CallbackHelper& helper, const ExpectedResult& expectedResult,
+ bool finalState) {
+ CallbackData callbackData;
+ ASSERT_NO_FATAL_FAILURE(helper.getCallbackData(&callbackData));
+ EXPECT_NO_FATAL_FAILURE(expectedResult.verifyCallbackData(callbackData));
+
+ if (finalState) {
+ ASSERT_NO_FATAL_FAILURE(helper.verifyFinalState());
+ }
+}
+
+sp<IIPCTest> IPCTest::initRemoteService() {
+ static std::mutex mMutex;
+ static sp<IIPCTest> remote;
+ const String16 serviceName("IPCTest");
+
+ std::unique_lock<decltype(mMutex)> lock;
+ if (remote == nullptr) {
+ pid_t forkPid = fork();
+ EXPECT_NE(forkPid, -1);
+
+ if (forkPid == 0) {
+ sp<IIPCTest> nativeService = new BnIPCTest;
+ if (!nativeService) {
+ ALOGE("null service...");
+ }
+ status_t err = defaultServiceManager()->addService(serviceName,
+ IInterface::asBinder(nativeService));
+ if (err != NO_ERROR) {
+ ALOGE("failed to add service: %d", err);
+ }
+ ProcessState::self()->startThreadPool();
+ IPCThreadState::self()->joinThreadPool();
+ [&]() { exit(0); }();
+ }
+ sp<IBinder> binder = defaultServiceManager()->getService(serviceName);
+ remote = interface_cast<IIPCTest>(binder);
+ remote->setDeathToken(mDeathRecipient);
+ }
+ return remote;
+}
+
+TEST_F(IPCTest, MergeBasic) {
+ CallbackHelper helper1;
+ sc = mClient->createSurface(String8("parentProcessSurface"), 0, 0, PIXEL_FORMAT_RGBA_8888,
+ ISurfaceComposerClient::eFXSurfaceBufferState,
+ /*parent*/ nullptr);
+ sp<GraphicBuffer> gb;
+ sp<Fence> fence;
+ int err = IPCTestUtils::getBuffer(&gb, &fence);
+ ASSERT_EQ(NO_ERROR, err);
+ TransactionUtils::fillGraphicBufferColor(gb,
+ {0, 0, static_cast<int32_t>(mDisplayWidth),
+ static_cast<int32_t>(mDisplayHeight)},
+ Color::RED);
+
+ Transaction transaction;
+ transaction.setLayerStack(sc, 0)
+ .setLayer(sc, std::numeric_limits<int32_t>::max() - 1)
+ .setBuffer(sc, gb)
+ .setAcquireFence(sc, fence)
+ .show(sc)
+ .addTransactionCompletedCallback(helper1.function, helper1.getContext());
+
+ TransactionHelper remote;
+ mRemote->initClient();
+ mRemote->createTransaction(&remote, mDisplayWidth / 2, mDisplayHeight / 2);
+ ASSERT_EQ(1, remote.getNumListeners());
+ auto remoteListenerCallbacks = remote.getListenerCallbacks();
+ auto remoteCallback = remoteListenerCallbacks.begin();
+ auto remoteCallbackInfo = remoteCallback->second;
+ auto remoteListenerScs = remoteCallbackInfo.surfaceControls;
+ ASSERT_EQ(1, remoteCallbackInfo.callbackIds.size());
+ ASSERT_EQ(1, remoteListenerScs.size());
+
+ sp<SurfaceControl> remoteSc = *(remoteListenerScs.begin());
+ transaction.merge(std::move(remote));
+ transaction.apply();
+
+ sleep(1);
+ ExpectedResult expected;
+ expected.addSurface(ExpectedResult::Transaction::PRESENTED, sc);
+ expected.addSurface(ExpectedResult::Transaction::PRESENTED, remoteSc);
+ EXPECT_NO_FATAL_FAILURE(IPCTestUtils::waitForCallback(helper1, expected, true));
+
+ mRemote->verifyCallbacks();
+ mRemote->cleanUp();
+}
+
+} // namespace test
+} // namespace android