SurfaceControl life-cycle refactoring.
We provide a method called "release" which allows the client
to drop its references without reparenting the SurfaceControl
to null as the destructor would. The Java and NDK API's already
have this method but it's not perfectly functional at the moment.
Additionally we remove the strange pass-through of clear to destroy
and expose destroy directly.
Test: Builds
Bug: 123587983
Change-Id: Ia89ada1476daef97e6f30d50a57065c3636a6489
diff --git a/libs/gui/SurfaceControl.cpp b/libs/gui/SurfaceControl.cpp
index 008f520..d2b0e6a 100644
--- a/libs/gui/SurfaceControl.cpp
+++ b/libs/gui/SurfaceControl.cpp
@@ -66,10 +66,7 @@
if (mClient != nullptr && mHandle != nullptr && mOwned) {
SurfaceComposerClient::doDropReferenceTransaction(mHandle, mClient->getClient());
}
- mClient.clear();
- mHandle.clear();
- mGraphicBufferProducer.clear();
- IPCThreadState::self()->flushCommands();
+ release();
}
void SurfaceControl::destroy()
@@ -79,7 +76,12 @@
if (isValid() && mOwned) {
SurfaceComposerClient::Transaction().reparent(this, nullptr).apply();
}
- // clear all references and trigger an IPC now, to make sure things
+ release();
+}
+
+void SurfaceControl::release()
+{
+ // Trigger an IPC now, to make sure things
// happen without delay, since these resources are quite heavy.
mClient.clear();
mHandle.clear();
@@ -87,17 +89,6 @@
IPCThreadState::self()->flushCommands();
}
-void SurfaceControl::clear()
-{
- // here, the window manager tells us explicitly that we should destroy
- // the surface's resource. Soon after this call, it will also release
- // its last reference (which will call the dtor); however, it is possible
- // that a client living in the same process still holds references which
- // would delay the call to the dtor -- that is why we need this explicit
- // "clear()" call.
- destroy();
-}
-
void SurfaceControl::disconnect() {
if (mGraphicBufferProducer != nullptr) {
mGraphicBufferProducer->disconnect(
diff --git a/libs/gui/include/gui/SurfaceControl.h b/libs/gui/include/gui/SurfaceControl.h
index b584f36..55efcbf 100644
--- a/libs/gui/include/gui/SurfaceControl.h
+++ b/libs/gui/include/gui/SurfaceControl.h
@@ -58,8 +58,12 @@
static bool isSameSurface(
const sp<SurfaceControl>& lhs, const sp<SurfaceControl>& rhs);
- // release surface data from java
- void clear();
+ // Release the handles assosciated with the SurfaceControl, without reparenting
+ // them off-screen. At the moment if this isn't executed before ~SurfaceControl
+ // is called then the destructor will reparent the layer off-screen for you.
+ void release();
+ // Reparent off-screen and release. This is invoked by the destructor.
+ void destroy();
// disconnect any api that's connected
void disconnect();
@@ -98,7 +102,6 @@
sp<Surface> generateSurfaceLocked() const;
status_t validate() const;
- void destroy();
sp<SurfaceComposerClient> mClient;
sp<IBinder> mHandle;
diff --git a/services/surfaceflinger/tests/Stress_test.cpp b/services/surfaceflinger/tests/Stress_test.cpp
index 89c26f4..ee857b0 100644
--- a/services/surfaceflinger/tests/Stress_test.cpp
+++ b/services/surfaceflinger/tests/Stress_test.cpp
@@ -34,7 +34,7 @@
auto surf = client->createSurface(String8("t"), 100, 100,
PIXEL_FORMAT_RGBA_8888, 0);
ASSERT_TRUE(surf != nullptr);
- surf->clear();
+ surf.clear();
}
};
diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index 4f0ded3..50a2e60 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -1137,7 +1137,7 @@
.setRelativeLayer(layerG, layerR->getHandle(), 1)
.apply();
- layerG->clear();
+ layerG.clear();
// layerG should have been removed
getScreenCapture()->expectColor(Rect(0, 0, 32, 32), Color::RED);
}
@@ -4360,7 +4360,7 @@
mCapture->checkPixel(64, 64, 111, 111, 111);
}
- mChild->clear();
+ mChild.clear();
{
SCOPED_TRACE("After destroying child");
@@ -5232,7 +5232,7 @@
ASSERT_NO_FATAL_FAILURE(fillBufferQueueLayerColor(redLayer, Color::RED, 60, 60));
auto redLayerHandle = redLayer->getHandle();
- redLayer->clear();
+ redLayer.clear();
SurfaceComposerClient::Transaction().apply(true);
sp<GraphicBuffer> outBuffer;