Clean up HttpStreamFactoryImpl::JobController when Impl::Jobs complete

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 83049f512854e7646367d832cc64beafb971da9f..d1b185d52bea53bba01ad277658d703b61caad82 100644
--- a/net/http/http_stream_factory_impl_job_controller_unittest.cc
+++ b/net/http/http_stream_factory_impl_job_controller_unittest.cc
@@ -371,6 +371,75 @@ TEST_F(HttpStreamFactoryImplJobControllerTest,
   EXPECT_TRUE(HttpStreamFactoryImplPeer::IsJobControllerDeleted(factory_));
 }
 
+// Regression test for crbug.com/678768.
+TEST_F(HttpStreamFactoryImplJobControllerTest,
+       AltJobSucceedsControllerDestroyed) {
+  ProxyConfig proxy_config;
+  proxy_config.set_auto_detect(true);
+  // Use asynchronous proxy resolver.
+  MockAsyncProxyResolverFactory* proxy_resolver_factory =
+      new MockAsyncProxyResolverFactory(false);
+  session_deps_.proxy_service.reset(
+      new ProxyService(base::MakeUnique<ProxyConfigServiceFixed>(proxy_config),
+                       base::WrapUnique(proxy_resolver_factory), nullptr));
+  Initialize(false);
+
+  HttpRequestInfo request_info;
+  request_info.method = "GET";
+  request_info.url = GURL("https://www.google.com");
+
+  url::SchemeHostPort server(request_info.url);
+  AlternativeService alternative_service(kProtoQUIC, server.host(), 443);
+  SetAlternativeService(request_info, alternative_service);
+  // Hack to use different URL for the main job to help differentiate the proxy
+  // requests.
+  job_factory_.UseDifferentURLForMainJob(GURL("http://www.google.com"));
+  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(JobControllerPeer::main_job_is_blocked(job_controller_));
+
+  base::RunLoop().RunUntilIdle();
+  MockAsyncProxyResolver resolver;
+  ASSERT_EQ(1u, proxy_resolver_factory->pending_requests().size());
+  proxy_resolver_factory->pending_requests()[0]->CompleteNowWithForwarder(
+      net::OK, &resolver);
+
+  // Resolve proxy for the main job which then proceed to wait for the
+  // alternative job which is IO_PENDING.
+  ASSERT_EQ(2u, resolver.pending_jobs().size());
+  int main_job_request_id =
+      resolver.pending_jobs()[0]->url().SchemeIs("http") ? 0 : 1;
+  resolver.pending_jobs()[main_job_request_id]->results()->UseNamedProxy(
+      "result1:80");
+  resolver.pending_jobs()[main_job_request_id]->CompleteNow(net::OK);
+  EXPECT_TRUE(job_controller_->main_job()->is_waiting());
+
+  ASSERT_EQ(1u, resolver.pending_jobs().size());
+  // Resolve proxy for the alternative job to proceed to create a connection.
+  // Use failing HostResolver to fail creation of a QUIC session for the
+  // alternative job. The alternative job will thus resume the main job.
+  resolver.pending_jobs()[0]->results()->UseNamedProxy("result1:80");
+  resolver.pending_jobs()[0]->CompleteNow(net::OK);

[xunjieli, 2017/01/06 17:55:59]

Note: The existing test fixture is a bit weird. QUIC proxy doesn't work yet, and
here we use QUIC to speak to a non-QUIC proxy, which is prohibited. Therefore we
will end up getting ERR_NO_SUPPORTED_PROXIES in
HttpStreamFactoryImpl::Job::DoResolveProxyComplete for the alt job, but we
forcefully call job_controller_->OnStreamReady() for the alt job in the test..
If we run the message loop, we will later get OnStreamFailed() for the alt job
because of ERR_NO_SUPPORTED_PROXIES. I find this setup weird, but I don't know a
way to work around it.

+
+  // |alternative_job| succeeds and should report status to Request.
+  HttpStream* http_stream =
+      new HttpBasicStream(base::MakeUnique<ClientSocketHandle>(), false, false);
+  job_factory_.alternative_job()->SetStream(http_stream);
+  EXPECT_CALL(request_delegate_, OnStreamReady(_, _, http_stream))
+      .WillOnce(Invoke(DeleteHttpStreamPointer));
+  job_controller_->OnStreamReady(job_factory_.alternative_job(), SSLConfig());
+
+  EXPECT_TRUE(job_controller_->main_job()->is_waiting());
+  request_.reset();
+  VerifyBrokenAlternateProtocolMapping(request_info, false);
+  // This fails without the fix for crbug.com/678768.
+  EXPECT_TRUE(HttpStreamFactoryImplPeer::IsJobControllerDeleted(factory_));
+}
+
 TEST_F(HttpStreamFactoryImplJobControllerTest,
        AltJobSucceedsAfterMainJobFailed) {
   ProxyConfig proxy_config;