HttpStream::SendRequest

Modify HttpStream::SendRequest to take the HttpRequestHeaders instead of a string so that HttpNetworkTransaction can handle both HTTP and SPDY requests the same way.

BUG=
TEST=

Review URL: http://codereview.chromium.org/4061005

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

11 files changed, 61 insertions, 49 deletions

diff --git a/net/http/http_basic_stream.cc b/net/http/http_basic_stream.cc
index ecd692b..433b08e 100644
--- a/net/http/http_basic_stream.cc
+++ b/net/http/http_basic_stream.cc
@@ -4,33 +4,48 @@
 
 #include "net/http/http_basic_stream.h"
 
+#include "base/stringprintf.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
+#include "net/http/http_request_headers.h"
+#include "net/http/http_request_info.h"
 #include "net/http/http_stream_parser.h"
+#include "net/http/http_util.h"
 #include "net/socket/client_socket_handle.h"
 
 namespace net {
 
-HttpBasicStream::HttpBasicStream(ClientSocketHandle* connection)
+HttpBasicStream::HttpBasicStream(ClientSocketHandle* connection,
+                                 bool using_proxy)
     : read_buf_(new GrowableIOBuffer()),
-      connection_(connection) {
+      connection_(connection),
+      using_proxy_(using_proxy),
+      request_info_(NULL) {
 }
 
 int HttpBasicStream::InitializeStream(const HttpRequestInfo* request_info,
                                       const BoundNetLog& net_log,
                                       CompletionCallback* callback) {
+  request_info_ = request_info;
   parser_.reset(new HttpStreamParser(connection_.get(), request_info,
                                      read_buf_, net_log));
   return OK;
 }
 
 
-int HttpBasicStream::SendRequest(const std::string& headers,
+int HttpBasicStream::SendRequest(const HttpRequestHeaders& headers,
                                  UploadDataStream* request_body,
                                  HttpResponseInfo* response,
                                  CompletionCallback* callback) {
   DCHECK(parser_.get());
-  return parser_->SendRequest(headers, request_body, response, callback);
+  const std::string path = using_proxy_ ?
+                           HttpUtil::SpecForRequest(request_info_->url) :
+                           HttpUtil::PathForRequest(request_info_->url);
+  request_line_ = base::StringPrintf("%s %s HTTP/1.1\r\n",
+                                     request_info_->method.c_str(),
+                                     path.c_str()) + headers.ToString();
+
+  return parser_->SendRequest(request_line_, request_body, response, callback);
 }
 
 HttpBasicStream::~HttpBasicStream() {}
@@ -60,7 +75,7 @@ HttpStream* HttpBasicStream::RenewStreamForAuth() {
   DCHECK(IsResponseBodyComplete());
   DCHECK(!IsMoreDataBuffered());
   parser_.reset();
-  return new HttpBasicStream(connection_.release());
+  return new HttpBasicStream(connection_.release(), using_proxy_);
 }
 
 bool HttpBasicStream::IsResponseBodyComplete() const {
diff --git a/net/http/http_basic_stream.h b/net/http/http_basic_stream.h
index ffab967..83136c0 100644
--- a/net/http/http_basic_stream.h
+++ b/net/http/http_basic_stream.h
@@ -23,13 +23,14 @@ class ClientSocketHandle;
 class GrowableIOBuffer;
 class HttpResponseInfo;
 struct HttpRequestInfo;
+class HttpRequestHeaders;
 class HttpStreamParser;
 class IOBuffer;
 class UploadDataStream;
 
 class HttpBasicStream : public HttpStream {
  public:
-  explicit HttpBasicStream(ClientSocketHandle* connection);
+  HttpBasicStream(ClientSocketHandle* connection, bool using_proxy);
   virtual ~HttpBasicStream();
 
   // HttpStream methods:
@@ -37,7 +38,7 @@ class HttpBasicStream : public HttpStream {
                                const BoundNetLog& net_log,
                                CompletionCallback* callback);
 
-  virtual int SendRequest(const std::string& headers,
+  virtual int SendRequest(const HttpRequestHeaders& headers,
                           UploadDataStream* request_body,
                           HttpResponseInfo* response,
                           CompletionCallback* callback);
@@ -76,6 +77,12 @@ class HttpBasicStream : public HttpStream {
 
   scoped_ptr<ClientSocketHandle> connection_;
 
+  bool using_proxy_;
+
+  std::string request_line_;
+
+  const HttpRequestInfo* request_info_;
+
   DISALLOW_COPY_AND_ASSIGN(HttpBasicStream);
 };
 
diff --git a/net/http/http_network_transaction.cc b/net/http/http_network_transaction.cc
index e0b9c39..f563835 100644
--- a/net/http/http_network_transaction.cc
+++ b/net/http/http_network_transaction.cc
@@ -89,6 +89,7 @@ HttpNetworkTransaction::HttpNetworkTransaction(HttpNetworkSession* session)
       request_(NULL),
       headers_valid_(false),
       logged_response_time_(false),
+      request_headers_(),
       read_buf_len_(0),
       next_state_(STATE_NONE),
       establishing_tunnel_(false) {
@@ -624,38 +625,22 @@ int HttpNetworkTransaction::DoSendRequest() {
 
   // This is constructed lazily (instead of within our Start method), so that
   // we have proxy info available.
-  if (request_headers_.empty() && !response_.was_fetched_via_spdy) {
+  if (request_headers_.IsEmpty()) {
     bool using_proxy = (proxy_info_.is_http()|| proxy_info_.is_https()) &&
                         !is_https_request();
-    const std::string path = using_proxy ?
-                             HttpUtil::SpecForRequest(request_->url) :
-                             HttpUtil::PathForRequest(request_->url);
-    std::string request_line = base::StringPrintf(
-        "%s %s HTTP/1.1\r\n", request_->method.c_str(), path.c_str());
-
-    HttpRequestHeaders request_headers;
     HttpUtil::BuildRequestHeaders(request_, request_body, auth_controllers_,
                                   ShouldApplyServerAuth(),
                                   ShouldApplyProxyAuth(), using_proxy,
-                                  &request_headers);
+                                  &request_headers_);
 
     if (session_->network_delegate())
-      session_->network_delegate()->OnSendHttpRequest(&request_headers);
-
-    if (net_log_.IsLoggingAllEvents()) {
-      net_log_.AddEvent(
-          NetLog::TYPE_HTTP_TRANSACTION_SEND_REQUEST_HEADERS,
-          new NetLogHttpRequestParameter(request_line, request_headers));
-    }
-
-    request_headers_ = request_line + request_headers.ToString();
-  } else {
-    if (net_log_.IsLoggingAllEvents()) {
-      net_log_.AddEvent(
-          NetLog::TYPE_HTTP_TRANSACTION_SEND_REQUEST_HEADERS,
-          new NetLogHttpRequestParameter(request_->url.spec(),
-                                         request_->extra_headers));
-    }
+      session_->network_delegate()->OnSendHttpRequest(&request_headers_);
+  }
+  if (net_log_.IsLoggingAllEvents()) {
+    net_log_.AddEvent(
+        NetLog::TYPE_HTTP_TRANSACTION_SEND_REQUEST_HEADERS,
+        new NetLogHttpRequestParameter(request_->url.spec(),
+                                       request_->extra_headers));
   }
 
   headers_valid_ = false;
@@ -1050,7 +1035,7 @@ void HttpNetworkTransaction::ResetStateForAuthRestart() {
   read_buf_ = NULL;
   read_buf_len_ = 0;
   headers_valid_ = false;
-  request_headers_.clear();
+  request_headers_.Clear();
   response_ = HttpResponseInfo();
   establishing_tunnel_ = false;
 }
@@ -1080,7 +1065,7 @@ void HttpNetworkTransaction::ResetConnectionAndRequestForResend() {
   // We need to clear request_headers_ because it contains the real request
   // headers, but we may need to resend the CONNECT request first to recreate
   // the SSL tunnel.
-  request_headers_.clear();
+  request_headers_.Clear();
   next_state_ = STATE_CREATE_STREAM;  // Resend the request.
 }
 
diff --git a/net/http/http_network_transaction.h b/net/http/http_network_transaction.h
index ce0c5bd..255fcdf 100644
--- a/net/http/http_network_transaction.h
+++ b/net/http/http_network_transaction.h
@@ -17,6 +17,7 @@
 #include "net/base/request_priority.h"
 #include "net/base/ssl_config_service.h"
 #include "net/http/http_auth.h"
+#include "net/http/http_request_headers.h"
 #include "net/http/http_response_info.h"
 #include "net/http/http_transaction.h"
 #include "net/http/stream_factory.h"
@@ -222,7 +223,7 @@ class HttpNetworkTransaction : public HttpTransaction,
 
   SSLConfig ssl_config_;
 
-  std::string request_headers_;
+  HttpRequestHeaders request_headers_;
 
   // The size in bytes of the buffer we use to drain the response body that
   // we want to throw away.  The response body is typically a small error
diff --git a/net/http/http_network_transaction_unittest.cc b/net/http/http_network_transaction_unittest.cc
index 2066264..b609511 100644
--- a/net/http/http_network_transaction_unittest.cc
+++ b/net/http/http_network_transaction_unittest.cc
@@ -3991,7 +3991,7 @@ TEST_F(HttpNetworkTransactionTest, ResetStateForRestart) {
   // Setup some state (which we expect ResetStateForRestart() will clear).
   trans->read_buf_ = new IOBuffer(15);
   trans->read_buf_len_ = 15;
-  trans->request_headers_ = "Authorization: NTLM";
+  trans->request_headers_.SetHeader("Authorization", "NTLM");
 
   // Setup state in response_
   HttpResponseInfo* response = &trans->response_;
@@ -4016,7 +4016,7 @@ TEST_F(HttpNetworkTransactionTest, ResetStateForRestart) {
   // Verify that the state that needed to be reset, has been reset.
   EXPECT_TRUE(trans->read_buf_.get() == NULL);
   EXPECT_EQ(0, trans->read_buf_len_);
-  EXPECT_EQ(0U, trans->request_headers_.size());
+  EXPECT_TRUE(trans->request_headers_.IsEmpty());
   EXPECT_TRUE(response->auth_challenge.get() == NULL);
   EXPECT_TRUE(response->headers.get() == NULL);
   EXPECT_FALSE(response->was_cached);
diff --git a/net/http/http_response_body_drainer_unittest.cc b/net/http/http_response_body_drainer_unittest.cc
index d57952d..d8c9bb7 100644
--- a/net/http/http_response_body_drainer_unittest.cc
+++ b/net/http/http_response_body_drainer_unittest.cc
@@ -78,7 +78,7 @@ class MockHttpStream : public HttpStream {
                                CompletionCallback* callback) {
     return ERR_UNEXPECTED;
   }
-  virtual int SendRequest(const std::string& request_headers,
+  virtual int SendRequest(const HttpRequestHeaders& request_headers,
                           UploadDataStream* request_body,
                           HttpResponseInfo* response,
                           CompletionCallback* callback) {
diff --git a/net/http/http_stream.h b/net/http/http_stream.h
index 5aa84e0..16f84cc 100644
--- a/net/http/http_stream.h
+++ b/net/http/http_stream.h
@@ -22,12 +22,13 @@
 namespace net {
 
 class BoundNetLog;
+class HttpRequestHeaders;
+struct HttpRequestInfo;
 class HttpResponseInfo;
 class IOBuffer;
 class SSLCertRequestInfo;
 class SSLInfo;
 class UploadDataStream;
-struct HttpRequestInfo;
 
 class HttpStream {
  public:
@@ -45,7 +46,7 @@ class HttpStream {
   // synchronously, in which case the result will be passed to the callback
   // when available. Returns OK on success. The HttpStream takes ownership
   // of the request_body.
-  virtual int SendRequest(const std::string& request_headers,
+  virtual int SendRequest(const HttpRequestHeaders& request_headers,
                           UploadDataStream* request_body,
                           HttpResponseInfo* response,
                           CompletionCallback* callback) = 0;
diff --git a/net/http/http_stream_request.cc b/net/http/http_stream_request.cc
index a7d06559..3517b4e 100644
--- a/net/http/http_stream_request.cc
+++ b/net/http/http_stream_request.cc
@@ -726,8 +726,12 @@ int HttpStreamRequest::DoCreateStream() {
   if (connection_->socket() && !connection_->is_reused())
     SetSocketMotivation();
 
+  const ProxyServer& proxy_server = proxy_info()->proxy_server();
+
   if (!using_spdy_) {
-    stream_.reset(new HttpBasicStream(connection_.release()));
+    bool using_proxy = (proxy_info()->is_http() || proxy_info()->is_https()) &&
+        request_info().url.SchemeIs("http");
+    stream_.reset(new HttpBasicStream(connection_.release(), using_proxy));
     return OK;
   }
 
@@ -737,7 +741,6 @@ int HttpStreamRequest::DoCreateStream() {
   SpdySessionPool* spdy_pool = session_->spdy_session_pool();
   scoped_refptr<SpdySession> spdy_session;
 
-  const ProxyServer& proxy_server = proxy_info()->proxy_server();
   HostPortProxyPair pair(endpoint_, proxy_server);
   if (spdy_pool->HasSession(pair)) {
     // We have a SPDY session to the origin server.  This might be a direct
diff --git a/net/spdy/spdy_http_stream.cc b/net/spdy/spdy_http_stream.cc
index 6fede6c..737eba5 100644
--- a/net/spdy/spdy_http_stream.cc
+++ b/net/spdy/spdy_http_stream.cc
@@ -12,6 +12,7 @@
 #include "base/message_loop.h"
 #include "net/base/load_flags.h"
 #include "net/base/net_util.h"
+#include "net/http/http_request_headers.h"
 #include "net/http/http_request_info.h"
 #include "net/http/http_response_info.h"
 #include "net/http/http_util.h"
@@ -139,7 +140,7 @@ void SpdyHttpStream::Close(bool not_reusable) {
   Cancel();
 }
 
-int SpdyHttpStream::SendRequest(const std::string& /*headers_string*/,
+int SpdyHttpStream::SendRequest(const HttpRequestHeaders& request_headers,
                                 UploadDataStream* request_body,
                                 HttpResponseInfo* response,
                                 CompletionCallback* callback) {
@@ -148,9 +149,6 @@ int SpdyHttpStream::SendRequest(const std::string& /*headers_string*/,
 
   stream_->SetDelegate(this);
 
-  HttpRequestHeaders request_headers;
-  HttpUtil::BuildRequestHeaders(request_info_, request_body, NULL, false, false,
-                                !direct_, &request_headers);
   linked_ptr<spdy::SpdyHeaderBlock> headers(new spdy::SpdyHeaderBlock);
   CreateSpdyHeadersFromHttpRequest(*request_info_, request_headers,
                                    headers.get(), direct_);
diff --git a/net/spdy/spdy_http_stream.h b/net/spdy/spdy_http_stream.h
index 372db57..a878ff9 100644
--- a/net/spdy/spdy_http_stream.h
+++ b/net/spdy/spdy_http_stream.h
@@ -49,7 +49,7 @@ class SpdyHttpStream : public SpdyStream::Delegate, public HttpStream {
   // |callback| is used when this completes asynchronously.
   // SpdyHttpStream takes ownership of |upload_data|. |upload_data| may be NULL.
   // The actual SYN_STREAM packet will be sent if the stream is non-pushed.
-  virtual int SendRequest(const std::string& headers,
+  virtual int SendRequest(const HttpRequestHeaders& headers,
                           UploadDataStream* request_body,
                           HttpResponseInfo* response,
                           CompletionCallback* callback);
diff --git a/net/spdy/spdy_http_stream_unittest.cc b/net/spdy/spdy_http_stream_unittest.cc
index ee1a34e..df979ec 100644
--- a/net/spdy/spdy_http_stream_unittest.cc
+++ b/net/spdy/spdy_http_stream_unittest.cc
@@ -74,6 +74,7 @@ TEST_F(SpdyHttpStreamTest, SendRequest) {
   request.url = GURL("http://www.google.com/");
   TestCompletionCallback callback;
   HttpResponseInfo response;
+  HttpRequestHeaders headers;
   BoundNetLog net_log;
   scoped_ptr<SpdyHttpStream> http_stream(
       new SpdyHttpStream(session_.get(), true));
@@ -82,7 +83,7 @@ TEST_F(SpdyHttpStreamTest, SendRequest) {
       http_stream->InitializeStream(&request, net_log, NULL));
 
   EXPECT_EQ(ERR_IO_PENDING,
-            http_stream->SendRequest("", NULL, &response, &callback));
+            http_stream->SendRequest(headers, NULL, &response, &callback));
   EXPECT_TRUE(http_session_->spdy_session_pool()->HasSession(pair));
 
   // This triggers the MockWrite and read 2
@@ -125,6 +126,7 @@ TEST_F(SpdyHttpStreamTest, SpdyURLTest) {
   request.url = GURL(full_url);
   TestCompletionCallback callback;
   HttpResponseInfo response;
+  HttpRequestHeaders headers;
   BoundNetLog net_log;
   scoped_ptr<SpdyHttpStream> http_stream(new SpdyHttpStream(session_, true));
   ASSERT_EQ(
@@ -132,7 +134,7 @@ TEST_F(SpdyHttpStreamTest, SpdyURLTest) {
       http_stream->InitializeStream(&request, net_log, NULL));
 
   EXPECT_EQ(ERR_IO_PENDING,
-            http_stream->SendRequest("", NULL, &response, &callback));
+            http_stream->SendRequest(headers, NULL, &response, &callback));
 
   spdy::SpdyHeaderBlock* spdy_header =
     http_stream->stream()->spdy_headers().get();