Remove the UploadDataStream::Reset method. Instead,

rewind an UploadDataStream by recreating the object.

Fix a bug in MockTCPClientSocket::Write.  It needs to
increment write_index_.  Add two DCHECKs to simulate
the DCHECKS in SSLClientSocketWin::DoPayloadEncrypt.

Add a unit test.

R=darin,eroman
BUG=4062
Review URL: http://codereview.chromium.org/9384

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@4819 0039d316-1c4b-4281-b951-d872f2087c98

4 files changed, 109 insertions, 22 deletions

diff --git a/net/base/upload_data_stream.cc b/net/base/upload_data_stream.cc
index e874439..c25ccba 100644
--- a/net/base/upload_data_stream.cc
+++ b/net/base/upload_data_stream.cc
@@ -11,8 +11,12 @@ namespace net {
 
 UploadDataStream::UploadDataStream(const UploadData* data)
     : data_(data),
-      total_size_(data->GetContentLength()) {
-  Reset();
+      buf_len_(0),
+      next_element_(data->elements().begin()),
+      next_element_offset_(0),
+      next_element_remaining_(0),
+      total_size_(data->GetContentLength()),
+      current_position_(0) {
   FillBuf();
 }
 
@@ -31,15 +35,6 @@ void UploadDataStream::DidConsume(size_t num_bytes) {
   current_position_ += num_bytes;
 }
 
-void UploadDataStream::Reset() {
-  next_element_stream_.Close();
-  buf_len_ = 0;
-  next_element_ = data_->elements().begin();
-  next_element_offset_ = 0;
-  next_element_remaining_ = 0;
-  current_position_ = 0;
-}
-
 void UploadDataStream::FillBuf() {
   std::vector<UploadData::Element>::const_iterator end =
       data_->elements().end();
@@ -68,7 +63,7 @@ void UploadDataStream::FillBuf() {
       DCHECK(element.type() == UploadData::TYPE_FILE);
 
       if (!next_element_stream_.IsOpen()) {
-        int flags = base::PLATFORM_FILE_OPEN | 
+        int flags = base::PLATFORM_FILE_OPEN |
                     base::PLATFORM_FILE_READ;
         int rv = next_element_stream_.Open(element.file_path(), flags);
         // If the file does not exist, that's technically okay.. we'll just
diff --git a/net/base/upload_data_stream.h b/net/base/upload_data_stream.h
index 7c2bb6d..c7dc2f1 100644
--- a/net/base/upload_data_stream.h
+++ b/net/base/upload_data_stream.h
@@ -12,7 +12,7 @@ namespace net {
 
 class UploadDataStream {
  public:
-  UploadDataStream(const UploadData* data);
+  explicit UploadDataStream(const UploadData* data);
   ~UploadDataStream();
 
   // Returns the stream's buffer and buffer length.
@@ -24,9 +24,6 @@ class UploadDataStream {
   // the upload data to be consumed.
   void DidConsume(size_t num_bytes);
 
-  // Call to reset the stream position to the beginning.
-  void Reset();
-
   // Returns the total size of the data stream and the current position.
   uint64 size() const { return total_size_; }
   uint64 position() const { return current_position_; }
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index fa4bf73..8e947f57 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -964,9 +964,13 @@ bool HttpNetworkTransaction::ShouldResendRequest() {
   }
   connection_.set_socket(NULL);
   connection_.Reset();
+  // There are two reasons we need to clear request_headers_.  1) It contains
+  // the real request headers, but we may need to resend the CONNECT request
+  // first to recreate the SSL tunnel.  2) An empty request_headers_ causes
+  // BuildRequestHeaders to be called, which rewinds request_body_stream_ to
+  // the beginning of request_->upload_data.
+  request_headers_.clear();
   request_headers_bytes_sent_ = 0;
-  if (request_body_stream_.get())
-    request_body_stream_->Reset();
   next_state_ = STATE_INIT_CONNECTION;  // Resend the request.
   return true;
 }
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 389fdb6..dc53f4f 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -32,7 +32,7 @@ struct MockRead {
       data_len(0) { }
 
   // Asynchronous read success (inferred data length).
-  MockRead(const char* data) : async(true),  result(0), data(data),
+  explicit MockRead(const char* data) : async(true),  result(0), data(data),
       data_len(strlen(data)) { }
 
   // Read success (inferred data length).
@@ -76,7 +76,7 @@ int mock_sockets_index;
 
 class MockTCPClientSocket : public net::ClientSocket {
  public:
-  MockTCPClientSocket(const net::AddressList& addresses)
+  explicit MockTCPClientSocket(const net::AddressList& addresses)
       : data_(mock_sockets[mock_sockets_index++]),
         ALLOW_THIS_IN_INITIALIZER_LIST(method_factory_(this)),
         callback_(NULL),
@@ -135,6 +135,8 @@ class MockTCPClientSocket : public net::ClientSocket {
   }
   virtual int Write(const char* buf, int buf_len,
                     net::CompletionCallback* callback) {
+    DCHECK(buf);
+    DCHECK(buf_len > 0);
     DCHECK(!callback_);
     // Not using mock writes; succeed synchronously.
     if (!data_->writes)
@@ -142,7 +144,7 @@ class MockTCPClientSocket : public net::ClientSocket {
 
     // Check that what we are writing matches the expectation.
     // Then give the mocked return value.
-    MockWrite& w = data_->writes[write_index_];
+    MockWrite& w = data_->writes[write_index_++];
     int result = w.result;
     if (w.data) {
       std::string expected_data(w.data, w.data_len);
@@ -279,7 +281,7 @@ void FillLargeHeadersString(std::string* str, int size) {
   const int sizeof_data = num_rows * sizeof_row;
   DCHECK(sizeof_data >= size);
   str->reserve(sizeof_data);
-  
+
   for (int i = 0; i < num_rows; ++i)
     str->append(row, sizeof_row);
 }
@@ -926,3 +928,92 @@ TEST_F(HttpNetworkTransactionTest, DontRecycleTCPSocketForSSLTunnel) {
   // Make sure that the socket didn't get recycled after calling the destructor.
   EXPECT_EQ(0, session->connection_pool()->idle_socket_count());
 }
+
+TEST_F(HttpNetworkTransactionTest, ResendRequestOnWriteBodyError) {
+  net::HttpRequestInfo request[2];
+  // Transaction 1: a GET request that succeeds.  The socket is recycled
+  // after use.
+  request[0].method = "GET";
+  request[0].url = GURL("http://www.google.com/");
+  request[0].load_flags = 0;
+  // Transaction 2: a POST request.  Reuses the socket kept alive from
+  // transaction 1.  The first attempts fails when writing the POST data.
+  // This causes the transaction to retry with a new socket.  The second
+  // attempt succeeds.
+  request[1].method = "POST";
+  request[1].url = GURL("http://www.google.com/login.cgi");
+  request[1].upload_data = new net::UploadData;
+  request[1].upload_data->AppendBytes("foo", 3);
+  request[1].load_flags = 0;
+
+  scoped_refptr<net::HttpNetworkSession> session = CreateSession();
+
+  // The first socket is used for transaction 1 and the first attempt of
+  // transaction 2.
+
+  // The response of transaction 1.
+  MockRead data_reads1[] = {
+    MockRead("HTTP/1.1 200 OK\r\nContent-Length: 11\r\n\r\n"),
+    MockRead("hello world"),
+    MockRead(false, net::OK),
+  };
+  // The mock write results of transaction 1 and the first attempt of
+  // transaction 2.
+  MockWrite data_writes1[] = {
+    MockWrite(false, 64),  // GET
+    MockWrite(false, 93),  // POST
+    MockWrite(false, net::ERR_CONNECTION_ABORTED),  // POST data
+  };
+  MockSocket data1;
+  data1.reads = data_reads1;
+  data1.writes = data_writes1;
+
+  // The second socket is used for the second attempt of transaction 2.
+
+  // The response of transaction 2.
+  MockRead data_reads2[] = {
+    MockRead("HTTP/1.1 200 OK\r\nContent-Length: 7\r\n\r\n"),
+    MockRead("welcome"),
+    MockRead(false, net::OK),
+  };
+  // The mock write results of the second attempt of transaction 2.
+  MockWrite data_writes2[] = {
+    MockWrite(false, 93),  // POST
+    MockWrite(false, 3),  // POST data
+  };
+  MockSocket data2;
+  data2.reads = data_reads2;
+  data2.writes = data_writes2;
+
+  mock_sockets[0] = &data1;
+  mock_sockets[1] = &data2;
+  mock_sockets[2] = NULL;
+
+  const char* kExpectedResponseData[] = {
+    "hello world", "welcome"
+  };
+
+  for (int i = 0; i < 2; ++i) {
+    scoped_ptr<net::HttpTransaction> trans(
+        new net::HttpNetworkTransaction(session, &mock_socket_factory));
+
+    TestCompletionCallback callback;
+
+    int rv = trans->Start(&request[i], &callback);
+    EXPECT_EQ(net::ERR_IO_PENDING, rv);
+
+    rv = callback.WaitForResult();
+    EXPECT_EQ(net::OK, rv);
+
+    const net::HttpResponseInfo* response = trans->GetResponseInfo();
+    EXPECT_TRUE(response != NULL);
+
+    EXPECT_TRUE(response->headers != NULL);
+    EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
+
+    std::string response_data;
+    rv = ReadTransaction(trans.get(), &response_data);
+    EXPECT_EQ(net::OK, rv);
+    EXPECT_EQ(kExpectedResponseData[i], response_data);
+  }
+}