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

content/browser/service_worker/service_worker_version.cc

Index: content/browser/service_worker/service_worker_version.cc
diff --git a/content/browser/service_worker/service_worker_version.cc b/content/browser/service_worker/service_worker_version.cc
index 0bcec8983900ca78b5beb9bd25191707c4c7138b..0cda1187b4b5d71b95c8f6375a2c8a5699403eb2 100644
--- a/content/browser/service_worker/service_worker_version.cc
+++ b/content/browser/service_worker/service_worker_version.cc
@@ -11,6 +11,7 @@
 #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"
@@ -22,6 +23,12 @@ 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.
+
 void RunSoon(const base::Closure& callback) {
   if (!callback.is_null())
     base::MessageLoop::current()->PostTask(FROM_HERE, callback);
@@ -103,10 +110,13 @@ ServiceWorkerVersion::ServiceWorkerVersion(
 }
 
 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();
+    embedded_worker_->Stop();

[michaeln, 2014/05/14 19:44:23]

What happens when a running worker instance is deleted without having been
stopped? Seems like that should cause it to stop if its running.

[kinuko, 2014/05/14 21:56:30]

Yes, and this CL does so (stops the worker if it's running).

[kinuko, 2014/05/14 22:00:26]

(I could change this to call StopWorker() if it's clearer)

[michaeln, 2014/05/14 23:20:21]

i was referring to EmbeddedWorkerInstance class, seems like the smarts to stop
the remote thread on destruction could/should be in
EmbeddedWorkerInstance::~EmbeddedWorkerInstance()

[kinuko, 2014/05/14 23:50:06]

Hmm... ok will move the Stop part to EmbeddedWorkerInstance's dtor once I'm
online.

[kinuko, 2014/05/15 17:17:26]

Done.

   }
+  embedded_worker_->RemoveListener(this);
+  embedded_worker_.reset();
   if (context_)
     context_->RemoveLiveVersion(version_id_);
 }
@@ -148,7 +158,6 @@ void ServiceWorkerVersion::StartWorker(const StatusCallback& 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));
@@ -173,7 +182,6 @@ void ServiceWorkerVersion::StartWorkerWithCandidateProcesses(
 }
 
 void ServiceWorkerVersion::StopWorker(const StatusCallback& callback) {
-  DCHECK(embedded_worker_);
   if (running_status() == STOPPED) {
     RunSoon(base::Bind(callback, SERVICE_WORKER_OK));
     return;
@@ -190,7 +198,6 @@ void ServiceWorkerVersion::StopWorker(const StatusCallback& callback) {
 
 void ServiceWorkerVersion::SendMessage(
     const IPC::Message& message, const StatusCallback& callback) {
-  DCHECK(embedded_worker_);

[michaeln, 2014/05/14 19:44:23]

why remove these?

[kinuko, 2014/05/14 21:56:30]

Since we no longer reset embedded_worker_ anywhere other than in dtor.. (these
were added when we had Shutdown())).

[michaeln, 2014/05/14 23:20:21]

k... got it

   if (running_status() != RUNNING) {
     // Schedule calling this method after starting the worker.
     StartWorker(base::Bind(&RunTaskAfterStartWorker,
@@ -318,6 +325,8 @@ void ServiceWorkerVersion::AddControllee(
   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(
@@ -327,6 +336,8 @@ void ServiceWorkerVersion::RemoveControllee(
   controllee_by_id_.Remove(found->second);
   controllee_map_.erase(found);
   RemoveProcessFromWorker(provider_host->process_id());
+  if (!HasControllee())
+    ScheduleStopWorker();
   // 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).
@@ -565,4 +576,18 @@ void ServiceWorkerVersion::OnPostMessageToDocument(
   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();
+  } else {
+    stop_worker_timer_.Start(
+        FROM_HERE, base::TimeDelta::FromSeconds(kStopWorkerDelay),
+        base::Bind(&ServiceWorkerVersion::StopWorker,
+                   weak_factory_.GetWeakPtr(),
+                   base::Bind(&ServiceWorkerUtils::NoOpStatusCallback)));
+  }
+}
+
 }  // namespace content