Enable SpdyStream's HALF_CLOSED_REMOTE state.

Unit-tests cover HTTP case of a response received before post completes,
and websocket case of a CLOSE frame with SPDY FIN set (client should
still send a websocket CLOSE).

Also rename SpdyStream::IsIdleTemporaryRename() back to IsIdle().

TEST=Also tested with Fedor's test case from the ticket.
BUG=330860

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

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

5 files changed, 133 insertions, 68 deletions

diff --git a/net/spdy/spdy_http_stream.cc b/net/spdy/spdy_http_stream.cc
index 1aa1ffe..5008819 100644
--- a/net/spdy/spdy_http_stream.cc
+++ b/net/spdy/spdy_http_stream.cc
@@ -110,7 +110,7 @@ int SpdyHttpStream::ReadResponseHeaders(const CompletionCallback& callback) {
 
   // Check if we already have the response headers. If so, return synchronously.
   if (response_headers_status_ == RESPONSE_HEADERS_ARE_COMPLETE) {
-    CHECK(!stream_->IsIdleTemporaryRename());
+    CHECK(!stream_->IsIdle());
     return OK;
   }
 
@@ -123,7 +123,7 @@ int SpdyHttpStream::ReadResponseHeaders(const CompletionCallback& callback) {
 int SpdyHttpStream::ReadResponseBody(
     IOBuffer* buf, int buf_len, const CompletionCallback& callback) {
   if (stream_.get())
-    CHECK(!stream_->IsIdleTemporaryRename());
+    CHECK(!stream_->IsIdle());
 
   CHECK(buf);
   CHECK(buf_len);
diff --git a/net/spdy/spdy_network_transaction_unittest.cc b/net/spdy/spdy_network_transaction_unittest.cc
index 1c9abed..38fe7ed 100644
--- a/net/spdy/spdy_network_transaction_unittest.cc
+++ b/net/spdy/spdy_network_transaction_unittest.cc
@@ -2059,57 +2059,50 @@ TEST_P(SpdyNetworkTransactionTest, EmptyPost) {
   EXPECT_EQ("hello!", out.response_data);
 }
 
-// While we're doing a post, the server sends back a SYN_REPLY.
-TEST_P(SpdyNetworkTransactionTest, PostWithEarlySynReply) {
-  static const char upload[] = { "hello!" };
-  ScopedVector<UploadElementReader> element_readers;
-  element_readers.push_back(
-      new UploadBytesElementReader(upload, sizeof(upload)));
-  UploadDataStream stream(element_readers.Pass(), 0);
-
-  // Setup the request
-  HttpRequestInfo request;
-  request.method = "POST";
-  request.url = GURL(kRequestUrl);
-  request.upload_data_stream = &stream;
-
-  scoped_ptr<SpdyFrame> stream_reply(
-      spdy_util_.ConstructSpdyPostSynReply(NULL, 0));
-  MockRead reads[] = {
-    CreateMockRead(*stream_reply, 1),
-    MockRead(ASYNC, 0, 4)  // EOF
-  };
-
-  scoped_ptr<SpdyFrame> req(
-      spdy_util_.ConstructSpdyPost(
-          kRequestUrl, 1, kUploadDataSize, LOWEST, NULL, 0));
+// While we're doing a post, the server sends the reply before upload completes.
+TEST_P(SpdyNetworkTransactionTest, ResponseBeforePostCompletes) {
+  scoped_ptr<SpdyFrame> req(spdy_util_.ConstructChunkedSpdyPost(NULL, 0));
   scoped_ptr<SpdyFrame> body(spdy_util_.ConstructSpdyBodyFrame(1, true));
-  scoped_ptr<SpdyFrame> rst(
-      spdy_util_.ConstructSpdyRstStream(1, RST_STREAM_PROTOCOL_ERROR));
   MockWrite writes[] = {
     CreateMockWrite(*req, 0),
-    CreateMockWrite(*body, 2),
-    CreateMockWrite(*rst, 3)
+    CreateMockWrite(*body, 3),
+  };
+  scoped_ptr<SpdyFrame> resp(spdy_util_.ConstructSpdyPostSynReply(NULL, 0));
+  MockRead reads[] = {
+    CreateMockRead(*resp, 1),
+    CreateMockRead(*body, 2),
+    MockRead(ASYNC, 0, 4)  // EOF
   };
 
+  // Write the request headers, and read the complete response
+  // while still waiting for chunked request data.
   DeterministicSocketData data(reads, arraysize(reads),
                                writes, arraysize(writes));
-  NormalSpdyTransactionHelper helper(CreatePostRequest(), DEFAULT_PRIORITY,
+  NormalSpdyTransactionHelper helper(CreateChunkedPostRequest(),
+                                     DEFAULT_PRIORITY,
                                      BoundNetLog(), GetParam(), NULL);
   helper.SetDeterministic();
   helper.RunPreTestSetup();
   helper.AddDeterministicData(&data);
-  HttpNetworkTransaction* trans = helper.trans();
 
-  TestCompletionCallback callback;
-  int rv = trans->Start(
-      &CreatePostRequest(), callback.callback(), BoundNetLog());
-  EXPECT_EQ(ERR_IO_PENDING, rv);
+  ASSERT_TRUE(helper.StartDefaultTest());
 
-  data.RunFor(4);
-  rv = callback.WaitForResult();
-  EXPECT_EQ(ERR_SPDY_PROTOCOL_ERROR, rv);
-  data.RunFor(1);
+  // Process the request headers, SYN_REPLY, and response body.
+  // The request body is still in flight.
+  data.RunFor(3);
+
+  const HttpResponseInfo* response = helper.trans()->GetResponseInfo();
+  EXPECT_EQ("HTTP/1.1 200 OK", response->headers->GetStatusLine());
+
+  // Finish sending the request body.
+  helper.request().upload_data_stream->AppendChunk(
+      kUploadData, kUploadDataSize, true);
+  data.RunFor(2);
+
+  std::string response_body;
+  EXPECT_EQ(OK, ReadTransaction(helper.trans(), &response_body));
+  EXPECT_EQ(kUploadData, response_body);
+  helper.VerifyDataConsumed();
 }
 
 // The client upon cancellation tries to send a RST_STREAM frame. The mock
diff --git a/net/spdy/spdy_stream.cc b/net/spdy/spdy_stream.cc
index 7ed562c..ce9ddd3 100644
--- a/net/spdy/spdy_stream.cc
+++ b/net/spdy/spdy_stream.cc
@@ -403,9 +403,8 @@ int SpdyStream::OnInitialResponseHeadersReceived(
 
     case SPDY_REQUEST_RESPONSE_STREAM:
       // For a request/response stream, we're ready for the response
-      // headers once we've finished sending the request headers and
-      // the request body (if we have one).
-      if (io_state_ != STATE_HALF_CLOSED_LOCAL) {
+      // headers once we've finished sending the request headers.
+      if (io_state_ == STATE_IDLE) {
         session_->ResetStream(stream_id_, RST_STREAM_PROTOCOL_ERROR,
                               "Response received before request sent");
         return ERR_SPDY_PROTOCOL_ERROR;
@@ -485,16 +484,15 @@ void SpdyStream::OnDataReceived(scoped_ptr<SpdyBuffer> buffer) {
 
   if (!buffer) {
     metrics_.StopStream();
-    // TODO(jgraettinger): Closing from OPEN preserves current SpdyStream
-    // behavior, but is incorrect HTTP/2 behavior. |io_state_| should be
-    // HALF_CLOSED_REMOTE. To be changed in a subsequent CL.
-    if (io_state_ == STATE_OPEN || io_state_ == STATE_HALF_CLOSED_LOCAL) {
+    if (io_state_ == STATE_OPEN) {
+      io_state_ = STATE_HALF_CLOSED_REMOTE;
+    } else if (io_state_ == STATE_HALF_CLOSED_LOCAL) {
       io_state_ = STATE_CLOSED;
       // Deletes |this|.
       session_->CloseActiveStream(stream_id_, OK);
-      return;
+    } else {
+      NOTREACHED() << io_state_;
     }
-    NOTREACHED() << io_state_;
     return;
   }
 
@@ -535,18 +533,29 @@ void SpdyStream::OnFrameWriteComplete(SpdyFrameType frame_type,
   }
 
   if (pending_send_status_ == NO_MORE_DATA_TO_SEND) {
-    // TODO(jgraettinger): Once HALF_CLOSED_REMOTE is added,
-    // we'll need to handle transitions to fully closed here.
-    CHECK_EQ(io_state_, STATE_OPEN);
-    io_state_ = STATE_HALF_CLOSED_LOCAL;
+    if(io_state_ == STATE_OPEN) {
+      io_state_ = STATE_HALF_CLOSED_LOCAL;
+    } else if(io_state_ == STATE_HALF_CLOSED_REMOTE) {
+      io_state_ = STATE_CLOSED;
+    } else {
+      NOTREACHED() << io_state_;
+    }
   }
-
-  // Notify delegate of write completion. May destroy |this|.
+  // Notify delegate of write completion. Must not destroy |this|.
   CHECK(delegate_);
-  if (frame_type == SYN_STREAM) {
-    delegate_->OnRequestHeadersSent();
-  } else {
-    delegate_->OnDataSent();
+  {
+    base::WeakPtr<SpdyStream> weak_this = GetWeakPtr();
+    if (frame_type == SYN_STREAM) {
+      delegate_->OnRequestHeadersSent();
+    } else {
+      delegate_->OnDataSent();
+    }
+    CHECK(weak_this);
+  }
+
+  if (io_state_ == STATE_CLOSED) {
+    // Deletes |this|.
+    session_->CloseActiveStream(stream_id_, OK);
   }
 }
 
@@ -559,7 +568,8 @@ int SpdyStream::OnRequestHeadersSent() {
 }
 
 int SpdyStream::OnDataSent(size_t frame_size) {
-  CHECK_EQ(io_state_, STATE_OPEN);
+  CHECK(io_state_ == STATE_OPEN ||
+        io_state_ == STATE_HALF_CLOSED_REMOTE) << io_state_;
 
   size_t frame_payload_size = frame_size - session_->GetDataFrameMinimumSize();
 
@@ -654,7 +664,8 @@ void SpdyStream::SendData(IOBuffer* data,
                           SpdySendStatus send_status) {
   CHECK_NE(type_, SPDY_PUSH_STREAM);
   CHECK_EQ(pending_send_status_, MORE_DATA_TO_SEND);
-  CHECK_EQ(io_state_, STATE_OPEN);
+  CHECK(io_state_ == STATE_OPEN ||
+        io_state_ == STATE_HALF_CLOSED_REMOTE) << io_state_;
   CHECK(!pending_send_data_.get());
   pending_send_data_ = new DrainableIOBuffer(data, length);
   pending_send_status_ = send_status;
@@ -696,7 +707,7 @@ bool SpdyStream::IsLocallyClosed() const {
       io_state_ == STATE_CLOSED;
 }
 
-bool SpdyStream::IsIdleTemporaryRename() const {
+bool SpdyStream::IsIdle() const {
   return io_state_ == STATE_IDLE;
 }
 
@@ -762,7 +773,8 @@ void SpdyStream::UpdateHistograms() {
 void SpdyStream::QueueNextDataFrame() {
   // Until the request has been completely sent, we cannot be sure
   // that our stream_id is correct.
-  CHECK_EQ(io_state_, STATE_OPEN);
+  CHECK(io_state_ == STATE_OPEN ||
+        io_state_ == STATE_HALF_CLOSED_REMOTE) << io_state_;
   CHECK_GT(stream_id_, 0u);
   CHECK(pending_send_data_.get());
   CHECK_GT(pending_send_data_->BytesRemaining(), 0);
diff --git a/net/spdy/spdy_stream.h b/net/spdy/spdy_stream.h
index 083225c..a8cc7dc 100644
--- a/net/spdy/spdy_stream.h
+++ b/net/spdy/spdy_stream.h
@@ -395,9 +395,7 @@ class NET_EXPORT_PRIVATE SpdyStream {
 
   // Returns whether this stream is IDLE: request and response headers
   // have neither been sent nor receieved.
-  // TODO(jgraettinger): Renamed to force compilation error & semantics
-  // update at call sites. Undo this.
-  bool IsIdleTemporaryRename() const;
+  bool IsIdle() const;
 
   // Returns whether or not this stream is fully open: that request and
   // response headers are complete, and it is not in a half-closed state.
@@ -444,13 +442,12 @@ class NET_EXPORT_PRIVATE SpdyStream {
   // a remote SYN_STREAM (the client can only initate streams).
   // TODO(jgraettinger): RESERVED_REMOTE must be added to the state
   // machine when PUSH_PROMISE is implemented.
-  // TODO(jgraettinger): HALF_CLOSED_REMOTE must be added to the state
-  // machine to support remotely closed, ongoing sends.
   enum State {
     STATE_IDLE,
     STATE_OPEN,
     STATE_HALF_CLOSED_LOCAL_UNCLAIMED,
     STATE_HALF_CLOSED_LOCAL,
+    STATE_HALF_CLOSED_REMOTE,
     STATE_CLOSED,
   };
 
diff --git a/net/spdy/spdy_websocket_stream_unittest.cc b/net/spdy/spdy_websocket_stream_unittest.cc
index 3c1b3cb..1813947 100644
--- a/net/spdy/spdy_websocket_stream_unittest.cc
+++ b/net/spdy/spdy_websocket_stream_unittest.cc
@@ -235,6 +235,12 @@ class SpdyWebSocketStreamTest
         kClosingFrameLength,
         stream_id_,
         false));
+
+    closing_frame_fin_.reset(spdy_util_.ConstructSpdyWebSocketDataFrame(
+        kClosingFrame,
+        kClosingFrameLength,
+        stream_id_,
+        true));
   }
 
   void InitSession(MockRead* reads, size_t reads_count,
@@ -273,6 +279,7 @@ class SpdyWebSocketStreamTest
   scoped_ptr<SpdyFrame> response_frame_;
   scoped_ptr<SpdyFrame> message_frame_;
   scoped_ptr<SpdyFrame> closing_frame_;
+  scoped_ptr<SpdyFrame> closing_frame_fin_;
   HostPortPair host_port_pair_;
   SpdySessionKey spdy_session_key_;
   TestCompletionCallback completion_callback_;
@@ -375,6 +382,62 @@ TEST_P(SpdyWebSocketStreamTest, Basic) {
   EXPECT_TRUE(data()->at_write_eof());
 }
 
+// A SPDY websocket may still send it's close frame after
+// recieving a close with SPDY stream FIN.
+TEST_P(SpdyWebSocketStreamTest, RemoteCloseWithFin) {
+  Prepare(1);
+  MockWrite writes[] = {
+    CreateMockWrite(*request_frame_.get(), 1),
+    CreateMockWrite(*closing_frame_.get(), 4),
+  };
+  MockRead reads[] = {
+    CreateMockRead(*response_frame_.get(), 2),
+    CreateMockRead(*closing_frame_fin_.get(), 3),
+    MockRead(SYNCHRONOUS, 0, 5)  // EOF cause OnCloseSpdyStream event.
+  };
+  InitSession(reads, arraysize(reads), writes, arraysize(writes));
+
+  SpdyWebSocketStreamEventRecorder delegate(completion_callback_.callback());
+  delegate.SetOnReceivedData(
+      base::Bind(&SpdyWebSocketStreamTest::DoSendClosingFrame,
+                 base::Unretained(this)));
+
+  websocket_stream_.reset(new SpdyWebSocketStream(session_, &delegate));
+  BoundNetLog net_log;
+  GURL url("ws://example.com/echo");
+  ASSERT_EQ(OK, websocket_stream_->InitializeStream(url, HIGHEST, net_log));
+
+  SendRequest();
+  completion_callback_.WaitForResult();
+  websocket_stream_.reset();
+
+  const std::vector<SpdyWebSocketStreamEvent>& events =
+      delegate.GetSeenEvents();
+  EXPECT_EQ(5U, events.size());
+
+  EXPECT_EQ(SpdyWebSocketStreamEvent::EVENT_SENT_HEADERS,
+            events[0].event_type);
+  EXPECT_EQ(OK, events[0].result);
+  EXPECT_EQ(SpdyWebSocketStreamEvent::EVENT_RECEIVED_HEADER,
+            events[1].event_type);
+  EXPECT_EQ(OK, events[1].result);
+  EXPECT_EQ(SpdyWebSocketStreamEvent::EVENT_RECEIVED_DATA,
+            events[2].event_type);
+  EXPECT_EQ(static_cast<int>(kClosingFrameLength), events[2].result);
+  EXPECT_EQ(SpdyWebSocketStreamEvent::EVENT_SENT_DATA,
+            events[3].event_type);
+  EXPECT_EQ(static_cast<int>(kClosingFrameLength), events[3].result);
+  EXPECT_EQ(SpdyWebSocketStreamEvent::EVENT_CLOSE,
+            events[4].event_type);
+  EXPECT_EQ(OK, events[4].result);
+
+  // EOF closes SPDY session.
+  EXPECT_FALSE(
+      HasSpdySession(http_session_->spdy_session_pool(), spdy_session_key_));
+  EXPECT_TRUE(data()->at_read_eof());
+  EXPECT_TRUE(data()->at_write_eof());
+}
+
 TEST_P(SpdyWebSocketStreamTest, DestructionBeforeClose) {
   Prepare(1);
   MockWrite writes[] = {