Add detachChildren transaction.
Add SurfaceControl#detachChildren for use by the WindowManager.
This method is used in cases where the WM would previously preserve
windows the client tried to destroy. For example, when becoming invisible
(in the activity lifecycle sense, not in the SurfaceFlinger sense)
an app will destroy its child surfaces. Previously the WM would keep child
windows alive until the animation finishes to prevent glitches. The new
scheme for this is the WM will detach the children at this point,
at which point the parent layer becomes the owner of the children and the WM
can control the lifecycle as it wishes. I also included a test for reparentChildren
as I realized I had forgotten that.
Test: New test in Transaction_test.cpp
Change-Id: I79c22b2ccccceb9bdcc37b70c491bdf33dcf83d2
diff --git a/services/surfaceflinger/Client.cpp b/services/surfaceflinger/Client.cpp
index 2b4f4cb..9ddae2b 100644
--- a/services/surfaceflinger/Client.cpp
+++ b/services/surfaceflinger/Client.cpp
@@ -49,7 +49,10 @@
{
const size_t count = mLayers.size();
for (size_t i=0 ; i<count ; i++) {
- mFlinger->removeLayer(mLayers.valueAt(i));
+ sp<Layer> l = mLayers.valueAt(i).promote();
+ if (l != nullptr) {
+ mFlinger->removeLayer(l);
+ }
}
}
diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 6908c88..9b02d3e 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -2455,6 +2455,21 @@
return true;
}
+bool Layer::detachChildren() {
+ traverseInZOrder([this](Layer* child) {
+ if (child == this) {
+ return;
+ }
+
+ sp<Client> client(child->mClientRef.promote());
+ if (client != nullptr) {
+ client->detachLayer(child);
+ }
+ });
+
+ return true;
+}
+
void Layer::setParent(const sp<Layer>& layer) {
mParent = layer;
}
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index fc33c99..f2e5f22 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -176,6 +176,7 @@
bool setOverrideScalingMode(int32_t overrideScalingMode);
void setInfo(uint32_t type, uint32_t appId);
bool reparentChildren(const sp<IBinder>& layer);
+ bool detachChildren();
// If we have received a new buffer this frame, we will pass its surface
// damage down to hardware composer. Otherwise, we must send a region with
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 7a31a15..62c9f25 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2534,14 +2534,7 @@
return NO_ERROR;
}
-status_t SurfaceFlinger::removeLayer(const wp<Layer>& weakLayer) {
- Mutex::Autolock _l(mStateLock);
- sp<Layer> layer = weakLayer.promote();
- if (layer == nullptr) {
- // The layer has already been removed, carry on
- return NO_ERROR;
- }
-
+status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer) {
const auto& p = layer->getParent();
const ssize_t index = (p != nullptr) ? p->removeChild(layer) :
mCurrentState.layersSortedByZ.remove(layer);
@@ -2824,6 +2817,9 @@
flags |= eTransactionNeeded|eTraversalNeeded;
}
}
+ if (what & layer_state_t::eDetachChildren) {
+ layer->detachChildren();
+ }
if (what & layer_state_t::eOverrideScalingModeChanged) {
layer->setOverrideScalingMode(s.overrideScalingMode);
// We don't trigger a traversal here because if no other state is
@@ -2920,7 +2916,7 @@
status_t SurfaceFlinger::onLayerRemoved(const sp<Client>& client, const sp<IBinder>& handle)
{
- // called by the window manager when it wants to remove a Layer
+ // called by a client when it wants to remove a Layer
status_t err = NO_ERROR;
sp<Layer> l(client->getLayerUser(handle));
if (l != NULL) {
@@ -2936,7 +2932,15 @@
{
// called by ~LayerCleaner() when all references to the IBinder (handle)
// are gone
- return removeLayer(layer);
+ sp<Layer> l = layer.promote();
+ if (l == nullptr) {
+ // The layer has already been removed, carry on
+ return NO_ERROR;
+ } if (l->getParent() != nullptr) {
+ // If we have a parent, then we can continue to live as long as it does.
+ return NO_ERROR;
+ }
+ return removeLayer(l);
}
// ---------------------------------------------------------------------------
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 970a4ef..5bdedf3 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -332,7 +332,7 @@
status_t onLayerDestroyed(const wp<Layer>& layer);
// remove a layer from SurfaceFlinger immediately
- status_t removeLayer(const wp<Layer>& layer);
+ status_t removeLayer(const sp<Layer>& layer);
// add a layer to SurfaceFlinger
status_t addClientLayer(const sp<Client>& client,
diff --git a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
index 056d733..9aa2fab 100644
--- a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
+++ b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
@@ -2318,14 +2318,7 @@
return NO_ERROR;
}
-status_t SurfaceFlinger::removeLayer(const wp<Layer>& weakLayer) {
- Mutex::Autolock _l(mStateLock);
- sp<Layer> layer = weakLayer.promote();
- if (layer == nullptr) {
- // The layer has already been removed, carry on
- return NO_ERROR;
- }
-
+status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer) {
const auto& p = layer->getParent();
const ssize_t index = (p != nullptr) ? p->removeChild(layer) :
mCurrentState.layersSortedByZ.remove(layer);
@@ -2608,6 +2601,9 @@
flags |= eTransactionNeeded|eTraversalNeeded;
}
}
+ if (what & layer_state_t::eDetachChildren) {
+ layer->detachChildren();
+ }
if (what & layer_state_t::eOverrideScalingModeChanged) {
layer->setOverrideScalingMode(s.overrideScalingMode);
// We don't trigger a traversal here because if no other state is
@@ -2704,7 +2700,7 @@
status_t SurfaceFlinger::onLayerRemoved(const sp<Client>& client, const sp<IBinder>& handle)
{
- // called by the window manager when it wants to remove a Layer
+ // called by a client when it wants to remove a Layer
status_t err = NO_ERROR;
sp<Layer> l(client->getLayerUser(handle));
if (l != NULL) {
@@ -2720,7 +2716,15 @@
{
// called by ~LayerCleaner() when all references to the IBinder (handle)
// are gone
- return removeLayer(layer);
+ sp<Layer> l = layer.promote();
+ if (l == nullptr) {
+ // The layer has already been removed, carry on
+ return NO_ERROR;
+ } if (l->getParent() != nullptr) {
+ // If we have a parent, then we can continue to live as long as it does.
+ return NO_ERROR;
+ }
+ return removeLayer(l);
}
// ---------------------------------------------------------------------------
diff --git a/services/surfaceflinger/tests/Transaction_test.cpp b/services/surfaceflinger/tests/Transaction_test.cpp
index d9f1438..5b61ae0 100644
--- a/services/surfaceflinger/tests/Transaction_test.cpp
+++ b/services/surfaceflinger/tests/Transaction_test.cpp
@@ -630,4 +630,68 @@
mCapture->expectFGColor(20, 20);
}
}
+
+TEST_F(ChildLayerTest, ReparentChildren) {
+ SurfaceComposerClient::openGlobalTransaction();
+ mChild->show();
+ mChild->setPosition(10, 10);
+ mFGSurfaceControl->setPosition(64, 64);
+ SurfaceComposerClient::closeGlobalTransaction(true);
+
+ {
+ ScreenCapture::captureScreen(&mCapture);
+ // Top left of foreground must now be visible
+ mCapture->expectFGColor(64, 64);
+ // But 10 pixels in we should see the child surface
+ mCapture->expectChildColor(74, 74);
+ // And 10 more pixels we should be back to the foreground surface
+ mCapture->expectFGColor(84, 84);
+ }
+ mFGSurfaceControl->reparentChildren(mBGSurfaceControl->getHandle());
+ {
+ ScreenCapture::captureScreen(&mCapture);
+ mCapture->expectFGColor(64, 64);
+ // In reparenting we should have exposed the entire foreground surface.
+ mCapture->expectFGColor(74, 74);
+ // And the child layer should now begin at 10, 10 (since the BG
+ // layer is at (0, 0)).
+ mCapture->expectBGColor(9, 9);
+ mCapture->expectChildColor(10, 10);
+ }
+}
+
+TEST_F(ChildLayerTest, DetachChildren) {
+ SurfaceComposerClient::openGlobalTransaction();
+ mChild->show();
+ mChild->setPosition(10, 10);
+ mFGSurfaceControl->setPosition(64, 64);
+ SurfaceComposerClient::closeGlobalTransaction(true);
+
+ {
+ ScreenCapture::captureScreen(&mCapture);
+ // Top left of foreground must now be visible
+ mCapture->expectFGColor(64, 64);
+ // But 10 pixels in we should see the child surface
+ mCapture->expectChildColor(74, 74);
+ // And 10 more pixels we should be back to the foreground surface
+ mCapture->expectFGColor(84, 84);
+ }
+
+ SurfaceComposerClient::openGlobalTransaction();
+ mFGSurfaceControl->detachChildren();
+ SurfaceComposerClient::closeGlobalTransaction();
+
+ SurfaceComposerClient::openGlobalTransaction();
+ mChild->hide();
+ SurfaceComposerClient::closeGlobalTransaction();
+
+ // Nothing should have changed.
+ {
+ ScreenCapture::captureScreen(&mCapture);
+ mCapture->expectFGColor(64, 64);
+ mCapture->expectChildColor(74, 74);
+ mCapture->expectFGColor(84, 84);
+ }
+}
+
}