Detect and recover from relative z loops

The caller can create loops in the hierarchy by relatively
reparenting layers to each other. This can be done directly
or via a chain of layers.

The logic to detect and fix this was not being called. Fix this
by making loop detection a bit harder to ignore and part of
hierarchy builder's update call.

Test: presubmit
Bug: 316236833
Change-Id: I484f9b8e2742fef22a5d76362a715eb6850c26f6

10 files changed, 134 insertions, 88 deletions

diff --git a/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp b/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp
index 1e5a6fbd1e..821ac0cf88 100644
--- a/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp
@@ -190,8 +190,12 @@ bool LayerHierarchy::hasRelZLoop(uint32_t& outInvalidRelativeRoot) const {
     return outInvalidRelativeRoot != UNASSIGNED_LAYER_ID;
 }
 
-LayerHierarchyBuilder::LayerHierarchyBuilder(
-        const std::vector<std::unique_ptr<RequestedLayerState>>& layers) {
+void LayerHierarchyBuilder::init(const std::vector<std::unique_ptr<RequestedLayerState>>& layers) {
+    mLayerIdToHierarchy.clear();
+    mHierarchies.clear();
+    mRoot = nullptr;
+    mOffscreenRoot = nullptr;
+
     mHierarchies.reserve(layers.size());
     mLayerIdToHierarchy.reserve(layers.size());
     for (auto& layer : layers) {
@@ -202,6 +206,7 @@ LayerHierarchyBuilder::LayerHierarchyBuilder(
         onLayerAdded(layer.get());
     }
     detachHierarchyFromRelativeParent(&mOffscreenRoot);
+    mInitialized = true;
 }
 
 void LayerHierarchyBuilder::attachToParent(LayerHierarchy* hierarchy) {
@@ -332,7 +337,7 @@ void LayerHierarchyBuilder::updateMirrorLayer(RequestedLayerState* layer) {
     }
 }
 
-void LayerHierarchyBuilder::update(
+void LayerHierarchyBuilder::doUpdate(
         const std::vector<std::unique_ptr<RequestedLayerState>>& layers,
         const std::vector<std::unique_ptr<RequestedLayerState>>& destroyedLayers) {
     // rebuild map
@@ -381,6 +386,32 @@ void LayerHierarchyBuilder::update(
     attachHierarchyToRelativeParent(&mRoot);
 }
 
+void LayerHierarchyBuilder::update(LayerLifecycleManager& layerLifecycleManager) {
+    if (!mInitialized) {
+        ATRACE_NAME("LayerHierarchyBuilder:init");
+        init(layerLifecycleManager.getLayers());
+    } else if (layerLifecycleManager.getGlobalChanges().test(
+                       RequestedLayerState::Changes::Hierarchy)) {
+        ATRACE_NAME("LayerHierarchyBuilder:update");
+        doUpdate(layerLifecycleManager.getLayers(), layerLifecycleManager.getDestroyedLayers());
+    } else {
+        return; // nothing to do
+    }
+
+    uint32_t invalidRelativeRoot;
+    bool hasRelZLoop = mRoot.hasRelZLoop(invalidRelativeRoot);
+    while (hasRelZLoop) {
+        ATRACE_NAME("FixRelZLoop");
+        TransactionTraceWriter::getInstance().invoke("relz_loop_detected",
+                                                     /*overwrite=*/false);
+        layerLifecycleManager.fixRelativeZLoop(invalidRelativeRoot);
+        // reinitialize the hierarchy with the updated layer data
+        init(layerLifecycleManager.getLayers());
+        // check if we have any remaining loops
+        hasRelZLoop = mRoot.hasRelZLoop(invalidRelativeRoot);
+    }
+}
+
 const LayerHierarchy& LayerHierarchyBuilder::getHierarchy() const {
     return mRoot;
 }
diff --git a/services/surfaceflinger/FrontEnd/LayerHierarchy.h b/services/surfaceflinger/FrontEnd/LayerHierarchy.h
index ba2e262baf..a1c73c38b0 100644
--- a/services/surfaceflinger/FrontEnd/LayerHierarchy.h
+++ b/services/surfaceflinger/FrontEnd/LayerHierarchy.h
@@ -17,6 +17,7 @@
 #pragma once
 
 #include "FrontEnd/LayerCreationArgs.h"
+#include "FrontEnd/LayerLifecycleManager.h"
 #include "RequestedLayerState.h"
 #include "ftl/small_vector.h"
 
@@ -197,9 +198,8 @@ private:
 // hierarchy from a list of RequestedLayerState and associated change flags.
 class LayerHierarchyBuilder {
 public:
-    LayerHierarchyBuilder(const std::vector<std::unique_ptr<RequestedLayerState>>&);
-    void update(const std::vector<std::unique_ptr<RequestedLayerState>>& layers,
-                const std::vector<std::unique_ptr<RequestedLayerState>>& destroyedLayers);
+    LayerHierarchyBuilder() = default;
+    void update(LayerLifecycleManager& layerLifecycleManager);
     LayerHierarchy getPartialHierarchy(uint32_t, bool childrenOnly) const;
     const LayerHierarchy& getHierarchy() const;
     const LayerHierarchy& getOffscreenHierarchy() const;
@@ -213,14 +213,18 @@ private:
     void detachFromRelativeParent(LayerHierarchy*);
     void attachHierarchyToRelativeParent(LayerHierarchy*);
     void detachHierarchyFromRelativeParent(LayerHierarchy*);
-
+    void init(const std::vector<std::unique_ptr<RequestedLayerState>>&);
+    void doUpdate(const std::vector<std::unique_ptr<RequestedLayerState>>& layers,
+                  const std::vector<std::unique_ptr<RequestedLayerState>>& destroyedLayers);
     void onLayerDestroyed(RequestedLayerState* layer);
     void updateMirrorLayer(RequestedLayerState* layer);
     LayerHierarchy* getHierarchyFromId(uint32_t layerId, bool crashOnFailure = true);
+
     std::unordered_map<uint32_t, LayerHierarchy*> mLayerIdToHierarchy;
     std::vector<std::unique_ptr<LayerHierarchy>> mHierarchies;
     LayerHierarchy mRoot{nullptr};
     LayerHierarchy mOffscreenRoot{nullptr};
+    bool mInitialized = false;
 };
 
 } // namespace android::surfaceflinger::frontend
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 8311617870..bf1da7c964 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2327,11 +2327,7 @@ bool SurfaceFlinger::updateLayerSnapshots(VsyncId vsyncId, nsecs_t frameTimeNs,
                 mLegacyLayers[layer->sequence] = layer;
             }
         }
-        if (mLayerLifecycleManager.getGlobalChanges().test(Changes::Hierarchy)) {
-            ATRACE_NAME("LayerHierarchyBuilder:update");
-            mLayerHierarchyBuilder.update(mLayerLifecycleManager.getLayers(),
-                                          mLayerLifecycleManager.getDestroyedLayers());
-        }
+        mLayerHierarchyBuilder.update(mLayerLifecycleManager);
     }
 
     bool mustComposite = false;
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 6909055801..ebdd7211db 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -1439,7 +1439,7 @@ private:
     bool mLegacyFrontEndEnabled = true;
 
     frontend::LayerLifecycleManager mLayerLifecycleManager;
-    frontend::LayerHierarchyBuilder mLayerHierarchyBuilder{{}};
+    frontend::LayerHierarchyBuilder mLayerHierarchyBuilder;
     frontend::LayerSnapshotBuilder mLayerSnapshotBuilder;
 
     std::vector<std::pair<uint32_t, std::string>> mDestroyedHandles;
diff --git a/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp b/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp
index c2d1954ee5..3f46e6c7e8 100644
--- a/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp
+++ b/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp
@@ -52,7 +52,7 @@ bool LayerTraceGenerator::generate(const perfetto::protos::TransactionTraceFile&
 
     // frontend
     frontend::LayerLifecycleManager lifecycleManager;
-    frontend::LayerHierarchyBuilder hierarchyBuilder{{}};
+    frontend::LayerHierarchyBuilder hierarchyBuilder;
     frontend::LayerSnapshotBuilder snapshotBuilder;
     ui::DisplayMap<ui::LayerStack, frontend::DisplayInfo> displayInfos;
 
@@ -119,12 +119,10 @@ bool LayerTraceGenerator::generate(const perfetto::protos::TransactionTraceFile&
         lifecycleManager.applyTransactions(transactions, /*ignoreUnknownHandles=*/true);
         lifecycleManager.onHandlesDestroyed(destroyedHandles, /*ignoreUnknownHandles=*/true);
 
-        if (lifecycleManager.getGlobalChanges().test(
-                    frontend::RequestedLayerState::Changes::Hierarchy)) {
-            hierarchyBuilder.update(lifecycleManager.getLayers(),
-                                    lifecycleManager.getDestroyedLayers());
-        }
+        // update hierarchy
+        hierarchyBuilder.update(lifecycleManager);
 
+        // update snapshots
         frontend::LayerSnapshotBuilder::Args args{.root = hierarchyBuilder.getHierarchy(),
                                                   .layerLifecycleManager = lifecycleManager,
                                                   .displays = displayInfos,
diff --git a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp
index 95f19406b4..2b333f4b87 100644
--- a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.cpp
@@ -45,7 +45,8 @@ protected:
 
 // reparenting tests
 TEST_F(LayerHierarchyTest, addLayer) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     std::vector<uint32_t> expectedTraversalPath = {1, 11, 111, 12, 121, 122, 1221, 13, 2};
     EXPECT_EQ(getTraversalPath(hierarchyBuilder.getHierarchy()), expectedTraversalPath);
     EXPECT_EQ(getTraversalPathInZOrder(hierarchyBuilder.getHierarchy()), expectedTraversalPath);
@@ -64,7 +65,8 @@ TEST_F(LayerHierarchyTest, addLayer) {
 }
 
 TEST_F(LayerHierarchyTest, reparentLayer) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     reparentLayer(2, 11);
     reparentLayer(111, 12);
     reparentLayer(1221, 1);
@@ -79,7 +81,8 @@ TEST_F(LayerHierarchyTest, reparentLayer) {
 }
 
 TEST_F(LayerHierarchyTest, reparentLayerToNull) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
 
     reparentLayer(2, UNASSIGNED_LAYER_ID);
     reparentLayer(11, UNASSIGNED_LAYER_ID);
@@ -96,7 +99,8 @@ TEST_F(LayerHierarchyTest, reparentLayerToNull) {
 }
 
 TEST_F(LayerHierarchyTest, reparentLayerToNullAndDestroyHandles) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     reparentLayer(2, UNASSIGNED_LAYER_ID);
     reparentLayer(11, UNASSIGNED_LAYER_ID);
     reparentLayer(1221, UNASSIGNED_LAYER_ID);
@@ -115,7 +119,8 @@ TEST_F(LayerHierarchyTest, reparentLayerToNullAndDestroyHandles) {
 }
 
 TEST_F(LayerHierarchyTest, destroyHandleThenDestroyParentLayer) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     destroyLayerHandle(111);
     UPDATE_AND_VERIFY(hierarchyBuilder);
 
@@ -139,7 +144,8 @@ TEST_F(LayerHierarchyTest, destroyHandleThenDestroyParentLayer) {
 }
 
 TEST_F(LayerHierarchyTest, layerSurvivesTemporaryReparentToNull) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     reparentLayer(11, UNASSIGNED_LAYER_ID);
     reparentLayer(11, 1);
 
@@ -154,7 +160,8 @@ TEST_F(LayerHierarchyTest, layerSurvivesTemporaryReparentToNull) {
 
 // offscreen tests
 TEST_F(LayerHierarchyTest, layerMovesOnscreen) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
 
     reparentLayer(11, UNASSIGNED_LAYER_ID);
     UPDATE_AND_VERIFY(hierarchyBuilder);
@@ -170,7 +177,8 @@ TEST_F(LayerHierarchyTest, layerMovesOnscreen) {
 }
 
 TEST_F(LayerHierarchyTest, addLayerToOffscreenParent) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
 
     reparentLayer(11, UNASSIGNED_LAYER_ID);
     UPDATE_AND_VERIFY(hierarchyBuilder);
@@ -187,7 +195,8 @@ TEST_F(LayerHierarchyTest, addLayerToOffscreenParent) {
 
 // rel-z tests
 TEST_F(LayerHierarchyTest, setRelativeParent) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     reparentRelativeLayer(11, 2);
     UPDATE_AND_VERIFY(hierarchyBuilder);
 
@@ -200,7 +209,8 @@ TEST_F(LayerHierarchyTest, setRelativeParent) {
 }
 
 TEST_F(LayerHierarchyTest, reparentFromRelativeParentWithSetLayer) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     reparentRelativeLayer(11, 2);
     UPDATE_AND_VERIFY(hierarchyBuilder);
 
@@ -216,7 +226,8 @@ TEST_F(LayerHierarchyTest, reparentFromRelativeParentWithSetLayer) {
 }
 
 TEST_F(LayerHierarchyTest, reparentToRelativeParent) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     reparentRelativeLayer(11, 2);
     UPDATE_AND_VERIFY(hierarchyBuilder);
 
@@ -231,7 +242,8 @@ TEST_F(LayerHierarchyTest, reparentToRelativeParent) {
 }
 
 TEST_F(LayerHierarchyTest, setParentAsRelativeParent) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     reparentLayer(11, 2);
     UPDATE_AND_VERIFY(hierarchyBuilder);
 
@@ -246,7 +258,8 @@ TEST_F(LayerHierarchyTest, setParentAsRelativeParent) {
 }
 
 TEST_F(LayerHierarchyTest, relativeChildMovesOffscreenIsNotTraversable) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     reparentRelativeLayer(11, 2);
     UPDATE_AND_VERIFY(hierarchyBuilder);
 
@@ -262,7 +275,8 @@ TEST_F(LayerHierarchyTest, relativeChildMovesOffscreenIsNotTraversable) {
 }
 
 TEST_F(LayerHierarchyTest, reparentRelativeLayer) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     reparentRelativeLayer(11, 2);
     UPDATE_AND_VERIFY(hierarchyBuilder);
 
@@ -294,7 +308,8 @@ TEST_F(LayerHierarchyTest, reparentRelativeLayer) {
 
 // mirror tests
 TEST_F(LayerHierarchyTest, canTraverseMirrorLayer) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
 
     mirrorLayer(/*layer*/ 14, /*parent*/ 1, /*layerToMirror*/ 11);
     UPDATE_AND_VERIFY(hierarchyBuilder);
@@ -308,7 +323,8 @@ TEST_F(LayerHierarchyTest, canTraverseMirrorLayer) {
 }
 
 TEST_F(LayerHierarchyTest, canMirrorOffscreenLayer) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
 
     reparentLayer(11, UNASSIGNED_LAYER_ID);
     mirrorLayer(/*layer*/ 14, /*parent*/ 1, /*layerToMirror*/ 11);
@@ -324,7 +340,8 @@ TEST_F(LayerHierarchyTest, canMirrorOffscreenLayer) {
 TEST_F(LayerHierarchyTest, newChildLayerIsUpdatedInMirrorHierarchy) {
     mirrorLayer(/*layer*/ 14, /*parent*/ 1, /*layerToMirror*/ 11);
     mLifecycleManager.commitChanges();
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
 
     createLayer(1111, 111);
     createLayer(112, 11);
@@ -340,7 +357,8 @@ TEST_F(LayerHierarchyTest, newChildLayerIsUpdatedInMirrorHierarchy) {
 
 // mirror & relatives tests
 TEST_F(LayerHierarchyTest, mirrorWithRelativeOutsideMirrorHierarchy) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     reparentRelativeLayer(111, 12);
     mirrorLayer(/*layer*/ 14, /*parent*/ 1, /*layerToMirror*/ 11);
 
@@ -371,7 +389,8 @@ TEST_F(LayerHierarchyTest, mirrorWithRelativeOutsideMirrorHierarchy) {
 }
 
 TEST_F(LayerHierarchyTest, mirrorWithRelativeInsideMirrorHierarchy) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     reparentRelativeLayer(1221, 12);
     mirrorLayer(/*layer*/ 14, /*parent*/ 1, /*layerToMirror*/ 12);
 
@@ -401,7 +420,8 @@ TEST_F(LayerHierarchyTest, mirrorWithRelativeInsideMirrorHierarchy) {
 }
 
 TEST_F(LayerHierarchyTest, childMovesOffscreenWhenRelativeParentDies) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
 
     reparentRelativeLayer(11, 2);
     reparentLayer(2, UNASSIGNED_LAYER_ID);
@@ -427,7 +447,8 @@ TEST_F(LayerHierarchyTest, childMovesOffscreenWhenRelativeParentDies) {
 }
 
 TEST_F(LayerHierarchyTest, offscreenLayerCannotBeRelativeToOnscreenLayer) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     reparentRelativeLayer(1221, 2);
     UPDATE_AND_VERIFY(hierarchyBuilder);
 
@@ -462,7 +483,8 @@ TEST_F(LayerHierarchyTest, offscreenLayerCannotBeRelativeToOnscreenLayer) {
 }
 
 TEST_F(LayerHierarchyTest, backgroundLayersAreBehindParentLayer) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
 
     updateBackgroundColor(1, 0.5);
     UPDATE_AND_VERIFY(hierarchyBuilder);
@@ -485,7 +507,8 @@ TEST_F(LayerHierarchyTest, ParentBecomesTheChild) {
     createLayer(11, 1);
     reparentLayer(1, 11);
     mLifecycleManager.commitChanges();
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
 
     std::vector<uint32_t> expectedTraversalPath = {};
     EXPECT_EQ(getTraversalPath(hierarchyBuilder.getHierarchy()), expectedTraversalPath);
@@ -502,17 +525,11 @@ TEST_F(LayerHierarchyTest, RelativeLoops) {
     createLayer(11, 1);
     reparentRelativeLayer(11, 2);
     reparentRelativeLayer(2, 11);
-    mLifecycleManager.commitChanges();
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
-
-    // fix loop
-    uint32_t invalidRelativeRoot;
-    bool hasRelZLoop = hierarchyBuilder.getHierarchy().hasRelZLoop(invalidRelativeRoot);
-    EXPECT_TRUE(hasRelZLoop);
-    mLifecycleManager.fixRelativeZLoop(invalidRelativeRoot);
-    hierarchyBuilder.update(mLifecycleManager.getLayers(), mLifecycleManager.getDestroyedLayers());
-    EXPECT_EQ(invalidRelativeRoot, 11u);
-    EXPECT_FALSE(hierarchyBuilder.getHierarchy().hasRelZLoop(invalidRelativeRoot));
+    LayerHierarchyBuilder hierarchyBuilder;
+    // this call is expected to fix the loop!
+    hierarchyBuilder.update(mLifecycleManager);
+    uint32_t unused;
+    EXPECT_FALSE(hierarchyBuilder.getHierarchy().hasRelZLoop(unused));
 
     std::vector<uint32_t> expectedTraversalPath = {1, 11, 2, 2};
     EXPECT_EQ(getTraversalPath(hierarchyBuilder.getHierarchy()), expectedTraversalPath);
@@ -534,16 +551,11 @@ TEST_F(LayerHierarchyTest, IndirectRelativeLoops) {
     createLayer(221, 22);
     reparentRelativeLayer(22, 111);
     reparentRelativeLayer(11, 221);
-    mLifecycleManager.commitChanges();
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
-
-    // fix loop
-    uint32_t invalidRelativeRoot;
-    bool hasRelZLoop = hierarchyBuilder.getHierarchy().hasRelZLoop(invalidRelativeRoot);
-    EXPECT_TRUE(hasRelZLoop);
-    mLifecycleManager.fixRelativeZLoop(invalidRelativeRoot);
-    hierarchyBuilder.update(mLifecycleManager.getLayers(), mLifecycleManager.getDestroyedLayers());
-    EXPECT_FALSE(hierarchyBuilder.getHierarchy().hasRelZLoop(invalidRelativeRoot));
+    LayerHierarchyBuilder hierarchyBuilder;
+    // this call is expected to fix the loop!
+    hierarchyBuilder.update(mLifecycleManager);
+    uint32_t unused;
+    EXPECT_FALSE(hierarchyBuilder.getHierarchy().hasRelZLoop(unused));
 
     std::vector<uint32_t> expectedTraversalPath = {1, 11, 111, 22, 221, 2, 21, 22, 221};
     EXPECT_EQ(getTraversalPath(hierarchyBuilder.getHierarchy()), expectedTraversalPath);
@@ -554,7 +566,8 @@ TEST_F(LayerHierarchyTest, IndirectRelativeLoops) {
 }
 
 TEST_F(LayerHierarchyTest, ReparentRootLayerToNull) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     reparentLayer(1, UNASSIGNED_LAYER_ID);
     UPDATE_AND_VERIFY(hierarchyBuilder);
 
@@ -568,7 +581,8 @@ TEST_F(LayerHierarchyTest, ReparentRootLayerToNull) {
 TEST_F(LayerHierarchyTest, AddRemoveLayerInSameTransaction) {
     // remove default hierarchy
     mLifecycleManager = LayerLifecycleManager();
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     createRootLayer(1);
     destroyLayerHandle(1);
     UPDATE_AND_VERIFY(hierarchyBuilder);
@@ -582,7 +596,8 @@ TEST_F(LayerHierarchyTest, AddRemoveLayerInSameTransaction) {
 // traversal path test
 TEST_F(LayerHierarchyTest, traversalPathId) {
     setZ(122, -1);
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     auto checkTraversalPathIdVisitor =
             [](const LayerHierarchy& hierarchy,
                const LayerHierarchy::TraversalPath& traversalPath) -> bool {
@@ -605,7 +620,8 @@ TEST_F(LayerHierarchyTest, zorderRespectsLayerSequenceId) {
     createLayer(53, 5);
 
     mLifecycleManager.commitChanges();
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     UPDATE_AND_VERIFY(hierarchyBuilder);
     std::vector<uint32_t> expectedTraversalPath = {1, 11, 2, 4, 5, 51, 53};
     EXPECT_EQ(getTraversalPath(hierarchyBuilder.getHierarchy()), expectedTraversalPath);
@@ -639,7 +655,8 @@ TEST_F(LayerHierarchyTest, zorderRespectsLayerZ) {
     setZ(13, 1);
 
     mLifecycleManager.commitChanges();
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     UPDATE_AND_VERIFY(hierarchyBuilder);
     std::vector<uint32_t> expectedTraversalPath = {1, 11, 13, 12};
     EXPECT_EQ(getTraversalPath(hierarchyBuilder.getHierarchy()), expectedTraversalPath);
@@ -661,7 +678,8 @@ TEST_F(LayerHierarchyTest, zorderRespectsLayerStack) {
     setLayerStack(2, 10);
 
     mLifecycleManager.commitChanges();
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     UPDATE_AND_VERIFY(hierarchyBuilder);
     std::vector<uint32_t> expectedTraversalPath = {2, 21, 1, 11};
     EXPECT_EQ(getTraversalPath(hierarchyBuilder.getHierarchy()), expectedTraversalPath);
@@ -672,7 +690,8 @@ TEST_F(LayerHierarchyTest, zorderRespectsLayerStack) {
 }
 
 TEST_F(LayerHierarchyTest, canMirrorDisplay) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     setFlags(12, layer_state_t::eLayerSkipScreenshot, layer_state_t::eLayerSkipScreenshot);
     createDisplayMirrorLayer(3, ui::LayerStack::fromValue(0));
     setLayerStack(3, 1);
@@ -687,7 +706,8 @@ TEST_F(LayerHierarchyTest, canMirrorDisplay) {
 }
 
 TEST_F(LayerHierarchyTest, mirrorNonExistingDisplay) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     setFlags(12, layer_state_t::eLayerSkipScreenshot, layer_state_t::eLayerSkipScreenshot);
     createDisplayMirrorLayer(3, ui::LayerStack::fromValue(5));
     setLayerStack(3, 1);
@@ -701,7 +721,8 @@ TEST_F(LayerHierarchyTest, mirrorNonExistingDisplay) {
 }
 
 TEST_F(LayerHierarchyTest, newRootLayerIsMirrored) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     setFlags(12, layer_state_t::eLayerSkipScreenshot, layer_state_t::eLayerSkipScreenshot);
     createDisplayMirrorLayer(3, ui::LayerStack::fromValue(0));
     setLayerStack(3, 1);
@@ -719,7 +740,8 @@ TEST_F(LayerHierarchyTest, newRootLayerIsMirrored) {
 }
 
 TEST_F(LayerHierarchyTest, removedRootLayerIsNoLongerMirrored) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     setFlags(12, layer_state_t::eLayerSkipScreenshot, layer_state_t::eLayerSkipScreenshot);
     createDisplayMirrorLayer(3, ui::LayerStack::fromValue(0));
     setLayerStack(3, 1);
@@ -737,7 +759,8 @@ TEST_F(LayerHierarchyTest, removedRootLayerIsNoLongerMirrored) {
 }
 
 TEST_F(LayerHierarchyTest, canMirrorDisplayWithMirrors) {
-    LayerHierarchyBuilder hierarchyBuilder(mLifecycleManager.getLayers());
+    LayerHierarchyBuilder hierarchyBuilder;
+    hierarchyBuilder.update(mLifecycleManager);
     reparentLayer(12, UNASSIGNED_LAYER_ID);
     mirrorLayer(/*layer*/ 14, /*parent*/ 1, /*layerToMirror*/ 11);
     UPDATE_AND_VERIFY(hierarchyBuilder);
diff --git a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h
index 7e9abce690..67e624922c 100644
--- a/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h
+++ b/services/surfaceflinger/tests/unittests/LayerHierarchyTest.h
@@ -176,14 +176,12 @@ protected:
     void destroyLayerHandle(uint32_t id) { mLifecycleManager.onHandlesDestroyed({{id, "test"}}); }
 
     void updateAndVerify(LayerHierarchyBuilder& hierarchyBuilder) {
-        if (mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Hierarchy)) {
-            hierarchyBuilder.update(mLifecycleManager.getLayers(),
-                                    mLifecycleManager.getDestroyedLayers());
-        }
+        hierarchyBuilder.update(mLifecycleManager);
         mLifecycleManager.commitChanges();
 
         // rebuild layer hierarchy from scratch and verify that it matches the updated state.
-        LayerHierarchyBuilder newBuilder(mLifecycleManager.getLayers());
+        LayerHierarchyBuilder newBuilder;
+        newBuilder.update(mLifecycleManager);
         EXPECT_EQ(getTraversalPath(hierarchyBuilder.getHierarchy()),
                   getTraversalPath(newBuilder.getHierarchy()));
         EXPECT_EQ(getTraversalPathInZOrder(hierarchyBuilder.getHierarchy()),
@@ -512,10 +510,7 @@ protected:
     }
 
     void update(LayerSnapshotBuilder& snapshotBuilder) {
-        if (mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Hierarchy)) {
-            mHierarchyBuilder.update(mLifecycleManager.getLayers(),
-                                     mLifecycleManager.getDestroyedLayers());
-        }
+        mHierarchyBuilder.update(mLifecycleManager);
         LayerSnapshotBuilder::Args args{.root = mHierarchyBuilder.getHierarchy(),
                                         .layerLifecycleManager = mLifecycleManager,
                                         .includeMetadata = false,
@@ -530,7 +525,7 @@ protected:
         mLifecycleManager.commitChanges();
     }
 
-    LayerHierarchyBuilder mHierarchyBuilder{{}};
+    LayerHierarchyBuilder mHierarchyBuilder;
 
     DisplayInfos mFrontEndDisplayInfos;
     bool mHasDisplayChanges = false;
diff --git a/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp b/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp
index 787fa1c5e4..e9d231956f 100644
--- a/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerHistoryIntegrationTest.cpp
@@ -56,7 +56,7 @@ protected:
     LayerHistoryIntegrationTest() : LayerSnapshotTestBase() {
         mFlinger.resetScheduler(mScheduler);
         mLifecycleManager = {};
-        mHierarchyBuilder = {{}};
+        mHierarchyBuilder = {};
     }
 
     void updateLayerSnapshotsAndLayerHistory(nsecs_t now) {
diff --git a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
index 50cd784725..ba32c68b61 100644
--- a/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
+++ b/services/surfaceflinger/tests/unittests/LayerSnapshotTest.cpp
@@ -58,8 +58,7 @@ protected:
 
     void update(LayerSnapshotBuilder& actualBuilder, LayerSnapshotBuilder::Args& args) {
         if (mLifecycleManager.getGlobalChanges().test(RequestedLayerState::Changes::Hierarchy)) {
-            mHierarchyBuilder.update(mLifecycleManager.getLayers(),
-                                     mLifecycleManager.getDestroyedLayers());
+            mHierarchyBuilder.update(mLifecycleManager);
         }
         args.root = mHierarchyBuilder.getHierarchy();
         actualBuilder.update(args);
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index e515895a1a..2007d2a129 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -98,7 +98,7 @@ protected:
     mock::VsyncTrackerCallback mVsyncTrackerCallback;
     TestableScheduler* mScheduler =
             new TestableScheduler{mSelector, mSchedulerCallback, mVsyncTrackerCallback};
-    surfaceflinger::frontend::LayerHierarchyBuilder mLayerHierarchyBuilder{{}};
+    surfaceflinger::frontend::LayerHierarchyBuilder mLayerHierarchyBuilder;
 
     ConnectionHandle mConnectionHandle;
     MockEventThread* mEventThread;