surfaceflinger: fix z-relative layer destruction

Layer::commitTransaction is called before Layer::onRemoved.  The
removal of a layer from another layer's zOrderRelatives is not
reflected in Layer::mDrawingState until Layer::commitTransaction is
called again.  As a result, we may draw a removed layer, which is
not supposed to own any HWC or GL resource.

Add Layer::onRemovedFromCurrentState that is called when a layer is
removed from mCurrentState to mLayersPendingRemoval.  Move
zOrderRelative* updates from onRemoved to the new
onRemovedFromCurrentState, and set eTraversalNeeded as needed.

Also fix Layer::~Layer to restore the old behavior and destroy all
stale HWC layers just in case.

Bug: 64572777
Test: manual and AUPT
Change-Id: I546c5b4b7ecac0937fead655733402fae664331c

diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index 4edde14..038ece2 100755 (executable)
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -202,9 +202,11 @@ Layer::~Layer() {
     mFrameTracker.logAndResetStats(mName);
 
 #ifdef USE_HWC2
-    ALOGE_IF(!mHwcLayers.empty(),
-            "Found stale hardware composer layers when destroying "
-            "surface flinger layer");
+    if (!mHwcLayers.empty()) {
+        ALOGE("Found stale hardware composer layers when destroying "
+                "surface flinger layer %s", mName.string());
+        destroyAllHwcLayers();
+    }
 #endif
 }
 
@@ -293,20 +295,27 @@ void Layer::onSidebandStreamChanged() {
     }
 }
 
-// called with SurfaceFlinger::mStateLock from the drawing thread after
-// the layer has been remove from the current state list (and just before
-// it's removed from the drawing state list)
-void Layer::onRemoved() {
+void Layer::onRemovedFromCurrentState() {
+    // the layer is removed from SF mCurrentState to mLayersPendingRemoval
+
     if (mCurrentState.zOrderRelativeOf != nullptr) {
         sp<Layer> strongRelative = mCurrentState.zOrderRelativeOf.promote();
         if (strongRelative != nullptr) {
             strongRelative->removeZOrderRelative(this);
+            mFlinger->setTransactionFlags(eTraversalNeeded);
         }
         mCurrentState.zOrderRelativeOf = nullptr;
     }
 
-    mSurfaceFlingerConsumer->abandon();
+    for (const auto& child : mCurrentChildren) {
+        child->onRemovedFromCurrentState();
+    }
+}
 
+void Layer::onRemoved() {
+    // the layer is removed from SF mLayersPendingRemoval
+
+    mSurfaceFlingerConsumer->abandon();
 #ifdef USE_HWC2
     destroyAllHwcLayers();
 #endif

diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index 222718b..c34d8a0 100644 (file)
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -419,8 +419,14 @@ public:
     bool isPotentialCursor() const { return mPotentialCursor;}
 
     /*
-     * called with the state lock when the surface is removed from the
-     * current list
+     * called with the state lock from a binder thread when the layer is
+     * removed from the current list to the pending removal list
+     */
+    void onRemovedFromCurrentState();
+
+    /*
+     * called with the state lock from the main thread when the layer is
+     * removed from the pending removal list
      */
     void onRemoved();
 

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 4aaa8c0..769ff9c 100644 (file)
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2790,6 +2790,7 @@ status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly)
         return NO_ERROR;
     }
 
+    layer->onRemovedFromCurrentState();
     mLayersPendingRemoval.add(layer);
     mLayersRemoved = true;
     mNumLayers -= 1 + layer->getChildrenCount();

diff --git a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
index e5bb228..b28fe68 100644 (file)
--- a/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
+++ b/services/surfaceflinger/SurfaceFlinger_hwc1.cpp
@@ -2382,6 +2382,7 @@ status_t SurfaceFlinger::removeLayer(const sp<Layer>& layer, bool topLevelOnly)
         return NO_ERROR;
     }
 
+    layer->onRemovedFromCurrentState();
     mLayersPendingRemoval.add(layer);
     mLayersRemoved = true;
     mNumLayers -= 1 + layer->getChildrenCount();