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>

[davidben, 2017/02/10 23:33:48]

Nit: Swap this and the blank line below.

[xunjieli, 2017/02/13 20:28:18]

Done.

 
 #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/stl_util.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;
-
   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(
+      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.
+  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;
   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 23 matching lines @@
  private:
   // 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;
 
+  // Used to buffer result returned by a synchronously completed ReadIfReady()
+  // which needs to be blocked.
+  std::string blocked_sync_read_if_ready_data_;

[davidben, 2017/02/10 23:33:48]

Hah! Writing into a buffer like that hadn't occured to me. Good idea.

[xunjieli, 2017/02/13 20:28:18]

Acknowledged.

+  // True if a synchronous ReadIfReady() has been blocked. Reset to false when
+  // the data from the blocked sync ReadIfReady() is consumed.
+  bool has_blocked_sync_read_if_ready_ = false;
+
   // 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 (has_blocked_sync_read_if_ready_) {

[davidben, 2017/02/10 23:33:48]

I think this doesn't quite work if should_block_read_ is true. Consider this
scenario:

  BlockReadResult()
  ReadIfReady(10) => get 10 bytes, but should_block_read_ so it goes to a
buffer.
  UnblockReadResult() => ReadIfReady completes with OK.
  ReadIfReady(5) => okay, we consume 5 bytes from the stashed buffer.
  BlockReadResult()
  ReadIfReady(5) => we have bytes in the stashed buffer, but need to resolve it
later.

But this is also a test socket and I think we'd never do that? We can probably
just stick a bunch of CHECKs and simplify this. See comment below.

[xunjieli, 2017/02/13 20:28:18]

Acknowledged.

+    DCHECK(!pending_read_callback_);
+    DCHECK(pending_read_buf_);
+    DCHECK_NE(ERR_IO_PENDING, pending_read_result_);
+    if (pending_read_result_ > 0) {
+      DCHECK_LE(0, pending_read_buf_len_);
+      int bytes_read = std::min(len, pending_read_buf_len_);
+      memcpy(buf->data(), pending_read_buf_->data(), bytes_read);
+      blocked_sync_read_if_ready_data_.erase(0, bytes_read);
+      if (!blocked_sync_read_if_ready_data_.empty()) {
+        pending_read_buf_ =
+            new StringIOBuffer(blocked_sync_read_if_ready_data_);
+        pending_read_buf_len_ = blocked_sync_read_if_ready_data_.length();
+        pending_read_result_ = pending_read_buf_len_;
+        return bytes_read;
+      }
+    }
+    int rv = pending_read_result_;
+    has_blocked_sync_read_if_ready_ = false;
+    pending_read_buf_ = nullptr;
+    pending_read_buf_len_ = -1;
+    pending_read_result_ = ERR_IO_PENDING;
+    return rv;
+  }
+  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_->ReadIfReady(
+      buf, len, base::Bind(&FakeBlockingStreamSocket::OnReadCompleted,
+                           base::Unretained(this)));
+  if (rv == ERR_IO_PENDING) {
+    pending_read_callback_ = callback;
+    return rv;
+  }
+  if (should_block_read_) {
+    // Save the data to be called later.
+    if (rv >= 0)
+      blocked_sync_read_if_ready_data_.append(buf->data(), rv);
+    has_blocked_sync_read_if_ready_ = true;
+    // Save the callback to be called later.
+    pending_read_buf_ = new StringIOBuffer(blocked_sync_read_if_ready_data_);

[davidben, 2017/02/10 23:33:48]

Do you need both the std::string and the StringIOBuffer? This looks a little
funny. StringIOBuffer makes a copy of its input, so if you modify things later
with ReplaceReadResult, it doesn't actually notice.

I like the idea to redirect it to another buffer though. What if we implemented
ReadIfReady using Read?

  CompletionCallback read_if_ready_callback_;
  string read_if_ready_buf_;

int FBSS::ReadIfReady(buf, len, callback) {
  if (!read_if_ready_buf_.empty()) {
    // This is possible if the caller calls ReadIfReady with a smaller
    // buffer size than previously or if BlockReadResult is called
    // after ReadIfReady reports there is data available but before the
    // data is drained.
    CHECK(!should_block_read_);
    CHECK(len >= read_if_ready_buf_.size());

    int rv = read_if_read_buf_.size();
    memcpy(buf->data(), read_if_read_buf_.data(), rv);
    read_if_ready_buf_.clear();
    return rv;
  }

  buf_copy = new IOBuffer(len);
  int rv = Read(read_if_ready_buf_, len, Bind(CompleteReadIfReady, buf_copy));
  if (rv > 0)
    memcpy(buf->data(), buf_copy->data(), rv);
  if (rv == ERR_IO_PENDING)
    read_if_ready_callback_ = callback;
  return rv;
}

void FBSS::CompleteReadIfReady(IOBuffer* buf, int rv) {
  DCHECK(read_if_ready_buf_.empty());
  DCHECK(!should_block_read_);

  if (rv > 0) {
    read_if_ready_buf_ = string(buf->data(), buf->data() + rv);
    rv = OK;
  }

  ResetAndReturn(read_if_ready_callback_, rv);
}

(If one of those CHECKs crashes, you might need to make this a little more
complicated. Notably the read_if_ready_buf_ + should_block_read_ case will
probably require UnblockReadResult to be aware of read_if_ready_callback_
whereas right now should_block_read_ is almost entirely hidden from
ReadIfReady.)

[xunjieli, 2017/02/13 20:28:19]

Done. Neat! I somehow though StringIOBuffer doesn't own the underlying buffer or
do a string copy

+    pending_read_buf_len_ = len;
+    pending_read_callback_ = callback;
+    OnReadCompleted(rv);
+    rv = ERR_IO_PENDING;
+  }
+  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 20 matching lines @@
   DCHECK(should_block_read_);
   should_block_read_ = false;
 
   // If the operation has since completed, return the result to the caller.
   if (pending_read_result_ != ERR_IO_PENDING)
     ReturnReadResult();
 }
 
 bool FakeBlockingStreamSocket::ReplaceReadResult(const std::string& data) {
   DCHECK(should_block_read_);
-  DCHECK_NE(ERR_IO_PENDING, pending_read_result_);
   DCHECK(pending_read_buf_);
   DCHECK_NE(-1, pending_read_buf_len_);
 
   if (static_cast<size_t>(pending_read_buf_len_) < data.size())
     return false;
 
+  if (has_blocked_sync_read_if_ready_)
+    blocked_sync_read_if_ready_data_ = data;
+
+  DCHECK_NE(ERR_IO_PENDING, pending_read_result_);
+  DCHECK(pending_read_buf_);
+  DCHECK_NE(-1, pending_read_buf_len_);
   memcpy(pending_read_buf_->data(), data.data(), data.size());
   pending_read_result_ = data.size();
   return true;
 }
 
 void FakeBlockingStreamSocket::WaitForReadResult() {
   DCHECK(should_block_read_);
   DCHECK(!read_loop_);
 
   if (pending_read_result_ != ERR_IO_PENDING)
@@ skipping 52 matching lines @@
     if (read_loop_)
       read_loop_->Quit();
     return;
   }
 
   ReturnReadResult();
 }
 
 void FakeBlockingStreamSocket::ReturnReadResult() {
   int result = pending_read_result_;
+  if (has_blocked_sync_read_if_ready_) {
+    base::ResetAndReturn(&pending_read_callback_).Run(result > 0 ? OK : result);
+    return;
+  }
   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.
 class CountingStreamSocket : public WrappedStreamSocket {
  public:
@@ skipping 252 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())
+      scoped_feature_list_.InitAndEnableFeature(Socket::kReadIfReadyExperiment);
+  }
+
+  // Convienient wrapper to call Read() or 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);
+  }
+
+  // Calls Read()/ReadIfReady() and waits for it to return data.
+  int ReadAndWaitForCompletion(StreamSocket* socket,
+                               IOBuffer* buf,
+                               int buf_len) {
+    TestCompletionCallback callback;
+    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);
+        }
+      }
+      return rv;

