Add Socket::ReadIfReady()

net/socket/ssl_client_socket_impl.cc

Index: net/socket/ssl_client_socket_impl.cc
diff --git a/net/socket/ssl_client_socket_impl.cc b/net/socket/ssl_client_socket_impl.cc
index c993fa8fb23da2d1d58c35731b3801a19283ff11..4e6924446494db2503173078443a1ac0a859f3ea 100644
--- a/net/socket/ssl_client_socket_impl.cc
+++ b/net/socket/ssl_client_socket_impl.cc
@@ -877,6 +877,16 @@ int SSLClientSocketImpl::Read(IOBuffer* buf,
   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) {
@@ -905,15 +915,24 @@ 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();
 }