Split WorkerTaskProvider

The WorkerTaskProvider's responsibility is now to track existing
profiles and let the PerProfileWorkerTaskTracker track the running
workers and create the tasks.

This avoids keeping track of the ServiceWorkerContext that each
service worker is associated with.

Bug: 1041093
Change-Id: Ie0cd4c7f46c84c2f9a8c96d10afef8440abc63a3
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1994178Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Cr-Commit-Position: refs/heads/master@{#737682}

chrome/browser/BUILD.gn

@@ -3606,6 +3606,8 @@ jumbo_static_library("browser") {
       "task_manager/providers/child_process_task_provider.h",
       "task_manager/providers/fallback_task_provider.cc",
       "task_manager/providers/fallback_task_provider.h",
+      "task_manager/providers/per_profile_worker_task_tracker.cc",
+      "task_manager/providers/per_profile_worker_task_tracker.h",
       "task_manager/providers/render_process_host_task_provider.cc",
       "task_manager/providers/render_process_host_task_provider.h",
       "task_manager/providers/task.cc",

chrome/browser/task_manager/providers/per_profile_worker_task_tracker.cc

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "chrome/browser/task_manager/providers/per_profile_worker_task_tracker.h"
+
+#include "base/logging.h"
+#include "chrome/browser/task_manager/providers/worker_task.h"
+#include "chrome/browser/task_manager/providers/worker_task_provider.h"
+#include "content/public/browser/render_process_host.h"
+#include "content/public/browser/storage_partition.h"
+
+namespace task_manager {
+
+PerProfileWorkerTaskTracker::PerProfileWorkerTaskTracker(
+    WorkerTaskProvider* worker_task_provider,
+    Profile* profile)
+    : worker_task_provider_(worker_task_provider) {
+  DCHECK(worker_task_provider);
+  DCHECK(profile);
+
+  content::StoragePartition* storage_partition =
+      content::BrowserContext::GetDefaultStoragePartition(profile);
+
+  content::ServiceWorkerContext* service_worker_context =
+      storage_partition->GetServiceWorkerContext();
+  scoped_service_worker_context_observer_.Add(service_worker_context);
+
+  for (const auto& kv :
+       service_worker_context->GetRunningServiceWorkerInfos()) {
+    const int64_t version_id = kv.first;
+    const content::ServiceWorkerRunningInfo& running_info = kv.second;
+    OnVersionStartedRunning(version_id, running_info);
+  }
+}
+
+PerProfileWorkerTaskTracker::~PerProfileWorkerTaskTracker() = default;
+
+void PerProfileWorkerTaskTracker::OnVersionStartedRunning(
+    int64_t version_id,
+    const content::ServiceWorkerRunningInfo& running_info) {
+  CreateWorkerTask(version_id, running_info.render_process_id,
+                   running_info.script_url);
+}
+
+void PerProfileWorkerTaskTracker::OnVersionStoppedRunning(int64_t version_id) {
+  DeleteWorkerTask(version_id);
+}
+
+void PerProfileWorkerTaskTracker::CreateWorkerTask(int version_id,
+                                                   int worker_process_id,
+                                                   const GURL& script_url) {
+  auto* worker_process_host =
+      content::RenderProcessHost::FromID(worker_process_id);
+  auto insertion_result = service_worker_tasks_.emplace(
+      version_id, std::make_unique<WorkerTask>(
+                      worker_process_host->GetProcess().Handle(), script_url,
+                      Task::Type::SERVICE_WORKER, worker_process_id));
+  DCHECK(insertion_result.second);
+  worker_task_provider_->OnWorkerTaskAdded(
+      insertion_result.first->second.get());
+}
+
+void PerProfileWorkerTaskTracker::DeleteWorkerTask(int version_id) {
+  auto it = service_worker_tasks_.find(version_id);
+  DCHECK(it != service_worker_tasks_.end());
+  worker_task_provider_->OnWorkerTaskRemoved(it->second.get());
+  service_worker_tasks_.erase(it);
+}
+
+}  // namespace task_manager

chrome/browser/task_manager/providers/per_profile_worker_task_tracker.h

+// Copyright 2020 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef CHROME_BROWSER_TASK_MANAGER_PROVIDERS_PER_PROFILE_WORKER_TASK_TRACKER_H_
+#define CHROME_BROWSER_TASK_MANAGER_PROVIDERS_PER_PROFILE_WORKER_TASK_TRACKER_H_
+
+#include <memory>
+
+#include "base/containers/flat_map.h"
+#include "base/scoped_observer.h"
+#include "chrome/browser/task_manager/providers/task.h"
+#include "content/public/browser/service_worker_context.h"
+#include "content/public/browser/service_worker_context_observer.h"
+
+class Profile;
+
+namespace task_manager {
+
+class WorkerTask;
+class WorkerTaskProvider;
+
+// This is a helper class owned by WorkerTaskProvider to track all workers
+// associated with a single profile. It manages the WorkerTasks and sends
+// lifetime notifications to the WorkerTaskProvider.
+//
+// TODO(https://crbug.com/1041093): Add support for dedicated and shared
+//                                  workers.
+class PerProfileWorkerTaskTracker
+    : public content::ServiceWorkerContextObserver {
+ public:
+  PerProfileWorkerTaskTracker(WorkerTaskProvider* worker_task_provider,
+                              Profile* profile);
+
+  ~PerProfileWorkerTaskTracker() override;
+
+  PerProfileWorkerTaskTracker(const PerProfileWorkerTaskTracker&) = delete;
+  PerProfileWorkerTaskTracker& operator=(const PerProfileWorkerTaskTracker&) =
+      delete;
+
+  // content::ServiceWorkerContextObserver:
+  void OnVersionStartedRunning(
+      int64_t version_id,
+      const content::ServiceWorkerRunningInfo& running_info) override;
+  void OnVersionStoppedRunning(int64_t version_id) override;
+
+ private:
+  // Creates a service worker task and inserts it into |service_worker_tasks_|.
+  // Then it notifies the |worker_task_provider_| about the new Task.
+  void CreateWorkerTask(int version_id,
+                        int worker_process_id,
+                        const GURL& script_url);
+
+  // Deletes an existing service worker task from |service_worker_tasks_| and
+  // notifies |worker_task_provider_| about the deletion of the task.
+  void DeleteWorkerTask(int version_id);
+
+  // The provider that gets notified when a WorkerTask is created/deleted.
+  WorkerTaskProvider* const worker_task_provider_;  // Owner.
+
+  ScopedObserver<content::ServiceWorkerContext,
+                 content::ServiceWorkerContextObserver>
+      scoped_service_worker_context_observer_{this};
+
+  base::flat_map<int64_t /*version_id*/, std::unique_ptr<WorkerTask>>
+      service_worker_tasks_;
+};
+
+}  // namespace task_manager
+
+#endif  // CHROME_BROWSER_TASK_MANAGER_PROVIDERS_PER_PROFILE_WORKER_TASK_TRACKER_H_

chrome/browser/task_manager/providers/worker_task_provider.cc

@@ -4,14 +4,9 @@
 
 #include "chrome/browser/task_manager/providers/worker_task_provider.h"
 
-#include "base/stl_util.h"
 #include "chrome/browser/browser_process.h"
+#include "chrome/browser/task_manager/providers/per_profile_worker_task_tracker.h"
 #include "content/public/browser/browser_thread.h"
-#include "content/public/browser/render_process_host.h"
-#include "content/public/browser/service_worker_running_info.h"
-#include "content/public/browser/storage_partition.h"
-
-using content::BrowserThread;
 
 namespace task_manager {
 
@@ -24,60 +19,49 @@ Task* WorkerTaskProvider::GetTaskOfUrlRequest(int child_id, int route_id) {
 }
 
 void WorkerTaskProvider::OnProfileAdded(Profile* profile) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   observed_profiles_.Add(profile);
 
-  content::ServiceWorkerContext* context =
-      content::BrowserContext::GetDefaultStoragePartition(profile)
-          ->GetServiceWorkerContext();
-  scoped_context_observer_.Add(context);
-
-  // Create tasks for existing service workers.
-  for (const auto& kv : context->GetRunningServiceWorkerInfos()) {
-    const int64_t version_id = kv.first;
-    const content::ServiceWorkerRunningInfo& running_info = kv.second;
-
-    CreateTask(context, version_id, running_info);
-  }
+  auto per_profile_worker_task_tracker =
+      std::make_unique<PerProfileWorkerTaskTracker>(this, profile);
+  const bool inserted =
+      per_profile_worker_task_trackers_
+          .emplace(profile, std::move(per_profile_worker_task_tracker))
+          .second;
+  DCHECK(inserted);
 }
 
 void WorkerTaskProvider::OnOffTheRecordProfileCreated(Profile* off_the_record) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   OnProfileAdded(off_the_record);
 }
 
 void WorkerTaskProvider::OnProfileWillBeDestroyed(Profile* profile) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   observed_profiles_.Remove(profile);
 
-  content::ServiceWorkerContext* context =
-      content::BrowserContext::GetDefaultStoragePartition(profile)
-          ->GetServiceWorkerContext();
-  scoped_context_observer_.Remove(context);
+  auto it = per_profile_worker_task_trackers_.find(profile);
+  DCHECK(it != per_profile_worker_task_trackers_.end());
+  per_profile_worker_task_trackers_.erase(it);
 }
 
