Merge "Remove ScopedAddToTraversalPath" into main

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;