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 <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/metrics/field_trial.h"
 #include "base/run_loop.h"
 #include "base/single_thread_task_runner.h"
+#include "base/test/mock_entropy_provider.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"
 #include "net/cert/ct_policy_status.h"
@@ skipping 104 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:
   enum State {
     STATE_NONE,
     STATE_READ,
     STATE_READ_COMPLETE,
   };
 
-  int DoLoop(int result);
-  int DoRead();
-  int DoReadComplete(int result);
-  void OnReadCompleted(int result);
+  // Routine shared between Read() and ReadIfReady().
+  int ReadCommon(IOBuffer* buf,
+                 int buf_len,
+                 const CompletionCallback& callback,
+                 bool use_read_if_ready);
+  int DoLoop(int result, bool use_read_if_ready);
+  int DoRead(bool use_read_if_ready);
+  int DoReadComplete(int result, bool use_read_if_ready);
+  void OnReadCompleted(bool use_read_if_ready, int result);
 
   State state_;
   scoped_refptr<GrowableIOBuffer> read_buffer_;
   int buffer_size_;
 
   scoped_refptr<IOBuffer> user_read_buf_;
   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());
   buffer_size_ = size;
   read_buffer_->SetCapacity(size);
 }
 
 int ReadBufferingStreamSocket::Read(IOBuffer* buf,
                                     int buf_len,
                                     const CompletionCallback& callback) {
   if (buffer_size_ == 0)
     return transport_->Read(buf, buf_len, callback);
+  return ReadCommon(buf, buf_len, callback, false);
+}
+
+int ReadBufferingStreamSocket::ReadIfReady(IOBuffer* buf,
+                                           int buf_len,
+                                           const CompletionCallback& callback) {

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

Similar comment. I think you can structure this like:

- Read calls ReadIfReady and, if ERR_IO_PENDING, stashes user_read_buf_.
- ReadCommon inlines into ReadIfReady and doesn't set user_read_buf_.
- No need for boolean.
- DoRead can just call transport_->Read; the tests only care about the external
behavior of ReadBufferingStreamSocket and leaving it alone is simpler.
- DoReadComplete checks user_read_buf_ and does the ReadIfReady (just return OK)
or Read (memcpy and return capacity) thing accordingly.

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

Done.

+  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();
+  }
+  int rv = ReadCommon(buf, buf_len, callback, true);
+  user_read_buf_ = nullptr;
+  return rv;
+}
+
+int ReadBufferingStreamSocket::ReadCommon(IOBuffer* buf,
+                                          int buf_len,
+                                          const CompletionCallback& callback,
+                                          bool use_read_if_ready) {
+  DCHECK(!user_read_buf_);
 
   if (buf_len < buffer_size_)
     return ERR_UNEXPECTED;
 
   state_ = STATE_READ;
   user_read_buf_ = buf;
-  int result = DoLoop(OK);
+  int result = DoLoop(OK, use_read_if_ready);
   if (result == ERR_IO_PENDING)
     user_read_callback_ = callback;
   else
-    user_read_buf_ = NULL;
+    user_read_buf_ = nullptr;
   return result;
 }
 
