Revert 59972 (chromium os leaks) - Reland r59511: Eagerly set the IO loop used for OCSP.

ChromeOS will create a special Profile for login.  Previously, OCSP initialization was done for the "default" ChromeURLRequestContext for each Profile.  Since we can have multiple profiles, this causes the initialization (and uninitialization) to happen multiple times, which causes problems for OCSP since we use statics.  The solution is to identify the "main" Profile.  We create said Profile in BrowserMain.  I add an "is_main_" variable to URLRequestContextGetter and URLRequestContext, so that only the "main" URLRequestContext will initialize OCSP.

Other than minor plumbing of "is_main_", this changelist is identical to r59511.

BUG=55940
TEST=Startup ChromeOS's Chrome.  Should not crash on startup.

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

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

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

9 files changed, 102 insertions, 227 deletions

diff --git a/chrome/browser/browser_main.cc b/chrome/browser/browser_main.cc
index e0b6df0..2c83d32 100644
--- a/chrome/browser/browser_main.cc
+++ b/chrome/browser/browser_main.cc
@@ -49,9 +49,8 @@
 #include "chrome/browser/metrics/metrics_log.h"
 #include "chrome/browser/metrics/metrics_service.h"
 #include "chrome/browser/net/blob_url_request_job_factory.h"
-#include "chrome/browser/net/chrome_url_request_context.h"
-#include "chrome/browser/net/metadata_url_request.h"
 #include "chrome/browser/net/predictor_api.h"
+#include "chrome/browser/net/metadata_url_request.h"
 #include "chrome/browser/net/sdch_dictionary_fetcher.h"
 #include "chrome/browser/net/websocket_experiment/websocket_experiment_runner.h"
 #include "chrome/browser/plugin_service.h"
