Make dedicated workers enumerable via the DedicatedWorkerService

This will allow the WorkerTaskProvider to do accurate bookkeeping on
running dedicated workers, even though it dynamically subscribe and
unsubscribe to the DedicatedWorkerService while some workers are already
running.

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

content/browser/worker_host/dedicated_worker_service_impl.cc

@@ -18,6 +18,17 @@ void DedicatedWorkerServiceImpl::RemoveObserver(Observer* observer) {
   observers_.RemoveObserver(observer);
 }
 
+void DedicatedWorkerServiceImpl::EnumerateDedicatedWorkers(Observer* observer) {
+  for (const auto& kv : dedicated_worker_infos_) {
+    DedicatedWorkerId dedicated_worker_id = kv.first;
+    const DedicatedWorkerInfo& dedicated_worker_info = kv.second;
+
+    observer->OnWorkerStarted(
+        dedicated_worker_id, dedicated_worker_info.worker_process_id,
+        dedicated_worker_info.ancestor_render_frame_host_id);
+  }
+}
+
 DedicatedWorkerId DedicatedWorkerServiceImpl::GenerateNextDedicatedWorkerId() {
   return dedicated_worker_id_generator_.GenerateNextId();
 }
@@ -26,6 +37,14 @@ void DedicatedWorkerServiceImpl::NotifyWorkerStarted(
     DedicatedWorkerId dedicated_worker_id,
     int worker_process_id,
     GlobalFrameRoutingId ancestor_render_frame_host_id) {
+  bool inserted =
+      dedicated_worker_infos_
+          .emplace(dedicated_worker_id,
+                   DedicatedWorkerInfo{worker_process_id,
+                                       ancestor_render_frame_host_id})
+          .second;
+  DCHECK(inserted);
+
   for (Observer& observer : observers_) {
     observer.OnWorkerStarted(dedicated_worker_id, worker_process_id,
                              ancestor_render_frame_host_id);
@@ -35,6 +54,9 @@ void DedicatedWorkerServiceImpl::NotifyWorkerStarted(
 void DedicatedWorkerServiceImpl::NotifyWorkerTerminating(
     DedicatedWorkerId dedicated_worker_id,
     GlobalFrameRoutingId ancestor_render_frame_host_id) {
+  size_t removed = dedicated_worker_infos_.erase(dedicated_worker_id);
+  DCHECK_EQ(removed, 1u);
+
   for (Observer& observer : observers_) {
     observer.OnBeforeWorkerTerminated(dedicated_worker_id,
                                       ancestor_render_frame_host_id);

content/browser/worker_host/dedicated_worker_service_impl.h

@@ -21,6 +21,7 @@ class CONTENT_EXPORT DedicatedWorkerServiceImpl
   // DedicatedWorkerService:
   void AddObserver(Observer* observer) override;
   void RemoveObserver(Observer* observer) override;
+  void EnumerateDedicatedWorkers(Observer* observer) override;
 
   DedicatedWorkerId GenerateNextDedicatedWorkerId();
 
@@ -37,6 +38,13 @@ class CONTENT_EXPORT DedicatedWorkerServiceImpl
   DedicatedWorkerId::Generator dedicated_worker_id_generator_;
 
   base::ObserverList<Observer> observers_;
+
+  struct DedicatedWorkerInfo {
+    int worker_process_id;
+    GlobalFrameRoutingId ancestor_render_frame_host_id;
+  };
+  base::flat_map<DedicatedWorkerId, DedicatedWorkerInfo>
+      dedicated_worker_infos_;
 };
 
 }  // namespace content

content/browser/worker_host/dedicated_worker_service_impl_unittest.cc

@@ -5,6 +5,7 @@
 #include "content/browser/worker_host/dedicated_worker_service_impl.h"
 
 #include <memory>
+#include <utility>
 
 #include "base/run_loop.h"
 #include "base/scoped_observer.h"
@@ -81,38 +82,72 @@ class MockDedicatedWorker
   mojo::Remote<blink::mojom::DedicatedWorkerHost> remote_host_;
 };
 
-class DedicatedWorkerServiceImplTest : public RenderViewHostImplTestHarness,
-                                       public testing::WithParamInterface<bool>,
-                                       public DedicatedWorkerService::Observer {
+class DedicatedWorkerServiceImplTest
+    : public RenderViewHostImplTestHarness,
+      public testing::WithParamInterface<bool> {
  public:
-  struct DedicatedWorkerInfo {
-    int worker_process_id;
-    GlobalFrameRoutingId ancestor_render_frame_host_id;
-  };
-
   DedicatedWorkerServiceImplTest() = default;
   ~DedicatedWorkerServiceImplTest() override = default;
 
   // Non-copyable.
   DedicatedWorkerServiceImplTest(const DedicatedWorkerServiceImplTest& other) =
       delete;
+  DedicatedWorkerServiceImplTest& operator=(
+      const DedicatedWorkerServiceImplTest& other) = delete;
 
   void SetUp() override {
     scoped_feature_list_.InitWithFeatureState(
         blink::features::kPlzDedicatedWorker, GetParam());
     RenderViewHostImplTestHarness::SetUp();
     browser_context_ = std::make_unique<TestBrowserContext>();
-    scoped_dedicated_worker_service_observer_.Add(
-        BrowserContext::GetDefaultStoragePartition(browser_context_.get())
-            ->GetDedicatedWorkerService());
   }
 
   void TearDown() override {
-    scoped_dedicated_worker_service_observer_.RemoveAll();
     browser_context_ = nullptr;
     RenderViewHostImplTestHarness::TearDown();
   }
 
+  std::unique_ptr<TestWebContents> CreateWebContents(const GURL& url) {
+    std::unique_ptr<TestWebContents> web_contents(TestWebContents::Create(
+        browser_context_.get(),
+        SiteInstanceImpl::Create(browser_context_.get())));
+    web_contents->NavigateAndCommit(url);
+    return web_contents;
+  }
+
+  DedicatedWorkerService* GetDedicatedWorkerService() const {
+    return BrowserContext::GetDefaultStoragePartition(browser_context_.get())
+        ->GetDedicatedWorkerService();
+  }
+
+ private:
+  // Controls the state of the blink::features::kPlzDedicatedWorker feature.
+  base::test::ScopedFeatureList scoped_feature_list_;
+
+  std::unique_ptr<TestBrowserContext> browser_context_;
+};
+
+class TestDedicatedWorkerServiceObserver
+    : public DedicatedWorkerService::Observer {
+ public:
+  struct DedicatedWorkerInfo {
+    int worker_process_id;
+    GlobalFrameRoutingId ancestor_render_frame_host_id;
+
+    bool operator==(const DedicatedWorkerInfo& other) const {
+      return std::tie(worker_process_id, ancestor_render_frame_host_id) ==
+             std::tie(other.worker_process_id,
+                      other.ancestor_render_frame_host_id);
+    }
+  };
+
+  TestDedicatedWorkerServiceObserver() = default;
+  ~TestDedicatedWorkerServiceObserver() override = default;
+
+  // Non-copyable.
+  TestDedicatedWorkerServiceObserver(
+      const TestDedicatedWorkerServiceObserver& other) = delete;
+
   // DedicatedWorkerService::Observer:
   void OnWorkerStarted(
       DedicatedWorkerId dedicated_worker_id,
@@ -126,8 +161,8 @@ class DedicatedWorkerServiceImplTest : public RenderViewHostImplTestHarness,
             .second;
     DCHECK(inserted);
 
-    DCHECK(on_worker_event_callback_);
-    std::move(on_worker_event_callback_).Run();
+    if (on_worker_event_callback_)
+      std::move(on_worker_event_callback_).Run();
   }
   void OnBeforeWorkerTerminated(
       DedicatedWorkerId dedicated_worker_id,
@@ -135,8 +170,8 @@ class DedicatedWorkerServiceImplTest : public RenderViewHostImplTestHarness,
     size_t removed = dedicated_worker_infos_.erase(dedicated_worker_id);
     DCHECK_EQ(removed, 1u);
 
-    DCHECK(on_worker_event_callback_);
-    std::move(on_worker_event_callback_).Run();
+    if (on_worker_event_callback_)
+      std::move(on_worker_event_callback_).Run();
   }
 
   void RunUntilWorkerEvent() {
@@ -146,27 +181,11 @@ class DedicatedWorkerServiceImplTest : public RenderViewHostImplTestHarness,
   }
 
   const base::flat_map<DedicatedWorkerId, DedicatedWorkerInfo>&
-  dedicated_worker_infos() {
+  dedicated_worker_infos() const {
     return dedicated_worker_infos_;
   }
 
-  std::unique_ptr<TestWebContents> CreateWebContents(const GURL& url) {
-    std::unique_ptr<TestWebContents> web_contents(TestWebContents::Create(
-        browser_context_.get(),
-        SiteInstanceImpl::Create(browser_context_.get())));
-    web_contents->NavigateAndCommit(url);
-    return web_contents;
-  }
-
  private:
-  // Controls the state of the blink::features::kPlzDedicatedWorker feature.
-  base::test::ScopedFeatureList scoped_feature_list_;
-
-  std::unique_ptr<TestBrowserContext> browser_context_;
-
-  ScopedObserver<DedicatedWorkerService, DedicatedWorkerService::Observer>
-      scoped_dedicated_worker_service_observer_{this};
-
   // Used to wait until one of OnWorkerStarted() or OnBeforeWorkerTerminated()
   // is called.
   base::OnceClosure on_worker_event_callback_;
@@ -175,36 +194,58 @@ class DedicatedWorkerServiceImplTest : public RenderViewHostImplTestHarness,
       dedicated_worker_infos_;
 };
 
-TEST_P(DedicatedWorkerServiceImplTest, ObserveWorkerCreationAndDestruction) {
+TEST_P(DedicatedWorkerServiceImplTest, DedicatedWorkerServiceObserver) {
+  // Set up the observer.
+  TestDedicatedWorkerServiceObserver observer;
+  ScopedObserver<DedicatedWorkerService, DedicatedWorkerService::Observer>
+      scoped_dedicated_worker_service_observer_(&observer);
+  scoped_dedicated_worker_service_observer_.Add(GetDedicatedWorkerService());
+
   std::unique_ptr<TestWebContents> web_contents =
       CreateWebContents(GURL("http://example.com/"));
   TestRenderFrameHost* render_frame_host = web_contents->GetMainFrame();
 
   // At first, there is no live dedicated worker.
-  EXPECT_TRUE(dedicated_worker_infos().empty());
+  EXPECT_TRUE(observer.dedicated_worker_infos().empty());
 
   // Create the dedicated worker.
-  GlobalFrameRoutingId ancestor_render_frame_host_id =
+  const GlobalFrameRoutingId ancestor_render_frame_host_id =
       render_frame_host->GetGlobalFrameRoutingId();
+  const int render_process_host_id = render_frame_host->GetProcess()->GetID();
   auto mock_dedicated_worker = std::make_unique<MockDedicatedWorker>(
-      render_frame_host->GetProcess()->GetID(), ancestor_render_frame_host_id);
-  RunUntilWorkerEvent();
+      render_process_host_id, ancestor_render_frame_host_id);
+  observer.RunUntilWorkerEvent();
 
   // The service sent a OnWorkerStarted() notification.
-  ASSERT_EQ(dedicated_worker_infos().size(), 1u);
-  const DedicatedWorkerInfo& dedicated_worker_info =
-      dedicated_worker_infos().begin()->second;
-  EXPECT_EQ(dedicated_worker_info.worker_process_id,
-            render_frame_host->GetProcess()->GetID());
+  ASSERT_EQ(observer.dedicated_worker_infos().size(), 1u);
+  const auto& dedicated_worker_info =
+      observer.dedicated_worker_infos().begin()->second;
+  EXPECT_EQ(dedicated_worker_info.worker_process_id, render_process_host_id);
   EXPECT_EQ(dedicated_worker_info.ancestor_render_frame_host_id,
             ancestor_render_frame_host_id);
 
+  // Test EnumerateDedicatedWorkers().
+  {
+    TestDedicatedWorkerServiceObserver enumeration_observer;
+    EXPECT_TRUE(enumeration_observer.dedicated_worker_infos().empty());
+
+    GetDedicatedWorkerService()->EnumerateDedicatedWorkers(
+        &enumeration_observer);
+
+    ASSERT_EQ(enumeration_observer.dedicated_worker_infos().size(), 1u);
+    const auto& dedicated_worker_info =
+        enumeration_observer.dedicated_worker_infos().begin()->second;
+    EXPECT_EQ(dedicated_worker_info.worker_process_id, render_process_host_id);
+    EXPECT_EQ(dedicated_worker_info.ancestor_render_frame_host_id,
+              ancestor_render_frame_host_id);
+  }
+
   // Delete the dedicated worker.
   mock_dedicated_worker = nullptr;
-  RunUntilWorkerEvent();
+  observer.RunUntilWorkerEvent();
 
   // The service sent a OnBeforeWorkerTerminated() notification.
-  EXPECT_TRUE(dedicated_worker_infos().empty());
+  EXPECT_TRUE(observer.dedicated_worker_infos().empty());
 }
 
 // Runs DedicatedWorkerServiceImplTest with both the enabled and disabled state

content/public/browser/dedicated_worker_service.h

@@ -34,6 +34,15 @@ class CONTENT_EXPORT DedicatedWorkerService {
   virtual void AddObserver(Observer* observer) = 0;
   virtual void RemoveObserver(Observer* observer) = 0;
 
+  // Invokes OnWorkerStarted() on |observer| for all existing dedicated workers.
+  //
+  // This function must be invoked in conjunction with AddObserver(). It is
+  // meant to be used by an observer that dynamically subscribes to the
+  // DedicatedWorkerService while some workers are already running. It avoids
+  // receiving a OnBeforeWorkerTerminated() event without having received the
+  // corresponding OnWorkerStart() event.
+  virtual void EnumerateDedicatedWorkers(Observer* observer) = 0;
+
  protected:
   virtual ~DedicatedWorkerService() = default;
 };