-int ReadBufferingStreamSocket::DoLoop(int result) {
+int ReadBufferingStreamSocket::DoLoop(int result, bool use_read_if_ready) {
   int rv = result;
   do {
     State current_state = state_;
     state_ = STATE_NONE;
     switch (current_state) {
       case STATE_READ:
-        rv = DoRead();
+        rv = DoRead(use_read_if_ready);
         break;
       case STATE_READ_COMPLETE:
-        rv = DoReadComplete(rv);
+        rv = DoReadComplete(rv, use_read_if_ready);
         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() {
+int ReadBufferingStreamSocket::DoRead(bool use_read_if_ready) {
   state_ = STATE_READ_COMPLETE;
-  int rv =
-      transport_->Read(read_buffer_.get(),
-                       read_buffer_->RemainingCapacity(),
-                       base::Bind(&ReadBufferingStreamSocket::OnReadCompleted,
-                                  base::Unretained(this)));
-  return rv;
+  if (use_read_if_ready) {
+    return transport_->ReadIfReady(
+        read_buffer_.get(), read_buffer_->RemainingCapacity(),
+        base::Bind(&ReadBufferingStreamSocket::OnReadCompleted,
+                   base::Unretained(this), true));
+  } else {
+    return transport_->Read(
+        read_buffer_.get(), read_buffer_->RemainingCapacity(),
+        base::Bind(&ReadBufferingStreamSocket::OnReadCompleted,
+                   base::Unretained(this), false));
+  }
 }
 
-int ReadBufferingStreamSocket::DoReadComplete(int result) {
+int ReadBufferingStreamSocket::DoReadComplete(int result,
+                                              bool use_read_if_ready) {
   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 (use_read_if_ready)
+    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) {
-  result = DoLoop(result);
+void ReadBufferingStreamSocket::OnReadCompleted(bool use_read_if_ready,
+                                                int result) {
+  DCHECK_NE(ERR_IO_PENDING, result);
+
+  if (use_read_if_ready && result == 0)
+    state_ = STATE_READ;
+
+  result = DoLoop(result, use_read_if_ready);
   if (result == ERR_IO_PENDING)
     return;
-
   user_read_buf_ = NULL;
   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 14 matching lines @@
   // is a pending asynchronous write, it is NOT blocked. For purposes of
   // blocking writes, data is considered to have reached the underlying
   // transport as soon as Write() is called.
   void BlockWrite();
   void UnblockWrite();
 
   // Waits for the blocked Write() call to be scheduled.
   void WaitForWrite();
 
  private:
+  // Shared between Read() and ReadIfReady(). If |use_read_if_ready| is true,
+  // ReadIfReady() will be used.
+  int ReadCommon(IOBuffer* buf,
+                 int buf_len,
+                 const CompletionCallback& callback,
+                 bool use_read_if_ready);
+
   // Handles completion from the underlying transport read.
   void OnReadCompleted(int result);
 
   // Finishes the current read.
   void ReturnReadResult();
 
   // True if read callbacks are blocked.
   bool should_block_read_ = false;
 
   // The buffer for the pending read, or NULL if not consumed.
@@ skipping 24 matching lines @@
   // The length for the pending write, or -1 if not scheduled.
   int pending_write_len_ = -1;
 
   // WaitForWrite() wait loop.
   std::unique_ptr<base::RunLoop> write_loop_;
 };
 
 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());
+  return ReadCommon(buf, len, callback, false);
+}
 
-  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;
-    }
-  }
+int FakeBlockingStreamSocket::ReadIfReady(IOBuffer* buf,
+                                          int len,
+                                          const CompletionCallback& callback) {
+  int rv = ReadCommon(buf, len, callback, true);
+  pending_read_buf_ = nullptr;
+  pending_read_buf_len_ = 0;
   return rv;
 }
 
 int FakeBlockingStreamSocket::Write(IOBuffer* buf,
                                     int len,
                                     const CompletionCallback& callback) {
   DCHECK(buf);
   DCHECK_LE(0, len);
 
   if (!should_block_write_)
@@ skipping 84 matching lines @@
   DCHECK(!write_loop_);
 
   if (pending_write_buf_.get())
     return;
   write_loop_.reset(new base::RunLoop);
   write_loop_->Run();
   write_loop_.reset();
   DCHECK(pending_write_buf_.get());
 }
 
+int FakeBlockingStreamSocket::ReadCommon(IOBuffer* buf,
+                                         int len,
+                                         const CompletionCallback& callback,
+                                         bool use_read_if_ready) {
+  DCHECK(!pending_read_buf_);
+  DCHECK(pending_read_callback_.is_null());
+  DCHECK_EQ(ERR_IO_PENDING, pending_read_result_);
+  DCHECK(!callback.is_null());
+
+  int rv;
+  if (!use_read_if_ready) {
+    rv = transport_->Read(buf, len,
+                          base::Bind(&FakeBlockingStreamSocket::OnReadCompleted,
+                                     base::Unretained(this)));
+  } else {
+    rv = transport_->ReadIfReady(
+        buf, len, base::Bind(&FakeBlockingStreamSocket::OnReadCompleted,
+                             base::Unretained(this)));

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

Hrm. This one's a little interesting. If ReadIfReady returns synchronously while
should_block_read_ is true, I think things will get confused here. Because
you've actually written to the buffer and taken data out of the socket, but
you're not returning it yet which means the caller thinks it can throw that data
away. (This socket blocks read *results* rather than read *calls*. I think there
was some awkward False Start test synchronizing reason why we needed that.
Though arguably we care less about those tests now that the code is actually in
BoringSSL.)

I'm actually not sure how FakeBlockingStreamSocket could implement ReadIfReady +
BlockReadResult + WaitForReadResult. Also ReplaceReadResult. Without those two
functions, I think BlockRead and BlockReadResult are the same though.

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

Ah, sorry I missed that. Good catch! How about we create a temp buffer to hold
the result? That way, if caller throws away |buf|, we can still unblock/replace
correctly? I uploaded a new patch.

+  }
+  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;
+}
+
 void FakeBlockingStreamSocket::OnReadCompleted(int result) {
   DCHECK_EQ(ERR_IO_PENDING, pending_read_result_);
   DCHECK(!pending_read_callback_.is_null());
 
   pending_read_result_ = result;
 
   if (should_block_read_) {
     // Defer the result until UnblockReadResult is called.
     if (read_loop_)
       read_loop_->Quit();
@@ skipping 270 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.
+class SSLClientSocketReadTest : public SSLClientSocketTest,
+                                public ::testing::WithParamInterface<bool> {
+ protected:
+  void SetUp() override {
+    if (GetParam()) {
+      field_trial_list_ = base::MakeUnique<base::FieldTrialList>(
+          base::MakeUnique<base::MockEntropyProvider>());
+      base::FieldTrialList::CreateFieldTrial(Socket::kReadIfReadyTrialName,
+                                             "enable");
+    }
+  }
+
+  // Convienient wrapper to call Read() or ReadIfReady() depending on
+  // GetParam().
+  int Read(StreamSocket* socket,
+           IOBuffer* buf,
+           int buf_len,
+           const CompletionCallback& callback) {
+    if (GetParam()) {
+      int rv = socket->ReadIfReady(buf, buf_len, callback);
+      if (rv != ERR_READ_IF_READY_NOT_IMPLEMENTED)
+        return rv;

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

Since this is test code, do we want to have this check? Otherwise if
SSLClientSocket just didn't bother implementing ReadIfReady, I think it'd pass
all the tests.

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

Done.

+    }
+    return socket->Read(buf, buf_len, callback);
+  }
+
+  // Calls Read()/ReadIfReady() and waits for it to return data.
+  int ReadAndWaitForCompletion(StreamSocket* socket,
+                               IOBuffer* buf,
+                               int buf_len,
+                               TestCompletionCallback* callback) {

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

Since you pass in and wait on callback in the same function, I think you don't
need to pass it in and can instead just create your own one in this function.

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

Done.

+    if (GetParam()) {
+      int rv = socket->ReadIfReady(buf, buf_len, callback->callback());
+      if (rv == ERR_IO_PENDING) {
+        rv = callback->GetResult(rv);
+        if (rv == OK) {
+          rv = socket->ReadIfReady(buf, buf_len, callback->callback());
+          DCHECK_NE(ERR_IO_PENDING, rv);
+        }
+      }
+      if (rv != ERR_READ_IF_READY_NOT_IMPLEMENTED)

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

Ditto re ERR_READ_IF_READY_NOT_IMPLEMENTED.

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

Done.

+        return rv;
+    }
+    return callback->GetResult(
+        socket->Read(buf, buf_len, callback->callback()));
+  }
+
+ private:
+  std::unique_ptr<base::FieldTrialList> field_trial_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 324 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()));
+    TestCompletionCallback read_callback;
+    rv = ReadAndWaitForCompletion(sock.get(), buf.get(), 4096, &read_callback);
     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 = Read(sock.get(), buf.get(), 4096, callback.callback());
   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();
+  if (GetParam()) {
+    EXPECT_THAT(rv, IsOk());
+    rv = Read(sock_.get(), buf.get(), 4096, callback.callback());
+  }
   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();
 
   rv = read_callback.WaitForResult();
   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 47 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, &callback);
   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 = read_callback.GetResult(rv);
+  if (GetParam()) {
+    EXPECT_EQ(OK, rv);
+    rv = Read(sock.get(), buf.get(), 4096, read_callback.callback());
+  }
   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, &callback));
 }
 
