[SPDY] Add a SpdyStreamType enum and use it for SpdyStream

net/spdy/spdy_stream.h

Index: net/spdy/spdy_stream.h
diff --git a/net/spdy/spdy_stream.h b/net/spdy/spdy_stream.h
index 883e7a3d4818044a4ed18aed4ccb7e0cba388bf0..91f24006c096661ae3676be1e287ee082cf477b5 100644
--- a/net/spdy/spdy_stream.h
+++ b/net/spdy/spdy_stream.h
@@ -21,10 +21,10 @@
 #include "net/base/net_log.h"
 #include "net/base/request_priority.h"
 #include "net/socket/ssl_client_socket.h"
+#include "net/spdy/spdy_buffer.h"
 #include "net/spdy/spdy_framer.h"
 #include "net/spdy/spdy_header_block.h"
 #include "net/spdy/spdy_protocol.h"
-#include "net/spdy/spdy_session.h"
 #include "net/ssl/server_bound_cert_service.h"
 #include "net/ssl/ssl_client_cert_type.h"
 
@@ -34,6 +34,19 @@ class AddressList;
 class IPEndPoint;
 class SSLCertRequestInfo;
 class SSLInfo;
+class SpdySession;
+
+enum SpdyStreamType {
+  // The most general type of stream; there are no restrictions on
+  // when data can be sent and received.
+  SPDY_BIDIRECTIONAL_STREAM,
+  // A stream where the client sends a request with possibly a body,
+  // and the server then sends a response with a body.
+  SPDY_REQUEST_RESPONSE_STREAM,
+  // A server-initiated stream where the server just sends a response
+  // with a body and the client does not send anything.
+  SPDY_PUSH_STREAM
+};
 
 // Returned by some SpdyStream::Delegate functions to indicate whether
 // there's more data to send.
