Reland "[Task Manager] Properly clear worker tasks on profile shutdown."

This is a reland of b7526e4f, but
by with an additional fix.

When a PerProfileWorkerTaskTracker instance is deleted because
StopUpdating() was called, then the removal notifications must
not be sent to the task manager (as part of the TaskProvider's
API contract).

This is handled in WorkerTaskProvider by dropping the notification
when IsUpdating() is false.

Original change's description:
> [Task Manager] Properly clear worker tasks on profile shutdown.
>
> Fixes PerProfileWorkerTaskTracker so that the WorkerTaskTracker is
> notified when there are outstanding tasks that are about to be
> deleted.
>
> Bug: 1060506
> Change-Id: Ic81ad1557c1e27a8fc96fc079f7447efcc2ce7f3
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2110350
> Auto-Submit: Patrick Monette <pmonette@chromium.org>
> Commit-Queue: Ahmed Fakhry <afakhry@chromium.org>
> Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#751915}

Bug: 1060506
Change-Id: I3c1a7f8f94a580d021b7e3658af885110bfa8d2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2118770
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#753519}

chrome/browser/task_manager/providers/per_profile_worker_task_tracker.cc

@@ -47,7 +47,16 @@ PerProfileWorkerTaskTracker::PerProfileWorkerTaskTracker(
   }
 }
 
-PerProfileWorkerTaskTracker::~PerProfileWorkerTaskTracker() = default;
+PerProfileWorkerTaskTracker::~PerProfileWorkerTaskTracker() {
+  // Notify the |worker_task_provider_| for all outstanding tasks that are about
+  // to be deleted.
+  for (const auto& kv : dedicated_worker_tasks_)
+    worker_task_provider_->OnWorkerTaskRemoved(kv.second.get());
+  for (const auto& kv : shared_worker_tasks_)
+    worker_task_provider_->OnWorkerTaskRemoved(kv.second.get());
+  for (const auto& kv : service_worker_tasks_)
+    worker_task_provider_->OnWorkerTaskRemoved(kv.second.get());
+}
 
 void PerProfileWorkerTaskTracker::OnWorkerStarted(
     content::DedicatedWorkerId dedicated_worker_id,

chrome/browser/task_manager/providers/task_provider.cc

@@ -26,6 +26,10 @@ void TaskProvider::ClearObserver() {
   StopUpdating();
 }
 
+bool TaskProvider::IsUpdating() const {
+  return observer_ != nullptr;
+}
+
 void TaskProvider::NotifyObserverTaskAdded(Task* task) const {
   DCHECK(observer_);
   observer_->TaskAdded(task);

chrome/browser/task_manager/providers/task_provider.h

@@ -40,6 +40,10 @@ class TaskProvider {
   void ClearObserver();
 
  protected:
+  // Indicates if this instance is currently tracking tasks. Will return true
+  // between the calls to StartUpdating() and StopUpdating().
+  bool IsUpdating() const;
+
   // Used by concrete task providers to notify the observer of tasks addition/
   // removal/renderer unresponsive. These methods should only be called after
   // StartUpdating() has been called and before StopUpdating() is called.

chrome/browser/task_manager/providers/worker_task_provider.cc

@@ -57,6 +57,11 @@ void WorkerTaskProvider::OnWorkerTaskAdded(Task* worker_task) {
 void WorkerTaskProvider::OnWorkerTaskRemoved(Task* worker_task) {
   DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
 
+  // Do not forward the notification when StopUpdating() has been called, as the
+  // observer is now null.
+  if (!IsUpdating())
+    return;
+
   NotifyObserverTaskRemoved(worker_task);
 }
 

chrome/browser/task_manager/providers/worker_task_provider_browsertest.cc

@@ -325,4 +325,38 @@ IN_PROC_BROWSER_TEST_F(WorkerTaskProviderBrowserTest, CreateExistingTasks) {
   StopUpdating();
 }
 
+// Tests that destroying a profile while updating will correctly remove the
+// existing tasks. An incognito browser is used because a regular profile is
+// never truly destroyed until browser shutdown (See https://crbug.com/88586).
+IN_PROC_BROWSER_TEST_F(WorkerTaskProviderBrowserTest, DestroyedProfile) {
+  StartUpdating();
+
+  EXPECT_TRUE(tasks().empty());
+  Browser* browser = CreateIncognitoBrowser();
+
+  ui_test_utils::NavigateToURL(
+      browser, embedded_test_server()->GetURL(
+                   "/service_worker/create_service_worker.html"));
+  EXPECT_EQ("DONE", EvalJs(browser->tab_strip_model()->GetActiveWebContents(),
+                           "register('respond_with_fetch_worker.js');"));
+  WaitUntilTaskCount(1);
+
+  const Task* task_1 = tasks()[0];
+  EXPECT_EQ(task_1->GetChildProcessUniqueID(), GetChildProcessID(browser));
+  EXPECT_EQ(Task::SERVICE_WORKER, task_1->GetType());
+  EXPECT_TRUE(base::StartsWith(
+      task_1->title(),
+      ExpectedTaskTitle(
+          embedded_test_server()
+              ->GetURL("/service_worker/respond_with_fetch_worker.js")
+              .spec()),
+      base::CompareCase::INSENSITIVE_ASCII));
+
+  CloseBrowserSynchronously(browser);
+
+  WaitUntilTaskCount(0);
+
+  StopUpdating();
+}
+
 }  // namespace task_manager