JobController 2: Remove reference between HttpStreamFactoryImpl::Jobs.

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 bd3f2e3bd7cdad801af6e1202c231e9f134b909f..3230168c66b86b06f5783a0ec48e4b034c044724 100644
--- a/net/http/http_stream_factory_impl_job_controller.cc
+++ b/net/http/http_stream_factory_impl_job_controller.cc
@@ -26,6 +26,7 @@ HttpStreamFactoryImpl::JobController::JobController(
       delegate_(delegate),
       is_preconnect_(false),
       job_bound_(false),
+      main_job_is_blocked_(false),
       bound_job_(nullptr) {
   DCHECK(factory);
 }
@@ -223,6 +224,8 @@ void HttpStreamFactoryImpl::JobController::OnStreamFailed(
     int status,
     const SSLConfig& used_ssl_config,
     SSLFailureState ssl_failure_state) {
+  MaybeResumeMainJob(job, base::TimeDelta());
+
   if (job_bound_ && bound_job_ != job) {
     // We have bound a job to the associated Request, |job| has been orphaned.
     OnOrphanedJobComplete(job);
@@ -262,6 +265,8 @@ void HttpStreamFactoryImpl::JobController::OnCertificateError(
     int status,
     const SSLConfig& used_ssl_config,
     const SSLInfo& ssl_info) {
+  MaybeResumeMainJob(job, base::TimeDelta());
+
   if (job_bound_ && bound_job_ != job) {
     // We have bound a job to the associated Request, |job| has been orphaned.
     OnOrphanedJobComplete(job);
@@ -283,6 +288,8 @@ void HttpStreamFactoryImpl::JobController::OnHttpsProxyTunnelResponse(
     const SSLConfig& used_ssl_config,
     const ProxyInfo& used_proxy_info,
     HttpStream* stream) {
+  MaybeResumeMainJob(job, base::TimeDelta());
+
   if (job_bound_ && bound_job_ != job) {
     // We have bound a job to the associated Request, |job| has been orphaned.
     OnOrphanedJobComplete(job);
@@ -301,6 +308,8 @@ void HttpStreamFactoryImpl::JobController::OnNeedsClientAuth(
     Job* job,
     const SSLConfig& used_ssl_config,
     SSLCertRequestInfo* cert_info) {
+  MaybeResumeMainJob(job, base::TimeDelta());
+
   if (job_bound_ && bound_job_ != job) {
     // We have bound a job to the associated Request, |job| has been orphaned.
     OnOrphanedJobComplete(job);
@@ -320,6 +329,8 @@ void HttpStreamFactoryImpl::JobController::OnNeedsProxyAuth(
     const SSLConfig& used_ssl_config,
     const ProxyInfo& used_proxy_info,
     HttpAuthController* auth_controller) {
+  MaybeResumeMainJob(job, base::TimeDelta());
+
   if (job_bound_ && bound_job_ != job) {
     // We have bound a job to the associated Request, |job| has been orphaned.
     OnOrphanedJobComplete(job);
@@ -399,6 +410,7 @@ void HttpStreamFactoryImpl::JobController::OnNewSpdySessionReady(
 
 void HttpStreamFactoryImpl::JobController::OnPreconnectsComplete(Job* job) {
   DCHECK_EQ(main_job_.get(), job);
+  DCHECK(!bound_job_);
   main_job_.reset();
   factory_->OnPreconnectsCompleteInternal();
   MaybeNotifyFactoryOfCompletion();
@@ -427,6 +439,44 @@ void HttpStreamFactoryImpl::JobController::AddConnectionAttemptsToRequest(
   request_->AddConnectionAttempts(attempts);
 }
 
+void HttpStreamFactoryImpl::JobController::MaybeResumeMainJob(
+    Job* job,
+    const base::TimeDelta& delay) {
+  DCHECK(job == main_job_.get() || job == alternative_job_.get());
+  if (!main_job_is_blocked_)
+    return;
+
+  if (job == alternative_job_.get() && main_job_) {
+    main_job_is_blocked_ = false;
+    main_job_->Resume(delay);
+  }
+}
+
+void HttpStreamFactoryImpl::JobController::OnConnectionInitialized(Job* job,
+                                                                   int rv) {
+  if (rv != OK && rv != ERR_SPDY_SESSION_ALREADY_EXISTS) {

[Ryan Hamilton, 2016/07/01 00:13:30]

I'm surprised to see the SPDY_SESSION_ALREADY_EXISTS error here. Can you say
more about this?

[Zhongyi Shi, 2016/07/11 23:04:57]

Sure. This is refactored from the old code. In the old code, we will resume the
waiting job in DoInitConnectionComplete(). We will resume the main job if we are
using_quic AND the DoInitConnection is NOT OK. In addition, we will also resume
if we got any error when creating SPDY session.  

The check for SPDY_SESSION_ALREADY_EXISTS prevents us to incorrectly resume the
job. 
https://cs.chromium.org/chromium/src/net/http/http_stream_factory_impl_job.cc...

+    // Resume the main job as there's an error raised in connection
+    // initiation except the case where we find an existing SPDY session for
+    // this Job.
+    return MaybeResumeMainJob(job, main_job_wait_time_);
+  }
+}
+
+bool HttpStreamFactoryImpl::JobController::ShouldWait(Job* job) const {
+  base::WeakPtrFactory<Job> ptr_factory(job);
+  // The alternative job never waits.
+  if (job == alternative_job_.get())
+    return false;
+
+  if (!main_job_is_blocked_ && main_job_wait_time_.is_zero())
+    return false;
+
+  if (!main_job_wait_time_.is_zero()) {
+    job->Resume(main_job_wait_time_);
+  }
+  return true;
+}
+
 void HttpStreamFactoryImpl::JobController::SetSpdySessionKey(
     Job* job,
     const SpdySessionKey& spdy_session_key) {
@@ -476,6 +526,20 @@ const BoundNetLog* HttpStreamFactoryImpl::JobController::GetNetLog(
   return &request_->net_log();
 }
 
+bool HttpStreamFactoryImpl::JobController::main_job_is_blocked() {
+  return main_job_is_blocked_;
+}
+
+const base::TimeDelta&
+HttpStreamFactoryImpl::JobController::wait_time_for_main_job() const {
+  return main_job_wait_time_;
+}
+
+void HttpStreamFactoryImpl::JobController::set_wait_time_for_main_job(
+    const base::TimeDelta& delay) {
+  main_job_wait_time_ = delay;
+}
+
 WebSocketHandshakeStreamBase::CreateHelper* HttpStreamFactoryImpl::
     JobController::websocket_handshake_stream_create_helper() {
   DCHECK(request_);
@@ -521,10 +585,7 @@ void HttpStreamFactoryImpl::JobController::CreateJobs(
         alternative_service, net_log.net_log()));
     AttachJob(alternative_job_.get());
 
-    main_job_->WaitFor(alternative_job_.get());
-    // Make sure to wait until we call WaitFor(), before starting
-    // |alternative_job|, otherwise |alternative_job| will not notify |job|
-    // appropriately.
+    main_job_is_blocked_ = true;
     alternative_job_->Start(request_->stream_type());
   }
   // Even if |alternative_job| has already finished, it will not have notified