net: uncork NSS sockets after a 200ms timeout.

We found that, after the recent corking change (r58838), we could end
up not sending the Finished message for an extended period of time.
This would cause servers to time out our SSL connections because they
thought that we were still performing the handshake.

With this change, we'll uncork and flush buffers after 200ms if no
application data is ready.

BUG=58017
TEST=Make HTTPS connections to Google sites. Check that the servers are terminating our idle connections.

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

2 files changed, 28 insertions, 2 deletions

diff --git a/net/socket/ssl_client_socket_nss.cc b/net/socket/ssl_client_socket_nss.cc
index df476e9..f30d56c 100644
--- a/net/socket/ssl_client_socket_nss.cc
+++ b/net/socket/ssl_client_socket_nss.cc
@@ -91,6 +91,12 @@
 
 static const int kRecvBufferSize = 4096;
 
+// kCorkTimeoutMs is the number of milliseconds for which we'll wait for a
+// Write to an SSL socket which we're False Starting. Since corking stops the
+// Finished message from being sent, the server sees an incomplete handshake
+// and some will time out such sockets quite aggressively.
+static const int kCorkTimeoutMs = 200;
+
 namespace net {
 
 // State machines are easier to debug if you log state transitions.
@@ -691,6 +697,14 @@ bool SSLClientSocketNSS::IsNPNProtocolMispredicted() {
   return predicted_npn_proto_ != npn_proto;
 }
 
+void SSLClientSocketNSS::UncorkAfterTimeout() {
+  corked_ = false;
+  int nsent;
+  do {
+    nsent = BufferSend();
+  } while (nsent > 0);
+}
+
 int SSLClientSocketNSS::Connect(CompletionCallback* callback) {
   EnterFunction("");
   DCHECK(transport_.get());
@@ -1108,7 +1122,10 @@ int SSLClientSocketNSS::Write(IOBuffer* buf, int buf_len,
       return rv;
   }
 
-  corked_ = false;
+  if (corked_) {
+    corked_ = false;
+    uncork_timer_.Reset();
+  }
   int rv = DoWriteLoop(OK);
 
   if (rv == ERR_IO_PENDING) {
@@ -1713,8 +1730,12 @@ SECStatus SSLClientSocketNSS::OwnAuthCertHandler(void* arg,
     NOTREACHED();
   if (false_start) {
     SSLClientSocketNSS* that = reinterpret_cast<SSLClientSocketNSS*>(arg);
-    if (!that->handshake_callback_called_)
+    if (!that->handshake_callback_called_) {
       that->corked_ = true;
+      that->uncork_timer_.Start(
+          base::TimeDelta::FromMilliseconds(kCorkTimeoutMs),
+          that, &SSLClientSocketNSS::UncorkAfterTimeout);
+    }
   }
 #endif
 
diff --git a/net/socket/ssl_client_socket_nss.h b/net/socket/ssl_client_socket_nss.h
index 7f5ee94..7b9809b 100644
--- a/net/socket/ssl_client_socket_nss.h
+++ b/net/socket/ssl_client_socket_nss.h
@@ -16,6 +16,7 @@
 
 #include "base/scoped_ptr.h"
 #include "base/time.h"
+#include "base/timer.h"
 #include "net/base/cert_verify_result.h"
 #include "net/base/completion_callback.h"
 #include "net/base/net_log.h"
@@ -102,6 +103,7 @@ class SSLClientSocketNSS : public SSLClientSocket {
   void SaveSnapStartInfo();
   bool LoadSnapStartInfo(const std::string& info);
   bool IsNPNProtocolMispredicted();
+  void UncorkAfterTimeout();
 
   bool DoTransportIO();
   int BufferSend(void);
@@ -130,6 +132,9 @@ class SSLClientSocketNSS : public SSLClientSocket {
   // corked_ is true if we are currently suspending writes to the network. This
   // is named after the similar kernel flag, TCP_CORK.
   bool corked_;
+  // uncork_timer_ is used to limit the amount of time that we'll delay the
+  // Finished message while waiting for a Write.
+  base::OneShotTimer<SSLClientSocketNSS> uncork_timer_;
   scoped_refptr<IOBuffer> recv_buffer_;
 
   CompletionCallbackImpl<SSLClientSocketNSS> handshake_io_callback_;