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/include/gui/SurfaceComposerClient.h b/include/gui/SurfaceComposerClient.h
index df73ff6..520a7ab 100644
--- a/include/gui/SurfaceComposerClient.h
+++ b/include/gui/SurfaceComposerClient.h
@@ -160,6 +160,7 @@
const sp<Surface>& handle, uint64_t frameNumber);
status_t reparentChildren(const sp<IBinder>& id,
const sp<IBinder>& newParentHandle);
+ status_t detachChildren(const sp<IBinder>& id);
status_t setOverrideScalingMode(const sp<IBinder>& id,
int32_t overrideScalingMode);
status_t setGeometryAppliesWithResize(const sp<IBinder>& id);
diff --git a/include/gui/SurfaceControl.h b/include/gui/SurfaceControl.h
index 8d338f9..58ec6dc 100644
--- a/include/gui/SurfaceControl.h
+++ b/include/gui/SurfaceControl.h
@@ -93,6 +93,18 @@
// Reparents all children of this layer to the new parent handle.
status_t reparentChildren(const sp<IBinder>& newParentHandle);
+ // Detaches all child surfaces (and their children recursively)
+ // from their SurfaceControl.
+ // The child SurfaceControl's will not throw exceptions or return errors,
+ // but transactions will have no effect.
+ // The child surfaces will continue to follow their parent surfaces,
+ // and remain eligible for rendering, but their relative state will be
+ // frozen. We use this in the WindowManager, in app shutdown/relaunch
+ // scenarios, where the app would otherwise clean up its child Surfaces.
+ // Sometimes the WindowManager needs to extend their lifetime slightly
+ // in order to perform an exit animation or prevent flicker.
+ status_t detachChildren();
+
// Set an override scaling mode as documented in <system/window.h>
// the override scaling mode will take precedence over any client
// specified scaling mode. -1 will clear the override scaling mode.
diff --git a/include/private/gui/LayerState.h b/include/private/gui/LayerState.h
index fac5d2c..6d2b082 100644
--- a/include/private/gui/LayerState.h
+++ b/include/private/gui/LayerState.h
@@ -58,6 +58,7 @@
eOverrideScalingModeChanged = 0x00000800,
eGeometryAppliesWithResize = 0x00001000,
eReparentChildren = 0x00002000,
+ eDetachChildren = 0x00004000
};
layer_state_t()
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index fd277f1..2632781 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -175,6 +175,8 @@
status_t reparentChildren(const sp<SurfaceComposerClient>& client,
const sp<IBinder>& id,
const sp<IBinder>& newParentHandle);
+ status_t detachChildren(const sp<SurfaceComposerClient>& client,
+ const sp<IBinder>& id);
status_t setOverrideScalingMode(const sp<SurfaceComposerClient>& client,
const sp<IBinder>& id, int32_t overrideScalingMode);
status_t setGeometryAppliesWithResize(const sp<SurfaceComposerClient>& client,
@@ -476,6 +478,18 @@
return NO_ERROR;
}
+status_t Composer::detachChildren(
+ const sp<SurfaceComposerClient>& client,
+ const sp<IBinder>& id) {
+ Mutex::Autolock lock(mLock);
+ layer_state_t* s = getLayerStateLocked(client, id);
+ if (!s) {
+ return BAD_INDEX;
+ }
+ s->what |= layer_state_t::eDetachChildren;
+ return NO_ERROR;
+}
+
status_t Composer::setOverrideScalingMode(
const sp<SurfaceComposerClient>& client,
const sp<IBinder>& id, int32_t overrideScalingMode) {
@@ -805,6 +819,10 @@
return getComposer().reparentChildren(this, id, newParentHandle);
}
+status_t SurfaceComposerClient::detachChildren(const sp<IBinder>& id) {
+ return getComposer().detachChildren(this, id);
+}
+
status_t SurfaceComposerClient::setOverrideScalingMode(
const sp<IBinder>& id, int32_t overrideScalingMode) {
return getComposer().setOverrideScalingMode(
diff --git a/libs/gui/SurfaceControl.cpp b/libs/gui/SurfaceControl.cpp
index 94094e5..0080ff9 100644
--- a/libs/gui/SurfaceControl.cpp
+++ b/libs/gui/SurfaceControl.cpp
@@ -183,6 +183,12 @@
return mClient->reparentChildren(mHandle, newParentHandle);
}
+status_t SurfaceControl::detachChildren() {
+ status_t err = validate();
+ if (err < 0) return err;
+ return mClient->detachChildren(mHandle);
+}
+
status_t SurfaceControl::setOverrideScalingMode(int32_t overrideScalingMode) {
status_t err = validate();
if (err < 0) return err;
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);
+ }
+}
+
}