service worker: Observe when resource context is shutting down

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 85d9830a529c493123c99ac40774c86816ed4696..b09348da3e48b7b30210fbd967289d384dec3b71 100644
--- a/content/browser/service_worker/service_worker_context_wrapper.cc
+++ b/content/browser/service_worker/service_worker_context_wrapper.cc
@@ -130,6 +130,27 @@ void ServiceWorkerContextWrapper::Shutdown() {
       base::Bind(&ServiceWorkerContextWrapper::ShutdownOnIO, this));
 }
 
+void ServiceWorkerContextWrapper::InitializeOnIO(

[michaeln, 2016/03/08 22:04:27]

naming nit driveby: since there already are Init() and InitInternal() methods on
this object (with the latter intended to be run on the IO thread), maybe call
this one InitializeResourceContext

[falken, 2016/03/10 01:25:35]

Done.

+    ResourceContext* resource_context,
+    scoped_refptr<net::URLRequestContextGetter> request_context_getter) {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);
+  resource_context_ = resource_context;
+  request_context_getter_ = request_context_getter;
+  request_context_getter_->AddObserver(this);
+}
+
+void ServiceWorkerContextWrapper::OnContextShuttingDown() {
+  DCHECK_CURRENTLY_ON(BrowserThread::IO);

[falken, 2016/03/08 09:59:11]

mmenke: URLRequestContextGetterObserver documentation says "network thread" but
it seems to be called on the IO thread via
ChromeURLRequestContextGetter::NotifyContextShuttingDown. Is this DCHECK safe?

[michaeln, 2016/03/08 22:04:27]

yes, content::iothread == net::networkthread

[mmenke, 2016/03/09 18:33:42]

Correct.  net/ is below content/, so doesn't know what thread it will be used on
(It should also be possible to create multiple URLRequestContexts that live on
different threads, though there are currently some globals that make doing so a
bit iffy)

+  request_context_getter_->RemoveObserver(this);
+
+  // OnContextShuttingDown is called when the ProfileIOData (ResourceContext) is
+  // shutting down, so call ShutdownOnIO() to clear resource_context_.
+  // This doesn't seem to be called when using content_shell, so we still must

[mmenke, 2016/03/09 18:33:42]

This bug may still exist with content shell, or other embedders, depending on
their URLRequestContext ownership model (If the getter owns the context, they're
safe.  Otherwise, they will probably still have this problem)

It's up to the class that owns the URLRequestContext to tell the
URLRequestContextGetter when it goes away, but currently, only ProfileIOData
does that, I believe.

+  // also call ShutdownOnIO() in Shutdown(), which is called when the storage
+  // partition is destroyed.
+  ShutdownOnIO();
+}
+
 void ServiceWorkerContextWrapper::DeleteAndStartOver() {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   if (!context_core_) {
@@ -158,12 +179,6 @@ ResourceContext* ServiceWorkerContextWrapper::resource_context() {
   return resource_context_;
 }
 
-void ServiceWorkerContextWrapper::set_resource_context(
-    ResourceContext* resource_context) {
-  DCHECK_CURRENTLY_ON(BrowserThread::IO);
-  resource_context_ = resource_context;
-}
-
 static void FinishRegistrationOnIO(
     const ServiceWorkerContext::ResultCallback& continuation,
     ServiceWorkerStatusCode status,