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..a4b5856796664967dfbf0257c8f0cc3c64254359 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"
@@ -56,6 +57,8 @@ void WorkerStarted(const ServiceWorkerContextWrapper::StatusCallback& callback,
                           base::Bind(callback, status));
 }
 
+void DoNothing(ServiceWorkerStatusCode status) {}

[falken, 2016/09/23 02:27:35]

you can use ServiceWorkerUtils::NoOpStatusCallback

[lazyboy, 2016/09/27 21:26:45]

Done.

+
 void StartActiveWorkerOnIO(
     const ServiceWorkerContextWrapper::StatusCallback& callback,
     ServiceWorkerStatusCode status,
@@ -102,6 +105,16 @@ ServiceWorkerMetrics::EventType GetNavigationHintEventType(
 
 }  // namespace
 
+bool ServiceWorkerContextWrapper::IncrementPendingActivity(
+    int64_t service_worker_version_id) {
+  return IncrementPendingActivityOnIO(service_worker_version_id);

[falken, 2016/09/23 02:27:35]

Can the *OnIO functions just be removed?

[lazyboy, 2016/09/27 21:26:45]

Oh yes, these used to do thread hops, done.

+}
+
+bool ServiceWorkerContextWrapper::DecrementPendingActivity(
+    int64_t service_worker_version_id) {
+  return DecrementPendingActivityOnIO(service_worker_version_id);
+}
+
 void ServiceWorkerContext::AddExcludedHeadersForFetchEvent(
     const std::set<std::string>& header_names) {
   // TODO(pkasting): Remove ScopedTracker below once crbug.com/477117 is fixed.
@@ -825,6 +838,42 @@ void ServiceWorkerContextWrapper::ShutdownOnIO() {
   context_core_.reset();
 }
 
+bool ServiceWorkerContextWrapper::IncrementPendingActivityOnIO(
+    int64_t service_worker_version_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  ServiceWorkerVersion* version =
+      context()->GetLiveVersion(service_worker_version_id);
+  if (!version) {

[falken, 2016/09/23 02:27:35]

You should also confirm the version is RUNNING. It's possible the renderer is
confused/lying or the version started stopping right around the time of the IPC.

[lazyboy, 2016/09/27 21:26:45]

Done.

+    LOG(WARNING) << "ServiceWorkerVersion not found: "
+                 << service_worker_version_id;

[falken, 2016/09/23 02:27:35]

do you need these logs in production code? DVLOG(1)?

[lazyboy, 2016/09/27 21:26:45]

No, changed to DVLOG, thanks.

+    return false;
+  }
+  int request_id =
+      version->StartRequest(ServiceWorkerMetrics::EventType::EXTERNAL_REQUEST,
+                            base::Bind(&DoNothing));
+  pending_external_requests_[service_worker_version_id].insert(request_id);
+  return true;
+}
+
+bool ServiceWorkerContextWrapper::DecrementPendingActivityOnIO(
+    int64_t service_worker_version_id) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  ServiceWorkerVersion* version =
+      context()->GetLiveVersion(service_worker_version_id);
+  if (!version) {

[falken, 2016/09/23 02:27:35]

I'm not too sure when we get in this state: the renderer might have made a
mistake or the version might have been destructed due to some timing issue. We
should probably clean up the pending_external_requests  in this case though. Or
maybe the set of external requests should just be in ServiceWorkerVersion?

Also, the case where the version is not RUNNING should probably be handled.

[lazyboy, 2016/09/27 21:26:45]

Changed to DCHECK. This could uncover any timing issue we might have...

We
Done.

Or
Done.

+    LOG(WARNING) << "ServiceWorkerVersion not found: "
+                 << service_worker_version_id;
+    return false;
+  }
+  std::set<int>& pending_requests =
+      pending_external_requests_[service_worker_version_id];
+  DCHECK(pending_requests.size() > 0);
+  int request_id = *pending_requests.begin();
+  pending_requests.erase(request_id);
+  version->FinishRequest(request_id, true, base::Time::Now());
+  return true;

[falken, 2016/09/23 02:27:35]

This ignores FinishRequest's return value. Maybe it should be used?

[lazyboy, 2016/09/27 21:26:45]

Done.

+}
+
 void ServiceWorkerContextWrapper::DidDeleteAndStartOver(
     ServiceWorkerStatusCode status) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);