Move the core state machine of SSLClientSocketNSS into a thread-safe Core

net/socket/client_socket_factory.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/socket/client_socket_factory.h"
 
 #include "base/lazy_instance.h"
+#include "base/thread_task_runner_handle.h"
+#include "base/threading/thread.h"
 #include "build/build_config.h"
 #include "net/base/cert_database.h"
 #include "net/socket/client_socket_handle.h"
 #if defined(OS_WIN)
 #include "net/socket/ssl_client_socket_nss.h"
 #include "net/socket/ssl_client_socket_win.h"
 #elif defined(USE_OPENSSL)
 #include "net/socket/ssl_client_socket_openssl.h"
 #elif defined(USE_NSS)
 #include "net/socket/ssl_client_socket_nss.h"
 #elif defined(OS_MACOSX)
 #include "net/socket/ssl_client_socket_mac.h"
 #include "net/socket/ssl_client_socket_nss.h"
 #endif
 #include "net/socket/ssl_host_info.h"
 #include "net/socket/tcp_client_socket.h"
 #include "net/udp/udp_client_socket.h"
 
 namespace net {
 
 class X509Certificate;
 
 namespace {
 
 bool g_use_system_ssl = false;
 
+// ChromeOS uses a hardware TPM module that may cause NSS operations to
+// block for upwards of several seconds. To avoid blocking all network and
+// IPC activity, run NSS SSL functions on a dedicated thread.
+#if defined(OS_CHROMEOS)
+bool g_use_dedicated_nss_thread = true;
+#else
+bool g_use_dedicated_nss_thread = false;
+#endif
+
 class DefaultClientSocketFactory : public ClientSocketFactory,
                                    public CertDatabase::Observer {
  public:
   DefaultClientSocketFactory() {
+    if (g_use_dedicated_nss_thread) {
+      nss_thread_.reset(new base::Thread("NSS SSL Thread"));
+      nss_thread_->Start();

[wtc, 2012/06/01 01:02:38]

This creates a thread with MessageLoop::TYPE_DEFAULT, right?
Just wanted to confirm it.

[Ryan Sleevi, 2012/06/01 01:30:04]

Yes.

This is why I need to test on the other platforms (will Windows require a
MessageLoopForUI, because of the possible prompting, and will OS X require a
MessageLoopForUI so that there is a CFRunLoop to receive Security.framework
notifications/UI prompts)

+    }
+
     CertDatabase::AddObserver(this);
   }
 
   virtual ~DefaultClientSocketFactory() {
     CertDatabase::RemoveObserver(this);
   }
 
   virtual void OnUserCertAdded(const X509Certificate* cert) {
     ClearSSLSessionCache();
   }
@@ skipping 21 matching lines @@
   }
 
   virtual SSLClientSocket* CreateSSLClientSocket(
       ClientSocketHandle* transport_socket,
       const HostPortPair& host_and_port,
       const SSLConfig& ssl_config,
       SSLHostInfo* ssl_host_info,
       const SSLClientSocketContext& context) {
     scoped_ptr<SSLHostInfo> shi(ssl_host_info);
 
-#if defined(OS_WIN)
+    scoped_refptr<base::SingleThreadTaskRunner> nss_task_runner(
+        base::ThreadTaskRunnerHandle::Get());

[willchan no longer on Chromium, 2012/06/04 16:50:38]

How about changing this to be acquired once in the constructor? I think this
makes more sense, since I could foresee in the future actually passing this into
the constructor.

[Ryan Sleevi, 2012/06/06 00:57:06]

Undid this change. Unit tests may change the current thread's task runner. Turns
out this happens in content_unittests, but not net_unittests. Go figure.

I am caching the thread's MLP, and I agree, we may wish to pass this in the ctor
in the future, although as my comments to wtc (comment #2/#3) indicate, I think
the higher up we pass the buck, the more complicated the state becomes, and the
whole "NSS on one thread or two" is really implementation-specific.

+
+    if (g_use_dedicated_nss_thread && nss_thread_->message_loop_proxy())
+      nss_task_runner = nss_thread_->message_loop_proxy();
+
+#if defined(USE_OPENSSL)
+    return new SSLClientSocketOpenSSL(transport_socket, host_and_port,
+                                      ssl_config, context);
+#elif defined(USE_NSS)
+    return new SSLClientSocketNSS(nss_task_runner, transport_socket,
+                                  host_and_port, ssl_config, shi.release(),
+                                  context);
+#elif defined(OS_WIN)
     if (g_use_system_ssl) {
       return new SSLClientSocketWin(transport_socket, host_and_port,
                                     ssl_config, context);
     }
-    return new SSLClientSocketNSS(transport_socket, host_and_port, ssl_config,
-                                  shi.release(), context);
-#elif defined(USE_OPENSSL)
-    return new SSLClientSocketOpenSSL(transport_socket, host_and_port,
-                                      ssl_config, context);
-#elif defined(USE_NSS)
-    return new SSLClientSocketNSS(transport_socket, host_and_port, ssl_config,
-                                  shi.release(), context);
+    return new SSLClientSocketNSS(nss_task_runner, transport_socket,
+                                  host_and_port, ssl_config, shi.release(),
+                                  context);
 #elif defined(OS_MACOSX)
     if (g_use_system_ssl) {
       return new SSLClientSocketMac(transport_socket, host_and_port,
                                     ssl_config, context);
     }
-    return new SSLClientSocketNSS(transport_socket, host_and_port, ssl_config,
-                                  shi.release(), context);
+    return new SSLClientSocketNSS(nss_task_runner, transport_socket,
+                                  host_and_port, ssl_config, shi.release(),
+                                  context);
 #else
     NOTIMPLEMENTED();
     return NULL;
 #endif
   }
 
   void ClearSSLSessionCache() {
     SSLClientSocket::ClearSessionCache();
   }
 
+ private:
+  scoped_ptr<base::Thread> nss_thread_;
 };
 
 static base::LazyInstance<DefaultClientSocketFactory>
     g_default_client_socket_factory = LAZY_INSTANCE_INITIALIZER;
 
 }  // namespace
 
 // Deprecated function (http://crbug.com/37810) that takes a StreamSocket.
 SSLClientSocket* ClientSocketFactory::CreateSSLClientSocket(
     StreamSocket* transport_socket,
@@ skipping 19 matching lines @@
 #if defined(OS_WIN)
   // Reflect the capability of SSLClientSocketWin.
   SSLConfigService::SetDefaultVersionMax(SSL_PROTOCOL_VERSION_TLS1);
 #elif defined(OS_MACOSX)
   // Reflect the capability of SSLClientSocketMac.
   SSLConfigService::SetDefaultVersionMax(SSL_PROTOCOL_VERSION_TLS1);
 #endif
 }
 
 }  // namespace net