Store layer state changes by layer handle in Transaction objects
Layer state changes are stored in an unordered map with the surface control as the key
and state changes as the value. This causes a few problems when merging the transactions. If
transactions contained state changes from a cloned surface control then it will not be merged.
This will cause ordering issues when the transaction is applied since the changes are stored in
and unordered map.
When parcelling transactions, a new surface control is created from the existing one and this
surfaces the problem more frequently.
Instead we store the layer changes by the layer handle which is consistent across processes.
Test: atest SurfaceFlinger_test
Test: go/wm-smoke
Change-Id: I2e041d70ae24db2c1f26ada003532ad97f667167
diff --git a/libs/gui/LayerState.cpp b/libs/gui/LayerState.cpp
index 6066421..a09ceae 100644
--- a/libs/gui/LayerState.cpp
+++ b/libs/gui/LayerState.cpp
@@ -169,12 +169,10 @@
}
status_t ComposerState::write(Parcel& output) const {
- output.writeStrongBinder(IInterface::asBinder(client));
return state.write(output);
}
status_t ComposerState::read(const Parcel& input) {
- client = interface_cast<ISurfaceComposerClient>(input.readStrongBinder());
return state.read(input);
}
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index c59fddf..de36511 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -202,7 +202,8 @@
* callbackIds to generate one super map that contains all the sp<IBinder> to sp<SurfaceControl>
* that could possibly exist for the callbacks.
*/
- std::unordered_map<sp<IBinder>, sp<SurfaceControl>, IBinderHash> surfaceControls;
+ std::unordered_map<sp<IBinder>, sp<SurfaceControl>, SurfaceComposerClient::IBinderHash>
+ surfaceControls;
for (const auto& transactionStats : listenerStats.transactionStats) {
for (auto callbackId : transactionStats.callbackIds) {
auto& [callbackFunction, callbackSurfaceControls] = mCallbacks[callbackId];
@@ -365,16 +366,16 @@
if (count > parcel->dataSize()) {
return BAD_VALUE;
}
- std::unordered_map<sp<SurfaceControl>, ComposerState, SCHash> composerStates;
+ std::unordered_map<sp<IBinder>, ComposerState, IBinderHash> composerStates;
composerStates.reserve(count);
for (size_t i = 0; i < count; i++) {
- sp<SurfaceControl> surfaceControl = SurfaceControl::readFromParcel(parcel);
+ sp<IBinder> surfaceControlHandle = parcel->readStrongBinder();
ComposerState composerState;
if (composerState.read(*parcel) == BAD_VALUE) {
return BAD_VALUE;
}
- composerStates[surfaceControl] = composerState;
+ composerStates[surfaceControlHandle] = composerState;
}
InputWindowCommands inputWindowCommands;
@@ -408,8 +409,8 @@
}
parcel->writeUint32(static_cast<uint32_t>(mComposerStates.size()));
- for (auto const& [surfaceControl, composerState] : mComposerStates) {
- surfaceControl->writeToParcel(parcel);
+ for (auto const& [surfaceHandle, composerState] : mComposerStates) {
+ parcel->writeStrongBinder(surfaceHandle);
composerState.write(*parcel);
}
@@ -418,11 +419,11 @@
}
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::merge(Transaction&& other) {
- for (auto const& kv : other.mComposerStates) {
- if (mComposerStates.count(kv.first) == 0) {
- mComposerStates[kv.first] = kv.second;
+ for (auto const& [surfaceHandle, composerState] : other.mComposerStates) {
+ if (mComposerStates.count(surfaceHandle) == 0) {
+ mComposerStates[surfaceHandle] = composerState;
} else {
- mComposerStates[kv.first].state.merge(kv.second.state);
+ mComposerStates[surfaceHandle].state.merge(composerState.state);
}
}
@@ -465,14 +466,12 @@
mDesiredPresentTime = -1;
}
-void SurfaceComposerClient::doDropReferenceTransaction(const sp<IBinder>& handle,
- const sp<ISurfaceComposerClient>& client) {
+void SurfaceComposerClient::doDropReferenceTransaction(const sp<IBinder>& handle) {
sp<ISurfaceComposer> sf(ComposerService::getComposerService());
Vector<ComposerState> composerStates;
Vector<DisplayState> displayStates;
ComposerState s;
- s.client = client;
s.state.surface = handle;
s.state.what |= layer_state_t::eReparent;
s.state.parentHandleForChild = nullptr;
@@ -499,8 +498,8 @@
}
size_t count = 0;
- for (auto& [sc, cs] : mComposerStates) {
- layer_state_t* s = getLayerState(sc);
+ for (auto& [handle, cs] : mComposerStates) {
+ layer_state_t* s = getLayerState(handle);
if (!(s->what & layer_state_t::eBufferChanged)) {
continue;
}
@@ -639,16 +638,15 @@
mEarlyWakeup = true;
}
-layer_state_t* SurfaceComposerClient::Transaction::getLayerState(const sp<SurfaceControl>& sc) {
- if (mComposerStates.count(sc) == 0) {
+layer_state_t* SurfaceComposerClient::Transaction::getLayerState(const sp<IBinder>& handle) {
+ if (mComposerStates.count(handle) == 0) {
// we don't have it, add an initialized layer_state to our list
ComposerState s;
- s.client = sc->getClient()->mClient;
- s.state.surface = sc->getHandle();
- mComposerStates[sc] = s;
+ s.state.surface = handle;
+ mComposerStates[handle] = s;
}
- return &(mComposerStates[sc].state);
+ return &(mComposerStates[handle].state);
}
void SurfaceComposerClient::Transaction::registerSurfaceControlForCallback(
diff --git a/libs/gui/SurfaceControl.cpp b/libs/gui/SurfaceControl.cpp
index b9defdd..071314f 100644
--- a/libs/gui/SurfaceControl.cpp
+++ b/libs/gui/SurfaceControl.cpp
@@ -65,8 +65,8 @@
{
// Avoid reparenting the server-side surface to null if we are not the owner of it,
// meaning that we retrieved it from another process.
- if (mClient != nullptr && mHandle != nullptr && mOwned) {
- SurfaceComposerClient::doDropReferenceTransaction(mHandle, mClient->getClient());
+ if (mHandle != nullptr && mOwned) {
+ SurfaceComposerClient::doDropReferenceTransaction(mHandle);
}
release();
}
diff --git a/libs/gui/include/gui/LayerState.h b/libs/gui/include/gui/LayerState.h
index f438eb3..cbd1c85 100644
--- a/libs/gui/include/gui/LayerState.h
+++ b/libs/gui/include/gui/LayerState.h
@@ -206,7 +206,6 @@
};
struct ComposerState {
- sp<ISurfaceComposerClient> client;
layer_state_t state;
status_t write(Parcel& output) const;
status_t read(const Parcel& input);
@@ -274,8 +273,6 @@
};
static inline int compare_type(const ComposerState& lhs, const ComposerState& rhs) {
- if (lhs.client < rhs.client) return -1;
- if (lhs.client > rhs.client) return 1;
if (lhs.state.surface < rhs.state.surface) return -1;
if (lhs.state.surface > rhs.state.surface) return 1;
return 0;
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 4dda97f..22ab62d 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -163,8 +163,7 @@
* Called from SurfaceControl d'tor to 'destroy' the surface (or rather, reparent it
* to null), but without needing an sp<SurfaceControl> to avoid infinite ressurection.
*/
- static void doDropReferenceTransaction(const sp<IBinder>& handle,
- const sp<ISurfaceComposerClient>& client);
+ static void doDropReferenceTransaction(const sp<IBinder>& handle);
/**
* Uncaches a buffer in ISurfaceComposer. It must be uncached via a transaction so that it is
@@ -270,6 +269,12 @@
}
};
+ struct IBinderHash {
+ std::size_t operator()(const sp<IBinder>& iBinder) const {
+ return std::hash<IBinder*>{}(iBinder.get());
+ }
+ };
+
struct TCLHash {
std::size_t operator()(const sp<ITransactionCompletedListener>& tcl) const {
return std::hash<IBinder*>{}((tcl) ? IInterface::asBinder(tcl).get() : nullptr);
@@ -286,7 +291,7 @@
};
class Transaction : Parcelable {
- std::unordered_map<sp<SurfaceControl>, ComposerState, SCHash> mComposerStates;
+ std::unordered_map<sp<IBinder>, ComposerState, IBinderHash> mComposerStates;
SortedVector<DisplayState > mDisplayStates;
std::unordered_map<sp<ITransactionCompletedListener>, CallbackInfo, TCLHash>
mListenerCallbacks;
@@ -314,7 +319,10 @@
InputWindowCommands mInputWindowCommands;
int mStatus = NO_ERROR;
- layer_state_t* getLayerState(const sp<SurfaceControl>& sc);
+ layer_state_t* getLayerState(const sp<IBinder>& surfaceHandle);
+ layer_state_t* getLayerState(const sp<SurfaceControl>& sc) {
+ return getLayerState(sc->getHandle());
+ }
DisplayState& getDisplayState(const sp<IBinder>& token);
void cacheBuffers();
@@ -560,15 +568,10 @@
CallbackId mCallbackIdCounter GUARDED_BY(mMutex) = 1;
- struct IBinderHash {
- std::size_t operator()(const sp<IBinder>& iBinder) const {
- return std::hash<IBinder*>{}(iBinder.get());
- }
- };
-
struct CallbackTranslation {
TransactionCompletedCallback callbackFunction;
- std::unordered_map<sp<IBinder>, sp<SurfaceControl>, IBinderHash> surfaceControls;
+ std::unordered_map<sp<IBinder>, sp<SurfaceControl>, SurfaceComposerClient::IBinderHash>
+ surfaceControls;
};
std::unordered_map<CallbackId, CallbackTranslation> mCallbacks GUARDED_BY(mMutex);