Add Socket::ReadIfReady()

net/socket/ssl_client_socket_impl.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/ssl_client_socket_impl.h"
 
 #include <errno.h>
 #include <string.h>
 
 #include <algorithm>
@@ skipping 859 matching lines @@
   } else {
     if (rv > 0)
       was_ever_used_ = true;
     user_read_buf_ = NULL;
     user_read_buf_len_ = 0;
   }
 
   return rv;
 }
 
+int SSLClientSocketImpl::ReadIfReady(IOBuffer* buf,
+                                     int buf_len,
+                                     const CompletionCallback& callback) {
+  // Reuse Read() but do not hold on to |buf| and |buf_len|.
+  int rv = Read(buf, buf_len, callback);
+  user_read_buf_ = nullptr;
+  user_read_buf_len_ = 0;

[davidben, 2017/02/01 22:25:57]

Hrm. Rather than let Read() set up some state and undo it, what if you
implemented Read() on top of ReadIfReady(), like the socket_posix.cc suggestion?

DoPayloadRead is basically socket_posix.cc's DoRead, just it doesn't take
arguments. You could make it take those arguments and have ReadIfReady just pass
them in. RetryAllOperations would check if user_read_buf_ is set and, if so,
call DoPayloadRead on it rather than calling the read callback immediately.

[xunjieli, 2017/02/03 16:35:33]

Done.

+  return rv;
+}
+
 int SSLClientSocketImpl::Write(IOBuffer* buf,
                                int buf_len,
                                const CompletionCallback& callback) {
   user_write_buf_ = buf;
   user_write_buf_len_ = buf_len;
 
   int rv = DoPayloadWrite();
 
   if (rv == ERR_IO_PENDING) {
     user_write_callback_ = callback;
   } else {
     if (rv > 0)
       was_ever_used_ = true;
     user_write_buf_ = NULL;
     user_write_buf_len_ = 0;
   }
 
   return rv;
 }
 
 int SSLClientSocketImpl::SetReceiveBufferSize(int32_t size) {
   return transport_->socket()->SetReceiveBufferSize(size);
 }
 
 int SSLClientSocketImpl::SetSendBufferSize(int32_t size) {
   return transport_->socket()->SetSendBufferSize(size);
 }
 
-void SSLClientSocketImpl::OnReadReady() {
+void SSLClientSocketImpl::OnReadReady(int rv) {
+  // If ReadIfReady() is called instead of Read().

[davidben, 2017/02/01 22:25:57]

This logic might be better in RetryAllOperations(). There's actually a third
case that Read[IfReady] might be blocked, OnPrivateKeyComplete.

This does mean we'll wake up ReadIfReady in cases where we may not need to, but
that's hopefully fine? If we need to later, we can track the exact operation
each side is blocking on.

[xunjieli, 2017/02/03 16:35:33]

Done. Ah, there's a third case! Should have done a grep.

+  if (!user_read_callback_.is_null() && user_read_buf_ == nullptr) {
+    base::ResetAndReturn(&user_read_callback_).Run(rv);
+    return;

[davidben, 2017/02/01 22:25:57]

This return means we won't wake up other operations if we have a Read() and
Write() flowing at the same time.

[xunjieli, 2017/02/03 16:35:33]

Acknowledged.

+  }
   // During a renegotiation, either Read or Write calls may be blocked on a
   // transport read.
   RetryAllOperations();
 }
 
-void SSLClientSocketImpl::OnWriteReady() {
+void SSLClientSocketImpl::OnWriteReady(int rv) {
   // During a renegotiation, either Read or Write calls may be blocked on a
   // transport read.
+  if (!user_read_callback_.is_null() && user_read_buf_ == nullptr) {
+    base::ResetAndReturn(&user_read_callback_).Run(rv);
+    return;
+  }
   RetryAllOperations();
 }
 
 int SSLClientSocketImpl::Init() {
   DCHECK(!ssl_);
 
 #if defined(USE_NSS_CERTS)
   if (ssl_config_.cert_io_enabled) {
     // TODO(davidben): Move this out of SSLClientSocket. See
     // https://crbug.com/539520.
@@ skipping 1113 matching lines @@
     if (ERR_GET_REASON(info->error_code) == SSL_R_TLSV1_ALERT_ACCESS_DENIED &&
         !certificate_requested_) {
       net_error = ERR_SSL_PROTOCOL_ERROR;
     }
   }
 
   return net_error;
 }
 
 }  // namespace net