Switch to using the system request context for OCSP.

This gets rid of all the ugly "is_main" logic for URLRequestContext.

BUG=none
TEST=none


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

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

4 files changed, 32 insertions, 29 deletions

diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index 8aa6e01..78b2165 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -45,14 +45,15 @@
 #include "net/http/http_auth_handler_factory.h"
 #include "net/http/http_network_layer.h"
 #include "net/http/http_network_session.h"
-#if defined(USE_NSS)
-#include "net/ocsp/nss_ocsp.h"
-#endif  // defined(USE_NSS)
 #include "net/proxy/proxy_config_service.h"
 #include "net/proxy/proxy_script_fetcher_impl.h"
 #include "net/proxy/proxy_service.h"
 #include "webkit/glue/webkit_glue.h"
 
+#if defined(USE_NSS)
+#include "net/ocsp/nss_ocsp.h"
+#endif  // defined(USE_NSS)
+
 namespace {
 
 // Custom URLRequestContext used by requests which aren't associated with a
@@ -66,6 +67,25 @@ class URLRequestContextWithUserAgent : public net::URLRequestContext {
   }
 };
 
+// Used for the "system" URLRequestContext. If this grows more complicated, then
+// consider inheriting directly from URLRequestContext rather than using
+// implementation inheritance.
+class SystemURLRequestContext : public URLRequestContextWithUserAgent {
+ public:
+  SystemURLRequestContext() {
+#if defined(USE_NSS)
+    net::SetURLRequestContextForOCSP(this);
+#endif  // defined(USE_NSS)
+  }
+
+ private:
+  virtual ~SystemURLRequestContext() {
+#if defined(USE_NSS)
+    net::SetURLRequestContextForOCSP(NULL);
+#endif  // defined(USE_NSS)
+  }
+};
+
 net::HostResolver* CreateGlobalHostResolver(net::NetLog* net_log) {
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
 
@@ -180,6 +200,8 @@ class LoggingNetworkChangeObserver
   DISALLOW_COPY_AND_ASSIGN(LoggingNetworkChangeObserver);
 };
 
+// Create a separate request context for PAC fetches to avoid reference cycles.
+// See IOThread::Globals for details.
 scoped_refptr<net::URLRequestContext>
 ConstructProxyScriptFetcherContext(IOThread::Globals* globals,
                                    net::NetLog* net_log) {
@@ -206,7 +228,7 @@ scoped_refptr<net::URLRequestContext>
 ConstructSystemRequestContext(IOThread::Globals* globals,
                               net::NetLog* net_log) {
   scoped_refptr<net::URLRequestContext> context(
-      new URLRequestContextWithUserAgent);
+      new SystemURLRequestContext);
   context->set_net_log(net_log);
   context->set_host_resolver(globals->host_resolver.get());
   context->set_cert_verifier(globals->cert_verifier.get());
diff --git a/chrome/browser/io_thread.h b/chrome/browser/io_thread.h
index 0e20611..1522cfa 100644
--- a/chrome/browser/io_thread.h
+++ b/chrome/browser/io_thread.h
@@ -66,6 +66,12 @@ class IOThread : public BrowserProcessSubThread {
     scoped_ptr<net::FtpTransactionFactory>
         proxy_script_fetcher_ftp_transaction_factory;
     scoped_ptr<net::URLSecurityManager> url_security_manager;
+    // We use a separate URLRequestContext for PAC fetches, in order to break
+    // the reference cycle:
+    // URLRequestContext=>PAC fetch=>URLRequest=>URLRequestContext.
+    // The first URLRequestContext is |system_url_request_context|. We introduce
+    // |proxy_script_fetcher_context| for the second context. It has a direct
+    // ProxyService, since we always directly connect to fetch the PAC script.
     scoped_refptr<net::URLRequestContext> proxy_script_fetcher_context;
     scoped_ptr<net::ProxyService> system_proxy_service;
     scoped_ptr<net::HttpTransactionFactory> system_http_transaction_factory;
diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index 8888979..1f8dc03 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -21,10 +21,6 @@
 #include "net/http/http_util.h"
 #include "webkit/glue/webkit_glue.h"
 
-#if defined(USE_NSS)
-#include "net/ocsp/nss_ocsp.h"
-#endif
-
 class ChromeURLRequestContextFactory {
  public:
   ChromeURLRequestContextFactory() {}
@@ -150,15 +146,6 @@ net::URLRequestContext* ChromeURLRequestContextGetter::GetURLRequestContext() {
   if (!url_request_context_) {
     DCHECK(factory_.get());
     url_request_context_ = factory_->Create();
-    if (is_main()) {
-      url_request_context_->set_is_main(true);
-#if defined(USE_NSS)
-      // TODO(ukai): find a better way to set the net::URLRequestContext for
-      // OCSP.
-      net::SetURLRequestContextForOCSP(url_request_context_);
-#endif
-    }
-
     factory_.reset();
     io_thread_->RegisterURLRequestContextGetter(this);
   }
@@ -399,17 +386,6 @@ ChromeURLRequestContext::~ChromeURLRequestContext() {
   if (appcache_service_.get() && appcache_service_->request_context() == this)
     appcache_service_->set_request_context(NULL);
 
-#if defined(USE_NSS)
-  if (is_main()) {
-    net::URLRequestContext* ocsp_context = net::GetURLRequestContextForOCSP();
-    if (ocsp_context) {
-      DCHECK_EQ(this, ocsp_context);
-      // We are releasing the net::URLRequestContext used by OCSP handlers.
-      net::SetURLRequestContextForOCSP(NULL);
-    }
-  }
-#endif
-
   NotificationService::current()->Notify(
       NotificationType::URL_REQUEST_CONTEXT_RELEASED,
       Source<net::URLRequestContext>(this),
diff --git a/chrome/browser/profiles/profile_impl.cc b/chrome/browser/profiles/profile_impl.cc
index 57716423..c4d3f7a 100644
--- a/chrome/browser/profiles/profile_impl.cc
+++ b/chrome/browser/profiles/profile_impl.cc
@@ -871,7 +871,6 @@ net::URLRequestContextGetter* ProfileImpl::GetRequestContext() {
   // created first.
   if (!default_request_context_) {
     default_request_context_ = request_context;
-    request_context->set_is_main(true);
     // TODO(eroman): this isn't terribly useful anymore now that the
     // net::URLRequestContext is constructed by the IO thread...
     NotificationService::current()->Notify(