Revert "Streams send a Rst frame upon being closed by client. Some minor editorial fixes."

net/spdy/spdy_session.cc

Index: net/spdy/spdy_session.cc
diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc
index 5417674250c853d576fa231bce0a06d9a31056e9..bc9be5e791e62df13f255ef13cfde8118ce6f5e4 100644
--- a/net/spdy/spdy_session.cc
+++ b/net/spdy/spdy_session.cc
@@ -230,7 +230,7 @@ net::Error SpdySession::Connect(const std::string& group_name,
   int rv = connection_->Init(group_name, destination, priority,
                              &connect_callback_, session_->tcp_socket_pool(),
                              net_log_);
-  DCHECK_LE(rv, 0);
+  DCHECK(rv <= 0);
 
   // If the connect is pending, we still return ok.  The APIs enqueue
   // work until after the connect completes asynchronously later.
@@ -368,21 +368,11 @@ int SpdySession::WriteStreamData(spdy::SpdyStreamId stream_id,
   const int kMaxSpdyFrameChunkSize = (2 * kMss) - spdy::SpdyFrame::size();
 
   // Find our stream
-  ActiveStreamMap::const_iterator it = active_streams_.find(stream_id);
-  if (it == active_streams_.end()) {
-    LOG(DFATAL) << "Attempting to write to stream " << stream_id
-                << ", which does not exist!";
-    return ERR_INVALID_SPDY_STREAM;
-  }
-
-  const scoped_refptr<SpdyStream>& stream = it->second;
-  if (!stream) {
-    LOG(DFATAL) << "Attempting to write to stream " << stream_id
-                << ", which does not exist!";
-    return ERR_INVALID_SPDY_STREAM;
-  }
-
+  DCHECK(IsStreamActive(stream_id));
+  scoped_refptr<SpdyStream> stream = active_streams_[stream_id];
   CHECK_EQ(stream->stream_id(), stream_id);
+  if (!stream)
+    return ERR_INVALID_SPDY_STREAM;
 
   // TODO(mbelshe):  Setting of the FIN is assuming that the caller will pass
   //                 all data to write in a single chunk.  Is this always true?
@@ -401,20 +391,12 @@ int SpdySession::WriteStreamData(spdy::SpdyStreamId stream_id,
   return ERR_IO_PENDING;
 }
 
-void SpdySession::CloseStreamAndSendRst(spdy::SpdyStreamId stream_id,
-                                        int status) {
-  LOG(INFO) << "Closing stream " << stream_id << " with status " << status
-            << " and sending RST_STREAM frame.";
-
-  DCHECK(IsStreamActive(stream_id));
-  const scoped_refptr<SpdyStream>& stream = active_streams_[stream_id];
-  // We send a RST_STREAM control frame here so that the server can cancel a
-  // large send.
-  scoped_ptr<spdy::SpdyRstStreamControlFrame> rst_frame(
-      spdy_framer_.CreateRstStream(stream_id, spdy::CANCEL));
-  QueueFrame(rst_frame.get(), stream->priority(), stream);
+void SpdySession::CloseStream(spdy::SpdyStreamId stream_id, int status) {
+  LOG(INFO) << "Closing stream " << stream_id << " with status " << status;
+  // TODO(mbelshe): We should send a RST_STREAM control frame here
+  //                so that the server can cancel a large send.
 
-  CloseStream(stream_id, status);
+  DeleteStream(stream_id, status);
 }
 
 bool SpdySession::IsStreamActive(spdy::SpdyStreamId stream_id) const {
@@ -541,6 +523,7 @@ void SpdySession::OnReadComplete(int bytes_read) {
 void SpdySession::OnWriteComplete(int result) {
   DCHECK(write_pending_);
   DCHECK(in_flight_write_.size());
+  DCHECK_NE(result, 0);  // This shouldn't happen for write.
 
   write_pending_ = false;
 
@@ -560,17 +543,15 @@ void SpdySession::OnWriteComplete(int result) {
     // We only notify the stream when we've fully written the pending frame.
     if (!in_flight_write_.buffer()->BytesRemaining()) {
       if (stream) {
-        // If we finished writing all the data from the buffer, it should not be
-        // the case that we wrote nothing.
-        DCHECK_NE(result, 0);
-
         // Report the number of bytes written to the caller, but exclude the
         // frame size overhead.  NOTE: if this frame was compressed the
         // reported bytes written is the compressed size, not the original
         // size.
-        result = in_flight_write_.buffer()->size();
-        DCHECK_GT(result, static_cast<int>(spdy::SpdyFrame::size()));
-        result -= static_cast<int>(spdy::SpdyFrame::size());
+        if (result > 0) {
+          result = in_flight_write_.buffer()->size();
+          DCHECK_GT(result, static_cast<int>(spdy::SpdyFrame::size()));
+          result -= static_cast<int>(spdy::SpdyFrame::size());
+        }
 
         // It is possible that the stream was cancelled while we were writing
         // to the socket.
@@ -729,11 +710,11 @@ void SpdySession::CloseAllStreams(net::Error status) {
     DCHECK(stream);
     LOG(ERROR) << "ABANDONED (stream_id=" << stream->stream_id()
       << "): " << stream->path();
-    CloseStream(stream->stream_id(), status);
+    DeleteStream(stream->stream_id(), status);
   }
 
   // TODO(erikchen): ideally stream->OnClose() is only ever called by
-  // CloseStream, but pending streams fall into their own category for now.
+  // DeleteStream, but pending streams fall into their own category for now.
   PendingStreamMap::iterator it;
   for (it = pending_streams_.begin(); it != pending_streams_.end(); ++it)
   {
@@ -794,7 +775,7 @@ void SpdySession::ActivateStream(SpdyStream* stream) {
   active_streams_[id] = stream;
 }
 
-void SpdySession::CloseStream(spdy::SpdyStreamId id, int status) {
+void SpdySession::DeleteStream(spdy::SpdyStreamId id, int status) {
   // Remove the stream from pushed_streams_ and active_streams_.
   ActivePushedStreamList::iterator it;
   for (it = pushed_streams_.begin(); it != pushed_streams_.end(); ++it) {
@@ -813,13 +794,8 @@ void SpdySession::CloseStream(spdy::SpdyStreamId id, int status) {
   // If this is an active stream, call the callback.
   const scoped_refptr<SpdyStream> stream(it2->second);
   active_streams_.erase(it2);
-  if (stream) {
-    // This is the only place that should set the half_closed flag on the
-    // stream, and it should only be done once.
-    DCHECK(!stream->half_closed_client_side());
-    stream->HalfCloseClientSide();
+  if (stream)
     stream->OnClose(status);
-  }
 }
 
 void SpdySession::RemoveFromPool() {
@@ -892,7 +868,7 @@ bool SpdySession::Respond(const spdy::SpdyHeaderBlock& headers,
   if (rv < 0) {
     DCHECK_NE(rv, ERR_IO_PENDING);
     const spdy::SpdyStreamId stream_id = stream->stream_id();
-    CloseStream(stream_id, rv);
+    DeleteStream(stream_id, rv);
     return false;
   }
   return true;
@@ -1071,7 +1047,7 @@ void SpdySession::OnControl(const spdy::SpdyControlFrame* frame) {
           *reinterpret_cast<const spdy::SpdySettingsControlFrame*>(frame));
       break;
     case spdy::RST_STREAM:
-      OnRst(*reinterpret_cast<const spdy::SpdyRstStreamControlFrame*>(frame));
+      OnFin(*reinterpret_cast<const spdy::SpdyRstStreamControlFrame*>(frame));
       break;
     case spdy::SYN_STREAM:
       OnSyn(*reinterpret_cast<const spdy::SpdySynStreamControlFrame*>(frame),
@@ -1087,7 +1063,7 @@ void SpdySession::OnControl(const spdy::SpdyControlFrame* frame) {
   }
 }
 
-void SpdySession::OnRst(const spdy::SpdyRstStreamControlFrame& frame) {
+void SpdySession::OnFin(const spdy::SpdyRstStreamControlFrame& frame) {
   spdy::SpdyStreamId stream_id = frame.stream_id();
   LOG(INFO) << "Spdy Fin for stream " << stream_id;
 
@@ -1112,7 +1088,7 @@ void SpdySession::OnRst(const spdy::SpdyRstStreamControlFrame& frame) {
     LOG(ERROR) << "Spdy stream closed: " << frame.status();
     // TODO(mbelshe): Map from Spdy-protocol errors to something sensical.
     //                For now, it doesn't matter much - it is a protocol error.
-    CloseStream(stream_id, ERR_SPDY_PROTOCOL_ERROR);
+    DeleteStream(stream_id, ERR_SPDY_PROTOCOL_ERROR);
   }
 }