Check callback is run in QuicStreamFactory::Job

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 6d95f0f1f8747a28769df89f1cce31015f99a7fc..ac150d27d8b148d9af4dbc71c95d70f7b8ba780c 100644
--- a/net/http/http_stream_factory_impl_job_controller_unittest.cc
+++ b/net/http/http_stream_factory_impl_job_controller_unittest.cc
@@ -72,6 +72,8 @@ class FailingHostResolver : public MockHostResolverBase {
   }
 };
 
+// TODO(xunjieli): This should just use HangingHostResolver from
+// mock_host_resolver.h
 class HangingResolver : public MockHostResolverBase {
  public:
   HangingResolver() : MockHostResolverBase(false /*use_caching*/) {}
@@ -899,8 +901,10 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
 
 TEST_F(HttpStreamFactoryImplJobControllerTest, DelayedTCP) {
   base::ScopedMockTimeMessageLoopTaskRunner test_task_runner;
-  HangingResolver* resolver = new HangingResolver();
-  session_deps_.host_resolver.reset(resolver);
+  auto failing_resolver = base::MakeUnique<MockHostResolver>();
+  failing_resolver->set_ondemand_mode(true);
+  failing_resolver->rules()->AddSimulatedFailure("*google.com");
+  session_deps_.host_resolver = std::move(failing_resolver);
 
   HttpRequestInfo request_info;
   request_info.method = "GET";
@@ -948,8 +952,9 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, DelayedTCP) {
   // 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());
+  // Now unblock Resolver so that alternate job (and QuicStreamFactory::Job) can
+  // be cleaned up.
+  session_deps_.host_resolver->ResolveAllPending();

[Zhongyi Shi, 2017/04/03 22:55:39]

nit: could you add EXPECT_FALSE(job_controller_->alternative_job()) after host
resolver being unblocked to verify that alt job falis and is cleaned up?

[xunjieli, 2017/04/04 15:27:29]

Done.

   EXPECT_EQ(1u, test_task_runner->GetPendingTaskCount());
   test_task_runner->FastForwardUntilNoTasksRemain();
 }
@@ -964,8 +969,10 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, DelayedTCPWithLargeSrtt) {
   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);
+  auto failing_resolver = base::MakeUnique<MockHostResolver>();
+  failing_resolver->set_ondemand_mode(true);
+  failing_resolver->rules()->AddSimulatedFailure("*google.com");
+  session_deps_.host_resolver = std::move(failing_resolver);
 
   HttpRequestInfo request_info;
   request_info.method = "GET";
@@ -1005,6 +1012,12 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, DelayedTCPWithLargeSrtt) {
   // main job is resumed.
   test_task_runner->FastForwardBy(kMaxDelayTimeForMainJob);
   EXPECT_FALSE(test_task_runner->HasPendingTask());
+
+  // Now unblock Resolver so that alternate job (and QuicStreamFactory::Job) can
+  // be cleaned up.
+  session_deps_.host_resolver->ResolveAllPending();
+  EXPECT_EQ(1u, test_task_runner->GetPendingTaskCount());

[Zhongyi Shi, 2017/04/03 22:55:39]

ditto

[xunjieli, 2017/04/04 15:27:28]

Done.

+  test_task_runner->FastForwardUntilNoTasksRemain();
 }
 
 TEST_F(HttpStreamFactoryImplJobControllerTest,
@@ -1013,8 +1026,10 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
   // 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);
+  auto failing_resolver = base::MakeUnique<MockHostResolver>();
+  failing_resolver->set_ondemand_mode(true);
+  failing_resolver->rules()->AddSimulatedFailure("*google.com");
+  session_deps_.host_resolver = std::move(failing_resolver);
 
   HttpRequestInfo request_info;
   request_info.method = "GET";
@@ -1052,8 +1067,8 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
 
   // |alternative_job| fails but should not report status to Request.
   EXPECT_CALL(request_delegate_, OnStreamFailed(_, _)).Times(0);
-  job_controller_->OnStreamFailed(job_factory_.alternative_job(),
-                                  ERR_NETWORK_CHANGED, SSLConfig());
+  // Now unblock Resolver to fail the alternate job.
+  session_deps_.host_resolver->ResolveAllPending();

[Zhongyi Shi, 2017/04/03 22:55:39]

ditto

[xunjieli, 2017/04/04 15:27:28]

Done.

   EXPECT_EQ(2u, test_task_runner->GetPendingTaskCount());
 
   // Verify the main job will be resumed immediately.
@@ -1132,9 +1147,10 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, DelayedTCPAlternativeProxy) {
   // could verify the main job is resumed with appropriate delay.
   base::ScopedMockTimeMessageLoopTaskRunner test_task_runner;
 
-  // Using hanging resolver will cause the alternative job to hang indefinitely.
-  HangingResolver* resolver = new HangingResolver();
-  session_deps_.host_resolver.reset(resolver);
+  auto failing_resolver = base::MakeUnique<MockHostResolver>();
+  failing_resolver->set_ondemand_mode(true);
+  failing_resolver->rules()->AddSimulatedFailure("*myproxy.org");
+  session_deps_.host_resolver = std::move(failing_resolver);
 
   UseAlternativeProxy();
 
@@ -1183,6 +1199,12 @@ TEST_F(HttpStreamFactoryImplJobControllerTest, DelayedTCPAlternativeProxy) {
   EXPECT_TRUE(test_proxy_delegate()->alternative_proxy_server().is_valid());
   EXPECT_EQ(1, test_proxy_delegate()->get_alternative_proxy_invocations());
   EXPECT_FALSE(test_task_runner->HasPendingTask());
+
+  // Now unblock Resolver so that alternate job (and QuicStreamFactory::Job) can
+  // be cleaned up.
+  session_deps_.host_resolver->ResolveAllPending();
+  EXPECT_EQ(1u, test_task_runner->GetPendingTaskCount());
+  test_task_runner->FastForwardUntilNoTasksRemain();

[Zhongyi Shi, 2017/04/03 22:55:39]

ditto

[xunjieli, 2017/04/04 15:27:28]

Done.

 }
 
 // Verifies that the alternative proxy server job fails immediately, and the