[SPDY] Refactor SpdyStream state machine

In particular, remove STATE_SEND_BODY,
STATE_SEND_BODY_COMPLETE, SPDY_WAITING_FOR_RESPONSE and
instead add logic to detect when we receive an out-of-place
response.

Fix tests that were using the wrong SpdyStreamType and
weren't using a delegate.

Add various TODOs.

BUG=243643
R=rch@chromium.org

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

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

net/spdy/spdy_http_stream.cc

@@ -292,6 +292,8 @@ void SpdyHttpStream::OnRequestHeadersSent() {
   if (!callback_.is_null())
     DoCallback(OK);
 
+  // TODO(akalin): Do this immediately after sending the request
+  // headers.
   if (HasUploadData())
     ReadAndSendRequestBodyData();
 }

net/spdy/spdy_network_transaction_spdy3_unittest.cc

@@ -2446,8 +2446,9 @@ TEST_P(SpdyNetworkTransactionSpdy3Test, FlowControlStallResume) {
   // TODO(satorux): This is because of the weirdness in reading the request
   // body in OnSendBodyComplete(). See crbug.com/113107.
   EXPECT_TRUE(upload_data_stream.IsEOF());
-  // But the body is not yet fully sent (kUploadData is not yet sent).
-  EXPECT_FALSE(stream->stream()->body_sent());
+  // But the body is not yet fully sent (kUploadData is not yet sent)
+  // since we're send-stalled.
+  EXPECT_TRUE(stream->stream()->send_stalled_by_flow_control());
 
   data.ForceNextRead();   // Read in WINDOW_UPDATE frame.
   rv = callback.WaitForResult();
@@ -2548,8 +2549,8 @@ TEST_P(SpdyNetworkTransactionSpdy3Test, FlowControlStallResumeAfterSettings) {
   // TODO(satorux): This is because of the weirdness in reading the request
   // body in OnSendBodyComplete(). See crbug.com/113107.
   EXPECT_TRUE(upload_data_stream.IsEOF());
-  // But the body is not yet fully sent (kUploadData is not yet sent).
-  EXPECT_FALSE(stream->stream()->body_sent());
+  // But the body is not yet fully sent (kUploadData is not yet sent)
+  // since we're send-stalled.
   EXPECT_TRUE(stream->stream()->send_stalled_by_flow_control());
 
   data.ForceNextRead();   // Read in SETTINGS frame to unstall.
@@ -2658,8 +2659,9 @@ TEST_P(SpdyNetworkTransactionSpdy3Test, FlowControlNegativeSendWindowSize) {
   // TODO(satorux): This is because of the weirdness in reading the request
   // body in OnSendBodyComplete(). See crbug.com/113107.
   EXPECT_TRUE(upload_data_stream.IsEOF());
-  // But the body is not yet fully sent (kUploadData is not yet sent).
-  EXPECT_FALSE(stream->stream()->body_sent());
+  // But the body is not yet fully sent (kUploadData is not yet sent)
+  // since we're send-stalled.
+  EXPECT_TRUE(stream->stream()->send_stalled_by_flow_control());
 
   data.ForceNextRead();   // Read in WINDOW_UPDATE or SETTINGS frame.
   rv = callback.WaitForResult();

net/spdy/spdy_session_spdy2_unittest.cc

@@ -165,12 +165,16 @@ TEST_F(SpdySessionSpdy2Test, GoAway) {
 
   GURL url("http://www.google.com");
   base::WeakPtr<SpdyStream> spdy_stream1 =
-      CreateStreamSynchronously(
-          SPDY_BIDIRECTIONAL_STREAM, session, url, MEDIUM, BoundNetLog());
+      CreateStreamSynchronously(SPDY_REQUEST_RESPONSE_STREAM,
+                                session, url, MEDIUM, BoundNetLog());
+  test::StreamDelegateDoNothing delegate1(spdy_stream1);
+  spdy_stream1->SetDelegate(&delegate1);
 
   base::WeakPtr<SpdyStream> spdy_stream2 =
-      CreateStreamSynchronously(
-          SPDY_BIDIRECTIONAL_STREAM, session, url, MEDIUM, BoundNetLog());
+      CreateStreamSynchronously(SPDY_REQUEST_RESPONSE_STREAM,
+                                session, url, MEDIUM, BoundNetLog());
+  test::StreamDelegateDoNothing delegate2(spdy_stream2);
+  spdy_stream2->SetDelegate(&delegate2);
 
   scoped_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock);
   (*headers)["method"] = "GET";
@@ -1170,15 +1174,15 @@ TEST_F(SpdySessionSpdy2Test, OutOfOrderSynStreams) {
   GURL url("http://www.google.com");
 
   base::WeakPtr<SpdyStream> spdy_stream1 =
-      CreateStreamSynchronously(
-          SPDY_BIDIRECTIONAL_STREAM, session, url, LOWEST, BoundNetLog());
+      CreateStreamSynchronously(SPDY_REQUEST_RESPONSE_STREAM,
+                                session, url, LOWEST, BoundNetLog());
   ASSERT_TRUE(spdy_stream1.get() != NULL);
   EXPECT_EQ(0u, spdy_stream1->stream_id());
   test::StreamDelegateDoNothing delegate1(spdy_stream1);
   spdy_stream1->SetDelegate(&delegate1);
 
   base::WeakPtr<SpdyStream> spdy_stream2 =
-      CreateStreamSynchronously(SPDY_BIDIRECTIONAL_STREAM,
+      CreateStreamSynchronously(SPDY_REQUEST_RESPONSE_STREAM,
                                 session, url, HIGHEST, BoundNetLog());
   ASSERT_TRUE(spdy_stream2.get() != NULL);
   EXPECT_EQ(0u, spdy_stream2->stream_id());
@@ -2128,8 +2132,10 @@ TEST_F(SpdySessionSpdy2Test, GoAwayWhileInDoLoop) {
 
   GURL url1("http://www.google.com");
   base::WeakPtr<SpdyStream> spdy_stream1 =
-      CreateStreamSynchronously(SPDY_BIDIRECTIONAL_STREAM,
+      CreateStreamSynchronously(SPDY_REQUEST_RESPONSE_STREAM,
                                 session, url1, MEDIUM, BoundNetLog());
+  test::StreamDelegateDoNothing delegate1(spdy_stream1);
+  spdy_stream1->SetDelegate(&delegate1);
   session = NULL;
   ASSERT_TRUE(spdy_stream1.get() != NULL);
   EXPECT_EQ(0u, spdy_stream1->stream_id());
@@ -2351,10 +2357,12 @@ TEST_F(SpdySessionSpdy2Test, CloseOneIdleConnectionFailsWhenSessionInUse) {
 
   TestCompletionCallback callback1;
   base::WeakPtr<SpdyStream> spdy_stream1 =
-      CreateStreamSynchronously(SPDY_BIDIRECTIONAL_STREAM,
+      CreateStreamSynchronously(SPDY_REQUEST_RESPONSE_STREAM,
                                 session1, url1, DEFAULT_PRIORITY,
                                 BoundNetLog());
   ASSERT_TRUE(spdy_stream1.get());
+  test::StreamDelegateDoNothing delegate1(spdy_stream1);
+  spdy_stream1->SetDelegate(&delegate1);
 
   scoped_ptr<SpdyHeaderBlock> headers(
       spdy_util_.ConstructGetHeaderBlock(url1.spec()));

net/spdy/spdy_session_spdy3_unittest.cc

@@ -248,12 +248,16 @@ TEST_F(SpdySessionSpdy3Test, GoAway) {
 
   GURL url("http://www.google.com");
   base::WeakPtr<SpdyStream> spdy_stream1 =
-      CreateStreamSynchronously(SPDY_BIDIRECTIONAL_STREAM,
+      CreateStreamSynchronously(SPDY_REQUEST_RESPONSE_STREAM,
                                 session, url, MEDIUM, BoundNetLog());
+  test::StreamDelegateDoNothing delegate1(spdy_stream1);
+  spdy_stream1->SetDelegate(&delegate1);
 
   base::WeakPtr<SpdyStream> spdy_stream2 =
-      CreateStreamSynchronously(SPDY_BIDIRECTIONAL_STREAM,
+      CreateStreamSynchronously(SPDY_REQUEST_RESPONSE_STREAM,
                                 session, url, MEDIUM, BoundNetLog());
+  test::StreamDelegateDoNothing delegate2(spdy_stream2);
+  spdy_stream2->SetDelegate(&delegate2);
 
   scoped_ptr<SpdyHeaderBlock> headers(new SpdyHeaderBlock);
   (*headers)[":method"] = "GET";
@@ -1317,7 +1321,7 @@ TEST_F(SpdySessionSpdy3Test, OutOfOrderSynStreams) {
   GURL url("http://www.google.com");
 
   base::WeakPtr<SpdyStream> spdy_stream1 =
-      CreateStreamSynchronously(SPDY_BIDIRECTIONAL_STREAM,
+      CreateStreamSynchronously(SPDY_REQUEST_RESPONSE_STREAM,
                                 session, url, LOWEST, BoundNetLog());
   ASSERT_TRUE(spdy_stream1.get() != NULL);
   EXPECT_EQ(0u, spdy_stream1->stream_id());
@@ -1325,7 +1329,7 @@ TEST_F(SpdySessionSpdy3Test, OutOfOrderSynStreams) {
   spdy_stream1->SetDelegate(&delegate1);
 
   base::WeakPtr<SpdyStream> spdy_stream2 =
-      CreateStreamSynchronously(SPDY_BIDIRECTIONAL_STREAM,
+      CreateStreamSynchronously(SPDY_REQUEST_RESPONSE_STREAM,
                                 session, url, HIGHEST, BoundNetLog());
   ASSERT_TRUE(spdy_stream2.get() != NULL);
   EXPECT_EQ(0u, spdy_stream2->stream_id());
@@ -2363,8 +2367,10 @@ TEST_F(SpdySessionSpdy3Test, GoAwayWhileInDoLoop) {
 
   GURL url1("http://www.google.com");
   base::WeakPtr<SpdyStream> spdy_stream1 =
-      CreateStreamSynchronously(SPDY_BIDIRECTIONAL_STREAM,
+      CreateStreamSynchronously(SPDY_REQUEST_RESPONSE_STREAM,
                                 session, url1, MEDIUM, BoundNetLog());
+  test::StreamDelegateDoNothing delegate1(spdy_stream1);
+  spdy_stream1->SetDelegate(&delegate1);
   session = NULL;
   ASSERT_TRUE(spdy_stream1.get() != NULL);
   EXPECT_EQ(0u, spdy_stream1->stream_id());
@@ -3694,10 +3700,12 @@ TEST_F(SpdySessionSpdy3Test, CloseOneIdleConnectionFailsWhenSessionInUse) {
 
   TestCompletionCallback callback1;
   base::WeakPtr<SpdyStream> spdy_stream1 =
-      CreateStreamSynchronously(SPDY_BIDIRECTIONAL_STREAM,
+      CreateStreamSynchronously(SPDY_REQUEST_RESPONSE_STREAM,
                                 session1, url1, DEFAULT_PRIORITY,
                                 BoundNetLog());
   ASSERT_TRUE(spdy_stream1.get());
+  test::StreamDelegateDoNothing delegate1(spdy_stream1);
+  spdy_stream1->SetDelegate(&delegate1);
 
   scoped_ptr<SpdyHeaderBlock> headers1(
       spdy_util_.ConstructGetHeaderBlock(url1.spec()));

net/spdy/spdy_stream.h

@@ -278,10 +278,6 @@ class NET_EXPORT_PRIVATE SpdyStream {
   // OnClose() method.
   bool closed() const { return io_state_ == STATE_DONE; }
 
-  // TODO(satorux): This is only for testing. We should be able to remove
-  // this once crbug.com/113107 is addressed.
-  bool body_sent() const { return io_state_ > STATE_SEND_BODY_COMPLETE; }
-
   // Interface for the delegate to use.
 
   // Only one send can be in flight at a time, except for push
@@ -349,9 +345,6 @@ class NET_EXPORT_PRIVATE SpdyStream {
     STATE_SEND_DOMAIN_BOUND_CERT_COMPLETE,
     STATE_SEND_REQUEST_HEADERS,
     STATE_SEND_REQUEST_HEADERS_COMPLETE,
-    STATE_SEND_BODY,
-    STATE_SEND_BODY_COMPLETE,
-    STATE_WAITING_FOR_RESPONSE,
     STATE_OPEN,
     STATE_DONE
   };
@@ -368,19 +361,10 @@ class NET_EXPORT_PRIVATE SpdyStream {
   int DoSendDomainBoundCertComplete(int result);
   int DoSendRequestHeaders();
   int DoSendRequestHeadersComplete();
-  int DoSendBody();
-  int DoSendBodyComplete(int result);
   int DoReadHeaders();
   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();
@@ -400,7 +384,7 @@ class NET_EXPORT_PRIVATE SpdyStream {
 
   // Queues the send for next frame of the remaining data in
   // |pending_send_data_|. Must be called only when
-  // |pending_send_data_| and |pending_send_flags_| are set.
+  // |pending_send_data_| is set.
   void QueueNextDataFrame();
 
   const SpdyStreamType type_;