Add Socket::ReadIfReady()

net/socket/ssl_client_socket_unittest.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.h"
 
 #include <errno.h>
 #include <string.h>
 
+#include <algorithm>
 #include <utility>
 
 #include "base/callback_helpers.h"
 #include "base/files/file_util.h"
 #include "base/location.h"
 #include "base/macros.h"
 #include "base/memory/ref_counted.h"
 #include "base/message_loop/message_loop.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
+#include "base/test/scoped_feature_list.h"
 #include "base/test/scoped_task_scheduler.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "base/time/time.h"
 #include "base/values.h"
 #include "net/base/address_list.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 #include "net/base/test_completion_callback.h"
 #include "net/cert/asn1_util.h"
 #include "net/cert/ct_policy_enforcer.h"
@@ skipping 105 matching lines @@
   int64_t GetTotalReceivedBytes() const override {
     return transport_->GetTotalReceivedBytes();
   }
 
   // Socket implementation:
   int Read(IOBuffer* buf,
            int buf_len,
            const CompletionCallback& callback) override {
     return transport_->Read(buf, buf_len, callback);
   }
+  int ReadIfReady(IOBuffer* buf,
+                  int buf_len,
+                  const CompletionCallback& callback) override {
+    return transport_->ReadIfReady(buf, buf_len, callback);
+  }
   int Write(IOBuffer* buf,
             int buf_len,
             const CompletionCallback& callback) override {
     return transport_->Write(buf, buf_len, callback);
   }
   int SetReceiveBufferSize(int32_t size) override {
     return transport_->SetReceiveBufferSize(size);
   }
   int SetSendBufferSize(int32_t size) override {
     return transport_->SetSendBufferSize(size);
@@ skipping 12 matching lines @@
 class ReadBufferingStreamSocket : public WrappedStreamSocket {
  public:
   explicit ReadBufferingStreamSocket(std::unique_ptr<StreamSocket> transport);
   ~ReadBufferingStreamSocket() override {}
 
   // Socket implementation:
   int Read(IOBuffer* buf,
            int buf_len,
            const CompletionCallback& callback) override;
 
+  int ReadIfReady(IOBuffer* buf,
+                  int buf_len,
+                  const CompletionCallback& callback) override;
+
   // Sets the internal buffer to |size|. This must not be greater than
   // the largest value supplied to Read() - that is, it does not handle
   // having "leftovers" at the end of Read().
   // Each call to Read() will be prevented from completion until at least
   // |size| data has been read.
   // Set to 0 to turn off buffering, causing Read() to transparently
   // read via the underlying transport.
   void SetBufferSize(int size);
 
  private:
@@ skipping 16 matching lines @@
   CompletionCallback user_read_callback_;
 };
 
 ReadBufferingStreamSocket::ReadBufferingStreamSocket(
     std::unique_ptr<StreamSocket> transport)
     : WrappedStreamSocket(std::move(transport)),
       read_buffer_(new GrowableIOBuffer()),
       buffer_size_(0) {}
 
 void ReadBufferingStreamSocket::SetBufferSize(int size) {
-  DCHECK(!user_read_buf_.get());
+  DCHECK(!user_read_buf_);
   buffer_size_ = size;
   read_buffer_->SetCapacity(size);
 }
 
 int ReadBufferingStreamSocket::Read(IOBuffer* buf,
                                     int buf_len,
                                     const CompletionCallback& callback) {
+  DCHECK(!user_read_buf_);
   if (buffer_size_ == 0)
     return transport_->Read(buf, buf_len, callback);
+  int rv = ReadIfReady(buf, buf_len, callback);
+  if (rv == ERR_IO_PENDING)
+    user_read_buf_ = buf;
+  return rv;
+}
+
+int ReadBufferingStreamSocket::ReadIfReady(IOBuffer* buf,
+                                           int buf_len,
+                                           const CompletionCallback& callback) {
+  DCHECK(!user_read_buf_);
+  if (buffer_size_ == 0)
+    return transport_->ReadIfReady(buf, buf_len, callback);
+
+  if (read_buffer_->RemainingCapacity() == 0) {
+    memcpy(buf->data(), read_buffer_->StartOfBuffer(),
+           read_buffer_->capacity());
+    read_buffer_->set_offset(0);
+    return read_buffer_->capacity();
+  }
 
   if (buf_len < buffer_size_)
     return ERR_UNEXPECTED;
-

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

Optional: maybe keep this empty like for better readability.

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

Done.

   state_ = STATE_READ;
-  user_read_buf_ = buf;
-  int result = DoLoop(OK);
-  if (result == ERR_IO_PENDING)
+  int rv = DoLoop(OK);
+  if (rv == ERR_IO_PENDING)
     user_read_callback_ = callback;
-  else
-    user_read_buf_ = NULL;
-  return result;
+  return rv;
 }
 
 int ReadBufferingStreamSocket::DoLoop(int result) {
   int rv = result;
   do {
     State current_state = state_;
     state_ = STATE_NONE;
     switch (current_state) {
       case STATE_READ:
         rv = DoRead();
         break;
       case STATE_READ_COMPLETE:
         rv = DoReadComplete(rv);
         break;
       case STATE_NONE:
       default:
         NOTREACHED() << "Unexpected state: " << current_state;
         rv = ERR_UNEXPECTED;
         break;
     }
   } while (rv != ERR_IO_PENDING && state_ != STATE_NONE);
   return rv;
 }
 
 int ReadBufferingStreamSocket::DoRead() {
   state_ = STATE_READ_COMPLETE;
-  int rv =
-      transport_->Read(read_buffer_.get(),
-                       read_buffer_->RemainingCapacity(),
-                       base::Bind(&ReadBufferingStreamSocket::OnReadCompleted,
-                                  base::Unretained(this)));
-  return rv;
+  return transport_->Read(

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

Nice!

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

Acknowledged.

+      read_buffer_.get(), read_buffer_->RemainingCapacity(),
+      base::Bind(&ReadBufferingStreamSocket::OnReadCompleted,
+                 base::Unretained(this)));
 }
 
 int ReadBufferingStreamSocket::DoReadComplete(int result) {
   state_ = STATE_NONE;
+
   if (result <= 0)
     return result;
 
   read_buffer_->set_offset(read_buffer_->offset() + result);
   if (read_buffer_->RemainingCapacity() > 0) {
     state_ = STATE_READ;
     return OK;
   }
 
+  // If ReadIfReady() is used.

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

Read() can call into ReadIfReady() which can return asynchronously in which case
ReadIfReady() was used but |user_read_buf_ != nullptr|.  Is there a way to
modify this comment?  Maybe something along the lines of "ReadIfReady() already
read from the buffer synchronously." (with which wording it should be after the
|if|, not before).

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

Done.

+  if (user_read_buf_ == nullptr)
+    return OK;
+
   memcpy(user_read_buf_->data(),
          read_buffer_->StartOfBuffer(),
          read_buffer_->capacity());
   read_buffer_->set_offset(0);
   return read_buffer_->capacity();
 }
 
 void ReadBufferingStreamSocket::OnReadCompleted(int result) {
+  DCHECK_NE(ERR_IO_PENDING, result);
+  DCHECK(user_read_callback_);
+
   result = DoLoop(result);
   if (result == ERR_IO_PENDING)
     return;
-
-  user_read_buf_ = NULL;
+  user_read_buf_ = nullptr;

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

Nice!

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

Acknowledged.

   base::ResetAndReturn(&user_read_callback_).Run(result);
 }
 
 // Simulates synchronously receiving an error during Read() or Write()
 class SynchronousErrorStreamSocket : public WrappedStreamSocket {
  public:
   explicit SynchronousErrorStreamSocket(std::unique_ptr<StreamSocket> transport)
       : WrappedStreamSocket(std::move(transport)) {}
   ~SynchronousErrorStreamSocket() override {}
 
   // Socket implementation:
   int Read(IOBuffer* buf,
            int buf_len,
            const CompletionCallback& callback) override;
+  int ReadIfReady(IOBuffer* buf,
+                  int buf_len,
+                  const CompletionCallback& callback) override;
   int Write(IOBuffer* buf,
             int buf_len,
             const CompletionCallback& callback) override;
 
   // Sets the next Read() call and all future calls to return |error|.
   // If there is already a pending asynchronous read, the configured error
   // will not be returned until that asynchronous read has completed and Read()
   // is called again.
   void SetNextReadError(int error) {
     DCHECK_GE(0, error);
@@ skipping 22 matching lines @@
 };
 
 int SynchronousErrorStreamSocket::Read(IOBuffer* buf,
                                        int buf_len,
                                        const CompletionCallback& callback) {
   if (have_read_error_)
     return pending_read_error_;
   return transport_->Read(buf, buf_len, callback);
 }
 
+int SynchronousErrorStreamSocket::ReadIfReady(
+    IOBuffer* buf,
+    int buf_len,
+    const CompletionCallback& callback) {
+  if (have_read_error_)
+    return pending_read_error_;
+  return transport_->ReadIfReady(buf, buf_len, callback);
+}
+
 int SynchronousErrorStreamSocket::Write(IOBuffer* buf,
                                         int buf_len,
                                         const CompletionCallback& callback) {
   if (have_write_error_)
     return pending_write_error_;
   return transport_->Write(buf, buf_len, callback);
 }
 
 // FakeBlockingStreamSocket wraps an existing StreamSocket and simulates the
 // underlying transport needing to complete things asynchronously in a
 // deterministic manner (e.g.: independent of the TestServer and the OS's
 // semantics).
 class FakeBlockingStreamSocket : public WrappedStreamSocket {
  public:
   explicit FakeBlockingStreamSocket(std::unique_ptr<StreamSocket> transport)
       : WrappedStreamSocket(std::move(transport)) {}
   ~FakeBlockingStreamSocket() override {}
 
   // Socket implementation:
   int Read(IOBuffer* buf,
            int buf_len,
            const CompletionCallback& callback) override;
+  int ReadIfReady(IOBuffer* buf,
+                  int buf_len,
+                  const CompletionCallback& callback) override;
   int Write(IOBuffer* buf,
             int buf_len,
             const CompletionCallback& callback) override;
 
   int pending_read_result() const { return pending_read_result_; }
   IOBuffer* pending_read_buf() const { return pending_read_buf_.get(); }
 
   // Blocks read results on the socket. Reads will not complete until
   // UnblockReadResult() has been called and a result is ready from the
   // underlying transport. Note: if BlockReadResult() is called while there is a
@@ skipping 17 matching lines @@
   void BlockWrite();
   void UnblockWrite();
 
   // Waits for the blocked Write() call to be scheduled.
   void WaitForWrite();
 
  private:
   // Handles completion from the underlying transport read.
   void OnReadCompleted(int result);
 
+  // Handles async completion of ReadIfReady().
+  void CompleteReadIfReady(scoped_refptr<IOBuffer> buffer, int rv);
+
   // Finishes the current read.
   void ReturnReadResult();
 
   // True if read callbacks are blocked.
   bool should_block_read_ = false;
 
+  // Used to buffer result returned by a completed ReadIfReady().
+  std::string read_if_ready_buf_;
+
+  // Non-null if there is a pending ReadIfReady().
+  CompletionCallback read_if_ready_callback_;
+
   // The buffer for the pending read, or NULL if not consumed.
   scoped_refptr<IOBuffer> pending_read_buf_;
 
   // The size of the pending read buffer, or -1 if not set.
   int pending_read_buf_len_ = -1;
 
   // The user callback for the pending read call.
   CompletionCallback pending_read_callback_;
 
   // The result for the blocked read callback, or ERR_IO_PENDING if not
@@ skipping 20 matching lines @@
 };
 
 int FakeBlockingStreamSocket::Read(IOBuffer* buf,
                                    int len,
                                    const CompletionCallback& callback) {
   DCHECK(!pending_read_buf_);
   DCHECK(pending_read_callback_.is_null());
   DCHECK_EQ(ERR_IO_PENDING, pending_read_result_);
   DCHECK(!callback.is_null());
 
-  int rv = transport_->Read(buf, len, base::Bind(
-      &FakeBlockingStreamSocket::OnReadCompleted, base::Unretained(this)));
+  int rv =
+      transport_->Read(buf, len,
+                       base::Bind(&FakeBlockingStreamSocket::OnReadCompleted,
+                                  base::Unretained(this)));
   if (rv == ERR_IO_PENDING || should_block_read_) {
     // Save the callback to be called later.
     pending_read_buf_ = buf;
     pending_read_buf_len_ = len;
     pending_read_callback_ = callback;
     // Save the read result.
     if (rv != ERR_IO_PENDING) {
       OnReadCompleted(rv);
       rv = ERR_IO_PENDING;
     }
   }
   return rv;
 }
 
+int FakeBlockingStreamSocket::ReadIfReady(IOBuffer* buf,
+                                          int len,
+                                          const CompletionCallback& callback) {
+  if (!read_if_ready_buf_.empty()) {
+    // If ReadIfReady() is used, asynchronous reads with a large enough buffer
+    // and no BlockReadResult() are supported by this class. Explicitly check
+    // that |should_block_read_| doesn't apply and |len| is greater than the
+    // size of the buffered data.
+    CHECK(!should_block_read_);
+    CHECK_GE(len, static_cast<int>(read_if_ready_buf_.size()));
+    int rv = read_if_ready_buf_.size();
+    memcpy(buf->data(), read_if_ready_buf_.data(), rv);
+    read_if_ready_buf_.clear();
+    return rv;
+  }
+  scoped_refptr<IOBuffer> buf_copy = new IOBuffer(len);
+  int rv = Read(buf_copy.get(), len,
+                base::Bind(&FakeBlockingStreamSocket::CompleteReadIfReady,
+                           base::Unretained(this), buf_copy));
+  if (rv > 0)
+    memcpy(buf->data(), buf_copy->data(), rv);
+  if (rv == ERR_IO_PENDING)
+    read_if_ready_callback_ = callback;
+  return rv;
+}
+
 int FakeBlockingStreamSocket::Write(IOBuffer* buf,
                                     int len,
                                     const CompletionCallback& callback) {
   DCHECK(buf);
   DCHECK_LE(0, len);
 
   if (!should_block_write_)
     return transport_->Write(buf, len, callback);
 
   // Schedule the write, but do nothing.
@@ skipping 97 matching lines @@
   if (should_block_read_) {
     // Defer the result until UnblockReadResult is called.
     if (read_loop_)
       read_loop_->Quit();
     return;
   }
 
   ReturnReadResult();
 }
 
+void FakeBlockingStreamSocket::CompleteReadIfReady(scoped_refptr<IOBuffer> buf,
+                                                   int rv) {
+  DCHECK(read_if_ready_callback_);
+  DCHECK(read_if_ready_buf_.empty());
+  DCHECK(!should_block_read_);
+  if (rv > 0)
+    read_if_ready_buf_ = std::string(buf->data(), buf->data() + rv);
+  base::ResetAndReturn(&read_if_ready_callback_).Run(rv > 0 ? OK : rv);
+}
+
 void FakeBlockingStreamSocket::ReturnReadResult() {
   int result = pending_read_result_;
   pending_read_result_ = ERR_IO_PENDING;
   pending_read_buf_ = nullptr;
   pending_read_buf_len_ = -1;
   base::ResetAndReturn(&pending_read_callback_).Run(result);
 }
 
 // CountingStreamSocket wraps an existing StreamSocket and maintains a count of
 // reads and writes on the socket.
@@ skipping 254 matching lines @@
   SSLClientSocketContext context_;
   std::unique_ptr<SSLClientSocket> sock_;
   TestNetLog log_;
 
  private:
   std::unique_ptr<SpawnedTestServer> spawned_test_server_;
   TestCompletionCallback callback_;
   AddressList addr_;
 };
 
