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

net/http/http_stream_factory_impl_job_controller.cc

Index: net/http/http_stream_factory_impl_job_controller.cc
diff --git a/net/http/http_stream_factory_impl_job_controller.cc b/net/http/http_stream_factory_impl_job_controller.cc
index 7e76ce5ce3436767ba5f47262d75ec4dbcaf16b8..686a68f7ca3ea9bc2cf406a80a915e95ce45262b 100644
--- a/net/http/http_stream_factory_impl_job_controller.cc
+++ b/net/http/http_stream_factory_impl_job_controller.cc
@@ -143,12 +143,10 @@ void HttpStreamFactoryImpl::JobController::OnRequestComplete() {
   DCHECK(request_);
   request_ = nullptr;
   if (bound_job_) {
-    if (bound_job_->job_type() == MAIN) {
-      main_job_.reset();
-    } else {
-      DCHECK(bound_job_->job_type() == ALTERNATIVE);
-      alternative_job_.reset();
-    }
+    DCHECK(bound_job_->job_type() == MAIN ||
+           bound_job_->job_type() == ALTERNATIVE);
+    alternative_job_.reset();
+    main_job_.reset();

[Wez, 2017/01/06 18:21:44]

So IIUC the issue was that MaybeNotifyFactoryOfCompletion() would never notify
the factory if the alternative job completed, because the main_job is still
non-null, hence the JobController and Job, etc would be leaked?

Can we make that clearer in the CL description?

[Ryan Sleevi, 2017/01/06 20:24:13]

A big +1 to that. A longer CL description that covers the bug, how it
manifested, and what edge cases are being resolved would be a great idea :)

     bound_job_ = nullptr;
   }
   MaybeNotifyFactoryOfCompletion();