@@ -56,32 +69,24 @@ class NET_EXPORT_PRIVATE SpdyStream {
    public:
     Delegate() {}
 
-    // Called when the request headers have been sent.  Must return
-    // whether there's body data to send.
-    //
-    // There's some redundancy in SendRequestHeaders() taking a
-    // SpdySendStatus and this function returning one, but it's
-    // necessary. Bidirectional streams always pass in
-    // MORE_DATA_TO_SEND to SendRequestHeaders() but must return
-    // NO_MORE_DATA_TO_SEND from OnSendRequestHeadersComplete(), while
-    // request/response streams always return the same value from
-    // OnSendRequestHeadersComplete() as the one they pass into
-    // SendRequestHeaders().
-    //
-    // TODO(akalin): Have a less subtle way of differentiating
-    // request/response streams from bidirectional ones.
-    virtual SpdySendStatus OnSendRequestHeadersComplete() = 0;
+    // Called when the request headers have been sent. Never called
+    // for push streams.
+    virtual void OnSendRequestHeadersComplete() = 0;
 
     // Called when the stream is ready to send body data.  The
     // delegate must call SendStreamData() on the stream, either
     // immediately or asynchronously (e.g., if the data to be send has
     // to be read asynchronously).
     //
-    // Called only when OnSendRequestHeadersComplete() or
-    // OnSendBodyComplete() returns MORE_DATA_TO_SEND.
+    // Called only for request/response streams when
+    // SendRequestHeaders() is called with MORE_DATA_TO_SEND.
+    //
+    // TODO(akalin): Unify this with OnSendRequestHeadersComplete().
     virtual void OnSendBody() = 0;
 
     // Called when body data has been sent.
+    //
+    // TODO(akalin): Unify this with OnDataSent().
     virtual void OnSendBodyComplete() = 0;
 
     // Called when the SYN_STREAM, SYN_REPLY, or HEADERS frames are received.
@@ -115,12 +120,12 @@ class NET_EXPORT_PRIVATE SpdyStream {
   };
 
   // SpdyStream constructor
-  SpdyStream(SpdySession* session,
+  SpdyStream(SpdyStreamType type,
+             SpdySession* session,
              const std::string& path,
              RequestPriority priority,
              int32 initial_send_window_size,
              int32 initial_recv_window_size,
-             bool pushed,
              const BoundNetLog& net_log);
 
   ~SpdyStream();
@@ -135,8 +140,7 @@ class NET_EXPORT_PRIVATE SpdyStream {
   // closed, and cancel it.
   void DetachDelegate();
 
-  // Is this stream a pushed stream from the server.
-  bool pushed() const { return pushed_; }
+  SpdyStreamType type() const { return type_; }
 
   SpdyStreamId stream_id() const { return stream_id_; }
   void set_stream_id(SpdyStreamId stream_id) { stream_id_ = stream_id; }
@@ -144,7 +148,6 @@ class NET_EXPORT_PRIVATE SpdyStream {
   bool response_received() const { return response_received_; }
   void set_response_received() { response_received_ = true; }
 
-  // For pushed streams, we track a path to identify them.
   const std::string& path() const { return path_; }
 
   RequestPriority priority() const { return priority_; }
@@ -241,8 +244,9 @@ class NET_EXPORT_PRIVATE SpdyStream {
   base::Time GetRequestTime() const;
   void SetRequestTime(base::Time t);
 
-  // Called by the SpdySession when a response (e.g. a SYN_STREAM or SYN_REPLY)
-  // has been received for this stream. Returns a status code.
+  // Called by the SpdySession when a response (e.g. a SYN_STREAM or
+  // SYN_REPLY) has been received for this stream. This is the entry
+  // point for a push stream. Returns a status code.
   int OnResponseReceived(const SpdyHeaderBlock& response);
 
   // Called by the SpdySession when late-bound headers are received for a
@@ -294,19 +298,26 @@ class NET_EXPORT_PRIVATE SpdyStream {
   bool body_sent() const { return io_state_ > STATE_SEND_BODY_COMPLETE; }
 
   // Interface for the delegate to use.
-  //
-  // TODO(akalin): Mandate that only one send can be in flight at one
-  // time.
 
-  // Sends the request headers.
-  // For non push stream, it will send SYN_STREAM frame.
+  // Only one send can be in flight at a time, except for push
+  // streams, which must not send anything.
+
+  // Sends the request headers. The delegate is called back via
+  // OnSendRequestHeadersComplete() when the request headers have
+  // completed sending. |send_status| must be MORE_DATA_TO_SEND for
+  // bidirectional streams; for request/response streams, it must be
+  // MORE_DATA_TO_SEND if the request has data to upload, or
+  // NO_MORE_DATA_TO_SEND if not.
   int SendRequestHeaders(scoped_ptr<SpdyHeaderBlock> headers,
                          SpdySendStatus send_status);
 
   // Sends a DATA frame. The delegate will be notified via
   // OnSendBodyComplete() (if the response hasn't been received yet)
   // or OnDataSent() (if the response has been received) when the send
-  // is complete. Only one data send can be in flight at one time.
+  // is complete. |send_status| must be MORE_DATA_TO_SEND for
+  // bidirectional streams; for request/response streams, it must be

[Ryan Hamilton, 2013/05/26 15:33:49]

What if a bidirectional stream knew (because of some higher level protocol) that
this was in-fact the last data it would send.  Could it use
NO_MORE_DATA_TO_SEND?

[akalin, 2013/05/27 08:13:35]

Yeah, I think we eventually want to allow that. For now, I'm just matching the
current behavior as closely as possible.

+  // MORE_DATA_TO_SEND if there is more data to upload, or
+  // NO_MORE_DATA_TO_SEND if not.
   void SendStreamData(IOBuffer* data, int length, SpdySendStatus send_status);
 
   // Fills SSL info in |ssl_info| and returns true when SSL is in use.
@@ -409,6 +420,8 @@ class NET_EXPORT_PRIVATE SpdyStream {
   // |pending_send_data_| and |pending_send_flags_| are set.
   void QueueNextDataFrame();
 
+  const SpdyStreamType type_;
+
   base::WeakPtrFactory<SpdyStream> weak_ptr_factory_;
 
   // Sentinel variable used to make sure we don't get destroyed by a
@@ -431,7 +444,6 @@ class NET_EXPORT_PRIVATE SpdyStream {
   int32 recv_window_size_;
   int32 unacked_recv_window_bytes_;
 
-  const bool pushed_;
   ScopedBandwidthMetrics metrics_;
   bool response_received_;
 
@@ -444,6 +456,9 @@ class NET_EXPORT_PRIVATE SpdyStream {
   SpdySendStatus send_status_;
 
   // The headers for the request to send.
+  //
+  // TODO(akalin): Hang onto this only until we send it. This
+  // necessitates stashing the URL separately.
   scoped_ptr<SpdyHeaderBlock> request_;
 
   // The data waiting to be sent.