Move the SharedWorkerId to the SharedWorkerHost

This CL also changes the SharedWorkerService::Observer interface so
that SharedWorkerId replaces SharedWorkerInstance to uniquely
identify shared workers outside of content/.

The motivation behind this change is to make it cheaper to use
the shared worker identifier as a key into a map, since copying
a SharedWorkerInstance is quite expensive.

Bug: 1054596
Change-Id: I7270d9e5fabac2d2e00aecd407a745a1cc779791
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2067389
Commit-Queue: Patrick Monette <pmonette@chromium.org>
Reviewed-by: François Doray <fdoray@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Ahmed Fakhry <afakhry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#743988}

chrome/browser/task_manager/providers/per_profile_worker_task_tracker.cc

@@ -8,7 +8,6 @@
 #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/shared_worker_instance.h"
 #include "content/public/browser/storage_partition.h"
 
 namespace task_manager {
@@ -67,16 +66,18 @@ void PerProfileWorkerTaskTracker::OnBeforeWorkerTerminated(
 }
 
 void PerProfileWorkerTaskTracker::OnWorkerStarted(
-    const content::SharedWorkerInstance& instance,
+    content::SharedWorkerId shared_worker_id,
     int worker_process_id,
     const base::UnguessableToken& dev_tools_token) {
-  CreateWorkerTask(instance, Task::Type::SHARED_WORKER, worker_process_id,
-                   instance.url(), &shared_worker_tasks_);
+  // TODO(https://crbug.com/1047787): Make use of the worker's URL when it is
+  //                                  available.
+  CreateWorkerTask(shared_worker_id, Task::Type::SHARED_WORKER,
+                   worker_process_id, GURL(), &shared_worker_tasks_);
 }
 
 void PerProfileWorkerTaskTracker::OnBeforeWorkerTerminated(
-    const content::SharedWorkerInstance& instance) {
-  DeleteWorkerTask(instance, &shared_worker_tasks_);
+    content::SharedWorkerId shared_worker_id) {
+  DeleteWorkerTask(shared_worker_id, &shared_worker_tasks_);
 }
 
 void PerProfileWorkerTaskTracker::OnVersionStartedRunning(

chrome/browser/task_manager/providers/per_profile_worker_task_tracker.h

@@ -49,16 +49,16 @@ class PerProfileWorkerTaskTracker
       content::GlobalFrameRoutingId ancestor_render_frame_host_id) override;
 
   // content::SharedWorkerService::Observer:
-  void OnWorkerStarted(const content::SharedWorkerInstance& instance,
+  void OnWorkerStarted(content::SharedWorkerId shared_worker_id,
                        int worker_process_id,
                        const base::UnguessableToken& dev_tools_token) override;
   void OnBeforeWorkerTerminated(
-      const content::SharedWorkerInstance& instance) override;
+      content::SharedWorkerId shared_worker_id) override;
   void OnClientAdded(
-      const content::SharedWorkerInstance& instance,
+      content::SharedWorkerId shared_worker_id,
       content::GlobalFrameRoutingId render_frame_host_id) override {}
   void OnClientRemoved(
-      const content::SharedWorkerInstance& instance,
+      content::SharedWorkerId shared_worker_id,
       content::GlobalFrameRoutingId render_frame_host_id) override {}
 
   // content::ServiceWorkerContextObserver:
@@ -104,7 +104,7 @@ class PerProfileWorkerTaskTracker
                  content::SharedWorkerService::Observer>
       scoped_shared_worker_service_observer_{this};
 
-  base::flat_map<content::SharedWorkerInstance, std::unique_ptr<WorkerTask>>
+  base::flat_map<content::SharedWorkerId, std::unique_ptr<WorkerTask>>
       shared_worker_tasks_;
 
   // For service workers:

components/performance_manager/worker_watcher.cc

@@ -12,7 +12,6 @@
 #include "components/performance_manager/graph/worker_node_impl.h"
 #include "components/performance_manager/performance_manager_impl.h"
 #include "components/performance_manager/process_node_source.h"
-#include "content/public/browser/shared_worker_instance.h"
 
 namespace performance_manager {
 
@@ -142,20 +141,21 @@ void WorkerWatcher::OnBeforeWorkerTerminated(
 }
 
 void WorkerWatcher::OnWorkerStarted(
-    const content::SharedWorkerInstance& instance,
+    content::SharedWorkerId shared_worker_id,
     int worker_process_id,
     const base::UnguessableToken& dev_tools_token) {
   auto worker_node = PerformanceManagerImpl::GetInstance()->CreateWorkerNode(
       browser_context_id_, WorkerNode::WorkerType::kShared,
       process_node_source_->GetProcessNode(worker_process_id), dev_tools_token);
   bool inserted =
-      shared_worker_nodes_.emplace(instance, std::move(worker_node)).second;
+      shared_worker_nodes_.emplace(shared_worker_id, std::move(worker_node))
+          .second;
   DCHECK(inserted);
 }
 
 void WorkerWatcher::OnBeforeWorkerTerminated(
-    const content::SharedWorkerInstance& instance) {
-  auto it = shared_worker_nodes_.find(instance);
+    content::SharedWorkerId shared_worker_id) {
+  auto it = shared_worker_nodes_.find(shared_worker_id);
   DCHECK(it != shared_worker_nodes_.end());
 
   auto worker_node = std::move(it->second);
@@ -168,15 +168,16 @@ void WorkerWatcher::OnBeforeWorkerTerminated(
 }
 
 void WorkerWatcher::OnClientAdded(
-    const content::SharedWorkerInstance& instance,
+    content::SharedWorkerId shared_worker_id,
     content::GlobalFrameRoutingId render_frame_host_id) {
-  AddClientFrame(GetSharedWorkerNode(instance), render_frame_host_id);
+  AddClientFrame(GetSharedWorkerNode(shared_worker_id), render_frame_host_id);
 }
 
 void WorkerWatcher::OnClientRemoved(
-    const content::SharedWorkerInstance& instance,
+    content::SharedWorkerId shared_worker_id,
     content::GlobalFrameRoutingId render_frame_host_id) {
-  RemoveClientFrame(GetSharedWorkerNode(instance), render_frame_host_id);
+  RemoveClientFrame(GetSharedWorkerNode(shared_worker_id),
+                    render_frame_host_id);
 }
 
 void WorkerWatcher::AddClientFrame(
@@ -306,8 +307,8 @@ WorkerNodeImpl* WorkerWatcher::GetDedicatedWorkerNode(
 }
 
 WorkerNodeImpl* WorkerWatcher::GetSharedWorkerNode(
-    const content::SharedWorkerInstance& instance) {
-  auto it = shared_worker_nodes_.find(instance);
+    content::SharedWorkerId shared_worker_id) {
+  auto it = shared_worker_nodes_.find(shared_worker_id);
   if (it == shared_worker_nodes_.end()) {
     NOTREACHED();
     return nullptr;

components/performance_manager/worker_watcher.h

@@ -17,10 +17,6 @@
 #include "content/public/browser/global_routing_id.h"
 #include "content/public/browser/shared_worker_service.h"
 
-namespace content {
-class SharedWorkerInstance;
-}
-
 namespace performance_manager {
 
 class FrameNodeImpl;
@@ -56,16 +52,16 @@ class WorkerWatcher : public content::DedicatedWorkerService::Observer,
       content::GlobalFrameRoutingId ancestor_render_frame_host_id) override;
 
   // content::SharedWorkerService::Observer:
-  void OnWorkerStarted(const content::SharedWorkerInstance& instance,
+  void OnWorkerStarted(content::SharedWorkerId shared_worker_id,
                        int worker_process_id,
                        const base::UnguessableToken& dev_tools_token) override;
   void OnBeforeWorkerTerminated(
-      const content::SharedWorkerInstance& instance) override;
+      content::SharedWorkerId shared_worker_id) override;
   void OnClientAdded(
-      const content::SharedWorkerInstance& instance,
+      content::SharedWorkerId shared_worker_id,
       content::GlobalFrameRoutingId render_frame_host_id) override;
   void OnClientRemoved(
-      const content::SharedWorkerInstance& instance,
+      content::SharedWorkerId shared_worker_id,
       content::GlobalFrameRoutingId render_frame_host_id) override;
 
  private:
@@ -90,8 +86,7 @@ class WorkerWatcher : public content::DedicatedWorkerService::Observer,
   // Helper function to retrieve an existing shared worker node.
   WorkerNodeImpl* GetDedicatedWorkerNode(
       content::DedicatedWorkerId dedicated_worker_id);
-  WorkerNodeImpl* GetSharedWorkerNode(
-      const content::SharedWorkerInstance& instance);
+  WorkerNodeImpl* GetSharedWorkerNode(content::SharedWorkerId shared_worker_id);
 
   // The ID of the BrowserContext who owns the shared worker service.
   const std::string browser_context_id_;
@@ -117,8 +112,8 @@ class WorkerWatcher : public content::DedicatedWorkerService::Observer,
   base::flat_map<content::DedicatedWorkerId, std::unique_ptr<WorkerNodeImpl>>
       dedicated_worker_nodes_;
 
-  // Maps each SharedWorkerInstance to its worker node.
-  base::flat_map<content::SharedWorkerInstance, std::unique_ptr<WorkerNodeImpl>>
+  // Maps each shared worker ID to its worker node.
+  base::flat_map<content::SharedWorkerId, std::unique_ptr<WorkerNodeImpl>>
       shared_worker_nodes_;
 
   // Maps each frame to the shared workers that this frame is a client of. This

content/browser/worker_host/shared_worker_host.cc

@@ -88,9 +88,11 @@ class SharedWorkerHost::ScopedProcessHostRef {
 };
 
 SharedWorkerHost::SharedWorkerHost(SharedWorkerServiceImpl* service,
+                                   SharedWorkerId id,
                                    const SharedWorkerInstance& instance,
                                    RenderProcessHost* worker_process_host)
     : service_(service),
+      id_(id),
       instance_(instance),
       worker_process_host_(worker_process_host),
       scoped_process_host_ref_(
@@ -117,9 +119,9 @@ SharedWorkerHost::~SharedWorkerHost() {
     // Notify the service that each client still connected will be removed and
     // that the worker will terminate.
     for (const auto& client : clients_) {
-      service_->NotifyClientRemoved(instance_, client.render_frame_host_id);
+      service_->NotifyClientRemoved(id_, client.render_frame_host_id);
     }
-    service_->NotifyWorkerTerminating(instance_);
+    service_->NotifyWorkerTerminating(id_);
   } else {
     // Tell clients that this worker failed to start.
     for (const ClientInfo& info : clients_)
@@ -237,10 +239,10 @@ void SharedWorkerHost::Start(
 
   // Notify the service that the worker was started and that some clients were
   // already connected.
-  service_->NotifyWorkerStarted(instance_, worker_process_host_->GetID(),
+  service_->NotifyWorkerStarted(id_, worker_process_host_->GetID(),
                                 dev_tools_token_);
   for (const auto& client : clients_) {
-    service_->NotifyClientAdded(instance_, client.render_frame_host_id);
+    service_->NotifyClientAdded(id_, client.render_frame_host_id);
   }
 }
 
@@ -436,7 +438,7 @@ void SharedWorkerHost::AddClient(
   // Start() function will handle sending a notification for each existing
   // client.
   if (started_)
-    service_->NotifyClientAdded(instance_, client_render_frame_host_id);
+    service_->NotifyClientAdded(id_, client_render_frame_host_id);
 }
 
 void SharedWorkerHost::SetAppCacheHandle(
@@ -485,7 +487,7 @@ void SharedWorkerHost::OnClientConnectionLost() {
       // Notify the service that a client was removed while the worker was
       // running.
       if (started_) {
-        service_->NotifyClientRemoved(instance_, it->render_frame_host_id);
+        service_->NotifyClientRemoved(id_, it->render_frame_host_id);
       }
       clients_.erase(it);
       break;

content/browser/worker_host/shared_worker_host.h

@@ -22,6 +22,7 @@
 #include "content/public/browser/render_process_host.h"
 #include "content/public/browser/render_process_host_observer.h"
 #include "content/public/browser/shared_worker_instance.h"
+#include "content/public/browser/shared_worker_service.h"
 #include "media/mojo/mojom/video_decode_perf_history.mojom.h"
 #include "mojo/public/cpp/bindings/pending_receiver.h"
 #include "mojo/public/cpp/bindings/pending_remote.h"
@@ -54,14 +55,14 @@ class ServiceWorkerObjectHost;
 class SharedWorkerContentSettingsProxyImpl;
 class SharedWorkerServiceImpl;
 
-// The SharedWorkerHost is the interface that represents the browser side of
-// the browser <-> worker communication channel. This is owned by
-// SharedWorkerServiceImpl and destroyed when the connection to the worker in
-// the renderer is lost, or the RenderProcessHost of the worker is destroyed.
+// SharedWorkerHost is the browser-side host of a single shared worker running
+// in the renderer. This class is owned by the SharedWorkerServiceImpl of the
+// current BrowserContext.
 class CONTENT_EXPORT SharedWorkerHost : public blink::mojom::SharedWorkerHost,
                                         public RenderProcessHostObserver {
  public:
   SharedWorkerHost(SharedWorkerServiceImpl* service,
+                   SharedWorkerId id,
                    const SharedWorkerInstance& instance,
                    RenderProcessHost* worker_process_host);
   ~SharedWorkerHost() override;
@@ -128,6 +129,8 @@ class CONTENT_EXPORT SharedWorkerHost : public blink::mojom::SharedWorkerHost,
   // Returns true if this worker is connected to at least one client.
   bool HasClients() const;
 
+  SharedWorkerId id() const { return id_; }
+
   const SharedWorkerInstance& instance() const { return instance_; }
 
   const base::UnguessableToken& dev_tools_token() const {
@@ -192,7 +195,13 @@ class CONTENT_EXPORT SharedWorkerHost : public blink::mojom::SharedWorkerHost,
   mojo::Receiver<blink::mojom::SharedWorkerHost> receiver_{this};
 
   // |service_| owns |this|.
-  SharedWorkerServiceImpl* service_;
+  SharedWorkerServiceImpl* const service_;
+
+  // An identifier for this worker that is unique within a storage partition.
+  SharedWorkerId id_;
+
+  // This holds information used to match a shared worker connection request to
+  // this shared worker.
   SharedWorkerInstance instance_;
   ClientList clients_;
 

content/browser/worker_host/shared_worker_host_unittest.cc

@@ -53,15 +53,15 @@ class SharedWorkerHostTest : public testing::Test {
     GURL url("http://www.example.com/w.js");
 
     SharedWorkerInstance instance(
-        service_.shared_worker_id_generator_.GenerateNextId(), url,
-        blink::mojom::ScriptType::kClassic,
+        url, blink::mojom::ScriptType::kClassic,
         network::mojom::CredentialsMode::kSameOrigin, "name",
         url::Origin::Create(url), /*content_security_policy=*/"",
         network::mojom::ContentSecurityPolicyType::kReport,
         network::mojom::IPAddressSpace::kPublic,
         blink::mojom::SharedWorkerCreationContextType::kSecure);
-    auto host = std::make_unique<SharedWorkerHost>(&service_, instance,
-                                                   &mock_render_process_host_);
+    auto host = std::make_unique<SharedWorkerHost>(
+        &service_, service_.shared_worker_id_generator_.GenerateNextId(),
+        instance, &mock_render_process_host_);
     auto weak_host = host->AsWeakPtr();
     service_.worker_hosts_.insert(std::move(host));
     return weak_host;

content/browser/worker_host/shared_worker_instance_unittest.cc

@@ -22,8 +22,7 @@ class SharedWorkerInstanceTest : public testing::Test {
                                       const std::string& name,
                                       const url::Origin& constructor_origin) {
     return SharedWorkerInstance(
-        shared_worker_id_generator_.GenerateNextId(), script_url,
-        blink::mojom::ScriptType::kClassic,
+        script_url, blink::mojom::ScriptType::kClassic,
         network::mojom::CredentialsMode::kSameOrigin, name, constructor_origin,
         std::string(), network::mojom::ContentSecurityPolicyType::kReport,
         network::mojom::IPAddressSpace::kPublic,
@@ -42,8 +41,6 @@ class SharedWorkerInstanceTest : public testing::Test {
   }
 
  private:
-  SharedWorkerId::Generator shared_worker_id_generator_;
-
   DISALLOW_COPY_AND_ASSIGN(SharedWorkerInstanceTest);
 };
 
@@ -273,8 +270,7 @@ TEST_F(SharedWorkerInstanceTest, AddressSpace) {
       network::mojom::IPAddressSpace::kPublic};
   for (auto address_space : kAddressSpaces) {
     SharedWorkerInstance instance(
-        SharedWorkerId(), GURL("http://example.com/w.js"),
-        blink::mojom::ScriptType::kClassic,
+        GURL("http://example.com/w.js"), blink::mojom::ScriptType::kClassic,
         network::mojom::CredentialsMode::kSameOrigin, "name",
         url::Origin::Create(GURL("http://example.com/")), std::string(),
         network::mojom::ContentSecurityPolicyType::kReport, address_space,
@@ -283,30 +279,4 @@ TEST_F(SharedWorkerInstanceTest, AddressSpace) {
   }
 }
 
-// This test ensures that 2 distinct SharedWorkerInstance using the same file:
-// script URL have different identities and can be ordered.
-TEST_F(SharedWorkerInstanceTest, StrictWeakOrderingFileURLs) {
-  GURL script_url("file://path/to/script.js");
-  std::string name = "name";
-  url::Origin constructor_origin = url::Origin::Create(script_url);
-
-  SharedWorkerInstance instance1 =
-      CreateInstance(script_url, name, constructor_origin);
-  SharedWorkerInstance instance2 =
-      CreateInstance(script_url, name, constructor_origin);
-
-  // file: URLs are treated as opaque. Both instances should not match.
-  EXPECT_FALSE(instance1.Matches(instance2.url(), instance2.name(),
-                                 instance2.constructor_origin()));
-  EXPECT_FALSE(instance2.Matches(instance1.url(), instance1.name(),
-                                 instance1.constructor_origin()));
-
-  // The instances are not equivalent
-  EXPECT_TRUE(instance1 < instance2 || instance2 < instance1);
-
-  // An instance is equivalent to itself.
-  EXPECT_FALSE(instance1 < instance1);
-  EXPECT_FALSE(instance2 < instance2);
-}
-
 }  // namespace content

content/browser/worker_host/shared_worker_service_impl.cc

@@ -79,8 +79,7 @@ void SharedWorkerServiceImpl::RemoveObserver(Observer* observer) {
 void SharedWorkerServiceImpl::EnumerateSharedWorkers(Observer* observer) {
   for (const auto& host : worker_hosts_) {
     if (host->started()) {
-      observer->OnWorkerStarted(host->instance(),
-                                host->GetProcessHost()->GetID(),
+      observer->OnWorkerStarted(host->id(), host->GetProcessHost()->GetID(),
                                 host->dev_tools_token());
     }
   }
@@ -196,12 +195,11 @@ void SharedWorkerServiceImpl::ConnectToWorker(
       /*can_be_default=*/true, &storage_domain, &partition_name, &in_memory);
 
   SharedWorkerInstance instance(
-      shared_worker_id_generator_.GenerateNextId(), info->url,
-      info->options->type, info->options->credentials, info->options->name,
-      constructor_origin, info->content_security_policy,
+      info->url, info->options->type, info->options->credentials,
+      info->options->name, constructor_origin, info->content_security_policy,
       info->content_security_policy_type, info->creation_address_space,
       creation_context_type);
-  host = CreateWorker(instance,
+  host = CreateWorker(shared_worker_id_generator_.GenerateNextId(), instance,
                       std::move(info->outside_fetch_client_settings_object),
                       client_render_frame_host_id, storage_domain, message_port,
                       std::move(blob_url_loader_factory));
@@ -216,24 +214,26 @@ void SharedWorkerServiceImpl::DestroyHost(SharedWorkerHost* host) {
 }
 
 void SharedWorkerServiceImpl::NotifyWorkerStarted(
-    const SharedWorkerInstance& instance,
+    SharedWorkerId shared_worker_id,
     int worker_process_id,
     const base::UnguessableToken& dev_tools_token) {
-  for (Observer& observer : observers_)
-    observer.OnWorkerStarted(instance, worker_process_id, dev_tools_token);
+  for (Observer& observer : observers_) {
+    observer.OnWorkerStarted(shared_worker_id, worker_process_id,
+                             dev_tools_token);
+  }
 }
 
 void SharedWorkerServiceImpl::NotifyWorkerTerminating(
-    const SharedWorkerInstance& instance) {
+    SharedWorkerId shared_worker_id) {
   for (Observer& observer : observers_)
-    observer.OnBeforeWorkerTerminated(instance);
+    observer.OnBeforeWorkerTerminated(shared_worker_id);
 }
 
 void SharedWorkerServiceImpl::NotifyClientAdded(
-    const SharedWorkerInstance& instance,
+    SharedWorkerId shared_worker_id,
     GlobalFrameRoutingId client_render_frame_host_id) {
   auto insertion_result = shared_worker_client_counts_.insert(
-      {{instance, client_render_frame_host_id}, 0});
+      {{shared_worker_id, client_render_frame_host_id}, 0});
 
   int& count = insertion_result.first->second;
   ++count;
@@ -242,15 +242,15 @@ void SharedWorkerServiceImpl::NotifyClientAdded(
   // shared worker.
   if (insertion_result.second) {
     for (Observer& observer : observers_)
-      observer.OnClientAdded(instance, client_render_frame_host_id);
+      observer.OnClientAdded(shared_worker_id, client_render_frame_host_id);
   }
 }
 
 void SharedWorkerServiceImpl::NotifyClientRemoved(
-    const SharedWorkerInstance& instance,
+    SharedWorkerId shared_worker_id,
     GlobalFrameRoutingId client_render_frame_host_id) {
   auto it = shared_worker_client_counts_.find(
-      std::make_pair(instance, client_render_frame_host_id));
+      std::make_pair(shared_worker_id, client_render_frame_host_id));
   DCHECK(it != shared_worker_client_counts_.end());
 
   int& count = it->second;
@@ -262,11 +262,12 @@ void SharedWorkerServiceImpl::NotifyClientRemoved(
   if (count == 0) {
     shared_worker_client_counts_.erase(it);
     for (Observer& observer : observers_)
-      observer.OnClientRemoved(instance, client_render_frame_host_id);
+      observer.OnClientRemoved(shared_worker_id, client_render_frame_host_id);
   }
 }
 
 SharedWorkerHost* SharedWorkerServiceImpl::CreateWorker(
+    SharedWorkerId shared_worker_id,
     const SharedWorkerInstance& instance,
     blink::mojom::FetchClientSettingsObjectPtr
         outside_fetch_client_settings_object,
@@ -286,8 +287,9 @@ SharedWorkerHost* SharedWorkerServiceImpl::CreateWorker(
   // because we are about to bounce to the IO thread. If another ConnectToWorker
   // request arrives in the meantime, it finds and reuses the host instead of
   // creating a new host and therefore new SharedWorker thread.
-  auto insertion_result = worker_hosts_.insert(
-      std::make_unique<SharedWorkerHost>(this, instance, worker_process_host));
+  auto insertion_result =
+      worker_hosts_.insert(std::make_unique<SharedWorkerHost>(
+          this, shared_worker_id, instance, worker_process_host));
   DCHECK(insertion_result.second);
   SharedWorkerHost* host = insertion_result.first->get();
 
@@ -335,8 +337,7 @@ SharedWorkerHost* SharedWorkerServiceImpl::CreateWorker(
       std::move(blob_url_loader_factory), url_loader_factory_override_,
       storage_partition_, storage_domain,
       base::BindOnce(&SharedWorkerServiceImpl::StartWorker,
-                     weak_factory_.GetWeakPtr(), instance, weak_host,
-                     message_port,
+                     weak_factory_.GetWeakPtr(), weak_host, message_port,
                      std::move(cloned_outside_fetch_client_settings_object)));
 
   // Ensures that WorkerScriptFetchInitiator::Start() doesn't synchronously
@@ -347,7 +348,6 @@ SharedWorkerHost* SharedWorkerServiceImpl::CreateWorker(
 }
 
 void SharedWorkerServiceImpl::StartWorker(
-    const SharedWorkerInstance& instance,
     base::WeakPtr<SharedWorkerHost> host,
     const blink::MessagePortChannel& message_port,
     blink::mojom::FetchClientSettingsObjectPtr

content/browser/worker_host/shared_worker_service_impl.h

@@ -74,13 +74,13 @@ class CONTENT_EXPORT SharedWorkerServiceImpl : public SharedWorkerService {
   // Virtual for testing.
   virtual void DestroyHost(SharedWorkerHost* host);
 
-  void NotifyWorkerStarted(const SharedWorkerInstance& instance,
+  void NotifyWorkerStarted(SharedWorkerId shared_worker_id,
                            int worker_process_id,
                            const base::UnguessableToken& dev_tools_token);
-  void NotifyWorkerTerminating(const SharedWorkerInstance& instance);
-  void NotifyClientAdded(const SharedWorkerInstance& instance,
+  void NotifyWorkerTerminating(SharedWorkerId shared_worker_id);
+  void NotifyClientAdded(SharedWorkerId shared_worker_id,
                          GlobalFrameRoutingId render_frame_host_id);
-  void NotifyClientRemoved(const SharedWorkerInstance& instance,
+  void NotifyClientRemoved(SharedWorkerId shared_worker_id,
                            GlobalFrameRoutingId render_frame_host_id);
 
   StoragePartitionImpl* storage_partition() { return storage_partition_; }
@@ -93,6 +93,7 @@ class CONTENT_EXPORT SharedWorkerServiceImpl : public SharedWorkerService {
 
   // Creates a new worker in the creator's renderer process.
   SharedWorkerHost* CreateWorker(
+      SharedWorkerId shared_worker_id,
       const SharedWorkerInstance& instance,
       blink::mojom::FetchClientSettingsObjectPtr
           outside_fetch_client_settings_object,
@@ -102,7 +103,6 @@ class CONTENT_EXPORT SharedWorkerServiceImpl : public SharedWorkerService {
       scoped_refptr<network::SharedURLLoaderFactory> blob_url_loader_factory);
 
   void StartWorker(
-      const SharedWorkerInstance& instance,
       base::WeakPtr<SharedWorkerHost> host,
       const blink::MessagePortChannel& message_port,
       blink::mojom::FetchClientSettingsObjectPtr
@@ -142,7 +142,7 @@ class CONTENT_EXPORT SharedWorkerServiceImpl : public SharedWorkerService {
   // duplicate OnClientAdded() notifications if the same frame connects multiple
   // times to the same shared worker. Note that this is a situation unique to
   // shared worker and cannot happen with dedicated workers and service workers.
-  base::flat_map<std::pair<SharedWorkerInstance, GlobalFrameRoutingId>, int>
+  base::flat_map<std::pair<SharedWorkerId, GlobalFrameRoutingId>, int>
       shared_worker_client_counts_;
 
   base::ObserverList<Observer> observers_;

content/browser/worker_host/shared_worker_service_impl_unittest.cc

@@ -1272,26 +1272,26 @@ class TestSharedWorkerServiceObserver : public SharedWorkerService::Observer {
   ~TestSharedWorkerServiceObserver() override = default;
 
   // SharedWorkerService::Observer:
-  void OnWorkerStarted(const SharedWorkerInstance& instance,
+  void OnWorkerStarted(SharedWorkerId shared_worker_id,
                        int worker_process_id,
                        const base::UnguessableToken& dev_tools_token) override {
-    EXPECT_TRUE(running_workers_.insert({instance, {}}).second);
+    EXPECT_TRUE(running_workers_.insert({shared_worker_id, {}}).second);
   }
-  void OnBeforeWorkerTerminated(const SharedWorkerInstance& instance) override {
-    EXPECT_EQ(1u, running_workers_.erase(instance));
+  void OnBeforeWorkerTerminated(SharedWorkerId shared_worker_id) override {
+    EXPECT_EQ(1u, running_workers_.erase(shared_worker_id));
   }
   void OnClientAdded(
-      const SharedWorkerInstance& instance,
+      SharedWorkerId shared_worker_id,
       GlobalFrameRoutingId client_render_frame_host_id) override {
-    auto it = running_workers_.find(instance);
+    auto it = running_workers_.find(shared_worker_id);
     EXPECT_TRUE(it != running_workers_.end());
     std::set<GlobalFrameRoutingId>& clients = it->second;
     EXPECT_TRUE(clients.insert(client_render_frame_host_id).second);
   }
   void OnClientRemoved(
-      const SharedWorkerInstance& instance,
+      SharedWorkerId shared_worker_id,
       GlobalFrameRoutingId client_render_frame_host_id) override {
-    auto it = running_workers_.find(instance);
+    auto it = running_workers_.find(shared_worker_id);
     EXPECT_TRUE(it != running_workers_.end());
     std::set<GlobalFrameRoutingId>& clients = it->second;
     EXPECT_EQ(1u, clients.erase(client_render_frame_host_id));
@@ -1307,7 +1307,7 @@ class TestSharedWorkerServiceObserver : public SharedWorkerService::Observer {
   }
 
  private:
-  base::flat_map<SharedWorkerInstance, std::set<GlobalFrameRoutingId>>
+  base::flat_map<SharedWorkerId, std::set<GlobalFrameRoutingId>>
       running_workers_;
 
   DISALLOW_COPY_AND_ASSIGN(TestSharedWorkerServiceObserver);

content/public/browser/shared_worker_instance.cc

@@ -4,8 +4,6 @@
 
 #include "content/public/browser/shared_worker_instance.h"
 
-#include <tuple>
-
 #include "base/logging.h"
 #include "content/public/browser/content_browser_client.h"
 #include "content/public/common/content_client.h"
@@ -13,7 +11,6 @@
 namespace content {
 
 SharedWorkerInstance::SharedWorkerInstance(
-    SharedWorkerId id,
     const GURL& url,
     blink::mojom::ScriptType script_type,
     network::mojom::CredentialsMode credentials_mode,
@@ -23,8 +20,7 @@ SharedWorkerInstance::SharedWorkerInstance(
     network::mojom::ContentSecurityPolicyType security_policy_type,
     network::mojom::IPAddressSpace creation_address_space,
     blink::mojom::SharedWorkerCreationContextType creation_context_type)
-    : id_(id),
-      url_(url),
+    : url_(url),
       script_type_(script_type),
       credentials_mode_(credentials_mode),
       name_(name),
@@ -46,12 +42,6 @@ SharedWorkerInstance::SharedWorkerInstance(const SharedWorkerInstance& other) =
 SharedWorkerInstance::SharedWorkerInstance(SharedWorkerInstance&& other) =
     default;
 
-SharedWorkerInstance& SharedWorkerInstance::operator=(
-    const SharedWorkerInstance& other) = default;
-
-SharedWorkerInstance& SharedWorkerInstance::operator=(
-    SharedWorkerInstance&& other) = default;
-
 SharedWorkerInstance::~SharedWorkerInstance() = default;
 
 bool SharedWorkerInstance::Matches(
@@ -76,9 +66,4 @@ bool SharedWorkerInstance::Matches(
   return true;
 }
 
-bool operator<(const SharedWorkerInstance& lhs,
-               const SharedWorkerInstance& rhs) {
-  return lhs.id_ < rhs.id_;
-}
-
 }  // namespace content

content/public/browser/shared_worker_instance.h

@@ -7,7 +7,6 @@
 
 #include <string>
 
-#include "base/util/type_safety/id_type.h"
 #include "content/common/content_export.h"
 #include "services/network/public/mojom/content_security_policy.mojom.h"
 #include "services/network/public/mojom/fetch_api.mojom.h"
@@ -19,14 +18,17 @@
 
 namespace content {
 
-using SharedWorkerId = util::IdType64<class SharedWorkerTag>;
-
-// SharedWorkerInstance is the browser-side representation of one instance of a
+// This class hold the necessary information to decide if a shared worker
+// connection request (SharedWorkerConnector::Connect()) matches an existing
 // shared worker.
+//
+// Note: There exist one SharedWorkerInstance per SharedWorkerHost but it's
+// possible to have 2 distinct SharedWorkerHost that have an identical
+// SharedWorkerInstance. An example is if |url_| or |constructor_origin| has a
+// "file:" scheme, which is treated as opaque.
 class CONTENT_EXPORT SharedWorkerInstance {
  public:
   SharedWorkerInstance(
-      SharedWorkerId id,
       const GURL& url,
       blink::mojom::ScriptType script_type,
       network::mojom::CredentialsMode credentials_mode,
@@ -38,8 +40,8 @@ class CONTENT_EXPORT SharedWorkerInstance {
       blink::mojom::SharedWorkerCreationContextType creation_context_type);
   SharedWorkerInstance(const SharedWorkerInstance& other);
   SharedWorkerInstance(SharedWorkerInstance&& other);
-  SharedWorkerInstance& operator=(const SharedWorkerInstance& other);
-  SharedWorkerInstance& operator=(SharedWorkerInstance&& other);
+  SharedWorkerInstance& operator=(const SharedWorkerInstance& other) = delete;
+  SharedWorkerInstance& operator=(SharedWorkerInstance&& other) = delete;
   ~SharedWorkerInstance();
 
   // Checks if this SharedWorkerInstance matches the passed url, name, and
@@ -73,34 +75,23 @@ class CONTENT_EXPORT SharedWorkerInstance {
   }
 
  private:
-  // Compares SharedWorkerInstances using the |id_|.
-  CONTENT_EXPORT friend bool operator<(const SharedWorkerInstance& lhs,
-                                       const SharedWorkerInstance& rhs);
-
-  // An internal ID that is unique within a storage partition. It is needed to
-  // differentiate 2 SharedWorkerInstance that have the same url, name and
-  // constructor origin but actually represent different workers. This is
-  // possible with a file: |url| or |constructor_origin| since they are treated
-  // as opaque in this class.
-  SharedWorkerId id_;
-
-  GURL url_;
-  blink::mojom::ScriptType script_type_;
+  const GURL url_;
+  const blink::mojom::ScriptType script_type_;
 
   // Used for fetching the top-level worker script.
-  network::mojom::CredentialsMode credentials_mode_;
+  const network::mojom::CredentialsMode credentials_mode_;
 
-  std::string name_;
+  const std::string name_;
 
   // The origin of the document that created this shared worker instance. Used
   // for security checks. See Matches() for details.
   // https://html.spec.whatwg.org/multipage/workers.html#concept-sharedworkerglobalscope-constructor-origin
-  url::Origin constructor_origin_;
+  const url::Origin constructor_origin_;
 
-  std::string content_security_policy_;
-  network::mojom::ContentSecurityPolicyType content_security_policy_type_;
-  network::mojom::IPAddressSpace creation_address_space_;
-  blink::mojom::SharedWorkerCreationContextType creation_context_type_;
+  const std::string content_security_policy_;
+  const network::mojom::ContentSecurityPolicyType content_security_policy_type_;
+  const network::mojom::IPAddressSpace creation_address_space_;
+  const blink::mojom::SharedWorkerCreationContextType creation_context_type_;
 };
 
 }  // namespace content

content/public/browser/shared_worker_service.h

@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/observer_list_types.h"
+#include "base/util/type_safety/id_type.h"
 #include "content/common/content_export.h"
 #include "content/public/browser/global_routing_id.h"
 
@@ -23,7 +24,7 @@ class Origin;
 
 namespace content {
 
-class SharedWorkerInstance;
+using SharedWorkerId = util::IdType64<class SharedWorkerTag>;
 
 // An interface for managing shared workers. These may be run in a separate
 // process, since multiple renderer processes can be talking to a single shared
@@ -38,20 +39,19 @@ class CONTENT_EXPORT SharedWorkerService {
     // running in the renderer. This differs a bit from the "started" state of
     // the embedded worker.
     virtual void OnWorkerStarted(
-        const SharedWorkerInstance& instance,
+        SharedWorkerId shared_worker_id,
         int worker_process_id,
         const base::UnguessableToken& dev_tools_token) = 0;
-    virtual void OnBeforeWorkerTerminated(
-        const SharedWorkerInstance& instance) = 0;
+    virtual void OnBeforeWorkerTerminated(SharedWorkerId shared_worker_id) = 0;
 
     // Called when a frame starts/stop being a client of a shared worker. It is
     // guaranteed that OnWorkerStarted() is called before receiving these
     // notifications.
     virtual void OnClientAdded(
-        const SharedWorkerInstance& instance,
+        SharedWorkerId shared_worker_id,
         content::GlobalFrameRoutingId render_frame_host_id) = 0;
     virtual void OnClientRemoved(
-        const SharedWorkerInstance& instance,
+        SharedWorkerId shared_worker_id,
         content::GlobalFrameRoutingId render_frame_host_id) = 0;
   };