JobController3: Move MarkAlternativeServiceBroken to job controller

net/http/http_stream_factory_impl_job.cc

Index: net/http/http_stream_factory_impl_job.cc
diff --git a/net/http/http_stream_factory_impl_job.cc b/net/http/http_stream_factory_impl_job.cc
index 481a740d2a6bd00e9f7767606f4a4020524eca9d..c22314c5905fcd00e4c1679c212ffa36a6cd9b8c 100644
--- a/net/http/http_stream_factory_impl_job.cc
+++ b/net/http/http_stream_factory_impl_job.cc
@@ -581,8 +581,6 @@ int HttpStreamFactoryImpl::Job::RunLoop(int result) {
     }
 
     case OK:
-      job_status_ = STATUS_SUCCEEDED;
-      MaybeMarkAlternativeServiceBroken();
       next_state_ = STATE_DONE;
       if (new_spdy_session_.get()) {
         base::ThreadTaskRunnerHandle::Get()->PostTask(
@@ -613,10 +611,9 @@ int HttpStreamFactoryImpl::Job::RunLoop(int result) {
       return ERR_IO_PENDING;
 
     default:
-      if (job_status_ != STATUS_BROKEN) {
-        DCHECK_EQ(STATUS_RUNNING, job_status_);
-        job_status_ = STATUS_FAILED;
-        MaybeMarkAlternativeServiceBroken();
+      // Notify job controlelr that alternative proxy server is broken.
+      if (alternative_proxy_server_.is_valid()) {
+        delegate_->OnAlternativeProxyServerBroken(this);

[Ryan Hamilton, 2016/09/13 04:17:04]

Instead of requiring the job to explicitly notify the delegate of broken-ness,
it seems like it would be cleaner to have the controller figure this out via
OnStreamFailed (called from OnStreamFailedCallback()). Would that work?

       }
       base::ThreadTaskRunnerHandle::Get()->PostTask(
           FROM_HERE, base::Bind(&Job::OnStreamFailedCallback,
@@ -1065,22 +1062,34 @@ int HttpStreamFactoryImpl::Job::DoInitConnectionComplete(int result) {
   }
 
   if (IsSpdyAlternative() && !using_spdy_) {
-    job_status_ = STATUS_BROKEN;
-    MaybeMarkAlternativeServiceBroken();
+    if (alternative_proxy_server_.is_valid()) {
+      // |this| connected to the alternative proxy server.
+      delegate_->OnAlternativeProxyServerBroken(this);
+    } else {
+      delegate_->OnAlternativeServiceBroken(this);
+    }
     return ERR_NPN_NEGOTIATION_FAILED;
   }
 
   if (!ssl_started && result < 0 &&
       (IsSpdyAlternative() || IsQuicAlternative())) {
-    job_status_ = STATUS_BROKEN;
-    MaybeMarkAlternativeServiceBroken();
+    if (alternative_proxy_server_.is_valid()) {
+      // |this| connected to the alternative proxy server.
+      delegate_->OnAlternativeProxyServerBroken(this);
+    } else {
+      delegate_->OnAlternativeServiceBroken(this);
+    }
     return result;
   }
 
   if (using_quic_) {
     if (result < 0) {
-      job_status_ = STATUS_BROKEN;
-      MaybeMarkAlternativeServiceBroken();
+      if (alternative_proxy_server_.is_valid()) {
+        // |this| connected to the alternative proxy server.
+        delegate_->OnAlternativeProxyServerBroken(this);
+      } else {
+        delegate_->OnAlternativeServiceBroken(this);
+      }
       return result;
     }
     if (stream_type_ == HttpStreamRequest::BIDIRECTIONAL_STREAM) {
@@ -1520,93 +1529,6 @@ void HttpStreamFactoryImpl::Job::ReportJobSucceededForRequest() {
   }
 }
 
-void HttpStreamFactoryImpl::Job::MarkOtherJobComplete(const Job& job) {
-  DCHECK_EQ(STATUS_RUNNING, other_job_status_);
-  DCHECK(!other_job_alternative_proxy_server_.is_valid());
-
-  other_job_status_ = job.job_status_;
-  other_job_alternative_service_ = job.alternative_service_;
-  other_job_alternative_proxy_server_ = job.alternative_proxy_server_;
-
-  // At most one job can have a valid |alternative_proxy_server_|.
-  DCHECK(!alternative_proxy_server_.is_valid() ||
-         !other_job_alternative_proxy_server_.is_valid());
-
-  MaybeMarkAlternativeServiceBroken();
-}
-
-void HttpStreamFactoryImpl::Job::MaybeMarkAlternativeServiceBroken() {
-  // At least one job should not be an alternative job.
-  DCHECK(alternative_service_.protocol == UNINITIALIZED_ALTERNATE_PROTOCOL ||
-         other_job_alternative_service_.protocol ==
-             UNINITIALIZED_ALTERNATE_PROTOCOL);
-
-  if (job_status_ == STATUS_RUNNING || other_job_status_ == STATUS_RUNNING)
-    return;
-
-  MaybeNotifyAlternativeProxyServerBroken();
-
-  if (IsSpdyAlternative() || IsQuicAlternative()) {
-    if (job_status_ == STATUS_BROKEN && other_job_status_ == STATUS_SUCCEEDED) {
-      HistogramBrokenAlternateProtocolLocation(
-          BROKEN_ALTERNATE_PROTOCOL_LOCATION_HTTP_STREAM_FACTORY_IMPL_JOB_ALT);
-      session_->http_server_properties()->MarkAlternativeServiceBroken(
-          alternative_service_);
-    }
-    return;
-  }
-
-  session_->quic_stream_factory()->OnTcpJobCompleted(job_status_ ==
-                                                     STATUS_SUCCEEDED);
-  if (job_status_ == STATUS_SUCCEEDED && other_job_status_ == STATUS_BROKEN) {
-    HistogramBrokenAlternateProtocolLocation(
-        BROKEN_ALTERNATE_PROTOCOL_LOCATION_HTTP_STREAM_FACTORY_IMPL_JOB_MAIN);
-    session_->http_server_properties()->MarkAlternativeServiceBroken(
-        other_job_alternative_service_);
-  }
-}
-
-void HttpStreamFactoryImpl::Job::MaybeNotifyAlternativeProxyServerBroken()
-    const {
-  if (!alternative_proxy_server_.is_valid() &&
-      !other_job_alternative_proxy_server_.is_valid()) {
-    // Neither of the two jobs used an alternative proxy server.
-    return;
-  }
-
-  // Neither this job, nor the other job should have used the alternative
-  // service.
-  DCHECK_EQ(UNINITIALIZED_ALTERNATE_PROTOCOL, alternative_service_.protocol);
-  DCHECK_EQ(UNINITIALIZED_ALTERNATE_PROTOCOL,
-            other_job_alternative_service_.protocol);
-
-  ProxyDelegate* proxy_delegate = session_->params().proxy_delegate;
-  if (!proxy_delegate)
-    return;
-
-  if (alternative_proxy_server_.is_valid()) {
-    // |this| connected to the alternative proxy server.
-    if ((job_status_ == STATUS_BROKEN || job_status_ == STATUS_FAILED) &&
-        other_job_status_ == STATUS_SUCCEEDED) {
-      // Notify ProxyDelegate.
-      proxy_delegate->OnAlternativeProxyBroken(alternative_proxy_server_);
-    }
-    return;
-  }
-
-  if (other_job_alternative_proxy_server_.is_valid()) {
-    // Other job connected to the alternative proxy server.
-    if (job_status_ == STATUS_SUCCEEDED &&
-        (other_job_status_ == STATUS_BROKEN ||
-         other_job_status_ == STATUS_FAILED)) {
-      // Notify ProxyDelegate.
-      proxy_delegate->OnAlternativeProxyBroken(
-          other_job_alternative_proxy_server_);
-    }
-    return;
-  }
-}
-
 ClientSocketPoolManager::SocketGroupType
 HttpStreamFactoryImpl::Job::GetSocketGroup() const {
   std::string scheme = origin_url_.scheme();