Add ref count to service workers for extension API.

content/browser/service_worker/service_worker_context_wrapper.cc

Index: content/browser/service_worker/service_worker_context_wrapper.cc
diff --git a/content/browser/service_worker/service_worker_context_wrapper.cc b/content/browser/service_worker/service_worker_context_wrapper.cc
index 64ae3ea762f36fabfff18ff825a3dd97d0accf63..2cec1fa6b5a32940b412683be6c811fcb69094c2 100644
--- a/content/browser/service_worker/service_worker_context_wrapper.cc
+++ b/content/browser/service_worker/service_worker_context_wrapper.cc
@@ -22,6 +22,7 @@
 #include "base/threading/sequenced_worker_pool.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "content/browser/renderer_host/render_process_host_impl.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_context_observer.h"
 #include "content/browser/service_worker/service_worker_process_manager.h"
@@ -825,6 +826,49 @@ void ServiceWorkerContextWrapper::ShutdownOnIO() {
   context_core_.reset();
 }
 
+bool ServiceWorkerContextWrapper::IncrementPendingActivity(
+    int64_t service_worker_version_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  ServiceWorkerVersion* version =
+      context()->GetLiveVersion(service_worker_version_id);
+  if (!version) {
+    DVLOG(1) << "ServiceWorkerVersion not found: " << service_worker_version_id;
+    return false;
+  }
+  if (version->running_status() != EmbeddedWorkerStatus::RUNNING) {
+    DVLOG(1) << "ServiceWorkerVersion is not running: "
+             << service_worker_version_id;
+    return false;
+  }
+  int request_id = version->StartRequest(
+      ServiceWorkerMetrics::EventType::EXTERNAL_REQUEST,
+      base::Bind(&ServiceWorkerUtils::NoOpStatusCallback));

[falken, 2016/09/29 14:52:24]

Are you sure you don't need to use the error callback? It gets called if the
worker was stopped for whatever reason while we were trying to keep it alive.
Does the renderer always call DecrementPendingActivity to balance out the
IncrementPendingActivity, even if the worker died? If not, one potentially bad
situation is:
(1) IncrementPendingActivity
(2) worker stops, requests are cancelled
(3) maybe sometime later we try again
(4) IncrementPendingActivity
(5) DecrementPendingActivity

The DecrementPendingActivity call would use the old request_id in
pending_external_requests_, but that would have no effect. Eventually the real
request would timeout and the worker gets terminated, and again
pending_external_requests_ holds an obsolete id.

Since the renderer could always crash leaving an Increment call unbalanced, I
guess it's possible this situation can occur.

[lazyboy, 2016/09/29 18:07:19]

(I was thinking that since the worker is running, StartRequest won't fail.)

Added cleaning up |pending_external_requests_| on error_callback.

+  pending_external_requests_[service_worker_version_id].insert(request_id);
+  return true;
+}
+
+bool ServiceWorkerContextWrapper::DecrementPendingActivity(
+    int64_t service_worker_version_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  ServiceWorkerVersion* version =
+      context()->GetLiveVersion(service_worker_version_id);
+
+  std::set<int>& pending_requests =
+      pending_external_requests_[service_worker_version_id];
+  DCHECK(pending_requests.size() > 0);
+  int request_id = *pending_requests.begin();

[falken, 2016/09/29 14:52:24]

It seems like pending_requests should just be a queue instead of a set. It looks
like we're subtly depending on requests ids to be monotonically increasing here,
in order for the oldest pending request to be the one removed. This can matter
due to the timeout mechanism: long-running requests will be timed out and the
service worker terminated.

[lazyboy, 2016/09/29 18:07:19]

Done.

 It looks

+  pending_requests.erase(request_id);
+
+  // The service worker should be present and running.
+  if (!version || version->running_status() != EmbeddedWorkerStatus::RUNNING) {
+    NOTREACHED()
+        << "Failed to decrement pending activity of ServiceWorkerVersion: "
+        << service_worker_version_id;
+    return false;
+  }
+  return version->FinishRequest(request_id, true, base::Time::Now());
+}
+
 void ServiceWorkerContextWrapper::DidDeleteAndStartOver(
     ServiceWorkerStatusCode status) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);