Add Socket::ReadIfReady()

net/socket/tcp_client_socket.cc

Index: net/socket/tcp_client_socket.cc
diff --git a/net/socket/tcp_client_socket.cc b/net/socket/tcp_client_socket.cc
index 01ce316ee14f54761fa1eefa7da3bc464383fb0a..d56406289822966978f55e9eba712ef04be5e1e6 100644
--- a/net/socket/tcp_client_socket.cc
+++ b/net/socket/tcp_client_socket.cc
@@ -100,6 +100,26 @@ int TCPClientSocket::Connect(const CompletionCallback& callback) {
   return rv;
 }
 
+int TCPClientSocket::ReadCommon(IOBuffer* buf,

[Bence, 2017/03/03 16:33:41]

Consider keeping methods in the same order in the .h and .cc files.

[xunjieli, 2017/03/03 19:41:06]

The thing is that the order is not maintained in this file :( I tried to match
the order by having ReadCommon() above DoConnectLoop() in both h and cc. 

+                                int buf_len,
+                                const CompletionCallback& callback,
+                                bool read_if_ready) {
+  DCHECK(!callback.is_null());
+
+  // |socket_| is owned by this class and the callback won't be run once
+  // |socket_| is gone. Therefore, it is safe to use base::Unretained() here.

[Bence, 2017/03/03 16:33:41]

Optional: Can you please elaborate on "the callback won't be run once |socket_|
is gone"?  Is the callback cancelled every time the |socket_| is closed?  (What
about lines 157 and 211?)  Is the callback cancelled in the destructor?

This is optional because this comment is pre-existing.

[xunjieli, 2017/03/03 19:41:06]

The callback won't be run because the underlying socket won't callback to us
since it's already gone/closed. I added the "closed" case in the comment.
Hopefully that's clearer?

[Bence, 2017/03/06 23:43:14]

Yes, thank you.

+  CompletionCallback read_callback = base::Bind(
+      &TCPClientSocket::DidCompleteRead, base::Unretained(this), callback);
+  int result = read_if_ready ? socket_->ReadIfReady(buf, buf_len, read_callback)
+                             : socket_->Read(buf, buf_len, read_callback);
+  if (result > 0) {
+    use_history_.set_was_used_to_convey_data();
+    total_received_bytes_ += result;
+  }
+
+  return result;
+}
+
 int TCPClientSocket::DoConnectLoop(int result) {
   DCHECK_NE(next_connect_state_, CONNECT_STATE_NONE);
 
@@ -272,19 +292,13 @@ bool TCPClientSocket::GetSSLInfo(SSLInfo* ssl_info) {
 int TCPClientSocket::Read(IOBuffer* buf,
                           int buf_len,
                           const CompletionCallback& callback) {
-  DCHECK(!callback.is_null());
-
-  // |socket_| is owned by this class and the callback won't be run once
-  // |socket_| is gone. Therefore, it is safe to use base::Unretained() here.
-  CompletionCallback read_callback = base::Bind(
-      &TCPClientSocket::DidCompleteRead, base::Unretained(this), callback);
-  int result = socket_->Read(buf, buf_len, read_callback);
-  if (result > 0) {
-    use_history_.set_was_used_to_convey_data();
-    total_received_bytes_ += result;
-  }
+  return ReadCommon(buf, buf_len, callback, /*read_if_ready=*/false);
+}
 
-  return result;
+int TCPClientSocket::ReadIfReady(IOBuffer* buf,
+                                 int buf_len,
+                                 const CompletionCallback& callback) {
+  return ReadCommon(buf, buf_len, callback, /*read_if_ready=*/true);
 }
 
 int TCPClientSocket::Write(IOBuffer* buf,