Add Socket::ReadIfReady()

net/socket/socket_bio_adapter_unittest.cc

 // Copyright 2016 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/socket_bio_adapter.h"
 
 #include <string.h>
 
 #include <memory>
 
 #include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/memory/ptr_util.h"
+#include "base/metrics/field_trial.h"
 #include "base/run_loop.h"
+#include "base/test/mock_entropy_provider.h"
 #include "crypto/openssl_util.h"
 #include "net/base/address_list.h"
 #include "net/base/net_errors.h"
 #include "net/log/net_log_source.h"
 #include "net/socket/socket_test_util.h"
 #include "net/socket/stream_socket.h"
 #include "net/ssl/openssl_ssl_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "third_party/boringssl/src/include/openssl/bio.h"
 #include "third_party/boringssl/src/include/openssl/err.h"
 #include "third_party/boringssl/src/include/openssl/ssl.h"
 
 namespace net {
 
-class SocketBIOAdapterTest : public testing::Test,
-                             public SocketBIOAdapter::Delegate {
+enum ReadIfReadySupport {

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

Thanks for being thorough with testing this!

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

:)

+  // ReadIfReady() field trial is enabled, but ReadyIfReady() is unimplemented.
+  READ_IF_READY_ENABLED_SUPPORTED,
+  // ReadIfReady() field trial is enabled, and ReadyIfReady() is implemented.
+  READ_IF_READY_ENABLED_NOT_SUPPORTED,
+  // ReadIfReady() field trial is disabled.
+  READ_IF_READY_DISABLED,
+};
+
+class SocketBIOAdapterTest
+    : public testing::Test,
+      public testing::WithParamInterface<ReadIfReadySupport>,

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

I think you can replace this and the line above with
testing::TestWithParam<ReadIfReadySupport>. (Or do we prefer this setup in
Chromium? I could be out-of-date on style.)

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

Done. Thanks! I thought there was one class for this purpose but couldn't find
it. 

+      public SocketBIOAdapter::Delegate {
  protected:
+  void SetUp() override {
+    if (GetParam() != READ_IF_READY_DISABLED) {
+      field_trial_list_.reset(new base::FieldTrialList(
+          base::MakeUnique<base::MockEntropyProvider>()));
+      base::FieldTrialList::CreateFieldTrial(Socket::kReadIfReadyTrialName,
+                                             "enable");
+    }
+    if (GetParam() == READ_IF_READY_ENABLED_SUPPORTED)
+      factory_.set_enable_read_if_ready(true);
+  }
+
   std::unique_ptr<StreamSocket> MakeTestSocket(SocketDataProvider* data) {
     data->set_connect_data(MockConnect(SYNCHRONOUS, OK));
     factory_.AddSocketDataProvider(data);
     std::unique_ptr<StreamSocket> socket = factory_.CreateTransportClientSocket(
         AddressList(), nullptr, nullptr, NetLogSource());
     CHECK_EQ(OK, socket->Connect(net::CompletionCallback()));
     return socket;
   }
 
   void set_reset_on_write_ready(
@@ skipping 72 matching lines @@
 
   void WaitForBothReady() {
     expect_read_ready_ = true;
     expect_write_ready_ = true;
     base::RunLoop().RunUntilIdle();
     EXPECT_FALSE(expect_read_ready_);
     EXPECT_FALSE(expect_write_ready_);
   }
 
   // SocketBIOAdapter::Delegate implementation:
-  void OnReadReady() override {
+  void OnReadReady(int rv) override {
+    DCHECK_GE(OK, rv);

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

Nit: EXPECT. Ditto below.

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

Done.

     EXPECT_TRUE(expect_read_ready_);
     expect_read_ready_ = false;
   }
 
-  void OnWriteReady() override {
+  void OnWriteReady(int rv) override {
+    DCHECK_GE(OK, rv);
     EXPECT_TRUE(expect_write_ready_);
     expect_write_ready_ = false;
     if (reset_on_write_ready_)
       reset_on_write_ready_->reset();
   }
 
  private:
   bool expect_read_ready_ = false;
   bool expect_write_ready_ = false;
   MockClientSocketFactory factory_;
   std::unique_ptr<SocketBIOAdapter>* reset_on_write_ready_ = nullptr;
+  std::unique_ptr<base::FieldTrialList> field_trial_list_;
 };
 
+INSTANTIATE_TEST_CASE_P(/* no prefix */,
+                        SocketBIOAdapterTest,
+                        testing::Values(READ_IF_READY_ENABLED_SUPPORTED,
+                                        READ_IF_READY_ENABLED_NOT_SUPPORTED,
+                                        READ_IF_READY_DISABLED));
+
 // Test that data can be read synchronously.
-TEST_F(SocketBIOAdapterTest, ReadSync) {
+TEST_P(SocketBIOAdapterTest, ReadSync) {
   crypto::OpenSSLErrStackTracer tracer(FROM_HERE);
 
   MockRead reads[] = {
       MockRead(SYNCHRONOUS, 0, "hello"), MockRead(SYNCHRONOUS, 1, "world"),
       MockRead(SYNCHRONOUS, ERR_CONNECTION_RESET, 2),
   };
 
   SequencedSocketData data(reads, arraysize(reads), nullptr, 0);
   std::unique_ptr<StreamSocket> socket = MakeTestSocket(&data);
   std::unique_ptr<SocketBIOAdapter> adapter =
@@ skipping 20 matching lines @@
   // The remainder may be consumed in a single BIO_read.
   EXPECT_EQ(3, BIO_read(bio, buf, sizeof(buf)));
   EXPECT_EQ(0, memcmp("rld", buf, 3));
   EXPECT_FALSE(adapter->HasPendingReadData());
 
   // The error is available synchoronously.
   ExpectReadError(bio, ERR_CONNECTION_RESET, tracer);
 }
 
 // Test that data can be read asynchronously.
-TEST_F(SocketBIOAdapterTest, ReadAsync) {
+TEST_P(SocketBIOAdapterTest, ReadAsync) {
   crypto::OpenSSLErrStackTracer tracer(FROM_HERE);
 
   MockRead reads[] = {
       MockRead(ASYNC, 0, "hello"), MockRead(ASYNC, 1, "world"),
       MockRead(ASYNC, ERR_CONNECTION_RESET, 2),
   };
 
   SequencedSocketData data(reads, arraysize(reads), nullptr, 0);
   std::unique_ptr<StreamSocket> socket = MakeTestSocket(&data);
   std::unique_ptr<SocketBIOAdapter> adapter =
       base::MakeUnique<SocketBIOAdapter>(socket.get(), 100, 100, this);
   BIO* bio = adapter->bio();
   EXPECT_FALSE(adapter->HasPendingReadData());
 
   // Attempt to read data. It will fail but schedule a Read.
   char buf[10];
   ExpectBlockingRead(bio, buf, sizeof(buf));
   EXPECT_FALSE(adapter->HasPendingReadData());
 
-  // After waiting, the data is available.
+  // After waiting, the data is available if Read() is used.
   WaitForReadReady();
-  EXPECT_TRUE(adapter->HasPendingReadData());
+  if (GetParam() == READ_IF_READY_ENABLED_SUPPORTED) {
+    EXPECT_FALSE(adapter->HasPendingReadData());
+  } else {
+    EXPECT_TRUE(adapter->HasPendingReadData());
+  }
 
   // The first read is now available synchronously.
   EXPECT_EQ(5, BIO_read(bio, buf, sizeof(buf)));
   EXPECT_EQ(0, memcmp("hello", buf, 5));
   EXPECT_FALSE(adapter->HasPendingReadData());
 
   // The adapter does not schedule another Read until BIO_read is next called.
   base::RunLoop().RunUntilIdle();
   EXPECT_FALSE(adapter->HasPendingReadData());
 
   // This time, under-request the data. The adapter should still read the full
   // amount.
   ExpectBlockingRead(bio, buf, 1);
   EXPECT_FALSE(adapter->HasPendingReadData());
+
+  // After waiting, the data is available if Read() is used.
   WaitForReadReady();
-  EXPECT_TRUE(adapter->HasPendingReadData());
+  if (GetParam() == READ_IF_READY_ENABLED_SUPPORTED) {
+    EXPECT_FALSE(adapter->HasPendingReadData());
+  } else {
+    EXPECT_TRUE(adapter->HasPendingReadData());
+  }
 
   // The next read is now available synchronously.
   EXPECT_EQ(5, BIO_read(bio, buf, sizeof(buf)));
   EXPECT_EQ(0, memcmp("world", buf, 5));
   EXPECT_FALSE(adapter->HasPendingReadData());
 
   // The error is not yet available.
   ExpectBlockingRead(bio, buf, sizeof(buf));
   WaitForReadReady();
 
   // The error is now available synchoronously.
   ExpectReadError(bio, ERR_CONNECTION_RESET, tracer);
 }
 
 // Test that synchronous EOF is mapped to ERR_CONNECTION_CLOSED.
-TEST_F(SocketBIOAdapterTest, ReadEOFSync) {
+TEST_P(SocketBIOAdapterTest, ReadEOFSync) {
   crypto::OpenSSLErrStackTracer tracer(FROM_HERE);
 
   MockRead reads[] = {
       MockRead(SYNCHRONOUS, 0, 0),
   };
 
   SequencedSocketData data(reads, arraysize(reads), nullptr, 0);
   std::unique_ptr<StreamSocket> socket = MakeTestSocket(&data);
   std::unique_ptr<SocketBIOAdapter> adapter =
       base::MakeUnique<SocketBIOAdapter>(socket.get(), 100, 100, this);
 
   ExpectReadError(adapter->bio(), ERR_CONNECTION_CLOSED, tracer);
 }
 
 // Test that asynchronous EOF is mapped to ERR_CONNECTION_CLOSED.
-TEST_F(SocketBIOAdapterTest, ReadEOFAsync) {
+TEST_P(SocketBIOAdapterTest, ReadEOFAsync) {
   crypto::OpenSSLErrStackTracer tracer(FROM_HERE);
 
   MockRead reads[] = {
       MockRead(ASYNC, 0, 0),
   };
 
   SequencedSocketData data(reads, arraysize(reads), nullptr, 0);
   std::unique_ptr<StreamSocket> socket = MakeTestSocket(&data);
   std::unique_ptr<SocketBIOAdapter> adapter =
       base::MakeUnique<SocketBIOAdapter>(socket.get(), 100, 100, this);
 
   char buf;
   ExpectBlockingRead(adapter->bio(), &buf, 1);
   WaitForReadReady();
   ExpectReadError(adapter->bio(), ERR_CONNECTION_CLOSED, tracer);
 }
 
 // Test that data can be written synchronously.
-TEST_F(SocketBIOAdapterTest, WriteSync) {
+TEST_P(SocketBIOAdapterTest, WriteSync) {
   crypto::OpenSSLErrStackTracer tracer(FROM_HERE);
 
   MockWrite writes[] = {
       MockWrite(SYNCHRONOUS, 0, "hello"),
       MockWrite(SYNCHRONOUS, 1, "wor"),
       MockWrite(SYNCHRONOUS, 2, "ld"),
       MockWrite(SYNCHRONOUS, 3, "helloworld"),
       MockWrite(SYNCHRONOUS, ERR_CONNECTION_RESET, 4),
   };
 
@@ skipping 14 matching lines @@
 
   // Writing "aaaaa" fails (event 4), but there is a write buffer, so errors
   // are delayed.
   EXPECT_EQ(5, BIO_write(bio, "aaaaa", 5));
 
   // However once the error is registered, subsequent writes fail.
   ExpectWriteError(bio, ERR_CONNECTION_RESET, tracer);
 }
 
 // Test that data can be written asynchronously.
-TEST_F(SocketBIOAdapterTest, WriteAsync) {
+TEST_P(SocketBIOAdapterTest, WriteAsync) {
   crypto::OpenSSLErrStackTracer tracer(FROM_HERE);
 
   MockWrite writes[] = {
       MockWrite(ASYNC, 0, "aaa"),
       MockWrite(ASYNC, ERR_IO_PENDING, 1),  // pause
       MockWrite(ASYNC, 2, "aabbbbb"),
       MockWrite(ASYNC, 3, "ccc"),
       MockWrite(ASYNC, 4, "ddd"),
       MockWrite(ASYNC, ERR_IO_PENDING, 5),  // pause
       MockWrite(ASYNC, 6, "dd"),
@@ skipping 108 matching lines @@
   EXPECT_EQ(5, BIO_write(bio, "hhhhh", 5));
 
   // Release the write error (event 14). At this point future BIO_write calls
   // fail. The buffer was not full, so OnWriteReady is not signalled.
   base::RunLoop().RunUntilIdle();
   ExpectWriteError(bio, ERR_CONNECTION_RESET, tracer);
 }
 
 // Test that a failed socket write is reported through BIO_read and prevents it
 // from scheduling a socket read. See https://crbug.com/249848.
-TEST_F(SocketBIOAdapterTest, WriteStopsRead) {
+TEST_P(SocketBIOAdapterTest, WriteStopsRead) {
   crypto::OpenSSLErrStackTracer tracer(FROM_HERE);
 
   MockWrite writes[] = {
       MockWrite(SYNCHRONOUS, ERR_CONNECTION_RESET, 0),
   };
 
   SequencedSocketData data(nullptr, 0, writes, arraysize(writes));
   std::unique_ptr<StreamSocket> socket = MakeTestSocket(&data);
   std::unique_ptr<SocketBIOAdapter> adapter =
       base::MakeUnique<SocketBIOAdapter>(socket.get(), 100, 100, this);
   BIO* bio = adapter->bio();
 
   // The write fails, but there is a write buffer, so errors are delayed.
   EXPECT_EQ(5, BIO_write(bio, "aaaaa", 5));
 
   // The write error is surfaced out of BIO_read. There are no MockReads, so
   // this also tests that no socket reads are attempted.
   ExpectReadError(bio, ERR_CONNECTION_RESET, tracer);
 }
 
 // Test that a synchronous failed socket write interrupts a blocked
 // BIO_read. See https://crbug.com/249848.
-TEST_F(SocketBIOAdapterTest, SyncWriteInterruptsRead) {
+TEST_P(SocketBIOAdapterTest, SyncWriteInterruptsRead) {
   crypto::OpenSSLErrStackTracer tracer(FROM_HERE);
 
   MockRead reads[] = {
       MockRead(SYNCHRONOUS, ERR_IO_PENDING, 0),
   };
 
   MockWrite writes[] = {
       MockWrite(SYNCHRONOUS, ERR_CONNECTION_RESET, 1),
   };
 
@@ skipping 12 matching lines @@
 
   // The write error triggers OnReadReady.
   WaitForReadReady();
 
   // The write error is surfaced out of BIO_read.
   ExpectReadError(bio, ERR_CONNECTION_RESET, tracer);
 }
 
 // Test that an asynchronous failed socket write interrupts a blocked
 // BIO_read. See https://crbug.com/249848.
-TEST_F(SocketBIOAdapterTest, AsyncWriteInterruptsRead) {
+TEST_P(SocketBIOAdapterTest, AsyncWriteInterruptsRead) {
   crypto::OpenSSLErrStackTracer tracer(FROM_HERE);
 
   MockRead reads[] = {
       MockRead(SYNCHRONOUS, ERR_IO_PENDING, 0),
   };
 
   MockWrite writes[] = {
       MockWrite(ASYNC, ERR_CONNECTION_RESET, 1),
   };
 
@@ skipping 14 matching lines @@
   // OnReadReady is signaled. The write buffer was not full, so OnWriteReady is
   // not signaled.
   WaitForReadReady();
 
   // The write error is surfaced out of BIO_read.
   ExpectReadError(bio, ERR_CONNECTION_RESET, tracer);
 }
 
 // Test that an asynchronous failed socket write interrupts a blocked BIO_read,
 // signaling both if the buffer was full. See https://crbug.com/249848.
-TEST_F(SocketBIOAdapterTest, AsyncWriteInterruptsBoth) {
+TEST_P(SocketBIOAdapterTest, AsyncWriteInterruptsBoth) {
   crypto::OpenSSLErrStackTracer tracer(FROM_HERE);
 
   MockRead reads[] = {
       MockRead(SYNCHRONOUS, ERR_IO_PENDING, 0),
   };
 
   MockWrite writes[] = {
       MockWrite(ASYNC, ERR_CONNECTION_RESET, 1),
   };
 
@@ skipping 14 matching lines @@
   // OnReadReady is signaled. The write buffer was full, so both OnWriteReady is
   // also signaled.
   WaitForBothReady();
 
   // The write error is surfaced out of BIO_read.
   ExpectReadError(bio, ERR_CONNECTION_RESET, tracer);
 }
 
 // Test that SocketBIOAdapter handles OnWriteReady deleting itself when both
 // need to be signaled.
-TEST_F(SocketBIOAdapterTest, DeleteOnWriteReady) {
+TEST_P(SocketBIOAdapterTest, DeleteOnWriteReady) {
   crypto::OpenSSLErrStackTracer tracer(FROM_HERE);
 
   MockRead reads[] = {
       MockRead(SYNCHRONOUS, ERR_IO_PENDING, 0),
   };
 
   MockWrite writes[] = {
       MockWrite(ASYNC, ERR_CONNECTION_RESET, 1),
   };
 
@@ skipping 12 matching lines @@
   // Both OnWriteReady and OnReadReady would be signaled, but OnWriteReady
   // deletes the adapter first.
   set_reset_on_write_ready(&adapter);
   WaitForWriteReady(nullptr);
 
   EXPECT_FALSE(adapter);
 }
 
 // Test that using a BIO after the underlying adapter is destroyed fails
 // gracefully.
-TEST_F(SocketBIOAdapterTest, Detached) {
+TEST_P(SocketBIOAdapterTest, Detached) {
   crypto::OpenSSLErrStackTracer tracer(FROM_HERE);
 
   SequencedSocketData data(nullptr, 0, nullptr, 0);
   std::unique_ptr<StreamSocket> socket = MakeTestSocket(&data);
   std::unique_ptr<SocketBIOAdapter> adapter =
       base::MakeUnique<SocketBIOAdapter>(socket.get(), 100, 100, this);
 
   // Retain an additional reference to the BIO.
   bssl::UniquePtr<BIO> bio(adapter->bio());
   BIO_up_ref(bio.get());
 
   // Release the adapter.
   adapter.reset();
 
   ExpectReadError(bio.get(), ERR_UNEXPECTED, tracer);
   ExpectWriteError(bio.get(), ERR_UNEXPECTED, tracer);
 }
 
 }  // namespace net