Revert "SurfaceFlinger: Allows Surfaces to outlive their parents."
This reverts commit 4340607f5f5c277eb031fdd1424fc8a1a69924e2.
Reason for revert: Causes bug 117401269
Change-Id: I10973c9dd734499e09c18f48c06f1b1a6a1f8da0
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 7ccf8bc..9410cdb 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2833,7 +2833,6 @@
recordBufferingStats(l->getName().string(),
l->getOccupancyHistory(true));
l->onRemoved();
- mNumLayers -= 1;
}
mLayersPendingRemoval.clear();
}
@@ -3249,7 +3248,7 @@
if (parent == nullptr) {
mCurrentState.layersSortedByZ.add(lbc);
} else {
- if (parent->isRemoved()) {
+ if (parent->isPendingRemoval()) {
ALOGE("addClientLayer called with a removed parent");
return NAME_NOT_FOUND;
}
@@ -3281,7 +3280,7 @@
status_t SurfaceFlinger::removeLayerLocked(const Mutex&, const sp<Layer>& layer,
bool topLevelOnly) {
- if (layer->isRemoved()) {
+ if (layer->isPendingRemoval()) {
return NO_ERROR;
}
@@ -3291,14 +3290,39 @@
if (topLevelOnly) {
return NO_ERROR;
}
+
+ sp<Layer> ancestor = p;
+ while (ancestor->getParent() != nullptr) {
+ ancestor = ancestor->getParent();
+ }
+ if (mCurrentState.layersSortedByZ.indexOf(ancestor) < 0) {
+ ALOGE("removeLayer called with a layer whose parent has been removed");
+ return NAME_NOT_FOUND;
+ }
+
index = p->removeChild(layer);
} else {
index = mCurrentState.layersSortedByZ.remove(layer);
}
+ // As a matter of normal operation, the LayerCleaner will produce a second
+ // attempt to remove the surface. The Layer will be kept alive in mDrawingState
+ // so we will succeed in promoting it, but it's already been removed
+ // from mCurrentState. As long as we can find it in mDrawingState we have no problem
+ // otherwise something has gone wrong and we are leaking the layer.
+ if (index < 0 && mDrawingState.layersSortedByZ.indexOf(layer) < 0) {
+ ALOGE("Failed to find layer (%s) in layer parent (%s).",
+ layer->getName().string(),
+ (p != nullptr) ? p->getName().string() : "no-parent");
+ return BAD_VALUE;
+ } else if (index < 0) {
+ return NO_ERROR;
+ }
+
layer->onRemovedFromCurrentState();
mLayersPendingRemoval.add(layer);
mLayersRemoved = true;
+ mNumLayers -= 1 + layer->getChildrenCount();
setTransactionFlags(eTransactionNeeded);
return NO_ERROR;
}
@@ -3501,7 +3525,7 @@
return 0;
}
- if (layer->isRemoved()) {
+ if (layer->isPendingRemoval()) {
ALOGW("Attempting to set client state on removed layer: %s", layer->getName().string());
return 0;
}
@@ -3704,7 +3728,7 @@
return;
}
- if (layer->isRemoved()) {
+ if (layer->isPendingRemoval()) {
ALOGW("Attempting to destroy on removed layer: %s", layer->getName().string());
return;
}
@@ -3881,6 +3905,19 @@
return err;
}
+status_t SurfaceFlinger::onLayerDestroyed(const wp<Layer>& layer)
+{
+ // called by ~LayerCleaner() when all references to the IBinder (handle)
+ // are gone
+ sp<Layer> l = layer.promote();
+ if (l == nullptr) {
+ // The layer has already been removed, carry on
+ return NO_ERROR;
+ }
+ // If we have a parent, then we can continue to live as long as it does.
+ return removeLayer(l, true);
+}
+
// ---------------------------------------------------------------------------
void SurfaceFlinger::onInitializeDisplays() {
@@ -5254,7 +5291,7 @@
auto layerHandle = reinterpret_cast<Layer::Handle*>(layerHandleBinder.get());
auto parent = layerHandle->owner.promote();
- if (parent == nullptr || parent->isRemoved()) {
+ if (parent == nullptr || parent->isPendingRemoval()) {
ALOGE("captureLayers called with a removed parent");
return NAME_NOT_FOUND;
}