Release ChromeURLRequestContextGetters' references on IO thread shutdown.

We register all ChromeURLRequestContextGetters that have a reference to a ChromeURLRequestContext with the IOThread.  We unregister them when they go away.  In IOThread::CleanUp(), we tell the known ChromeURLRequestContextGetters to release their references to ChromeURLRequestContexts.

BUG=58859
TEST=none

Review URL: http://codereview.chromium.org/3686003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@63578 0039d316-1c4b-4281-b951-d872f2087c98

7 files changed, 103 insertions, 18 deletions

diff --git a/chrome/browser/browser_process_sub_thread.cc b/chrome/browser/browser_process_sub_thread.cc
index 39729d0..0ddbf3b 100644
--- a/chrome/browser/browser_process_sub_thread.cc
+++ b/chrome/browser/browser_process_sub_thread.cc
@@ -28,7 +28,7 @@ void BrowserProcessSubThread::Init() {
   notification_service_ = new NotificationService;
 }
 
-void BrowserProcessSubThread::CleanUp() {
+void BrowserProcessSubThread::CleanUpAfterMessageLoopDestruction() {
   delete notification_service_;
   notification_service_ = NULL;
 
diff --git a/chrome/browser/browser_process_sub_thread.h b/chrome/browser/browser_process_sub_thread.h
index c2012cd..011cdf0 100644
--- a/chrome/browser/browser_process_sub_thread.h
+++ b/chrome/browser/browser_process_sub_thread.h
@@ -27,7 +27,7 @@ class BrowserProcessSubThread : public BrowserThread {
 
  protected:
   virtual void Init();
-  virtual void CleanUp();
+  virtual void CleanUpAfterMessageLoopDestruction();
 
  private:
   // Each specialized thread has its own notification service.
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index 990f9a4..4dc520a 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -16,6 +16,7 @@
 #include "chrome/browser/browser_thread.h"
 #include "chrome/browser/gpu_process_host.h"
 #include "chrome/browser/net/chrome_net_log.h"
+#include "chrome/browser/net/chrome_url_request_context.h"
 #include "chrome/browser/net/connect_interceptor.h"
 #include "chrome/browser/net/passive_log_collector.h"
 #include "chrome/browser/net/predictor_api.h"
@@ -217,6 +218,30 @@ void IOThread::InitNetworkPredictor(
           startup_urls, referral_list, preconnect_enabled));
 }
 
+void IOThread::RegisterURLRequestContextGetter(
+    ChromeURLRequestContextGetter* url_request_context_getter) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  std::list<ChromeURLRequestContextGetter*>::const_iterator it =
+      std::find(url_request_context_getters_.begin(),
+                url_request_context_getters_.end(),
+                url_request_context_getter);
+  DCHECK(it == url_request_context_getters_.end());
+  url_request_context_getters_.push_back(url_request_context_getter);
+}
+
+void IOThread::UnregisterURLRequestContextGetter(
+    ChromeURLRequestContextGetter* url_request_context_getter) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
+  std::list<ChromeURLRequestContextGetter*>::iterator it =
+      std::find(url_request_context_getters_.begin(),
+                url_request_context_getters_.end(),
+                url_request_context_getter);
+  DCHECK(it != url_request_context_getters_.end());
+  // This does not scale, but we shouldn't have many URLRequestContextGetters in
+  // the first place, so this should be fine.
+  url_request_context_getters_.erase(it);
+}
+
 void IOThread::ChangedToOnTheRecord() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   message_loop()->PostTask(
@@ -259,6 +284,9 @@ void IOThread::Init() {
 }
 
 void IOThread::CleanUp() {
+  // Step 1: Kill all things that might be holding onto
+  // URLRequest/URLRequestContexts.
+
 #if defined(USE_NSS)
   net::ShutdownOCSP();
 #endif  // defined(USE_NSS)
@@ -266,14 +294,32 @@ void IOThread::CleanUp() {
   // Destroy all URLRequests started by URLFetchers.
   URLFetcher::CancelAll();
 
-  // This must be reset before the ChromeNetLog is destroyed.
-  network_change_observer_.reset();
+  // Break any cycles between the ProxyScriptFetcher and URLRequestContext.
+  for (ProxyScriptFetchers::const_iterator it = fetchers_.begin();
+       it != fetchers_.end(); ++it) {
+    (*it)->Cancel();
+  }
 
   // If any child processes are still running, terminate them and
   // and delete the BrowserChildProcessHost instances to release whatever
   // IO thread only resources they are referencing.
   BrowserChildProcessHost::TerminateAll();
 
+  std::list<ChromeURLRequestContextGetter*> url_request_context_getters;
+  url_request_context_getters.swap(url_request_context_getters_);
+  for (std::list<ChromeURLRequestContextGetter*>::iterator it =
+       url_request_context_getters.begin();
+       it != url_request_context_getters.end(); ++it) {
+    ChromeURLRequestContextGetter* getter = *it;
+    getter->ReleaseURLRequestContext();
+  }
+
+  // Step 2: Release objects that the URLRequestContext could have been pointing
+  // to.
+
+  // This must be reset before the ChromeNetLog is destroyed.
+  network_change_observer_.reset();
+
   // Not initialized in Init().  May not be initialized.
   if (predictor_) {
     predictor_->Shutdown();
@@ -294,12 +340,6 @@ void IOThread::CleanUp() {
     globals_->host_resolver.get()->GetAsHostResolverImpl()->Shutdown();
   }
 
-  // Break any cycles between the ProxyScriptFetcher and URLRequestContext.
-  for (ProxyScriptFetchers::const_iterator it = fetchers_.begin();
-       it != fetchers_.end(); ++it) {
-    (*it)->Cancel();
-  }
-
   // We will delete the NetLog as part of CleanUpAfterMessageLoopDestruction()
   // in case any of the message loop destruction observers try to access it.
   deferred_net_log_to_delete_.reset(globals_->net_log.release());
@@ -312,10 +352,13 @@ void IOThread::CleanUp() {
 
 void IOThread::CleanUpAfterMessageLoopDestruction() {
   // TODO(eroman): get rid of this special case for 39723. If we could instead
-  // have a method that runs after the message loop destruction obsevers have
+  // have a method that runs after the message loop destruction observers have
   // run, but before the message loop itself is destroyed, we could safely
   // combine the two cleanups.
   deferred_net_log_to_delete_.reset();
+
+  // This will delete the |notification_service_|.  Make sure it's done after
+  // anything else can reference it.
   BrowserProcessSubThread::CleanUpAfterMessageLoopDestruction();
 
   // URLRequest instances must NOT outlive the IO thread.
diff --git a/chrome/browser/io_thread.h b/chrome/browser/io_thread.h
index 2baec87..68abe75 100644
--- a/chrome/browser/io_thread.h
+++ b/chrome/browser/io_thread.h
@@ -6,8 +6,8 @@
 #define CHROME_BROWSER_IO_THREAD_H_
 #pragma once
 
+#include <list>
 #include <set>
-
 #include "base/basictypes.h"
 #include "base/ref_counted.h"
 #include "base/scoped_ptr.h"
@@ -17,6 +17,7 @@
 #include "net/base/network_change_notifier.h"
 
 class ChromeNetLog;
+class ChromeURLRequestContextGetter;
 class ListValue;
 class URLRequestContext;
 
@@ -66,6 +67,21 @@ class IOThread : public BrowserProcessSubThread {
                             ListValue* referral_list,
                             bool preconnect_enabled);
 
+  // Registers |url_request_context_getter| into the IO thread.  During
+  // IOThread::CleanUp(), IOThread will iterate through known getters and
+  // release their URLRequestContexts.  Only called on the IO thread.  It does
+  // not acquire a refcount for |url_request_context_getter|.  If
+  // |url_request_context_getter| is being deleted before IOThread::CleanUp() is
+  // invoked, then this needs to be balanced with a call to
+  // UnregisterURLRequestContextGetter().
+  void RegisterURLRequestContextGetter(
+      ChromeURLRequestContextGetter* url_request_context_getter);
+
+  // Unregisters |url_request_context_getter| from the IO thread.  Only called
+  // on the IO thread.
+  void UnregisterURLRequestContextGetter(
+      ChromeURLRequestContextGetter* url_request_context_getter);
+
   // Handles changing to On The Record mode.  Posts a task for this onto the
   // IOThread's message loop.
   void ChangedToOnTheRecord();
@@ -132,6 +148,11 @@ class IOThread : public BrowserProcessSubThread {
   // List of live ProxyScriptFetchers.
   ProxyScriptFetchers fetchers_;
 
+  // Keeps track of all live ChromeURLRequestContextGetters, so the
+  // ChromeURLRequestContexts can be released during
+  // IOThread::CleanUpAfterMessageLoopDestruction().
+  std::list<ChromeURLRequestContextGetter*> url_request_context_getters_;
+
   DISALLOW_COPY_AND_ASSIGN(IOThread);
 };
 
diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index 0abe211..eeb9fc9 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -531,8 +531,9 @@ ChromeURLRequestContext* FactoryForMedia::Create() {
 ChromeURLRequestContextGetter::ChromeURLRequestContextGetter(
     Profile* profile,
     ChromeURLRequestContextFactory* factory)
-  : factory_(factory),
-    url_request_context_(NULL) {
+    : io_thread_(g_browser_process->io_thread()),
+      factory_(factory),
+      url_request_context_(NULL) {
   DCHECK(factory);
 
   // If a base profile was specified, listen for changes to the preferences.
@@ -550,6 +551,9 @@ ChromeURLRequestContextGetter::~ChromeURLRequestContextGetter() {
   DCHECK((factory_.get() && !url_request_context_.get()) ||
          (!factory_.get() && url_request_context_.get()));
 
+  if (url_request_context_)
+    io_thread_->UnregisterURLRequestContextGetter(this);
+
   // The scoped_refptr / scoped_ptr destructors take care of releasing
   // |factory_| and |url_request_context_| now.
 }
@@ -570,11 +574,17 @@ URLRequestContext* ChromeURLRequestContextGetter::GetURLRequestContext() {
     }
 
     factory_.reset();
+    io_thread_->RegisterURLRequestContextGetter(this);
   }
 
   return url_request_context_;
 }
 
+void ChromeURLRequestContextGetter::ReleaseURLRequestContext() {
+  DCHECK(url_request_context_);
+  url_request_context_ = NULL;
+}
+
 void ChromeURLRequestContextGetter::RegisterUserPrefs(
     PrefService* pref_service) {
   pref_service->RegisterBooleanPref(prefs::kNoProxyServer, false);
@@ -767,9 +777,12 @@ ChromeURLRequestContext::~ChromeURLRequestContext() {
 
 #if defined(USE_NSS)
   if (is_main()) {
-    DCHECK_EQ(this, net::GetURLRequestContextForOCSP());
-    // We are releasing the URLRequestContext used by OCSP handlers.
-    net::SetURLRequestContextForOCSP(NULL);
+    URLRequestContext* ocsp_context = net::GetURLRequestContextForOCSP();
+    if (ocsp_context) {
+      DCHECK_EQ(this, ocsp_context);
+      // We are releasing the URLRequestContext used by OCSP handlers.
+      net::SetURLRequestContextForOCSP(NULL);
+    }
   }
 #endif
 
diff --git a/chrome/browser/net/chrome_url_request_context.h b/chrome/browser/net/chrome_url_request_context.h
index 43b1b21..51d9246 100644
--- a/chrome/browser/net/chrome_url_request_context.h
+++ b/chrome/browser/net/chrome_url_request_context.h
@@ -242,6 +242,10 @@ class ChromeURLRequestContextGetter : public URLRequestContextGetter,
   virtual net::CookieStore* GetCookieStore();
   virtual scoped_refptr<base::MessageLoopProxy> GetIOMessageLoopProxy() const;
 
+  // Releases |url_request_context_|.  It's invalid to call
+  // GetURLRequestContext() after this point.
+  void ReleaseURLRequestContext();
+
   // Convenience overload of GetURLRequestContext() that returns a
   // ChromeURLRequestContext* rather than a URLRequestContext*.
   ChromeURLRequestContext* GetIOContext() {
@@ -309,6 +313,10 @@ class ChromeURLRequestContextGetter : public URLRequestContextGetter,
 
   PrefChangeRegistrar registrar_;
 
+  // |io_thread_| is always valid during the lifetime of |this| since |this| is
+  // deleted on the IO thread.
+  IOThread* const io_thread_;
+
   // Deferred logic for creating a ChromeURLRequestContext.
   // Access only from the IO thread.
   scoped_ptr<ChromeURLRequestContextFactory> factory_;
diff --git a/net/ocsp/nss_ocsp.cc b/net/ocsp/nss_ocsp.cc
index b7fcb65..45ee5bd 100644
--- a/net/ocsp/nss_ocsp.cc
+++ b/net/ocsp/nss_ocsp.cc
@@ -878,7 +878,7 @@ URLRequestContext* GetURLRequestContextForOCSP() {
   pthread_mutex_lock(&g_request_context_lock);
   URLRequestContext* request_context = g_request_context;
   pthread_mutex_unlock(&g_request_context_lock);
-  DCHECK(request_context->is_main());
+  DCHECK(!request_context || request_context->is_main());
   return request_context;
 }