content/browser/service_worker/embedded_worker_instance.cc - Issue 2039743003: Introduce ServiceWorker.ActivatedWorkerPreparationForMainFrame.Time UMA.

Unified Diff: content/browser/service_worker/embedded_worker_instance.cc

Issue 2039743003: Introduce ServiceWorker.ActivatedWorkerPreparationForMainFrame.Time UMA. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master

Patch Set: incorporated falken's comment Created 4 years, 6 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Download patch

« no previous file with comments | « content/browser/service_worker/embedded_worker_instance.h ('k') | content/browser/service_worker/embedded_worker_instance_unittest.cc » ('j') | content/browser/service_worker/service_worker_url_request_job.cc » ('J')
Expand Comments ('e') | Collapse Comments ('c') | Hide Comments ('s')

Index: content/browser/service_worker/embedded_worker_instance.cc

diff --git a/content/browser/service_worker/embedded_worker_instance.cc b/content/browser/service_worker/embedded_worker_instance.cc

index f6f463883623462f21342bbab0df5e2306527c87..45edd7a4718c4cd728554d11573b2f679f122aac 100644

--- a/content/browser/service_worker/embedded_worker_instance.cc

+++ b/content/browser/service_worker/embedded_worker_instance.cc

@@ -14,6 +14,7 @@

#include "base/trace_event/trace_event.h"

#include "content/browser/devtools/service_worker_devtools_manager.h"

#include "content/browser/service_worker/embedded_worker_registry.h"

+#include "content/browser/service_worker/embedded_worker_status.h"

#include "content/browser/service_worker/service_worker_context_core.h"

#include "content/common/content_switches_internal.h"

#include "content/common/mojo/service_registry_impl.h"

