Plumbing for TCP FastOpen for SSL sockets.

net/socket/tcp_socket.cc

Index: net/socket/tcp_socket.cc
diff --git a/net/socket/tcp_socket.cc b/net/socket/tcp_socket.cc
index 864159750a8d2c700af63ff7692633dac7e50453..4a201cc6dc8dbdb1552febcd56561cf5ebb94cb3 100644
--- a/net/socket/tcp_socket.cc
+++ b/net/socket/tcp_socket.cc
@@ -14,66 +14,64 @@ namespace net {
 
 namespace {
 
+// True if OS supports TCP FastOpen.
+bool g_tcp_fastopen_supported = false;

[Randy Smith (Not in Mondays), 2014/08/11 19:07:01]

Is there a reason why all this logic is here rather than in
tcp_socket_libevent.cc?  It seems like it's all used there, and this code isn't
tied to TCPSocket in any way I see, so wouldn't it make sense to have that code
over there?

+// True if OS supports TCP FastOpen and is turned on for all connections.
 bool g_tcp_fastopen_enabled = false;
 
 #if defined(OS_LINUX) || defined(OS_ANDROID)
 
 typedef base::RefCountedData<bool> SharedBoolean;
 
-// Checks to see if the system supports TCP FastOpen. Notably, it requires
-// kernel support. Additionally, this checks system configuration to ensure that
-// it's enabled.
-void SystemSupportsTCPFastOpen(scoped_refptr<SharedBoolean> supported) {
-  supported->data = false;
+// Checks if the kernel supports TCP FastOpen.
+void SystemSupportsTCPFastOpen(scoped_refptr<SharedBoolean> system_supported) {
+  system_supported->data = false;
   static const base::FilePath::CharType kTCPFastOpenProcFilePath[] =
       "/proc/sys/net/ipv4/tcp_fastopen";
-  std::string system_enabled_tcp_fastopen;
+  std::string system_supports_tcp_fastopen;
   if (!base::ReadFileToString(base::FilePath(kTCPFastOpenProcFilePath),
-                              &system_enabled_tcp_fastopen)) {
+                              &system_supports_tcp_fastopen)) {
     return;
   }
-
-  // As per http://lxr.linux.no/linux+v3.7.7/include/net/tcp.h#L225
-  // TFO_CLIENT_ENABLE is the LSB
-  if (system_enabled_tcp_fastopen.empty() ||
-      (system_enabled_tcp_fastopen[0] & 0x1) == 0) {
+  // The read from /proc should return '1' if TCP FastOpen is enabled in the OS.
+  if (system_supports_tcp_fastopen.empty() ||
+      (system_supports_tcp_fastopen[0] != '1')) {
     return;
   }
-
-  supported->data = true;
+  system_supported->data = true;
 }
 
-void EnableCallback(scoped_refptr<SharedBoolean> supported) {
-  g_tcp_fastopen_enabled = supported->data;
-}
-
-// This is asynchronous because it needs to do file IO, and it isn't allowed to
-// do that on the IO thread.
-void EnableFastOpenIfSupported() {
-  scoped_refptr<SharedBoolean> supported = new SharedBoolean;
-  base::WorkerPool::PostTaskAndReply(
-      FROM_HERE,
-      base::Bind(SystemSupportsTCPFastOpen, supported),
-      base::Bind(EnableCallback, supported),
-      false);
+void CallbackForTCPFastOpenCheck(scoped_refptr<SharedBoolean> system_supported,
+                                 scoped_refptr<SharedBoolean> user_enabled) {
+  g_tcp_fastopen_supported = system_supported->data;
+  if (user_enabled->data == true) {
+    g_tcp_fastopen_enabled = system_supported->data;
+  }
 }
 
 #else
 
-void EnableFastOpenIfSupported() {
-  g_tcp_fastopen_enabled = false;
-}
+void CheckSupportAndMaybeEnableTCPFastOpen(bool user_enabled) {}
 
 #endif
 
 }  // namespace
 
-void SetTCPFastOpenEnabled(bool value) {
-  if (value) {
-    EnableFastOpenIfSupported();
-  } else {
-    g_tcp_fastopen_enabled = false;
-  }
+// This is asynchronous because it needs to do file IO, and it isn't allowed to
+// do that on the IO thread.
+void CheckSupportAndMaybeEnableTCPFastOpen(bool user_enabled_tfo) {
+  scoped_refptr<SharedBoolean> system_supported = new SharedBoolean;
+  scoped_refptr<SharedBoolean> user_enabled = new SharedBoolean;

[Adam Rice, 2014/08/11 05:33:38]

user_enabled can be just an ordinary bool. It is not accessed from another
thread, and the value is not written to from within the callback.

[Randy Smith (Not in Mondays), 2014/08/11 19:07:01]

Hmmm.  Alternatively to using PostTaskAndReply, I think you could use
PostTaskAndReplayWithResult(base::WorkerPool::GetTaskRunner()) and skip any
SharedBooleans at all.  

[mmenke, 2014/08/12 18:44:23]

You can bind just one argument, actually...

base::WorkerPool::PostTaskAndReplyWithResult(
  FROM_HERE,
  // Do you need bind when not passing in arguments?
  base::Bind(SystemSupportsTCPFastOpen),
  base::Bind(CallbackForTCPFastOpenCheck, user_enabled),
  false);

Then just reverse the arguments order for CallbackForTCPFastOpenCheck, and make
them both bools, and you're done.

+  user_enabled->data = user_enabled_tfo;
+  base::WorkerPool::PostTaskAndReply(
+      FROM_HERE,
+      base::Bind(SystemSupportsTCPFastOpen, system_supported),
+      base::Bind(CallbackForTCPFastOpenCheck, system_supported, user_enabled),
+      false);
+}
+
+bool IsTCPFastOpenSupported() {
+  return g_tcp_fastopen_supported;
 }
 
 bool IsTCPFastOpenEnabled() {