[davidben, 2017/02/10 23:33:48]

It's legal for ReadIfReady to signal OK before it's actually ready to complete.
(SSLClientSocketImpl will do this since we don't keep perfect track of what is
blocking on what.) I suspect this needs to be a loop to avoid flakiness.
Something like:

  int rv;
  do {
    int rv = socket->ReadIfReady(...);
    if (rv != ERR_IO_PENDING)
      break;
    rv = callback.GetResult(rv);
  } while (rv == OK);

-----

Edit: Actually, I see you sometimes split things up above. You might need
something like:

int Read(StreamSocket* socket, IOBuffer* buf, int buf_len,
         TestCompletionCallback* callback) {
  if (GetParam())
    return socket->ReadIfReady(buf, buf_len, callback->callback());
  return socket->Read(buf, buf_len, callback->callback());
}

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;
}

int ReadAndWaitForCompletion(StreamSocket* socket,
                             IOBuffer* buf,
                             int buf_len) {
  TestCompletionCallback callback;
  int rv = Read(socket, buf, buf_len, &callback);
  return WaitForReadCompletion(socket, buf, buf_len, &callback, rv);
}

[xunjieli, 2017/02/13 20:28:18]

Done.

+    }
+    return callback.GetResult(socket->Read(buf, buf_len, callback.callback()));
+  }
+
+ 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 = 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());

[davidben, 2017/02/10 23:33:48]

Given the issue with ReadIfReady always needing to be driven to completion in a
loop, perhaps we need a helper function for completing a Read|ReadIfReady
result.

(See amended comment above.)

[xunjieli, 2017/02/13 20:28:18]

Done.

+  }
   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));

[davidben, 2017/02/10 23:33:48]

You'll probably need WaitForReadResult here, once you change SSLClientSocketImpl
to require an extra loop to surface errors. Probably in a bunch of other places
too.

[xunjieli, 2017/02/13 20:28:18]

Actually this particularly one deletes the socket during OnComplete(), so we
can't continue with ReadIfReady(). I update the test case accordingly. PTAL.

 
   // 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);
   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));
 }
 
-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.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));
-  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()) {
+#if defined(OS_POSIX)
+    buffer_released = true;

[davidben, 2017/02/10 23:33:48]

Probably wants a comment for what's happening here. (That you only implemented
it for POSIX and not Windows I assume.)

[xunjieli, 2017/02/13 20:28:18]

Done.

+#endif
+  }
+  if (buffer_released) {
+    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);
+  }
   EXPECT_EQ(1u, stats2.cert_count);
   EXPECT_LT(0u, stats2.serialized_cert_size);
-  EXPECT_LT(17 * 1024u, stats2.total_size);
 }
 
 }  // namespace net