Re-land: Defer SpdySession destruction to support closing writes

net/spdy/spdy_session_unittest.cc

Index: net/spdy/spdy_session_unittest.cc
diff --git a/net/spdy/spdy_session_unittest.cc b/net/spdy/spdy_session_unittest.cc
index 1f7ca60daf75cd4a5577812870b720fbd2408592..dd8679174efd403905fd7440e2e8340cb1ce6b44 100644
--- a/net/spdy/spdy_session_unittest.cc
+++ b/net/spdy/spdy_session_unittest.cc
@@ -282,8 +282,7 @@ TEST_P(SpdySessionTest, PendingStreamCancellingAnother) {
   EXPECT_EQ(ERR_ABORTED, callback1.WaitForResult());
 }
 
-// A session receiving a GOAWAY frame with no active streams should
-// immediately close.
+// A session receiving a GOAWAY frame with no active streams should close.
 TEST_P(SpdySessionTest, GoAwayWithNoActiveStreams) {
   session_deps_.host_resolver->set_synchronous_mode(true);
 
@@ -307,9 +306,8 @@ TEST_P(SpdySessionTest, GoAwayWithNoActiveStreams) {
 
   // Read and process the GOAWAY frame.
   data.RunFor(1);
-
   EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_));
-
+  base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(session == NULL);
 }
 
@@ -409,12 +407,13 @@ TEST_P(SpdySessionTest, GoAwayWithActiveStreams) {
   EXPECT_EQ(NULL, spdy_stream2.get());
   EXPECT_TRUE(session->IsStreamActive(1));
 
-  EXPECT_FALSE(session->IsClosed());
+  EXPECT_TRUE(session->IsGoingAway());
 
   // Should close the session.
   spdy_stream1->Close();
   EXPECT_EQ(NULL, spdy_stream1.get());
 
+  base::MessageLoop::current()->RunUntilIdle();
   EXPECT_TRUE(session == NULL);
 }
 
@@ -489,13 +488,12 @@ TEST_P(SpdySessionTest, GoAwayTwice) {
   EXPECT_FALSE(session->IsStreamActive(3));
   EXPECT_EQ(NULL, spdy_stream2.get());
   EXPECT_TRUE(session->IsStreamActive(1));
-
-  EXPECT_FALSE(session->IsClosed());
+  EXPECT_TRUE(session->IsGoingAway());
 
   // Read and process the second GOAWAY frame, which should close the
   // session.
   data.RunFor(1);
-
+  base::MessageLoop::current()->RunUntilIdle();
   EXPECT_TRUE(session == NULL);
 }
 
