Skip to content

Commit

Permalink
Remove direct dependency of the main thread from ServiceWorkerContext…
Browse files Browse the repository at this point in the history 
…Client

When off-the-main-thread service worker startup is enabled, we will
create ServiceWorkerContextClient on the IO thread. Stop accessing
GetWebMainThreadScheduler() directly from
EmbeddedWorkerInstanceClientImpl. Instead, plumb a task runner
from the RenderThreadImpl. This way we could change the task runner
easily to the one that is bound to the IO thread in the future.

Update comments, methods, and member fields that mention the main
thread. As an alternative, I chose "the starter thread."

Bug: 692909
Change-Id: I061d0bb6da3c8c6985a3e591d55ae57e9f5c3924
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1743227
Commit-Queue: Kenichi Ishibashi <bashi@chromium.org>
Reviewed-by: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: Hiroki Nakagawa <nhiroki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#685114}
  • Loading branch information
@bashi
bashi authored and Commit Bot committed Aug 8, 2019
1 parent 855688f commit 5480820
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 54 deletions.
4 changes: 3 additions & 1 deletion content/renderer/render_thread_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2082,7 +2082,9 @@ void RenderThreadImpl::SetUpEmbeddedWorkerChannelForServiceWorker(
init_end_ = base::TimeTicks();
}

EmbeddedWorkerInstanceClientImpl::Create(std::move(client_request));
EmbeddedWorkerInstanceClientImpl::Create(
std::move(client_request),
GetWebMainThreadScheduler()->DefaultTaskRunner());
}

