[PM] Move WebMemoryAggregator result state into a Visitor helper class

Moves the code that statefully updates the aggregation result into an
AggregationPointVisitor helper class. This will make it easier to
implement new algorithms that gather different information from each
aggregation point (eg. finding the set of processes spanned by all
aggregation points) by subclassing AggregationPointVisitor.

R=siggi

Bug: 1168573
Change-Id: I0b674fdc4282e164e9541ed17ca6b53483bb525e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2640675
Commit-Queue: Joe Mason <joenotcharles@chromium.org>
Reviewed-by: Sigurður Ásgeirsson <siggi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845713}

components/performance_manager/v8_memory/web_memory_aggregator.h

@@ -41,10 +41,22 @@ class WebMemoryAggregator {
   WebMemoryAggregator(const WebMemoryAggregator& other) = delete;
   WebMemoryAggregator& operator=(const WebMemoryAggregator& other) = delete;
 
+  // Returns the origin of |requesting_node|.
+  const url::Origin& requesting_origin() const { return requesting_origin_; }
+
+  // Performs the aggregation.
+  mojom::WebMemoryMeasurementPtr AggregateMeasureMemoryResult();
+
+ private:
+  friend class WebMemoryAggregatorTest;
+
+  class AggregationPointVisitor;
+
   // The various ways a node can be treated during the aggregation.
   enum class NodeAggregationType {
     // Node is same-origin to |requesting_node|; will be a new aggregation
-    // point with scope "Window".
+    // point with a scope depending on the node type (eg. "Window" or
+    // "DedicatedWorker").
     kSameOriginAggregationPoint,
     // Node is cross-origin with |requesting_node| but its parent is not; will
     // be a new aggregation point with scope
@@ -58,9 +70,6 @@ class WebMemoryAggregator {
     kInvisible,
   };
 
-  // Returns the origin of |requesting_node|.
-  const url::Origin& requesting_origin() const { return requesting_origin_; }
-
   // Returns the way that |frame_node| should be treated during the
   // aggregation.  |aggregation_start_node_| must be reachable from
   // |frame_node| by following parent/child or opener links. This will always
@@ -72,83 +81,69 @@ class WebMemoryAggregator {
       const WorkerNode* worker_node,
       NodeAggregationType parent_aggregation_type);
 
-  // Performs the aggregation.
-  mojom::WebMemoryMeasurementPtr AggregateMeasureMemoryResult();
-
- private:
   // FrameNodeVisitor that recursively adds |frame_node| and its children to
-  // the aggregation. |enclosing_aggregation_point| is the aggregation point
-  // that |frame_node|'s parent or opener is in. Always returns true to
-  // continue traversal.
-  bool VisitFrame(mojom::WebMemoryBreakdownEntry* enclosing_aggregation_point,
+  // the aggregation using |ap_visitor|. Always returns true to continue
+  // traversal.
+  bool VisitFrame(AggregationPointVisitor* ap_visitor,
                   const FrameNode* frame_node);
 
   // WorkerNodeVisitor that recursively adds |worker_node| and its children to
-  // the aggregation. |enclosing_aggregation_point| is the aggregation point
-  // that |worker_node|'s parent is in. Similarly |enclosing_aggregation_type|
-  // is the aggregation type of the parent.
-  // Always returns true to continue traversal.
-  bool VisitWorker(mojom::WebMemoryBreakdownEntry* enclosing_aggregation_point,
+  // the aggregation using |ap_visitor|. |enclosing_aggregation_type| is the
+  // type of the aggregation point that |worker_node|'s parent is in. Always
+  // returns true to continue traversal.
+  bool VisitWorker(AggregationPointVisitor* ap_visitor,
                    NodeAggregationType enclosing_aggregation_type,
                    const WorkerNode* worker_node);
 
   // PageNodeVisitor that recursively adds |page_node|'s main frames and their
-  // children to the aggregation. |enclosing_aggregation_point| is the
-  // aggregation point that |page_node|'s opener is in. Always returns true to
+  // children to the aggregation using |ap_visitor|. Always returns true to
   // continue traversal.
-  bool VisitOpenedPage(
-      mojom::WebMemoryBreakdownEntry* enclosing_aggregation_point,
+  bool VisitOpenedPage(AggregationPointVisitor* ap_visitor,
                        const PageNode* page_node);
 
-  // The origin of |requesting_node|. Cached so it doesn't have to be
-  // recalculated in each call to VisitFrame.
-  const url::Origin requesting_origin_;
+  // Static private methods are implementation details, but can be accessed from
+  // friend classes for testing.
 
-  // The node that the graph traversal should start from, found from
-  // |requesting_node| using FindAggregationStartNode.
-  const FrameNode* aggregation_start_node_;
-
-  // Stores the result of the aggregation. This is populated by
-  // AggregateMeasureMemoryResult.
-  mojom::WebMemoryMeasurementPtr aggregation_result_
-      GUARDED_BY_CONTEXT(sequence_checker_);
-
-  SEQUENCE_CHECKER(sequence_checker_);
-};
-
-namespace internal {
-
-// These functions are used in the implementation and exposed in the header for
-// testing.
-
-// Returns |frame_node|'s parent or opener if the parent or opener is
-// same-origin with |origin|, nullptr otherwise.
-const FrameNode* GetSameOriginParentOrOpener(const FrameNode* frame_node,
+  // Returns |frame_node|'s parent or opener if the parent or opener is
+  // same-origin with |origin|, nullptr otherwise.
+  static const FrameNode* GetSameOriginParentOrOpener(
+      const FrameNode* frame_node,
       const url::Origin& origin);
 
-// Walks back the chain of parents and openers from |requesting_node| to find
-// the farthest ancestor that should be visible to it (all intermediate nodes
-// in the chain are same-origin).
-const FrameNode* FindAggregationStartNode(const FrameNode* requesting_node);
+  // Walks back the chain of parents and openers from |requesting_node| to find
+  // the farthest ancestor that should be visible to it (all intermediate nodes
+  // in the chain are same-origin).
+  static const FrameNode* FindAggregationStartNode(
+      const FrameNode* requesting_node);
 
-// Creates a new breakdown entry with the given |scope| and |url|, and adds it
-// to the list in |measurement|. Returns a pointer to the newly created entry.
-mojom::WebMemoryBreakdownEntry* CreateBreakdownEntry(
+  // Creates a new breakdown entry with the given |scope| and |url|, and adds it
+  // to the list in |measurement|. Returns a pointer to the newly created entry.
+  static mojom::WebMemoryBreakdownEntry* CreateBreakdownEntry(
       mojom::WebMemoryAttribution::Scope scope,
       base::Optional<std::string> url,
       mojom::WebMemoryMeasurement* measurement);
 
-// Sets the id and src attributes of |breakdown| using those stored in the
-// V8ContextTracker for the given |frame_node|.
-void SetBreakdownAttributionFromFrame(
+  // Sets the id and src attributes of |breakdown| using those stored in the
+  // V8ContextTracker for the given |frame_node|.
+  static void SetBreakdownAttributionFromFrame(
       const FrameNode* frame_node,
       mojom::WebMemoryBreakdownEntry* breakdown);
 
-// Copies the id and src attributes from |from| to |to|.
-void CopyBreakdownAttribution(const mojom::WebMemoryBreakdownEntry* from,
+  // Copies the id and src attributes from |from| to |to|.
+  static void CopyBreakdownAttribution(
+      const mojom::WebMemoryBreakdownEntry* from,
       mojom::WebMemoryBreakdownEntry* to);
 
-}  // namespace internal
+  // The origin of |requesting_node|. Cached so it doesn't have to be
+  // recalculated in each call to VisitFrame.
+  const url::Origin requesting_origin_;
+
+  // The node that the graph traversal should start from, found from
+  // |requesting_node| using FindAggregationStartNode.
+  const FrameNode* aggregation_start_node_;
+
+  SEQUENCE_CHECKER(sequence_checker_);
+};
 
 }  // namespace v8_memory