Stop Service Workers that execute JavaScript for too long.

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/stl_util.h"
 #include "base/strings/string16.h"
@@ skipping 66 matching lines @@
 // (Note that if all references to the version is dropped the worker
 // is also stopped without delay)
 const int64 kStopWorkerDelay = 30;  // 30 secs.
 
 // Delay for attempting to stop a doomed worker with in-flight requests.
 const int64 kStopDoomedWorkerDelay = 5;  // 5 secs.
 
 // Default delay for scheduled update.
 const int kUpdateDelaySeconds = 1;
 
+// Delay between sending pings to the worker.
+const int kPingIntervalTime = 10;  // 10 secs.
+
+// Timeout for waiting for a response to a ping.
+const int kPingTimeoutTime = 30;  // 30 secs.
+
 const char kClaimClientsStateErrorMesage[] =
     "Only the active worker can claim clients.";
 
 const char kClaimClientsShutdownErrorMesage[] =
     "Failed to claim clients due to Service Worker system shutdown.";
 
 void RunSoon(const base::Closure& callback) {
   if (!callback.is_null())
     base::MessageLoop::current()->PostTask(FROM_HERE, callback);
 }
@@ skipping 175 matching lines @@
     ServiceWorkerRegistration* registration,
     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_timed_out_(false),
       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);
@@ skipping 488 matching lines @@
 void ServiceWorkerVersion::Doom() {
   if (is_doomed_)
     return;
   is_doomed_ = true;
   if (!HasControllee())
     DoomInternal();
 }
 
 void ServiceWorkerVersion::SetDevToolsAttached(bool attached) {
   embedded_worker()->set_devtools_attached(attached);
-  if (!attached && !stop_worker_timer_.IsRunning()) {
-    // If devtools is detached from this version and stop-worker-timer is not
-    // running, try scheduling stop-worker-timer now.
+  if (attached)
+    return;
+  // If devtools is detached try scheduling the timers for stopping the worker
+  // now.
+  if (!stop_worker_timer_.IsRunning())
     ScheduleStopWorker();
-  }
+  if (!ping_worker_timer_.IsRunning())
+    SchedulePingWorker();
 }
 
 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();
 }
 
+void ServiceWorkerVersion::OnScriptLoaded() {
+  DCHECK_EQ(STARTING, running_status());
+  SchedulePingWorker();

[michaeln, 2015/02/11 01:02:21]

Would it make sense to only turn the ping monitor on when HasInflightRequests()?
Might help avoid context switches and lighting up otherwise idle processes.

[falken, 2015/02/11 16:10:24]

That's a good thing to think about. In practice, ping will essentially only
happen in that case anyway since the stop_worker_timer_ will kill a worker that
doesn't have in-flight requests. Additionally, we want ping to kill a worker
that's taking too long to startup, which doesn't currently count as an in-flight
request.

So I'm thinking it should be OK as coded. It might be possible to fold the stop
timer and ping timer into one, or have more explicit coupling between the two,
but they are kind of two different forces: the stop timer tries to kill "idle"
workers and ping tries to kill "busy" ones.

+}
+
 void ServiceWorkerVersion::OnStarted() {
   DCHECK_EQ(RUNNING, running_status());
   DCHECK(cache_listener_.get());
   ScheduleStopWorker();
 
   // Fire all start callbacks.
   scoped_refptr<ServiceWorkerVersion> protect(this);
   RunCallbacks(this, &start_callbacks_, SERVICE_WORKER_OK);
   FOR_EACH_OBSERVER(Listener, listeners_, OnWorkerStarted(this));
 }
 
 void ServiceWorkerVersion::OnStopped(
     EmbeddedWorkerInstance::Status old_status) {
   DCHECK_EQ(STOPPED, running_status());
   scoped_refptr<ServiceWorkerVersion> protect(this);
 
   bool should_restart = !is_doomed() && !start_callbacks_.empty() &&
                         (old_status != EmbeddedWorkerInstance::STARTING);
 
+  ping_worker_timer_.Stop();
+  if (ping_timed_out_)
+    should_restart = false;
+
   // Fire all stop callbacks.
   RunCallbacks(this, &stop_callbacks_, SERVICE_WORKER_OK);
 
   if (!should_restart) {
     // Let all start callbacks fail.
     RunCallbacks(this, &start_callbacks_,
                  SERVICE_WORKER_ERROR_START_WORKER_FAILED);
   }
 
   // Let all message callbacks fail (this will also fire and clear all
@@ skipping 89 matching lines @@
     IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_OpenWindow,
                         OnOpenWindow)
     IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_PostMessageToDocument,
                         OnPostMessageToDocument)
     IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_FocusClient,
                         OnFocusClient)
     IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_SkipWaiting,
                         OnSkipWaiting)
     IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_ClaimClients,
                         OnClaimClients)