void RenderThreadImpl::OnNetworkConnectionChanged(
Expand Down
34 changes: 22 additions & 12 deletions content/renderer/service_worker/embedded_worker_instance_client_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "content/renderer/render_thread_impl.h"
#include "content/renderer/service_worker/service_worker_context_client.h"
#include "third_party/blink/public/common/features.h"
#include "third_party/blink/public/platform/scheduler/web_thread_scheduler.h"
#include "third_party/blink/public/platform/web_content_settings_client.h"
#include "third_party/blink/public/platform/web_security_origin.h"
#include "third_party/blink/public/platform/web_url.h"
Expand All @@ -28,24 +27,27 @@ namespace content {

// static
void EmbeddedWorkerInstanceClientImpl::Create(
blink::mojom::EmbeddedWorkerInstanceClientRequest request) {
blink::mojom::EmbeddedWorkerInstanceClientRequest request,
scoped_refptr<base::SingleThreadTaskRunner> starter_thread_task_runner) {
// This won't be leaked because the lifetime will be managed internally.
// See the class documentation for detail.
// We can't use MakeStrongBinding because must give the worker thread
// a chance to stop by calling TerminateWorkerContext() and waiting
// before destructing.
new EmbeddedWorkerInstanceClientImpl(std::move(request));
new EmbeddedWorkerInstanceClientImpl(std::move(request),
std::move(starter_thread_task_runner));
}

void EmbeddedWorkerInstanceClientImpl::WorkerContextDestroyed() {
DCHECK(starter_thread_task_runner_->BelongsToCurrentThread());
TRACE_EVENT0("ServiceWorker",
"EmbeddedWorkerInstanceClientImpl::WorkerContextDestroyed");
delete this;
}

void EmbeddedWorkerInstanceClientImpl::StartWorker(
blink::mojom::EmbeddedWorkerStartParamsPtr params) {
DCHECK(ChildThreadImpl::current());
DCHECK(starter_thread_task_runner_->BelongsToCurrentThread());
DCHECK(!service_worker_context_client_);
TRACE_EVENT0("ServiceWorker",
"EmbeddedWorkerInstanceClientImpl::StartWorker");
Expand All @@ -71,9 +73,7 @@ void EmbeddedWorkerInstanceClientImpl::StartWorker(
std::move(params->preference_watcher_request),
std::move(params->subresource_loader_factories),
std::move(params->subresource_loader_updater),
RenderThreadImpl::current()
->GetWebMainThreadScheduler()
->DefaultTaskRunner());
starter_thread_task_runner_);
// Record UMA to indicate StartWorker is received on renderer.
StartWorkerHistogramEnum metric =
params->is_installed ? StartWorkerHistogramEnum::RECEIVED_ON_INSTALLED
Expand Down Expand Up @@ -105,24 +105,27 @@ void EmbeddedWorkerInstanceClientImpl::StartWorker(
std::move(installed_scripts_manager_params),
params->content_settings_proxy.PassHandle(), cache_storage.PassHandle(),
interface_provider.PassHandle());
service_worker_context_client_->StartWorkerContext(std::move(worker),
start_data);
service_worker_context_client_->StartWorkerContextOnStarterThread(
std::move(worker), start_data);
}

void EmbeddedWorkerInstanceClientImpl::StopWorker() {
DCHECK(starter_thread_task_runner_->BelongsToCurrentThread());
TRACE_EVENT0("ServiceWorker", "EmbeddedWorkerInstanceClientImpl::StopWorker");
// StopWorker must be called after StartWorker is called.
service_worker_context_client_->worker().TerminateWorkerContext();
// We continue in WorkerContextDestroyed() after the worker thread is stopped.
}

void EmbeddedWorkerInstanceClientImpl::ResumeAfterDownload() {
DCHECK(starter_thread_task_runner_->BelongsToCurrentThread());
service_worker_context_client_->worker().ResumeAfterDownload();
}

void EmbeddedWorkerInstanceClientImpl::AddMessageToConsole(
blink::mojom::ConsoleMessageLevel level,
const std::string& message) {
DCHECK(starter_thread_task_runner_->BelongsToCurrentThread());
service_worker_context_client_->worker().AddMessageToConsole(
blink::WebConsoleMessage(level, blink::WebString::FromUTF8(message)));
}
Expand All @@ -135,15 +138,21 @@ void EmbeddedWorkerInstanceClientImpl::BindDevToolsAgent(
}

EmbeddedWorkerInstanceClientImpl::EmbeddedWorkerInstanceClientImpl(
blink::mojom::EmbeddedWorkerInstanceClientRequest request)
: binding_(this, std::move(request)) {
blink::mojom::EmbeddedWorkerInstanceClientRequest request,
scoped_refptr<base::SingleThreadTaskRunner> starter_thread_task_runner)
: binding_(this, std::move(request)),
starter_thread_task_runner_(std::move(starter_thread_task_runner)) {
DCHECK(starter_thread_task_runner_->BelongsToCurrentThread());
binding_.set_connection_error_handler(base::BindOnce(
&EmbeddedWorkerInstanceClientImpl::OnError, base::Unretained(this)));
}

EmbeddedWorkerInstanceClientImpl::~EmbeddedWorkerInstanceClientImpl() {}
EmbeddedWorkerInstanceClientImpl::~EmbeddedWorkerInstanceClientImpl() {
DCHECK(starter_thread_task_runner_->BelongsToCurrentThread());
}

void EmbeddedWorkerInstanceClientImpl::OnError() {
DCHECK(starter_thread_task_runner_->BelongsToCurrentThread());
// The connection to the browser process broke.
if (service_worker_context_client_) {
// The worker is running, so tell it to stop. We continue in
Expand All @@ -159,6 +168,7 @@ void EmbeddedWorkerInstanceClientImpl::OnError() {
blink::WebEmbeddedWorkerStartData
EmbeddedWorkerInstanceClientImpl::BuildStartData(
const blink::mojom::EmbeddedWorkerStartParams& params) {
DCHECK(starter_thread_task_runner_->BelongsToCurrentThread());
blink::WebEmbeddedWorkerStartData start_data;
start_data.script_url = params.script_url;
start_data.user_agent = blink::WebString::FromUTF8(params.user_agent);
Expand Down
17 changes: 13 additions & 4 deletions content/renderer/service_worker/embedded_worker_instance_client_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,9 @@ class ServiceWorkerContextClient;
// the Mojo connection to the browser breaks first, the instance waits for the
// service worker to stop and then deletes itself.
//
// All methods are called on the main thread.
// All methods are called on the thread that creates the instance of this class.
// Currently it's the main thread but it could be the IO thread in the future.
// https://crbug.com/692909
class CONTENT_EXPORT EmbeddedWorkerInstanceClientImpl
: public blink::mojom::EmbeddedWorkerInstanceClient {
public:
Expand All @@ -42,7 +44,9 @@ class CONTENT_EXPORT EmbeddedWorkerInstanceClientImpl
// documentation.
// TODO(shimazu): Create a service worker's execution context by this method
// instead of just creating an instance of EmbeddedWorkerInstanceClient.
static void Create(blink::mojom::EmbeddedWorkerInstanceClientRequest request);
static void Create(
blink::mojom::EmbeddedWorkerInstanceClientRequest request,
scoped_refptr<base::SingleThreadTaskRunner> starter_task_runner);

~EmbeddedWorkerInstanceClientImpl() override;

Expand All @@ -56,8 +60,9 @@ class CONTENT_EXPORT EmbeddedWorkerInstanceClientImpl
private:
friend class ServiceWorkerContextClientTest;

explicit EmbeddedWorkerInstanceClientImpl(
blink::mojom::EmbeddedWorkerInstanceClientRequest request);
EmbeddedWorkerInstanceClientImpl(
blink::mojom::EmbeddedWorkerInstanceClientRequest request,
scoped_refptr<base::SingleThreadTaskRunner> starter_thread_task_runner);

// blink::mojom::EmbeddedWorkerInstanceClient implementation
void StartWorker(blink::mojom::EmbeddedWorkerStartParamsPtr params) override;
Expand All @@ -76,6 +81,10 @@ class CONTENT_EXPORT EmbeddedWorkerInstanceClientImpl

mojo::Binding<blink::mojom::EmbeddedWorkerInstanceClient> binding_;

// A copy of this runner is also passed to ServiceWorkerContextClient in
// StartWorker().
scoped_refptr<base::SingleThreadTaskRunner> starter_thread_task_runner_;

// nullptr means worker is not running.
std::unique_ptr<ServiceWorkerContextClient> service_worker_context_client_;

Expand Down
45 changes: 23 additions & 22 deletions content/renderer/service_worker/service_worker_context_client.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -109,27 +109,27 @@ ServiceWorkerContextClient::ServiceWorkerContextClient(
std::unique_ptr<blink::URLLoaderFactoryBundleInfo> subresource_loaders,
mojo::PendingReceiver<blink::mojom::ServiceWorkerSubresourceLoaderUpdater>
subresource_loader_updater,
scoped_refptr<base::SingleThreadTaskRunner> main_thread_task_runner)
scoped_refptr<base::SingleThreadTaskRunner> starter_thread_task_runner)
: service_worker_version_id_(service_worker_version_id),
service_worker_scope_(service_worker_scope),
script_url_(script_url),
is_starting_installed_worker_(is_starting_installed_worker),
renderer_preferences_(std::move(renderer_preferences)),
preference_watcher_request_(std::move(preference_watcher_request)),
main_thread_task_runner_(std::move(main_thread_task_runner)),
starter_thread_task_runner_(std::move(starter_thread_task_runner)),
proxy_(nullptr),
pending_service_worker_request_(std::move(service_worker_request)),
controller_receiver_(std::move(controller_receiver)),
pending_subresource_loader_updater_(
std::move(subresource_loader_updater)),
owner_(owner),
start_timing_(std::move(start_timing)) {
DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
DCHECK(starter_thread_task_runner_->RunsTasksInCurrentSequence());
DCHECK(owner_);
DCHECK(subresource_loaders);
instance_host_ =
blink::mojom::ThreadSafeEmbeddedWorkerInstanceHostAssociatedPtr::Create(
std::move(instance_host), main_thread_task_runner_);
std::move(instance_host), starter_thread_task_runner_);

if (IsOutOfProcessNetworkService()) {
// If the network service crashes, this worker self-terminates, so it can
Expand All @@ -143,9 +143,9 @@ ServiceWorkerContextClient::ServiceWorkerContextClient(
subresource_loaders->pending_default_factory()
.InitWithNewPipeAndPassReceiver());
network_service_connection_error_handler_holder_
.set_connection_error_handler(
base::BindOnce(&ServiceWorkerContextClient::StopWorkerOnMainThread,
base::Unretained(this)));
.set_connection_error_handler(base::BindOnce(
&ServiceWorkerContextClient::StopWorkerOnStarterThread,
base::Unretained(this)));
}

loader_factories_ = base::MakeRefCounted<ChildURLLoaderFactoryBundle>(
Expand All @@ -164,29 +164,29 @@ ServiceWorkerContextClient::ServiceWorkerContextClient(
}

ServiceWorkerContextClient::~ServiceWorkerContextClient() {
DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
DCHECK(starter_thread_task_runner_->RunsTasksInCurrentSequence());
}

void ServiceWorkerContextClient::StartWorkerContext(
void ServiceWorkerContextClient::StartWorkerContextOnStarterThread(
std::unique_ptr<blink::WebEmbeddedWorker> worker,
const blink::WebEmbeddedWorkerStartData& start_data) {
DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
DCHECK(starter_thread_task_runner_->RunsTasksInCurrentSequence());
worker_ = std::move(worker);
worker_->StartWorkerContext(start_data);
}

blink::WebEmbeddedWorker& ServiceWorkerContextClient::worker() {
DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
DCHECK(starter_thread_task_runner_->RunsTasksInCurrentSequence());
return *worker_;
}

void ServiceWorkerContextClient::WorkerReadyForInspectionOnMainThread() {
DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
DCHECK(starter_thread_task_runner_->RunsTasksInCurrentSequence());
(*instance_host_)->OnReadyForInspection();
}

void ServiceWorkerContextClient::WorkerContextFailedToStartOnMainThread() {
DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
DCHECK(starter_thread_task_runner_->RunsTasksInCurrentSequence());
DCHECK(!proxy_);

(*instance_host_)->OnStopped();
Expand Down Expand Up @@ -225,7 +225,7 @@ void ServiceWorkerContextClient::FailedToFetchModuleScript() {
}

void ServiceWorkerContextClient::WorkerScriptLoadedOnMainThread() {
DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
DCHECK(starter_thread_task_runner_->RunsTasksInCurrentSequence());
DCHECK(!is_starting_installed_worker_);
(*instance_host_)->OnScriptLoaded();
TRACE_EVENT_NESTABLE_ASYNC_END0("ServiceWorker", "LOAD_SCRIPT", this);
Expand All @@ -240,8 +240,9 @@ void ServiceWorkerContextClient::WorkerScriptLoadedOnWorkerThread() {
void ServiceWorkerContextClient::WorkerContextStarted(
blink::WebServiceWorkerContextProxy* proxy,
scoped_refptr<base::SequencedTaskRunner> worker_task_runner) {
DCHECK_NE(0, WorkerThread::GetCurrentId())
<< "service worker started on the main thread instead of a worker thread";
DCHECK(!starter_thread_task_runner_->RunsTasksInCurrentSequence())
<< "service worker started on the starter thread instead of a worker "
"thread";
DCHECK(worker_task_runner->RunsTasksInCurrentSequence());
DCHECK(!worker_task_runner_);
worker_task_runner_ = std::move(worker_task_runner);
Expand Down Expand Up @@ -344,7 +345,7 @@ void ServiceWorkerContextClient::WorkerContextDestroyed() {

// base::Unretained is safe because |owner_| does not destroy itself until
// WorkerContextDestroyed is called.
main_thread_task_runner_->PostTask(
starter_thread_task_runner_->PostTask(
FROM_HERE,
base::BindOnce(&EmbeddedWorkerInstanceClientImpl::WorkerContextDestroyed,
base::Unretained(owner_)));
Expand Down Expand Up @@ -378,15 +379,15 @@ void ServiceWorkerContextClient::ReportConsoleMessage(

std::unique_ptr<blink::WebServiceWorkerNetworkProvider>
ServiceWorkerContextClient::CreateServiceWorkerNetworkProviderOnMainThread() {
DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
DCHECK(starter_thread_task_runner_->RunsTasksInCurrentSequence());
return std::make_unique<ServiceWorkerNetworkProviderForServiceWorker>(
std::move(service_worker_provider_info_->script_loader_factory_ptr_info));
}

scoped_refptr<blink::WebWorkerFetchContext>
ServiceWorkerContextClient::CreateWorkerFetchContextOnMainThreadLegacy(
blink::WebServiceWorkerNetworkProvider* provider) {
DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
DCHECK(starter_thread_task_runner_->RunsTasksInCurrentSequence());
DCHECK(preference_watcher_request_.is_pending());

// TODO(crbug.com/796425): Temporarily wrap the raw
Expand All @@ -413,7 +414,7 @@ ServiceWorkerContextClient::CreateWorkerFetchContextOnMainThreadLegacy(

scoped_refptr<blink::WebWorkerFetchContext>
ServiceWorkerContextClient::CreateWorkerFetchContextOnMainThread() {
DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
DCHECK(starter_thread_task_runner_->RunsTasksInCurrentSequence());
DCHECK(preference_watcher_request_.is_pending());

// TODO(bashi): Consider changing ServiceWorkerFetchContextImpl to take
Expand Down Expand Up @@ -543,8 +544,8 @@ void ServiceWorkerContextClient::RequestTermination(
(*instance_host_)->RequestTermination(std::move(callback));
}

void ServiceWorkerContextClient::StopWorkerOnMainThread() {
DCHECK(main_thread_task_runner_->RunsTasksInCurrentSequence());
void ServiceWorkerContextClient::StopWorkerOnStarterThread() {
DCHECK(starter_thread_task_runner_->RunsTasksInCurrentSequence());
owner_->StopWorker();
}

Expand Down

0 comments on commit 5480820

Please sign in to comment.