Cap the delay time for the main job when racing with QUIC for delayed

net/http/http_stream_factory_impl_job_controller_unittest.cc

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..bb55d6eaaf62fb86eba57f682282bc90420a54a7 100644
--- a/net/http/http_stream_factory_impl_job_controller_unittest.cc
+++ b/net/http/http_stream_factory_impl_job_controller_unittest.cc
@@ -11,6 +11,7 @@
 #include "base/run_loop.h"
 #include "base/test/histogram_tester.h"
 #include "base/test/scoped_feature_list.h"
+#include "base/test/scoped_mock_time_message_loop_task_runner.h"
 #include "base/threading/platform_thread.h"
 #include "net/base/test_proxy_delegate.h"
 #include "net/dns/mock_host_resolver.h"
@@ -886,6 +887,7 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
 }
 
 TEST_F(HttpStreamFactoryImplJobControllerTest, DelayedTCP) {
+  base::ScopedMockTimeMessageLoopTaskRunner test_task_runner;
   HangingResolver* resolver = new HangingResolver();
   session_deps_.host_resolver.reset(resolver);
 
@@ -919,17 +921,11 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, DelayedTCP) {
 
   // 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_,

[Ryan Hamilton, 2017/02/16 00:30:58]

Do we need this method any more? If not, feel free to land a followup to nuke
it.

[Zhongyi Shi, 2017/02/16 00:33:38]

Not any more! I'll do a clean up CL to change those tests to use test task
runner.

[Ryan Hamilton, 2017/02/16 00:39:26]

\o/

-              base::TimeDelta::FromMicroseconds(15))),
-          testing::Invoke([&run_loop]() { run_loop.Quit(); })));
-
-  // Wait for the main job to be resumed.
-  run_loop.Run();
+  EXPECT_TRUE(test_task_runner->HasPendingTask());
+  EXPECT_EQ(1u, test_task_runner->GetPendingTaskCount());
+  EXPECT_CALL(*job_factory_.main_job(), Resume()).Times(1);
+  test_task_runner->FastForwardBy(base::TimeDelta::FromMicroseconds(15));
+  EXPECT_FALSE(test_task_runner->HasPendingTask());
 
   EXPECT_TRUE(job_controller_->main_job());
   EXPECT_TRUE(job_controller_->alternative_job());
@@ -938,16 +934,74 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, DelayedTCP) {
   EXPECT_CALL(request_delegate_, OnStreamFailed(_, _)).Times(0);
 
   EXPECT_FALSE(JobControllerPeer::main_job_is_blocked(job_controller_));
-  // OnStreamFailed will not resume the main job again since it's been resumed
-  // already.
+  // OnStreamFailed will post a task to resume the main job immediately but
+  // won't call Resume() on the main job since it's been resumed already.
   EXPECT_CALL(*job_factory_.main_job(), Resume()).Times(0);
-
   job_controller_->OnStreamFailed(job_factory_.alternative_job(),
                                   ERR_NETWORK_CHANGED, SSLConfig());
+  EXPECT_EQ(1u, test_task_runner->GetPendingTaskCount());
+  test_task_runner->RunUntilIdle();
+}
+
+// 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) {
+  // Overrides the main thread's message loop with a mock tick clock so that we
+  // could verify the main job is resumed with appropriate delay.
+  base::ScopedMockTimeMessageLoopTaskRunner test_task_runner;
+  // 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.
+  EXPECT_TRUE(test_task_runner->HasPendingTask());
+  EXPECT_EQ(1u, test_task_runner->GetPendingTaskCount());
+
+  EXPECT_CALL(*job_factory_.main_job(), Resume()).Times(1);
+  // Move forward the task runner with kMaxDelayTimeForMainJob and verify the
+  // main job is resumed.
+  test_task_runner->FastForwardBy(kMaxDelayTimeForMainJob);
+  EXPECT_FALSE(test_task_runner->HasPendingTask());
 }
 
 TEST_F(HttpStreamFactoryImplJobControllerTest,
        ResumeMainJobImmediatelyOnStreamFailed) {
+  // Overrides the main thread's message loop with a mock tick clock so that we
+  // could verify the main job is resumed with appropriate delay.
+  base::ScopedMockTimeMessageLoopTaskRunner test_task_runner;
+
   HangingResolver* resolver = new HangingResolver();
   session_deps_.host_resolver.reset(resolver);
 
@@ -971,6 +1025,9 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
   AlternativeService alternative_service(kProtoQUIC, server.host(), 443);
   SetAlternativeService(request_info, alternative_service);
 
+  // The alternative job stalls as host resolution hangs when creating the QUIC
+  // request and controller should resume the main job with delay.
+  // OnStreamFailed should resume the main job immediately.
   request_.reset(
       job_controller_->Start(request_info, &request_delegate_, nullptr,
                              NetLogWithSource(), HttpStreamRequest::HTTP_STREAM,
@@ -979,23 +1036,30 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
   EXPECT_TRUE(job_controller_->alternative_job());
   EXPECT_TRUE(job_controller_->main_job()->is_waiting());
 
+  EXPECT_TRUE(test_task_runner->HasPendingTask());
+  EXPECT_EQ(1u, test_task_runner->GetPendingTaskCount());
+
   // |alternative_job| fails but should not report status to Request.
   EXPECT_CALL(request_delegate_, OnStreamFailed(_, _)).Times(0);
-
-  // The alternative job stalls as host resolution hangs when creating the QUIC
-  // request and controller should resume the main job with delay.
-  // OnStreamFailed should resume the main job immediately.
-  EXPECT_CALL(*job_factory_.main_job(), Resume())
-      .Times(1)
-      .WillOnce(Invoke(testing::CreateFunctor(
-          &JobControllerPeer::VerifyWaitingTimeForMainJob, job_controller_,
-          base::TimeDelta::FromMicroseconds(0))));
-
   job_controller_->OnStreamFailed(job_factory_.alternative_job(),
                                   ERR_NETWORK_CHANGED, SSLConfig());
+  EXPECT_EQ(2u, test_task_runner->GetPendingTaskCount());
 
-  base::RunLoop().RunUntilIdle();
+  // Verify the main job will be resumed immediately.
+  EXPECT_CALL(*job_factory_.main_job(), Resume()).Times(1);
+  // Execute tasks that have no remaining delay. Tasks with nonzero delay will
+  // remain queued.
+  test_task_runner->RunUntilIdle();
+
+  // Verify there is another task to resume main job with delay but should
+  // not call Resume() on the main job as main job has been resumed.
+  EXPECT_TRUE(test_task_runner->HasPendingTask());
+  EXPECT_EQ(1u, test_task_runner->GetPendingTaskCount());
+  EXPECT_CALL(*job_factory_.main_job(), Resume()).Times(0);
+  test_task_runner->FastForwardBy(base::TimeDelta::FromMicroseconds(15));
+  EXPECT_FALSE(test_task_runner->HasPendingTask());
 }
+
 // Verifies that the alternative proxy server job is not created if the URL
 // scheme is HTTPS.
 TEST_F(HttpStreamFactoryImplJobControllerTest, HttpsURL) {