SSLServerSocketTest.DataTransfer should keep reading until it has read

net/socket/ssl_server_socket_unittest.cc

 // Copyright (c) 2011 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.
 
 // This test suite uses SSLClientSocket to test the implementation of
 // SSLServerSocket. In order to establish connections between the sockets
 // we need two additional classes:
 // 1. FakeSocket
 //    Connects SSL socket to FakeDataChannel. This class is just a stub.
 //
@@ skipping 353 matching lines @@
   if (client_ret == net::ERR_IO_PENDING) {
     ASSERT_EQ(net::OK, connect_callback.WaitForResult());
   }
   if (server_ret == net::ERR_IO_PENDING) {
     ASSERT_EQ(net::OK, handshake_callback.WaitForResult());
   }
 
   const int kReadBufSize = 1024;
   scoped_refptr<net::StringIOBuffer> write_buf =
       new net::StringIOBuffer("testing123");
-  scoped_refptr<net::IOBuffer> read_buf = new net::IOBuffer(kReadBufSize);
+  scoped_refptr<net::DrainableIOBuffer> read_buf =
+      new net::DrainableIOBuffer(new net::IOBuffer(kReadBufSize),
+                                 kReadBufSize);
 
   // Write then read.
   TestCompletionCallback write_callback;
   TestCompletionCallback read_callback;
   server_ret = server_socket_->Write(write_buf, write_buf->size(),
                                      &write_callback);
   EXPECT_TRUE(server_ret > 0 || server_ret == net::ERR_IO_PENDING);
-  client_ret = client_socket_->Read(read_buf, kReadBufSize, &read_callback);
+  client_ret = client_socket_->Read(read_buf, read_buf->BytesRemaining(),

[rvargas (doing something else), 2011/08/15 18:31:41]

Is it important for this test that the write and the read start before running
the message loop? If not, we could get the result of the write and then loop for
the read instead of making a first read outside of the loop.

[wtc, 2011/08/16 00:04:05]

I am not sure either.  I can implement this change in
Patch Set 3 and let hclam pick one.

+                                    &read_callback);
   EXPECT_TRUE(client_ret > 0 || client_ret == net::ERR_IO_PENDING);
 
   if (server_ret == net::ERR_IO_PENDING) {

[rvargas (doing something else), 2011/08/15 18:31:41]

This can be simplified with write_callback.GetResult().

-    EXPECT_GT(write_callback.WaitForResult(), 0);
+    server_ret = write_callback.WaitForResult();
+    EXPECT_GT(server_ret, 0);
   }
   if (client_ret == net::ERR_IO_PENDING) {
-    EXPECT_GT(read_callback.WaitForResult(), 0);
+    client_ret = read_callback.WaitForResult();
+    EXPECT_GT(client_ret, 0);
   }
+  int bytes_read = client_ret;
+  read_buf->DidConsume(client_ret);
+  while (bytes_read < write_buf->size()) {
+    client_ret = client_socket_->Read(read_buf, read_buf->BytesRemaining(),
+                                      &read_callback);
+    EXPECT_TRUE(client_ret > 0 || client_ret == net::ERR_IO_PENDING);
+    if (client_ret == net::ERR_IO_PENDING) {
+      client_ret = read_callback.WaitForResult();
+      EXPECT_GT(client_ret, 0);

[rvargas (doing something else), 2011/08/15 18:31:41]

If we really care about failures, we should add code to handle them because
right now we could get stuck in the loop.

+    }
+    bytes_read += client_ret;

[rvargas (doing something else), 2011/08/15 18:31:41]

nit: we could use BytesConsumed() here, instead of keeping an external counter.

+    read_buf->DidConsume(client_ret);
+  }
+  read_buf->SetOffset(0);
+  EXPECT_EQ(write_buf->size(), bytes_read);
   EXPECT_EQ(0, memcmp(write_buf->data(), read_buf->data(), write_buf->size()));
 
   // Read then write.

[rvargas (doing something else), 2011/08/15 18:31:41]

Looks like we are not really doing this.

   write_buf = new net::StringIOBuffer("hello123");
-  server_ret = server_socket_->Read(read_buf, kReadBufSize, &read_callback);
+  server_ret = server_socket_->Read(read_buf, read_buf->BytesRemaining(),
+                                    &read_callback);
   EXPECT_TRUE(server_ret > 0 || server_ret == net::ERR_IO_PENDING);
   client_ret = client_socket_->Write(write_buf, write_buf->size(),
                                      &write_callback);
   EXPECT_TRUE(client_ret > 0 || client_ret == net::ERR_IO_PENDING);
 
   if (server_ret == net::ERR_IO_PENDING) {
-    EXPECT_GT(read_callback.WaitForResult(), 0);
+    server_ret = read_callback.WaitForResult();
+    EXPECT_GT(server_ret, 0);
   }
   if (client_ret == net::ERR_IO_PENDING) {
-    EXPECT_GT(write_callback.WaitForResult(), 0);
+    client_ret = write_callback.WaitForResult();
+    EXPECT_GT(client_ret, 0);
   }
+  bytes_read = server_ret;
+  read_buf->DidConsume(server_ret);
+  while (bytes_read < write_buf->size()) {
+    server_ret = server_socket_->Read(read_buf, read_buf->BytesRemaining(),
+                                      &read_callback);
+    EXPECT_TRUE(server_ret > 0 || server_ret == net::ERR_IO_PENDING);
+    if (server_ret == net::ERR_IO_PENDING) {
+      server_ret = read_callback.WaitForResult();
+      EXPECT_GT(server_ret, 0);
+    }
+    bytes_read += server_ret;
+    read_buf->DidConsume(server_ret);
+  }
+  read_buf->SetOffset(0);
+  EXPECT_EQ(write_buf->size(), bytes_read);
   EXPECT_EQ(0, memcmp(write_buf->data(), read_buf->data(), write_buf->size()));
 }
 
 // This test executes ExportKeyingMaterial() on the client and server sockets,
 // after connecting them, and verifies that the results match.
 // This test will fail if False Start is enabled (see crbug.com/90208).
 TEST_F(SSLServerSocketTest, ExportKeyingMaterial) {
   Initialize();
 
   TestCompletionCallback connect_callback;
@@ skipping 29 matching lines @@
   const char* kKeyingLabelBad = "EXPERIMENTAL-server-socket-test-bad";
   unsigned char client_bad[kKeyingMaterialSize];
   rv = client_socket_->ExportKeyingMaterial(kKeyingLabelBad, kKeyingContext,
                                             client_bad, sizeof(client_bad));
   ASSERT_EQ(rv, net::OK);
   EXPECT_TRUE(memcmp(server_out, client_bad, sizeof(server_out)) != 0);
 }
 #endif
 
 }  // namespace net