Remove ScopedAddToTraversalPath

ScopedAddToTraversalPath is an RAII wrapper that copies the existing traversal path, modifies it, and then restores the modified properties from the copy when deleted. It's simpler to make the child path a copy and not modify the parent.

Bug: 403312802
Flag: EXEMPT refactor
Test: presubmits
Change-Id: I8f06ed557f29444be6df51c1c8ea60958a82ee95

6 files changed, 48 insertions, 85 deletions

diff --git a/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp b/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp
index da536b6660..00ec863bd9 100644
--- a/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerHierarchy.cpp
@@ -54,7 +54,8 @@ LayerHierarchy::LayerHierarchy(const LayerHierarchy& hierarchy, bool childrenOnl
     mChildren = hierarchy.mChildren;
 }
 
-void LayerHierarchy::traverse(const Visitor& visitor, LayerHierarchy::TraversalPath& traversalPath,
+void LayerHierarchy::traverse(const Visitor& visitor,
+                              const LayerHierarchy::TraversalPath& traversalPath,
                               uint32_t depth) const {
     LLOG_ALWAYS_FATAL_WITH_TRACE_IF(depth > 50,
                                     "Cycle detected in LayerHierarchy::traverse. See "
@@ -70,14 +71,13 @@ void LayerHierarchy::traverse(const Visitor& visitor, LayerHierarchy::TraversalP
     LLOG_ALWAYS_FATAL_WITH_TRACE_IF(traversalPath.hasRelZLoop(), "Found relative z loop layerId:%d",
                                     traversalPath.invalidRelativeRootId);
     for (auto& [child, childVariant] : mChildren) {
-        ScopedAddToTraversalPath addChildToTraversalPath(traversalPath, child->mLayer->id,
-                                                         childVariant);
-        child->traverse(visitor, traversalPath, depth + 1);
+        child->traverse(visitor, traversalPath.makeChild(child->mLayer->id, childVariant),
+                        depth + 1);
     }
 }
 
 void LayerHierarchy::traverseInZOrder(const Visitor& visitor,
-                                      LayerHierarchy::TraversalPath& traversalPath) const {
+                                      const LayerHierarchy::TraversalPath& traversalPath) const {
     bool traverseThisLayer = (mLayer != nullptr);
     for (auto it = mChildren.begin(); it < mChildren.end(); it++) {
         auto& [child, childVariant] = *it;
@@ -91,9 +91,7 @@ void LayerHierarchy::traverseInZOrder(const Visitor& visitor,
         if (childVariant == LayerHierarchy::Variant::Detached) {
             continue;
         }
-        ScopedAddToTraversalPath addChildToTraversalPath(traversalPath, child->mLayer->id,
-                                                         childVariant);
-        child->traverseInZOrder(visitor, traversalPath);
+        child->traverseInZOrder(visitor, traversalPath.makeChild(child->mLayer->id, childVariant));
     }
 
     if (traverseThisLayer) {
@@ -568,42 +566,23 @@ std::string LayerHierarchy::TraversalPath::toString() const {
     return ss.str();
 }
 
-// Helper class to update a passed in TraversalPath when visiting a child. When the object goes out
-// of scope the TraversalPath is reset to its original state.
-LayerHierarchy::ScopedAddToTraversalPath::ScopedAddToTraversalPath(TraversalPath& traversalPath,
-                                                                   uint32_t layerId,
-                                                                   LayerHierarchy::Variant variant)
-      : mTraversalPath(traversalPath), mParentPath(traversalPath) {
-    // Update the traversal id with the child layer id and variant. Parent id and variant are
-    // stored to reset the id upon destruction.
-    traversalPath.id = layerId;
-    traversalPath.variant = variant;
+LayerHierarchy::TraversalPath LayerHierarchy::TraversalPath::makeChild(
+        uint32_t layerId, LayerHierarchy::Variant variant) const {
+    TraversalPath child{*this};
+    child.id = layerId;
+    child.variant = variant;
     if (LayerHierarchy::isMirror(variant)) {
-        traversalPath.mirrorRootIds.emplace_back(mParentPath.id);
+        child.mirrorRootIds.emplace_back(id);
     } else if (variant == LayerHierarchy::Variant::Relative) {
-        if (std::find(traversalPath.relativeRootIds.begin(), traversalPath.relativeRootIds.end(),
-                      layerId) != traversalPath.relativeRootIds.end()) {
-            traversalPath.invalidRelativeRootId = layerId;
+        if (std::find(relativeRootIds.begin(), relativeRootIds.end(), layerId) !=
+            relativeRootIds.end()) {
+            child.invalidRelativeRootId = layerId;
         }
-        traversalPath.relativeRootIds.emplace_back(layerId);
+        child.relativeRootIds.emplace_back(layerId);
     } else if (variant == LayerHierarchy::Variant::Detached) {
-        traversalPath.detached = true;
+        child.detached = true;
     }
-}
-LayerHierarchy::ScopedAddToTraversalPath::~ScopedAddToTraversalPath() {
-    // Reset the traversal id to its original parent state using the state that was saved in
-    // the constructor.
-    if (LayerHierarchy::isMirror(mTraversalPath.variant)) {
-        mTraversalPath.mirrorRootIds.pop_back();
-    } else if (mTraversalPath.variant == LayerHierarchy::Variant::Relative) {
-        mTraversalPath.relativeRootIds.pop_back();
-    }
-    if (mTraversalPath.invalidRelativeRootId == mTraversalPath.id) {
-        mTraversalPath.invalidRelativeRootId = UNASSIGNED_LAYER_ID;
-    }
-    mTraversalPath.id = mParentPath.id;
-    mTraversalPath.variant = mParentPath.variant;
-    mTraversalPath.detached = mParentPath.detached;
+    return child;
 }
 
 } // namespace android::surfaceflinger::frontend
diff --git a/services/surfaceflinger/FrontEnd/LayerHierarchy.h b/services/surfaceflinger/FrontEnd/LayerHierarchy.h
index 4fdbae1831..c8c6b4df15 100644
--- a/services/surfaceflinger/FrontEnd/LayerHierarchy.h
+++ b/services/surfaceflinger/FrontEnd/LayerHierarchy.h
@@ -104,6 +104,8 @@ public:
 
         TraversalPath getClonedFrom() const { return {.id = id, .variant = variant}; }
 
+        TraversalPath makeChild(uint32_t layerId, LayerHierarchy::Variant variant) const;
+
         bool operator==(const TraversalPath& other) const {
             return id == other.id && mirrorRootIds == other.mirrorRootIds;
         }
@@ -122,18 +124,6 @@ public:
         }
     };
 
-    // Helper class to add nodes to an existing traversal id and removes the
-    // node when it goes out of scope.
-    class ScopedAddToTraversalPath {
-    public:
-        ScopedAddToTraversalPath(TraversalPath& traversalPath, uint32_t layerId,
-                                 LayerHierarchy::Variant variantArg);
-        ~ScopedAddToTraversalPath();
-
-    private:
-        TraversalPath& mTraversalPath;
-        TraversalPath mParentPath;
-    };
     LayerHierarchy(RequestedLayerState* layer);
 
     // Visitor function that provides the hierarchy node and a traversal id which uniquely
@@ -191,8 +181,9 @@ private:
     void removeChild(LayerHierarchy*);
     void sortChildrenByZOrder();
     void updateChild(LayerHierarchy*, LayerHierarchy::Variant);
-    void traverseInZOrder(const Visitor& visitor, LayerHierarchy::TraversalPath& parent) const;
-    void traverse(const Visitor& visitor, LayerHierarchy::TraversalPath& parent,
+    void traverseInZOrder(const Visitor& visitor,
+                          const LayerHierarchy::TraversalPath& parent) const;
+    void traverse(const Visitor& visitor, const LayerHierarchy::TraversalPath& parent,
                   uint32_t depth = 0) const;
     void dump(std::ostream& out, const std::string& prefix, LayerHierarchy::Variant variant,
               bool isLastChild, bool includeMirroredHierarchy) const;
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
index 28a6031c97..50ed72de9e 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.cpp
@@ -447,15 +447,14 @@ void LayerSnapshotBuilder::updateSnapshots(const Args& args) {
     if (args.root.getLayer()) {
         // The hierarchy can have a root layer when used for screenshots otherwise, it will have
         // multiple children.
-        LayerHierarchy::ScopedAddToTraversalPath addChildToPath(root, args.root.getLayer()->id,
-                                                                LayerHierarchy::Variant::Attached);
-        updateSnapshotsInHierarchy(args, args.root, root, rootSnapshot, /*depth=*/0);
+        LayerHierarchy::TraversalPath childPath =
+                root.makeChild(args.root.getLayer()->id, LayerHierarchy::Variant::Attached);
+        updateSnapshotsInHierarchy(args, args.root, childPath, rootSnapshot, /*depth=*/0);
     } else {
         for (auto& [childHierarchy, variant] : args.root.mChildren) {
-            LayerHierarchy::ScopedAddToTraversalPath addChildToPath(root,
-                                                                    childHierarchy->getLayer()->id,
-                                                                    variant);
-            updateSnapshotsInHierarchy(args, *childHierarchy, root, rootSnapshot, /*depth=*/0);
+            LayerHierarchy::TraversalPath childPath =
+                    root.makeChild(childHierarchy->getLayer()->id, variant);
+            updateSnapshotsInHierarchy(args, *childHierarchy, childPath, rootSnapshot, /*depth=*/0);
         }
     }
 
@@ -520,7 +519,7 @@ void LayerSnapshotBuilder::update(const Args& args) {
 
 const LayerSnapshot& LayerSnapshotBuilder::updateSnapshotsInHierarchy(
         const Args& args, const LayerHierarchy& hierarchy,
-        LayerHierarchy::TraversalPath& traversalPath, const LayerSnapshot& parentSnapshot,
+        const LayerHierarchy::TraversalPath& traversalPath, const LayerSnapshot& parentSnapshot,
         int depth) {
     LLOG_ALWAYS_FATAL_WITH_TRACE_IF(depth > 50,
                                     "Cycle detected in LayerSnapshotBuilder. See "
@@ -549,12 +548,10 @@ const LayerSnapshot& LayerSnapshotBuilder::updateSnapshotsInHierarchy(
 
     bool childHasValidFrameRate = false;
     for (auto& [childHierarchy, variant] : hierarchy.mChildren) {
-        LayerHierarchy::ScopedAddToTraversalPath addChildToPath(traversalPath,
-                                                                childHierarchy->getLayer()->id,
-                                                                variant);
+        LayerHierarchy::TraversalPath childPath =
+                traversalPath.makeChild(childHierarchy->getLayer()->id, variant);
         const LayerSnapshot& childSnapshot =
-                updateSnapshotsInHierarchy(args, *childHierarchy, traversalPath, *snapshot,
-                                           depth + 1);
+                updateSnapshotsInHierarchy(args, *childHierarchy, childPath, *snapshot, depth + 1);
         updateFrameRateFromChildSnapshot(*snapshot, childSnapshot, *childHierarchy->getLayer(),
                                          args, &childHasValidFrameRate);
     }
diff --git a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h
index 486cb33959..94b7e5fa5a 100644
--- a/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h
+++ b/services/surfaceflinger/FrontEnd/LayerSnapshotBuilder.h
@@ -106,9 +106,10 @@ private:
 
     void updateSnapshots(const Args& args);
 
-    const LayerSnapshot& updateSnapshotsInHierarchy(const Args&, const LayerHierarchy& hierarchy,
-                                                    LayerHierarchy::TraversalPath& traversalPath,
-                                                    const LayerSnapshot& parentSnapshot, int depth);
+    const LayerSnapshot& updateSnapshotsInHierarchy(
+            const Args&, const LayerHierarchy& hierarchy,
+            const LayerHierarchy::TraversalPath& traversalPath, const LayerSnapshot& parentSnapshot,
+            int depth);
     void updateSnapshot(LayerSnapshot&, const Args&, const RequestedLayerState&,
                         const LayerSnapshot& parentSnapshot, const LayerHierarchy::TraversalPath&);
     static void updateRelativeState(LayerSnapshot& snapshot, const LayerSnapshot& parentSnapshot,
diff --git a/services/surfaceflinger/LayerProtoHelper.cpp b/services/surfaceflinger/LayerProtoHelper.cpp
index 84b1a73e0b..280d66e12a 100644
--- a/services/surfaceflinger/LayerProtoHelper.cpp
+++ b/services/surfaceflinger/LayerProtoHelper.cpp
@@ -278,10 +278,9 @@ LayerProtoFromSnapshotGenerator& LayerProtoFromSnapshotGenerator::with(
             stackIdsToSkip.find(child->getLayer()->layerStack.id) != stackIdsToSkip.end()) {
             continue;
         }
-        frontend::LayerHierarchy::ScopedAddToTraversalPath addChildToPath(path,
-                                                                          child->getLayer()->id,
-                                                                          variant);
-        LayerProtoFromSnapshotGenerator::writeHierarchyToProto(*child, path);
+        LayerProtoFromSnapshotGenerator::writeHierarchyToProto(*child,
+                                                               path.makeChild(child->getLayer()->id,
+                                                                              variant));
     }
 
     // fill in relative and parent info
@@ -338,7 +337,8 @@ LayerProtoFromSnapshotGenerator& LayerProtoFromSnapshotGenerator::withOffscreenL
 }
 
 frontend::LayerSnapshot* LayerProtoFromSnapshotGenerator::getSnapshot(
-        frontend::LayerHierarchy::TraversalPath& path, const frontend::RequestedLayerState& layer) {
+        const frontend::LayerHierarchy::TraversalPath& path,
+        const frontend::RequestedLayerState& layer) {
     frontend::LayerSnapshot* snapshot = mSnapshotBuilder.getSnapshot(path);
     if (snapshot) {
         return snapshot;
@@ -349,7 +349,7 @@ frontend::LayerSnapshot* LayerProtoFromSnapshotGenerator::getSnapshot(
 }
 
 void LayerProtoFromSnapshotGenerator::writeHierarchyToProto(
-        const frontend::LayerHierarchy& root, frontend::LayerHierarchy::TraversalPath& path) {
+        const frontend::LayerHierarchy& root, const frontend::LayerHierarchy::TraversalPath& path) {
     using Variant = frontend::LayerHierarchy::Variant;
     perfetto::protos::LayerProto* layerProto = mLayersProto.add_layers();
     const frontend::RequestedLayerState& layer = *root.getLayer();
@@ -362,10 +362,8 @@ void LayerProtoFromSnapshotGenerator::writeHierarchyToProto(
     LayerProtoHelper::writeSnapshotToProto(layerProto, layer, *snapshot, mTraceFlags);
 
     for (const auto& [child, variant] : root.mChildren) {
-        frontend::LayerHierarchy::ScopedAddToTraversalPath addChildToPath(path,
-                                                                          child->getLayer()->id,
-                                                                          variant);
-        frontend::LayerSnapshot* childSnapshot = getSnapshot(path, layer);
+        frontend::LayerSnapshot* childSnapshot =
+                getSnapshot(path.makeChild(child->getLayer()->id, variant), layer);
         if (variant == Variant::Attached || variant == Variant::Detached ||
             frontend::LayerHierarchy::isMirror(variant)) {
             mChildToParent[childSnapshot->uniqueSequence] = snapshot->uniqueSequence;
@@ -388,10 +386,7 @@ void LayerProtoFromSnapshotGenerator::writeHierarchyToProto(
         if (variant == Variant::Detached) {
             continue;
         }
-        frontend::LayerHierarchy::ScopedAddToTraversalPath addChildToPath(path,
-                                                                          child->getLayer()->id,
-                                                                          variant);
-        writeHierarchyToProto(*child, path);
+        writeHierarchyToProto(*child, path.makeChild(child->getLayer()->id, variant));
     }
 }
 
diff --git a/services/surfaceflinger/LayerProtoHelper.h b/services/surfaceflinger/LayerProtoHelper.h
index 3ca553a903..28924e45be 100644
--- a/services/surfaceflinger/LayerProtoHelper.h
+++ b/services/surfaceflinger/LayerProtoHelper.h
@@ -98,8 +98,8 @@ public:
 
 private:
     void writeHierarchyToProto(const frontend::LayerHierarchy& root,
-                               frontend::LayerHierarchy::TraversalPath& path);
-    frontend::LayerSnapshot* getSnapshot(frontend::LayerHierarchy::TraversalPath& path,
+                               const frontend::LayerHierarchy::TraversalPath& path);
+    frontend::LayerSnapshot* getSnapshot(const frontend::LayerHierarchy::TraversalPath& path,
                                          const frontend::RequestedLayerState& layer);
 
     const frontend::LayerSnapshotBuilder& mSnapshotBuilder;