@@ -1193,8 +1192,6 @@ int BrowserMain(const MainFunctionParams& parameters) {
   PrefService* user_prefs = profile->GetPrefs();
   DCHECK(user_prefs);
 
-  profile->GetDefaultRequestContext()->set_is_main(true);
-
   // Convert active labs into switches. Modifies the current command line.
   about_labs::ConvertLabsToSwitches(profile, CommandLine::ForCurrentProcess());
 
diff --git a/chrome/browser/io_thread.cc b/chrome/browser/io_thread.cc
index ed82c27..f361b77 100644
--- a/chrome/browser/io_thread.cc
+++ b/chrome/browser/io_thread.cc
@@ -24,9 +24,6 @@
 #include "net/base/net_util.h"
 #include "net/http/http_auth_filter.h"
 #include "net/http/http_auth_handler_factory.h"
-#if defined(USE_NSS)
-#include "net/ocsp/nss_ocsp.h"
-#endif  // defined(USE_NSS)
 
 namespace {
 
@@ -160,12 +157,6 @@ void IOThread::ChangedToOnTheRecord() {
 void IOThread::Init() {
   BrowserProcessSubThread::Init();
 
-  DCHECK_EQ(MessageLoop::TYPE_IO, message_loop()->type());
-
-#if defined(USE_NSS)
-  net::SetMessageLoopForOCSP();
-#endif // defined(USE_NSS)
-
   DCHECK(!globals_);
   globals_ = new Globals;
 
diff --git a/chrome/browser/net/chrome_url_request_context.cc b/chrome/browser/net/chrome_url_request_context.cc
index 9237928..f14958f 100644
--- a/chrome/browser/net/chrome_url_request_context.cc
+++ b/chrome/browser/net/chrome_url_request_context.cc
@@ -313,6 +313,11 @@ ChromeURLRequestContext* FactoryForOriginal::Create() {
 
   appcache_service_->set_request_context(context);
 
+#if defined(USE_NSS)
+  // TODO(ukai): find a better way to set the URLRequestContext for OCSP.
+  net::SetURLRequestContextForOCSP(context);
+#endif
+
   context->set_net_log(io_thread_globals->net_log.get());
   return context;
 }
@@ -552,14 +557,6 @@ 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 URLRequestContext for OCSP.
-      net::SetURLRequestContextForOCSP(url_request_context_);
-#endif
-    }
-
     factory_.reset();
   }
 
@@ -774,8 +771,7 @@ ChromeURLRequestContext::~ChromeURLRequestContext() {
   }
 
 #if defined(USE_NSS)
-  if (is_main()) {
-    DCHECK_EQ(this, net::GetURLRequestContextForOCSP());
+  if (this == net::GetURLRequestContextForOCSP()) {
     // We are releasing the URLRequestContext used by OCSP handlers.
     net::SetURLRequestContextForOCSP(NULL);
   }
diff --git a/chrome/browser/net/chrome_url_request_context.h b/chrome/browser/net/chrome_url_request_context.h
index 649213f..97997e3 100644
--- a/chrome/browser/net/chrome_url_request_context.h
+++ b/chrome/browser/net/chrome_url_request_context.h
@@ -263,6 +263,7 @@ class ChromeURLRequestContext : public URLRequestContext {
   bool is_off_the_record_;
 
  private:
+
   DISALLOW_COPY_AND_ASSIGN(ChromeURLRequestContext);
 };
 
diff --git a/chrome/common/net/url_request_context_getter.h b/chrome/common/net/url_request_context_getter.h
index c3e6d09..34aa668 100644
--- a/chrome/common/net/url_request_context_getter.h
+++ b/chrome/common/net/url_request_context_getter.h
@@ -35,32 +35,15 @@ class URLRequestContextGetter
   // may be used).
   virtual scoped_refptr<base::MessageLoopProxy> GetIOMessageLoopProxy() = 0;
 
-  // Controls whether or not the URLRequestContextGetter considers itself to be
-  // the the "main" URLRequestContextGetter.  Note that each Profile will have a
-  // "default" URLRequestContextGetter.  Therefore, "is_main" refers to the
-  // default URLRequestContextGetter for the "main" Profile.
-  // TODO(willchan): Move this code to ChromeURLRequestContextGetter, since this
-  // ia a browser process specific concept.
-  void set_is_main(bool is_main) { is_main_ = true; }
-
  protected:
   friend class DeleteTask<URLRequestContextGetter>;
   friend struct URLRequestContextGetterTraits;
 
-  URLRequestContextGetter() : is_main_(false) {}
-
   virtual ~URLRequestContextGetter() {}
-
-  bool is_main() const { return is_main_; }
-
  private:
   // OnDestruct is meant to ensure deletion on the thread on which the request
   // IO happens.
   void OnDestruct();
-
-  // Indicates whether or not this is the default URLRequestContextGetter for
-  // the main Profile.
-  bool is_main_;
 };
 
 struct URLRequestContextGetterTraits {
@@ -70,3 +53,4 @@ struct URLRequestContextGetterTraits {
 };
 
 #endif  // CHROME_COMMON_NET_URL_REQUEST_CONTEXT_GETTER_H_
+
diff --git a/net/ocsp/nss_ocsp.cc b/net/ocsp/nss_ocsp.cc
index 4fd51a9..bd6a70b 100644
--- a/net/ocsp/nss_ocsp.cc
+++ b/net/ocsp/nss_ocsp.cc
@@ -9,19 +9,16 @@
 #include <ocsp.h>
 #include <nspr.h>
 #include <nss.h>
-#include <pthread.h>
 #include <secerr.h>
 
 #include <string>
 
-#include "base/basictypes.h"
 #include "base/compiler_specific.h"
 #include "base/condition_variable.h"
 #include "base/histogram.h"
-#include "base/lazy_instance.h"
-#include "base/lock.h"
 #include "base/logging.h"
 #include "base/message_loop.h"
+#include "base/singleton.h"
 #include "base/string_util.h"
 #include "base/thread.h"
 #include "base/time.h"
@@ -35,93 +32,6 @@
 
 namespace {
 
-// Protects |g_request_context|.
-pthread_mutex_t g_request_context_lock = PTHREAD_MUTEX_INITIALIZER;
-static URLRequestContext* g_request_context = NULL;
-
-class OCSPIOLoop : public MessageLoop::DestructionObserver {
- public:
-  // MessageLoop::DestructionObserver:
-  virtual void WillDestroyCurrentMessageLoop();
-
-  void StartUsing() {
-    AutoLock autolock(lock_);
-    used_ = true;
-  }
-
-  bool used() const {
-    AutoLock autolock(lock_);
-    return used_;
-  }
-
-  // Called from worker thread.
-  void PostTaskToIOLoop(const tracked_objects::Location& from_here, Task* task);
-
-  void EnsureIOLoop();
-
- private:
-  friend struct base::DefaultLazyInstanceTraits<OCSPIOLoop>;
-
-  OCSPIOLoop();
-  ~OCSPIOLoop();
-
-  mutable Lock lock_;
-  bool used_;  // Protected by |lock_|.
-  // This should not be modified after |used_|.
-  MessageLoopForIO* io_loop_;  // Protected by |lock_|.
-
-  DISALLOW_COPY_AND_ASSIGN(OCSPIOLoop);
-};
-
-OCSPIOLoop::OCSPIOLoop()
-    : used_(false),
-      io_loop_(MessageLoopForIO::current()) {
-  DCHECK(io_loop_);
-  io_loop_->AddDestructionObserver(this);
-}
-
-OCSPIOLoop::~OCSPIOLoop() {
-  // IO thread was already deleted before the singleton is deleted
-  // in AtExitManager.
-  {
-    AutoLock autolock(lock_);
-    DCHECK(!io_loop_);
-    DCHECK(!used_);
-  }
-
-  pthread_mutex_lock(&g_request_context_lock);
-  DCHECK(!g_request_context);
-  pthread_mutex_unlock(&g_request_context_lock);
-}
-
-void OCSPIOLoop::WillDestroyCurrentMessageLoop() {
-  // Prevent the worker thread from trying to access |io_loop_|.
-  {
-    AutoLock autolock(lock_);
-    DCHECK_EQ(MessageLoopForIO::current(), io_loop_);
-    io_loop_ = NULL;
-    used_ = false;
-  }
-
-  pthread_mutex_lock(&g_request_context_lock);
-  g_request_context = NULL;
-  pthread_mutex_unlock(&g_request_context_lock);
-}
-
-void OCSPIOLoop::PostTaskToIOLoop(
-    const tracked_objects::Location& from_here, Task* task) {
-  AutoLock autolock(lock_);
-  if (io_loop_)
-    io_loop_->PostTask(from_here, task);
-}
-
-void OCSPIOLoop::EnsureIOLoop() {
-  AutoLock autolock(lock_);
-  DCHECK_EQ(MessageLoopForIO::current(), io_loop_);
-}
-
-base::LazyInstance<OCSPIOLoop> g_ocsp_io_loop(base::LINKER_INITIALIZED);
-
 const int kRecvBufferSize = 4096;
 
 // All OCSP handlers should be called in the context of
@@ -158,58 +68,97 @@ SECStatus OCSPFree(SEC_HTTP_REQUEST_SESSION request);
 
 char* GetAlternateOCSPAIAInfo(CERTCertificate *cert);
 
-class OCSPNSSInitialization {
- private:
-  friend struct base::DefaultLazyInstanceTraits<OCSPNSSInitialization>;
+class OCSPInitSingleton : public MessageLoop::DestructionObserver {
+ public:
+  // Called on IO thread.
+  virtual void WillDestroyCurrentMessageLoop() {
+    AutoLock autolock(lock_);
+    DCHECK_EQ(MessageLoopForIO::current(), io_loop_);
+    io_loop_ = NULL;
+    request_context_ = NULL;
+  };
 
-  OCSPNSSInitialization();
-  ~OCSPNSSInitialization();
+  // Called from worker thread.
+  void PostTaskToIOLoop(
+      const tracked_objects::Location& from_here, Task* task) {
+    AutoLock autolock(lock_);
+    if (io_loop_)
+      io_loop_->PostTask(from_here, task);
+  }
 
-  SEC_HttpClientFcn client_fcn_;
+  // This is static method because it is called before NSS initialization,
+  // that is, before OCSPInitSingleton is initialized.
+  static void set_url_request_context(URLRequestContext* request_context) {
+    request_context_ = request_context;
+  }
+  static URLRequestContext* url_request_context() {
+    return request_context_;
+  }
 
-  DISALLOW_COPY_AND_ASSIGN(OCSPNSSInitialization);
-};
+ private:
+  friend struct DefaultSingletonTraits<OCSPInitSingleton>;
+
+  OCSPInitSingleton()
+      : io_loop_(MessageLoopForIO::current()) {
+    DCHECK(io_loop_);
+    io_loop_->AddDestructionObserver(this);
+
+    // NSS calls the functions in the function table to download certificates
+    // or CRLs or talk to OCSP responders over HTTP.  These functions must
+    // set an NSS/NSPR error code when they fail.  Otherwise NSS will get the
+    // residual error code from an earlier failed function call.
+    client_fcn_.version = 1;
+    SEC_HttpClientFcnV1Struct *ft = &client_fcn_.fcnTable.ftable1;
+    ft->createSessionFcn = OCSPCreateSession;
+    ft->keepAliveSessionFcn = OCSPKeepAliveSession;
+    ft->freeSessionFcn = OCSPFreeSession;
+    ft->createFcn = OCSPCreate;
+    ft->setPostDataFcn = OCSPSetPostData;
+    ft->addHeaderFcn = OCSPAddHeader;
+    ft->trySendAndReceiveFcn = OCSPTrySendAndReceive;
+    ft->cancelFcn = NULL;
+    ft->freeFcn = OCSPFree;
+    SECStatus status = SEC_RegisterDefaultHttpClient(&client_fcn_);
+    if (status != SECSuccess) {
+      NOTREACHED() << "Error initializing OCSP: " << PR_GetError();
+    }
 
-OCSPNSSInitialization::OCSPNSSInitialization() {
-  // NSS calls the functions in the function table to download certificates
-  // or CRLs or talk to OCSP responders over HTTP.  These functions must
-  // set an NSS/NSPR error code when they fail.  Otherwise NSS will get the
-  // residual error code from an earlier failed function call.
-  client_fcn_.version = 1;
-  SEC_HttpClientFcnV1Struct *ft = &client_fcn_.fcnTable.ftable1;
-  ft->createSessionFcn = OCSPCreateSession;
-  ft->keepAliveSessionFcn = OCSPKeepAliveSession;
-  ft->freeSessionFcn = OCSPFreeSession;
-  ft->createFcn = OCSPCreate;
-  ft->setPostDataFcn = OCSPSetPostData;
-  ft->addHeaderFcn = OCSPAddHeader;
-  ft->trySendAndReceiveFcn = OCSPTrySendAndReceive;
-  ft->cancelFcn = NULL;
-  ft->freeFcn = OCSPFree;
-  SECStatus status = SEC_RegisterDefaultHttpClient(&client_fcn_);
-  if (status != SECSuccess) {
-    NOTREACHED() << "Error initializing OCSP: " << PR_GetError();
-  }
-
-  // Work around NSS bugs 524013 and 564334.  NSS incorrectly thinks the
-  // CRLs for Network Solutions Certificate Authority have bad signatures,
-  // which causes certificates issued by that CA to be reported as revoked.
-  // By using OCSP for those certificates, which don't have AIA extensions,
-  // we can work around these bugs.  See http://crbug.com/41730.
-  CERT_StringFromCertFcn old_callback = NULL;
-  status = CERT_RegisterAlternateOCSPAIAInfoCallBack(
-      GetAlternateOCSPAIAInfo, &old_callback);
-  if (status == SECSuccess) {
-    DCHECK(!old_callback);
-  } else {
-    NOTREACHED() << "Error initializing OCSP: " << PR_GetError();
+    // Work around NSS bugs 524013 and 564334.  NSS incorrectly thinks the
+    // CRLs for Network Solutions Certificate Authority have bad signatures,
+    // which causes certificates issued by that CA to be reported as revoked.
+    // By using OCSP for those certificates, which don't have AIA extensions,
+    // we can work around these bugs.  See http://crbug.com/41730.
+    CERT_StringFromCertFcn old_callback = NULL;
+    status = CERT_RegisterAlternateOCSPAIAInfoCallBack(
+        GetAlternateOCSPAIAInfo, &old_callback);
+    if (status == SECSuccess) {
+      DCHECK(!old_callback);
+    } else {
+      NOTREACHED() << "Error initializing OCSP: " << PR_GetError();
+    }
   }
-}
 
-OCSPNSSInitialization::~OCSPNSSInitialization() {}
+  virtual ~OCSPInitSingleton() {
+    // IO thread was already deleted before the singleton is deleted
+    // in AtExitManager.
+    AutoLock autolock(lock_);
+    DCHECK(!io_loop_);
+    DCHECK(!request_context_);
+  }
+
+  SEC_HttpClientFcn client_fcn_;
+
+  // |lock_| protects |io_loop_|.
+  Lock lock_;
+  // I/O thread.
+  MessageLoop* io_loop_;  // I/O thread
+  // URLRequestContext for OCSP handlers.
+  static URLRequestContext* request_context_;
 
-base::LazyInstance<OCSPNSSInitialization> g_ocsp_nss_initialization(
-    base::LINKER_INITIALIZED);
+  DISALLOW_COPY_AND_ASSIGN(OCSPInitSingleton);
+};
+
+URLRequestContext* OCSPInitSingleton::request_context_ = NULL;
 
 // Concrete class for SEC_HTTP_REQUEST_SESSION.
 // Public methods except virtual methods of URLRequest::Delegate (On* methods)
@@ -250,7 +199,7 @@ class OCSPRequestSession
     // |io_loop_| was initialized to be NULL in constructor, and
     // set only in StartURLRequest, so no need to lock |lock_| here.
     DCHECK(!io_loop_);
-    g_ocsp_io_loop.Get().PostTaskToIOLoop(
+    Singleton<OCSPInitSingleton>()->PostTaskToIOLoop(
         FROM_HERE,
         NewRunnableMethod(this, &OCSPRequestSession::StartURLRequest));
   }
@@ -389,14 +338,11 @@ class OCSPRequestSession
     }
   }
 
-  // Runs on |g_ocsp_io_loop|'s IO loop.
   void StartURLRequest() {
     DCHECK(!request_);
 
-    pthread_mutex_lock(&g_request_context_lock);
-    URLRequestContext* url_request_context = g_request_context;
-    pthread_mutex_unlock(&g_request_context_lock);
-
+    URLRequestContext* url_request_context =
+        OCSPInitSingleton::url_request_context();
     if (url_request_context == NULL)
       return;
 
@@ -529,10 +475,7 @@ SECStatus OCSPCreateSession(const char* host, PRUint16 portnum,
                             SEC_HTTP_SERVER_SESSION* pSession) {
   LOG(INFO) << "OCSP create session: host=" << host << " port=" << portnum;
   DCHECK(!MessageLoop::current());
-  pthread_mutex_lock(&g_request_context_lock);
-  URLRequestContext* request_context = g_request_context;
-  pthread_mutex_unlock(&g_request_context_lock);
-  if (request_context == NULL) {
+  if (OCSPInitSingleton::url_request_context() == NULL) {
     LOG(ERROR) << "No URLRequestContext for OCSP handler.";
     // The application failed to call SetURLRequestContextForOCSP, so we
     // can't create and use URLRequest.  PR_NOT_IMPLEMENTED_ERROR is not an
@@ -839,40 +782,17 @@ char* GetAlternateOCSPAIAInfo(CERTCertificate *cert) {
 
 namespace net {
 
-void SetMessageLoopForOCSP() {
-  // Must have a MessageLoopForIO.
-  DCHECK(MessageLoopForIO::current());
-
-  bool used = g_ocsp_io_loop.Get().used();
-
-  // Should not be called when g_ocsp_io_loop has already been used.
-  DCHECK(!used);
-}
-
 void EnsureOCSPInit() {
-  g_ocsp_io_loop.Get().StartUsing();
-  g_ocsp_nss_initialization.Get();
+  Singleton<OCSPInitSingleton>::get();
 }
 
 // This function would be called before NSS initialization.
 void SetURLRequestContextForOCSP(URLRequestContext* request_context) {
-  pthread_mutex_lock(&g_request_context_lock);
-  if (request_context) {
-    DCHECK(request_context->is_main());
-    DCHECK(!g_request_context);
-  } else {
-    DCHECK(g_request_context);
-  }
-  g_request_context = request_context;
-  pthread_mutex_unlock(&g_request_context_lock);
+  OCSPInitSingleton::set_url_request_context(request_context);
 }
 
 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());
-  return request_context;
+  return OCSPInitSingleton::url_request_context();
 }
 
 }  // namespace net
diff --git a/net/ocsp/nss_ocsp.h b/net/ocsp/nss_ocsp.h
index 97d69b9..a31d025 100644
--- a/net/ocsp/nss_ocsp.h
+++ b/net/ocsp/nss_ocsp.h
@@ -10,11 +10,6 @@ class URLRequestContext;
 
 namespace net {
 
-// Sets the MessageLoop for OCSP to the current message loop.
-// This should be called before EnsureOCSPInit() if you want to
-// control the message loop for OCSP.
-void SetMessageLoopForOCSP();
-
 // Initializes OCSP handlers for NSS.  This must be called before any
 // certificate verification functions.  This function is thread-safe, and OCSP
 // handlers will only ever be initialized once.
diff --git a/net/url_request/url_request_context.cc b/net/url_request/url_request_context.cc
index 5c50bbb..0ea8477 100644
--- a/net/url_request/url_request_context.cc
+++ b/net/url_request/url_request_context.cc
@@ -15,8 +15,7 @@ URLRequestContext::URLRequestContext()
       http_auth_handler_factory_(NULL),
       network_delegate_(NULL),
       cookie_policy_(NULL),
-      transport_security_state_(NULL),
-      is_main_(false) {
+      transport_security_state_(NULL) {
 }
 
 const std::string& URLRequestContext::GetUserAgent(const GURL& url) const {
diff --git a/net/url_request/url_request_context.h b/net/url_request/url_request_context.h
index 7273429..6ddc940 100644
--- a/net/url_request/url_request_context.h
+++ b/net/url_request/url_request_context.h
@@ -104,11 +104,6 @@ class URLRequestContext
     referrer_charset_ = charset;
   }
 
-  // Controls whether or not the URLRequestContext considers itself to be the
-  // "main" URLRequestContext.
-  bool is_main() const { return is_main_; }
-  void set_is_main(bool is_main) { is_main_ = true; }
-
  protected:
   friend class base::RefCountedThreadSafe<URLRequestContext>;
 
@@ -136,9 +131,6 @@ class URLRequestContext
   std::string referrer_charset_;
 
  private:
-  // Indicates whether or not this is the main URLRequestContext.
-  bool is_main_;
-
   DISALLOW_COPY_AND_ASSIGN(URLRequestContext);
 };