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

Unified Diff: net/http/http_stream_factory_impl_job_controller_unittest.cc

Issue 2690393005: Cap the delay time for the main job when racing with QUIC for delayed (Closed)
Patch Set: add comments Created 3 years, 10 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
Index: net/http/http_stream_factory_impl_job_controller_unittest.cc
diff --git a/net/http/http_stream_factory_impl_job_controller_unittest.cc b/net/http/http_stream_factory_impl_job_controller_unittest.cc
index 8327b2f99db272b44f688921aaf0e694cf672295..d0184d02fd4589dbb00abac52ae4343de41fe0ba 100644
--- a/net/http/http_stream_factory_impl_job_controller_unittest.cc
+++ b/net/http/http_stream_factory_impl_job_controller_unittest.cc
@@ -946,6 +946,59 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, DelayedTCP) {
ERR_NETWORK_CHANGED, SSLConfig());
}
+// Test that main job is blocked for kMaxDelayTimeForMainJob(3s) if
+// http_server_properties cached an inappropriate large srtt for the server,
+// which would potentially delay the main job for a extremely long time in
+// delayed tcp case.
+TEST_F(HttpStreamFactoryImplJobControllerTest, DelayedTCPWithLargeSrtt) {
+ // The max delay time should be in sync with .cc file.
+ base::TimeDelta kMaxDelayTimeForMainJob = base::TimeDelta::FromSeconds(3);
+ HangingResolver* resolver = new HangingResolver();
+ session_deps_.host_resolver.reset(resolver);
+
+ HttpRequestInfo request_info;
+ request_info.method = "GET";
+ request_info.url = GURL("https://www.google.com");
+
+ Initialize(request_info, false, false);
+
+ // Enable delayed TCP and set a extremely large time delay for waiting job.
+ QuicStreamFactory* quic_stream_factory = session_->quic_stream_factory();
+ test::QuicStreamFactoryPeer::SetDelayTcpRace(quic_stream_factory, true);
+ quic_stream_factory->set_require_confirmation(false);
+ ServerNetworkStats stats1;
+ stats1.srtt = base::TimeDelta::FromSeconds(100);
+ session_->http_server_properties()->SetServerNetworkStats(
+ url::SchemeHostPort(GURL("https://www.google.com")), stats1);
+
+ // Set a SPDY alternative service for the server.
+ url::SchemeHostPort server(request_info.url);
+ AlternativeService alternative_service(kProtoQUIC, server.host(), 443);
+ SetAlternativeService(request_info, alternative_service);
+
+ request_.reset(
+ job_controller_->Start(request_info, &request_delegate_, nullptr,
+ NetLogWithSource(), HttpStreamRequest::HTTP_STREAM,
+ DEFAULT_PRIORITY, SSLConfig(), SSLConfig()));
+ EXPECT_TRUE(job_controller_->main_job());
+ EXPECT_TRUE(job_controller_->alternative_job());
+ EXPECT_TRUE(job_controller_->main_job()->is_waiting());
+
+ // The alternative job stalls as host resolution hangs when creating the QUIC
+ // request and controller should resume the main job after delay.
+ base::RunLoop run_loop;
+ EXPECT_CALL(*job_factory_.main_job(), Resume())
+ .Times(1)
+ .WillOnce(
+ testing::DoAll(testing::Invoke(testing::CreateFunctor(
+ &JobControllerPeer::VerifyWaitingTimeForMainJob,
+ job_controller_, kMaxDelayTimeForMainJob)),
Ryan Hamilton 2017/02/15 22:26:55 I don't' think I understand how this tests that th
+ testing::Invoke([&run_loop]() { run_loop.Quit(); })));
+
+ // Wait for the main job to be resumed.
+ run_loop.Run();
+}
Ryan Hamilton 2017/02/15 22:26:55 To confirm, this test fails with the old code?
Zhongyi Shi 2017/02/15 23:32:33 Yup.
+
TEST_F(HttpStreamFactoryImplJobControllerTest,
ResumeMainJobImmediatelyOnStreamFailed) {
HangingResolver* resolver = new HangingResolver();

Powered by Google App Engine
This is Rietveld 408576698