Commit 68e4f9b4 authored by Matt Falkenhagen's avatar Matt Falkenhagen Committed by Commit Bot

service worker: Assume context_ in EmbeddedWorkerInstance::Start.

This is only called by ServiceWorkerVersion::StartInternal, which
already assumes the context is alive (since it passes it to
ServiceWorkerProviderHost which uses it).

The motivation is removing the callback passed to Start. This is
currently invoked in only two places:
* When context_ was null (removed in this CL)
* When a OnScriptEvaluated IPC is received from the renderer.

The callback isn't invoked when startup failed before script evaluation,
and in success cases it is invoked before startup finishes (OnStarted()).
It can even be invoked with failure when startup succeeds, when an
uncaught runtime error occurred during script evaluation. I aim to
improve the code by first eliminating the callback then adding back one
when the implementation is capable of calling it in a more sensible
manner.

Bug: 859912
Change-Id: I74643f1a64ef6a637ccd9db6bf1f9e889c814b3a
Reviewed-on: https://chromium-review.googlesource.com/1124722
Commit-Queue: Matt Falkenhagen <falken@chromium.org>
Reviewed-by: default avatarMakoto Shimazu <shimazu@chromium.org>
Cr-Commit-Position: refs/heads/master@{#572534}
parent 96b7e77a
...@@ -407,6 +407,7 @@ class EmbeddedWorkerInstance::StartTask { ...@@ -407,6 +407,7 @@ class EmbeddedWorkerInstance::StartTask {
StatusCallback callback) { StatusCallback callback) {
DCHECK_CURRENTLY_ON(BrowserThread::IO); DCHECK_CURRENTLY_ON(BrowserThread::IO);
DCHECK(instance_->context_); DCHECK(instance_->context_);
base::WeakPtr<ServiceWorkerContextCore> context = instance_->context_;
state_ = ProcessAllocationState::ALLOCATING; state_ = ProcessAllocationState::ALLOCATING;
start_callback_ = std::move(callback); start_callback_ = std::move(callback);
is_installed_ = params->is_installed; is_installed_ = params->is_installed;
...@@ -415,12 +416,11 @@ class EmbeddedWorkerInstance::StartTask { ...@@ -415,12 +416,11 @@ class EmbeddedWorkerInstance::StartTask {
started_during_browser_startup_ = true; started_during_browser_startup_ = true;
bool can_use_existing_process = bool can_use_existing_process =
instance_->context_->GetVersionFailureCount( context->GetVersionFailureCount(params->service_worker_version_id) <
params->service_worker_version_id) < kMaxSameProcessFailureCount; kMaxSameProcessFailureCount;
DCHECK_EQ(params->embedded_worker_id, instance_->embedded_worker_id_); DCHECK_EQ(params->embedded_worker_id, instance_->embedded_worker_id_);
TRACE_EVENT_NESTABLE_ASYNC_BEGIN0("ServiceWorker", "ALLOCATING_PROCESS", TRACE_EVENT_NESTABLE_ASYNC_BEGIN0("ServiceWorker", "ALLOCATING_PROCESS",
this); this);
base::WeakPtr<ServiceWorkerContextCore> context = instance_->context_;
base::WeakPtr<ServiceWorkerProcessManager> process_manager = base::WeakPtr<ServiceWorkerProcessManager> process_manager =
context->process_manager()->AsWeakPtr(); context->process_manager()->AsWeakPtr();
...@@ -558,12 +558,8 @@ EmbeddedWorkerInstance::~EmbeddedWorkerInstance() { ...@@ -558,12 +558,8 @@ EmbeddedWorkerInstance::~EmbeddedWorkerInstance() {
void EmbeddedWorkerInstance::Start(mojom::EmbeddedWorkerStartParamsPtr params, void EmbeddedWorkerInstance::Start(mojom::EmbeddedWorkerStartParamsPtr params,
ProviderInfoGetter provider_info_getter, ProviderInfoGetter provider_info_getter,
StatusCallback callback) { StatusCallback callback) {
DCHECK(context_);
restart_count_++; restart_count_++;
if (!context_) {
std::move(callback).Run(blink::ServiceWorkerStatusCode::kErrorAbort);
// |this| may be destroyed by the callback.
return;
}
DCHECK_EQ(EmbeddedWorkerStatus::STOPPED, status_); DCHECK_EQ(EmbeddedWorkerStatus::STOPPED, status_);
DCHECK(!params->pause_after_download || !params->is_installed); DCHECK(!params->pause_after_download || !params->is_installed);
...@@ -776,7 +772,7 @@ void EmbeddedWorkerInstance::OnWorkerVersionDoomed() { ...@@ -776,7 +772,7 @@ void EmbeddedWorkerInstance::OnWorkerVersionDoomed() {
} }
void EmbeddedWorkerInstance::OnThreadStarted(int thread_id) { void EmbeddedWorkerInstance::OnThreadStarted(int thread_id) {
if (!context_ || !inflight_start_task_) if (!inflight_start_task_)
return; return;
starting_phase_ = THREAD_STARTED; starting_phase_ = THREAD_STARTED;
......
...@@ -1484,6 +1484,7 @@ void ServiceWorkerVersion::DidEnsureLiveRegistrationForStartWorker( ...@@ -1484,6 +1484,7 @@ void ServiceWorkerVersion::DidEnsureLiveRegistrationForStartWorker(
} }
void ServiceWorkerVersion::StartWorkerInternal() { void ServiceWorkerVersion::StartWorkerInternal() {
DCHECK(context_);
DCHECK_EQ(EmbeddedWorkerStatus::STOPPED, running_status()); DCHECK_EQ(EmbeddedWorkerStatus::STOPPED, running_status());
DCHECK(inflight_requests_.IsEmpty()); DCHECK(inflight_requests_.IsEmpty());
DCHECK(request_timeouts_.empty()); DCHECK(request_timeouts_.empty());
......
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