@@ -568,12 +566,79 @@ TEST_P(SpdySessionTest, GoAwayWithActiveStreamsThenClose) {
   EXPECT_FALSE(session->IsStreamActive(3));
   EXPECT_EQ(NULL, spdy_stream2.get());
   EXPECT_TRUE(session->IsStreamActive(1));
-
-  EXPECT_FALSE(session->IsClosed());
+  EXPECT_TRUE(session->IsGoingAway());
 
   session->CloseSessionOnError(ERR_ABORTED, "Aborting session");
-
   EXPECT_EQ(NULL, spdy_stream1.get());
+
+  base::MessageLoop::current()->RunUntilIdle();
+  EXPECT_TRUE(session == NULL);
+}
+
+// Process a joint read buffer which causes the session to begin draining, and
+// then processes a GOAWAY. The session should gracefully drain. Regression test
+// for crbug.com/379469
+TEST_P(SpdySessionTest, GoAwayWhileDraining) {
+  session_deps_.host_resolver->set_synchronous_mode(true);
+
+  scoped_ptr<SpdyFrame> req(
+      spdy_util_.ConstructSpdyGet(NULL, 0, false, 1, MEDIUM, true));
+  MockWrite writes[] = {
+      CreateMockWrite(*req, 0),
+  };
+
+  scoped_ptr<SpdyFrame> resp(spdy_util_.ConstructSpdyGetSynReply(NULL, 0, 1));
+  scoped_ptr<SpdyFrame> goaway(spdy_util_.ConstructSpdyGoAway(1));
+  scoped_ptr<SpdyFrame> body(spdy_util_.ConstructSpdyBodyFrame(1, true));
+  size_t joint_size = goaway->size() * 2 + body->size();
+
+  // Compose interleaved |goaway| and |body| frames into a single read.
+  scoped_ptr<char[]> buffer(new char[joint_size]);
+  {
+    size_t out = 0;
+    memcpy(&buffer[out], goaway->data(), goaway->size());
+    out += goaway->size();
+    memcpy(&buffer[out], body->data(), body->size());
+    out += body->size();
+    memcpy(&buffer[out], goaway->data(), goaway->size());
+    out += goaway->size();
+    ASSERT_EQ(out, joint_size);
+  }
+  SpdyFrame joint_frames(buffer.get(), joint_size, false);
+
+  MockRead reads[] = {
+      CreateMockRead(*resp, 1), CreateMockRead(joint_frames, 2),
+      MockRead(ASYNC, 0, 3)  // EOF
+  };
+
+  MockConnect connect_data(SYNCHRONOUS, OK);
+  DeterministicSocketData data(
+      reads, arraysize(reads), writes, arraysize(writes));
+  data.set_connect_data(connect_data);
+  session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data);
+
+  CreateDeterministicNetworkSession();
+  base::WeakPtr<SpdySession> session =
+      CreateInsecureSpdySession(http_session_, key_, BoundNetLog());
+
+  GURL url(kDefaultURL);
+  base::WeakPtr<SpdyStream> spdy_stream = CreateStreamSynchronously(
+      SPDY_REQUEST_RESPONSE_STREAM, session, url, MEDIUM, BoundNetLog());
+  test::StreamDelegateDoNothing delegate(spdy_stream);
+  spdy_stream->SetDelegate(&delegate);
+
+  scoped_ptr<SpdyHeaderBlock> headers(
+      spdy_util_.ConstructGetHeaderBlock(url.spec()));
+  spdy_stream->SendRequestHeaders(headers.Pass(), NO_MORE_DATA_TO_SEND);
+  EXPECT_TRUE(spdy_stream->HasUrlFromHeaders());
+
+  data.RunFor(3);
+  base::MessageLoop::current()->RunUntilIdle();
+
+  // Stream and session closed gracefully.
+  EXPECT_TRUE(delegate.StreamIsClosed());
+  EXPECT_EQ(OK, delegate.WaitForClose());
+  EXPECT_EQ(kUploadData, delegate.TakeReceivedData());
   EXPECT_TRUE(session == NULL);
 }
 
@@ -702,7 +767,7 @@ TEST_P(SpdySessionTest, SynStreamAfterGoAway) {
   // Read and process the SYN_STREAM frame, the subsequent RST_STREAM,
   // and EOF.
   data.RunFor(3);
-
+  base::MessageLoop::current()->RunUntilIdle();
   EXPECT_TRUE(session == NULL);
 }
 
@@ -761,7 +826,7 @@ TEST_P(SpdySessionTest, NetworkChangeWithActiveStreams) {
   // pre-existing stream is still active.
   EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_));
 
-  EXPECT_FALSE(session->IsClosed());
+  EXPECT_TRUE(session->IsGoingAway());
 
   EXPECT_TRUE(session->IsStreamActive(1));
 
@@ -770,6 +835,7 @@ TEST_P(SpdySessionTest, NetworkChangeWithActiveStreams) {
 #endif
   EXPECT_EQ(NULL, spdy_stream.get());
 
+  base::MessageLoop::current()->RunUntilIdle();
   EXPECT_TRUE(session == NULL);
 }
 
@@ -1042,6 +1108,7 @@ TEST_P(SpdySessionTest, StreamIdSpaceExhausted) {
   EXPECT_EQ(kUploadData, delegate2.TakeReceivedData());
 
   // Session was destroyed.
+  base::MessageLoop::current()->RunUntilIdle();
   EXPECT_FALSE(session.get());
 }
 
@@ -1195,6 +1262,7 @@ TEST_P(SpdySessionTest, DeleteExpiredPushStreams) {
 
   // Read and process EOF.
   data.RunFor(1);
+  base::MessageLoop::current()->RunUntilIdle();
   EXPECT_TRUE(session == NULL);
 }
 
