PM: Use the FrameToken in the V8 per frame memory reporter.

Bug: 1096543
Change-Id: Ife6342aaaf70dcecd1da2712ac50b8a934163a09
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2254079Reviewed-by: Will Harris <wfh@chromium.org>
Reviewed-by: Joe Mason <joenotcharles@chromium.org>
Commit-Queue: Sigurður Ásgeirsson <siggi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#780444}

components/performance_manager/decorators/v8_per_frame_memory_decorator.cc

@@ -135,22 +135,22 @@ void NodeAttachedProcessData::OnPerFrameV8MemoryUsageData(
   uint64_t unassociated_v8_bytes_used = result->unassociated_bytes_used;
 
   // Create a mapping from token to per-frame usage for the merge below.
-  // Note that in the presence of multiple records with identical
-  // dev_tools_token fields, this will throw away all but one of those
-  // records.
-  // TODO(https://crbug.com/1096543): Change to using the frame-unique token and
-  //     validate that multiple frames/records don't carry the same token.
-  std::vector<
-      std::pair<base::UnguessableToken, mojom::PerFrameV8MemoryUsageDataPtr>>
-      tmp;
-  for (auto& entry : result->associated_memory)
-    tmp.emplace_back(std::make_pair(entry->dev_tools_token, std::move(entry)));
-  base::flat_map<base::UnguessableToken, mojom::PerFrameV8MemoryUsageDataPtr>
+  std::vector<std::pair<FrameToken, mojom::PerFrameV8MemoryUsageDataPtr>> tmp;
+  for (auto& entry : result->associated_memory) {
+    tmp.emplace_back(
+        std::make_pair(FrameToken(entry->frame_token), std::move(entry)));
+  }
+  DCHECK_EQ(tmp.size(), result->associated_memory.size());
+
+  base::flat_map<FrameToken, mojom::PerFrameV8MemoryUsageDataPtr>
       associated_memory(std::move(tmp));
+  // Validate that the frame tokens were all unique. If there are duplicates,
+  // the map will arbirarily drop all but one record per unique token.
+  DCHECK_EQ(associated_memory.size(), result->associated_memory.size());
 
   base::flat_set<const FrameNode*> frame_nodes = process_node_->GetFrameNodes();
   for (const FrameNode* frame_node : frame_nodes) {
-    auto it = associated_memory.find(frame_node->GetDevToolsToken());
+    auto it = associated_memory.find(frame_node->GetFrameToken());
     if (it == associated_memory.end()) {
       // No data for this node, clear any data associated with it.
       NodeAttachedFrameData::Destroy(frame_node);

components/performance_manager/decorators/v8_per_frame_memory_decorator_unittest.cc

@@ -68,13 +68,13 @@ class LenientMockV8PerFrameMemoryReporter
 using MockV8PerFrameMemoryReporter =
     testing::StrictMock<LenientMockV8PerFrameMemoryReporter>;
 
-void AddPerFrameIsolateMemoryUsage(base::UnguessableToken frame_id,
+void AddPerFrameIsolateMemoryUsage(FrameToken frame_token,
                                    int64_t world_id,
                                    uint64_t bytes_used,
                                    mojom::PerProcessV8MemoryUsageData* data) {
   mojom::PerFrameV8MemoryUsageData* per_frame_data = nullptr;
   for (auto& datum : data->associated_memory) {
-    if (datum->dev_tools_token == frame_id) {
+    if (datum->frame_token == frame_token.value()) {
       per_frame_data = datum.get();
       break;
     }
@@ -83,7 +83,7 @@ void AddPerFrameIsolateMemoryUsage(base::UnguessableToken frame_id,
   if (!per_frame_data) {
     mojom::PerFrameV8MemoryUsageDataPtr datum =
         mojom::PerFrameV8MemoryUsageData::New();
-    datum->dev_tools_token = frame_id;
+    datum->frame_token = frame_token.value();
     per_frame_data = datum.get();
     data->associated_memory.push_back(std::move(datum));
   }
@@ -402,8 +402,8 @@ TEST_F(V8PerFrameMemoryDecoratorTest, PerFrameDataIsDistributed) {
   {
     auto data = mojom::PerProcessV8MemoryUsageData::New();
     // Add data for an unknown frame.
-    AddPerFrameIsolateMemoryUsage(base::UnguessableToken::Create(), 0, 1024u,
-                                  data.get());
+    AddPerFrameIsolateMemoryUsage(FrameToken(base::UnguessableToken::Create()),
+                                  0, 1024u, data.get());
 
     ExpectBindAndRespondToQuery(&reporter, std::move(data));
   }
@@ -424,13 +424,15 @@ TEST_F(V8PerFrameMemoryDecoratorTest, PerFrameDataIsDistributed) {
   // Create a couple of frames with specified IDs.
   auto page = CreateNode<PageNodeImpl>();
 
-  base::UnguessableToken frame1_id = base::UnguessableToken::Create();
-  auto frame1 = CreateNode<FrameNodeImpl>(process.get(), page.get(), nullptr, 1,
-                                          2, frame1_id);
+  FrameToken frame1_id = FrameToken(base::UnguessableToken::Create());
+  auto frame1 =
+      CreateNode<FrameNodeImpl>(process.get(), page.get(), nullptr, 1, 2,
+                                base::UnguessableToken::Create(), frame1_id);
 
-  base::UnguessableToken frame2_id = base::UnguessableToken::Create();
-  auto frame2 = CreateNode<FrameNodeImpl>(process.get(), page.get(), nullptr, 3,
-                                          4, frame2_id);
+  FrameToken frame2_id = FrameToken(base::UnguessableToken::Create());
+  auto frame2 =
+      CreateNode<FrameNodeImpl>(process.get(), page.get(), nullptr, 3, 4,
+                                base::UnguessableToken::Create(), frame2_id);
   {
     auto data = mojom::PerProcessV8MemoryUsageData::New();
     AddPerFrameIsolateMemoryUsage(frame1_id, 0, 1001u, data.get());
@@ -451,8 +453,8 @@ TEST_F(V8PerFrameMemoryDecoratorTest, PerFrameDataIsDistributed) {
   {
     auto data = mojom::PerProcessV8MemoryUsageData::New();
     AddPerFrameIsolateMemoryUsage(frame1_id, 0, 1003u, data.get());
-    AddPerFrameIsolateMemoryUsage(base::UnguessableToken::Create(), 0, 2233u,
-                                  data.get());
+    AddPerFrameIsolateMemoryUsage(FrameToken(base::UnguessableToken::Create()),
+                                  0, 2233u, data.get());
     ExpectQueryAndReply(&reporter, std::move(data));
   }
   task_env().FastForwardBy(kMinTimeBetweenRequests);

content/public/common/performance_manager/v8_per_frame_memory.mojom

@@ -27,8 +27,8 @@ struct V8IsolatedWorldMemoryUsage {
 
 // Returns the number of bytes used by the v8 heap per frame.
 struct PerFrameV8MemoryUsageData {
-  // The devtools token associated with the frame.
-  mojo_base.mojom.UnguessableToken dev_tools_token;
+  // The frame-unique token.
+  mojo_base.mojom.UnguessableToken frame_token;
 
   // The resources used by this frame, mapped on the isolated world ID.
   // World ID 0 is the main world.

content/renderer/performance_manager/v8_per_frame_memory_reporter_impl.cc

@@ -65,9 +65,13 @@ class FrameAssociatedMeasurementDelegate : public v8::MeasureMemoryDelegate {
         mojom::PerFrameV8MemoryUsageData* per_frame_resources =
             frames[frame].get();
         if (!per_frame_resources) {
+#if DCHECK_IS_ON()
+          // Check that the frame token didn't already occur.
+          for (const auto& entry : frames)
+            DCHECK_NE(entry.first->GetFrameToken(), frame->GetFrameToken());
+#endif
           auto new_resources = mojom::PerFrameV8MemoryUsageData::New();
-          new_resources->dev_tools_token =
-              frame->Client()->GetDevToolsFrameToken();
+          new_resources->frame_token = frame->GetFrameToken();
           per_frame_resources = new_resources.get();
           frames[frame] = std::move(new_resources);
         }