Revert 63578 - 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

TBR=willchan@chromium.org
Review URL: http://codereview.chromium.org/4014010

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

6 files changed, 17 insertions, 102 deletions

diff --git a/chrome/browser/browser_process_sub_thread.cc b/chrome/browser/browser_process_sub_thread.cc
index 0ddbf3b..39729d0 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::CleanUpAfterMessageLoopDestruction() {
+void BrowserProcessSubThread::CleanUp() {
   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 011cdf0..c2012cd 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 CleanUpAfterMessageLoopDestruction();
+  virtual void CleanUp();
 
  private:
   // Each specialized thread has its own notification service.
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index 8772ffd..a2d0a50 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -17,7 +17,6 @@
 #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"
@@ -219,30 +218,6 @@ 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(
@@ -292,9 +267,6 @@ 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)
@@ -302,32 +274,14 @@ void IOThread::CleanUp() {
   // Destroy all URLRequests started by URLFetchers.
   URLFetcher::CancelAll();
 
-  // Break any cycles between the ProxyScriptFetcher and URLRequestContext.
-  for (ProxyScriptFetchers::const_iterator it = fetchers_.begin();
-       it != fetchers_.end(); ++it) {
-    (*it)->Cancel();
-  }
+  // This must be reset before the ChromeNetLog is destroyed.
+  network_change_observer_.reset();
 
   // 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();
@@ -348,6 +302,12 @@ 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());
@@ -360,13 +320,10 @@ 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 observers have
+  // have a method that runs after the message loop destruction obsevers 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 68abe75..2baec87 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,7 +17,6 @@
 #include "net/base/network_change_notifier.h"
 
 class ChromeNetLog;
-class ChromeURLRequestContextGetter;
 class ListValue;
 class URLRequestContext;
 
@@ -67,21 +66,6 @@ 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();
@@ -148,11 +132,6 @@ 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 eeb9fc9..0abe211 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -531,9 +531,8 @@ ChromeURLRequestContext* FactoryForMedia::Create() {
 ChromeURLRequestContextGetter::ChromeURLRequestContextGetter(
     Profile* profile,
     ChromeURLRequestContextFactory* factory)
-    : io_thread_(g_browser_process->io_thread()),
-      factory_(factory),
-      url_request_context_(NULL) {
+  : factory_(factory),
+    url_request_context_(NULL) {
   DCHECK(factory);
 
   // If a base profile was specified, listen for changes to the preferences.
@@ -551,9 +550,6 @@ 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.
 }
@@ -574,17 +570,11 @@ 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);
@@ -777,12 +767,9 @@ ChromeURLRequestContext::~ChromeURLRequestContext() {
 
 #if defined(USE_NSS)
   if (is_main()) {
-    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);
-    }
+    DCHECK_EQ(this, net::GetURLRequestContextForOCSP());
+    // 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 51d9246..43b1b21 100644
--- a/chrome/browser/net/chrome_url_request_context.h
+++ b/chrome/browser/net/chrome_url_request_context.h
@@ -242,10 +242,6 @@ 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() {
@@ -313,10 +309,6 @@ 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_;