ServiceWorker: Rename ServiceWorkerContainerHost::provider_host() to service_worker_host()

This is a preparation CL for stopping creating ServiceWorkerProviderHost for
service worker clients.

This CL renames ServiceWorkerContainerHost::provider_host() to
service_worker_host(), and ensures it's used only for service worker execution
contexts (not for service worker clients).

Design doc: https://docs.google.com/document/d/1epWIgelE-7uwxJHrYPKlbwqMRP9in2xLUR6mpiU_afY/edit?usp=sharing

Bug: 931087
Change-Id: I6c625c00f00e96c14623a80ced0afd5ba0e0ca49
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1947510Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Kenichi Ishibashi <bashi@chromium.org>
Commit-Queue: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721375}

content/browser/service_worker/service_worker_container_host.cc

@@ -83,7 +83,7 @@ ServiceWorkerContainerHost::ServiceWorkerContainerHost(
         host_receiver,
     mojo::PendingAssociatedRemote<blink::mojom::ServiceWorkerContainer>
         container_remote,
-    ServiceWorkerProviderHost* provider_host,
+    ServiceWorkerProviderHost* service_worker_host,
     base::WeakPtr<ServiceWorkerContextCore> context)
     : type_(type),
       create_time_(base::TimeTicks::Now()),
@@ -96,20 +96,21 @@ ServiceWorkerContainerHost::ServiceWorkerContainerHost(
                                     frame_tree_node_id)),
       client_uuid_(IsContainerForClient() ? base::GenerateGUID()
                                           : std::string()),
-      provider_host_(provider_host),
+      service_worker_host_(service_worker_host),
       context_(std::move(context)) {
-  DCHECK(provider_host_);
   DCHECK(context_);
 
   DCHECK(host_receiver.is_valid());
   receiver_.Bind(std::move(host_receiver));
 
   if (IsContainerForClient()) {
+    DCHECK(!service_worker_host_);
     DCHECK(container_remote);
     container_.Bind(std::move(container_remote));
     context_->RegisterContainerHostByClientID(client_uuid(), this);
   } else {
     DCHECK(IsContainerForServiceWorker());
+    DCHECK(service_worker_host_);
   }
 }
 
@@ -136,7 +137,7 @@ ServiceWorkerContainerHost::~ServiceWorkerContainerHost() {
       controller_->OnControlleeDestroyed(client_uuid());
   }
 
-  // Remove |provider_host_| as an observer of ServiceWorkerRegistrations.
+  // Remove |this| as an observer of ServiceWorkerRegistrations.
   // TODO(falken): Use ScopedObserver instead of this explicit call.
   controller_.reset();
   controller_registration_.reset();
@@ -944,6 +945,11 @@ ServiceWorkerRegistration* ServiceWorkerContainerHost::controller_registration()
   return controller_registration_.get();
 }
 