@@ -216,7 +217,7 @@ class EmbeddedWorkerInstance::StartTask {

: instance_(instance),

state_(ProcessAllocationState::NOT_ALLOCATED),

is_installed_(false),

- start_situation_(ServiceWorkerMetrics::StartSituation::UNKNOWN),

+ started_during_browser_startup_(false),

weak_factory_(this) {

TRACE_EVENT_ASYNC_BEGIN1("ServiceWorker", "EmbeddedWorkerInstance::Start",

this, "Script", script_url.spec());

@@ -263,7 +264,7 @@ class EmbeddedWorkerInstance::StartTask {

is_installed_ = params->is_installed;

if (!GetContentClient()->browser()->IsBrowserStartupComplete())

- start_situation_ = ServiceWorkerMetrics::StartSituation::DURING_STARTUP;

+ started_during_browser_startup_ = true;

GURL scope(params->scope);

GURL script_url(params->script_url);

@@ -288,9 +289,6 @@ class EmbeddedWorkerInstance::StartTask {

}

bool is_installed() const { return is_installed_; }

- ServiceWorkerMetrics::StartSituation start_situation() const {

- return start_situation_;

- }

private:

void OnProcessAllocated(

@@ -319,19 +317,22 @@ class EmbeddedWorkerInstance::StartTask {

if (is_installed_)

ServiceWorkerMetrics::RecordProcessCreated(is_new_process);

- if (start_situation_ == ServiceWorkerMetrics::StartSituation::UNKNOWN) {

- if (is_new_process)

- start_situation_ = ServiceWorkerMetrics::StartSituation::NEW_PROCESS;

- else

- start_situation_ =

- ServiceWorkerMetrics::StartSituation::EXISTING_PROCESS;

- }

+ ServiceWorkerMetrics::StartSituation start_situation =

+ ServiceWorkerMetrics::StartSituation::UNKNOWN;

+ if (started_during_browser_startup_)

+ start_situation = ServiceWorkerMetrics::StartSituation::DURING_STARTUP;

+ else if (is_new_process)

+ start_situation = ServiceWorkerMetrics::StartSituation::NEW_PROCESS;

+ else

+ start_situation = ServiceWorkerMetrics::StartSituation::EXISTING_PROCESS;

// Notify the instance that a process is allocated.

state_ = ProcessAllocationState::ALLOCATED;

- instance_->OnProcessAllocated(base::WrapUnique(new WorkerProcessHandle(

- instance_->context_, instance_->embedded_worker_id(), process_id,

- is_new_process)));

+ instance_->OnProcessAllocated(

+ base::WrapUnique(new WorkerProcessHandle(

+ instance_->context_, instance_->embedded_worker_id(), process_id,

+ is_new_process)),

+ start_situation);

// TODO(bengr): Support changes to this setting while the worker

// is running.

@@ -399,7 +400,7 @@ class EmbeddedWorkerInstance::StartTask {

// Used for UMA.

bool is_installed_;

- ServiceWorkerMetrics::StartSituation start_situation_;

+ bool started_during_browser_startup_;

base::WeakPtrFactory<StartTask> weak_factory_;

@@ -412,7 +413,9 @@ bool EmbeddedWorkerInstance::Listener::OnMessageReceived(

}

EmbeddedWorkerInstance::~EmbeddedWorkerInstance() {

- DCHECK(status_ == STOPPING || status_ == STOPPED) << status_;

+ DCHECK(status_ == EmbeddedWorkerStatus::STOPPING ||

+ status_ == EmbeddedWorkerStatus::STOPPED)

+ << static_cast<int>(status_);

devtools_proxy_.reset();

if (registry_->GetWorker(embedded_worker_id_))

registry_->RemoveWorker(process_id(), embedded_worker_id_);

@@ -427,12 +430,12 @@ void EmbeddedWorkerInstance::Start(

// |this| may be destroyed by the callback.

return;

}

- DCHECK(status_ == STOPPED);

+ DCHECK(status_ == EmbeddedWorkerStatus::STOPPED);

DCHECK(!params->pause_after_download || !params->is_installed);

DCHECK_NE(kInvalidServiceWorkerVersionId, params->service_worker_version_id);

step_time_ = base::TimeTicks::Now();

- status_ = STARTING;

+ status_ = EmbeddedWorkerStatus::STARTING;

starting_phase_ = ALLOCATING_PROCESS;

network_accessed_for_script_ = false;

service_registry_.reset(new ServiceRegistryImpl());

@@ -448,7 +451,9 @@ void EmbeddedWorkerInstance::Start(

}

ServiceWorkerStatusCode EmbeddedWorkerInstance::Stop() {

- DCHECK(status_ == STARTING || status_ == RUNNING) << status_;

+ DCHECK(status_ == EmbeddedWorkerStatus::STARTING ||

+ status_ == EmbeddedWorkerStatus::RUNNING)

+ << static_cast<int>(status_);

// Abort an inflight start task.

inflight_start_task_.reset();

@@ -464,7 +469,7 @@ ServiceWorkerStatusCode EmbeddedWorkerInstance::Stop() {

return status;

}

- status_ = STOPPING;

+ status_ = EmbeddedWorkerStatus::STOPPING;

FOR_EACH_OBSERVER(Listener, listener_list_, OnStopping());

return status;

}

@@ -481,22 +486,28 @@ void EmbeddedWorkerInstance::StopIfIdle() {

ServiceWorkerStatusCode EmbeddedWorkerInstance::SendMessage(

const IPC::Message& message) {

DCHECK_NE(kInvalidEmbeddedWorkerThreadId, thread_id_);

- if (status_ != RUNNING && status_ != STARTING)

+ if (status_ != EmbeddedWorkerStatus::RUNNING &&

+ status_ != EmbeddedWorkerStatus::STARTING) {

return SERVICE_WORKER_ERROR_IPC_FAILED;

+ }

return registry_->Send(process_id(),

new EmbeddedWorkerContextMsg_MessageToWorker(

thread_id_, embedded_worker_id_, message));

}

void EmbeddedWorkerInstance::ResumeAfterDownload() {

- if (process_id() == ChildProcessHost::kInvalidUniqueID || status_ != STARTING)

+ if (process_id() == ChildProcessHost::kInvalidUniqueID ||

+ status_ != EmbeddedWorkerStatus::STARTING) {

return;

+ }

registry_->Send(process_id(), new EmbeddedWorkerMsg_ResumeAfterDownload(

embedded_worker_id_));

}

ServiceRegistry* EmbeddedWorkerInstance::GetServiceRegistry() {

- DCHECK(status_ == STARTING || status_ == RUNNING) << status_;

+ DCHECK(status_ == EmbeddedWorkerStatus::STARTING ||

+ status_ == EmbeddedWorkerStatus::RUNNING)

+ << static_cast<int>(status_);

return service_registry_.get();

}

@@ -506,7 +517,7 @@ EmbeddedWorkerInstance::EmbeddedWorkerInstance(

: context_(context),

registry_(context->embedded_worker_registry()),

embedded_worker_id_(embedded_worker_id),

- status_(STOPPED),

+ status_(EmbeddedWorkerStatus::STOPPED),

starting_phase_(NOT_STARTING),

thread_id_(kInvalidEmbeddedWorkerThreadId),

devtools_attached_(false),

@@ -514,12 +525,14 @@ EmbeddedWorkerInstance::EmbeddedWorkerInstance(

weak_factory_(this) {}

void EmbeddedWorkerInstance::OnProcessAllocated(

- std::unique_ptr<WorkerProcessHandle> handle) {

- DCHECK_EQ(STARTING, status_);

+ std::unique_ptr<WorkerProcessHandle> handle,

+ ServiceWorkerMetrics::StartSituation start_situation) {

+ DCHECK_EQ(EmbeddedWorkerStatus::STARTING, status_);

DCHECK(!process_handle_);

process_handle_ = std::move(handle);

starting_phase_ = REGISTERING_TO_DEVTOOLS;

+ start_situation_ = start_situation;

FOR_EACH_OBSERVER(Listener, listener_list_, OnProcessAllocated());

}

@@ -543,8 +556,8 @@ void EmbeddedWorkerInstance::OnStartWorkerMessageSent() {

if (!step_time_.is_null()) {

base::TimeDelta duration = UpdateStepTime();

if (inflight_start_task_->is_installed()) {

- ServiceWorkerMetrics::RecordTimeToSendStartWorker(

- duration, inflight_start_task_->start_situation());

+ ServiceWorkerMetrics::RecordTimeToSendStartWorker(duration,

+ start_situation_);

}

@@ -586,8 +599,7 @@ void EmbeddedWorkerInstance::OnScriptLoaded() {

if (!step_time_.is_null()) {

base::TimeDelta duration = UpdateStepTime();

- ServiceWorkerMetrics::RecordTimeToLoad(

- duration, source, inflight_start_task_->start_situation());

+ ServiceWorkerMetrics::RecordTimeToLoad(duration, source, start_situation_);

}

starting_phase_ = SCRIPT_LOADED;

@@ -603,8 +615,7 @@ void EmbeddedWorkerInstance::OnURLJobCreatedForMainScript() {

if (!step_time_.is_null()) {

base::TimeDelta duration = UpdateStepTime();

if (inflight_start_task_->is_installed())

- ServiceWorkerMetrics::RecordTimeToURLJob(

- duration, inflight_start_task_->start_situation());

+ ServiceWorkerMetrics::RecordTimeToURLJob(duration, start_situation_);

}

@@ -628,8 +639,7 @@ void EmbeddedWorkerInstance::OnThreadStarted(int thread_id) {

if (!step_time_.is_null()) {

base::TimeDelta duration = UpdateStepTime();

if (inflight_start_task_->is_installed())

- ServiceWorkerMetrics::RecordTimeToStartThread(

- duration, inflight_start_task_->start_situation());

+ ServiceWorkerMetrics::RecordTimeToStartThread(duration, start_situation_);

}

thread_id_ = thread_id;

@@ -659,7 +669,7 @@ void EmbeddedWorkerInstance::OnScriptLoadFailed() {

void EmbeddedWorkerInstance::OnScriptEvaluated(bool success) {

if (!inflight_start_task_)

return;

- DCHECK_EQ(STARTING, status_);

+ DCHECK_EQ(EmbeddedWorkerStatus::STARTING, status_);

TRACE_EVENT_ASYNC_STEP_PAST1("ServiceWorker", "EmbeddedWorkerInstance::Start",

inflight_start_task_.get(), "OnScriptEvaluated",

@@ -668,8 +678,8 @@ void EmbeddedWorkerInstance::OnScriptEvaluated(bool success) {

if (!step_time_.is_null()) {

base::TimeDelta duration = UpdateStepTime();

if (success && inflight_start_task_->is_installed())

- ServiceWorkerMetrics::RecordTimeToEvaluateScript(

- duration, inflight_start_task_->start_situation());

+ ServiceWorkerMetrics::RecordTimeToEvaluateScript(duration,

+ start_situation_);

}

base::WeakPtr<EmbeddedWorkerInstance> weak_this = weak_factory_.GetWeakPtr();

@@ -682,22 +692,22 @@ void EmbeddedWorkerInstance::OnScriptEvaluated(bool success) {

void EmbeddedWorkerInstance::OnStarted() {

// Stop is requested before OnStarted is sent back from the worker.

- if (status_ == STOPPING)

+ if (status_ == EmbeddedWorkerStatus::STOPPING)

return;

- DCHECK(status_ == STARTING);

- status_ = RUNNING;

+ DCHECK(status_ == EmbeddedWorkerStatus::STARTING);

+ status_ = EmbeddedWorkerStatus::RUNNING;

inflight_start_task_.reset();

FOR_EACH_OBSERVER(Listener, listener_list_, OnStarted());

}

void EmbeddedWorkerInstance::OnStopped() {

- Status old_status = status_;

+ EmbeddedWorkerStatus old_status = status_;

ReleaseProcess();

FOR_EACH_OBSERVER(Listener, listener_list_, OnStopped(old_status));

}

void EmbeddedWorkerInstance::OnDetached() {

- Status old_status = status_;

+ EmbeddedWorkerStatus old_status = status_;

ReleaseProcess();

FOR_EACH_OBSERVER(Listener, listener_list_, OnDetached(old_status));

}

@@ -785,18 +795,18 @@ void EmbeddedWorkerInstance::ReleaseProcess() {

devtools_proxy_.reset();

process_handle_.reset();

- status_ = STOPPED;

+ status_ = EmbeddedWorkerStatus::STOPPED;

thread_id_ = kInvalidEmbeddedWorkerThreadId;

service_registry_.reset();

}

void EmbeddedWorkerInstance::OnStartFailed(const StatusCallback& callback,

ServiceWorkerStatusCode status) {

- Status old_status = status_;

+ EmbeddedWorkerStatus old_status = status_;

ReleaseProcess();

base::WeakPtr<EmbeddedWorkerInstance> weak_this = weak_factory_.GetWeakPtr();

callback.Run(status);

- if (weak_this && old_status != STOPPED)

+ if (weak_this && old_status != EmbeddedWorkerStatus::STOPPED)

FOR_EACH_OBSERVER(Listener, weak_this->listener_list_,

OnStopped(old_status));

}

@@ -812,25 +822,28 @@ base::TimeDelta EmbeddedWorkerInstance::UpdateStepTime() {

void EmbeddedWorkerInstance::AddMessageToConsole(ConsoleMessageLevel level,

const std::string& message) {

- if (status_ != RUNNING && status_ != STARTING)

+ if (status_ != EmbeddedWorkerStatus::RUNNING &&

+ status_ != EmbeddedWorkerStatus::STARTING) {

return;

+ }

registry_->Send(process_id(), new EmbeddedWorkerMsg_AddMessageToConsole(

embedded_worker_id_, level, message));

}

// static

-std::string EmbeddedWorkerInstance::StatusToString(Status status) {

+std::string EmbeddedWorkerInstance::StatusToString(

+ EmbeddedWorkerStatus status) {

switch (status) {

- case STOPPED:

+ case EmbeddedWorkerStatus::STOPPED:

return "STOPPED";

- case STARTING:

+ case EmbeddedWorkerStatus::STARTING:

return "STARTING";

- case RUNNING:

+ case EmbeddedWorkerStatus::RUNNING:

return "RUNNING";

- case STOPPING:

+ case EmbeddedWorkerStatus::STOPPING:

return "STOPPING";

}

- NOTREACHED() << status;

+ NOTREACHED();

falken 2016/06/07 06:20:30 curious: why remove the status output?

horo 2016/06/07 06:58:48 Done.

return std::string();

}