Style refactoring to make MockRead initializers more readable.

net/http/http_network_transaction_unittest.cc

 // Copyright (c) 2006-2008 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 "base/compiler_specific.h"
 #include "base/platform_test.h"
 #include "net/base/client_socket_factory.h"
 #include "net/base/test_completion_callback.h"
 #include "net/base/upload_data.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_network_transaction.h"
 #include "net/http/http_transaction_unittest.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 //-----------------------------------------------------------------------------
 
 
 struct MockConnect {
   bool async;
   int result;
+
+  // Asynchronous connection success.
+  MockConnect() : async(true), result(net::OK) { }

[wtc, 2008/09/29 20:16:33]

Nit: list the constructor before the members, as you do
in MockRead and MockSocket.

[eroman, 2008/09/29 20:29:17]

oops. will fix.
(I already submitted this as I rely on it in another CL. I am fixing it in that
CL).

 };
 
 struct MockRead {
+  // Read failure (no data).
+  MockRead(bool async, int result) : async(async) , result(result), data(NULL),
+      data_len(0) { }
+
+  // Asynchronous read success (inferred data length).
+  MockRead(const char* data) : async(true), data(data), data_len(strlen(data)),
+      result(0) { }
+
+  // Read success (inferred data length).
+  MockRead(bool async, const char* data) : async(async), data(data),
+      data_len(strlen(data)), result(0) { }
+
+  // Read success.
+  MockRead(bool async, const char* data, int data_len) : async(async),
+      data(data), data_len(data_len), result(0) { }

[wtc, 2008/09/29 20:16:33]

Ideally, these should be accompanied with the C++ machinery
to ensure that MockRead can only be constructed in these
four ways.  In particular, we should disallow initializing
with an initializer list like { false, net::OK, NULL, 0 }.

Do you know how to do that?  Just curious.

[eroman, 2008/09/29 20:29:17]

That happens automatically -- once you have a user-defined constructor, the type
cannot be initialized with an initializer list.

+
   bool async;
-  int result;  // Ignored if data is non-null.
+  int result;
   const char* data;
-  int data_len;  // -1 if strlen(data) should be used.
+  int data_len;
 };
 
 struct MockSocket {
+  MockSocket() : reads(NULL) { }
+
   MockConnect connect;
-  MockRead* reads;  // Terminated by a MockRead element with data == NULL.

[wtc, 2008/09/29 20:16:33]

Do we still need to terminate the 'reads' array with a
MockRead element with data == NULL?

Or is this comment actually wrong?

[eroman, 2008/09/29 20:29:17]

as far as I can tell, that comment was wrong -- none of the tests do this. (they
do however terminate mock_sockets with a null entry).

+  MockRead* reads;
 };
 
 // Holds an array of MockSocket elements.  As MockTCPClientSocket objects get
 // instantiated, they take their data from the i'th element of this array.
 //
 // Tests should assign the first N entries of mock_sockets to point to valid
 // MockSocket objects.  The first unused entry should be NULL'd.
 //
 MockSocket* mock_sockets[10];
 
