[PM] Remove Graph::VisitAll*Nodes()

All the calls are converted to use GetAll*Nodes(), which no longer
incurs a full container copy.

Change-Id: I84680b72243fe092b0607974accab65a3af99a44
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5600070
Commit-Queue: Patrick Monette <[email protected]>
Reviewed-by: Joe Mason <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1311736}
diff --git a/components/performance_manager/decorators/process_hosted_content_types_aggregator_unittest.cc b/components/performance_manager/decorators/process_hosted_content_types_aggregator_unittest.cc
index e82cc52..a44c1f9 100644
--- a/components/performance_manager/decorators/process_hosted_content_types_aggregator_unittest.cc
+++ b/components/performance_manager/decorators/process_hosted_content_types_aggregator_unittest.cc
@@ -54,8 +54,8 @@
   // an extension.
   frame_node.reset();
   page_node.reset();
-  EXPECT_EQ(graph()->GetFrameNodeCount(), 0u);
-  EXPECT_EQ(graph()->GetPageNodeCount(), 0u);
+  EXPECT_EQ(graph()->GetAllFrameNodes().size(), 0u);
+  EXPECT_EQ(graph()->GetAllPageNodes().size(), 0u);
 
   EXPECT_TRUE(IsHosting(process_node, ContentType::kExtension));
   EXPECT_TRUE(IsHosting(process_node, ContentType::kMainFrame));
@@ -118,7 +118,7 @@
   // Remove the frames. This shouldn't affect hosted content types.
   child_frame_node.reset();
   main_frame_node.reset();
-  EXPECT_EQ(graph()->GetFrameNodeCount(), 0u);
+  EXPECT_EQ(graph()->GetAllFrameNodes().size(), 0u);
 
   EXPECT_FALSE(IsHosting(process_node_1, ContentType::kExtension));
   EXPECT_TRUE(IsHosting(process_node_1, ContentType::kMainFrame));
@@ -187,7 +187,7 @@
   // Remove the worker node. The process is still counted as having hosted a
   // worker.
   worker_node.reset();
-  EXPECT_EQ(graph()->GetWorkerNodeCount(), 0u);
+  EXPECT_EQ(graph()->GetAllWorkerNodes().size(), 0u);
 
   EXPECT_FALSE(IsHosting(process_node, ContentType::kExtension));
   EXPECT_FALSE(IsHosting(process_node, ContentType::kMainFrame));
diff --git a/components/performance_manager/graph/graph_impl.cc b/components/performance_manager/graph/graph_impl.cc
index f01d290..440b233 100644
--- a/components/performance_manager/graph/graph_impl.cc
+++ b/components/performance_manager/graph/graph_impl.cc
@@ -256,22 +256,6 @@
   return system_node_.get();
 }
 
-size_t GraphImpl::GetProcessNodeCount() const {
-  return GetNodesOfType(NodeTypeEnum::kProcess).size();
-}
-
-size_t GraphImpl::GetFrameNodeCount() const {
-  return GetNodesOfType(NodeTypeEnum::kFrame).size();
-}
-
-size_t GraphImpl::GetPageNodeCount() const {
-  return GetNodesOfType(NodeTypeEnum::kPage).size();
-}
-
-size_t GraphImpl::GetWorkerNodeCount() const {
-  return GetNodesOfType(NodeTypeEnum::kWorker).size();
-}
-
 Graph::NodeSetView<const ProcessNode*> GraphImpl::GetAllProcessNodes() const {
   return NodeSetView<const ProcessNode*>(
       GetNodesOfType(NodeTypeEnum::kProcess));
@@ -289,22 +273,6 @@
   return NodeSetView<const WorkerNode*>(GetNodesOfType(NodeTypeEnum::kWorker));
 }
 
-bool GraphImpl::VisitAllProcessNodes(ProcessNodeVisitor visitor) const {
-  return VisitAllNodesOfType<ProcessNodeImpl, const ProcessNode*>(visitor);
-}
-
-bool GraphImpl::VisitAllFrameNodes(FrameNodeVisitor visitor) const {
-  return VisitAllNodesOfType<FrameNodeImpl, const FrameNode*>(visitor);
-}
-
-bool GraphImpl::VisitAllPageNodes(PageNodeVisitor visitor) const {
-  return VisitAllNodesOfType<PageNodeImpl, const PageNode*>(visitor);
-}
-
-bool GraphImpl::VisitAllWorkerNodes(WorkerNodeVisitor visitor) const {
-  return VisitAllNodesOfType<WorkerNodeImpl, const WorkerNode*>(visitor);
-}
-
 bool GraphImpl::HasOnlySystemNode() const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
   if (!GetNodesOfType(NodeTypeEnum::kProcess).empty() ||
@@ -402,22 +370,6 @@
   return NodeSetView<WorkerNodeImpl*>(GetNodesOfType(NodeTypeEnum::kWorker));
 }
 
