commit 6597c5405f164fe5602ae5aba4e0c4fc41c20389
author Vishnu Nair <vishnun@google.com> Mon Jun 03 23:31:47 2024 -0700
committer Vishnu Nair <vishnun@google.com> Tue Jun 04 06:39:54 2024 +0000
tree fd6a877ec6f2e0043b73083bfe7d4f320ff5fced
parent b51f44c4c2afad7dcac8b880ffc43fcb03905e02

Clean up dangling layer hierarchy references

In order to support detached mirrors, we introduced a different path for
mirroring a hierarchy that excludes the root layer's transform. However,
when the mirrored layer was destroyed, references to this layer were not
properly removed from its mirroring hierarchy. Fix this and introduce a test
to cover this scenario.

Flag: EXEMPT bug fix
Fixes: 343901186
Test: presumbit

Change-Id: I32f41fc2c9db00590ede4509d75000ab0a38f116

services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp

diff --git a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
index 4b0618e..dd5e8bd 100644
--- a/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerLifecycleManager.cpp
@@ -152,6 +152,10 @@
             if (swapErase(linkedLayer->mirrorIds, layer.id)) {
                 linkedLayer->changes |= RequestedLayerState::Changes::Mirror;
             }
+            if (linkedLayer->layerIdToMirror == layer.id) {
+                linkedLayer->layerIdToMirror = UNASSIGNED_LAYER_ID;
+                linkedLayer->changes |= RequestedLayerState::Changes::Mirror;
+            }
             if (linkedLayer->touchCropId == layer.id) {
                 linkedLayer->touchCropId = UNASSIGNED_LAYER_ID;
             }

services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp

diff --git a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp
index 2b333f4..b79bdb4 100644
--- a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp
@@ -777,4 +777,28 @@
     EXPECT_EQ(getTraversalPath(hierarchyBuilder.getOffscreenHierarchy()), expected);
 }
 
+// (b/343901186)
+TEST_F(LayerHierarchyTest, cleanUpDanglingMirrorLayer) {
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
+    mirrorLayer(/*layer*/ 14, /*parent*/ 1, /*layerToMirror*/ 2);
+    UPDATE_AND_VERIFY(hierarchyBuilder);
+
+    std::vector<uint32_t> expectedTraversalPath = {1, 11, 111, 12, 121, 122, 1221, 13, 14, 2, 2};
+    EXPECT_EQ(getTraversalPath(hierarchyBuilder.getHierarchy()), expectedTraversalPath);
+    EXPECT_EQ(getTraversalPathInZOrder(hierarchyBuilder.getHierarchy()), expectedTraversalPath);
+    expectedTraversalPath = {};
+    EXPECT_EQ(getTraversalPath(hierarchyBuilder.getOffscreenHierarchy()), expectedTraversalPath);
+
+    // destroy layer handle
+    reparentLayer(2, UNASSIGNED_LAYER_ID);
+    destroyLayerHandle(2);
+    UPDATE_AND_VERIFY(hierarchyBuilder);
+    expectedTraversalPath = {1, 11, 111, 12, 121, 122, 1221, 13, 14};
+    EXPECT_EQ(getTraversalPath(hierarchyBuilder.getHierarchy()), expectedTraversalPath);
+    EXPECT_EQ(getTraversalPathInZOrder(hierarchyBuilder.getHierarchy()), expectedTraversalPath);
+    expectedTraversalPath = {};
+    EXPECT_EQ(getTraversalPath(hierarchyBuilder.getOffscreenHierarchy()), expectedTraversalPath);
+}
+
 } // namespace android::surfaceflinger::frontend