@@ skipping 33 matching lines @@
   }
   virtual bool IsConnected() const {
     return connected_;
   }
   // Socket methods:
   virtual int Read(char* buf, int buf_len, net::CompletionCallback* callback) {
     DCHECK(!callback_);
     MockRead& r = data_->reads[read_index_];
     int result;
     if (r.data) {
-      if (r.data_len == -1)
-        r.data_len = static_cast<int>(strlen(r.data));
       if (r.data_len - read_offset_ > 0) {
         result = std::min(buf_len, r.data_len - read_offset_);
         memcpy(buf, r.data + read_offset_, result);
         read_offset_ += result;
         if (read_offset_ == r.data_len) {
           read_index_++;
           read_offset_ = 0;
         }
       } else {
         result = 0;  // EOF
@@ skipping 86 matching lines @@
 
   net::HttpTransaction* trans = new net::HttpNetworkTransaction(
       CreateSession(), &mock_socket_factory);
 
   net::HttpRequestInfo request;
   request.method = "GET";
   request.url = GURL("http://www.google.com/");
   request.load_flags = 0;
 
   MockSocket data;
-  data.connect.async = true;
-  data.connect.result = net::OK;
   data.reads = data_reads;
   mock_sockets[0] = &data;
   mock_sockets[1] = NULL;
 
   TestCompletionCallback callback;
 
   int rv = trans->Start(&request, &callback);
   EXPECT_EQ(net::ERR_IO_PENDING, rv);
 
   rv = callback.WaitForResult();
@@ skipping 19 matching lines @@
 //-----------------------------------------------------------------------------
 
 TEST_F(HttpNetworkTransactionTest, Basic) {
   net::HttpTransaction* trans = new net::HttpNetworkTransaction(
       CreateSession(), &mock_socket_factory);
   trans->Destroy();
 }
 
 TEST_F(HttpNetworkTransactionTest, SimpleGET) {
   MockRead data_reads[] = {
-    { true, 0, "HTTP/1.0 200 OK\r\n\r\n", -1 },
-    { true, 0, "hello world", -1 },
-    { false, net::OK, NULL, 0 },
+    MockRead("HTTP/1.0 200 OK\r\n\r\n"),
+    MockRead("hello world"),
+    MockRead(false, net::OK),
   };
   SimpleGetHelperResult out = SimpleGetHelper(data_reads);
   EXPECT_EQ("HTTP/1.0 200 OK", out.status_line);
   EXPECT_EQ("hello world", out.response_data);
 }
 
 // Response with no status line.
 TEST_F(HttpNetworkTransactionTest, SimpleGETNoHeaders) {
   MockRead data_reads[] = {
-    { true, 0, "hello world", -1 },
-    { false, net::OK, NULL, 0 },
+    MockRead("hello world"),
+    MockRead(false, net::OK),
   };
   SimpleGetHelperResult out = SimpleGetHelper(data_reads);
   EXPECT_EQ("HTTP/0.9 200 OK", out.status_line);
   EXPECT_EQ("hello world", out.response_data);
 }
 
 // Allow up to 4 bytes of junk to precede status line.
 TEST_F(HttpNetworkTransactionTest, StatusLineJunk2Bytes) {
   MockRead data_reads[] = {
-    { true, 0, "xxxHTTP/1.0 404 Not Found\nServer: blah\n\nDATA", -1 },
-    { false, net::OK, NULL, 0 },
+    MockRead("xxxHTTP/1.0 404 Not Found\nServer: blah\n\nDATA"),
+    MockRead(false, net::OK),
   };
   SimpleGetHelperResult out = SimpleGetHelper(data_reads);
   EXPECT_EQ("HTTP/1.0 404 Not Found", out.status_line);
   EXPECT_EQ("DATA", out.response_data);
 }
 
 // Allow up to 4 bytes of junk to precede status line.
 TEST_F(HttpNetworkTransactionTest, StatusLineJunk4Bytes) {
   MockRead data_reads[] = {
-    { true, 0, "\n\nQJHTTP/1.0 404 Not Found\nServer: blah\n\nDATA", -1 },
-    { false, net::OK, NULL, 0 },
+    MockRead("\n\nQJHTTP/1.0 404 Not Found\nServer: blah\n\nDATA"),
+    MockRead(false, net::OK),
   };
   SimpleGetHelperResult out = SimpleGetHelper(data_reads);
   EXPECT_EQ("HTTP/1.0 404 Not Found", out.status_line);
   EXPECT_EQ("DATA", out.response_data);
 }
 
 // Beyond 4 bytes of slop and it should fail to find a status line.
 TEST_F(HttpNetworkTransactionTest, StatusLineJunk5Bytes) {
   MockRead data_reads[] = {
-    { true, 0, "xxxxxHTTP/1.1 404 Not Found\nServer: blah", -1 },
-    { false, net::OK, NULL, 0 },
+    MockRead("xxxxxHTTP/1.1 404 Not Found\nServer: blah"),
+    MockRead(false, net::OK),
   };
   SimpleGetHelperResult out = SimpleGetHelper(data_reads);
   EXPECT_EQ("HTTP/0.9 200 OK", out.status_line);
   EXPECT_EQ("xxxxxHTTP/1.1 404 Not Found\nServer: blah", out.response_data);
 }
 
 // Same as StatusLineJunk4Bytes, except the read chunks are smaller.
 TEST_F(HttpNetworkTransactionTest, StatusLineJunk4Bytes_Slow) {
   MockRead data_reads[] = {
-    { true, 0, "\n", -1 },
-    { true, 0, "\n", -1 },
-    { true, 0, "Q", -1 },
-    { true, 0, "J", -1 },
-    { true, 0, "HTTP/1.0 404 Not Found\nServer: blah\n\nDATA", -1 },
-    { false, net::OK, NULL, 0 },
+    MockRead("\n"),
+    MockRead("\n"),
+    MockRead("Q"),
+    MockRead("J"),
+    MockRead("HTTP/1.0 404 Not Found\nServer: blah\n\nDATA"),
+    MockRead(false, net::OK),
   };
   SimpleGetHelperResult out = SimpleGetHelper(data_reads);
   EXPECT_EQ("HTTP/1.0 404 Not Found", out.status_line);
   EXPECT_EQ("DATA", out.response_data);
 }
 
 // Close the connection before enough bytes to have a status line.
 TEST_F(HttpNetworkTransactionTest, StatusLinePartial) {
   MockRead data_reads[] = {
-    { true, 0, "HTT", -1 },
-    { false, net::OK, NULL, 0 },
+    MockRead("HTT"),
+    MockRead(false, net::OK),
   };
   SimpleGetHelperResult out = SimpleGetHelper(data_reads);
   EXPECT_EQ("HTTP/0.9 200 OK", out.status_line);
   EXPECT_EQ("HTT", out.response_data);
 }
 
 // Simulate a 204 response, lacking a Content-Length header, sent over a
 // persistent connection.  The response should still terminate since a 204
 // cannot have a response body.
 TEST_F(HttpNetworkTransactionTest, StopsReading204) {
   MockRead data_reads[] = {
-    { true, 0, "HTTP/1.1 204 No Content\r\n\r\n", -1 },
-    { true, 0, "junk", -1 },  // Should not be read!!
-    { false, net::OK, NULL, 0 },
+    MockRead("HTTP/1.1 204 No Content\r\n\r\n"),
+    MockRead("junk"),  // Should not be read!!
+    MockRead(false, net::OK),
   };
   SimpleGetHelperResult out = SimpleGetHelper(data_reads);
   EXPECT_EQ("HTTP/1.1 204 No Content", out.status_line);
   EXPECT_EQ("", out.response_data);
 }
 
 TEST_F(HttpNetworkTransactionTest, ReuseConnection) {
   scoped_refptr<net::HttpNetworkSession> session = CreateSession();
 
   MockRead data_reads[] = {
-    { true, 0, "HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n", -1 },
-    { true, 0, "hello", -1 },
-    { true, 0, "HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n", -1 },
-    { true, 0, "world", -1 },
-    { false, net::OK, NULL, 0 },
+    MockRead("HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n"),
+    MockRead("hello"),
+    MockRead("HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n"),
+    MockRead("world"),
+    MockRead(false, net::OK),
   };
   MockSocket data;
-  data.connect.async = true;
-  data.connect.result = net::OK;
   data.reads = data_reads;
   mock_sockets[0] = &data;
   mock_sockets[1] = NULL;
 
   const char* kExpectedResponseData[] = {
     "hello", "world"
   };
 
   for (int i = 0; i < 2; ++i) {
     net::HttpTransaction* trans =
@@ skipping 35 matching lines @@
       CreateSession(), &mock_socket_factory);
 
   net::HttpRequestInfo request;
   request.method = "POST";
   request.url = GURL("http://www.foo.com/");
   request.upload_data = new net::UploadData;
   request.upload_data->AppendBytes("foo", 3);
   request.load_flags = 0;
 
   MockRead data_reads[] = {
-    { true, 0, "HTTP/1.0 100 Continue\r\n\r\n", -1 },
-    { true, 0, "HTTP/1.0 200 OK\r\n\r\n", -1 },
-    { true, 0, "hello world", -1 },
-    { false, net::OK, NULL, 0 },
+    MockRead("HTTP/1.0 100 Continue\r\n\r\n"),
+    MockRead("HTTP/1.0 200 OK\r\n\r\n"),
+    MockRead("hello world"),
+    MockRead(false, net::OK),
   };
   MockSocket data;
-  data.connect.async = true;
-  data.connect.result = net::OK;
   data.reads = data_reads;
   mock_sockets[0] = &data;
   mock_sockets[1] = NULL;
 
   TestCompletionCallback callback;
 
   int rv = trans->Start(&request, &callback);
   EXPECT_EQ(net::ERR_IO_PENDING, rv);
 
   rv = callback.WaitForResult();
@@ skipping 21 matching lines @@
 void HttpNetworkTransactionTest::KeepAliveConnectionResendRequestTest(
     const MockRead& read_failure) {
   scoped_refptr<net::HttpNetworkSession> session = CreateSession();
 
   net::HttpRequestInfo request;
   request.method = "GET";
   request.url = GURL("http://www.foo.com/");
   request.load_flags = 0;
 
   MockRead data1_reads[] = {
-    { true, 0, "HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n", -1 },
-    { true, 0, "hello", -1 },
+    MockRead("HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n"),
+    MockRead("hello"),
     read_failure,  // Now, we reuse the connection and fail the first read.
   };
   MockSocket data1;
-  data1.connect.async = true;
-  data1.connect.result = net::OK;
   data1.reads = data1_reads;
   mock_sockets[0] = &data1;
 
   MockRead data2_reads[] = {
-    { true, 0, "HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n", -1 },
-    { true, 0, "world", -1 },
-    { true, net::OK, NULL, 0 },
+    MockRead("HTTP/1.1 200 OK\r\nContent-Length: 5\r\n\r\n"),
+    MockRead("world"),
+    MockRead(true, net::OK),
   };
   MockSocket data2;
-  data2.connect.async = true;
-  data2.connect.result = net::OK;
   data2.reads = data2_reads;
   mock_sockets[1] = &data2;
 
   const char* kExpectedResponseData[] = {
     "hello", "world"
   };
 
   for (int i = 0; i < 2; ++i) {
     TestCompletionCallback callback;
 
@@ skipping 18 matching lines @@
     EXPECT_EQ(kExpectedResponseData[i], response_data);
 
     trans->Destroy();
 
     // Empty the current queue.
     MessageLoop::current()->RunAllPending();
   }
 }
 
 TEST_F(HttpNetworkTransactionTest, KeepAliveConnectionReset) {
-  MockRead read_failure = { true, net::ERR_CONNECTION_RESET, NULL, 0 };
+  MockRead read_failure(true, net::ERR_CONNECTION_RESET);
   KeepAliveConnectionResendRequestTest(read_failure);
 }
 
 TEST_F(HttpNetworkTransactionTest, KeepAliveConnectionEOF) {
-  MockRead read_failure = { false, net::OK, NULL, 0 };  // EOF
+  MockRead read_failure(false, net::OK);  // EOF
   KeepAliveConnectionResendRequestTest(read_failure);
 }
 
 TEST_F(HttpNetworkTransactionTest, NonKeepAliveConnectionReset) {
   net::HttpTransaction* trans = new net::HttpNetworkTransaction(
       CreateSession(), &mock_socket_factory);
 
   net::HttpRequestInfo request;
   request.method = "GET";
   request.url = GURL("http://www.google.com/");
   request.load_flags = 0;
 
   MockRead data_reads[] = {
-    { true, net::ERR_CONNECTION_RESET, NULL, 0 },
-    { true, 0, "HTTP/1.0 200 OK\r\n\r\n", -1 },  // Should not be used
-    { true, 0, "hello world", -1 },
-    { false, net::OK, NULL, 0 },
+    MockRead(true, net::ERR_CONNECTION_RESET),
+    MockRead("HTTP/1.0 200 OK\r\n\r\n"),  // Should not be used
+    MockRead("hello world"),
+    MockRead(false, net::OK),
   };
   MockSocket data;
-  data.connect.async = true;
-  data.connect.result = net::OK;
   data.reads = data_reads;
   mock_sockets[0] = &data;
   mock_sockets[1] = NULL;
 
   TestCompletionCallback callback;
 
   int rv = trans->Start(&request, &callback);
   EXPECT_EQ(net::ERR_IO_PENDING, rv);
 
   rv = callback.WaitForResult();
@@ skipping 12 matching lines @@
 // connection without sending any response header or body?
 //
 // IE7: error page
 // Safari 3.1.2 (Windows): error page
 // Firefox 3.0.1: blank page
 // Opera 9.52: after five attempts, blank page
 // Us with WinHTTP: error page (net::ERR_INVALID_RESPONSE)
 // Us: blank page
 TEST_F(HttpNetworkTransactionTest, NonKeepAliveConnectionEOF) {
   MockRead data_reads[] = {
-    { false, net::OK, NULL, 0 },  // EOF
-    { true, 0, "HTTP/1.0 200 OK\r\n\r\n", -1 },  // Should not be used
-    { true, 0, "hello world", -1 },
-    { false, net::OK, NULL, 0 },
+    MockRead(false, net::OK),  // EOF
+    MockRead("HTTP/1.0 200 OK\r\n\r\n"),  // Should not be used
+    MockRead("hello world"),
+    MockRead(false, net::OK),
   };
   SimpleGetHelperResult out = SimpleGetHelper(data_reads);
   EXPECT_EQ("HTTP/0.9 200 OK", out.status_line);
   EXPECT_EQ("", out.response_data);
 }