+    IPC_MESSAGE_HANDLER(ServiceWorkerHostMsg_Pong, OnPongFromWorker)
     IPC_MESSAGE_UNHANDLED(handled = false)
   IPC_END_MESSAGE_MAP()
   return handled;
 }
 
 void ServiceWorkerVersion::OnStartMessageSent(
     ServiceWorkerStatusCode status) {
   if (status != SERVICE_WORKER_OK)
     RunCallbacks(this, &start_callbacks_, status);
 }
@@ skipping 370 matching lines @@
 
   ServiceWorkerRegistration* registration =
       context_->GetLiveRegistration(registration_id_);
   if (!registration) {
     callback.Run(SERVICE_WORKER_ERROR_ABORT);
     return;
   }
   registration->ClaimClients(callback);
 }
 
+void ServiceWorkerVersion::OnPongFromWorker() {
+  ping_worker_timer_.Stop();
+  SchedulePingWorker();
+}
+
 void ServiceWorkerVersion::DidClaimClients(
     int request_id, ServiceWorkerStatusCode status) {
   if (status == SERVICE_WORKER_ERROR_STATE) {
     embedded_worker_->SendMessage(ServiceWorkerMsg_ClaimClientsError(
         request_id, blink::WebServiceWorkerError::ErrorTypeState,
         base::ASCIIToUTF16(kClaimClientsStateErrorMesage)));
     return;
   }
   if (status == SERVICE_WORKER_ERROR_ABORT) {
     embedded_worker_->SendMessage(ServiceWorkerMsg_ClaimClientsError(
@@ skipping 18 matching lines @@
 
   // We can get info for a frame that was navigating end ended up with a
   // different URL than expected. In such case, we should make sure to not
   // expose cross-origin WindowClient.
   if (info.url.GetOrigin() != script_url_.GetOrigin())
     return;
 
   callback->AddClientInfo(client_id, info);
 }
 
+void ServiceWorkerVersion::PingWorker() {
+  if (running_status() != STARTING && running_status() != RUNNING)
+    return;
+  ServiceWorkerStatusCode status =
+      embedded_worker_->SendMessage(ServiceWorkerMsg_Ping());
+  if (status != SERVICE_WORKER_OK) {
+    OnPingTimeout();
+    return;
+  }
+  ping_worker_timer_.Start(FROM_HERE,
+                           base::TimeDelta::FromSeconds(kPingTimeoutTime),
+                           base::Bind(&ServiceWorkerVersion::OnPingTimeout,
+                                      weak_factory_.GetWeakPtr()));
+}
+
+void ServiceWorkerVersion::SchedulePingWorker() {
+  DCHECK(!ping_worker_timer_.IsRunning());
+  ping_timed_out_ = false;
+  ping_worker_timer_.Start(FROM_HERE,
+                           base::TimeDelta::FromSeconds(kPingIntervalTime),
+                           base::Bind(&ServiceWorkerVersion::PingWorker,
+                                      weak_factory_.GetWeakPtr()));
+}
+
+void ServiceWorkerVersion::OnPingTimeout() {
+  if (running_status() != STARTING && running_status() != RUNNING)
+    return;
+  ping_timed_out_ = true;
+  StopWorkerIfIdle();

[kinuko, 2015/02/10 09:15:52]

In this case we should probably show some error or warning to the developer.
Should we file a bug and add a comment for that?

[falken, 2015/02/11 16:10:24]

Yes I agree. I don't know a good place we'd show the error... maybe SW DevTools,
but if that's open we won't kill the worker.

+}
+
 void ServiceWorkerVersion::ScheduleStopWorker() {
   if (running_status() != RUNNING)
     return;
   stop_worker_timer_.Stop();
   stop_worker_timer_.Start(
       FROM_HERE, base::TimeDelta::FromSeconds(
           is_doomed_ ? kStopDoomedWorkerDelay : kStopWorkerDelay),
       base::Bind(&ServiceWorkerVersion::StopWorkerIfIdle,
                  weak_factory_.GetWeakPtr()));
 }
 
 void ServiceWorkerVersion::StopWorkerIfIdle() {
-  // Reschedule the stop the worker while there're inflight requests.
-  // (Note: we'll probably need to revisit this so that we can kill 'bad' SW.
-  // See https://github.com/slightlyoff/ServiceWorker/issues/527)
-  if (HasInflightRequests()) {
+  if (HasInflightRequests() && !ping_timed_out_) {
     ScheduleStopWorker();
     return;
   }
   if (running_status() == STOPPED || running_status() == STOPPING ||
       !stop_callbacks_.empty()) {
     return;
   }
   embedded_worker_->StopIfIdle();
 }
 
@@ skipping 28 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