Remove raw pointers from WebContentsTags and Tasks

task_manager::WebContentsTag::CreateTask returns ownership in a
raw pointer. Fix that. WebContentsTaskProvider::WebContentsEntry
then has to juggle those raw pointers. Fix that.

Bug: none
Change-Id: Ie19f3f67365b8f1ff22079949a6148f058d7ffd4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2361204
Commit-Queue: Avi Drissman <avi@chromium.org>
Auto-Submit: Avi Drissman <avi@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#799819}

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

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/task_manager/providers/web_contents/background_contents_tag.h"
 
+#include <memory>
+
 #include "base/strings/utf_string_conversions.h"
 #include "chrome/browser/background/background_contents.h"
 #include "chrome/browser/background/background_contents_service.h"
@@ -17,7 +19,7 @@
 
 namespace task_manager {
 
-BackgroundContentsTask* BackgroundContentsTag::CreateTask(
+std::unique_ptr<RendererTask> BackgroundContentsTag::CreateTask(
     WebContentsTaskProvider*) const {
   // Try to lookup the application name from the parent extension (if any).
   Profile* profile = Profile::FromBrowserContext(
@@ -34,7 +36,8 @@ BackgroundContentsTask* BackgroundContentsTag::CreateTask(
   if (extension)
     application_name = base::UTF8ToUTF16(extension->name());
 
-  return new BackgroundContentsTask(application_name, background_contents_);
+  return std::make_unique<BackgroundContentsTask>(application_name,
+                                                  background_contents_);
 }
 
 BackgroundContentsTag::BackgroundContentsTag(

chrome/browser/task_manager/providers/web_contents/background_contents_tag.h

@@ -20,7 +20,8 @@ class BackgroundContentsTag : public WebContentsTag {
   ~BackgroundContentsTag() override;
 
   // task_manager::WebContentsTag:
-  BackgroundContentsTask* CreateTask(WebContentsTaskProvider*) const override;
+  std::unique_ptr<RendererTask> CreateTask(
+      WebContentsTaskProvider*) const override;
 
  private:
   friend class WebContentsTags;

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

@@ -4,10 +4,13 @@
 
 #include "chrome/browser/task_manager/providers/web_contents/devtools_tag.h"
 
+#include <memory>
+
 namespace task_manager {
 
-DevToolsTask* DevToolsTag::CreateTask(WebContentsTaskProvider*) const {
-  return new DevToolsTask(web_contents());
+std::unique_ptr<RendererTask> DevToolsTag::CreateTask(
+    WebContentsTaskProvider*) const {
+  return std::make_unique<DevToolsTask>(web_contents());
 }
 
 DevToolsTag::DevToolsTag(content::WebContents* web_contents)

chrome/browser/task_manager/providers/web_contents/devtools_tag.h

@@ -17,7 +17,8 @@ class DevToolsTag : public WebContentsTag {
   ~DevToolsTag() override;
 
   // task_manager::WebContentsTag:
-  DevToolsTask* CreateTask(WebContentsTaskProvider*) const override;
+  std::unique_ptr<RendererTask> CreateTask(
+      WebContentsTaskProvider*) const override;
 
  private:
   friend class WebContentsTags;

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

@@ -4,6 +4,8 @@
 
 #include "chrome/browser/task_manager/providers/web_contents/extension_tag.h"
 
+#include <memory>
+
 #include "content/public/browser/web_contents.h"
 #include "extensions/buildflags/buildflags.h"
 
@@ -13,7 +15,8 @@
 
 namespace task_manager {
 
-ExtensionTask* ExtensionTag::CreateTask(WebContentsTaskProvider*) const {
+std::unique_ptr<RendererTask> ExtensionTag::CreateTask(
+    WebContentsTaskProvider*) const {
   // Upon being asked to create a task, it means that the site instance is ready
   // and connected, and the render frames have been initialized.
   // It's OK if the following returns nullptr, ExtensionTask will then get the
@@ -22,7 +25,7 @@ ExtensionTask* ExtensionTag::CreateTask(WebContentsTaskProvider*) const {
       extensions::ProcessManager::Get(web_contents()->GetBrowserContext())->
           GetExtensionForWebContents(web_contents());
 
-  return new ExtensionTask(web_contents(), extension, view_type_);
+  return std::make_unique<ExtensionTask>(web_contents(), extension, view_type_);
 }
 
 ExtensionTag::ExtensionTag(content::WebContents* web_contents,

chrome/browser/task_manager/providers/web_contents/extension_tag.h

@@ -17,7 +17,8 @@ class ExtensionTag : public WebContentsTag {
   ~ExtensionTag() override;
 
   // task_manager::WebContentsTag:
-  ExtensionTask* CreateTask(WebContentsTaskProvider*) const override;
+  std::unique_ptr<RendererTask> CreateTask(
+      WebContentsTaskProvider*) const override;
 
  private:
   friend class WebContentsTags;

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

@@ -4,10 +4,13 @@
 
 #include "chrome/browser/task_manager/providers/web_contents/guest_tag.h"
 
+#include <memory>
+
 namespace task_manager {
 
-GuestTask* GuestTag::CreateTask(WebContentsTaskProvider*) const {
-  return new GuestTask(web_contents());
+std::unique_ptr<RendererTask> GuestTag::CreateTask(
+    WebContentsTaskProvider*) const {
+  return std::make_unique<GuestTask>(web_contents());
 }
 
 GuestTag::GuestTag(content::WebContents* web_contents)

chrome/browser/task_manager/providers/web_contents/guest_tag.h

@@ -18,7 +18,8 @@ class GuestTag : public WebContentsTag {
   ~GuestTag() override;
 
   // task_manager::WebContentsTag:
-  GuestTask* CreateTask(WebContentsTaskProvider*) const override;
+  std::unique_ptr<RendererTask> CreateTask(
+      WebContentsTaskProvider*) const override;
 
  private:
   friend class WebContentsTags;

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

@@ -4,11 +4,13 @@
 
 #include "chrome/browser/task_manager/providers/web_contents/portal_tag.h"
 
+#include <memory>
+
 namespace task_manager {
 
-PortalTask* PortalTag::CreateTask(
+std::unique_ptr<RendererTask> PortalTag::CreateTask(
     WebContentsTaskProvider* task_provider) const {
-  return new PortalTask(web_contents(), task_provider);
+  return std::make_unique<PortalTask>(web_contents(), task_provider);
 }
 
 PortalTag::PortalTag(content::WebContents* web_contents)

chrome/browser/task_manager/providers/web_contents/portal_tag.h

@@ -19,7 +19,8 @@ class PortalTag : public WebContentsTag {
   ~PortalTag() override;
 
   // task_manager::WebContentsTag:
-  PortalTask* CreateTask(WebContentsTaskProvider* task_provider) const override;
+  std::unique_ptr<RendererTask> CreateTask(
+      WebContentsTaskProvider* task_provider) const override;
 
  private:
   friend class WebContentsTags;

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

@@ -4,10 +4,13 @@
 
 #include "chrome/browser/task_manager/providers/web_contents/prerender_tag.h"
 
+#include <memory>
+
 namespace task_manager {
 
-PrerenderTask* PrerenderTag::CreateTask(WebContentsTaskProvider*) const {
-  return new PrerenderTask(web_contents());
+std::unique_ptr<RendererTask> PrerenderTag::CreateTask(
+    WebContentsTaskProvider*) const {
+  return std::make_unique<PrerenderTask>(web_contents());
 }
 
 PrerenderTag::PrerenderTag(content::WebContents* web_contents)

chrome/browser/task_manager/providers/web_contents/prerender_tag.h

@@ -18,7 +18,8 @@ class PrerenderTag : public WebContentsTag {
   ~PrerenderTag() override;
 
   // task_manager::WebContentsTag:
-  PrerenderTask* CreateTask(WebContentsTaskProvider*) const override;
+  std::unique_ptr<RendererTask> CreateTask(
+      WebContentsTaskProvider*) const override;
 
  private:
   friend class WebContentsTags;

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

@@ -4,10 +4,13 @@
 
 #include "chrome/browser/task_manager/providers/web_contents/printing_tag.h"
 
+#include <memory>
+
 namespace task_manager {
 
-PrintingTask* PrintingTag::CreateTask(WebContentsTaskProvider*) const {
-  return new PrintingTask(web_contents());
+std::unique_ptr<RendererTask> PrintingTag::CreateTask(
+    WebContentsTaskProvider*) const {
+  return std::make_unique<PrintingTask>(web_contents());
 }
 
 PrintingTag::PrintingTag(content::WebContents* web_contents)

chrome/browser/task_manager/providers/web_contents/printing_tag.h

@@ -18,7 +18,8 @@ class PrintingTag : public WebContentsTag {
   ~PrintingTag() override;
 
   // task_manager::WebContentsTag:
-  PrintingTask* CreateTask(WebContentsTaskProvider*) const override;
+  std::unique_ptr<RendererTask> CreateTask(
+      WebContentsTaskProvider*) const override;
 
  private:
   friend class WebContentsTags;

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

@@ -4,10 +4,13 @@
 
 #include "chrome/browser/task_manager/providers/web_contents/tab_contents_tag.h"
 
+#include <memory>
+
 namespace task_manager {
 
-TabContentsTask* TabContentsTag::CreateTask(WebContentsTaskProvider*) const {
-  return new TabContentsTask(web_contents());
+std::unique_ptr<RendererTask> TabContentsTag::CreateTask(
+    WebContentsTaskProvider*) const {
+  return std::make_unique<TabContentsTask>(web_contents());
 }
 
 TabContentsTag::TabContentsTag(content::WebContents* web_contents)

chrome/browser/task_manager/providers/web_contents/tab_contents_tag.h

@@ -17,7 +17,8 @@ class TabContentsTag : public WebContentsTag {
   ~TabContentsTag() override;
 
   // task_manager::WebContentsTag:
-  TabContentsTask* CreateTask(WebContentsTaskProvider*) const override;
+  std::unique_ptr<RendererTask> CreateTask(
+      WebContentsTaskProvider*) const override;
 
  private:
   friend class WebContentsTags;

chrome/browser/task_manager/providers/web_contents/web_contents_tag.h

@@ -5,6 +5,8 @@
 #ifndef CHROME_BROWSER_TASK_MANAGER_PROVIDERS_WEB_CONTENTS_WEB_CONTENTS_TAG_H_
 #define CHROME_BROWSER_TASK_MANAGER_PROVIDERS_WEB_CONTENTS_WEB_CONTENTS_TAG_H_
 
+#include <memory>
+
 #include "base/macros.h"
 #include "base/supports_user_data.h"
 
@@ -36,11 +38,9 @@ class WebContentsTag : public base::SupportsUserData::Data {
 
   // The concrete Tags know how to instantiate a |RendererTask| that corresponds
   // to the owning WebContents and Service. This will be used by the
-  // WebContentsTaskProvider to create the appropriate Tasks.
-  //
-  // The returned |RendererTask| is owned by the caller (in this case it will be
-  // the provider, |task_provider|).
-  virtual RendererTask* CreateTask(
+  // WebContentsTaskProvider to create the appropriate Tasks. |task_provicer| is
+  // provided in case the task needs it for construction.
+  virtual std::unique_ptr<RendererTask> CreateTask(
       WebContentsTaskProvider* task_provider) const = 0;
 
   content::WebContents* web_contents() const { return web_contents_; }

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

@@ -4,10 +4,13 @@
 
 #include "chrome/browser/task_manager/providers/web_contents/web_contents_task_provider.h"
 
+#include <memory>
+
 #include "base/bind.h"
-#include "base/macros.h"
+#include "base/check.h"
+#include "base/containers/flat_map.h"
+#include "base/containers/flat_set.h"
 #include "base/memory/weak_ptr.h"
-#include "base/stl_util.h"
 #include "chrome/browser/task_manager/providers/web_contents/subframe_task.h"
 #include "chrome/browser/task_manager/providers/web_contents/web_contents_tags_manager.h"
 #include "content/public/browser/navigation_handle.h"
@@ -29,11 +32,14 @@ namespace task_manager {
 // WebContents) of these events.
 // The entry owns the created tasks representing the WebContents, and it is
 // itself owned by the provider.
-class WebContentsEntry : public content::WebContentsObserver {
+class WebContentsTaskProvider::WebContentsEntry
+    : public content::WebContentsObserver {
  public:
   WebContentsEntry(content::WebContents* web_contents,
                    WebContentsTaskProvider* provider);
   ~WebContentsEntry() override;
+  WebContentsEntry(const WebContentsEntry&) = delete;
+  WebContentsEntry& operator=(const WebContentsEntry&) = delete;
 
   // Creates all the tasks associated with each |RenderFrameHost| in this
   // entry's WebContents.
@@ -75,9 +81,6 @@ class WebContentsEntry : public content::WebContentsObserver {
   // |ancestor|.
   void ClearTasksForDescendantsOf(RenderFrameHost* ancestor);
 
-  // Calls |on_task| for each task managed by this WebContentsEntry.
-  void ForEachTask(const base::RepeatingCallback<void(RendererTask*)> on_task);
-
   // Walks parents until hitting a process boundary. Returns the highest frame
   // in the same SiteInstance as |render_frame_host|.
   RenderFrameHost* FindLocalRoot(RenderFrameHost* render_frame_host) const;
@@ -85,70 +88,68 @@ class WebContentsEntry : public content::WebContentsObserver {
   // The provider that owns this entry.
   WebContentsTaskProvider* provider_;
 
-  // The RenderFrameHosts associated with this entry's WebContents that we're
-  // tracking mapped by their SiteInstances.
-  using FramesList = std::vector<RenderFrameHost*>;
-  std::map<SiteInstance*, FramesList> frames_by_site_instance_;
-
-  // The RendererTasks that we create for the task manager, mapped by their
-  // RenderFrameHosts. This owns the RenderTasks.
-  std::map<RenderFrameHost*, RendererTask*> tasks_by_frames_;
+  // For each SiteInstance, the frames within it that are tracked (local roots
+  // only as per FindLocalRoot()), and its RendererTask. The number of tracked
+  // items is small, thus flat_map and flat_set.
+  struct SiteInstanceInfo {
+    base::flat_set<RenderFrameHost*> frames;
+    std::unique_ptr<RendererTask> renderer_task;
+  };
+  base::flat_map<SiteInstance*, SiteInstanceInfo> site_instance_infos_;
 
   // States whether we did record a main frame for this entry.
-  SiteInstance* main_frame_site_instance_;
+  SiteInstance* main_frame_site_instance_ = nullptr;
 
   base::WeakPtrFactory<WebContentsEntry> weak_factory_{this};
-
-  DISALLOW_COPY_AND_ASSIGN(WebContentsEntry);
 };
 
 ////////////////////////////////////////////////////////////////////////////////
 
-WebContentsEntry::WebContentsEntry(content::WebContents* web_contents,
-                                   WebContentsTaskProvider* provider)
-    : WebContentsObserver(web_contents),
-      provider_(provider),
-      main_frame_site_instance_(nullptr) {}
+WebContentsTaskProvider::WebContentsEntry::WebContentsEntry(
+    content::WebContents* web_contents,
+    WebContentsTaskProvider* provider)
+    : WebContentsObserver(web_contents), provider_(provider) {}
 
-WebContentsEntry::~WebContentsEntry() {
+WebContentsTaskProvider::WebContentsEntry::~WebContentsEntry() {
   ClearAllTasks(false);
 }
 
-void WebContentsEntry::CreateAllTasks() {
+void WebContentsTaskProvider::WebContentsEntry::CreateAllTasks() {
   DCHECK(web_contents()->GetMainFrame());
   web_contents()->ForEachFrame(base::BindRepeating(
       &WebContentsEntry::CreateTaskForFrame, base::Unretained(this)));
 }
 
-void WebContentsEntry::ClearAllTasks(bool notify_observer) {
-  ForEachTask(base::BindRepeating(
-      [](WebContentsTaskProvider* provider, bool notify_observer,
-         content::WebContents* web_contents, RendererTask* task) {
-        task->set_termination_status(web_contents->GetCrashedStatus());
-        task->set_termination_error_code(web_contents->GetCrashedErrorCode());
-
-        if (notify_observer)
-          provider->NotifyObserverTaskRemoved(task);
-        delete task;
-      },
-      provider_, notify_observer, web_contents()));
-
-  frames_by_site_instance_.clear();
-  tasks_by_frames_.clear();
+void WebContentsTaskProvider::WebContentsEntry::ClearAllTasks(
+    bool notify_observer) {
+  for (auto& it : site_instance_infos_) {
+    RendererTask* task = it.second.renderer_task.get();
+    task->set_termination_status(web_contents()->GetCrashedStatus());
+    task->set_termination_error_code(web_contents()->GetCrashedErrorCode());
+
+    if (notify_observer)
+      provider_->NotifyObserverTaskRemoved(task);
+  }
+
+  site_instance_infos_.clear();
   main_frame_site_instance_ = nullptr;
 }
 
-RendererTask* WebContentsEntry::GetTaskForFrame(
+RendererTask* WebContentsTaskProvider::WebContentsEntry::GetTaskForFrame(
     RenderFrameHost* render_frame_host) const {
-  // Only local roots are in |tasks_by_frames_|.
-  auto itr = tasks_by_frames_.find(FindLocalRoot(render_frame_host));
-  if (itr == tasks_by_frames_.end())
+  SiteInstance* site_instance = render_frame_host->GetSiteInstance();
+  auto itr = site_instance_infos_.find(site_instance);
+  if (itr == site_instance_infos_.end())
+    return nullptr;
+
+  // Only local roots are in the frame list.
+  if (!itr->second.frames.count(render_frame_host))
     return nullptr;
 
-  return itr->second;
+  return itr->second.renderer_task.get();
 }
 
-RenderFrameHost* WebContentsEntry::FindLocalRoot(
+RenderFrameHost* WebContentsTaskProvider::WebContentsEntry::FindLocalRoot(
     RenderFrameHost* render_frame_host) const {
   SiteInstance* site_instance = render_frame_host->GetSiteInstance();
   RenderFrameHost* candidate = render_frame_host;
@@ -160,24 +161,29 @@ RenderFrameHost* WebContentsEntry::FindLocalRoot(
   return candidate;
 }
 
-void WebContentsEntry::RenderFrameDeleted(RenderFrameHost* render_frame_host) {
+void WebContentsTaskProvider::WebContentsEntry::RenderFrameDeleted(
+    RenderFrameHost* render_frame_host) {
   ClearTaskForFrame(render_frame_host);
 }
 
-void WebContentsEntry::RenderFrameHostChanged(RenderFrameHost* old_host,
-                                              RenderFrameHost* new_host) {
+void WebContentsTaskProvider::WebContentsEntry::RenderFrameHostChanged(
+    RenderFrameHost* old_host,
+    RenderFrameHost* new_host) {
   DCHECK(new_host->IsCurrent());
 
   // The navigating frame and its subframes are now pending deletion. Stop
   // tracking them immediately rather than when they are destroyed. The order of
   // deletion is important. The children must be removed first.
-  ClearTasksForDescendantsOf(old_host);
-  ClearTaskForFrame(old_host);
+  if (old_host) {
+    ClearTasksForDescendantsOf(old_host);
+    ClearTaskForFrame(old_host);
+  }
 
   CreateTaskForFrame(new_host);
 }
 
-void WebContentsEntry::RenderFrameCreated(RenderFrameHost* render_frame_host) {
+void WebContentsTaskProvider::WebContentsEntry::RenderFrameCreated(
+    RenderFrameHost* render_frame_host) {
   DCHECK(render_frame_host->IsRenderFrameLive());
 
   // Skip pending/speculative hosts. We'll create tasks for these if the
@@ -188,8 +194,9 @@ void WebContentsEntry::RenderFrameCreated(RenderFrameHost* render_frame_host) {
   CreateTaskForFrame(render_frame_host);
 }
 
-void WebContentsEntry::RenderFrameReady(int render_process_id,
-                                        int render_frame_id) {
+void WebContentsTaskProvider::WebContentsEntry::RenderFrameReady(
+    int render_process_id,
+    int render_frame_id) {
   // We get here when a RenderProcessHost we are tracking transitions to the
   // IsReady state. This might mean we know its process ID.
   content::RenderFrameHost* render_frame_host =
@@ -208,28 +215,23 @@ void WebContentsEntry::RenderFrameReady(int render_process_id,
       determine_pid_from_handle);
 }
 
-void WebContentsEntry::WebContentsDestroyed() {
+void WebContentsTaskProvider::WebContentsEntry::WebContentsDestroyed() {
   ClearAllTasks(true);
   provider_->DeleteEntry(web_contents());
 }
 
-void WebContentsEntry::OnRendererUnresponsive(
+void WebContentsTaskProvider::WebContentsEntry::OnRendererUnresponsive(
     RenderProcessHost* render_process_host) {
-  // Find the first RenderFrameHost matching the RenderProcessHost.
-  RendererTask* task = nullptr;
-  for (const auto& pair : tasks_by_frames_) {
+  for (const auto& pair : site_instance_infos_) {
     if (pair.first->GetProcess() == render_process_host) {
-      task = pair.second;
-      break;
+      provider_->NotifyObserverTaskUnresponsive(
+          pair.second.renderer_task.get());
+      return;
     }
   }
-  if (!task)
-    return;
-
-  provider_->NotifyObserverTaskUnresponsive(task);
 }
 
-void WebContentsEntry::DidFinishNavigation(
+void WebContentsTaskProvider::WebContentsEntry::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   // We only need to update tasks for main frame navigations.
   if (!navigation_handle->IsInMainFrame())
@@ -240,7 +242,9 @@ void WebContentsEntry::DidFinishNavigation(
   if (!main_frame_task)
     return;
 
-  ForEachTask(base::BindRepeating([](RendererTask* task) {
+  for (auto& it : site_instance_infos_) {
+    RendererTask* task = it.second.renderer_task.get();
+
     // 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.
@@ -251,41 +255,41 @@ void WebContentsEntry::DidFinishNavigation(
     // RendererTask::OnFaviconUpdated() will update the current favicon once
     // FaviconDriver figures out the correct favicon for the page.
     task->UpdateFavicon();
-  }));
+  }
 }
 
-void WebContentsEntry::TitleWasSet(content::NavigationEntry* entry) {
-  ForEachTask(base::BindRepeating([](RendererTask* task) {
+void WebContentsTaskProvider::WebContentsEntry::TitleWasSet(
+    content::NavigationEntry* entry) {
+  for (auto& it : site_instance_infos_) {
+    RendererTask* task = it.second.renderer_task.get();
     task->UpdateTitle();
     task->UpdateFavicon();
-  }));
+  }
 }
 
-void WebContentsEntry::CreateTaskForFrame(RenderFrameHost* render_frame_host) {
+void WebContentsTaskProvider::WebContentsEntry::CreateTaskForFrame(
+    RenderFrameHost* render_frame_host) {
   // Currently we do not track pending hosts, or pending delete hosts.
-  DCHECK(render_frame_host->IsCurrent());
   DCHECK(render_frame_host);
-  DCHECK(!tasks_by_frames_.count(render_frame_host));
-
-  content::SiteInstance* site_instance = render_frame_host->GetSiteInstance();
+  DCHECK(render_frame_host->IsCurrent());
 
-  // Exclude sad tabs and sad oopifs.
+  // Exclude sad tabs and sad OOPIFs.
   if (!render_frame_host->IsRenderFrameLive())
     return;
 
-  // Exclude frames in the same SiteInstance as their parent; |tasks_by_frames_|
-  // only contains local roots.
+  // Exclude frames in the same SiteInstance as their parent;
+  // |site_instance_by_frames_| only contains local roots.
+  content::SiteInstance* site_instance = render_frame_host->GetSiteInstance();
   if (render_frame_host->GetParent() &&
       site_instance == render_frame_host->GetParent()->GetSiteInstance()) {
     return;
   }
 
-  bool site_instance_exists =
-      frames_by_site_instance_.count(site_instance) != 0;
+  bool site_instance_exists = site_instance_infos_.count(site_instance) != 0;
   bool is_main_frame = (render_frame_host == web_contents()->GetMainFrame());
   bool site_instance_is_main = (site_instance == main_frame_site_instance_);
 
-  RendererTask* new_task = nullptr;
+  std::unique_ptr<RendererTask> new_task;
 
   // We need to create a task if one doesn't already exist for this
   // SiteInstance, or if the main frame navigates to a process that currently is
@@ -297,42 +301,29 @@ void WebContentsEntry::CreateTaskForFrame(RenderFrameHost* render_frame_host) {
       new_task = tag->CreateTask(provider_);
       main_frame_site_instance_ = site_instance;
     } else {
-      new_task =
-          new SubframeTask(render_frame_host, web_contents(),
-                           GetTaskForFrame(web_contents()->GetMainFrame()));
+      new_task = std::make_unique<SubframeTask>(
+          render_frame_host, web_contents(),
+          GetTaskForFrame(web_contents()->GetMainFrame()));
     }
   }
 
-  if (site_instance_exists) {
-    // One of the existing frame hosts for this site instance.
-    FramesList& existing_frames_for_site_instance =
-        frames_by_site_instance_[site_instance];
-    RenderFrameHost* existing_rfh = existing_frames_for_site_instance[0];
-    RendererTask* old_task = tasks_by_frames_[existing_rfh];
-
-    if (!new_task) {
-      // We didn't create any new task, so we keep using the old one.
-      tasks_by_frames_[render_frame_host] = old_task;
-    } else {
-      // Overwrite all the existing old tasks with the new one, and delete the
-      // old one.
-      for (RenderFrameHost* frame : existing_frames_for_site_instance)
-        tasks_by_frames_[frame] = new_task;
+  auto insert_result =
+      site_instance_infos_[site_instance].frames.insert(render_frame_host);
+  DCHECK(insert_result.second);
 
-      provider_->NotifyObserverTaskRemoved(old_task);
-      delete old_task;
+  if (new_task) {
+    if (site_instance_exists) {
+      provider_->NotifyObserverTaskRemoved(
+          site_instance_infos_[site_instance].renderer_task.get());
     }
-  }
 
-  frames_by_site_instance_[site_instance].push_back(render_frame_host);
-
-  if (new_task) {
-    tasks_by_frames_[render_frame_host] = new_task;
-    provider_->NotifyObserverTaskAdded(new_task);
+    RendererTask* new_task_ptr = new_task.get();
+    site_instance_infos_[site_instance].renderer_task = std::move(new_task);
+    provider_->NotifyObserverTaskAdded(new_task_ptr);
 
     // If we don't know the OS process handle yet (e.g., because this task is
     // still launching), update the task when it becomes available.
-    if (new_task->process_id() == base::kNullProcessId) {
+    if (new_task_ptr->process_id() == base::kNullProcessId) {
       render_frame_host->GetProcess()->PostTaskWhenProcessIsReady(
           base::BindOnce(&WebContentsEntry::RenderFrameReady,
                          weak_factory_.GetWeakPtr(),
@@ -342,37 +333,43 @@ void WebContentsEntry::CreateTaskForFrame(RenderFrameHost* render_frame_host) {
   }
 }
 
-void WebContentsEntry::ClearTaskForFrame(RenderFrameHost* render_frame_host) {
-  auto itr = tasks_by_frames_.find(render_frame_host);
-  if (itr == tasks_by_frames_.end())
+void WebContentsTaskProvider::WebContentsEntry::ClearTaskForFrame(
+    RenderFrameHost* render_frame_host) {
+  DCHECK(render_frame_host);
+  content::SiteInstance* site_instance = render_frame_host->GetSiteInstance();
+  auto itr = site_instance_infos_.find(site_instance);
+  if (itr == site_instance_infos_.end())
     return;
 
-  RendererTask* task = itr->second;
-  tasks_by_frames_.erase(itr);
-  content::SiteInstance* site_instance = render_frame_host->GetSiteInstance();
-  FramesList& frames = frames_by_site_instance_[site_instance];
-  frames.erase(std::find(frames.begin(), frames.end(), render_frame_host));
+  auto& frames = itr->second.frames;
+  size_t count = frames.erase(render_frame_host);
+  if (!count)
+    return;
 
   if (frames.empty()) {
-    frames_by_site_instance_.erase(site_instance);
-    provider_->NotifyObserverTaskRemoved(task);
-    delete task;
+    std::unique_ptr<RendererTask> renderer_task =
+        std::move(itr->second.renderer_task);
+    site_instance_infos_.erase(itr);
+    provider_->NotifyObserverTaskRemoved(renderer_task.get());
 
     if (site_instance == main_frame_site_instance_)
       main_frame_site_instance_ = nullptr;
   }
 
   // Whenever we have a task, we should have a main frame site instance.
-  DCHECK(tasks_by_frames_.empty() == (main_frame_site_instance_ == nullptr));
+  DCHECK(site_instance_infos_.empty() ==
+         (main_frame_site_instance_ == nullptr));
 }
 
-void WebContentsEntry::ClearTasksForDescendantsOf(RenderFrameHost* ancestor) {
+void WebContentsTaskProvider::WebContentsEntry::ClearTasksForDescendantsOf(
+    RenderFrameHost* ancestor) {
   // 1) Collect descendants.
-  std::vector<RenderFrameHost*> descendants;
-  for (auto it : tasks_by_frames_) {
-    RenderFrameHost* frame = it.first;
-    if (frame->IsDescendantOf(ancestor))
-      descendants.push_back(frame);
+  base::flat_set<RenderFrameHost*> descendants;
+  for (const auto& site_instance_pair : site_instance_infos_) {
+    for (auto* frame : site_instance_pair.second.frames) {
+      if (frame->IsDescendantOf(ancestor))
+        descendants.insert(frame);
+    }
   }
 
   // 2) Delete them.
@@ -380,20 +377,9 @@ void WebContentsEntry::ClearTasksForDescendantsOf(RenderFrameHost* ancestor) {
     ClearTaskForFrame(rfh);
 }
 
-void WebContentsEntry::ForEachTask(
-    base::RepeatingCallback<void(RendererTask*)> on_task) {
-  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]];
-
-    on_task.Run(task);
-  }
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 
-WebContentsTaskProvider::WebContentsTaskProvider() : is_updating_(false) {}
+WebContentsTaskProvider::WebContentsTaskProvider() = default;
 
 WebContentsTaskProvider::~WebContentsTaskProvider() {
   if (is_updating_) {
@@ -418,7 +404,7 @@ void WebContentsTaskProvider::OnWebContentsTagCreated(
     return;
   }
 
-  entry.reset(new WebContentsEntry(web_contents, this));
+  entry = std::make_unique<WebContentsEntry>(web_contents, this);
   entry->CreateAllTasks();
 }
 

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

@@ -17,7 +17,6 @@ class WebContents;
 
 namespace task_manager {
 
-class WebContentsEntry;
 class WebContentsTag;
 
 // Defines a provider to provide the renderer tasks that are associated with
@@ -45,7 +44,7 @@ class WebContentsTaskProvider : public TaskProvider {
   Task* GetTaskOfFrame(content::RenderFrameHost* frame);
 
  private:
-  friend class WebContentsEntry;
+  class WebContentsEntry;
 
   // task_manager::TaskProvider:
   void StartUpdating() override;
@@ -62,7 +61,7 @@ class WebContentsTaskProvider : public TaskProvider {
 
   // True if this provider is listening to WebContentsTags and updating its
   // observers, false otherwise.
-  bool is_updating_;
+  bool is_updating_ = false;
 
   DISALLOW_COPY_AND_ASSIGN(WebContentsTaskProvider);
 };