Allow DoLoop to be called even while no user

request is pending.   Unlike the Windows implementation,
our DoLoop eagerly initiates I/O between its internal
buffers and the network, and we have to be ready to
pump data when that completes.

Also add debugging macros, but leave them disabled
until the next time somebody has to understand the state machine.

Fixes http://code.google.com/p/chromium/issues/detail?id=4061
Review URL: http://codereview.chromium.org/9152

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

1 files changed, 97 insertions, 21 deletions

diff --git a/net/base/ssl_client_socket_nss.cc b/net/base/ssl_client_socket_nss.cc
index 44fd1e9..c954ec9 100644
--- a/net/base/ssl_client_socket_nss.cc
+++ b/net/base/ssl_client_socket_nss.cc
@@ -38,6 +38,21 @@ ownBadCertHandler(void * arg, PRFileDesc * socket)
 
 namespace net {
 
+// State machines are easier to debug if you log state transitions.
+// Enable these if you want to see what's going on.
+#if 1
+#define EnterFunction(x)
+#define LeaveFunction(x)
+#define GotoState(s) next_state_ = s
+#else
+#define EnterFunction(x)  LOG(INFO) << (void *)this << " " << __FUNCTION__ << \
+                           " enter " << x << "; next_state " << next_state_
+#define LeaveFunction(x)  LOG(INFO) << (void *)this << " " << __FUNCTION__ << \
+                           " leave " << x << "; next_state " << next_state_
+#define GotoState(s) do { LOG(INFO) << (void *)this << " " << __FUNCTION__ << \
+                           " jump to state " << s; next_state_ = s; } while (0)
+#endif
+
 bool SSLClientSocketNSS::nss_options_initialized_ = false;
 
 SSLClientSocketNSS::SSLClientSocketNSS(ClientSocket* transport_socket,
@@ -59,37 +74,48 @@ SSLClientSocketNSS::SSLClientSocketNSS(ClientSocket* transport_socket,
       next_state_(STATE_NONE),
       nss_fd_(NULL),
       nss_bufs_(NULL) {
+  EnterFunction("");
 }
 
 SSLClientSocketNSS::~SSLClientSocketNSS() {
+  EnterFunction("");
   Disconnect();
+  LeaveFunction("");
 }
 
 int SSLClientSocketNSS::Init() {
-   // Call NSS_NoDB_Init() in a threadsafe way.
-   base::EnsureNSSInit();
+  EnterFunction("");
+  // Call NSS_NoDB_Init() in a threadsafe way.
+  base::EnsureNSSInit();
 
-   return OK;
+  LeaveFunction("");
+  return OK;
 }
 
 int SSLClientSocketNSS::Connect(CompletionCallback* callback) {
+  EnterFunction("");
   DCHECK(transport_.get());
   DCHECK(next_state_ == STATE_NONE);
   DCHECK(!user_callback_);
 
-  next_state_ = STATE_CONNECT;
+  GotoState(STATE_CONNECT);
   int rv = DoLoop(OK);
   if (rv == ERR_IO_PENDING)
     user_callback_ = callback;
+
+  LeaveFunction("");
   return rv;
 }
 
 int SSLClientSocketNSS::ReconnectIgnoringLastError(CompletionCallback* callback) {
+  EnterFunction("");
   // TODO(darin): implement me!
+  LeaveFunction("");
   return ERR_FAILED;
 }
 
 void SSLClientSocketNSS::Disconnect() {
+  EnterFunction("");
   // TODO(wtc): Send SSL close_notify alert.
   if (nss_fd_ != NULL) {
     PR_Close(nss_fd_);
@@ -97,50 +123,63 @@ void SSLClientSocketNSS::Disconnect() {
   }
   completed_handshake_ = false;
   transport_->Disconnect();
+  LeaveFunction("");
 }
 
 bool SSLClientSocketNSS::IsConnected() const {
-  return completed_handshake_ && transport_->IsConnected();
+  EnterFunction("");
+  bool ret = completed_handshake_ && transport_->IsConnected();
+  LeaveFunction("");
+  return ret;
 }
 
 int SSLClientSocketNSS::Read(char* buf, int buf_len,
                           CompletionCallback* callback) {
+  EnterFunction(buf_len);
   DCHECK(completed_handshake_);
   DCHECK(next_state_ == STATE_NONE);
   DCHECK(!user_callback_);
+  DCHECK(!user_buf_);
 
   user_buf_ = buf;
   user_buf_len_ = buf_len;
 
-  next_state_ = STATE_PAYLOAD_READ;
+  GotoState(STATE_PAYLOAD_READ);
   int rv = DoLoop(OK);
   if (rv == ERR_IO_PENDING)
     user_callback_ = callback;
+  LeaveFunction("");
   return rv;
 }
 
 int SSLClientSocketNSS::Write(const char* buf, int buf_len,
                            CompletionCallback* callback) {
+  EnterFunction(buf_len);
   DCHECK(completed_handshake_);
   DCHECK(next_state_ == STATE_NONE);
   DCHECK(!user_callback_);
+  DCHECK(!user_buf_);
 
   user_buf_ = const_cast<char*>(buf);
   user_buf_len_ = buf_len;
 
-  next_state_ = STATE_PAYLOAD_WRITE;
+  GotoState(STATE_PAYLOAD_WRITE);
   int rv = DoLoop(OK);
   if (rv == ERR_IO_PENDING)
     user_callback_ = callback;
+  LeaveFunction("");
   return rv;
 }
 
 void SSLClientSocketNSS::GetSSLInfo(SSLInfo* ssl_info) {
+  EnterFunction("");
   // TODO(port): implement!
   ssl_info->Reset();
+  LeaveFunction("");
 }
 
 void SSLClientSocketNSS::DoCallback(int rv) {
+  EnterFunction(rv);
   DCHECK(rv != ERR_IO_PENDING);
   DCHECK(user_callback_);
 
@@ -148,12 +187,15 @@ void SSLClientSocketNSS::DoCallback(int rv) {
   CompletionCallback* c = user_callback_;
   user_callback_ = NULL;
   c->Run(rv);
+  LeaveFunction("");
 }
 
 void SSLClientSocketNSS::OnIOComplete(int result) {
+  EnterFunction(result);
   int rv = DoLoop(result);
-  if (rv != ERR_IO_PENDING)
+  if (rv != ERR_IO_PENDING && user_callback_ != NULL)
     DoCallback(rv);
+  LeaveFunction("");
 }
 
 // Map a Chromium net error code to an NSS error code
@@ -178,6 +220,7 @@ int SSLClientSocketNSS::BufferSend(void) {
 
   const char *buf;
   int nb = memio_GetWriteParams(nss_bufs_, &buf);
+  EnterFunction(nb);
 
   int rv;
   if (!nb) {
@@ -190,22 +233,25 @@ int SSLClientSocketNSS::BufferSend(void) {
       memio_PutWriteResult(nss_bufs_, MapErrorToNSS(rv));
   }
 
+  LeaveFunction(rv);
   return rv;
 }
 
 void SSLClientSocketNSS::BufferSendComplete(int result) {
+  EnterFunction(result);
   memio_PutWriteResult(nss_bufs_, result);
   transport_send_busy_ = false;
   OnIOComplete(result);
+  LeaveFunction("");
 }
 
 
 int SSLClientSocketNSS::BufferRecv(void) {
-
   if (transport_recv_busy_) return ERR_IO_PENDING;
 
   char *buf;
   int nb = memio_GetReadParams(nss_bufs_, &buf);
+  EnterFunction(nb);
   int rv;
   if (!nb) {
     // buffer too full to read into, so no I/O possible at moment
@@ -217,27 +263,36 @@ int SSLClientSocketNSS::BufferRecv(void) {
     else
       memio_PutReadResult(nss_bufs_, MapErrorToNSS(rv));
   }
-
+  LeaveFunction(rv);
   return rv;
 }
 
 void SSLClientSocketNSS::BufferRecvComplete(int result) {
+  EnterFunction(result);
   memio_PutReadResult(nss_bufs_, result);
   transport_recv_busy_ = false;
   OnIOComplete(result);
+  LeaveFunction("");
 }
 
 
 int SSLClientSocketNSS::DoLoop(int last_io_result) {
-  DCHECK(next_state_ != STATE_NONE);
+  EnterFunction(last_io_result);
   bool network_moved;
   int rv = last_io_result;
   do {
     network_moved = false;
+    // Default to STATE_NONE for next state.
+    // (This is a quirk carried over from the windows 
+    // implementation.  It makes reading the logs a bit harder.)
+    // State handlers can and often do call GotoState just 
+    // to stay in the current state.
     State state = next_state_;
-    //DLOG(INFO) << "DoLoop state " << state;
-    next_state_ = STATE_NONE;
+    GotoState(STATE_NONE);
     switch (state) {
+      case STATE_NONE:
+        // we're just pumping data between the buffer and the network
+        break;
       case STATE_CONNECT:
         rv = DoConnect();
         break;
@@ -266,15 +321,18 @@ int SSLClientSocketNSS::DoLoop(int last_io_result) {
       network_moved = (nsent > 0 || nreceived >= 0);
     }
   } while ((rv != ERR_IO_PENDING || network_moved) && next_state_ != STATE_NONE);
+  LeaveFunction("");
   return rv;
 }
 
 int SSLClientSocketNSS::DoConnect() {
-  next_state_ = STATE_CONNECT_COMPLETE;
+  EnterFunction("");
+  GotoState(STATE_CONNECT_COMPLETE);
   return transport_->Connect(&io_callback_);
 }
 
 int SSLClientSocketNSS::DoConnectComplete(int result) {
+  EnterFunction(result);
   if (result < 0)
     return result;
 
@@ -341,52 +399,70 @@ int SSLClientSocketNSS::DoConnectComplete(int result) {
 
   // Tell SSL we're a client; needed if not letting NSPR do socket I/O
   SSL_ResetHandshake(nss_fd_, 0);
-  next_state_ = STATE_HANDSHAKE_READ;
+  GotoState(STATE_HANDSHAKE_READ);
   // Return OK so DoLoop tries handshaking
+  LeaveFunction("");
   return OK;
 }
 
 int SSLClientSocketNSS::DoHandshakeRead() {
+  EnterFunction("");
   int rv = SSL_ForceHandshake(nss_fd_);
   if (rv == SECSuccess) {
     // there's a callback for this, too
     completed_handshake_ = true;
     // Indicate we're ready to handle I/O.  Badly named?
-    next_state_ = STATE_NONE;
+    GotoState(STATE_NONE);
+    LeaveFunction("");
     return OK;
   }
   PRErrorCode prerr = PR_GetError();
   if (prerr == PR_WOULD_BLOCK_ERROR) {
     // at this point, it should have tried to send some bytes
-    next_state_ = STATE_HANDSHAKE_READ;
+    GotoState(STATE_HANDSHAKE_READ);
+    LeaveFunction("");
     return ERR_IO_PENDING;
   }
   // TODO: map rv to net error code properly
+  LeaveFunction("");
   return ERR_SSL_PROTOCOL_ERROR;
 }
     
 int SSLClientSocketNSS::DoPayloadRead() {
+  EnterFunction(user_buf_len_);
   int rv = PR_Read(nss_fd_, user_buf_, user_buf_len_);
-  if (rv >= 0)
+  if (rv >= 0) {
+    user_buf_ = NULL;
+    LeaveFunction("");
     return rv;
+  }
   PRErrorCode prerr = PR_GetError();
   if (prerr == PR_WOULD_BLOCK_ERROR) {
-    next_state_ = STATE_PAYLOAD_READ;
+    GotoState(STATE_PAYLOAD_READ);
+    LeaveFunction("");
     return ERR_IO_PENDING;
   }
+  user_buf_ = NULL;
+  LeaveFunction("");
   // TODO: map rv to net error code properly
   return ERR_SSL_PROTOCOL_ERROR;
 }
 
 int SSLClientSocketNSS::DoPayloadWrite() {
+  EnterFunction(user_buf_len_);
   int rv = PR_Write(nss_fd_, user_buf_, user_buf_len_);
-  if (rv >= 0)
+  if (rv >= 0) {
+    user_buf_ = NULL;
+    LeaveFunction("");
     return rv;
+  }
   PRErrorCode prerr = PR_GetError();
   if (prerr == PR_WOULD_BLOCK_ERROR) {
-    next_state_ = STATE_PAYLOAD_WRITE;
+    GotoState(STATE_PAYLOAD_WRITE);
     return ERR_IO_PENDING;
   }
+  user_buf_ = NULL;
+  LeaveFunction("");
   // TODO: map rv to net error code properly
   return ERR_SSL_PROTOCOL_ERROR;
 }