surfaceflinger: fix layer count

When a layer is removed, mLayerCount is always decremented by 1 even
though the layer has children.  This commit fixes that by making
sure mLayerCount is properly decremented, and makes sure that a
parent layer must be a valid layer in mCurrentState.

Bug: 37121786
Test: manually open and close Camera app
Change-Id: I5a3e18be41646b167b4d361dfe08db4d06e1c2d0
(cherry picked from commit 98f1c108b016dda77c808f3d099e7a45bdd70768)

diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 6f430a3..2ff14aa 100644 (file)
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -2523,6 +2523,14 @@ bool Layer::getTransformToDisplayInverse() const {
     return mSurfaceFlingerConsumer->getTransformToDisplayInverse();
 }
 
+size_t Layer::getChildrenCount() const {
+    size_t count = 0;
+    for (const sp<Layer>& child : mCurrentChildren) {
+        count += 1 + child->getChildrenCount();
+    }
+    return count;
+}
+
 void Layer::addChild(const sp<Layer>& layer) {
     mCurrentChildren.add(layer);
     layer->setParent(this);

diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 05c367d..5335fff 100644 (file)
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -519,6 +519,7 @@ public:
                                  const LayerVector::Visitor& visitor);
     void traverseInZOrder(LayerVector::StateSet stateSet, const LayerVector::Visitor& visitor);
 
+    size_t getChildrenCount() const;
     void addChild(const sp<Layer>& layer);
     // Returns index if removed, or negative value otherwise
     // for symmetry with Vector::remove

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 29a8292..801e8dd 100644 (file)
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2656,8 +2656,13 @@ status_t SurfaceFlinger::addClientLayer(const sp<Client>& client,
         if (parent == nullptr) {
             mCurrentState.layersSortedByZ.add(lbc);
         } else {
+            if (mCurrentState.layersSortedByZ.indexOf(parent) < 0) {
+                ALOGE("addClientLayer called with a removed parent");
+                return NAME_NOT_FOUND;
+            }
             parent->addChild(lbc);
         }
+
         mGraphicBufferProducerList.add(IInterface::asBinder(gbc));
         mLayersAdded = true;
         mNumLayers++;
@@ -2676,6 +2681,17 @@ status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer) {
     const ssize_t index = (p != nullptr) ? p->removeChild(layer) :
         mCurrentState.layersSortedByZ.remove(layer);
 
+    if (p != nullptr) {
+        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;
+        }
+    }
+
     // 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
@@ -2692,7 +2708,7 @@ status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer) {
 
     mLayersPendingRemoval.add(layer);
     mLayersRemoved = true;
-    mNumLayers--;
+    mNumLayers -= 1 + layer->getChildrenCount();
     setTransactionFlags(eTransactionNeeded);
     return NO_ERROR;
 }