@@ -1203,14 +1271,18 @@ TEST_P(SpdySessionTest, FailedPing) {
 
   MockConnect connect_data(SYNCHRONOUS, OK);
   MockRead reads[] = {
-    MockRead(ASYNC, 0, 0, 0)  // EOF
+      MockRead(SYNCHRONOUS, ERR_IO_PENDING)  // Stall forever.
   };
   scoped_ptr<SpdyFrame> write_ping(spdy_util_.ConstructSpdyPing(1, false));
-  DeterministicSocketData data(reads, arraysize(reads), NULL, 0);
+  MockWrite writes[] = {
+      CreateMockWrite(*write_ping),
+  };
+  StaticSocketDataProvider data(
+      reads, arraysize(reads), writes, arraysize(writes));
   data.set_connect_data(connect_data);
-  session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data);
+  session_deps_.socket_factory->AddSocketDataProvider(&data);
 
-  CreateDeterministicNetworkSession();
+  CreateNetworkSession();
 
   base::WeakPtr<SpdySession> session =
       CreateInsecureSpdySession(http_session_, key_, BoundNetLog());
@@ -1232,7 +1304,7 @@ TEST_P(SpdySessionTest, FailedPing) {
   EXPECT_TRUE(session->check_ping_status_pending());
 
   // Assert session is not closed.
-  EXPECT_FALSE(session->IsClosed());
+  EXPECT_TRUE(session->IsAvailable());
   EXPECT_LT(0u, session->num_active_streams() + session->num_created_streams());
   EXPECT_TRUE(HasSpdySession(spdy_session_pool_, key_));
 
@@ -1241,6 +1313,7 @@ TEST_P(SpdySessionTest, FailedPing) {
   base::TimeTicks now = base::TimeTicks::Now();
   session->last_activity_time_ = now - base::TimeDelta::FromSeconds(1);
   session->CheckPingStatus(now);
+  base::MessageLoop::current()->RunUntilIdle();
 
   EXPECT_TRUE(session == NULL);
   EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_));
@@ -1304,6 +1377,11 @@ TEST_P(SpdySessionTest, OnSettings) {
   EXPECT_EQ(OK, stream_releaser.WaitForResult());
 
   data.RunFor(1);
+  if (spdy_util_.spdy_version() >= SPDY4) {
+    // Allow the SETTINGS+ACK to write, so the session finishes draining.
+    data.RunFor(1);
+  }
+  base::MessageLoop::current()->RunUntilIdle();
   EXPECT_TRUE(session == NULL);
 }
 
@@ -1583,7 +1661,7 @@ TEST_P(SpdySessionTest, Initialize) {
   EXPECT_NE(log.bound().source().id, socket_source.id);
 }
 
