I've refactored HttpStream, SpdyHttpStream and HttpBasicStream so that

SpdyHttpStream now implements (a slightly wider) HttpStream interface.

BUG=50268
TEST=none

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


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

net/http/http_basic_stream.cc

@@ -6,19 +6,27 @@
 
 namespace net {
 
-HttpBasicStream::HttpBasicStream(ClientSocketHandle* handle,
-                                 const BoundNetLog& net_log)
+HttpBasicStream::HttpBasicStream(ClientSocketHandle* connection)
     : read_buf_(new GrowableIOBuffer()),
-      parser_(new HttpStreamParser(handle, read_buf_, net_log)) {
+      connection_(connection) {
 }
 
-int HttpBasicStream::SendRequest(const HttpRequestInfo* request,
-                                 const std::string& headers,
+int HttpBasicStream::InitializeStream(const HttpRequestInfo* request_info,
+                                      const BoundNetLog& net_log,
+                                      CompletionCallback* callback) {
+  parser_.reset(new HttpStreamParser(connection_, request_info,
+                                     read_buf_, net_log));
+  connection_ = NULL;
+  return OK;
+}
+
+
+int HttpBasicStream::SendRequest(const std::string& headers,
                                  UploadDataStream* request_body,
                                  HttpResponseInfo* response,
                                  CompletionCallback* callback) {
-  return parser_->SendRequest(
-      request, headers, request_body, response, callback);
+  DCHECK(parser_.get());
+  return parser_->SendRequest(headers, request_body, response, callback);
 }
 
 HttpBasicStream::~HttpBasicStream() {}

net/http/http_basic_stream.h

@@ -27,12 +27,15 @@ class UploadDataStream;
 
 class HttpBasicStream : public HttpStream {
  public:
-  HttpBasicStream(ClientSocketHandle* handle, const BoundNetLog& net_log);
+  explicit HttpBasicStream(ClientSocketHandle* connection);
   virtual ~HttpBasicStream();
 
   // HttpStream methods:
-  virtual int SendRequest(const HttpRequestInfo* request,
-                          const std::string& headers,
+  virtual int InitializeStream(const HttpRequestInfo* request_info,
+                               const BoundNetLog& net_log,
+                               CompletionCallback* callback);
+
+  virtual int SendRequest(const std::string& headers,
                           UploadDataStream* request_body,
                           HttpResponseInfo* response,
                           CompletionCallback* callback);
@@ -57,6 +60,8 @@ class HttpBasicStream : public HttpStream {
 
   scoped_ptr<HttpStreamParser> parser_;
 
+  ClientSocketHandle* connection_;
+
   DISALLOW_COPY_AND_ASSIGN(HttpBasicStream);
 };
 

net/http/http_network_transaction.cc

@@ -1052,10 +1052,10 @@ int HttpNetworkTransaction::DoSendRequest() {
   }
 
   headers_valid_ = false;
-  http_stream_.reset(new HttpBasicStream(connection_.get(), net_log_));
-
-  return http_stream_->SendRequest(request_, request_headers_,
-                                   request_body, &response_, &io_callback_);
+  http_stream_.reset(new HttpBasicStream(connection_.get()));
+  http_stream_->InitializeStream(request_, net_log_, NULL);
+  return http_stream_->SendRequest(request_headers_, request_body, &response_,
+                                   &io_callback_);
 }
 
 int HttpNetworkTransaction::DoSendRequestComplete(int result) {
@@ -1297,9 +1297,8 @@ int HttpNetworkTransaction::DoSpdyGetStream() {
 
   headers_valid_ = false;
 
-  spdy_http_stream_.reset(new SpdyHttpStream());
-  return spdy_http_stream_->InitializeStream(spdy_session, *request_,
-                                             net_log_, &io_callback_);
+  spdy_http_stream_.reset(new SpdyHttpStream(spdy_session));
+  return spdy_http_stream_->InitializeStream(request_, net_log_, &io_callback_);
 }
 
 int HttpNetworkTransaction::DoSpdyGetStreamComplete(int result) {
@@ -1321,9 +1320,8 @@ int HttpNetworkTransaction::DoSpdySendRequest() {
     if (!upload_data_stream)
       return error_code;
   }
-  spdy_http_stream_->InitializeRequest(base::Time::Now(), upload_data_stream);
-
-  return spdy_http_stream_->SendRequest(&response_, &io_callback_);
+  return spdy_http_stream_->SendRequest(request_headers_, upload_data_stream,
+                                        &response_, &io_callback_);
 }
 
 int HttpNetworkTransaction::DoSpdySendRequestComplete(int result) {

net/http/http_proxy_client_socket.cc

@@ -319,9 +319,9 @@ int HttpProxyClientSocket::DoSendRequest() {
     request_headers_ = request_line + request_headers.ToString();
   }
 
-  http_stream_.reset(new HttpBasicStream(transport_.get(), net_log_));
-
-  return http_stream_->SendRequest(&request_, request_headers_, NULL,
+  http_stream_.reset(new HttpBasicStream(transport_.get()));
+  http_stream_->InitializeStream(&request_, net_log_, NULL);
+  return http_stream_->SendRequest(request_headers_, NULL,
                                    &response_, &io_callback_);
 }
 

net/http/http_stream.h

@@ -22,19 +22,25 @@ struct HttpRequestInfo;
 class HttpResponseInfo;
 class IOBuffer;
 class UploadDataStream;
+class BoundNetLog;
 
 class HttpStream {
  public:
   HttpStream() {}
   virtual ~HttpStream() {}
 
+  // Initialize stream.  Must be called before calling SendRequest().
+  // Returns a net error code, possibly ERR_IO_PENDING.
+  virtual int InitializeStream(const HttpRequestInfo* request_info,
+                               const BoundNetLog& net_log,
+                               CompletionCallback* callback) = 0;
+
   // Writes the headers and uploads body data to the underlying socket.
   // ERR_IO_PENDING is returned if the operation could not be completed
   // 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 HttpRequestInfo* request,
-                          const std::string& request_headers,
+  virtual int SendRequest(const std::string& request_headers,
                           UploadDataStream* request_body,
                           HttpResponseInfo* response,
                           CompletionCallback* callback) = 0;
@@ -50,7 +56,7 @@ class HttpStream {
   virtual int ReadResponseHeaders(CompletionCallback* callback) = 0;
 
   // Provides access to HttpResponseInfo (owned by HttpStream).
-  virtual HttpResponseInfo* GetResponseInfo() const = 0;
+  virtual const HttpResponseInfo* GetResponseInfo() const = 0;
 
   // Reads response body data, up to |buf_len| bytes. |buf_len| should be a
   // reasonable size (<2MB). The number of bytes read is returned, or an

net/http/http_stream_parser.cc

@@ -14,10 +14,11 @@
 namespace net {
 
 HttpStreamParser::HttpStreamParser(ClientSocketHandle* connection,
+                                   const HttpRequestInfo* request,
                                    GrowableIOBuffer* read_buffer,
                                    const BoundNetLog& net_log)
     : io_state_(STATE_NONE),
-      request_(NULL),
+      request_(request),
       request_headers_(NULL),
       request_body_(NULL),
       read_buf_(read_buffer),
@@ -38,8 +39,7 @@ HttpStreamParser::HttpStreamParser(ClientSocketHandle* connection,
 
 HttpStreamParser::~HttpStreamParser() {}
 
-int HttpStreamParser::SendRequest(const HttpRequestInfo* request,
-                                  const std::string& headers,
+int HttpStreamParser::SendRequest(const std::string& headers,
                                   UploadDataStream* request_body,
                                   HttpResponseInfo* response,
                                   CompletionCallback* callback) {
@@ -48,7 +48,6 @@ int HttpStreamParser::SendRequest(const HttpRequestInfo* request,
   DCHECK(callback);
   DCHECK(response);
 
-  request_ = request;
   response_ = response;
   scoped_refptr<StringIOBuffer> headers_io_buf = new StringIOBuffer(headers);
   request_headers_ = new DrainableIOBuffer(headers_io_buf,

net/http/http_stream_parser.h

@@ -29,15 +29,15 @@ class HttpStreamParser {
   // buffer's offset will be set to the first free byte. |read_buffer| may
   // have its capacity changed.
   HttpStreamParser(ClientSocketHandle* connection,
+                   const HttpRequestInfo* request,
                    GrowableIOBuffer* read_buffer,
                    const BoundNetLog& net_log);
   ~HttpStreamParser();
 
   // These functions implement the interface described in HttpStream with
   // some additional functionality
-  int SendRequest(const HttpRequestInfo* request, const std::string& headers,
-                  UploadDataStream* request_body, HttpResponseInfo* response,
-                  CompletionCallback* callback);
+  int SendRequest(const std::string& headers, UploadDataStream* request_body,
+                  HttpResponseInfo* response, CompletionCallback* callback);
 
   int ReadResponseHeaders(CompletionCallback* callback);
 

net/spdy/spdy_http_stream.cc

@@ -129,10 +129,10 @@ void CreateSpdyHeadersFromHttpRequest(
 
 namespace net {
 
-SpdyHttpStream::SpdyHttpStream()
+SpdyHttpStream::SpdyHttpStream(SpdySession* spdy_session)
     : ALLOW_THIS_IN_INITIALIZER_LIST(read_callback_factory_(this)),
       stream_(NULL),
-      spdy_session_(NULL),
+      spdy_session_(spdy_session),
       response_info_(NULL),
       download_finished_(false),
       user_callback_(NULL),
@@ -145,15 +145,12 @@ SpdyHttpStream::~SpdyHttpStream() {
     stream_->DetachDelegate();
 }
 
-int SpdyHttpStream::InitializeStream(
-    SpdySession* spdy_session,
-    const HttpRequestInfo& request_info,
-    const BoundNetLog& stream_net_log,
-    CompletionCallback* callback) {
-  spdy_session_ = spdy_session;
+int SpdyHttpStream::InitializeStream(const HttpRequestInfo* request_info,
+                                     const BoundNetLog& stream_net_log,
+                                     CompletionCallback* callback) {
   request_info_ = request_info;
-  if (request_info_.method == "GET") {
-    int error = spdy_session_->GetPushStream(request_info.url, &stream_,
+  if (request_info_->method == "GET") {
+    int error = spdy_session_->GetPushStream(request_info_->url, &stream_,
                                              stream_net_log);
     if (error != OK)
       return error;
@@ -162,36 +159,11 @@ int SpdyHttpStream::InitializeStream(
   if (stream_.get())
     return OK;
   else
-    return spdy_session_->CreateStream(request_info_.url,
-                                       request_info_.priority, &stream_,
+    return spdy_session_->CreateStream(request_info_->url,
+                                       request_info_->priority, &stream_,
                                        stream_net_log, callback);
 }
 
-void SpdyHttpStream::InitializeRequest(
-    base::Time request_time,
-    UploadDataStream* upload_data) {
-  CHECK(stream_.get());
-  stream_->SetDelegate(this);
-  linked_ptr<spdy::SpdyHeaderBlock> headers(new spdy::SpdyHeaderBlock);
-  CreateSpdyHeadersFromHttpRequest(request_info_, headers.get());
-  stream_->set_spdy_headers(headers);
-
-  stream_->SetRequestTime(request_time);
-  // This should only get called in the case of a request occuring
-  // during server push that has already begun but hasn't finished,
-  // so we set the response's request time to be the actual one
-  if (response_info_)
-    response_info_->request_time = request_time;
-
-  CHECK(!request_body_stream_.get());
-  if (upload_data) {
-    if (upload_data->size())
-      request_body_stream_.reset(upload_data);
-    else
-      delete upload_data;
-  }
-}
-
 const HttpResponseInfo* SpdyHttpStream::GetResponseInfo() const {
   return response_info_;
 }
@@ -262,8 +234,33 @@ int SpdyHttpStream::ReadResponseBody(
   return ERR_IO_PENDING;
 }
 
-int SpdyHttpStream::SendRequest(HttpResponseInfo* response,
+int SpdyHttpStream::SendRequest(const std::string& /*headers_string*/,
+                                UploadDataStream* request_body,
+                                HttpResponseInfo* response,
                                 CompletionCallback* callback) {
+  base::Time request_time = base::Time::Now();
+  CHECK(stream_.get());
+
+  stream_->SetDelegate(this);
+  linked_ptr<spdy::SpdyHeaderBlock> headers(new spdy::SpdyHeaderBlock);
+  CreateSpdyHeadersFromHttpRequest(*request_info_, headers.get());
+  stream_->set_spdy_headers(headers);
+
+  stream_->SetRequestTime(request_time);
+  // This should only get called in the case of a request occurring
+  // during server push that has already begun but hasn't finished,
+  // so we set the response's request time to be the actual one
+  if (response_info_)
+    response_info_->request_time = request_time;
+
+  CHECK(!request_body_stream_.get());
+  if (request_body) {
+    if (request_body->size())
+      request_body_stream_.reset(request_body);
+    else
+      delete request_body;
+  }
+
   CHECK(callback);
   CHECK(!stream_->cancelled());
   CHECK(response);
@@ -343,7 +340,7 @@ int SpdyHttpStream::OnResponseReceived(const spdy::SpdyHeaderBlock& response,
     stream_->GetSSLInfo(&response_info_->ssl_info,
                         &response_info_->was_npn_negotiated);
     response_info_->request_time = stream_->GetRequestTime();
-    response_info_->vary_data.Init(request_info_, *response_info_->headers);
+    response_info_->vary_data.Init(*request_info_, *response_info_->headers);
     // TODO(ahendrickson): This is recorded after the entire SYN_STREAM control
     // frame has been received and processed.  Move to framer?
     response_info_->response_time = response_time;

net/spdy/spdy_http_stream.h

@@ -7,6 +7,7 @@
 #pragma once
 
 #include <list>
+#include <string>
 
 #include "base/basictypes.h"
 #include "base/ref_counted.h"
@@ -14,6 +15,7 @@
 #include "net/base/completion_callback.h"
 #include "net/base/net_log.h"
 #include "net/http/http_request_info.h"
+#include "net/http/http_stream.h"
 #include "net/spdy/spdy_protocol.h"
 #include "net/spdy/spdy_stream.h"
 
@@ -26,53 +28,61 @@ class UploadData;
 class UploadDataStream;
 
 // The SpdyHttpStream is a HTTP-specific type of stream known to a SpdySession.
-class SpdyHttpStream : public SpdyStream::Delegate {
+class SpdyHttpStream : public SpdyStream::Delegate, public HttpStream {
  public:
   // SpdyHttpStream constructor
-  SpdyHttpStream();
+  explicit SpdyHttpStream(SpdySession* spdy_session);
   virtual ~SpdyHttpStream();
 
   SpdyStream* stream() { return stream_.get(); }
 
-  // Initialize stream.  Must be called before calling InitializeRequest().
-  int InitializeStream(SpdySession* spdy_session,
-                       const HttpRequestInfo& request_info,
-                       const BoundNetLog& stream_net_log,
-                       CompletionCallback* callback);
-
-  // Initialize request.  Must be called before calling SendRequest().
-  // SpdyHttpStream takes ownership of |upload_data|. |upload_data| may be NULL.
-  void InitializeRequest(base::Time request_time,
-                         UploadDataStream* upload_data);
-
-  const HttpResponseInfo* GetResponseInfo() const;
-
   // ===================================================
-  // Interface for [Http|Spdy]NetworkTransaction to use.
+  // HttpStream methods:
+
+  // Initialize stream.  Must be called before calling SendRequest().
+  virtual int InitializeStream(const HttpRequestInfo* request_info,
+                               const BoundNetLog& net_log,
+                               CompletionCallback* callback);
 
   // Sends the request.
   // |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.
-  int SendRequest(HttpResponseInfo* response,
-                  CompletionCallback* callback);
+  virtual int SendRequest(const std::string& headers,
+                          UploadDataStream* request_body,
+                          HttpResponseInfo* response,
+                          CompletionCallback* callback);
+
+  // Returns the number of bytes uploaded.
+  virtual uint64 GetUploadProgress() const;
 
   // Reads the response headers.  Returns a net error code.
-  int ReadResponseHeaders(CompletionCallback* callback);
+  virtual int ReadResponseHeaders(CompletionCallback* callback);
+
+  virtual const HttpResponseInfo* GetResponseInfo() const;
 
   // Reads the response body.  Returns a net error code or the number of bytes
   // read.
-  int ReadResponseBody(
+  virtual int ReadResponseBody(
       IOBuffer* buf, int buf_len, CompletionCallback* callback);
 
-  // Cancels any callbacks from being invoked and deletes the stream.
-  void Cancel();
+  // Indicates if the response body has been completely read.
+  virtual bool IsResponseBodyComplete() const {
+    return stream_->response_complete();
+  }
 
-  // Returns the number of bytes uploaded.
-  uint64 GetUploadProgress() const;
+  // With SPDY the end of response is always detectable.
+  virtual bool CanFindEndOfResponse() const { return true; }
+
+  // A SPDY stream never has more data after the FIN.
+  virtual bool IsMoreDataBuffered() const { return false; }
 
   // ===================================================
   // SpdyStream::Delegate.
 
+  // Cancels any callbacks from being invoked and deletes the stream.
+  void Cancel();
+
   virtual bool OnSendHeadersComplete(int status);
   virtual int OnSendBody();
   virtual bool OnSendBodyComplete(int status);
@@ -118,7 +128,7 @@ class SpdyHttpStream : public SpdyStream::Delegate {
   scoped_refptr<SpdySession> spdy_session_;
 
   // The request to send.
-  HttpRequestInfo request_info_;
+  const HttpRequestInfo* request_info_;
 
   scoped_ptr<UploadDataStream> request_body_stream_;
 

net/spdy/spdy_http_stream_unittest.cc

@@ -67,14 +67,14 @@ TEST_F(SpdyHttpStreamTest, SendRequest) {
   request.url = GURL("http://www.google.com/");
   TestCompletionCallback callback;
   HttpResponseInfo response;
-  scoped_ptr<SpdyHttpStream> http_stream(new SpdyHttpStream());
+  BoundNetLog net_log;
+  scoped_ptr<SpdyHttpStream> http_stream(new SpdyHttpStream(session_.get()));
   ASSERT_EQ(
       OK,
-      http_stream->InitializeStream(session_, request, BoundNetLog(), NULL));
-  http_stream->InitializeRequest(base::Time::Now(), NULL);
+      http_stream->InitializeStream(&request, net_log, NULL));
 
   EXPECT_EQ(ERR_IO_PENDING,
-            http_stream->SendRequest(&response, &callback));
+            http_stream->SendRequest("", NULL, &response, &callback));
   EXPECT_TRUE(http_session_->spdy_session_pool()->HasSession(host_port_pair));
 
   // This triggers the MockWrite and reads 2 & 3
@@ -93,30 +93,39 @@ TEST_F(SpdyHttpStreamTest, SpdyURLTest) {
   EnableCompression(false);
   SpdySession::SetSSLMode(false);
 
+  const char * const full_url = "http://www.google.com/foo?query=what#anchor";
+  const char * const base_url = "http://www.google.com/foo?query=what";
+  scoped_ptr<spdy::SpdyFrame> req(ConstructSpdyGet(base_url, false, 1, LOWEST));
+  MockWrite writes[] = {
+    CreateMockWrite(*req.get(), 1),
+  };
   MockRead reads[] = {
     MockRead(false, 0, 2),  // EOF
   };
 
   HostPortPair host_port_pair("www.google.com", 80);
-  EXPECT_EQ(OK, InitSession(reads, arraysize(reads), NULL, 0,
+  EXPECT_EQ(OK, InitSession(reads, arraysize(reads), writes, arraysize(writes),
       host_port_pair));
 
   HttpRequestInfo request;
   request.method = "GET";
-  request.url = GURL("http://www.google.com/foo?query=what#anchor");
+  request.url = GURL(full_url);
   TestCompletionCallback callback;
   HttpResponseInfo response;
-  scoped_ptr<SpdyHttpStream> http_stream(new SpdyHttpStream());
+  BoundNetLog net_log;
+  scoped_ptr<SpdyHttpStream> http_stream(new SpdyHttpStream(session_));
   ASSERT_EQ(
       OK,
-      http_stream->InitializeStream(session_, request, BoundNetLog(), NULL));
-  http_stream->InitializeRequest(base::Time::Now(), NULL);
+      http_stream->InitializeStream(&request, net_log, NULL));
+
+  EXPECT_EQ(ERR_IO_PENDING,
+            http_stream->SendRequest("", NULL, &response, &callback));
 
   spdy::SpdyHeaderBlock* spdy_header =
     http_stream->stream()->spdy_headers().get();
+  EXPECT_TRUE(spdy_header != NULL);
   if (spdy_header->find("url") != spdy_header->end())
-    EXPECT_EQ("http://www.google.com/foo?query=what",
-              spdy_header->find("url")->second);
+    EXPECT_EQ(base_url, spdy_header->find("url")->second);
   else
     FAIL() << "No url is set in spdy_header!";
 

net/spdy/spdy_network_transaction.cc

@@ -256,9 +256,8 @@ int SpdyNetworkTransaction::DoGetStream() {
 
   CHECK(!stream_.get());
 
-  stream_.reset(new SpdyHttpStream());
-  return stream_->InitializeStream(spdy_, *request_,
-                                   net_log_, &io_callback_);
+  stream_.reset(new SpdyHttpStream(spdy_));
+  return stream_->InitializeStream(request_, net_log_, &io_callback_);
 }
 
 int SpdyNetworkTransaction::DoGetStreamComplete(int result) {
@@ -281,10 +280,12 @@ int SpdyNetworkTransaction::DoSendRequest() {
     if (!upload_data_stream)
       return error_code;
   }
-  stream_->InitializeRequest(base::Time::Now(), upload_data_stream);
   spdy_ = NULL;
 
-  return stream_->SendRequest(&response_, &io_callback_);
+  return stream_->SendRequest("",
+                              upload_data_stream,
+                              &response_,
+                              &io_callback_);
 }
 
 int SpdyNetworkTransaction::DoSendRequestComplete(int result) {

net/spdy/spdy_test_util.cc

@@ -248,6 +248,43 @@ int ConstructSpdyHeader(const char* const extra_headers[],
   return n;
 }
 
+// Constructs a standard SPDY GET SYN packet, optionally compressed
+// for the url |url|.
+// |extra_headers| are the extra header-value pairs, which typically
+// will vary the most between calls.
+// Returns a SpdyFrame.
+spdy::SpdyFrame* ConstructSpdyGet(const char* const url,
+                                  bool compressed,
+                                  int stream_id,
+                                  RequestPriority request_priority) {
+  const SpdyHeaderInfo kSynStartHeader = {
+    spdy::SYN_STREAM,             // Kind = Syn
+    stream_id,                    // Stream ID
+    0,                            // Associated stream ID
+    request_priority,             // Priority
+    spdy::CONTROL_FLAG_FIN,       // Control Flags
+    compressed,                   // Compressed
+    spdy::INVALID,                // Status
+    NULL,                         // Data
+    0,                            // Length
+    spdy::DATA_FLAG_NONE          // Data Flags
+  };
+  const char* const headers[] = {
+    "method",
+    "GET",
+    "url",
+    url,
+    "version",
+    "HTTP/1.1"
+  };
+  return ConstructSpdyPacket(
+      kSynStartHeader,
+      NULL,
+      0,
+      headers,
+      arraysize(headers) / 2);
+}
+
 // Constructs a standard SPDY GET SYN packet, optionally compressed.
 // |extra_headers| are the extra header-value pairs, which typically
 // will vary the most between calls.

net/spdy/spdy_test_util.h

@@ -149,6 +149,16 @@ int ConstructSpdyHeader(const char* const extra_headers[],
                         int buffer_length,
                         int index);
 
+// Constructs a standard SPDY GET SYN packet, optionally compressed
+// for the url |url|.
+// |extra_headers| are the extra header-value pairs, which typically
+// will vary the most between calls.
+// Returns a SpdyFrame.
+spdy::SpdyFrame* ConstructSpdyGet(const char* const url,
+                                  bool compressed,
+                                  int stream_id,
+                                  RequestPriority request_priority);
+
 // Constructs a standard SPDY GET SYN packet, optionally compressed.
 // |extra_headers| are the extra header-value pairs, which typically
 // will vary the most between calls.