-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, &callback);
     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, &callback);
   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, &callback);
   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, &callback);
     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 1652 matching lines @@
   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) {
+  std::unique_ptr<base::FieldTrialList> field_trial_list =
+      base::MakeUnique<base::FieldTrialList>(
+          base::MakeUnique<base::MockEntropyProvider>());
+
+  base::FieldTrialList::CreateFieldTrial(Socket::kReadIfReadyTrialName,
+                                         "enable");
   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.serialized_cert_size);
   EXPECT_EQ(stats.serialized_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));
+#if defined(OS_POSIX)

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

Not a TEST_P?

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

Done.

+  rv = sock_->ReadIfReady(buf.get(), 4096, read_callback.callback());
+#else
   rv = sock_->Read(buf.get(), 4096, read_callback.callback());
+#endif
   EXPECT_EQ(ERR_IO_PENDING, rv);
 
   // Dump memory again and check that |buffer_size| contain the read buffer.
   StreamSocket::SocketMemoryStats stats2;
   sock_->DumpMemoryStats(&stats2);
+
+#if defined(OS_POSIX)
+  EXPECT_EQ(0u, stats2.buffer_size);
+  EXPECT_EQ(stats.serialized_cert_size, stats2.total_size);
+#else
   EXPECT_EQ(17 * 1024u, stats2.buffer_size);
+  EXPECT_LT(17 * 1024u, stats2.total_size);
+#endif
   EXPECT_EQ(1u, stats2.cert_count);
   EXPECT_LT(0u, stats2.serialized_cert_size);
-  EXPECT_LT(17 * 1024u, stats2.total_size);
 }
 
 }  // namespace net