[SPDY] Remove more code related to sending HEADERS frames

Refactor out common code between
SpdyStream::Do{SendBodyComplete,Open}().

Add comments and TODOs re. calls to SpdyStream::Delegate::OnResponseReceived() ending up
deleting the stream.

BUG=242288
R=rch@chromium.org

Review URL: https://codereview.chromium.org/15967002

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

10 files changed, 44 insertions, 81 deletions

diff --git a/net/spdy/spdy_http_stream.cc b/net/spdy/spdy_http_stream.cc
index aa4ea9a..c6d1a6a 100644
--- a/net/spdy/spdy_http_stream.cc
+++ b/net/spdy/spdy_http_stream.cc
@@ -368,11 +368,6 @@ int SpdyHttpStream::OnResponseReceived(const SpdyHeaderBlock& response,
   return status;
 }
 
-void SpdyHttpStream::OnHeadersSent() {
-  // For HTTP streams, no HEADERS frame is sent from the client.
-  NOTREACHED();
-}
-
 int SpdyHttpStream::OnDataReceived(scoped_ptr<SpdyBuffer> buffer) {
   // SpdyStream won't call us with data if the header block didn't contain a
   // valid set of headers.  So we don't expect to not have headers received
diff --git a/net/spdy/spdy_http_stream.h b/net/spdy/spdy_http_stream.h
index 1fdc3cb..c02ec44 100644
--- a/net/spdy/spdy_http_stream.h
+++ b/net/spdy/spdy_http_stream.h
@@ -90,7 +90,6 @@ class NET_EXPORT_PRIVATE SpdyHttpStream : public SpdyStream::Delegate,
   virtual int OnResponseReceived(const SpdyHeaderBlock& response,
                                  base::Time response_time,
                                  int status) OVERRIDE;
-  virtual void OnHeadersSent() OVERRIDE;
   virtual int OnDataReceived(scoped_ptr<SpdyBuffer> buffer) OVERRIDE;
   virtual void OnDataSent() OVERRIDE;
   virtual void OnClose(int status) OVERRIDE;
diff --git a/net/spdy/spdy_proxy_client_socket.cc b/net/spdy/spdy_proxy_client_socket.cc
index 43ec5b2..b028ba2 100644
--- a/net/spdy/spdy_proxy_client_socket.cc
+++ b/net/spdy/spdy_proxy_client_socket.cc
@@ -478,11 +478,6 @@ int SpdyProxyClientSocket::OnResponseReceived(
   return OK;
 }
 
-void SpdyProxyClientSocket::OnHeadersSent() {
-  // Proxy client sockets don't send any HEADERS frame.
-  NOTREACHED();
-}
-
 // Called when data is received or on EOF (if |buffer| is NULL).
 int SpdyProxyClientSocket::OnDataReceived(scoped_ptr<SpdyBuffer> buffer) {
   if (buffer) {
diff --git a/net/spdy/spdy_proxy_client_socket.h b/net/spdy/spdy_proxy_client_socket.h
index 0e5f777..fb26c272 100644
--- a/net/spdy/spdy_proxy_client_socket.h
+++ b/net/spdy/spdy_proxy_client_socket.h
@@ -97,7 +97,6 @@ class NET_EXPORT_PRIVATE SpdyProxyClientSocket : public ProxyClientSocket,
   virtual int OnResponseReceived(const SpdyHeaderBlock& response,
                                  base::Time response_time,
                                  int status) OVERRIDE;
-  virtual void OnHeadersSent() OVERRIDE;
   virtual int OnDataReceived(scoped_ptr<SpdyBuffer> buffer) OVERRIDE;
   virtual void OnDataSent() OVERRIDE;
   virtual void OnClose(int status) OVERRIDE;
diff --git a/net/spdy/spdy_stream.cc b/net/spdy/spdy_stream.cc
index 485bbc3..8071b20 100644
--- a/net/spdy/spdy_stream.cc
+++ b/net/spdy/spdy_stream.cc
@@ -141,6 +141,8 @@ void SpdyStream::PushedStreamReplayData() {
 
   continue_buffering_data_ = false;
 
+  // TODO(akalin): This call may delete this object. Figure out what
+  // to do in that case.
   int rv = delegate_->OnResponseReceived(*response_, response_time_, OK);
   if (rv == ERR_INCOMPLETE_SPDY_HEADERS) {
     // We don't have complete headers.  Assume we're waiting for another
@@ -389,7 +391,7 @@ int SpdyStream::OnResponseReceived(const SpdyHeaderBlock& response) {
   if (!pushed_ && io_state_ != STATE_WAITING_FOR_RESPONSE)
     return ERR_SPDY_PROTOCOL_ERROR;
   if (pushed_)
-    CHECK(io_state_ == STATE_NONE);
+    CHECK_EQ(io_state_, STATE_NONE);
   io_state_ = STATE_OPEN;
 
   // Append all the headers into the response header block.
@@ -409,8 +411,10 @@ int SpdyStream::OnResponseReceived(const SpdyHeaderBlock& response) {
     return ERR_SPDY_PROTOCOL_ERROR;
   }
 
-  if (delegate_)
+  if (delegate_) {
+    // May delete this object.
     rv = delegate_->OnResponseReceived(*response_, response_time_, rv);
+  }
   // If delegate_ is not yet attached, we'll call OnResponseReceived after the
   // delegate gets attached to the stream.
 
@@ -448,6 +452,7 @@ int SpdyStream::OnHeaders(const SpdyHeaderBlock& headers) {
 
   int rv = OK;
   if (delegate_) {
+    // May delete this object.
     rv = delegate_->OnResponseReceived(*response_, response_time_, rv);
     // ERR_INCOMPLETE_SPDY_HEADERS means that we are waiting for more
     // headers before the response header block is complete.
@@ -828,6 +833,35 @@ int SpdyStream::DoSendBody() {
 }
 
 int SpdyStream::DoSendBodyComplete(int result) {
+  result = ProcessJustCompletedFrame(result, STATE_SEND_BODY_COMPLETE);
+
+  if (result != OK)
+    return result;
+
+  SpdySendStatus send_status = delegate_->OnSendBodyComplete();
+
+  io_state_ =
+      (send_status == MORE_DATA_TO_SEND) ?
+      STATE_SEND_BODY : STATE_WAITING_FOR_RESPONSE;
+
+  return OK;
+}
+
+int SpdyStream::DoOpen() {
+  int result = ProcessJustCompletedFrame(OK, STATE_OPEN);
+
+  if (result != OK)
+    return result;
+
+  // Set |io_state_| first as |delegate_| may check it.
+  io_state_ = STATE_OPEN;
+
+  delegate_->OnDataSent();
+
+  return OK;
+}
+
+int SpdyStream::ProcessJustCompletedFrame(int result, State io_pending_state) {
   if (result != OK)
     return result;
 
@@ -852,7 +886,7 @@ int SpdyStream::DoSendBodyComplete(int result) {
 
   pending_send_data_->DidConsume(frame_payload_size);
   if (pending_send_data_->BytesRemaining() > 0) {
-    io_state_ = STATE_SEND_BODY_COMPLETE;
+    io_state_ = io_pending_state;
     QueueNextDataFrame();
     return ERR_IO_PENDING;
   }
@@ -865,57 +899,6 @@ int SpdyStream::DoSendBodyComplete(int result) {
     return ERR_UNEXPECTED;
   }
 
-  io_state_ =
-      (delegate_->OnSendBodyComplete() == MORE_DATA_TO_SEND) ?
-      STATE_SEND_BODY : STATE_WAITING_FOR_RESPONSE;
-
-  return OK;
-}
-
-int SpdyStream::DoOpen() {
-  io_state_ = STATE_OPEN;
-
-  switch (just_completed_frame_type_) {
-    case DATA: {
-      if (just_completed_frame_size_ < session_->GetDataFrameMinimumSize()) {
-        NOTREACHED();
-        return ERR_UNEXPECTED;
-      }
-
-      size_t frame_payload_size =
-          just_completed_frame_size_ - session_->GetDataFrameMinimumSize();
-      if (frame_payload_size > session_->GetDataFrameMaximumPayload()) {
-        NOTREACHED();
-        return ERR_UNEXPECTED;
-      }
-
-      send_bytes_ += frame_payload_size;
-
-      pending_send_data_->DidConsume(frame_payload_size);
-      if (pending_send_data_->BytesRemaining() > 0) {
-        QueueNextDataFrame();
-        return ERR_IO_PENDING;
-      }
-
-      pending_send_data_ = NULL;
-      pending_send_flags_ = DATA_FLAG_NONE;
-
-      if (delegate_)
-        delegate_->OnDataSent();
-
-      break;
-    }
-
-    case HEADERS:
-      if (delegate_)
-        delegate_->OnHeadersSent();
-      break;
-
-    default:
-      NOTREACHED();
-      return ERR_UNEXPECTED;
-  }
-
   return OK;
 }
 
diff --git a/net/spdy/spdy_stream.h b/net/spdy/spdy_stream.h
index c2c9dcc..ebeb042 100644
--- a/net/spdy/spdy_stream.h
+++ b/net/spdy/spdy_stream.h
@@ -83,9 +83,6 @@ class NET_EXPORT_PRIVATE SpdyStream {
                                    base::Time response_time,
                                    int status) = 0;
 
-    // Called when a HEADERS frame is sent.
-    virtual void OnHeadersSent() = 0;
-
     // Called when data is received. |buffer| may be NULL, which
     // signals EOF.  Must return OK if the data was received
     // successfully, or a network error code otherwise.
@@ -377,6 +374,13 @@ class NET_EXPORT_PRIVATE SpdyStream {
   int DoReadHeadersComplete(int result);
   int DoOpen();
 
+  // Does the bookkeeping necessary after a frame has just been
+  // sent. If there's more data to be sent, |state_| is set to
+  // |io_pending_state|, the next frame is queued up, and
+  // ERR_IO_PENDING is returned. Otherwise, returns OK if successful
+  // or an error if not.
+  int ProcessJustCompletedFrame(int result, State io_pending_state);
+
   // Update the histograms.  Can safely be called repeatedly, but should only
   // be called after the stream has completed.
   void UpdateHistograms();
diff --git a/net/spdy/spdy_stream_test_util.cc b/net/spdy/spdy_stream_test_util.cc
index f3781f4..ee23b58 100644
--- a/net/spdy/spdy_stream_test_util.cc
+++ b/net/spdy/spdy_stream_test_util.cc
@@ -38,8 +38,6 @@ int ClosingDelegate::OnResponseReceived(const SpdyHeaderBlock& response,
   return OK;
 }
 
-void ClosingDelegate::OnHeadersSent() {}
-
 int ClosingDelegate::OnDataReceived(scoped_ptr<SpdyBuffer> buffer) {
   return OK;
 }
@@ -77,8 +75,6 @@ int StreamDelegateBase::OnResponseReceived(const SpdyHeaderBlock& response,
   return status;
 }
 
-void StreamDelegateBase::OnHeadersSent() {}
-
 int StreamDelegateBase::OnDataReceived(scoped_ptr<SpdyBuffer> buffer) {
   if (buffer)
     received_data_queue_.Enqueue(buffer.Pass());
diff --git a/net/spdy/spdy_stream_test_util.h b/net/spdy/spdy_stream_test_util.h
index 21e57d5..827f476 100644
--- a/net/spdy/spdy_stream_test_util.h
+++ b/net/spdy/spdy_stream_test_util.h
@@ -32,7 +32,6 @@ class ClosingDelegate : public SpdyStream::Delegate {
   virtual int OnResponseReceived(const SpdyHeaderBlock& response,
                                  base::Time response_time,
                                  int status) OVERRIDE;
-  virtual void OnHeadersSent() OVERRIDE;
   virtual int OnDataReceived(scoped_ptr<SpdyBuffer> buffer) OVERRIDE;
   virtual void OnDataSent() OVERRIDE;
   virtual void OnClose(int status) OVERRIDE;
@@ -57,7 +56,6 @@ class StreamDelegateBase : public SpdyStream::Delegate {
   virtual int OnResponseReceived(const SpdyHeaderBlock& response,
                                  base::Time response_time,
                                  int status) OVERRIDE;
-  virtual void OnHeadersSent() OVERRIDE;
   virtual int OnDataReceived(scoped_ptr<SpdyBuffer> buffer) OVERRIDE;
   virtual void OnDataSent() OVERRIDE;
   virtual void OnClose(int status) OVERRIDE;
diff --git a/net/spdy/spdy_websocket_stream.cc b/net/spdy/spdy_websocket_stream.cc
index aa75978..07760d1 100644
--- a/net/spdy/spdy_websocket_stream.cc
+++ b/net/spdy/spdy_websocket_stream.cc
@@ -105,11 +105,6 @@ int SpdyWebSocketStream::OnResponseReceived(
   return delegate_->OnReceivedSpdyResponseHeader(response, status);
 }
 
-void SpdyWebSocketStream::OnHeadersSent() {
-  // This will be called when WebSocket over SPDY supports new framing.
-  NOTREACHED();
-}
-
 int SpdyWebSocketStream::OnDataReceived(scoped_ptr<SpdyBuffer> buffer) {
   DCHECK(delegate_);
   delegate_->OnReceivedSpdyData(buffer.Pass());
diff --git a/net/spdy/spdy_websocket_stream.h b/net/spdy/spdy_websocket_stream.h
index d3290404..e497944 100644
--- a/net/spdy/spdy_websocket_stream.h
+++ b/net/spdy/spdy_websocket_stream.h
@@ -80,7 +80,6 @@ class NET_EXPORT_PRIVATE SpdyWebSocketStream
   virtual int OnResponseReceived(const SpdyHeaderBlock& response,
                                  base::Time response_time,
                                  int status) OVERRIDE;
-  virtual void OnHeadersSent() OVERRIDE;
   virtual int OnDataReceived(scoped_ptr<SpdyBuffer> buffer) OVERRIDE;
   virtual void OnDataSent() OVERRIDE;
   virtual void OnClose(int status) OVERRIDE;