-bool GraphImpl::VisitAllProcessNodeImpls(ProcessNodeImplVisitor visitor) const {
-  return VisitAllNodesOfType<ProcessNodeImpl, ProcessNodeImpl*>(visitor);
-}
-
-bool GraphImpl::VisitAllFrameNodeImpls(FrameNodeImplVisitor visitor) const {
-  return VisitAllNodesOfType<FrameNodeImpl, FrameNodeImpl*>(visitor);
-}
-
-bool GraphImpl::VisitAllPageNodeImpls(PageNodeImplVisitor visitor) const {
-  return VisitAllNodesOfType<PageNodeImpl, PageNodeImpl*>(visitor);
-}
-
-bool GraphImpl::VisitAllWorkerNodeImpls(WorkerNodeImplVisitor visitor) const {
-  return VisitAllNodesOfType<WorkerNodeImpl, WorkerNodeImpl*>(visitor);
-}
-
 size_t GraphImpl::GetNodeAttachedDataCountForTesting(const Node* node,
                                                      const void* key) const {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
@@ -694,22 +646,6 @@
   frames_by_id_.erase(process_and_frame_id);
 }
 
-template <typename NodeType, typename VisitedNodeType>
-bool GraphImpl::VisitAllNodesOfType(
-    base::FunctionRef<bool(VisitedNodeType)> visitor) const {
-  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
-  const auto type = NodeType::Type();
-  const NodeSet& nodes = GetNodesOfType(type);
-
-  for (const Node* node : nodes) {
-    VisitedNodeType visited_node = NodeType::FromNode(node);
-    if (!visitor(visited_node)) {
-      return false;
-    }
-  }
-  return true;
-}
-
 void GraphImpl::CreateSystemNode() {
   CHECK(!system_node_);
   // Create the singleton system node instance. Ownership is taken by the
diff --git a/components/performance_manager/graph/graph_impl.h b/components/performance_manager/graph/graph_impl.h
index b52e6611..84187f5 100644
--- a/components/performance_manager/graph/graph_impl.h
+++ b/components/performance_manager/graph/graph_impl.h
@@ -13,7 +13,6 @@
 #include <unordered_set>
 #include <utility>
 
-#include "base/functional/function_ref.h"
 #include "base/memory/raw_ptr.h"
 #include "base/observer_list.h"
 #include "base/process/process_handle.h"
@@ -45,11 +44,6 @@
 // a list of observers that are notified of node addition and removal.
 class GraphImpl : public Graph {
  public:
-  using FrameNodeImplVisitor = base::FunctionRef<bool(FrameNodeImpl*)>;
-  using PageNodeImplVisitor = base::FunctionRef<bool(PageNodeImpl*)>;
-  using ProcessNodeImplVisitor = base::FunctionRef<bool(ProcessNodeImpl*)>;
-  using WorkerNodeImplVisitor = base::FunctionRef<bool(WorkerNodeImpl*)>;
-
   // An ObserverList that DCHECK's if any observers are still in it when the
   // graph is deleted. `allow_reentrancy` is true because some observers update
   // node properties that also trigger observers. (For example
@@ -89,18 +83,10 @@
   void RegisterObject(GraphRegistered* object) override;
   void UnregisterObject(GraphRegistered* object) override;
   const SystemNode* GetSystemNode() const override;
-  size_t GetProcessNodeCount() const override;
-  size_t GetFrameNodeCount() const override;
-  size_t GetPageNodeCount() const override;
-  size_t GetWorkerNodeCount() const override;
   NodeSetView<const ProcessNode*> GetAllProcessNodes() const override;
   NodeSetView<const FrameNode*> GetAllFrameNodes() const override;
   NodeSetView<const PageNode*> GetAllPageNodes() const override;
   NodeSetView<const WorkerNode*> GetAllWorkerNodes() const override;
-  bool VisitAllProcessNodes(ProcessNodeVisitor visitor) const override;
-  bool VisitAllFrameNodes(FrameNodeVisitor visitor) const override;
-  bool VisitAllPageNodes(PageNodeVisitor visitor) const override;
-  bool VisitAllWorkerNodes(WorkerNodeVisitor visitor) const override;
 
   bool HasOnlySystemNode() const override;
   ukm::UkmRecorder* GetUkmRecorder() const override;
@@ -137,10 +123,6 @@
   NodeSetView<FrameNodeImpl*> GetAllFrameNodeImpls() const;
   NodeSetView<PageNodeImpl*> GetAllPageNodeImpls() const;
   NodeSetView<WorkerNodeImpl*> GetAllWorkerNodeImpls() const;
-  bool VisitAllProcessNodeImpls(ProcessNodeImplVisitor visitor) const;
-  bool VisitAllFrameNodeImpls(FrameNodeImplVisitor visitor) const;
-  bool VisitAllPageNodeImpls(PageNodeImplVisitor visitor) const;
-  bool VisitAllWorkerNodeImpls(WorkerNodeImplVisitor visitor) const;
 
   // Retrieves the process node with PID |pid|, if any.
   ProcessNodeImpl* GetProcessNodeByPid(base::ProcessId pid);
@@ -242,10 +224,6 @@
                                 int render_frame_id,
                                 FrameNodeImpl* frame_node);
 
-  template <typename NodeType, typename VisitedNodeType>
-  bool VisitAllNodesOfType(
-      base::FunctionRef<bool(VisitedNodeType)> visitor) const;
-
   void CreateSystemNode() VALID_CONTEXT_REQUIRED(sequence_checker_);
   void ReleaseSystemNode() VALID_CONTEXT_REQUIRED(sequence_checker_);
 
diff --git a/components/performance_manager/graph/graph_impl_unittest.cc b/components/performance_manager/graph/graph_impl_unittest.cc
index c43033b5..7356a94 100644
--- a/components/performance_manager/graph/graph_impl_unittest.cc
+++ b/components/performance_manager/graph/graph_impl_unittest.cc
@@ -6,6 +6,7 @@
 
 #include <string_view>
 
+#include "base/containers/contains.h"
 #include "base/memory/ptr_util.h"
 #include "base/memory/raw_ptr.h"
 #include "base/process/process.h"
@@ -24,7 +25,7 @@
 
 using GraphImplTest = GraphTestHarness;
 
-using ::testing::UnorderedElementsAreArray;
+using ::testing::ElementsAreArray;
 
 TEST_F(GraphImplTest, SafeCasting) {
   const Graph* graph_base = graph();
@@ -112,118 +113,54 @@
   EXPECT_NE(nullptr, pages[1]);
 }
 
-TEST_F(GraphImplTest, Visitors) {
+TEST_F(GraphImplTest, GetAllNodes) {
+  // This mock graphs contains 2 pages with 2 main frames in a single process.
+  // There is a total of 2 process nodes because of the browser process node.
   MockMultiplePagesInSingleProcessGraph mock_graph(graph());
 
-  std::vector<const FrameNode*> visited_frame_nodes;
-  EXPECT_EQ(true, graph()->VisitAllFrameNodes([&](const FrameNode* node) {
-    visited_frame_nodes.push_back(node);
-    return true;
-  }));
-  EXPECT_THAT(visited_frame_nodes,
-              UnorderedElementsAreArray(graph()->GetAllFrameNodes()));
+  // 1 renderer and 1 browser process.
+  auto process_nodes = graph()->GetAllProcessNodes().AsVector();
+  EXPECT_EQ(process_nodes.size(), 2u);
+  EXPECT_TRUE(base::Contains(process_nodes, mock_graph.process.get()));
 
-  std::vector<const PageNode*> visited_page_nodes;
-  EXPECT_EQ(true, graph()->VisitAllPageNodes([&](const PageNode* node) {
-    visited_page_nodes.push_back(node);
-    return true;
-  }));
-  EXPECT_THAT(visited_page_nodes,
-              UnorderedElementsAreArray(graph()->GetAllPageNodes()));
+  // 2 pages.
+  EXPECT_THAT(graph()->GetAllPageNodes().AsVector(),
+              ::testing::UnorderedElementsAre(mock_graph.page.get(),
+                                              mock_graph.other_page.get()));
 
-  std::vector<const ProcessNode*> visited_process_nodes;
-  EXPECT_EQ(true, graph()->VisitAllProcessNodes([&](const ProcessNode* node) {
-    visited_process_nodes.push_back(node);
-    return true;
-  }));
-  EXPECT_THAT(visited_process_nodes,
-              UnorderedElementsAreArray(graph()->GetAllProcessNodes()));
+  // 2 frames.
+  EXPECT_THAT(graph()->GetAllFrameNodes().AsVector(),
+              ::testing::UnorderedElementsAre(mock_graph.frame.get(),
+                                              mock_graph.other_frame.get()));
 
-  std::vector<const WorkerNode*> visited_worker_nodes;
-  EXPECT_EQ(true, graph()->VisitAllWorkerNodes([&](const WorkerNode* node) {
-    visited_worker_nodes.push_back(node);
-    return true;
-  }));
-  EXPECT_THAT(visited_worker_nodes,
-              UnorderedElementsAreArray(graph()->GetAllWorkerNodes()));
-
-  size_t visited_nodes = 0;
-  EXPECT_EQ(false, graph()->VisitAllFrameNodes([&](const FrameNode*) {
-    ++visited_nodes;
-    return false;
-  }));
-  EXPECT_EQ(false, graph()->VisitAllPageNodes([&](const PageNode*) {
-    ++visited_nodes;
-    return false;
-  }));
-  EXPECT_EQ(false, graph()->VisitAllProcessNodes([&](const ProcessNode*) {
-    ++visited_nodes;
-    return false;
-  }));
-  // There are no worker nodes, so the visitor never runs.
-  EXPECT_EQ(true, graph()->VisitAllWorkerNodes([&](const WorkerNode*) {
-    ++visited_nodes;
-    return false;
-  }));
-  // Visited one node of each type except workers.
-  EXPECT_EQ(visited_nodes, 3u);
+  // No workers.
+  EXPECT_THAT(graph()->GetAllWorkerNodes().AsVector(),
+              ::testing::UnorderedElementsAre());
 }
 
-TEST_F(GraphImplTest, ImplVisitors) {
+TEST_F(GraphImplTest, GetAllNodeImpls) {
+  // This mock graphs contains 2 pages with 2 main frames in a single process.
+  // There is a total of 2 process nodes because of the browser process node.
   MockMultiplePagesInSingleProcessGraph mock_graph(graph());
 
-  std::vector<FrameNodeImpl*> visited_frame_nodes;
-  EXPECT_EQ(true, graph()->VisitAllFrameNodeImpls([&](FrameNodeImpl* node) {
-    visited_frame_nodes.push_back(node);
-    return true;
-  }));
-  EXPECT_THAT(visited_frame_nodes,
-              UnorderedElementsAreArray(graph()->GetAllFrameNodeImpls()));
+  // 1 renderer and 1 browser process.
+  auto process_nodes = graph()->GetAllProcessNodeImpls().AsVector();
+  EXPECT_EQ(process_nodes.size(), 2u);
+  EXPECT_TRUE(base::Contains(process_nodes, mock_graph.process.get()));
 
-  std::vector<PageNodeImpl*> visited_page_nodes;
-  EXPECT_EQ(true, graph()->VisitAllPageNodeImpls([&](PageNodeImpl* node) {
-    visited_page_nodes.push_back(node);
-    return true;
-  }));
-  EXPECT_THAT(visited_page_nodes,
-              UnorderedElementsAreArray(graph()->GetAllPageNodeImpls()));
+  // 2 pages.
+  EXPECT_THAT(graph()->GetAllPageNodeImpls().AsVector(),
+              ::testing::UnorderedElementsAre(mock_graph.page.get(),
+                                              mock_graph.other_page.get()));
 
-  std::vector<ProcessNodeImpl*> visited_process_nodes;
-  EXPECT_EQ(true, graph()->VisitAllProcessNodeImpls([&](ProcessNodeImpl* node) {
-    visited_process_nodes.push_back(node);
-    return true;
-  }));
-  EXPECT_THAT(visited_process_nodes,
-              UnorderedElementsAreArray(graph()->GetAllProcessNodeImpls()));
+  // 2 frames.
+  EXPECT_THAT(graph()->GetAllFrameNodeImpls().AsVector(),
+              ::testing::UnorderedElementsAre(mock_graph.frame.get(),
+                                              mock_graph.other_frame.get()));
 
-  std::vector<WorkerNodeImpl*> visited_worker_nodes;
-  EXPECT_EQ(true, graph()->VisitAllWorkerNodeImpls([&](WorkerNodeImpl* node) {
-    visited_worker_nodes.push_back(node);
-    return true;
-  }));
-  EXPECT_THAT(visited_worker_nodes,
-              UnorderedElementsAreArray(graph()->GetAllWorkerNodeImpls()));
-
-  size_t visited_nodes = 0;
-  EXPECT_EQ(false, graph()->VisitAllFrameNodeImpls([&](FrameNodeImpl*) {
-    ++visited_nodes;
-    return false;
-  }));
-  EXPECT_EQ(false, graph()->VisitAllPageNodeImpls([&](PageNodeImpl*) {
-    ++visited_nodes;
-    return false;
-  }));
-  EXPECT_EQ(false, graph()->VisitAllProcessNodeImpls([&](ProcessNodeImpl*) {
-    ++visited_nodes;
-    return false;
-  }));
-  // There are no worker nodes, so the visitor never runs.
-  EXPECT_EQ(true, graph()->VisitAllWorkerNodeImpls([&](WorkerNodeImpl*) {
-    ++visited_nodes;
-    return false;
-  }));
-  // Visited one node of each type except workers.
-  EXPECT_EQ(visited_nodes, 3u);
+  // No workers.
+  EXPECT_THAT(graph()->GetAllWorkerNodeImpls().AsVector(),
+              ::testing::UnorderedElementsAre());
 }
 
 namespace {
diff --git a/components/performance_manager/graph/policies/bfcache_policy.cc b/components/performance_manager/graph/policies/bfcache_policy.cc
index de13ea3..9be4eb8 100644
--- a/components/performance_manager/graph/policies/bfcache_policy.cc
+++ b/components/performance_manager/graph/policies/bfcache_policy.cc
@@ -141,13 +141,12 @@
   }
 
   // Apply the cache limit to all pages.
-  graph_->VisitAllPageNodes([&, this](const PageNode* page_node) {
+  for (const PageNode* page_node : graph_->GetAllPageNodes()) {
     if (page_node->GetPageState() == PageNode::PageState::kActive &&
         PageMightHaveFramesInBFCache(page_node)) {
       MaybeFlushBFCache(page_node, new_level);
     }
-    return true;
-  });
+  }
 }
 
 }  // namespace performance_manager::policies
diff --git a/components/performance_manager/metrics/metrics_collector.cc b/components/performance_manager/metrics/metrics_collector.cc
index 2f96c0a..c6071ea 100644
--- a/components/performance_manager/metrics/metrics_collector.cc
+++ b/components/performance_manager/metrics/metrics_collector.cc
@@ -115,18 +115,16 @@
     return;
   }
 
