SurfaceFlinger: Remove removeLayer
We remove explicit layer destruction and replace it
with reparent->null, completing the transition to
a reference counted model.
Test: Manual
Bug: 62536731
Bug: 111373437
Bug: 111297488
Change-Id: I8ac7c5c5125e1c8daf84b42db00e1dd93a544bb5
diff --git a/libs/gui/ISurfaceComposerClient.cpp b/libs/gui/ISurfaceComposerClient.cpp
index a6890ee..369f523 100644
--- a/libs/gui/ISurfaceComposerClient.cpp
+++ b/libs/gui/ISurfaceComposerClient.cpp
@@ -31,7 +31,6 @@
enum class Tag : uint32_t {
CREATE_SURFACE = IBinder::FIRST_CALL_TRANSACTION,
- DESTROY_SURFACE,
CLEAR_LAYER_FRAME_STATS,
GET_LAYER_FRAME_STATS,
LAST = GET_LAYER_FRAME_STATS,
@@ -57,11 +56,6 @@
handle, gbp);
}
- status_t destroySurface(const sp<IBinder>& handle) override {
- return callRemote<decltype(&ISurfaceComposerClient::destroySurface)>(Tag::DESTROY_SURFACE,
- handle);
- }
-
status_t clearLayerFrameStats(const sp<IBinder>& handle) const override {
return callRemote<decltype(
&ISurfaceComposerClient::clearLayerFrameStats)>(Tag::CLEAR_LAYER_FRAME_STATS,
@@ -92,8 +86,6 @@
switch (tag) {
case Tag::CREATE_SURFACE:
return callLocal(data, reply, &ISurfaceComposerClient::createSurface);
- case Tag::DESTROY_SURFACE:
- return callLocal(data, reply, &ISurfaceComposerClient::destroySurface);
case Tag::CLEAR_LAYER_FRAME_STATS:
return callLocal(data, reply, &ISurfaceComposerClient::clearLayerFrameStats);
case Tag::GET_LAYER_FRAME_STATS:
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index 824e43f..6c39d6f 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -205,6 +205,22 @@
return *this;
}
+void SurfaceComposerClient::doDropReferenceTransaction(const sp<IBinder>& handle,
+ const sp<ISurfaceComposerClient>& client) {
+ 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;
+
+ composerStates.add(s);
+ sf->setTransactionState(composerStates, displayStates, 0, nullptr, {});
+}
+
status_t SurfaceComposerClient::Transaction::apply(bool synchronous) {
if (mStatus != NO_ERROR) {
return mStatus;
@@ -819,17 +835,6 @@
#endif
-SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::destroySurface(
- const sp<SurfaceControl>& sc) {
- layer_state_t* s = getLayerState(sc);
- if (!s) {
- mStatus = BAD_INDEX;
- return *this;
- }
- s->what |= layer_state_t::eDestroySurface;
- return *this;
-}
-
SurfaceComposerClient::Transaction& SurfaceComposerClient::Transaction::setColorTransform(
const sp<SurfaceControl>& sc, const mat3& matrix, const vec3& translation) {
layer_state_t* s = getLayerState(sc);
@@ -1004,13 +1009,6 @@
return err;
}
-status_t SurfaceComposerClient::destroySurface(const sp<IBinder>& sid) {
- if (mStatus != NO_ERROR)
- return mStatus;
- status_t err = mClient->destroySurface(sid);
- return err;
-}
-
status_t SurfaceComposerClient::clearLayerFrameStats(const sp<IBinder>& token) const {
if (mStatus != NO_ERROR) {
return mStatus;
diff --git a/libs/gui/SurfaceControl.cpp b/libs/gui/SurfaceControl.cpp
index b6ef286..8e500a4 100644
--- a/libs/gui/SurfaceControl.cpp
+++ b/libs/gui/SurfaceControl.cpp
@@ -63,7 +63,13 @@
SurfaceControl::~SurfaceControl()
{
- destroy();
+ if (mClient != nullptr && mHandle != nullptr && mOwned) {
+ SurfaceComposerClient::doDropReferenceTransaction(mHandle, mClient->getClient());
+ }
+ mClient.clear();
+ mHandle.clear();
+ mGraphicBufferProducer.clear();
+ IPCThreadState::self()->flushCommands();
}
void SurfaceControl::destroy()
@@ -71,7 +77,7 @@
// Avoid destroying the server-side surface if we are not the owner of it, meaning that we
// retrieved it from another process.
if (isValid() && mOwned) {
- mClient->destroySurface(mHandle);
+ SurfaceComposerClient::Transaction().reparent(this, nullptr).apply();
}
// clear all references and trigger an IPC now, to make sure things
// happen without delay, since these resources are quite heavy.
diff --git a/libs/gui/include/gui/ISurfaceComposerClient.h b/libs/gui/include/gui/ISurfaceComposerClient.h
index 82b01b8..56ca197 100644
--- a/libs/gui/include/gui/ISurfaceComposerClient.h
+++ b/libs/gui/include/gui/ISurfaceComposerClient.h
@@ -58,11 +58,6 @@
/*
* Requires ACCESS_SURFACE_FLINGER permission
*/
- virtual status_t destroySurface(const sp<IBinder>& handle) = 0;
-
- /*
- * Requires ACCESS_SURFACE_FLINGER permission
- */
virtual status_t clearLayerFrameStats(const sp<IBinder>& handle) const = 0;
/*
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 4a8e01b..e0cbb70 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -145,6 +145,13 @@
ui::Dataspace* wideColorGamutDataspace,
ui::PixelFormat* wideColorGamutPixelFormat);
+ /**
+ * 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);
+
// ------------------------------------------------------------------------
// surface creation / destruction
@@ -338,8 +345,6 @@
Transaction& transferTouchFocus(const sp<IBinder>& fromToken, const sp<IBinder>& toToken);
#endif
- Transaction& destroySurface(const sp<SurfaceControl>& sc);
-
// Set a color transform matrix on the given layer on the built-in display.
Transaction& setColorTransform(const sp<SurfaceControl>& sc, const mat3& matrix,
const vec3& translation);
@@ -368,8 +373,6 @@
void setEarlyWakeup();
};
- status_t destroySurface(const sp<IBinder>& id);
-
status_t clearLayerFrameStats(const sp<IBinder>& token) const;
status_t getLayerFrameStats(const sp<IBinder>& token, FrameStats* outStats) const;
static status_t clearAnimationFrameStats();