Use WebMemoryAggregator in implementation of performance.measureMemory

Bug: chromium:1085129
Change-Id: Ia0d1968c97542a7ed0455b61aaf2558e9c81272c
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2613024Reviewed-by: Joe Mason <joenotcharles@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#841087}

components/performance_manager/embedder/graph_features_helper.h

@@ -117,6 +117,7 @@ class GraphFeaturesHelper {
   // browser to work.
   constexpr GraphFeaturesHelper& EnableMinimal() {
     EnableExecutionContextRegistry();
+    EnableV8ContextTracker();
     return *this;
   }
 

components/performance_manager/v8_memory/v8_memory_test_helpers.cc

@@ -234,7 +234,7 @@ int WebMemoryTestHarness::GetNextUniqueId() {
 }
 
 FrameNodeImpl* WebMemoryTestHarness::AddFrameNodeImpl(
-    std::string url,
+    base::Optional<std::string> url,
     int browsing_instance_id,
     Bytes memory_usage,
     FrameNodeImpl* parent,
@@ -255,7 +255,9 @@ FrameNodeImpl* WebMemoryTestHarness::AddFrameNodeImpl(
   auto frame = CreateNode<FrameNodeImpl>(process_.get(), page, parent,
                                          frame_tree_node_id, frame_routing_id,
                                          frame_token, browsing_instance_id);
-  frame->OnNavigationCommitted(GURL(url), /*same document*/ true);
+  if (url) {
+    frame->OnNavigationCommitted(GURL(*url), /*same document*/ true);
+  }
   V8DetailedMemoryExecutionContextData::CreateForTesting(frame.get())
       ->set_v8_bytes_used(memory_usage.bytes);
   frames_.push_back(std::move(frame));

components/performance_manager/v8_memory/v8_memory_test_helpers.h

@@ -247,7 +247,7 @@ class WebMemoryTestHarness : public GraphTestHarness {
   }
 
   // Creates a frame node as if from window.open and adds it to the graph.
-  FrameNodeImpl* AddFrameNodeFromOpener(std::string url,
+  FrameNodeImpl* AddFrameNodeFromOpener(base::Optional<std::string> url,
                                         Bytes bytes,
                                         FrameNodeImpl* opener) {
     return AddFrameNodeImpl(url, kDefaultBrowsingInstanceId, bytes,
@@ -283,7 +283,7 @@ class WebMemoryTestHarness : public GraphTestHarness {
 
   // Creates and adds a new frame node to the graph.
   FrameNodeImpl* AddFrameNodeImpl(
-      std::string url,
+      base::Optional<std::string> url,
       int browsing_instance_id,
       Bytes bytes,
       FrameNodeImpl* parent = nullptr,

components/performance_manager/v8_memory/web_memory_aggregator.cc

@@ -106,9 +106,11 @@ WebMemoryAggregator::FindNodeAggregationType(const FrameNode* frame_node) {
     return NodeAggregationType::kInvisible;
   }
 
+  auto frame_origin = GetOrigin(frame_node);
+
   // If |frame_node| is same-origin to |start_node|, it's an aggregation point.
   // (This trivially includes the |start_node| itself.)
-  if (requesting_origin_.IsSameOriginWith(GetOrigin(frame_node)))
+  if (requesting_origin_.IsSameOriginWith(frame_origin))
     return NodeAggregationType::kSameOriginAggregationPoint;
   DCHECK_NE(frame_node, aggregation_start_node_);
 
@@ -116,8 +118,16 @@ WebMemoryAggregator::FindNodeAggregationType(const FrameNode* frame_node) {
   // a same-origin node, its existence is visible to |start_node| so it's an
   // aggregation point. But its current url will be hidden from |start_node|.
   const FrameNode* parent_node = frame_node->GetParentFrameNode();
-  // |frame_node| is a child of |start_node| so must have a parent.
-  DCHECK(parent_node);
+
+  if (!parent_node) {
+    // A cross-origin window opened via window.open gets its own browsing
+    // context group due to COOP. However, while the window is being loaded it
+    // belongs to the old browsing context group. In that case the origin is
+    // opaque.
+    DCHECK(frame_origin.opaque());
+    return NodeAggregationType::kInvisible;
+  }
+
   if (requesting_origin_.IsSameOriginWith(GetOrigin(parent_node)))
     return NodeAggregationType::kCrossOriginAggregationPoint;
 

components/performance_manager/v8_memory/web_memory_aggregator_unittest.cc

@@ -506,6 +506,28 @@ TEST_F(WebMemoryAggregatorTest, AggregateWindowOpener) {
   }
 }
 
+TEST_F(WebMemoryAggregatorTest, AggregateProvisionalWindowOpener) {
+  FrameNodeImpl* main_frame = AddFrameNode("https://example.com/", Bytes{10});
+
+  // This creates an openee window with pending navigation which should be
+  // skipped because it may get its own browsing context group once the
+  // navigation completes.
+  FrameNodeImpl* pending_frame =
+      AddFrameNodeFromOpener(base::nullopt, Bytes{4}, main_frame);
+
+  WebMemoryAggregator aggregator(main_frame);
+
+  EXPECT_EQ(aggregator.FindNodeAggregationType(pending_frame),
+            NodeAggregationType::kInvisible);
+
+  auto expected_result = CreateExpectedMemoryMeasurement({
+      ExpectedMemoryBreakdown(10, AttributionScope::kWindow,
+                              "https://example.com/"),
+  });
+  auto result = aggregator.AggregateMeasureMemoryResult();
+  EXPECT_EQ(MeasurementToJSON(result), MeasurementToJSON(expected_result));
+}
+
 }  // namespace
 
 }  // namespace v8_memory

components/performance_manager/v8_memory/web_memory_impl.cc

@@ -18,6 +18,7 @@
 #include "components/performance_manager/public/performance_manager.h"
 #include "components/performance_manager/public/render_frame_host_proxy.h"
 #include "components/performance_manager/public/v8_memory/web_memory.h"
+#include "components/performance_manager/v8_memory/web_memory_aggregator.h"
 #include "content/public/browser/browser_task_traits.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/render_frame_host.h"
@@ -36,46 +37,16 @@ mojom::WebMemoryMeasurementPtr BuildMemoryUsageResult(
     const blink::LocalFrameToken& frame_token,
     const ProcessNode* process_node) {
   const auto& frame_nodes = process_node->GetFrameNodes();
+  const auto it = std::find_if(frame_nodes.begin(), frame_nodes.end(),
+                               [frame_token](const FrameNode* node) {
+                                 return node->GetFrameToken() == frame_token;
+                               });
 
-  // Find the frame that made the request.
-  const FrameNode* requesting_frame = nullptr;
-  for (auto* frame_node : frame_nodes) {
-    if (frame_node->GetFrameToken() == frame_token) {
-      requesting_frame = frame_node;
-      break;
-    }
-  }
-
-  if (!requesting_frame) {
+  if (it == frame_nodes.end()) {
     // The frame no longer exists.
     return mojom::WebMemoryMeasurement::New();
   }
-
-  auto result = mojom::WebMemoryMeasurement::New();
-
-  for (const FrameNode* frame_node : frame_nodes) {
-    if (frame_node->GetBrowsingInstanceId() !=
-        requesting_frame->GetBrowsingInstanceId()) {
-      continue;
-    }
-    if (frame_node->GetURL().GetOrigin() !=
-        requesting_frame->GetURL().GetOrigin()) {
-      continue;
-    }
-    auto* data = v8_memory::V8DetailedMemoryExecutionContextData::ForFrameNode(
-        frame_node);
-    if (!data) {
-      continue;
-    }
-    auto attribution = mojom::WebMemoryAttribution::New();
-    attribution->url = frame_node->GetURL().spec();
-    attribution->scope = mojom::WebMemoryAttribution::Scope::kWindow;
-    auto entry = mojom::WebMemoryBreakdownEntry::New();
-    entry->bytes = data->v8_bytes_used();
-    entry->attribution.push_back(std::move(attribution));
-    result->breakdown.push_back(std::move(entry));
-  }
-  return result;
+  return WebMemoryAggregator(*it).AggregateMeasureMemoryResult();
 }
 
 v8_memory::V8DetailedMemoryRequest::MeasurementMode
@@ -160,8 +131,6 @@ void WebMemoryMeasurer::MeasureMemory(mojom::WebMemoryMeasurement::Mode mode,
 void WebMemoryMeasurer::MeasurementComplete(
     const ProcessNode* process_node,
     const V8DetailedMemoryProcessData*) {
-  // TODO(crbug.com/1085129): Use WebMemoryAggregator here instead of
-  // BuildMemoryUsageResult.
   std::move(callback_).Run(BuildMemoryUsageResult(frame_token_, process_node));
 }
 

components/performance_manager/v8_memory/web_memory_impl.h

@@ -25,8 +25,6 @@ namespace v8_memory {
 // A helper class for implementing WebMeasureMemory(). This manages a request
 // object that sends a V8 detailed memory request to the renderer, and formats
 // the result into a mojom::WebMemoryMeasurement.
-// TODO(crbug.com/1085129): Extend this to measure all renderers that are
-// reachable from the requesting node.
 class WebMemoryMeasurer {
  public:
   using MeasurementCallback =

components/performance_manager/v8_memory/web_memory_impl_unittest.cc

@@ -70,9 +70,12 @@ void WebMemoryImplTest::MeasureAndVerify(
         base::flat_map<std::string, Bytes> actual;
         for (const auto& entry : result->breakdown) {
           EXPECT_EQ(1u, entry->attribution.size());
-          EXPECT_EQ(mojom::WebMemoryAttribution::Scope::kWindow,
-                    entry->attribution[0]->scope);
-          actual[*entry->attribution[0]->url] = Bytes{entry->bytes};
+          if (mojom::WebMemoryAttribution::Scope::kWindow ==
+              entry->attribution[0]->scope) {
+            actual[*entry->attribution[0]->url] = Bytes{entry->bytes};
+          } else {
+            actual[*entry->attribution[0]->src] = Bytes{entry->bytes};
+          }
         }
         EXPECT_EQ(expected, actual);
         measurement_done = true;
@@ -93,15 +96,17 @@ TEST_F(WebMemoryImplTest, MeasurerIncludesSameOriginRelatedFrames) {
                          });
 }
 
-// TODO(b/1085129): Currently WebMemoryMeasurer only includes the results for a
-// single process. Once it invokes WebMemoryAggregator, update this test to
-// expect cross-origin frames to be included in the aggregation.
-TEST_F(WebMemoryImplTest, MeasurerSkipsCrossOriginFrames) {
+TEST_F(WebMemoryImplTest, MeasurerIncludesCrossOriginFrames) {
   auto* main = AddFrameNode("http://foo.com", Bytes{10u});
 
-  AddFrameNode("http://bar.com/iframe", Bytes{20}, main);
+  AddFrameNode("http://bar.com/iframe", Bytes{20}, main, "bar_id",
+               "http://bar.com/iframe_src");
 
-  MeasureAndVerify(main, {{"http://foo.com/", Bytes{10u}}});
+  MeasureAndVerify(main, {{"http://foo.com/", Bytes{10u}},
+                          {
+                              "http://bar.com/iframe_src",
+                              Bytes{20},
+                          }});
 }
 
 TEST_F(WebMemoryImplTest, MeasurerSkipsCrossBrowserContextGroupFrames) {

third_party/blink/renderer/core/timing/measure_memory/measure_memory_controller.cc

@@ -12,6 +12,7 @@
 #include "third_party/blink/renderer/bindings/core/v8/script_promise.h"
 #include "third_party/blink/renderer/bindings/core/v8/to_v8_for_core.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_memory_attribution.h"
+#include "third_party/blink/renderer/bindings/core/v8/v8_memory_attribution_container.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_memory_breakdown_entry.h"
 #include "third_party/blink/renderer/bindings/core/v8/v8_memory_measurement.h"
 #include "third_party/blink/renderer/core/execution_context/execution_context.h"
@@ -138,12 +139,27 @@ WTF::String ConvertScope(WebMemoryAttribution::Scope scope) {
   }
 }
 
+MemoryAttributionContainer* ConvertContainer(
+    const WebMemoryAttributionPtr& attribution) {
+  if (!attribution->src && !attribution->id) {
+    return nullptr;
+  }
+  auto* result = MemoryAttributionContainer::Create();
+  result->setSrc(attribution->src);
+  result->setId(attribution->id);
+  return result;
+}
+
 MemoryAttribution* ConvertAttribution(
     const WebMemoryAttributionPtr& attribution) {
   auto* result = MemoryAttribution::Create();
-  result->setUrl(attribution->url);
+  if (attribution->url) {
+    result->setUrl(attribution->url);
+  } else {
+    result->setUrl("cross-origin-url");
+  }
   result->setScope(ConvertScope(attribution->scope));
-  result->setContainer(nullptr);
+  result->setContainer(ConvertContainer(attribution));
   return result;
 }
 

third_party/blink/renderer/core/timing/measure_memory/memory_attribution_container.idl

@@ -6,6 +6,6 @@
 
 // The attributes of the container iframe element.
 dictionary MemoryAttributionContainer {
-  required DOMString id;
-  required USVString src;
+  DOMString id;
+  USVString src;
 };
\ No newline at end of file