ServiceWorker: Don't start the timeout timer when DevTools is detached

content/browser/service_worker/service_worker_version.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "content/browser/service_worker/service_worker_version.h"
 
 #include "base/command_line.h"
 #include "base/memory/ref_counted.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/stl_util.h"
@@ skipping 289 matching lines @@
     const GURL& script_url,
     int64 version_id,
     base::WeakPtr<ServiceWorkerContextCore> context)
     : version_id_(version_id),
       registration_id_(kInvalidServiceWorkerVersionId),
       script_url_(script_url),
       status_(NEW),
       context_(context),
       script_cache_map_(this, context),
       ping_state_(NOT_PINGING),
-      is_doomed_(false),
-      skip_waiting_(false),
       weak_factory_(this) {
   DCHECK(context_);
   DCHECK(registration);
   if (registration) {
     registration_id_ = registration->id();
     scope_ = registration->pattern();
   }
   context_->AddLiveVersion(this);
   embedded_worker_ = context_->embedded_worker_registry()->CreateWorker();
   embedded_worker_->AddListener(this);
@@ skipping 459 matching lines @@
     DoomInternal();
 }
 
 void ServiceWorkerVersion::SetDevToolsAttached(bool attached) {
   embedded_worker()->set_devtools_attached(attached);
   if (attached) {
     // Set to null time so we don't record the startup time metric.
     ClearTick(&start_time_);
     return;
   }
-  if (!timeout_timer_.IsRunning())
-    StartTimeoutTimer();
+  if (timeout_timer_.IsRunning() && running_status() == STARTING) {
+    // Reactivate the timer for start timeout.
+    RestartTick(&start_time_);

[falken, 2015/03/13 07:14:10]

The start timeout sometimes should be in effect for status STOPPING also. This
is because "StartWorker(); StopWorker();" results in a worker with status
STOPPING that may be in the process of starting up.

[nhiroki, 2015/03/16 01:58:02]

Thank you, I didn't notice it. Tweaked the condition of the if-statement.

+
+    // Once the DevTools is attached, we cannot record an accurate startup time.
+    skip_recording_startup_time_ = true;

[falken, 2015/03/13 07:14:10]

This line and line 785-786 essentially do the same thing, so should be combined
somehow or commented to explain the difference. The current comment is a bit
weird since this runs when DevTools is detached.

[nhiroki, 2015/03/16 01:58:02]

Exactly. Merged this into line 785.

+  }
 }
 
 void ServiceWorkerVersion::SetMainScriptHttpResponseInfo(
     const net::HttpResponseInfo& http_info) {
   main_script_http_info_.reset(new net::HttpResponseInfo(http_info));
 }
 
 const net::HttpResponseInfo*
 ServiceWorkerVersion::GetMainScriptHttpResponseInfo() {
   return main_script_http_info_.get();
@@ skipping 76 matching lines @@
   // the listener prevents any pending completion callbacks from causing
   // messages to be sent to the stopped worker.
   cache_listener_.reset();
 
   // Restart worker if we have any start callbacks and the worker isn't doomed.
   if (should_restart) {
     if (embedded_worker_->devtools_attached())
       ClearTick(&start_time_);
     else
       RestartTick(&start_time_);
+    skip_recording_startup_time_ = false;
     cache_listener_.reset(new ServiceWorkerCacheListener(this, context_));
     embedded_worker_->Start(
         version_id_, scope_, script_url_, false /* pause_after_download */,
         base::Bind(&ServiceWorkerVersion::OnStartSentAndScriptEvaluated,
                    weak_factory_.GetWeakPtr()));
   }
 }
 
 void ServiceWorkerVersion::OnReportException(
     const base::string16& error_message,
@@ skipping 543 matching lines @@
       ServiceWorkerMsg_DidGetClients(request_id, clients));
 }
 
 void ServiceWorkerVersion::StartTimeoutTimer() {
   DCHECK(!timeout_timer_.IsRunning());
 
   if (embedded_worker_->devtools_attached())
     ClearTick(&start_time_);
   else
     RestartTick(&start_time_);
+  skip_recording_startup_time_ = false;
   start_callbacks_.push_back(
       base::Bind(&ServiceWorkerVersion::RecordStartWorkerResult,
                  weak_factory_.GetWeakPtr()));
 
   ClearTick(&idle_time_);
   ClearTick(&ping_time_);
   ping_state_ = NOT_PINGING;
 
   timeout_timer_.Start(FROM_HERE,
                        base::TimeDelta::FromSeconds(kTimeoutTimerDelaySeconds),
@@ skipping 99 matching lines @@
   base::TimeTicks start_time = start_time_;
   ClearTick(&start_time_);
 
   // Failing to start a doomed worker isn't interesting and very common when
   // update dooms because the script is byte-to-byte identical.
   if (is_doomed_)
     return;
 
   UMA_HISTOGRAM_ENUMERATION("ServiceWorker.StartWorker.Status", status,
                             SERVICE_WORKER_ERROR_MAX_VALUE);
-  if (status == SERVICE_WORKER_OK && !start_time.is_null()) {
+  if (status == SERVICE_WORKER_OK && !start_time.is_null() &&
+      !skip_recording_startup_time_) {
     UMA_HISTOGRAM_MEDIUM_TIMES("ServiceWorker.StartWorker.Time",
                                GetTickDuration(start_time));
   }
 
   if (status != SERVICE_WORKER_ERROR_TIMEOUT)
     return;
   EmbeddedWorkerInstance::StartingPhase phase =
       EmbeddedWorkerInstance::NOT_STARTING;
   EmbeddedWorkerInstance::Status running_status = embedded_worker_->status();
   // Build an artifical JavaScript exception to show in the ServiceWorker
@@ skipping 33 matching lines @@
     int request_id) {
   callbacks->Remove(request_id);
   if (is_doomed_) {
     // The stop should be already scheduled, but try to stop immediately, in
     // order to release worker resources soon.
     StopWorkerIfIdle();
   }
 }
 
 }  // namespace content