Index: net/spdy/spdy_network_transaction_unittest.cc |
diff --git a/net/spdy/spdy_network_transaction_unittest.cc b/net/spdy/spdy_network_transaction_unittest.cc |
index bb63d96f3c83eb4f2aab1748c6e307a2f0977307..956b02ed9523f20585f8578ff2cad5c609d9fdbc 100644 |
--- a/net/spdy/spdy_network_transaction_unittest.cc |
+++ b/net/spdy/spdy_network_transaction_unittest.cc |
@@ -65,11 +65,16 @@ enum SpdyNetworkTransactionTestSSLType { |
struct SpdyNetworkTransactionTestParams { |
SpdyNetworkTransactionTestParams() |
- : protocol(kProtoSPDY31), ssl_type(HTTPS_SPDY_VIA_NPN) {} |
+ : protocol(kProtoSPDY31), |
+ ssl_type(HTTPS_SPDY_VIA_NPN), |
+ priority_to_dependency(false) {} |
SpdyNetworkTransactionTestParams(NextProto protocol, |
- SpdyNetworkTransactionTestSSLType ssl_type) |
- : protocol(protocol), ssl_type(ssl_type) {} |
+ SpdyNetworkTransactionTestSSLType ssl_type, |
+ bool priority_to_dependency) |
+ : protocol(protocol), |
+ ssl_type(ssl_type), |
+ priority_to_dependency(priority_to_dependency) {} |
friend std::ostream& operator<<(std::ostream& os, |
const SpdyNetworkTransactionTestParams& p) { |
@@ -83,12 +88,14 @@ struct SpdyNetworkTransactionTestParams { |
break; |
} |
os << "{ protocol: " << SSLClientSocket::NextProtoToString(p.protocol) |
- << ", ssl_type: " << type_str << " }"; |
+ << ", ssl_type: " << type_str |
+ << ", priority_to_dependency: " << p.priority_to_dependency << " }"; |
return os; |
} |
NextProto protocol; |
SpdyNetworkTransactionTestSSLType ssl_type; |
+ bool priority_to_dependency; |
}; |
void UpdateSpdySessionDependencies(SpdyNetworkTransactionTestParams test_params, |
@@ -127,7 +134,10 @@ scoped_ptr<SpdySessionDependencies> CreateSpdySessionDependencies( |
class SpdyNetworkTransactionTest |
: public ::testing::TestWithParam<SpdyNetworkTransactionTestParams> { |
protected: |
- SpdyNetworkTransactionTest() : spdy_util_(GetParam().protocol) { |
+ SpdyNetworkTransactionTest() |
+ : spdy_util_(GetParam().protocol, GetParam().priority_to_dependency) { |
+ SpdySession::SetPriorityDependencyDefaultForTesting( |
+ GetParam().priority_to_dependency); |
spdy_util_.set_default_url(GURL(GetDefaultUrl())); |
} |
@@ -136,6 +146,7 @@ class SpdyNetworkTransactionTest |
// destruction. |
upload_data_stream_.reset(); |
base::RunLoop().RunUntilIdle(); |
+ SpdySession::SetPriorityDependencyDefaultForTesting(false); |
} |
void SetUp() override { |
@@ -712,10 +723,22 @@ INSTANTIATE_TEST_CASE_P( |
Spdy, |
SpdyNetworkTransactionTest, |
::testing::Values( |
- SpdyNetworkTransactionTestParams(kProtoSPDY31, HTTPS_SPDY_VIA_NPN), |
- SpdyNetworkTransactionTestParams(kProtoSPDY31, HTTP_SPDY_VIA_ALT_SVC), |
- SpdyNetworkTransactionTestParams(kProtoHTTP2, HTTPS_SPDY_VIA_NPN), |
- SpdyNetworkTransactionTestParams(kProtoHTTP2, HTTP_SPDY_VIA_ALT_SVC))); |
+ SpdyNetworkTransactionTestParams(kProtoSPDY31, |
+ HTTPS_SPDY_VIA_NPN, |
+ false), |
+ SpdyNetworkTransactionTestParams(kProtoSPDY31, |
+ HTTP_SPDY_VIA_ALT_SVC, |
+ false), |
+ SpdyNetworkTransactionTestParams(kProtoHTTP2, |
+ HTTPS_SPDY_VIA_NPN, |
+ false), |
+ SpdyNetworkTransactionTestParams(kProtoHTTP2, HTTPS_SPDY_VIA_NPN, true), |
+ SpdyNetworkTransactionTestParams(kProtoHTTP2, |
+ HTTP_SPDY_VIA_ALT_SVC, |
+ false), |
+ SpdyNetworkTransactionTestParams(kProtoHTTP2, |
+ HTTP_SPDY_VIA_ALT_SVC, |
+ true))); |
// Verify HttpNetworkTransaction constructor. |
TEST_P(SpdyNetworkTransactionTest, Constructor) { |
@@ -819,6 +842,7 @@ TEST_P(SpdyNetworkTransactionTest, GetAtEachPriority) { |
NormalSpdyTransactionHelper helper(http_req, p, BoundNetLog(), |
GetParam(), NULL); |
helper.RunToCompletion(&data); |
+ spdy_util_.OnStreamDestruction(1); |
TransactionHelperResult out = helper.output(); |
EXPECT_EQ(OK, out.rv); |
EXPECT_EQ("HTTP/1.1 200 OK", out.status_line); |
@@ -1122,23 +1146,27 @@ TEST_P(SpdyNetworkTransactionTest, TwoGetsLateBindingFromPreconnect) { |
// second transaction completes, so we can assert on read_index(). |
TEST_P(SpdyNetworkTransactionTest, ThreeGetsWithMaxConcurrent) { |
// Construct the request. |
+ // Each request fully completes before the next starts. |
scoped_ptr<SpdyFrame> req( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 1, LOWEST, true)); |
scoped_ptr<SpdyFrame> resp(spdy_util_.ConstructSpdyGetSynReply(NULL, 0, 1)); |
scoped_ptr<SpdyFrame> body(spdy_util_.ConstructSpdyBodyFrame(1, false)); |
scoped_ptr<SpdyFrame> fbody(spdy_util_.ConstructSpdyBodyFrame(1, true)); |
+ spdy_util_.OnStreamDestruction(1); |
scoped_ptr<SpdyFrame> req2( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 3, LOWEST, true)); |
scoped_ptr<SpdyFrame> resp2(spdy_util_.ConstructSpdyGetSynReply(NULL, 0, 3)); |
scoped_ptr<SpdyFrame> body2(spdy_util_.ConstructSpdyBodyFrame(3, false)); |
scoped_ptr<SpdyFrame> fbody2(spdy_util_.ConstructSpdyBodyFrame(3, true)); |
+ spdy_util_.OnStreamDestruction(3); |
scoped_ptr<SpdyFrame> req3( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 5, LOWEST, true)); |
scoped_ptr<SpdyFrame> resp3(spdy_util_.ConstructSpdyGetSynReply(NULL, 0, 5)); |
scoped_ptr<SpdyFrame> body3(spdy_util_.ConstructSpdyBodyFrame(5, false)); |
scoped_ptr<SpdyFrame> fbody3(spdy_util_.ConstructSpdyBodyFrame(5, true)); |
+ spdy_util_.OnStreamDestruction(5); |
SettingsMap settings; |
const uint32 max_concurrent_streams = 1; |
@@ -1255,17 +1283,22 @@ TEST_P(SpdyNetworkTransactionTest, FourGetsWithMaxConcurrentPriority) { |
scoped_ptr<SpdyFrame> resp(spdy_util_.ConstructSpdyGetSynReply(NULL, 0, 1)); |
scoped_ptr<SpdyFrame> body(spdy_util_.ConstructSpdyBodyFrame(1, false)); |
scoped_ptr<SpdyFrame> fbody(spdy_util_.ConstructSpdyBodyFrame(1, true)); |
+ spdy_util_.OnStreamDestruction(1); |
scoped_ptr<SpdyFrame> req2( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 3, LOWEST, true)); |
scoped_ptr<SpdyFrame> resp2(spdy_util_.ConstructSpdyGetSynReply(NULL, 0, 3)); |
scoped_ptr<SpdyFrame> body2(spdy_util_.ConstructSpdyBodyFrame(3, false)); |
scoped_ptr<SpdyFrame> fbody2(spdy_util_.ConstructSpdyBodyFrame(3, true)); |
+ // TODO(rdsmith): I'm concerned that the actual creation/destruction order, |
+ // and hence the wire bytes, are racy here. |
+ spdy_util_.OnStreamDestruction(3); |
scoped_ptr<SpdyFrame> req4( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 5, HIGHEST, true)); |
scoped_ptr<SpdyFrame> resp4(spdy_util_.ConstructSpdyGetSynReply(NULL, 0, 5)); |
scoped_ptr<SpdyFrame> fbody4(spdy_util_.ConstructSpdyBodyFrame(5, true)); |
+ spdy_util_.OnStreamDestruction(5); |
scoped_ptr<SpdyFrame> req3( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 7, LOWEST, true)); |
@@ -1404,6 +1437,7 @@ TEST_P(SpdyNetworkTransactionTest, ThreeGetsWithMaxConcurrentDelete) { |
scoped_ptr<SpdyFrame> resp(spdy_util_.ConstructSpdyGetSynReply(NULL, 0, 1)); |
scoped_ptr<SpdyFrame> body(spdy_util_.ConstructSpdyBodyFrame(1, false)); |
scoped_ptr<SpdyFrame> fbody(spdy_util_.ConstructSpdyBodyFrame(1, true)); |
+ spdy_util_.OnStreamDestruction(1); |
scoped_ptr<SpdyFrame> req2( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 3, LOWEST, true)); |
@@ -1535,6 +1569,7 @@ TEST_P(SpdyNetworkTransactionTest, ThreeGetsWithMaxConcurrentSocketClose) { |
scoped_ptr<SpdyFrame> resp(spdy_util_.ConstructSpdyGetSynReply(NULL, 0, 1)); |
scoped_ptr<SpdyFrame> body(spdy_util_.ConstructSpdyBodyFrame(1, false)); |
scoped_ptr<SpdyFrame> fin_body(spdy_util_.ConstructSpdyBodyFrame(1, true)); |
+ spdy_util_.OnStreamDestruction(1); |
scoped_ptr<SpdyFrame> req2( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 3, LOWEST, true)); |
@@ -3086,6 +3121,7 @@ TEST_P(SpdyNetworkTransactionTest, SynReplyHeaders) { |
test_cases[i].num_headers, |
1)); |
scoped_ptr<SpdyFrame> body(spdy_util_.ConstructSpdyBodyFrame(1, true)); |
+ spdy_util_.OnStreamDestruction(1); |
MockRead reads[] = { |
CreateMockRead(*resp, 1), |
CreateMockRead(*body, 2), |
@@ -3183,6 +3219,7 @@ TEST_P(SpdyNetworkTransactionTest, SynReplyHeadersVary) { |
spdy_util_.ConstructSpdyReply(1, reply_headers)); |
scoped_ptr<SpdyFrame> body(spdy_util_.ConstructSpdyBodyFrame(1, true)); |
+ spdy_util_.OnStreamDestruction(1); |
MockRead reads[] = { |
CreateMockRead(*frame_reply, 1), |
CreateMockRead(*body, 2), |
@@ -3278,6 +3315,7 @@ TEST_P(SpdyNetworkTransactionTest, InvalidSynReply) { |
NormalSpdyTransactionHelper helper(CreateGetRequest(), DEFAULT_PRIORITY, |
BoundNetLog(), GetParam(), NULL); |
helper.RunToCompletion(&data); |
+ spdy_util_.OnStreamDestruction(1); |
TransactionHelperResult out = helper.output(); |
EXPECT_EQ(ERR_SPDY_PROTOCOL_ERROR, out.rv); |
} |
@@ -4647,6 +4685,7 @@ TEST_P(SpdyNetworkTransactionTest, DirectConnectProxyReconnect) { |
EXPECT_EQ(out.rv, ERR_IO_PENDING); |
out.rv = callback.WaitForResult(); |
EXPECT_EQ(out.rv, OK); |
+ spdy_util_.OnStreamDestruction(1); |
const HttpResponseInfo* response = trans->GetResponseInfo(); |
EXPECT_TRUE(response->headers.get() != NULL); |
@@ -4755,10 +4794,14 @@ TEST_P(SpdyNetworkTransactionTest, VerifyRetryOnConnectionReset) { |
scoped_ptr<SpdyFrame> req( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 1, LOWEST, true)); |
+ // In all cases the connection will be reset before req3 can be |
+ // dispatched, destroying both streams. |
+ spdy_util_.OnStreamDestruction(1); |
scoped_ptr<SpdyFrame> req3( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 3, LOWEST, true)); |
MockWrite writes1[] = {CreateMockWrite(*req, 0), CreateMockWrite(*req3, 5)}; |
MockWrite writes2[] = {CreateMockWrite(*req, 0)}; |
+ spdy_util_.OnStreamDestruction(3); |
// This test has a couple of variants. |
enum { |
@@ -4882,6 +4925,8 @@ TEST_P(SpdyNetworkTransactionTest, SpdyBasicAuth) { |
// GET with an Authorization header. |
scoped_ptr<SpdyFrame> req_get( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 1, LOWEST, true)); |
+ // Will be refused for lack of auth. |
+ spdy_util_.OnStreamDestruction(1); |
const char* const kExtraAuthorizationHeaders[] = { |
"authorization", "Basic Zm9vOmJhcg==" |
}; |
@@ -5510,6 +5555,7 @@ TEST_P(SpdyNetworkTransactionTest, ServerPushCrossOriginCorrectness) { |
spdy_util_.ConstructSpdyGet(url_to_fetch, false, 1, LOWEST)); |
scoped_ptr<SpdyFrame> stream1_body( |
spdy_util_.ConstructSpdyBodyFrame(1, true)); |
+ spdy_util_.OnStreamDestruction(1); |
scoped_ptr<SpdyFrame> push_rst( |
spdy_util_.ConstructSpdyRstStream(2, RST_STREAM_REFUSED_STREAM)); |
MockWrite writes[] = { |
@@ -5591,6 +5637,8 @@ TEST_P(SpdyNetworkTransactionTest, RetryAfterRefused) { |
// Construct the request. |
scoped_ptr<SpdyFrame> req( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 1, LOWEST, true)); |
+ // Will be destroyed by the RST before stream 3 starts. |
+ spdy_util_.OnStreamDestruction(1); |
scoped_ptr<SpdyFrame> req2( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 3, LOWEST, true)); |
MockWrite writes[] = { |
@@ -5641,10 +5689,22 @@ TEST_P(SpdyNetworkTransactionTest, OutOfOrderSynStream) { |
// (HIGHEST priority) request in such a way that the third will actually |
// start before the second, causing the second to be numbered differently |
// than the order they were created. |
+ // TODO(rdsmith): Comment above does not match priorities below? |
+ |
+ // Ordering note: the test below will run in three phases: |
+ // * Start req1, run -> seq 0 (i.e. all IO pending). |
+ // * Start req2 & req3, run -> seq 2 (i.e. attempt write of req2 frame, |
+ // return IO pending, run just allowing finish and close of req1). |
+ // * Run -> seq 10 (allow all operations to complete). |
+ // This will result in delaying the creation of the syn frame for |
+ // req3 (happens on first write of req3, blocked by IO_PENDING for req2) |
+ // until after req1 has finished. The OnStreamDestruction call below |
+ // reflects this order. |
scoped_ptr<SpdyFrame> req1( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 1, LOWEST, true)); |
scoped_ptr<SpdyFrame> req2( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 3, HIGHEST, true)); |
+ spdy_util_.OnStreamDestruction(1); |
scoped_ptr<SpdyFrame> req3( |
spdy_util_.ConstructSpdyGet(NULL, 0, false, 5, MEDIUM, true)); |
MockWrite writes[] = { |
@@ -6532,7 +6592,8 @@ INSTANTIATE_TEST_CASE_P( |
Spdy, |
SpdyNetworkTransactionNoTLSUsageCheckTest, |
::testing::Values(SpdyNetworkTransactionTestParams(kProtoSPDY31, |
- HTTPS_SPDY_VIA_NPN))); |
+ HTTPS_SPDY_VIA_NPN, |
+ false))); |
TEST_P(SpdyNetworkTransactionNoTLSUsageCheckTest, TLSVersionTooOld) { |
scoped_ptr<SSLSocketDataProvider> ssl_provider( |
@@ -6575,8 +6636,12 @@ class SpdyNetworkTransactionTLSUsageCheckTest |
INSTANTIATE_TEST_CASE_P( |
Spdy, |
SpdyNetworkTransactionTLSUsageCheckTest, |
- ::testing::Values( |
- SpdyNetworkTransactionTestParams(kProtoHTTP2, HTTPS_SPDY_VIA_NPN))); |
+ ::testing::Values(SpdyNetworkTransactionTestParams(kProtoHTTP2, |
+ HTTPS_SPDY_VIA_NPN, |
+ false), |
+ SpdyNetworkTransactionTestParams(kProtoHTTP2, |
+ HTTPS_SPDY_VIA_NPN, |
+ true))); |
TEST_P(SpdyNetworkTransactionTLSUsageCheckTest, TLSVersionTooOld) { |
scoped_ptr<SSLSocketDataProvider> ssl_provider( |