-TEST_P(SpdySessionTest, CloseSessionOnError) {
+TEST_P(SpdySessionTest, NetLogOnSessionGoaway) {
   session_deps_.host_resolver->set_synchronous_mode(true);
 
   MockConnect connect_data(SYNCHRONOUS, OK);
@@ -1625,6 +1703,53 @@ TEST_P(SpdySessionTest, CloseSessionOnError) {
     CapturingNetLog::CapturedEntry entry = entries[pos];
     int error_code = 0;
     ASSERT_TRUE(entry.GetNetErrorCode(&error_code));
+    EXPECT_EQ(OK, error_code);
+  } else {
+    ADD_FAILURE();
+  }
+}
+
+TEST_P(SpdySessionTest, NetLogOnSessionEOF) {
+  session_deps_.host_resolver->set_synchronous_mode(true);
+
+  MockConnect connect_data(SYNCHRONOUS, OK);
+  MockRead reads[] = {
+      MockRead(SYNCHRONOUS, 0, 0)  // EOF
+  };
+
+  StaticSocketDataProvider data(reads, arraysize(reads), NULL, 0);
+  data.set_connect_data(connect_data);
+  session_deps_.socket_factory->AddSocketDataProvider(&data);
+
+  CreateNetworkSession();
+
+  CapturingBoundNetLog log;
+  base::WeakPtr<SpdySession> session =
+      CreateInsecureSpdySession(http_session_, key_, log.bound());
+  EXPECT_TRUE(HasSpdySession(spdy_session_pool_, key_));
+
+  // Flush the read completion task.
+  base::MessageLoop::current()->RunUntilIdle();
+
+  EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_));
+  EXPECT_TRUE(session == NULL);
+
+  // Check that the NetLog was filled reasonably.
+  net::CapturingNetLog::CapturedEntryList entries;
+  log.GetEntries(&entries);
+  EXPECT_LT(0u, entries.size());
+
+  // Check that we logged SPDY_SESSION_CLOSE correctly.
+  int pos =
+      net::ExpectLogContainsSomewhere(entries,
+                                      0,
+                                      net::NetLog::TYPE_SPDY_SESSION_CLOSE,
+                                      net::NetLog::PHASE_NONE);
+
+  if (pos < static_cast<int>(entries.size())) {
+    CapturingNetLog::CapturedEntry entry = entries[pos];
+    int error_code = 0;
+    ASSERT_TRUE(entry.GetNetErrorCode(&error_code));
     EXPECT_EQ(ERR_CONNECTION_CLOSED, error_code);
   } else {
     ADD_FAILURE();
@@ -1857,6 +1982,7 @@ TEST_P(SpdySessionTest, CloseSessionWithTwoCreatedSelfClosingStreams) {
   EXPECT_TRUE(delegate1.StreamIsClosed());
   EXPECT_TRUE(delegate2.StreamIsClosed());
 
+  base::MessageLoop::current()->RunUntilIdle();
   EXPECT_TRUE(session == NULL);
 }
 
@@ -1930,6 +2056,7 @@ TEST_P(SpdySessionTest, CloseSessionWithTwoCreatedMutuallyClosingStreams) {
   EXPECT_TRUE(delegate1.StreamIsClosed());
   EXPECT_TRUE(delegate2.StreamIsClosed());
 
+  base::MessageLoop::current()->RunUntilIdle();
   EXPECT_TRUE(session == NULL);
 }
 
@@ -2011,6 +2138,7 @@ TEST_P(SpdySessionTest, CloseSessionWithTwoActivatedSelfClosingStreams) {
   EXPECT_TRUE(delegate1.StreamIsClosed());
   EXPECT_TRUE(delegate2.StreamIsClosed());
 
+  base::MessageLoop::current()->RunUntilIdle();
   EXPECT_TRUE(session == NULL);
 }
 
@@ -2094,6 +2222,7 @@ TEST_P(SpdySessionTest, CloseSessionWithTwoActivatedMutuallyClosingStreams) {
   EXPECT_TRUE(delegate1.StreamIsClosed());
   EXPECT_TRUE(delegate2.StreamIsClosed());
 
+  base::MessageLoop::current()->RunUntilIdle();
   EXPECT_TRUE(session == NULL);
 }
 
@@ -2124,12 +2253,14 @@ TEST_P(SpdySessionTest, CloseActivatedStreamThatClosesSession) {
 
   scoped_ptr<SpdyFrame> req(
       spdy_util_.ConstructSpdyGet(NULL, 0, false, 1, MEDIUM, true));
+  scoped_ptr<SpdyFrame> rst(
+      spdy_util_.ConstructSpdyRstStream(1, RST_STREAM_CANCEL));
   MockWrite writes[] = {
-    CreateMockWrite(*req, 0),
+      CreateMockWrite(*req, 0), CreateMockWrite(*rst, 1),
   };
 
   MockRead reads[] = {
-    MockRead(ASYNC, 0, 1)  // EOF
+      MockRead(ASYNC, 0, 2)  // EOF
   };
   DeterministicSocketData data(reads, arraysize(reads),
                                writes, arraysize(writes));
@@ -2166,8 +2297,12 @@ TEST_P(SpdySessionTest, CloseActivatedStreamThatClosesSession) {
   // session).
   spdy_stream->Cancel();
 
+  data.RunFor(1);
+  base::MessageLoop::current()->RunUntilIdle();
+
   EXPECT_EQ(NULL, spdy_stream.get());
   EXPECT_TRUE(delegate.StreamIsClosed());
+
   EXPECT_TRUE(session == NULL);
 }
 
@@ -4308,6 +4443,7 @@ TEST_P(SpdySessionTest, SendWindowSizeIncreaseWithDeletedSession) {
   // Close the session (since we can't do it from within the delegate
   // method, since it's in the stream's loop).
   session->CloseSessionOnError(ERR_CONNECTION_CLOSED, "Closing session");
+  base::RunLoop().RunUntilIdle();
   EXPECT_TRUE(session == NULL);
 
   EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_));