-void WorkerTaskProvider::OnVersionStartedRunning(
-    content::ServiceWorkerContext* context,
-    int64_t version_id,
-    const content::ServiceWorkerRunningInfo& running_info) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+void WorkerTaskProvider::OnWorkerTaskAdded(Task* worker_task) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  CreateTask(context, version_id, running_info);
+  NotifyObserverTaskAdded(worker_task);
 }
 
-void WorkerTaskProvider::OnVersionStoppedRunning(
-    content::ServiceWorkerContext* context,
-    int64_t version_id) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+void WorkerTaskProvider::OnWorkerTaskRemoved(Task* worker_task) {
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
-  DeleteTask(context, version_id);
+  NotifyObserverTaskRemoved(worker_task);
 }
 
 void WorkerTaskProvider::StartUpdating() {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   ProfileManager* profile_manager = g_browser_process->profile_manager();
   if (profile_manager) {
@@ -96,47 +80,14 @@ void WorkerTaskProvider::StartUpdating() {
 }
 
 void WorkerTaskProvider::StopUpdating() {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
+  DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
   // Stop observing profile creation and destruction.
   scoped_profile_manager_observer_.RemoveAll();
   observed_profiles_.RemoveAll();
 
-  // Stop observing contexts.
-  scoped_context_observer_.RemoveAll();
-
-  // Delete all tracked tasks.
-  service_worker_task_map_.clear();
-}
-
-void WorkerTaskProvider::CreateTask(
-    content::ServiceWorkerContext* context,
-    int64_t version_id,
-    const content::ServiceWorkerRunningInfo& running_info) {
-  DCHECK_CURRENTLY_ON(BrowserThread::UI);
-
-  const ServiceWorkerTaskKey key(context, version_id);
-  DCHECK(!base::Contains(service_worker_task_map_, key));
-
-  const int render_process_id = running_info.render_process_id;
-  auto* host = content::RenderProcessHost::FromID(render_process_id);
-  auto result = service_worker_task_map_.emplace(
-      key, std::make_unique<WorkerTask>(
-               host->GetProcess().Handle(), running_info.script_url,
-               Task::Type::SERVICE_WORKER, render_process_id));
-
-  DCHECK(result.second);
-  NotifyObserverTaskAdded(result.first->second.get());
-}
-
-void WorkerTaskProvider::DeleteTask(content::ServiceWorkerContext* context,
-                                    int version_id) {
-  const ServiceWorkerTaskKey key(context, version_id);
-  auto it = service_worker_task_map_.find(key);
-  DCHECK(it != service_worker_task_map_.end());
-
-  NotifyObserverTaskRemoved(it->second.get());
-  service_worker_task_map_.erase(it);
+  // Clear all ProfileWorkerTaskProvider instances to remove existing tasks.
+  per_profile_worker_task_trackers_.clear();
 }
 
 }  // namespace task_manager

chrome/browser/task_manager/providers/worker_task_provider.h

@@ -5,25 +5,28 @@
 #ifndef CHROME_BROWSER_TASK_MANAGER_PROVIDERS_WORKER_TASK_PROVIDER_H_
 #define CHROME_BROWSER_TASK_MANAGER_PROVIDERS_WORKER_TASK_PROVIDER_H_
 
-#include <map>
 #include <memory>
-#include <utility>
 
+#include "base/containers/flat_map.h"
 #include "base/scoped_observer.h"
 #include "chrome/browser/profiles/profile.h"
 #include "chrome/browser/profiles/profile_manager.h"
 #include "chrome/browser/profiles/profile_manager_observer.h"
 #include "chrome/browser/profiles/profile_observer.h"
 #include "chrome/browser/task_manager/providers/task_provider.h"
-#include "chrome/browser/task_manager/providers/worker_task.h"
-#include "content/public/browser/service_worker_context.h"
-#include "content/public/browser/service_worker_context_observer.h"
 
 namespace task_manager {
 
-// This class provides tasks that describe running workers of all types
-// (dedicated, shared or service workers).
+class PerProfileWorkerTaskTracker;
+
+// This is an implementation of TaskProvider that tracks dedicated workers,
+// shared workers and service workers.
 //
+// It doesn't directly create the tasks but instead creates an instance of
+// PerProfileWorkerTaskTracker for each existing profile. These
+// PerProfileWorkerTaskTracker instances are then responsible for tracking
+// workers and creating/deleting tasks.
+
 // See https://w3c.github.io/workers/ or https://w3c.github.io/ServiceWorker/
 // for more details.
 //
@@ -31,13 +34,11 @@ namespace task_manager {
 //                                  workers.
 class WorkerTaskProvider : public TaskProvider,
                            public ProfileManagerObserver,
-                           public ProfileObserver,
-                           public content::ServiceWorkerContextObserver {
+                           public ProfileObserver {
  public:
   WorkerTaskProvider();
   ~WorkerTaskProvider() override;
 
-  // Non-copyable.
   WorkerTaskProvider(const WorkerTaskProvider& other) = delete;
   WorkerTaskProvider& operator=(const WorkerTaskProvider& other) = delete;
 
@@ -51,43 +52,24 @@ class WorkerTaskProvider : public TaskProvider,
   void OnOffTheRecordProfileCreated(Profile* off_the_record) override;
   void OnProfileWillBeDestroyed(Profile* profile) override;
 
-  // content::ServiceWorkerContextObserver:
-  void OnVersionStartedRunning(
-      content::ServiceWorkerContext* context,
-      int64_t version_id,
-      const content::ServiceWorkerRunningInfo& running_info) override;
-  void OnVersionStoppedRunning(content::ServiceWorkerContext* context,
-                               int64_t version_id) override;
+  // Invoked by the PerProfileWorkerTaskTrackers when a new task is created or
+  // deleted.
+  void OnWorkerTaskAdded(Task* worker_task);
+  void OnWorkerTaskRemoved(Task* worker_task);
 
  private:
   // task_manager::TaskProvider:
   void StartUpdating() override;
   void StopUpdating() override;
 
-  // Creates a WorkerTask from the given |running_info| and notifies the
-  // observer of its addition.
-  void CreateTask(content::ServiceWorkerContext* context,
-                  int64_t version_id,
-                  const content::ServiceWorkerRunningInfo& running_info);
-
-  // Deletes a WorkerTask associated with |version_id| after notifying the
-  // observer of its deletion.
-  void DeleteTask(content::ServiceWorkerContext* context, int version_id);
-
   ScopedObserver<ProfileManager, ProfileManagerObserver>
       scoped_profile_manager_observer_{this};
 
   ScopedObserver<Profile, ProfileObserver> observed_profiles_{this};
 
-  using ServiceWorkerTaskKey =
-      std::pair<content::ServiceWorkerContext*, int64_t /*version_id*/>;
-  using ServiceWorkerTaskMap =
-      std::map<ServiceWorkerTaskKey, std::unique_ptr<WorkerTask>>;
-  ServiceWorkerTaskMap service_worker_task_map_;
-
-  ScopedObserver<content::ServiceWorkerContext,
-                 content::ServiceWorkerContextObserver>
-      scoped_context_observer_{this};
+  // Observes all types of workers for a given profile.
+  base::flat_map<Profile*, std::unique_ptr<PerProfileWorkerTaskTracker>>
+      per_profile_worker_task_trackers_;
 };
 
 }  // namespace task_manager

content/browser/service_worker/service_worker_browsertest.cc

@@ -458,7 +458,6 @@ class WorkerRunningStatusObserver : public ServiceWorkerContextObserver {
   }
 
   void OnVersionStartedRunning(
-      ServiceWorkerContext* context,
       int64_t version_id,
       const ServiceWorkerRunningInfo& running_info) override {
     version_id_ = version_id;

content/browser/service_worker/service_worker_context_unittest.cc

@@ -329,7 +329,6 @@ class TestServiceWorkerContextObserver : public ServiceWorkerContextObserver {
   }
 
   void OnVersionStartedRunning(
-      content::ServiceWorkerContext* context,
       int64_t version_id,
       const ServiceWorkerRunningInfo& running_info) override {
     EventLog log;
@@ -338,8 +337,7 @@ class TestServiceWorkerContextObserver : public ServiceWorkerContextObserver {
     events_.push_back(log);
   }
 
-  void OnVersionStoppedRunning(content::ServiceWorkerContext* context,
-                               int64_t version_id) override {
+  void OnVersionStoppedRunning(int64_t version_id) override {
     EventLog log;
     log.type = EventType::VersionStoppedRunning;
     log.version_id = version_id;

content/browser/service_worker/service_worker_context_wrapper.cc

@@ -391,9 +391,8 @@ void ServiceWorkerContextWrapper::OnStarted(int64_t version_id,
   DCHECK(insertion_result.second);
 
   const auto& running_info = insertion_result.first->second;
-  for (auto& observer : observer_list_) {
-    observer.OnVersionStartedRunning(this, version_id, running_info);
-  }
+  for (auto& observer : observer_list_)
+    observer.OnVersionStartedRunning(version_id, running_info);
 }
 
 void ServiceWorkerContextWrapper::OnStopped(int64_t version_id) {
@@ -403,7 +402,7 @@ void ServiceWorkerContextWrapper::OnStopped(int64_t version_id) {
   if (it != running_service_workers_.end()) {
     running_service_workers_.erase(it);
     for (auto& observer : observer_list_)
-      observer.OnVersionStoppedRunning(this, version_id);
+      observer.OnVersionStoppedRunning(version_id);
   }
 }
 
@@ -413,7 +412,7 @@ void ServiceWorkerContextWrapper::OnDeleteAndStartOver() {
   for (const auto& kv : running_service_workers_) {
     int64_t version_id = kv.first;
     for (auto& observer : observer_list_)
-      observer.OnVersionStoppedRunning(this, version_id);
+      observer.OnVersionStoppedRunning(version_id);
   }
   running_service_workers_.clear();
 }

content/public/browser/service_worker_context_observer.h

@@ -48,11 +48,9 @@ class ServiceWorkerContextObserver {
   // process of the service worker is allocated/released, instead of using the
   // running status of the embedded worker.
   virtual void OnVersionStartedRunning(
-      ServiceWorkerContext* context,
       int64_t version_id,
       const ServiceWorkerRunningInfo& running_info) {}
-  virtual void OnVersionStoppedRunning(ServiceWorkerContext* context,
-                                       int64_t version_id) {}
+  virtual void OnVersionStoppedRunning(int64_t version_id) {}
 
   // Called when there are no more controllees for the service worker with id
   // |version_id|.