+// If GetParam(), try ReadIfReady() and fall back to Read() if needed.

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

I agree that introducing a two-entry enum would be an overkill for this test
parameter.  How about introducing a private const bool enable_read_if_ready_
member in SSLClientSocketReadTest, initializing it in the constructor
initializer list with GetParam(), and using that instead of GetParam()
everywhere else.  (This advice is inspired by
https://big.corp.google.com/~jmcmaster/testing/2017/02/episode-465-to-comment...)

Then this comment could be moved to right above that member declaration.

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

Done.

+class SSLClientSocketReadTest : public SSLClientSocketTest,
+                                public ::testing::WithParamInterface<bool> {
+ protected:
+  void SetUp() override {
+    if (GetParam())
+      scoped_feature_list_.InitAndEnableFeature(Socket::kReadIfReadyExperiment);
+  }
+
+  // Convienient wrapper to call Read()/ReadIfReady() depending on GetParam().
+  int Read(StreamSocket* socket,
+           IOBuffer* buf,
+           int buf_len,
+           const CompletionCallback& callback) {
+    if (GetParam())
+      return socket->ReadIfReady(buf, buf_len, callback);
+    return socket->Read(buf, buf_len, callback);
+  }
+
+  // Wait for Read()/ReadIfReady() to complete.
+  int WaitForReadCompletion(StreamSocket* socket,
+                            IOBuffer* buf,
+                            int buf_len,
+                            TestCompletionCallback* callback,
+                            int rv) {
+    if (!GetParam())
+      return callback->GetResult(rv);
+    while (rv == ERR_IO_PENDING) {
+      rv = callback->GetResult(rv);
+      if (rv != OK)
+        return rv;
+      rv = socket->ReadIfReady(buf, buf_len, callback->callback());
+    }
+    return rv;
+  }
+
+  // Calls Read()/ReadIfReady() and waits for it to return data.
+  int ReadAndWaitForCompletion(StreamSocket* socket,
+                               IOBuffer* buf,
+                               int buf_len) {
+    TestCompletionCallback callback;
+    int rv = Read(socket, buf, buf_len, callback.callback());
+    return WaitForReadCompletion(socket, buf, buf_len, &callback, rv);
+  }
+
+ private:
+  base::test::ScopedFeatureList scoped_feature_list_;
+};
+
+INSTANTIATE_TEST_CASE_P(/* no prefix */,
+                        SSLClientSocketReadTest,
+                        ::testing::Bool());
+
 // Verifies the correctness of GetSSLCertRequestInfo.
 class SSLClientSocketCertRequestInfoTest : public SSLClientSocketTest {
  protected:
   // Creates a test server with the given SSLOptions, connects to it and returns
   // the SSLCertRequestInfo reported by the socket.
   scoped_refptr<SSLCertRequestInfo> GetCertRequest(
       SpawnedTestServer::SSLOptions ssl_options) {
     SpawnedTestServer spawned_test_server(SpawnedTestServer::TYPE_HTTPS,
                                           ssl_options, base::FilePath());
     if (!spawned_test_server.Start())
@@ skipping 328 matching lines @@
   EXPECT_FALSE(sock_->IsConnected());
 }
 
 // TODO(wtc): Add unit tests for IsConnectedAndIdle:
 //   - Server closes an SSL connection (with a close_notify alert message).
 //   - Server closes the underlying TCP connection directly.
 //   - Server sends data unexpectedly.
 
 // Tests that the socket can be read from successfully. Also test that a peer's
 // close_notify alert is successfully processed without error.
-TEST_F(SSLClientSocketTest, Read) {
+TEST_P(SSLClientSocketReadTest, Read) {
   ASSERT_TRUE(StartTestServer(SpawnedTestServer::SSLOptions()));
 
   TestCompletionCallback callback;
   std::unique_ptr<StreamSocket> transport(
       new TCPClientSocket(addr(), NULL, NULL, NetLogSource()));
   EXPECT_EQ(0, transport->GetTotalReceivedBytes());
 
   int rv = callback.GetResult(transport->Connect(callback.callback()));
   EXPECT_THAT(rv, IsOk());
 
@@ skipping 15 matching lines @@
   memcpy(request_buffer->data(), request_text, arraysize(request_text) - 1);
 
   rv = callback.GetResult(sock->Write(
       request_buffer.get(), arraysize(request_text) - 1, callback.callback()));
   EXPECT_EQ(static_cast<int>(arraysize(request_text) - 1), rv);
 
   scoped_refptr<IOBuffer> buf(new IOBuffer(4096));
   int64_t unencrypted_bytes_read = 0;
   int64_t network_bytes_read_during_handshake = sock->GetTotalReceivedBytes();
   do {
-    rv = callback.GetResult(sock->Read(buf.get(), 4096, callback.callback()));
+    rv = ReadAndWaitForCompletion(sock.get(), buf.get(), 4096);
     EXPECT_GE(rv, 0);
     if (rv >= 0) {
       unencrypted_bytes_read += rv;
     }
   } while (rv > 0);
   EXPECT_GT(unencrypted_bytes_read, 0);
   // Reading the payload should increase the number of bytes on network layer.
   EXPECT_GT(sock->GetTotalReceivedBytes(), network_bytes_read_during_handshake);
   // Number of bytes received on the network after the handshake should be
   // higher than the number of encrypted bytes read.
@@ skipping 30 matching lines @@
 
   rv = callback.GetResult(sock->Connect(callback.callback()));
   EXPECT_THAT(rv, IsError(ERR_CONNECTION_RESET));
   EXPECT_FALSE(sock->IsConnected());
 }
 
 // Tests that the SSLClientSocket properly handles when the underlying transport
 // synchronously returns an error code - such as if an intermediary terminates
 // the socket connection uncleanly.
 // This is a regression test for http://crbug.com/238536
-TEST_F(SSLClientSocketTest, Read_WithSynchronousError) {
+TEST_P(SSLClientSocketReadTest, Read_WithSynchronousError) {
   ASSERT_TRUE(StartTestServer(SpawnedTestServer::SSLOptions()));
 
   TestCompletionCallback callback;
   std::unique_ptr<StreamSocket> real_transport(
       new TCPClientSocket(addr(), NULL, NULL, NetLogSource()));
   std::unique_ptr<SynchronousErrorStreamSocket> transport(
       new SynchronousErrorStreamSocket(std::move(real_transport)));
   int rv = callback.GetResult(transport->Connect(callback.callback()));
   EXPECT_THAT(rv, IsOk());
 
@@ skipping 21 matching lines @@
   EXPECT_EQ(kRequestTextSize, rv);
 
   // Simulate an unclean/forcible shutdown.
   raw_transport->SetNextReadError(ERR_CONNECTION_RESET);
 
   scoped_refptr<IOBuffer> buf(new IOBuffer(4096));
 
   // Note: This test will hang if this bug has regressed. Simply checking that
   // rv != ERR_IO_PENDING is insufficient, as ERR_IO_PENDING is a legitimate
   // result when using a dedicated task runner for NSS.
-  rv = callback.GetResult(sock->Read(buf.get(), 4096, callback.callback()));
+  rv = ReadAndWaitForCompletion(sock.get(), buf.get(), 4096);
   EXPECT_THAT(rv, IsError(ERR_CONNECTION_RESET));
 }
 
 // Tests that the SSLClientSocket properly handles when the underlying transport
 // asynchronously returns an error code while writing data - such as if an
 // intermediary terminates the socket connection uncleanly.
 // This is a regression test for http://crbug.com/249848
 TEST_F(SSLClientSocketTest, Write_WithSynchronousError) {
   ASSERT_TRUE(StartTestServer(SpawnedTestServer::SSLOptions()));
 
@@ skipping 114 matching lines @@
   int old_write_count = raw_counting_socket->write_count();
   base::RunLoop loop;
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE, loop.QuitClosure(), base::TimeDelta::FromMilliseconds(100));
   loop.Run();
   EXPECT_EQ(old_write_count, raw_counting_socket->write_count());
 }
 
 // Test the full duplex mode, with Read and Write pending at the same time.
 // This test also serves as a regression test for http://crbug.com/29815.
-TEST_F(SSLClientSocketTest, Read_FullDuplex) {
+TEST_P(SSLClientSocketReadTest, Read_FullDuplex) {
   ASSERT_TRUE(StartTestServer(SpawnedTestServer::SSLOptions()));
 
   int rv;
   ASSERT_TRUE(CreateAndConnectSSLClientSocket(SSLConfig(), &rv));
   EXPECT_THAT(rv, IsOk());
 
   // Issue a "hanging" Read first.
   TestCompletionCallback callback;
   scoped_refptr<IOBuffer> buf(new IOBuffer(4096));
-  rv = sock_->Read(buf.get(), 4096, callback.callback());
+  rv = Read(sock_.get(), buf.get(), 4096, callback.callback());
   // We haven't written the request, so there should be no response yet.
   ASSERT_THAT(rv, IsError(ERR_IO_PENDING));
 
   // Write the request.
   // The request is padded with a User-Agent header to a size that causes the
   // memio circular buffer (4k bytes) in SSLClientSocketNSS to wrap around.
   // This tests the fix for http://crbug.com/29815.
   std::string request_text = "GET / HTTP/1.1\r\nUser-Agent: long browser name ";
   for (int i = 0; i < 3770; ++i)
     request_text.push_back('*');
   request_text.append("\r\n\r\n");
   scoped_refptr<IOBuffer> request_buffer(new StringIOBuffer(request_text));
 
   TestCompletionCallback callback2;  // Used for Write only.
   rv = callback2.GetResult(sock_->Write(
       request_buffer.get(), request_text.size(), callback2.callback()));
   EXPECT_EQ(static_cast<int>(request_text.size()), rv);
 
   // Now get the Read result.
-  rv = callback.WaitForResult();
+  rv = WaitForReadCompletion(sock_.get(), buf.get(), 4096, &callback, rv);
   EXPECT_GT(rv, 0);
 }
 
 // Attempts to Read() and Write() from an SSLClientSocketNSS in full duplex
 // mode when the underlying transport is blocked on sending data. When the
 // underlying transport completes due to an error, it should invoke both the
 // Read() and Write() callbacks. If the socket is deleted by the Read()
 // callback, the Write() callback should not be invoked.
 // Regression test for http://crbug.com/232633
-TEST_F(SSLClientSocketTest, Read_DeleteWhilePendingFullDuplex) {
+TEST_P(SSLClientSocketReadTest, Read_DeleteWhilePendingFullDuplex) {
   ASSERT_TRUE(StartTestServer(SpawnedTestServer::SSLOptions()));
 
   TestCompletionCallback callback;
   std::unique_ptr<StreamSocket> real_transport(
       new TCPClientSocket(addr(), NULL, NULL, NetLogSource()));
   // Note: |error_socket|'s ownership is handed to |transport|, but a pointer
   // is retained in order to configure additional errors.
   std::unique_ptr<SynchronousErrorStreamSocket> error_socket(
       new SynchronousErrorStreamSocket(std::move(real_transport)));
   SynchronousErrorStreamSocket* raw_error_socket = error_socket.get();
@@ skipping 28 matching lines @@
   // ... but have those errors returned asynchronously. Because the Write() will
   // return first, this will trigger the error.
   raw_transport->BlockReadResult();
   raw_transport->BlockWrite();
 
   // Enqueue a Read() before calling Write(), which should "hang" due to
   // the ERR_IO_PENDING caused by SetReadShouldBlock() and thus return.
   SSLClientSocket* raw_sock = sock.get();
   DeleteSocketCallback read_callback(sock.release());
   scoped_refptr<IOBuffer> read_buf(new IOBuffer(4096));
-  rv = raw_sock->Read(read_buf.get(), 4096, read_callback.callback());
+  rv = Read(raw_sock, read_buf.get(), 4096, read_callback.callback());
 
   // Ensure things didn't complete synchronously, otherwise |sock| is invalid.
   ASSERT_THAT(rv, IsError(ERR_IO_PENDING));
   ASSERT_FALSE(read_callback.have_result());
 
   // Attempt to write the remaining data. OpenSSL will return that its blocked
   // because the underlying transport is blocked.
   rv = raw_sock->Write(request_buffer.get(),
                        request_buffer->BytesRemaining(),
                        callback.callback());
   ASSERT_THAT(rv, IsError(ERR_IO_PENDING));
   ASSERT_FALSE(callback.have_result());
 
   // Now unblock Write(), which will invoke OnSendComplete and (eventually)
   // call the Read() callback, deleting the socket and thus aborting calling
   // the Write() callback.
   raw_transport->UnblockWrite();
 
+  // |read_callback| deletes |sock| so if ReadIfReady() is used, we will get OK
+  // asynchronously but can't continue reading because the socket is gone.
   rv = read_callback.WaitForResult();
-  EXPECT_THAT(rv, IsError(ERR_CONNECTION_RESET));
+  if (GetParam()) {
+    EXPECT_THAT(rv, IsOk());
+  } else {
+    EXPECT_THAT(rv, IsError(ERR_CONNECTION_RESET));
+  }
 
   // The Write callback should not have been called.
   EXPECT_FALSE(callback.have_result());
 }
 
 // Tests that the SSLClientSocket does not crash if data is received on the
 // transport socket after a failing write. This can occur if we have a Write
 // error in a SPDY socket.
 // Regression test for http://crbug.com/335557
-TEST_F(SSLClientSocketTest, Read_WithWriteError) {
+TEST_P(SSLClientSocketReadTest, Read_WithWriteError) {
   ASSERT_TRUE(StartTestServer(SpawnedTestServer::SSLOptions()));
 
   TestCompletionCallback callback;
   std::unique_ptr<StreamSocket> real_transport(
       new TCPClientSocket(addr(), NULL, NULL, NetLogSource()));
   // Note: |error_socket|'s ownership is handed to |transport|, but a pointer
   // is retained in order to configure additional errors.
   std::unique_ptr<SynchronousErrorStreamSocket> error_socket(
       new SynchronousErrorStreamSocket(std::move(real_transport)));
   SynchronousErrorStreamSocket* raw_error_socket = error_socket.get();
@@ skipping 24 matching lines @@
   memcpy(request_buffer->data(), request_text, kRequestTextSize);
 
   rv = callback.GetResult(
       sock->Write(request_buffer.get(), kRequestTextSize, callback.callback()));
   EXPECT_EQ(kRequestTextSize, rv);
 
   // Start a hanging read.
   TestCompletionCallback read_callback;
   raw_transport->BlockReadResult();
   scoped_refptr<IOBuffer> buf(new IOBuffer(4096));
-  rv = sock->Read(buf.get(), 4096, read_callback.callback());
+  rv = Read(sock.get(), buf.get(), 4096, read_callback.callback());
   EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
 
   // Perform another write, but have it fail. Write a request larger than the
   // internal socket buffers so that the request hits the underlying transport
   // socket and detects the error.
   std::string long_request_text =
       "GET / HTTP/1.1\r\nUser-Agent: long browser name ";
   long_request_text.append(20 * 1024, '*');
   long_request_text.append("\r\n\r\n");
   scoped_refptr<DrainableIOBuffer> long_request_buffer(new DrainableIOBuffer(
@@ skipping 11 matching lines @@
       // Abort if the entire input is ever consumed. The input is larger than
       // the SSLClientSocket's write buffers.
       ASSERT_LT(0, long_request_buffer->BytesRemaining());
     }
   } while (rv > 0);
 
   EXPECT_THAT(rv, IsError(ERR_CONNECTION_RESET));
 
   // At this point the Read result is available. Transport write errors are
   // surfaced through Writes. See https://crbug.com/249848.
-  rv = read_callback.WaitForResult();
+  rv = WaitForReadCompletion(sock.get(), buf.get(), 4096, &read_callback, rv);
   EXPECT_THAT(rv, IsError(ERR_CONNECTION_RESET));
 
   // Release the read. This does not cause a crash.
   raw_transport->UnblockReadResult();
   base::RunLoop().RunUntilIdle();
 }
 
 // Tests that SSLClientSocket fails the handshake if the underlying
 // transport is cleanly closed.
 TEST_F(SSLClientSocketTest, Connect_WithZeroReturn) {
@@ skipping 15 matching lines @@
   raw_transport->SetNextReadError(0);
 
   rv = callback.GetResult(sock->Connect(callback.callback()));
   EXPECT_THAT(rv, IsError(ERR_CONNECTION_CLOSED));
   EXPECT_FALSE(sock->IsConnected());
 }
 
 // Tests that SSLClientSocket returns a Read of size 0 if the underlying socket
 // is cleanly closed, but the peer does not send close_notify.
 // This is a regression test for https://crbug.com/422246
-TEST_F(SSLClientSocketTest, Read_WithZeroReturn) {
+TEST_P(SSLClientSocketReadTest, Read_WithZeroReturn) {
   ASSERT_TRUE(StartTestServer(SpawnedTestServer::SSLOptions()));
 
   TestCompletionCallback callback;
   std::unique_ptr<StreamSocket> real_transport(
       new TCPClientSocket(addr(), NULL, NULL, NetLogSource()));
   std::unique_ptr<SynchronousErrorStreamSocket> transport(
       new SynchronousErrorStreamSocket(std::move(real_transport)));
   int rv = callback.GetResult(transport->Connect(callback.callback()));
   EXPECT_THAT(rv, IsOk());
 
   // Disable TLS False Start to ensure the handshake has completed.
   SSLConfig ssl_config;
   ssl_config.false_start_enabled = false;
 
   SynchronousErrorStreamSocket* raw_transport = transport.get();
   std::unique_ptr<SSLClientSocket> sock(CreateSSLClientSocket(
       std::move(transport), spawned_test_server()->host_port_pair(),
       ssl_config));
 
   rv = callback.GetResult(sock->Connect(callback.callback()));
   EXPECT_THAT(rv, IsOk());
   EXPECT_TRUE(sock->IsConnected());
 
   raw_transport->SetNextReadError(0);
   scoped_refptr<IOBuffer> buf(new IOBuffer(4096));
-  rv = callback.GetResult(sock->Read(buf.get(), 4096, callback.callback()));
+  rv = ReadAndWaitForCompletion(sock.get(), buf.get(), 4096);
   EXPECT_EQ(0, rv);
 }
 
 // Tests that SSLClientSocket cleanly returns a Read of size 0 if the
 // underlying socket is cleanly closed asynchronously.
 // This is a regression test for https://crbug.com/422246
-TEST_F(SSLClientSocketTest, Read_WithAsyncZeroReturn) {
+TEST_P(SSLClientSocketReadTest, Read_WithAsyncZeroReturn) {
   ASSERT_TRUE(StartTestServer(SpawnedTestServer::SSLOptions()));
 
   TestCompletionCallback callback;
   std::unique_ptr<StreamSocket> real_transport(
       new TCPClientSocket(addr(), NULL, NULL, NetLogSource()));
   std::unique_ptr<SynchronousErrorStreamSocket> error_socket(
       new SynchronousErrorStreamSocket(std::move(real_transport)));
   SynchronousErrorStreamSocket* raw_error_socket = error_socket.get();
   std::unique_ptr<FakeBlockingStreamSocket> transport(
       new FakeBlockingStreamSocket(std::move(error_socket)));
   FakeBlockingStreamSocket* raw_transport = transport.get();
   int rv = callback.GetResult(transport->Connect(callback.callback()));
   EXPECT_THAT(rv, IsOk());
 
   // Disable TLS False Start to ensure the handshake has completed.
   SSLConfig ssl_config;
   ssl_config.false_start_enabled = false;
 
   std::unique_ptr<SSLClientSocket> sock(CreateSSLClientSocket(
       std::move(transport), spawned_test_server()->host_port_pair(),
       ssl_config));
 
   rv = callback.GetResult(sock->Connect(callback.callback()));
   EXPECT_THAT(rv, IsOk());
   EXPECT_TRUE(sock->IsConnected());
 
   raw_error_socket->SetNextReadError(0);
   raw_transport->BlockReadResult();
   scoped_refptr<IOBuffer> buf(new IOBuffer(4096));
-  rv = sock->Read(buf.get(), 4096, callback.callback());
+  TestCompletionCallback read_callback;
+  rv = Read(sock.get(), buf.get(), 4096, read_callback.callback());
   EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
 
   raw_transport->UnblockReadResult();
-  rv = callback.GetResult(rv);
+  rv = WaitForReadCompletion(sock.get(), buf.get(), 4096, &read_callback, rv);
   EXPECT_EQ(0, rv);
 }
 
 // Tests that fatal alerts from the peer are processed. This is a regression
 // test for https://crbug.com/466303.
-TEST_F(SSLClientSocketTest, Read_WithFatalAlert) {
+TEST_P(SSLClientSocketReadTest, Read_WithFatalAlert) {
   SpawnedTestServer::SSLOptions ssl_options;
   ssl_options.alert_after_handshake = true;
   ASSERT_TRUE(StartTestServer(ssl_options));
 
   int rv;
   ASSERT_TRUE(CreateAndConnectSSLClientSocket(SSLConfig(), &rv));
   EXPECT_THAT(rv, IsOk());
 
   // Receive the fatal alert.
   TestCompletionCallback callback;
   scoped_refptr<IOBuffer> buf(new IOBuffer(4096));
-  EXPECT_EQ(ERR_SSL_PROTOCOL_ERROR, callback.GetResult(sock_->Read(
-                                        buf.get(), 4096, callback.callback())));
+  EXPECT_EQ(ERR_SSL_PROTOCOL_ERROR,
+            ReadAndWaitForCompletion(sock_.get(), buf.get(), 4096));
 }
 
-TEST_F(SSLClientSocketTest, Read_SmallChunks) {
+TEST_P(SSLClientSocketReadTest, Read_SmallChunks) {
   ASSERT_TRUE(StartTestServer(SpawnedTestServer::SSLOptions()));
 
   int rv;
   ASSERT_TRUE(CreateAndConnectSSLClientSocket(SSLConfig(), &rv));
   EXPECT_THAT(rv, IsOk());
 
   const char request_text[] = "GET / HTTP/1.0\r\n\r\n";
   scoped_refptr<IOBuffer> request_buffer(
       new IOBuffer(arraysize(request_text) - 1));
   memcpy(request_buffer->data(), request_text, arraysize(request_text) - 1);
 
   TestCompletionCallback callback;
   rv = callback.GetResult(sock_->Write(
       request_buffer.get(), arraysize(request_text) - 1, callback.callback()));
   EXPECT_EQ(static_cast<int>(arraysize(request_text) - 1), rv);
 
   scoped_refptr<IOBuffer> buf(new IOBuffer(1));
   do {
-    rv = callback.GetResult(sock_->Read(buf.get(), 1, callback.callback()));
+    rv = ReadAndWaitForCompletion(sock_.get(), buf.get(), 1);
     EXPECT_GE(rv, 0);
   } while (rv > 0);
 }
 
-TEST_F(SSLClientSocketTest, Read_ManySmallRecords) {
+TEST_P(SSLClientSocketReadTest, Read_ManySmallRecords) {
   ASSERT_TRUE(StartTestServer(SpawnedTestServer::SSLOptions()));
 
   TestCompletionCallback callback;
 
   std::unique_ptr<StreamSocket> real_transport(
       new TCPClientSocket(addr(), NULL, NULL, NetLogSource()));
   std::unique_ptr<ReadBufferingStreamSocket> transport(
       new ReadBufferingStreamSocket(std::move(real_transport)));
   ReadBufferingStreamSocket* raw_transport = transport.get();
   int rv = callback.GetResult(transport->Connect(callback.callback()));
@@ skipping 22 matching lines @@
   // of SSL data is buffered first. The 15K of buffered data is made up of
   // many smaller SSL records (the TestServer writes along 1350 byte
   // plaintext boundaries), although there may also be a few records that are
   // smaller or larger, due to timing and SSL False Start.
   // 15K was chosen because 15K is smaller than the 17K (max) read issued by
   // the SSLClientSocket implementation, and larger than the minimum amount
   // of ciphertext necessary to contain the 8K of plaintext requested below.
   raw_transport->SetBufferSize(15000);
 
   scoped_refptr<IOBuffer> buffer(new IOBuffer(8192));
-  rv = callback.GetResult(sock->Read(buffer.get(), 8192, callback.callback()));
+  rv = ReadAndWaitForCompletion(sock.get(), buffer.get(), 8192);
   ASSERT_EQ(rv, 8192);
 }
 
-TEST_F(SSLClientSocketTest, Read_Interrupted) {
+TEST_P(SSLClientSocketReadTest, Read_Interrupted) {
   ASSERT_TRUE(StartTestServer(SpawnedTestServer::SSLOptions()));
 
   int rv;
   ASSERT_TRUE(CreateAndConnectSSLClientSocket(SSLConfig(), &rv));
   EXPECT_THAT(rv, IsOk());
 
   const char request_text[] = "GET / HTTP/1.0\r\n\r\n";
   scoped_refptr<IOBuffer> request_buffer(
       new IOBuffer(arraysize(request_text) - 1));
   memcpy(request_buffer->data(), request_text, arraysize(request_text) - 1);
 
   TestCompletionCallback callback;
   rv = callback.GetResult(sock_->Write(
       request_buffer.get(), arraysize(request_text) - 1, callback.callback()));
   EXPECT_EQ(static_cast<int>(arraysize(request_text) - 1), rv);
 
   // Do a partial read and then exit.  This test should not crash!
   scoped_refptr<IOBuffer> buf(new IOBuffer(512));
-  rv = callback.GetResult(sock_->Read(buf.get(), 512, callback.callback()));
+  rv = ReadAndWaitForCompletion(sock_.get(), buf.get(), 512);
   EXPECT_GT(rv, 0);
 }
 
-TEST_F(SSLClientSocketTest, Read_FullLogging) {
+TEST_P(SSLClientSocketReadTest, Read_FullLogging) {
   ASSERT_TRUE(StartTestServer(SpawnedTestServer::SSLOptions()));
 
   TestCompletionCallback callback;
   TestNetLog log;
   log.SetCaptureMode(NetLogCaptureMode::IncludeSocketBytes());
   std::unique_ptr<StreamSocket> transport(
       new TCPClientSocket(addr(), NULL, &log, NetLogSource()));
   int rv = callback.GetResult(transport->Connect(callback.callback()));
   EXPECT_THAT(rv, IsOk());
 
@@ skipping 15 matching lines @@
   EXPECT_EQ(static_cast<int>(arraysize(request_text) - 1), rv);
 
   TestNetLogEntry::List entries;
   log.GetEntries(&entries);
   size_t last_index = ExpectLogContainsSomewhereAfter(
       entries, 5, NetLogEventType::SSL_SOCKET_BYTES_SENT,
       NetLogEventPhase::NONE);
 
   scoped_refptr<IOBuffer> buf(new IOBuffer(4096));
   for (;;) {
-    rv = callback.GetResult(sock->Read(buf.get(), 4096, callback.callback()));
+    rv = ReadAndWaitForCompletion(sock.get(), buf.get(), 4096);
     EXPECT_GE(rv, 0);
     if (rv <= 0)
       break;
 
     log.GetEntries(&entries);
     last_index = ExpectLogContainsSomewhereAfter(
         entries, last_index + 1, NetLogEventType::SSL_SOCKET_BYTES_RECEIVED,
         NetLogEventPhase::NONE);
   }
 }
@@ skipping 1651 matching lines @@
   // Replace it with an alert.
   raw_transport->ReplaceReadResult(
       FormatTLS12Alert(49 /* AlertDescription.access_denied */));
   raw_transport->UnblockReadResult();
 
   rv = callback.GetResult(rv);
   EXPECT_THAT(rv, IsError(ERR_BAD_SSL_CLIENT_AUTH_CERT));
 }
 
 // Basic test for dumping memory stats.
-TEST_F(SSLClientSocketTest, DumpMemoryStats) {
+TEST_P(SSLClientSocketReadTest, DumpMemoryStats) {
   ASSERT_TRUE(StartTestServer(SpawnedTestServer::SSLOptions()));
 
   int rv;
   ASSERT_TRUE(CreateAndConnectSSLClientSocket(SSLConfig(), &rv));
   EXPECT_THAT(rv, IsOk());
   StreamSocket::SocketMemoryStats stats;
   sock_->DumpMemoryStats(&stats);
   EXPECT_EQ(0u, stats.buffer_size);
   EXPECT_EQ(1u, stats.cert_count);
   EXPECT_LT(0u, stats.cert_size);
   EXPECT_EQ(stats.cert_size, stats.total_size);
 
   // Read the response without writing a request, so the read will be pending.
   TestCompletionCallback read_callback;
   scoped_refptr<IOBuffer> buf(new IOBuffer(4096));
-  rv = sock_->Read(buf.get(), 4096, read_callback.callback());
+  rv = Read(sock_.get(), buf.get(), 4096, read_callback.callback());
   EXPECT_EQ(ERR_IO_PENDING, rv);
 
   // Dump memory again and check that |buffer_size| contain the read buffer.
   StreamSocket::SocketMemoryStats stats2;
   sock_->DumpMemoryStats(&stats2);
-  EXPECT_EQ(17 * 1024u, stats2.buffer_size);
+
+  bool buffer_released = false;
+  if (GetParam()) {
+// TODO(xunjieli): crbug.com/690915. Implement ReadIfReady() for windows.

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

Optional: use https:// prefix for crbug, because some viewers/editors need that
to make this a clickable link (for example, gnome-terminal, when text is output
using cat, less, diff, grep etc.).

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

Done.

+#if defined(OS_POSIX)
+    buffer_released = true;
+#endif
+  }
+  if (buffer_released) {
+    EXPECT_EQ(0u, stats2.buffer_size);
+    EXPECT_EQ(stats.cert_size, stats2.total_size);
+  } else {
+    EXPECT_EQ(17 * 1024u, stats2.buffer_size);
+    EXPECT_LT(17 * 1024u, stats2.total_size);
+  }
   EXPECT_EQ(1u, stats2.cert_count);
   EXPECT_LT(0u, stats2.cert_size);
-  EXPECT_LT(17 * 1024u, stats2.total_size);
 }
 
 }  // namespace net