Commit ac6884f4 authored by Han Leon's avatar Han Leon Committed by Commit Bot

[ServiceWorker] Manage ServiceWorkerHandles per ServiceWorkerProviderHost

As we have already mojofied all legacy IPCs handled by
ServiceWorkerHandle, now we no longer need ServiceWorkerDispatcherHost
to help routing, the renderer process uses the ServiceWorkerObjectHost
Mojo connection to talk with ServiceWorkerHandle directly.

Then, this CL moves ServiceWorkerHandles management out from
ServiceWorkerDispatcherHost into ServiceWorkerProviderHost, which
manages ServiceWorkerHandles keyed by the corresponding service worker's
version id. The key logic itself is equivalent with before:
ServiceWorkerDispatcherHost managed them keyed by the handle id, which
is actually an unique identifier of the (provider_id, version_id) pair.

This makes ServiceWorkerDispatcherHost slimmer.

BUG=772713

Change-Id: I4131a8a687f05a5fe8e297aaa4d83f74028bb5cf
Reviewed-on: https://chromium-review.googlesource.com/964243
Commit-Queue: Han Leon <leon.han@intel.com>
Reviewed-by: default avatarKinuko Yasuda <kinuko@chromium.org>
Reviewed-by: default avatarMatt Falkenhagen <falken@chromium.org>
Reviewed-by: default avatarMakoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#544305}
parent d4c38230
...@@ -137,33 +137,6 @@ bool ServiceWorkerDispatcherHost::Send(IPC::Message* message) { ...@@ -137,33 +137,6 @@ bool ServiceWorkerDispatcherHost::Send(IPC::Message* message) {
return true; return true;
} }
void ServiceWorkerDispatcherHost::RegisterServiceWorkerHandle(
std::unique_ptr<ServiceWorkerHandle> handle) {
int handle_id = handle->handle_id();
handles_.AddWithID(std::move(handle), handle_id);
}
ServiceWorkerHandle* ServiceWorkerDispatcherHost::FindServiceWorkerHandle(
int provider_id,
int64_t version_id) {
for (base::IDMap<std::unique_ptr<ServiceWorkerHandle>>::iterator iter(
&handles_);
!iter.IsAtEnd(); iter.Advance()) {
ServiceWorkerHandle* handle = iter.GetCurrentValue();
DCHECK(handle);
DCHECK(handle->version());
if (handle->provider_id() == provider_id &&
handle->version()->version_id() == version_id) {
return handle;
}
}
return nullptr;
}
void ServiceWorkerDispatcherHost::UnregisterServiceWorkerHandle(int handle_id) {
handles_.Remove(handle_id);
}
base::WeakPtr<ServiceWorkerDispatcherHost> base::WeakPtr<ServiceWorkerDispatcherHost>
ServiceWorkerDispatcherHost::AsWeakPtr() { ServiceWorkerDispatcherHost::AsWeakPtr() {
return weak_ptr_factory_.GetWeakPtr(); return weak_ptr_factory_.GetWeakPtr();
......
...@@ -10,7 +10,6 @@ ...@@ -10,7 +10,6 @@
#include <memory> #include <memory>
#include <vector> #include <vector>
#include "base/containers/id_map.h"
#include "base/macros.h" #include "base/macros.h"
#include "base/memory/weak_ptr.h" #include "base/memory/weak_ptr.h"
#include "base/optional.h" #include "base/optional.h"
...@@ -29,7 +28,6 @@ namespace content { ...@@ -29,7 +28,6 @@ namespace content {
class ResourceContext; class ResourceContext;
class ServiceWorkerContextCore; class ServiceWorkerContextCore;
class ServiceWorkerContextWrapper; class ServiceWorkerContextWrapper;
class ServiceWorkerHandle;
namespace service_worker_dispatcher_host_unittest { namespace service_worker_dispatcher_host_unittest {
class ServiceWorkerDispatcherHostTest; class ServiceWorkerDispatcherHostTest;
...@@ -85,14 +83,6 @@ class CONTENT_EXPORT ServiceWorkerDispatcherHost ...@@ -85,14 +83,6 @@ class CONTENT_EXPORT ServiceWorkerDispatcherHost
// be destroyed. // be destroyed.
bool Send(IPC::Message* message) override; bool Send(IPC::Message* message) override;
// These methods are virtual only for testing.
virtual void RegisterServiceWorkerHandle(
std::unique_ptr<ServiceWorkerHandle> handle);
virtual void UnregisterServiceWorkerHandle(int handle_id);
ServiceWorkerHandle* FindServiceWorkerHandle(int provider_id,
int64_t version_id);
ResourceContext* resource_context() { return resource_context_; } ResourceContext* resource_context() { return resource_context_; }
base::WeakPtr<ServiceWorkerDispatcherHost> AsWeakPtr(); base::WeakPtr<ServiceWorkerDispatcherHost> AsWeakPtr();
...@@ -133,8 +123,6 @@ class CONTENT_EXPORT ServiceWorkerDispatcherHost ...@@ -133,8 +123,6 @@ class CONTENT_EXPORT ServiceWorkerDispatcherHost
// Only accessed on the IO thread. // Only accessed on the IO thread.
scoped_refptr<ServiceWorkerContextWrapper> context_wrapper_; scoped_refptr<ServiceWorkerContextWrapper> context_wrapper_;
base::IDMap<std::unique_ptr<ServiceWorkerHandle>> handles_;
bool channel_ready_; // True after BrowserMessageFilter::sender_ != NULL. bool channel_ready_; // True after BrowserMessageFilter::sender_ != NULL.
std::vector<std::unique_ptr<IPC::Message>> pending_messages_; std::vector<std::unique_ptr<IPC::Message>> pending_messages_;
......
...@@ -7,7 +7,6 @@ ...@@ -7,7 +7,6 @@
#include "base/memory/ptr_util.h" #include "base/memory/ptr_util.h"
#include "content/browser/service_worker/service_worker_client_utils.h" #include "content/browser/service_worker/service_worker_client_utils.h"
#include "content/browser/service_worker/service_worker_context_core.h" #include "content/browser/service_worker/service_worker_context_core.h"
#include "content/browser/service_worker/service_worker_dispatcher_host.h"
#include "content/browser/service_worker/service_worker_provider_host.h" #include "content/browser/service_worker/service_worker_provider_host.h"
#include "content/browser/service_worker/service_worker_registration.h" #include "content/browser/service_worker/service_worker_registration.h"
#include "content/browser/service_worker/service_worker_type_converters.h" #include "content/browser/service_worker/service_worker_type_converters.h"
...@@ -153,40 +152,22 @@ void DispatchExtendableMessageEventFromServiceWorker( ...@@ -153,40 +152,22 @@ void DispatchExtendableMessageEventFromServiceWorker(
} // namespace } // namespace
// static
base::WeakPtr<ServiceWorkerHandle> ServiceWorkerHandle::Create(
ServiceWorkerDispatcherHost* dispatcher_host,
base::WeakPtr<ServiceWorkerContextCore> context,
base::WeakPtr<ServiceWorkerProviderHost> provider_host,
ServiceWorkerVersion* version,
blink::mojom::ServiceWorkerObjectInfoPtr* out_info) {
DCHECK(context && provider_host && version && out_info);
ServiceWorkerHandle* handle =
new ServiceWorkerHandle(dispatcher_host, context, provider_host, version);
*out_info = handle->CreateObjectInfo();
return handle->AsWeakPtr();
}
ServiceWorkerHandle::ServiceWorkerHandle( ServiceWorkerHandle::ServiceWorkerHandle(
ServiceWorkerDispatcherHost* dispatcher_host,
base::WeakPtr<ServiceWorkerContextCore> context, base::WeakPtr<ServiceWorkerContextCore> context,
base::WeakPtr<ServiceWorkerProviderHost> provider_host, ServiceWorkerProviderHost* provider_host,
ServiceWorkerVersion* version) scoped_refptr<ServiceWorkerVersion> version)
: dispatcher_host_(dispatcher_host), : context_(context),
context_(context),
provider_host_(provider_host), provider_host_(provider_host),
provider_origin_(url::Origin::Create(provider_host->document_url())), provider_origin_(url::Origin::Create(provider_host->document_url())),
provider_id_(provider_host->provider_id()), provider_id_(provider_host->provider_id()),
handle_id_(context->GetNewServiceWorkerHandleId()), handle_id_(context->GetNewServiceWorkerHandleId()),
version_(version), version_(std::move(version)),
weak_ptr_factory_(this) { weak_ptr_factory_(this) {
DCHECK(context_ && provider_host_ && version_); DCHECK(context_ && provider_host_ && version_);
DCHECK(context_->GetLiveRegistration(version_->registration_id())); DCHECK(context_->GetLiveRegistration(version_->registration_id()));
version_->AddListener(this); version_->AddListener(this);
bindings_.set_connection_error_handler(base::BindRepeating( bindings_.set_connection_error_handler(base::BindRepeating(
&ServiceWorkerHandle::OnConnectionError, base::Unretained(this))); &ServiceWorkerHandle::OnConnectionError, base::Unretained(this)));
if (dispatcher_host_)
dispatcher_host_->RegisterServiceWorkerHandle(base::WrapUnique(this));
} }
ServiceWorkerHandle::~ServiceWorkerHandle() { ServiceWorkerHandle::~ServiceWorkerHandle() {
...@@ -195,8 +176,6 @@ ServiceWorkerHandle::~ServiceWorkerHandle() { ...@@ -195,8 +176,6 @@ ServiceWorkerHandle::~ServiceWorkerHandle() {
void ServiceWorkerHandle::OnVersionStateChanged(ServiceWorkerVersion* version) { void ServiceWorkerHandle::OnVersionStateChanged(ServiceWorkerVersion* version) {
DCHECK(version); DCHECK(version);
if (!provider_host_)
return;
provider_host_->SendServiceWorkerStateChangedMessage( provider_host_->SendServiceWorkerStateChangedMessage(
handle_id_, handle_id_,
mojo::ConvertTo<blink::mojom::ServiceWorkerState>(version->status())); mojo::ConvertTo<blink::mojom::ServiceWorkerState>(version->status()));
...@@ -214,15 +193,6 @@ ServiceWorkerHandle::CreateObjectInfo() { ...@@ -214,15 +193,6 @@ ServiceWorkerHandle::CreateObjectInfo() {
return info; return info;
} }
void ServiceWorkerHandle::RegisterIntoDispatcherHost(
ServiceWorkerDispatcherHost* dispatcher_host) {
DCHECK(ServiceWorkerUtils::IsServicificationEnabled() ||
IsNavigationMojoResponseEnabled());
DCHECK(!dispatcher_host_);
dispatcher_host_ = dispatcher_host;
dispatcher_host_->RegisterServiceWorkerHandle(base::WrapUnique(this));
}
void ServiceWorkerHandle::PostMessageToServiceWorker( void ServiceWorkerHandle::PostMessageToServiceWorker(
::blink::TransferableMessage message) { ::blink::TransferableMessage message) {
// When this method is called the encoded_message inside message could just // When this method is called the encoded_message inside message could just
...@@ -242,8 +212,8 @@ void ServiceWorkerHandle::TerminateForTesting( ...@@ -242,8 +212,8 @@ void ServiceWorkerHandle::TerminateForTesting(
void ServiceWorkerHandle::DispatchExtendableMessageEvent( void ServiceWorkerHandle::DispatchExtendableMessageEvent(
::blink::TransferableMessage message, ::blink::TransferableMessage message,
StatusCallback callback) { StatusCallback callback) {
if (!context_ || !provider_host_) { if (!context_) {
std::move(callback).Run(SERVICE_WORKER_ERROR_FAILED); std::move(callback).Run(SERVICE_WORKER_ERROR_ABORT);
return; return;
} }
DCHECK_EQ(provider_origin_, DCHECK_EQ(provider_origin_,
...@@ -251,7 +221,7 @@ void ServiceWorkerHandle::DispatchExtendableMessageEvent( ...@@ -251,7 +221,7 @@ void ServiceWorkerHandle::DispatchExtendableMessageEvent(
switch (provider_host_->provider_type()) { switch (provider_host_->provider_type()) {
case blink::mojom::ServiceWorkerProviderType::kForWindow: case blink::mojom::ServiceWorkerProviderType::kForWindow:
service_worker_client_utils::GetClient( service_worker_client_utils::GetClient(
provider_host_.get(), provider_host_,
base::BindOnce(&DispatchExtendableMessageEventFromClient, version_, base::BindOnce(&DispatchExtendableMessageEventFromClient, version_,
std::move(message), provider_origin_, std::move(message), provider_origin_,
std::move(callback))); std::move(callback)));
...@@ -267,7 +237,7 @@ void ServiceWorkerHandle::DispatchExtendableMessageEvent( ...@@ -267,7 +237,7 @@ void ServiceWorkerHandle::DispatchExtendableMessageEvent(
base::BindOnce(&DispatchExtendableMessageEventFromServiceWorker, base::BindOnce(&DispatchExtendableMessageEventFromServiceWorker,
version_, std::move(message), provider_origin_, version_, std::move(message), provider_origin_,
base::make_optional(timeout), std::move(callback), base::make_optional(timeout), std::move(callback),
provider_host_)); provider_host_->AsWeakPtr()));
return; return;
} }
case blink::mojom::ServiceWorkerProviderType::kForSharedWorker: case blink::mojom::ServiceWorkerProviderType::kForSharedWorker:
...@@ -287,16 +257,8 @@ void ServiceWorkerHandle::OnConnectionError() { ...@@ -287,16 +257,8 @@ void ServiceWorkerHandle::OnConnectionError() {
// If there are still bindings, |this| is still being used. // If there are still bindings, |this| is still being used.
if (!bindings_.empty()) if (!bindings_.empty())
return; return;
// S13nServiceWorker: This handle may have been precreated before registering
// to a dispatcher host. Just self-destruct since we're no longer needed.
if (!dispatcher_host_) {
DCHECK(ServiceWorkerUtils::IsServicificationEnabled() ||
IsNavigationMojoResponseEnabled());
delete this;
return;
}
// Will destroy |this|. // Will destroy |this|.
dispatcher_host_->UnregisterServiceWorkerHandle(handle_id_); provider_host_->RemoveServiceWorkerHandle(version_->version_id());
} }
} // namespace content } // namespace content
...@@ -20,7 +20,6 @@ ...@@ -20,7 +20,6 @@
namespace content { namespace content {
class ServiceWorkerContextCore; class ServiceWorkerContextCore;
class ServiceWorkerDispatcherHost;
class ServiceWorkerProviderHost; class ServiceWorkerProviderHost;
namespace service_worker_handle_unittest { namespace service_worker_handle_unittest {
...@@ -39,22 +38,9 @@ class CONTENT_EXPORT ServiceWorkerHandle ...@@ -39,22 +38,9 @@ class CONTENT_EXPORT ServiceWorkerHandle
: public blink::mojom::ServiceWorkerObjectHost, : public blink::mojom::ServiceWorkerObjectHost,
public ServiceWorkerVersion::Listener { public ServiceWorkerVersion::Listener {
public: public:
// Creates a newly created instance for a live version. |out_info| holds the ServiceWorkerHandle(base::WeakPtr<ServiceWorkerContextCore> context,
// first ServiceWorkerObjectHost Mojo connection to this instance, which will ServiceWorkerProviderHost* provider_host,
// delete itself once it detects that all the Mojo connections have gone scoped_refptr<ServiceWorkerVersion> version);
// away.
//
// This instance registers itself into |dispatcher_host| to be owned by the
// dispatcher host. S13nServiceWorker: |dispatcher_host| may be null.
// RegisterIntoDispatcherHost() should be called later to register the handle
// once the host is known.
static base::WeakPtr<ServiceWorkerHandle> Create(
ServiceWorkerDispatcherHost* dispatcher_host,
base::WeakPtr<ServiceWorkerContextCore> context,
base::WeakPtr<ServiceWorkerProviderHost> provider_host,
ServiceWorkerVersion* version,
blink::mojom::ServiceWorkerObjectInfoPtr* out_info);
~ServiceWorkerHandle() override; ~ServiceWorkerHandle() override;
// ServiceWorkerVersion::Listener overrides. // ServiceWorkerVersion::Listener overrides.
...@@ -63,10 +49,6 @@ class CONTENT_EXPORT ServiceWorkerHandle ...@@ -63,10 +49,6 @@ class CONTENT_EXPORT ServiceWorkerHandle
// Establishes a new mojo connection into |bindings_|. // Establishes a new mojo connection into |bindings_|.
blink::mojom::ServiceWorkerObjectInfoPtr CreateObjectInfo(); blink::mojom::ServiceWorkerObjectInfoPtr CreateObjectInfo();
// Should only be called on a ServiceWorkerHandle instance constructed with
// null |dispatcher_host| before.
void RegisterIntoDispatcherHost(ServiceWorkerDispatcherHost* dispatcher_host);
int provider_id() const { return provider_id_; } int provider_id() const { return provider_id_; }
int handle_id() const { return handle_id_; } int handle_id() const { return handle_id_; }
ServiceWorkerVersion* version() { return version_.get(); } ServiceWorkerVersion* version() { return version_.get(); }
...@@ -74,11 +56,6 @@ class CONTENT_EXPORT ServiceWorkerHandle ...@@ -74,11 +56,6 @@ class CONTENT_EXPORT ServiceWorkerHandle
private: private:
friend class service_worker_handle_unittest::ServiceWorkerHandleTest; friend class service_worker_handle_unittest::ServiceWorkerHandleTest;
ServiceWorkerHandle(ServiceWorkerDispatcherHost* dispatcher_host,
base::WeakPtr<ServiceWorkerContextCore> context,
base::WeakPtr<ServiceWorkerProviderHost> provider_host,
ServiceWorkerVersion* version);
// Implements blink::mojom::ServiceWorkerObjectHost. // Implements blink::mojom::ServiceWorkerObjectHost.
void PostMessageToServiceWorker( void PostMessageToServiceWorker(
::blink::TransferableMessage message) override; ::blink::TransferableMessage message) override;
...@@ -95,13 +72,10 @@ class CONTENT_EXPORT ServiceWorkerHandle ...@@ -95,13 +72,10 @@ class CONTENT_EXPORT ServiceWorkerHandle
void OnConnectionError(); void OnConnectionError();
// |dispatcher_host_| may get a valid value via ctor or
// RegisterIntoDispatcherHost() function, after that |dispatcher_host_| starts
// to own |this|, then, |dispatcher_host_| is valid throughout the lifetime of
// |this|.
ServiceWorkerDispatcherHost* dispatcher_host_;
base::WeakPtr<ServiceWorkerContextCore> context_; base::WeakPtr<ServiceWorkerContextCore> context_;
base::WeakPtr<ServiceWorkerProviderHost> provider_host_; // |provider_host_| is valid throughout lifetime of |this| because it owns
// |this|.
ServiceWorkerProviderHost* provider_host_;
// The origin of the |provider_host_|. Note that this is const because once a // The origin of the |provider_host_|. Note that this is const because once a
// JavaScript ServiceWorker object is created for an execution context, we // JavaScript ServiceWorker object is created for an execution context, we
// don't expect that context to change origins and still hold on to the // don't expect that context to change origins and still hold on to the
......
...@@ -192,6 +192,15 @@ class ServiceWorkerHandleTest : public testing::Test { ...@@ -192,6 +192,15 @@ class ServiceWorkerHandleTest : public testing::Test {
return handle->bindings_.size(); return handle->bindings_.size();
} }
ServiceWorkerHandle* GetServiceWorkerHandle(
ServiceWorkerProviderHost* provider_host,
int64_t version_id) {
auto iter = provider_host->handles_.find(version_id);
if (iter != provider_host->handles_.end())
return iter->second.get();
return nullptr;
}
IPC::TestSink* ipc_sink() { return helper_->ipc_sink(); } IPC::TestSink* ipc_sink() { return helper_->ipc_sink(); }
TestBrowserThreadBundle browser_thread_bundle_; TestBrowserThreadBundle browser_thread_bundle_;
...@@ -221,12 +230,10 @@ TEST_F(ServiceWorkerHandleTest, OnVersionStateChanged) { ...@@ -221,12 +230,10 @@ TEST_F(ServiceWorkerHandleTest, OnVersionStateChanged) {
helper_->mock_render_process_id(), kProviderId, helper_->mock_render_process_id(), kProviderId,
helper_->context()->AsWeakPtr(), kRenderFrameId, helper_->context()->AsWeakPtr(), kRenderFrameId,
dispatcher_host_.get(), &remote_endpoint); dispatcher_host_.get(), &remote_endpoint);
blink::mojom::ServiceWorkerObjectInfoPtr info; blink::mojom::ServiceWorkerObjectInfoPtr info =
// ServiceWorkerHandle lifetime is controlled by |info| and is also owned by provider_host->GetOrCreateServiceWorkerHandle(version_.get());
// |dispatcher_host_|. ServiceWorkerHandle* handle =
base::WeakPtr<ServiceWorkerHandle> handle = ServiceWorkerHandle::Create( GetServiceWorkerHandle(provider_host.get(), version_->version_id());
dispatcher_host_.get(), helper_->context()->AsWeakPtr(),
provider_host->AsWeakPtr(), version_.get(), &info);
// Start the worker, and then... // Start the worker, and then...
ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_FAILED; ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_FAILED;
...@@ -281,16 +288,12 @@ TEST_F(ServiceWorkerHandleTest, ...@@ -281,16 +288,12 @@ TEST_F(ServiceWorkerHandleTest,
// Prepare a ServiceWorkerHandle corresponding to a JavaScript ServiceWorker // Prepare a ServiceWorkerHandle corresponding to a JavaScript ServiceWorker
// object in the service worker execution context for |version_|. // object in the service worker execution context for |version_|.
EXPECT_FALSE(dispatcher_host_->FindServiceWorkerHandle( ServiceWorkerProviderHost* provider_host = version_->provider_host();
version_->provider_host()->provider_id(), version_->version_id())); blink::mojom::ServiceWorkerObjectInfoPtr info =
blink::mojom::ServiceWorkerObjectInfoPtr info; provider_host->GetOrCreateServiceWorkerHandle(version_.get());
// ServiceWorkerHandle lifetime is controlled by |info| and is also owned by ServiceWorkerHandle* sender_worker_handle =
// |dispatcher_host_|. GetServiceWorkerHandle(provider_host, version_->version_id());
base::WeakPtr<ServiceWorkerHandle> sender_worker_handle = EXPECT_EQ(1u, GetBindingsCount(sender_worker_handle));
ServiceWorkerHandle::Create(
dispatcher_host_.get(), helper_->context()->AsWeakPtr(),
version_->provider_host()->AsWeakPtr(), version_.get(), &info);
EXPECT_EQ(1u, GetBindingsCount(sender_worker_handle.get()));
// Dispatch an ExtendableMessageEvent simulating calling // Dispatch an ExtendableMessageEvent simulating calling
// ServiceWorker#postMessage() on the ServiceWorker object corresponding to // ServiceWorker#postMessage() on the ServiceWorker object corresponding to
...@@ -300,7 +303,7 @@ TEST_F(ServiceWorkerHandleTest, ...@@ -300,7 +303,7 @@ TEST_F(ServiceWorkerHandleTest,
called = false; called = false;
status = SERVICE_WORKER_ERROR_MAX_VALUE; status = SERVICE_WORKER_ERROR_MAX_VALUE;
CallDispatchExtendableMessageEvent( CallDispatchExtendableMessageEvent(
sender_worker_handle.get(), std::move(message), sender_worker_handle, std::move(message),
base::BindOnce(&SaveStatusCallback, &called, &status)); base::BindOnce(&SaveStatusCallback, &called, &status));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_TRUE(called); EXPECT_TRUE(called);
...@@ -309,7 +312,7 @@ TEST_F(ServiceWorkerHandleTest, ...@@ -309,7 +312,7 @@ TEST_F(ServiceWorkerHandleTest,
// ExtendableMessageEventTestHelper, and the source service worker object info // ExtendableMessageEventTestHelper, and the source service worker object info
// should correspond to the pair (|version_->provider_host()|, |version_|), // should correspond to the pair (|version_->provider_host()|, |version_|),
// means it should correspond to |sender_worker_handle|. // means it should correspond to |sender_worker_handle|.
EXPECT_EQ(2u, GetBindingsCount(sender_worker_handle.get())); EXPECT_EQ(2u, GetBindingsCount(sender_worker_handle));
const std::vector<mojom::ExtendableMessageEventPtr>& events = const std::vector<mojom::ExtendableMessageEventPtr>& events =
static_cast<ExtendableMessageEventTestHelper*>(helper_.get())->events(); static_cast<ExtendableMessageEventTestHelper*>(helper_.get())->events();
EXPECT_EQ(1u, events.size()); EXPECT_EQ(1u, events.size());
...@@ -349,12 +352,10 @@ TEST_F(ServiceWorkerHandleTest, DispatchExtendableMessageEvent_FromClient) { ...@@ -349,12 +352,10 @@ TEST_F(ServiceWorkerHandleTest, DispatchExtendableMessageEvent_FromClient) {
helper_->context()->AsWeakPtr(), dispatcher_host_->AsWeakPtr()); helper_->context()->AsWeakPtr(), dispatcher_host_->AsWeakPtr());
provider_host->SetDocumentUrl(pattern); provider_host->SetDocumentUrl(pattern);
// Prepare a ServiceWorkerHandle for the above |provider_host|. // Prepare a ServiceWorkerHandle for the above |provider_host|.
blink::mojom::ServiceWorkerObjectInfoPtr info; blink::mojom::ServiceWorkerObjectInfoPtr info =
// ServiceWorkerHandle lifetime is controlled by |info| and is also owned by provider_host->GetOrCreateServiceWorkerHandle(version_.get());
// |dispatcher_host_|. ServiceWorkerHandle* handle =
base::WeakPtr<ServiceWorkerHandle> handle = ServiceWorkerHandle::Create( GetServiceWorkerHandle(provider_host.get(), version_->version_id());
dispatcher_host_.get(), helper_->context()->AsWeakPtr(),
provider_host->AsWeakPtr(), version_.get(), &info);
// Simulate dispatching an ExtendableMessageEvent. // Simulate dispatching an ExtendableMessageEvent.
blink::TransferableMessage message; blink::TransferableMessage message;
...@@ -362,7 +363,7 @@ TEST_F(ServiceWorkerHandleTest, DispatchExtendableMessageEvent_FromClient) { ...@@ -362,7 +363,7 @@ TEST_F(ServiceWorkerHandleTest, DispatchExtendableMessageEvent_FromClient) {
bool called = false; bool called = false;
ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_MAX_VALUE; ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_MAX_VALUE;
CallDispatchExtendableMessageEvent( CallDispatchExtendableMessageEvent(
handle.get(), std::move(message), handle, std::move(message),
base::BindOnce(&SaveStatusCallback, &called, &status)); base::BindOnce(&SaveStatusCallback, &called, &status));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_TRUE(called); EXPECT_TRUE(called);
...@@ -406,12 +407,10 @@ TEST_F(ServiceWorkerHandleTest, DispatchExtendableMessageEvent_Fail) { ...@@ -406,12 +407,10 @@ TEST_F(ServiceWorkerHandleTest, DispatchExtendableMessageEvent_Fail) {
helper_->context()->AsWeakPtr(), dispatcher_host_->AsWeakPtr()); helper_->context()->AsWeakPtr(), dispatcher_host_->AsWeakPtr());
provider_host->SetDocumentUrl(pattern); provider_host->SetDocumentUrl(pattern);
// Prepare a ServiceWorkerHandle for the above |provider_host|. // Prepare a ServiceWorkerHandle for the above |provider_host|.
blink::mojom::ServiceWorkerObjectInfoPtr info; blink::mojom::ServiceWorkerObjectInfoPtr info =
// ServiceWorkerHandle lifetime is controlled by |info| and is also owned by provider_host->GetOrCreateServiceWorkerHandle(version_.get());
// |dispatcher_host_|. ServiceWorkerHandle* handle =
base::WeakPtr<ServiceWorkerHandle> handle = ServiceWorkerHandle::Create( GetServiceWorkerHandle(provider_host.get(), version_->version_id());
dispatcher_host_.get(), helper_->context()->AsWeakPtr(),
provider_host->AsWeakPtr(), version_.get(), &info);
// Try to dispatch ExtendableMessageEvent. This should fail to start the // Try to dispatch ExtendableMessageEvent. This should fail to start the
// worker and to dispatch the event. // worker and to dispatch the event.
...@@ -420,7 +419,7 @@ TEST_F(ServiceWorkerHandleTest, DispatchExtendableMessageEvent_Fail) { ...@@ -420,7 +419,7 @@ TEST_F(ServiceWorkerHandleTest, DispatchExtendableMessageEvent_Fail) {
bool called = false; bool called = false;
ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_MAX_VALUE; ServiceWorkerStatusCode status = SERVICE_WORKER_ERROR_MAX_VALUE;
CallDispatchExtendableMessageEvent( CallDispatchExtendableMessageEvent(
handle.get(), std::move(message), handle, std::move(message),
base::BindOnce(&SaveStatusCallback, &called, &status)); base::BindOnce(&SaveStatusCallback, &called, &status));
base::RunLoop().RunUntilIdle(); base::RunLoop().RunUntilIdle();
EXPECT_TRUE(called); EXPECT_TRUE(called);
......
...@@ -19,7 +19,6 @@ ...@@ -19,7 +19,6 @@
#include "content/browser/service_worker/service_worker_context_core.h" #include "content/browser/service_worker/service_worker_context_core.h"
#include "content/browser/service_worker/service_worker_context_wrapper.h" #include "content/browser/service_worker/service_worker_context_wrapper.h"
#include "content/browser/service_worker/service_worker_disk_cache.h" #include "content/browser/service_worker/service_worker_disk_cache.h"
#include "content/browser/service_worker/service_worker_dispatcher_host.h"
#include "content/browser/service_worker/service_worker_handle.h" #include "content/browser/service_worker/service_worker_handle.h"
#include "content/browser/service_worker/service_worker_job_coordinator.h" #include "content/browser/service_worker/service_worker_job_coordinator.h"
#include "content/browser/service_worker/service_worker_registration.h" #include "content/browser/service_worker/service_worker_registration.h"
...@@ -50,42 +49,6 @@ namespace content { ...@@ -50,42 +49,6 @@ namespace content {
namespace { namespace {
// A dispatcher host that holds on to all registered ServiceWorkerHandles.
class KeepHandlesDispatcherHost : public ServiceWorkerDispatcherHost {
public:
KeepHandlesDispatcherHost(int render_process_id,
ResourceContext* resource_context)
: ServiceWorkerDispatcherHost(render_process_id, resource_context) {}
void RegisterServiceWorkerHandle(
std::unique_ptr<ServiceWorkerHandle> handle) override {
handles_.push_back(std::move(handle));
}
void UnregisterServiceWorkerHandle(int handle_id) override {
auto iter = handles_.begin();
for (; iter != handles_.end(); ++iter) {
if ((*iter)->handle_id() == handle_id)
break;
}
ASSERT_NE(handles_.end(), iter);
handles_.erase(iter);
}
void Clear() {
handles_.clear();
}
const std::vector<std::unique_ptr<ServiceWorkerHandle>>& handles() {
return handles_;
}
private:
~KeepHandlesDispatcherHost() override {}
std::vector<std::unique_ptr<ServiceWorkerHandle>> handles_;
DISALLOW_COPY_AND_ASSIGN(KeepHandlesDispatcherHost);
};
void SaveRegistrationCallback( void SaveRegistrationCallback(
ServiceWorkerStatusCode expected_status, ServiceWorkerStatusCode expected_status,
bool* called, bool* called,
...@@ -338,41 +301,32 @@ TEST_F(ServiceWorkerJobTest, Register) { ...@@ -338,41 +301,32 @@ TEST_F(ServiceWorkerJobTest, Register) {
// Make sure registrations are cleaned up when they are unregistered. // Make sure registrations are cleaned up when they are unregistered.
TEST_F(ServiceWorkerJobTest, Unregister) { TEST_F(ServiceWorkerJobTest, Unregister) {
// During registration, service worker handles will be created to host the
// {installing,waiting,active} service worker objects for
// ServiceWorkerGlobalScope#registration. KeepHandlesDispatcherHost will store
// the handles.
scoped_refptr<KeepHandlesDispatcherHost> dispatcher_host =
base::MakeRefCounted<KeepHandlesDispatcherHost>(
helper_->mock_render_process_id(),
helper_->browser_context()->GetResourceContext());
helper_->RegisterDispatcherHost(helper_->mock_render_process_id(),
dispatcher_host);
dispatcher_host->Init(helper_->context_wrapper());
blink::mojom::ServiceWorkerRegistrationOptions options; blink::mojom::ServiceWorkerRegistrationOptions options;
options.scope = GURL("http://www.example.com/"); options.scope = GURL("http://www.example.com/");
scoped_refptr<ServiceWorkerRegistration> registration = scoped_refptr<ServiceWorkerRegistration> registration =
RunRegisterJob(GURL("http://www.example.com/service_worker.js"), options); RunRegisterJob(GURL("http://www.example.com/service_worker.js"), options);
scoped_refptr<ServiceWorkerVersion> version = registration->active_version();
// During the above registration, a service worker registration object host
// for ServiceWorkerGlobalScope#registration has been created/added into
// |provider_host|.
ServiceWorkerProviderHost* provider_host = ServiceWorkerProviderHost* provider_host =
registration->active_version()->provider_host(); registration->active_version()->provider_host();
ASSERT_NE(nullptr, provider_host); ASSERT_NE(nullptr, provider_host);
// One ServiceWorkerRegistrationObjectHost should have been created for the
// new registration.
EXPECT_EQ(1UL, provider_host->registration_object_hosts_.size()); EXPECT_EQ(1UL, provider_host->registration_object_hosts_.size());
EXPECT_EQ(3UL, dispatcher_host->handles().size()); // One ServiceWorkerHandle should have been created for the new service
// worker.
EXPECT_EQ(1UL, provider_host->handles_.size());
RunUnregisterJob(options.scope); RunUnregisterJob(options.scope);
// Clear all service worker handles. // The service worker registration object host and service worker handle have
dispatcher_host->Clear(); // been destroyed together with |provider_host| by the above unregistration.
EXPECT_EQ(0UL, dispatcher_host->handles().size()); // Then |registration| and |version| should be the last one reference to the
// The service worker registration object host has been destroyed together // corresponding instance.
// with |provider_host| by the above unregistration. Then the only reference
// to the registration should be |registration|.
EXPECT_TRUE(registration->HasOneRef()); EXPECT_TRUE(registration->HasOneRef());
EXPECT_TRUE(version->HasOneRef());
EXPECT_EQ(EmbeddedWorkerStatus::STOPPED, version->running_status());
EXPECT_EQ(ServiceWorkerVersion::REDUNDANT, version->status());
registration = registration =
FindRegistrationForPattern(options.scope, SERVICE_WORKER_ERROR_NOT_FOUND); FindRegistrationForPattern(options.scope, SERVICE_WORKER_ERROR_NOT_FOUND);
...@@ -415,16 +369,6 @@ TEST_F(ServiceWorkerJobTest, RegisterNewScript) { ...@@ -415,16 +369,6 @@ TEST_F(ServiceWorkerJobTest, RegisterNewScript) {
// Make sure that when registering a duplicate pattern+script_url // Make sure that when registering a duplicate pattern+script_url
// combination, that the same registration is used. // combination, that the same registration is used.
TEST_F(ServiceWorkerJobTest, RegisterDuplicateScript) { TEST_F(ServiceWorkerJobTest, RegisterDuplicateScript) {
// During registration, handles will be created for hosting the worker's
// context. KeepHandlesDispatcherHost will store the handles.
scoped_refptr<KeepHandlesDispatcherHost> dispatcher_host =
new KeepHandlesDispatcherHost(
helper_->mock_render_process_id(),
helper_->browser_context()->GetResourceContext());
helper_->RegisterDispatcherHost(helper_->mock_render_process_id(),
dispatcher_host);
dispatcher_host->Init(helper_->context_wrapper());
GURL script_url("http://www.example.com/service_worker.js"); GURL script_url("http://www.example.com/service_worker.js");
blink::mojom::ServiceWorkerRegistrationOptions options; blink::mojom::ServiceWorkerRegistrationOptions options;
options.scope = GURL("http://www.example.com/"); options.scope = GURL("http://www.example.com/");
...@@ -440,7 +384,7 @@ TEST_F(ServiceWorkerJobTest, RegisterDuplicateScript) { ...@@ -440,7 +384,7 @@ TEST_F(ServiceWorkerJobTest, RegisterDuplicateScript) {
ASSERT_NE(nullptr, provider_host); ASSERT_NE(nullptr, provider_host);
// Clear all service worker handles. // Clear all service worker handles.
dispatcher_host->Clear(); provider_host->handles_.clear();
// Ensure that the registration's object host doesn't have the reference. // Ensure that the registration's object host doesn't have the reference.
EXPECT_EQ(1UL, provider_host->registration_object_hosts_.size()); EXPECT_EQ(1UL, provider_host->registration_object_hosts_.size());
provider_host->registration_object_hosts_.clear(); provider_host->registration_object_hosts_.clear();
...@@ -469,15 +413,6 @@ TEST_F(ServiceWorkerJobTest, RegisterDuplicateScript) { ...@@ -469,15 +413,6 @@ TEST_F(ServiceWorkerJobTest, RegisterDuplicateScript) {
// is updated when registering a duplicate pattern+script_url with a different // is updated when registering a duplicate pattern+script_url with a different
// update_via_cache value. // update_via_cache value.
TEST_F(ServiceWorkerJobTest, RegisterWithDifferentUpdateViaCache) { TEST_F(ServiceWorkerJobTest, RegisterWithDifferentUpdateViaCache) {
// During registration, handles will be created for hosting the worker's
// context. KeepHandlesDispatcherHost will store the handles.
auto dispatcher_host = base::MakeRefCounted<KeepHandlesDispatcherHost>(
helper_->mock_render_process_id(),
helper_->browser_context()->GetResourceContext());
helper_->RegisterDispatcherHost(helper_->mock_render_process_id(),
dispatcher_host);
dispatcher_host->Init(helper_->context_wrapper());
GURL script_url("https://www.example.com/service_worker.js"); GURL script_url("https://www.example.com/service_worker.js");
blink::mojom::ServiceWorkerRegistrationOptions options; blink::mojom::ServiceWorkerRegistrationOptions options;
options.scope = GURL("https://www.example.com/"); options.scope = GURL("https://www.example.com/");
...@@ -496,7 +431,7 @@ TEST_F(ServiceWorkerJobTest, RegisterWithDifferentUpdateViaCache) { ...@@ -496,7 +431,7 @@ TEST_F(ServiceWorkerJobTest, RegisterWithDifferentUpdateViaCache) {
ASSERT_NE(nullptr, provider_host); ASSERT_NE(nullptr, provider_host);
// Clear all service worker handles. // Clear all service worker handles.
dispatcher_host->Clear(); provider_host->handles_.clear();
// Ensure that the registration's object host doesn't have the reference. // Ensure that the registration's object host doesn't have the reference.
EXPECT_EQ(1UL, provider_host->registration_object_hosts_.size()); EXPECT_EQ(1UL, provider_host->registration_object_hosts_.size());
provider_host->registration_object_hosts_.clear(); provider_host->registration_object_hosts_.clear();
......
...@@ -444,6 +444,11 @@ void ServiceWorkerProviderHost::RemoveServiceWorkerRegistrationObjectHost( ...@@ -444,6 +444,11 @@ void ServiceWorkerProviderHost::RemoveServiceWorkerRegistrationObjectHost(
registration_object_hosts_.erase(registration_id); registration_object_hosts_.erase(registration_id);
} }
void ServiceWorkerProviderHost::RemoveServiceWorkerHandle(int64_t version_id) {
DCHECK(base::ContainsKey(handles_, version_id));
handles_.erase(version_id);
}
bool ServiceWorkerProviderHost::AllowServiceWorker(const GURL& scope) { bool ServiceWorkerProviderHost::AllowServiceWorker(const GURL& scope) {
return GetContentClient()->browser()->AllowServiceWorker( return GetContentClient()->browser()->AllowServiceWorker(
scope, IsProviderForClient() ? topmost_frame_url() : document_url(), scope, IsProviderForClient() ? topmost_frame_url() : document_url(),
...@@ -507,29 +512,15 @@ ServiceWorkerProviderHost::GetOrCreateServiceWorkerHandle( ...@@ -507,29 +512,15 @@ ServiceWorkerProviderHost::GetOrCreateServiceWorkerHandle(
ServiceWorkerVersion* version) { ServiceWorkerVersion* version) {
if (!context_ || !version) if (!context_ || !version)
return nullptr; return nullptr;
if (!dispatcher_host_) {
DCHECK(ServiceWorkerUtils::IsServicificationEnabled() ||
IsNavigationMojoResponseEnabled());
blink::mojom::ServiceWorkerObjectInfoPtr info;
// This is called before the dispatcher host is created.
// |precreated_controller_handle_| instance's lifetime is controlled by its
// own internal Mojo connections via |info|.
precreated_controller_handle_ = ServiceWorkerHandle::Create(
nullptr, context_, AsWeakPtr(), version, &info);
return info;
}
ServiceWorkerHandle* handle = dispatcher_host_->FindServiceWorkerHandle(
provider_id(), version->version_id());
if (handle) {
return handle->CreateObjectInfo();
}
blink::mojom::ServiceWorkerObjectInfoPtr info; const int64_t version_id = version->version_id();
// ServiceWorkerHandle lifetime is controlled by |info| and is also owned by auto existing_handle = handles_.find(version_id);
// |dispatcher_host_|. if (existing_handle != handles_.end())
ServiceWorkerHandle::Create(dispatcher_host_.get(), context_, AsWeakPtr(), return existing_handle->second->CreateObjectInfo();
version, &info);
return info; handles_[version_id] =
std::make_unique<ServiceWorkerHandle>(context_, this, version);
return handles_[version_id]->CreateObjectInfo();
} }
bool ServiceWorkerProviderHost::CanAssociateRegistration( bool ServiceWorkerProviderHost::CanAssociateRegistration(
...@@ -603,18 +594,6 @@ void ServiceWorkerProviderHost::CompleteNavigationInitialized( ...@@ -603,18 +594,6 @@ void ServiceWorkerProviderHost::CompleteNavigationInitialized(
if (!controller_) if (!controller_)
return; return;
if ((ServiceWorkerUtils::IsServicificationEnabled() ||
IsNavigationMojoResponseEnabled()) &&
precreated_controller_handle_) {
// S13nServiceWorker: register the pre-created handle for the controller
// service worker with the dispatcher host, now that it exists.
DCHECK_NE(blink::mojom::kInvalidServiceWorkerHandleId,
precreated_controller_handle_->handle_id());
precreated_controller_handle_->RegisterIntoDispatcherHost(
dispatcher_host_.get());
precreated_controller_handle_ = nullptr;
}
// In S13nServiceWorker/NavigationMojoResponse case the controller is already // In S13nServiceWorker/NavigationMojoResponse case the controller is already
// sent in navigation commit, but we still need this for // sent in navigation commit, but we still need this for
// S13nServiceWorker/NavigationMojoResponse case for setting the use counter // S13nServiceWorker/NavigationMojoResponse case for setting the use counter
......
...@@ -40,6 +40,10 @@ namespace network { ...@@ -40,6 +40,10 @@ namespace network {
class ResourceRequestBody; class ResourceRequestBody;
} }
namespace service_worker_handle_unittest {
class ServiceWorkerHandleTest;
}
namespace storage { namespace storage {
class BlobStorageContext; class BlobStorageContext;
} }
...@@ -282,6 +286,8 @@ class CONTENT_EXPORT ServiceWorkerProviderHost ...@@ -282,6 +286,8 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
// The object info holds a Mojo connection to the ServiceWorkerHandle for the // The object info holds a Mojo connection to the ServiceWorkerHandle for the
// |version| to ensure the handle stays alive while the object info is alive. // |version| to ensure the handle stays alive while the object info is alive.
// A new handle is created if one does not already exist. // A new handle is created if one does not already exist.
// TODO(leonhsl): Make |version| be a scoped_refptr because we'll take its
// ownership.
blink::mojom::ServiceWorkerObjectInfoPtr GetOrCreateServiceWorkerHandle( blink::mojom::ServiceWorkerObjectInfoPtr GetOrCreateServiceWorkerHandle(
ServiceWorkerVersion* version); ServiceWorkerVersion* version);
...@@ -345,6 +351,9 @@ class CONTENT_EXPORT ServiceWorkerProviderHost ...@@ -345,6 +351,9 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
// |registration_id|. // |registration_id|.
void RemoveServiceWorkerRegistrationObjectHost(int64_t registration_id); void RemoveServiceWorkerRegistrationObjectHost(int64_t registration_id);
// Removes the ServiceWorkerHandle corresponding to |version_id|.
void RemoveServiceWorkerHandle(int64_t version_id);
// Calls ContentBrowserClient::AllowServiceWorker(). Returns true if content // Calls ContentBrowserClient::AllowServiceWorker(). Returns true if content
// settings allows service workers to run at |scope|. If this provider is for // settings allows service workers to run at |scope|. If this provider is for
// a window client, the check involves the topmost frame url as well as // a window client, the check involves the topmost frame url as well as
...@@ -361,6 +370,7 @@ class CONTENT_EXPORT ServiceWorkerProviderHost ...@@ -361,6 +370,7 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
friend class ServiceWorkerProviderHostTest; friend class ServiceWorkerProviderHostTest;
friend class ServiceWorkerWriteToCacheJobTest; friend class ServiceWorkerWriteToCacheJobTest;
friend class ServiceWorkerContextRequestHandlerTest; friend class ServiceWorkerContextRequestHandlerTest;
friend class service_worker_handle_unittest::ServiceWorkerHandleTest;
FRIEND_TEST_ALL_PREFIXES(ServiceWorkerWriteToCacheJobTest, Update_SameScript); FRIEND_TEST_ALL_PREFIXES(ServiceWorkerWriteToCacheJobTest, Update_SameScript);
FRIEND_TEST_ALL_PREFIXES(ServiceWorkerWriteToCacheJobTest, FRIEND_TEST_ALL_PREFIXES(ServiceWorkerWriteToCacheJobTest,
Update_SameSizeScript); Update_SameSizeScript);
...@@ -529,6 +539,13 @@ class CONTENT_EXPORT ServiceWorkerProviderHost ...@@ -529,6 +539,13 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
std::unique_ptr<ServiceWorkerRegistrationObjectHost>> std::unique_ptr<ServiceWorkerRegistrationObjectHost>>
registration_object_hosts_; registration_object_hosts_;
// Contains all ServiceWorkerHandle instances corresponding to
// the service worker JavaScript objects for the hosted execution
// context (service worker global scope or service worker client) in the
// renderer process.
std::map<int64_t /* version_id */, std::unique_ptr<ServiceWorkerHandle>>
handles_;
// The ready() promise is only allowed to be created once. // The ready() promise is only allowed to be created once.
// |get_ready_callback_| has three states: // |get_ready_callback_| has three states:
// 1. |get_ready_callback_| is null when ready() has not yet been called. // 1. |get_ready_callback_| is null when ready() has not yet been called.
...@@ -578,12 +595,6 @@ class CONTENT_EXPORT ServiceWorkerProviderHost ...@@ -578,12 +595,6 @@ class CONTENT_EXPORT ServiceWorkerProviderHost
std::vector<base::Closure> queued_events_; std::vector<base::Closure> queued_events_;
// S13nServiceWorker/NavigationMojoResponse:
// A service worker handle for the controller service worker that is
// pre-created before the renderer process (and therefore the dispatcher host)
// is created.
base::WeakPtr<ServiceWorkerHandle> precreated_controller_handle_;
// For provider hosts that are hosting a running service worker. // For provider hosts that are hosting a running service worker.
mojo::Binding<service_manager::mojom::InterfaceProvider> mojo::Binding<service_manager::mojom::InterfaceProvider>
interface_provider_binding_; interface_provider_binding_;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment