Stop ServiceWorker context when no controllee is associated (and when all refs are dropped)

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/stl_util.h"
 #include "base/strings/string16.h"
 #include "content/browser/service_worker/embedded_worker_instance.h"
 #include "content/browser/service_worker/embedded_worker_registry.h"
 #include "content/browser/service_worker/service_worker_context_core.h"
 #include "content/browser/service_worker/service_worker_registration.h"
+#include "content/browser/service_worker/service_worker_utils.h"
 #include "content/common/service_worker/service_worker_messages.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/common/content_switches.h"
 
 namespace content {
 
 typedef ServiceWorkerVersion::StatusCallback StatusCallback;
 typedef ServiceWorkerVersion::MessageCallback MessageCallback;
 
 namespace {
 
+// Default delay to stop the worker context after all documents that
+// are associated to the worker are closed.
+// (Note that if all references to the version is dropped the worker
+// is also stopped without delay)
+const int64 kStopWorkerDelay = 5 * 60;  // 5 mins.

[Jeffrey Yasskin, 2014/05/15 19:16:44]

We should probably make this faster. Extension/App event pages die after 15
seconds of no events, which indicates that a short-ish timeout works for some
people. The counter-indication is that extension/app authors can extend their
event page's lifetime by calling various callback APIs, which we don't want to
allow in SWs. Even so, we don't really want someone using 5 minutes of CPU after
their last tab is closed or they handle the most recent event.

We're also going to need to make this delay configurable from devtools.

[kinuko, 2014/05/16 04:13:51]

I don't have a strong opinion on this delay for now, let me change this to
default to 30 secs for now.
Let us do this in a separate patch.

+
 void RunSoon(const base::Closure& callback) {
   if (!callback.is_null())
     base::MessageLoop::current()->PostTask(FROM_HERE, callback);
 }
 
 template <typename CallbackArray, typename Arg>
 void RunCallbacks(ServiceWorkerVersion* version,
                   CallbackArray* callbacks_ptr,
                   const Arg& arg) {
   CallbackArray callbacks;
@@ skipping 61 matching lines @@
     registration_id_ = registration->id();
     script_url_ = registration->script_url();
     scope_ = registration->pattern();
   }
   context_->AddLiveVersion(this);
   embedded_worker_ = context_->embedded_worker_registry()->CreateWorker();
   embedded_worker_->AddListener(this);
 }
 
 ServiceWorkerVersion::~ServiceWorkerVersion() {
-  if (embedded_worker_) {
-    embedded_worker_->RemoveListener(this);
-    embedded_worker_.reset();
+  if (running_status() == RUNNING) {
+    if (stop_worker_timer_.IsRunning())
+      stop_worker_timer_.Stop();

[michaeln, 2014/05/15 19:02:20]

base::Timer's destructor takes care of this

[kinuko, 2014/05/16 04:13:51]

Oh yeah that's handy...

   }
+  embedded_worker_->RemoveListener(this);
+  // EmbeddedWorker's dtor sends StopWorker if it's still running.
+  embedded_worker_.reset();

[michaeln, 2014/05/15 19:02:20]

is the call to .reset() needed for ordering reasons, if not might be nice to
remove it.

can this method could be simplified to...

ServiceWorkerVersion::~ServiceWorkerVersion() {
  embedded_worker_->RemoveListener(this);
  if (context_)
    context_->RemoveLiveVersion(version_id_);
}

[kinuko, 2014/05/16 04:13:51]

Done.

   if (context_)
     context_->RemoveLiveVersion(version_id_);
 }
 
 void ServiceWorkerVersion::SetStatus(Status status) {
   if (status_ == status)
     return;
 
   status_ = status;
 
@@ skipping 21 matching lines @@
                                   embedded_worker()->thread_id());
 }
 
 void ServiceWorkerVersion::StartWorker(const StatusCallback& callback) {
   StartWorkerWithCandidateProcesses(std::vector<int>(), callback);
 }
 
 void ServiceWorkerVersion::StartWorkerWithCandidateProcesses(
     const std::vector<int>& possible_process_ids,
     const StatusCallback& callback) {
-  DCHECK(embedded_worker_);
   switch (running_status()) {
     case RUNNING:
       RunSoon(base::Bind(callback, SERVICE_WORKER_OK));
       return;
     case STOPPING:
       RunSoon(base::Bind(callback, SERVICE_WORKER_ERROR_START_WORKER_FAILED));
       return;
     case STOPPED:
     case STARTING:
       start_callbacks_.push_back(callback);
       if (running_status() == STOPPED) {
         embedded_worker_->Start(
             version_id_,
             scope_,
             script_url_,
             possible_process_ids,
             base::Bind(&ServiceWorkerVersion::RunStartWorkerCallbacksOnError,
                        weak_factory_.GetWeakPtr()));
       }
       return;
   }
 }
 
 void ServiceWorkerVersion::StopWorker(const StatusCallback& callback) {
-  DCHECK(embedded_worker_);
   if (running_status() == STOPPED) {
     RunSoon(base::Bind(callback, SERVICE_WORKER_OK));
     return;
   }
   if (stop_callbacks_.empty()) {
     ServiceWorkerStatusCode status = embedded_worker_->Stop();
     if (status != SERVICE_WORKER_OK) {
       RunSoon(base::Bind(callback, status));
       return;
     }
   }
   stop_callbacks_.push_back(callback);
 }
 
 void ServiceWorkerVersion::SendMessage(
     const IPC::Message& message, const StatusCallback& callback) {
-  DCHECK(embedded_worker_);
   if (running_status() != RUNNING) {
     // Schedule calling this method after starting the worker.
     StartWorker(base::Bind(&RunTaskAfterStartWorker,
                            weak_factory_.GetWeakPtr(), callback,
                            base::Bind(&self::SendMessage,
                                       weak_factory_.GetWeakPtr(),
                                       message, callback)));
     return;
   }
 
@@ skipping 107 matching lines @@
 bool ServiceWorkerVersion::HasProcessToRun() const {
   return embedded_worker_->HasProcessToRun();
 }
 
 void ServiceWorkerVersion::AddControllee(
     ServiceWorkerProviderHost* provider_host) {
   DCHECK(!ContainsKey(controllee_map_, provider_host));
   int controllee_id = controllee_by_id_.Add(provider_host);
   controllee_map_[provider_host] = controllee_id;
   AddProcessToWorker(provider_host->process_id());
+  if (stop_worker_timer_.IsRunning())
+    stop_worker_timer_.Stop();
 }
 
 void ServiceWorkerVersion::RemoveControllee(
     ServiceWorkerProviderHost* provider_host) {
   ControlleeMap::iterator found = controllee_map_.find(provider_host);
   DCHECK(found != controllee_map_.end());
   controllee_by_id_.Remove(found->second);
   controllee_map_.erase(found);
   RemoveProcessFromWorker(provider_host->process_id());
+  if (!HasControllee())
+    ScheduleStopWorker();

[Jeffrey Yasskin, 2014/05/15 19:16:44]

No need to handle this in this CL, but we're going to need non-controllee events
like 'push' and 'sync' to be able to keep a SW alive too.

[kinuko, 2014/05/16 04:13:51]

Yeah we could add similar code in other events / upon completion of event
handlers too.

   // TODO(kinuko): Fire NoControllees notification when the # of controllees
   // reaches 0, so that a new pending version can be activated (which will
   // deactivate this version).
   // TODO(michaeln): On no controllees call storage DeleteVersionResources
   // if this version has been deactivated. Probably storage can listen for
   // NoControllees for versions that have been deleted.
 }
 
 void ServiceWorkerVersion::AddPendingControllee(
     ServiceWorkerProviderHost* provider_host) {
@@ skipping 218 matching lines @@
     const std::vector<int>& sent_message_port_ids) {
   ServiceWorkerProviderHost* provider_host =
       controllee_by_id_.Lookup(client_id);
   if (!provider_host) {
     // The client may already have been closed, just ignore.
     return;
   }
   provider_host->PostMessage(message, sent_message_port_ids);
 }
 
+void ServiceWorkerVersion::ScheduleStopWorker() {
+  if (running_status() != RUNNING)
+    return;
+  if (stop_worker_timer_.IsRunning()) {
+    stop_worker_timer_.Reset();

[michaeln, 2014/05/15 19:02:20]

nit: maybe prefer early return here

[kinuko, 2014/05/16 04:13:51]

Done.

+  } else {
+    stop_worker_timer_.Start(
+        FROM_HERE, base::TimeDelta::FromSeconds(kStopWorkerDelay),
+        base::Bind(&ServiceWorkerVersion::StopWorker,
+                   weak_factory_.GetWeakPtr(),
+                   base::Bind(&ServiceWorkerUtils::NoOpStatusCallback)));
+  }
+}
+
 }  // namespace content