Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(226)

Unified Diff: net/spdy/spdy_session_unittest.cc

Issue 129873010: Deprecate instead of close SPDY sessions upon network change. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Rework deprecation to be less intrusive and avoid making behavior on certain OSs worse Created 6 years, 11 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « net/spdy/spdy_session_pool.cc ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/spdy/spdy_session_unittest.cc
diff --git a/net/spdy/spdy_session_unittest.cc b/net/spdy/spdy_session_unittest.cc
index 2f185c7c01a09c6c7828f5acf526f8d8b4384365..af58716cb15986e0b6fe5d1abf66f2de39c6d342 100644
--- a/net/spdy/spdy_session_unittest.cc
+++ b/net/spdy/spdy_session_unittest.cc
@@ -728,6 +728,77 @@ TEST_P(SpdySessionTest, SynStreamAfterGoAway) {
EXPECT_TRUE(session == NULL);
}
+// A session observing a network change with active streams should close
+// when the last active stream is closed.
+TEST_P(SpdySessionTest, NetworkChangeWithActiveStreams) {
+ session_deps_.host_resolver->set_synchronous_mode(true);
+
+ MockConnect connect_data(SYNCHRONOUS, OK);
+ MockRead reads[] = {
+ MockRead(ASYNC, 0, 1) // EOF
+ };
+ scoped_ptr<SpdyFrame> req1(
+ spdy_util_.ConstructSpdyGet(NULL, 0, false, 1, MEDIUM, true));
+ MockWrite writes[] = {
+ CreateMockWrite(*req1, 0),
+ };
+ DeterministicSocketData data(reads, arraysize(reads),
+ writes, arraysize(writes));
+ data.set_connect_data(connect_data);
+ session_deps_.deterministic_socket_factory->AddSocketDataProvider(&data);
+
+ SSLSocketDataProvider ssl(SYNCHRONOUS, OK);
Johnny 2014/01/24 20:05:05 Possible to remove some of the boilerplate here no
+ session_deps_.deterministic_socket_factory->AddSSLSocketDataProvider(&ssl);
+
+ CreateDeterministicNetworkSession();
+
+ base::WeakPtr<SpdySession> session =
+ CreateInsecureSpdySession(http_session_, key_, BoundNetLog());
+
+ EXPECT_EQ(spdy_util_.spdy_version(), session->GetProtocolVersion());
+
+ GURL url("http://www.google.com");
+ 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(
Johnny 2014/01/24 20:05:05 indent
+ spdy_util_.ConstructGetHeaderBlock(url.spec()));
+
+ spdy_stream->SendRequestHeaders(headers.Pass(), NO_MORE_DATA_TO_SEND);
+ EXPECT_TRUE(spdy_stream->HasUrlFromHeaders());
+
+ data.RunFor(1);
+
+ EXPECT_EQ(1u, spdy_stream->stream_id());
+
+ EXPECT_TRUE(HasSpdySession(spdy_session_pool_, key_));
+
+ spdy_session_pool_->OnIPAddressChanged();
+
+ // The SpdySessionPool behavior differs based on how the OSs reacts to
+ // network changes, see comment in SpdySessionPool::OnIPAddressChanged().
+#if defined(OS_ANDROID) || defined(OS_WIN) || defined(OS_IOS)
+ // For OSs where the TCP connections will close upon relevant network
+ // changes, SpdySessionPool doesn't need to force them to close, so in these
+ // cases verify the session has become unavailable but remains open and the
+ // pre-existing stream is still active.
+ EXPECT_FALSE(HasSpdySession(spdy_session_pool_, key_));
+
+ EXPECT_FALSE(session->IsClosed());
+
+ EXPECT_TRUE(session->IsStreamActive(1));
+
+ // Should close the session.
+ spdy_stream->Close();
+#endif
+ EXPECT_EQ(NULL, spdy_stream.get());
+
+ EXPECT_TRUE(session == NULL);
+}
+
TEST_P(SpdySessionTest, ClientPing) {
session_deps_.enable_ping = true;
session_deps_.host_resolver->set_synchronous_mode(true);
« no previous file with comments | « net/spdy/spdy_session_pool.cc ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698