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

6 files changed, 144 insertions, 80 deletions

diff --git a/net/spdy/spdy_http_stream.cc b/net/spdy/spdy_http_stream.cc
index c813e96..76358aa 100644
--- a/net/spdy/spdy_http_stream.cc
+++ b/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;
diff --git a/net/spdy/spdy_http_stream.h b/net/spdy/spdy_http_stream.h
index ad643f2..939b929 100644
--- a/net/spdy/spdy_http_stream.h
+++ b/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_;
 
diff --git a/net/spdy/spdy_http_stream_unittest.cc b/net/spdy/spdy_http_stream_unittest.cc
index ab4c606..1a3ece6 100644
--- a/net/spdy/spdy_http_stream_unittest.cc
+++ b/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!";
 
diff --git a/net/spdy/spdy_network_transaction.cc b/net/spdy/spdy_network_transaction.cc
index e575ad0..6b3e26b 100644
--- a/net/spdy/spdy_network_transaction.cc
+++ b/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) {
diff --git a/net/spdy/spdy_test_util.cc b/net/spdy/spdy_test_util.cc
index 99af4f8..291f2cd 100644
--- a/net/spdy/spdy_test_util.cc
+++ b/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.
diff --git a/net/spdy/spdy_test_util.h b/net/spdy/spdy_test_util.h
index b8b5584..4114fe2 100644
--- a/net/spdy/spdy_test_util.h
+++ b/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.