Implement SSLServerContextImpl::SocketImpl::ReadIfReady.

Hit ERR_READ_IF_READY_NOT_IMPLEMENTED in SocketDataPump::ReceiveMore()
when switching the Cast HTTP server to network::server::HttpServer. Add
the implementation to unblock the migration.

Bug: b/118771830
Merge-With: eureka-internal/342047
Test: CQ & manual
Change-Id: I44666fa5b7f741d757c4382798b69f461a1e7fad
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2018597
Commit-Queue: Junbo Ke <juke@chromium.org>
Reviewed-by: David Benjamin <davidben@chromium.org>
Cr-Commit-Position: refs/heads/master@{#735731}

net/socket/ssl_server_socket_impl.cc

@@ -85,6 +85,9 @@ class SSLServerContextImpl::SocketImpl : public SSLServerSocket,
   int Read(IOBuffer* buf,
            int buf_len,
            CompletionOnceCallback callback) override;
+  int ReadIfReady(IOBuffer* buf,
+                  int buf_len,
+                  CompletionOnceCallback callback) override;
   int Write(IOBuffer* buf,
             int buf_len,
             CompletionOnceCallback callback,
@@ -162,7 +165,7 @@ class SSLServerContextImpl::SocketImpl : public SSLServerSocket,
 
   void OnHandshakeIOComplete(int result);
 
-  int DoPayloadRead();
+  int DoPayloadRead(IOBuffer* buf, int buf_len);
   int DoPayloadWrite();
 
   int DoHandshakeLoop(int last_io_result);
@@ -435,23 +438,28 @@ int SSLServerContextImpl::SocketImpl::ExportKeyingMaterial(
 int SSLServerContextImpl::SocketImpl::Read(IOBuffer* buf,
                                            int buf_len,
                                            CompletionOnceCallback callback) {
+  int rv = ReadIfReady(buf, buf_len, std::move(callback));
+  if (rv == ERR_IO_PENDING) {
+    user_read_buf_ = buf;
+    user_read_buf_len_ = buf_len;
+  }
+  return rv;
+}
+
+int SSLServerContextImpl::SocketImpl::ReadIfReady(
+    IOBuffer* buf,
+    int buf_len,
+    CompletionOnceCallback callback) {
   DCHECK(user_read_callback_.is_null());
   DCHECK(user_handshake_callback_.is_null());
   DCHECK(!user_read_buf_);
   DCHECK(!callback.is_null());
-
-  user_read_buf_ = buf;
-  user_read_buf_len_ = buf_len;
-
   DCHECK(completed_handshake_);
 
-  int rv = DoPayloadRead();
+  int rv = DoPayloadRead(buf, buf_len);
 
   if (rv == ERR_IO_PENDING) {
     user_read_callback_ = std::move(callback);
-  } else {
-    user_read_buf_ = nullptr;
-    user_read_buf_len_ = 0;
   }
 
   return rv;
@@ -583,10 +591,14 @@ void SSLServerContextImpl::SocketImpl::OnReadReady() {
 
   // BoringSSL does not support renegotiation as a server, so the only other
   // operation blocked on Read is DoPayloadRead.
-  if (!user_read_buf_)
+  if (!user_read_buf_) {
+    if (!user_read_callback_.is_null()) {
+      DoReadCallback(OK);
+    }
     return;
+  }
 
-  int rv = DoPayloadRead();
+  int rv = DoPayloadRead(user_read_buf_.get(), user_read_buf_len_);
   if (rv != ERR_IO_PENDING)
     DoReadCallback(rv);
 }
@@ -618,14 +630,15 @@ void SSLServerContextImpl::SocketImpl::OnHandshakeIOComplete(int result) {
     DoHandshakeCallback(rv);
 }
 
-int SSLServerContextImpl::SocketImpl::DoPayloadRead() {
+int SSLServerContextImpl::SocketImpl::DoPayloadRead(IOBuffer* buf,
+                                                    int buf_len) {
   DCHECK(completed_handshake_);
   DCHECK_EQ(STATE_NONE, next_handshake_state_);
-  DCHECK(user_read_buf_);
-  DCHECK_GT(user_read_buf_len_, 0);
+  DCHECK(buf);
+  DCHECK_GT(buf_len, 0);
 
   crypto::OpenSSLErrStackTracer err_tracer(FROM_HERE);
-  int rv = SSL_read(ssl_.get(), user_read_buf_->data(), user_read_buf_len_);
+  int rv = SSL_read(ssl_.get(), buf->data(), buf_len);
   if (rv >= 0) {
     if (SSL_in_early_data(ssl_.get()))
       early_data_received_ = true;

net/socket/ssl_server_socket_unittest.cc

@@ -524,6 +524,31 @@ class SSLServerSocketTest : public PlatformTest, public WithTaskEnvironment {
   scoped_refptr<X509Certificate> server_cert_;
 };
 
+class SSLServerSocketReadTest : public SSLServerSocketTest,
+                                public ::testing::WithParamInterface<bool> {
+ protected:
+  SSLServerSocketReadTest() : read_if_ready_enabled_(GetParam()) {}
+
+  int Read(StreamSocket* socket,
+           IOBuffer* buf,
+           int buf_len,
+           CompletionOnceCallback callback) {
+    if (read_if_ready_enabled()) {
+      return socket->ReadIfReady(buf, buf_len, std::move(callback));
+    }
+    return socket->Read(buf, buf_len, std::move(callback));
+  }
+
+  bool read_if_ready_enabled() const { return read_if_ready_enabled_; }
+
+ private:
+  const bool read_if_ready_enabled_;
+};
+
+INSTANTIATE_TEST_SUITE_P(/* no prefix */,
+                         SSLServerSocketReadTest,
+                         ::testing::Bool());
+
 // This test only executes creation of client and server sockets. This is to
 // test that creation of sockets doesn't crash and have minimal code to run
 // with memory leak/corruption checking tools.
@@ -972,7 +997,7 @@ TEST_F(SSLServerSocketTest, HandshakeWithWrongClientCertSuppliedCached) {
   EXPECT_EQ(ERR_BAD_SSL_CLIENT_AUTH_CERT, client_ret);
 }
 
-TEST_F(SSLServerSocketTest, DataTransfer) {
+TEST_P(SSLServerSocketReadTest, DataTransfer) {
   ASSERT_NO_FATAL_FAILURE(CreateContext());
   ASSERT_NO_FATAL_FAILURE(CreateSockets());
 
@@ -1028,25 +1053,31 @@ TEST_F(SSLServerSocketTest, DataTransfer) {
 
   // Read then write.
   write_buf = base::MakeRefCounted<StringIOBuffer>("hello123");
-  server_ret = server_socket_->Read(
-      read_buf.get(), read_buf->BytesRemaining(), read_callback.callback());
-  EXPECT_TRUE(server_ret > 0 || server_ret == ERR_IO_PENDING);
+  server_ret = Read(server_socket_.get(), read_buf.get(),
+                    read_buf->BytesRemaining(), read_callback.callback());
+  EXPECT_EQ(server_ret, ERR_IO_PENDING);
   client_ret = client_socket_->Write(write_buf.get(), write_buf->size(),
                                      write_callback.callback(),
                                      TRAFFIC_ANNOTATION_FOR_TESTS);
   EXPECT_TRUE(client_ret > 0 || client_ret == ERR_IO_PENDING);
 
   server_ret = read_callback.GetResult(server_ret);
-  ASSERT_GT(server_ret, 0);
+  if (read_if_ready_enabled()) {
+    // ReadIfReady signals the data is available but does not consume it.
+    // The data is consumed later below.
+    ASSERT_EQ(server_ret, OK);
+  } else {
+    ASSERT_GT(server_ret, 0);
+    read_buf->DidConsume(server_ret);
+  }
   client_ret = write_callback.GetResult(client_ret);
   EXPECT_GT(client_ret, 0);
 
-  read_buf->DidConsume(server_ret);
   while (read_buf->BytesConsumed() < write_buf->size()) {
-    server_ret = server_socket_->Read(
-        read_buf.get(), read_buf->BytesRemaining(), read_callback.callback());
-    EXPECT_TRUE(server_ret > 0 || server_ret == ERR_IO_PENDING);
-    server_ret = read_callback.GetResult(server_ret);
+    server_ret = Read(server_socket_.get(), read_buf.get(),
+                      read_buf->BytesRemaining(), read_callback.callback());
+    // All the data was written above, so the data should be synchronously
+    // available out of both Read() and ReadIfReady().
     ASSERT_GT(server_ret, 0);
     read_buf->DidConsume(server_ret);
   }