-  graph_->VisitAllPageNodes([&](const PageNode* page_node_it) {
-    if (page_node_it != page_node) {
-      if (page_node_it->GetBrowserContextID() ==
-              page_node->GetBrowserContextID() &&
-          url::IsSameOriginWith(page_node_it->GetMainFrameUrl(),
+  for (const PageNode* page : graph_->GetAllPageNodes()) {
+    if (page != page_node) {
+      if (page->GetBrowserContextID() == page_node->GetBrowserContextID() &&
+          url::IsSameOriginWith(page->GetMainFrameUrl(),
                                 page_node->GetMainFrameUrl())) {
         found_same_origin_page = true;
-        return false;
+        break;
       }
     }
-    return true;
-  });
+  }
   record->previous_url = page_node->GetMainFrameUrl();
   base::UmaHistogramBoolean(kTabNavigationWithSameOriginTabHistogramName,
                             found_same_origin_page);
diff --git a/components/performance_manager/performance_manager_tab_helper_unittest.cc b/components/performance_manager/performance_manager_tab_helper_unittest.cc
index 6b5cb23..b54f29b 100644
--- a/components/performance_manager/performance_manager_tab_helper_unittest.cc
+++ b/components/performance_manager/performance_manager_tab_helper_unittest.cc
@@ -104,7 +104,7 @@
   // Check out the graph itself.
   RunInGraph([&process_nodes, num_hosts, grandchild_url](GraphImpl* graph) {
     EXPECT_GE(num_hosts, CountAllRenderProcessNodes(graph));
-    EXPECT_EQ(4u, graph->GetFrameNodeCount());
+    EXPECT_EQ(4u, graph->GetAllFrameNodes().size());
 
     // Expect all frame nodes to be current. This fails if our
     // implementation of RenderFrameHostChanged is borked.
@@ -112,7 +112,7 @@
       EXPECT_TRUE(frame->IsCurrent());
     }
 
-    ASSERT_EQ(1u, graph->GetPageNodeCount());
+    ASSERT_EQ(1u, graph->GetAllPageNodes().size());
     auto* page = graph->GetAllPageNodeImpls().AsVector()[0];
 
     // Extra RPHs can and most definitely do exist.
@@ -203,8 +203,8 @@
 
   RunInGraph([num_hosts](GraphImpl* graph) {
     EXPECT_GE(num_hosts, CountAllRenderProcessNodes(graph));
-    EXPECT_EQ(0u, graph->GetFrameNodeCount());
-    ASSERT_EQ(0u, graph->GetPageNodeCount());
+    EXPECT_EQ(0u, graph->GetAllFrameNodes().size());
+    ASSERT_EQ(0u, graph->GetAllPageNodes().size());
   });
 }
 
@@ -212,7 +212,7 @@
 
 void ExpectPageIsAudible(bool is_audible) {
   RunInGraph([&](GraphImpl* graph) {
-    ASSERT_EQ(1u, graph->GetPageNodeCount());
+    ASSERT_EQ(1u, graph->GetAllPageNodes().size());
     auto* page = graph->GetAllPageNodeImpls().AsVector()[0];
     EXPECT_EQ(is_audible, page->IsAudible());
   });
@@ -222,7 +222,7 @@
 void ExpectNotificationPermissionStatus(
     std::optional<blink::mojom::PermissionStatus> status) {
   RunInGraph([&](GraphImpl* graph) {
-    ASSERT_EQ(1u, graph->GetPageNodeCount());
+    ASSERT_EQ(1u, graph->GetAllPageNodes().size());
     auto* page = graph->GetAllPageNodeImpls().AsVector()[0];
     EXPECT_EQ(status, page->GetNotificationPermissionStatus());
   });
diff --git a/components/performance_manager/public/graph/graph.h b/components/performance_manager/public/graph/graph.h
index 6b83c5b..ed5ca19 100644
--- a/components/performance_manager/public/graph/graph.h
+++ b/components/performance_manager/public/graph/graph.h
@@ -10,7 +10,6 @@
 #include <unordered_set>
 
 #include "base/dcheck_is_on.h"
-#include "base/functional/function_ref.h"
 #include "base/memory/ptr_util.h"
 #include "base/observer_list_types.h"
 #include "components/performance_manager/public/graph/node_set_view.h"
@@ -47,10 +46,6 @@
   using NodeSet = std::unordered_set<const Node*>;
   template <class NodeViewPtr>
   using NodeSetView = NodeSetView<NodeSet, NodeViewPtr>;
-  using FrameNodeVisitor = base::FunctionRef<bool(const FrameNode*)>;
-  using PageNodeVisitor = base::FunctionRef<bool(const PageNode*)>;
-  using ProcessNodeVisitor = base::FunctionRef<bool(const ProcessNode*)>;
-  using WorkerNodeVisitor = base::FunctionRef<bool(const WorkerNode*)>;
 
   Graph();
 
@@ -122,29 +117,12 @@
   // Returns the single system node.
   virtual const SystemNode* GetSystemNode() const = 0;
 
-  // Returns the count of all known nodes of the given type.
-  virtual size_t GetProcessNodeCount() const = 0;
-  virtual size_t GetFrameNodeCount() const = 0;
-  virtual size_t GetPageNodeCount() const = 0;
-  virtual size_t GetWorkerNodeCount() const = 0;
-
-  // Returns a collection of all known nodes of the given type. Note that this
-  // incurs a full container copy of all returned nodes. Please use
-  // VisitAll*Nodes() when that makes sense.
+  // Returns a collection of all known nodes of the given type.
   virtual NodeSetView<const ProcessNode*> GetAllProcessNodes() const = 0;
   virtual NodeSetView<const FrameNode*> GetAllFrameNodes() const = 0;
   virtual NodeSetView<const PageNode*> GetAllPageNodes() const = 0;
   virtual NodeSetView<const WorkerNode*> GetAllWorkerNodes() const = 0;
 
-  // Visits all nodes in the graph of the given type, invoking the provided
-  // `visitor` for each. If the visitor returns false then then the iteration is
-  // halted. The visitor must not modify the graph. Returns true if all calls to
-  // the visitor returned true, false otherwise.
-  virtual bool VisitAllProcessNodes(ProcessNodeVisitor visitor) const = 0;
-  virtual bool VisitAllFrameNodes(FrameNodeVisitor visitor) const = 0;
-  virtual bool VisitAllPageNodes(PageNodeVisitor visitor) const = 0;
-  virtual bool VisitAllWorkerNodes(WorkerNodeVisitor visitor) const = 0;
-
   // Returns true if the graph only contains the default nodes.
   virtual bool HasOnlySystemNode() const = 0;
 
diff --git a/components/performance_manager/resource_attribution/cpu_measurement_monitor.cc b/components/performance_manager/resource_attribution/cpu_measurement_monitor.cc
index f149429..cb06868 100644
--- a/components/performance_manager/resource_attribution/cpu_measurement_monitor.cc
+++ b/components/performance_manager/resource_attribution/cpu_measurement_monitor.cc
@@ -128,13 +128,11 @@
 
   // Start monitoring CPU usage for all existing processes. Can't read their CPU
   // usage until they have a pid assigned.
-  graph_->VisitAllProcessNodes([this](const ProcessNode* process_node) {
-    DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  for (const ProcessNode* process_node : graph_->GetAllProcessNodes()) {
     if (delegate_factory_->ShouldMeasureProcess(process_node)) {
       MonitorCPUUsage(process_node);
     }
-    return true;
-  });
+  }
 }
 
 void CPUMeasurementMonitor::StopMonitoring() {
diff --git a/components/performance_manager/resource_attribution/worker_context_unittest.cc b/components/performance_manager/resource_attribution/worker_context_unittest.cc
index fa35c4e6..d5b6746b 100644
--- a/components/performance_manager/resource_attribution/worker_context_unittest.cc
+++ b/components/performance_manager/resource_attribution/worker_context_unittest.cc
@@ -119,7 +119,7 @@
   EXPECT_EQ(worker_token, worker_context->GetWorkerToken());
   EXPECT_FALSE(WorkerContext::FromWorkerToken(worker_token).has_value());
   performance_manager::RunInGraph([&](Graph* graph) {
-    EXPECT_EQ(graph->GetWorkerNodeCount(), 0u);
+    EXPECT_EQ(graph->GetAllWorkerNodes().size(), 0u);
     EXPECT_FALSE(worker_node);
     EXPECT_EQ(nullptr, worker_context->GetWorkerNode());
     EXPECT_EQ(std::nullopt, WorkerContext::FromWeakWorkerNode(worker_node));
diff --git a/components/performance_manager/v8_memory/v8_detailed_memory_decorator.cc b/components/performance_manager/v8_memory/v8_detailed_memory_decorator.cc
index 6d2917ab..1def29a 100644
--- a/components/performance_manager/v8_memory/v8_detailed_memory_decorator.cc
+++ b/components/performance_manager/v8_memory/v8_detailed_memory_decorator.cc
@@ -338,16 +338,15 @@
 void NodeAttachedProcessData::ApplyToAllRenderers(
     Graph* graph,
     base::FunctionRef<void(NodeAttachedProcessData*)> func) {
-  graph->VisitAllProcessNodes([&](const ProcessNode* node) {
+  for (const ProcessNode* node : graph->GetAllProcessNodes()) {
     if (node->GetProcessType() != content::PROCESS_TYPE_RENDERER) {
-      return true;
+      continue;
     }
 
     NodeAttachedProcessData* process_data = NodeAttachedProcessData::Get(node);
     DCHECK(process_data);
     func(process_data);
-    return true;
-  });
+  }
 }
 
 void NodeAttachedProcessData::ScheduleNextMeasurement() {
@@ -672,10 +671,9 @@
   graph->RegisterObject(this);
 
   // Iterate over the existing process nodes to put them under observation.
-  graph->VisitAllProcessNodes([this](const ProcessNode* process_node) {
+  for (const ProcessNode* process_node : graph->GetAllProcessNodes()) {
     OnProcessNodeAdded(process_node);
-    return true;
-  });
+  }
 
   graph->AddProcessNodeObserver(this);
   graph->GetNodeDataDescriberRegistry()->RegisterDescriber(