Improve TDI render process naming in the task manager.

chrome/browser/task_manager/providers/web_contents/web_contents_task_provider.cc

Index: chrome/browser/task_manager/providers/web_contents/web_contents_task_provider.cc
diff --git a/chrome/browser/task_manager/providers/web_contents/web_contents_task_provider.cc b/chrome/browser/task_manager/providers/web_contents/web_contents_task_provider.cc
index e708be3d3527756e9ec395cf039f3c7885a73b73..00d2537150ff368d703fb5f60c419424982c8bad 100644
--- a/chrome/browser/task_manager/providers/web_contents/web_contents_task_provider.cc
+++ b/chrome/browser/task_manager/providers/web_contents/web_contents_task_provider.cc
@@ -74,13 +74,11 @@ class WebContentsEntry : public content::WebContentsObserver {
   // The RenderFrameHosts associated with this entry's WebContents that we're
   // tracking mapped by their SiteInstances.
   using FramesList = std::vector<RenderFrameHost*>;
-  using SiteInstanceToFramesMap = std::map<SiteInstance*, FramesList>;
-  SiteInstanceToFramesMap frames_by_site_instance_;
+  std::map<SiteInstance*, FramesList> frames_by_site_instance_;
 
   // The RendererTasks that we create for the task manager, mapped by their
-  // RenderFrameHosts.
-  using FramesToTasksMap = std::map<RenderFrameHost*, RendererTask*>;
-  FramesToTasksMap tasks_by_frames_;
+  // RenderFrameHosts. This owns the RenderTasks.
+  std::map<RenderFrameHost*, RendererTask*> tasks_by_frames_;
 
   // States whether we did record a main frame for this entry.
   SiteInstance* main_frame_site_instance_;
@@ -173,31 +171,33 @@ void WebContentsEntry::OnRendererUnresponsive(
 
 void WebContentsEntry::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
-  RendererTask* task = GetTaskForFrame(web_contents()->GetMainFrame());
-  if (!task)
+  RendererTask* main_frame_task =
+      GetTaskForFrame(web_contents()->GetMainFrame());
+  if (!main_frame_task)
     return;
 
-  // Listening to WebContentsObserver::TitleWasSet() only is not enough in
-  // some cases when the the webpage doesn't have a title. That's why we update
-  // the title here as well.
-  task->UpdateTitle();
-
-  // Call RendererTask::UpdateFavicon() to set the current favicon to the
-  // default favicon. If the page has a non-default favicon,
-  // RendererTask::OnFaviconUpdated() will update the current favicon once
-  // FaviconDriver figures out the correct favicon for the page.
-  task->UpdateFavicon();
-  task->UpdateRapporSampleName();
+  main_frame_task->UpdateRapporSampleName();

[ncarter (slow), 2016/10/26 17:45:27]

Maybe do this only if navigation_handle->IsInMainFrame()?

[Avi (use Gerrit), 2016/10/26 20:37:28]

Maybe? I'm not sure if that would change the rappor metric, adding that
qualification. Are you suggesting that's something I should do?

[ncarter (slow), 2016/10/26 20:55:48]

Ah, ignore this comment; I was mistaken about what was going on here.

+
+  for (const auto& frame_pair : tasks_by_frames_) {

[ncarter (slow), 2016/10/26 17:45:27]

In the a(b, b) case, or the non-oopif case, this will visit some tasks several
times, since a Task can correspond to several frames and if so, the Task will
appear multiple times in |tasks_by_frames_|.

To visit each task exactly once, use the loop pattern seen in
WebContentsEntry::ClearAllTasks:

for (const auto& pair : frames_by_site_instance_) {
  const FramesList& frames_list = pair.second;
  DCHECK(!frames_list.empty());
  RendererTask* task = tasks_by_frames_[frames_list[0]];
}

[Avi (use Gerrit), 2016/10/26 20:37:28]

Done.

+    // Listening to WebContentsObserver::TitleWasSet() only is not enough in
+    // some cases when the the web page doesn't have a title. That's why we
+    // update the title here as well.
+    frame_pair.second->UpdateTitle();
+
+    // Call RendererTask::UpdateFavicon() to set the current favicon to the
+    // default favicon. If the page has a non-default favicon,
+    // RendererTask::OnFaviconUpdated() will update the current favicon once
+    // FaviconDriver figures out the correct favicon for the page.
+    frame_pair.second->UpdateFavicon();
+  }
 }
 
 void WebContentsEntry::TitleWasSet(content::NavigationEntry* entry,
                                    bool explicit_set) {
-  RendererTask* task = GetTaskForFrame(web_contents()->GetMainFrame());
-  if (!task)
-    return;
-
-  task->UpdateTitle();
-  task->UpdateFavicon();
+  for (const auto& frame_pair : tasks_by_frames_) {

[ncarter (slow), 2016/10/26 17:45:27]

Same here. Might be worth extracting a function to do this loop.

[Avi (use Gerrit), 2016/10/26 20:37:28]

Done.

+    frame_pair.second->UpdateTitle();
+    frame_pair.second->UpdateFavicon();
+  }
 }
 
 void WebContentsEntry::CreateTaskForFrame(RenderFrameHost* render_frame_host) {