+ServiceWorkerProviderHost* ServiceWorkerContainerHost::service_worker_host() {
+  DCHECK(IsContainerForServiceWorker());
+  return service_worker_host_;
+}
+
 bool ServiceWorkerContainerHost::IsInBackForwardCache() const {
   DCHECK_CURRENTLY_ON(ServiceWorkerContext::GetCoreThreadId());
   return is_in_back_forward_cache_;

content/browser/service_worker/service_worker_container_host.h

@@ -48,7 +48,7 @@ struct ServiceWorkerRegistrationInfo;
 // in the browser process, associated with the execution context where the
 // container lives.
 //
-// ServiceWorkerObjectHost is tentatively owned by ServiceWorkerProviderHost,
+// ServiceWorkerContainerHost is tentatively owned by ServiceWorkerProviderHost,
 // and has the same lifetime with that.
 // TODO(https://crbug.com/931087): Make an execution context host (i.e.,
 // RenderFrameHostImpl, DedicatedWorkerHost etc) own this.
@@ -62,6 +62,8 @@ class CONTENT_EXPORT ServiceWorkerContainerHost final
   using ExecutionReadyCallback = base::OnceClosure;
   using WebContentsGetter = base::RepeatingCallback<WebContents*()>;
 
+  // |service_worker_host| must be non-null for service worker execution
+  // contexts, and null for service worker clients.
   // TODO(https://crbug.com/931087): Rename ServiceWorkerProviderType to
   // ServiceWorkerContainerType.
   ServiceWorkerContainerHost(
@@ -72,7 +74,7 @@ class CONTENT_EXPORT ServiceWorkerContainerHost final
           host_receiver,
       mojo::PendingAssociatedRemote<blink::mojom::ServiceWorkerContainer>
           container_remote,
-      ServiceWorkerProviderHost* provider_host,
+      ServiceWorkerProviderHost* service_worker_host,
       base::WeakPtr<ServiceWorkerContextCore> context);
   ~ServiceWorkerContainerHost() override;
 
@@ -343,9 +345,6 @@ class CONTENT_EXPORT ServiceWorkerContainerHost final
   // https://html.spec.whatwg.org/multipage/webappapis.html#concept-environment-execution-ready-flag
   bool is_execution_ready() const;
 
-  // TODO(https://crbug.com/931087): Remove this getter and |provider_host_|.
-  ServiceWorkerProviderHost* provider_host() { return provider_host_; }
-
   // For service worker clients. The flow is kInitial -> kResponseCommitted ->
   // kExecutionReady.
   //
@@ -403,6 +402,9 @@ class CONTENT_EXPORT ServiceWorkerContainerHost final
   // registration.
   ServiceWorkerRegistration* controller_registration() const;
 
+  // For service worker execution contexts.
+  ServiceWorkerProviderHost* service_worker_host();
+
   // BackForwardCache:
   // For service worker clients that are windows.
   bool IsInBackForwardCache() const;
@@ -653,9 +655,8 @@ class CONTENT_EXPORT ServiceWorkerContainerHost final
   // is hosting.
   mojo::AssociatedRemote<blink::mojom::ServiceWorkerContainer> container_;
 
-  // This provider host owns |this|.
-  // TODO(https://crbug.com/931087): Remove dependencies on |provider_host_|.
-  ServiceWorkerProviderHost* provider_host_ = nullptr;
+  // For service worker execution contexts. This provider host owns |this|.
+  ServiceWorkerProviderHost* service_worker_host_ = nullptr;
 
   base::WeakPtr<ServiceWorkerContextCore> context_;
 

content/browser/service_worker/service_worker_controllee_request_handler_unittest.cc

@@ -118,12 +118,10 @@ class ServiceWorkerControlleeRequestHandlerTest : public testing::Test {
 
     // An empty host.
     remote_endpoints_.emplace_back();
-    container_host_ =
-        CreateProviderHostForWindow(
-            helper_->mock_render_process_id(), is_parent_frame_secure,
-            helper_->context()->AsWeakPtr(), &remote_endpoints_.back())
-            ->container_host()
-            ->GetWeakPtr();
+    provider_host_ = CreateProviderHostForWindow(
+        helper_->mock_render_process_id(), is_parent_frame_secure,
+        helper_->context()->AsWeakPtr(), &remote_endpoints_.back());
+    container_host_ = provider_host_->container_host()->GetWeakPtr();
   }
 
   void TearDown() override {
@@ -139,6 +137,7 @@ class ServiceWorkerControlleeRequestHandlerTest : public testing::Test {
   std::unique_ptr<EmbeddedWorkerTestHelper> helper_;
   scoped_refptr<ServiceWorkerRegistration> registration_;
   scoped_refptr<ServiceWorkerVersion> version_;
+  base::WeakPtr<ServiceWorkerProviderHost> provider_host_;
   base::WeakPtr<ServiceWorkerContainerHost> container_host_;
   net::URLRequestContext url_request_context_;
   net::TestDelegate url_request_delegate_;
@@ -381,8 +380,7 @@ TEST_F(ServiceWorkerControlleeRequestHandlerTest, DeletedProviderHost) {
 
   // Shouldn't crash if the ProviderHost is deleted prior to completion of
   // the database lookup.
-  context()->RemoveProviderHost(
-      container_host_->provider_host()->provider_id());
+  context()->RemoveProviderHost(provider_host_->provider_id());
   EXPECT_FALSE(container_host_.get());
   base::RunLoop().RunUntilIdle();
   EXPECT_FALSE(test_resources.loader());

content/browser/service_worker/service_worker_object_host.cc

@@ -118,22 +118,21 @@ bool PrepareExtendableMessageEventFromClient(
 // details.
 bool PrepareExtendableMessageEventFromServiceWorker(
     scoped_refptr<ServiceWorkerVersion> worker,
-    base::WeakPtr<ServiceWorkerProviderHost>
-        source_service_worker_provider_host,
+    base::WeakPtr<ServiceWorkerContainerHost> source_container_host,
     blink::mojom::ExtendableMessageEventPtr* event) {
   // The service worker execution context may have been destroyed by the time we
   // get here.
-  if (!source_service_worker_provider_host)
+  if (!source_container_host)
     return false;
 
-  DCHECK_EQ(blink::mojom::ServiceWorkerProviderType::kForServiceWorker,
-            source_service_worker_provider_host->provider_type());
+  DCHECK(source_container_host->IsContainerForServiceWorker());
   blink::mojom::ServiceWorkerObjectInfoPtr source_worker_info;
   base::WeakPtr<ServiceWorkerObjectHost> service_worker_object_host =
       worker->provider_host()
           ->container_host()
           ->GetOrCreateServiceWorkerObjectHost(
-              source_service_worker_provider_host->running_hosted_version());
+              source_container_host->service_worker_host()
+                  ->running_hosted_version());
   if (service_worker_object_host) {
     // CreateCompleteObjectInfoToSend() is safe because |source_worker_info|
     // will be sent immediately by the caller of this function.
@@ -179,19 +178,17 @@ void DispatchExtendableMessageEventFromServiceWorker(
     const url::Origin& source_origin,
     const base::Optional<base::TimeDelta>& timeout,
     StatusCallback callback,
-    base::WeakPtr<ServiceWorkerProviderHost>
-        source_service_worker_provider_host) {
-  if (!source_service_worker_provider_host) {
+    base::WeakPtr<ServiceWorkerContainerHost> source_container_host) {
+  if (!source_container_host) {
     std::move(callback).Run(blink::ServiceWorkerStatusCode::kErrorFailed);
     return;
   }
 
-  DCHECK_EQ(blink::mojom::ServiceWorkerProviderType::kForServiceWorker,
-            source_service_worker_provider_host->provider_type());
+  DCHECK(source_container_host->IsContainerForServiceWorker());
   StartWorkerToDispatchExtendableMessageEvent(
       worker, std::move(message), source_origin, timeout, std::move(callback),
       base::BindOnce(&PrepareExtendableMessageEventFromServiceWorker, worker,
-                     source_service_worker_provider_host));
+                     std::move(source_container_host)));
 }
 
 }  // namespace
@@ -288,7 +285,7 @@ void ServiceWorkerObjectHost::DispatchExtendableMessageEvent(
     return;
   }
   DCHECK_EQ(container_origin_, url::Origin::Create(container_host_->url()));
-  switch (container_host_->provider_host()->provider_type()) {
+  switch (container_host_->type()) {
     case blink::mojom::ServiceWorkerProviderType::kForWindow:
       service_worker_client_utils::GetClient(
           container_host_,
@@ -299,7 +296,7 @@ void ServiceWorkerObjectHost::DispatchExtendableMessageEvent(
     case blink::mojom::ServiceWorkerProviderType::kForServiceWorker: {
       // Clamp timeout to the sending worker's remaining timeout, to prevent
       // postMessage from keeping workers alive forever.
-      base::TimeDelta timeout = container_host_->provider_host()
+      base::TimeDelta timeout = container_host_->service_worker_host()
                                     ->running_hosted_version()
                                     ->remaining_timeout();
 
@@ -308,7 +305,7 @@ void ServiceWorkerObjectHost::DispatchExtendableMessageEvent(
           base::BindOnce(&DispatchExtendableMessageEventFromServiceWorker,
                          version_, std::move(message), container_origin_,
                          base::make_optional(timeout), std::move(callback),
-                         container_host_->provider_host()->AsWeakPtr()));
+                         container_host_->GetWeakPtr()));
       return;
     }
     case blink::mojom::ServiceWorkerProviderType::kForDedicatedWorker:
@@ -318,7 +315,7 @@ void ServiceWorkerObjectHost::DispatchExtendableMessageEvent(
     case blink::mojom::ServiceWorkerProviderType::kUnknown:
       break;
   }
-  NOTREACHED() << container_host_->provider_host()->provider_type();
+  NOTREACHED() << container_host_->type();
 }
 
 void ServiceWorkerObjectHost::OnConnectionError() {

content/browser/service_worker/service_worker_provider_host.cc

@@ -148,7 +148,9 @@ ServiceWorkerProviderHost::ServiceWorkerProviderHost(
           frame_tree_node_id,
           std::move(host_receiver),
           std::move(container_remote),
-          this,
+          type == blink::mojom::ServiceWorkerProviderType::kForServiceWorker
+              ? this
+              : nullptr,
           context)) {
   DCHECK_NE(blink::mojom::ServiceWorkerProviderType::kUnknown, type);
   if (type == blink::mojom::ServiceWorkerProviderType::kForServiceWorker) {
@@ -179,11 +181,6 @@ ServiceWorkerVersion* ServiceWorkerProviderHost::running_hosted_version()
   return running_hosted_version_.get();
 }
 
-blink::mojom::ServiceWorkerProviderType
-ServiceWorkerProviderHost::provider_type() const {
-  return container_host_->type();
-}
-
 bool ServiceWorkerProviderHost::IsProviderForServiceWorker() const {
   return container_host_->IsContainerForServiceWorker();
 }

content/browser/service_worker/service_worker_provider_host.h

@@ -158,9 +158,8 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
   // CompleteStartWorkerPreparation() is called).
   ServiceWorkerVersion* running_hosted_version() const;
 
-  // TODO(https://crbug.com/931087): Remove these functions in favor of the
-  // equivalent functions on ServiceWorkerContainerHost.
-  blink::mojom::ServiceWorkerProviderType provider_type() const;
+  // TODO(https://crbug.com/931087): Remove this function in favor of the
+  // ServiceWorkerContainerHost::IsContainerForServiceWorker().
   bool IsProviderForServiceWorker() const;
 
   // For service worker execution contexts. Completes initialization of this

content/browser/service_worker/service_worker_registration_object_host.cc

@@ -185,9 +185,9 @@ void ServiceWorkerRegistrationObjectHost::Update(
   // globalObject is a ServiceWorkerGlobalScope object, and globalObject’s
   // associated service worker's state is installing, return a promise rejected
   // with an "InvalidStateError" DOMException and abort these steps.
-  if (container_host_->provider_host()->IsProviderForServiceWorker()) {
-    ServiceWorkerVersion* version =
-        container_host_->provider_host()->running_hosted_version();
+  ServiceWorkerVersion* version = nullptr;
+  if (container_host_->IsContainerForServiceWorker()) {
+    version = container_host_->service_worker_host()->running_hosted_version();
     DCHECK(version);
     if (ServiceWorkerVersion::Status::INSTALLING == version->status()) {
       // This can happen if update() is called during execution of the
@@ -201,8 +201,7 @@ void ServiceWorkerRegistrationObjectHost::Update(
   }
 
   DelayUpdate(
-      container_host_->provider_host()->provider_type(), registration,
-      container_host_->provider_host()->running_hosted_version(),
+      container_host_->type(), registration, version,
       base::BindOnce(
           &ExecuteUpdate, context_, registration->id(),
           false /* force_bypass_cache */, false /* skip_script_comparison */,