JobController3: Move MarkAlternativeServiceBroken to job controller

net/http/http_stream_factory_impl_job_controller.cc

 // Copyright (c) 2016 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/http/http_stream_factory_impl_job_controller.h"
 
 #include "base/metrics/histogram_macros.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/values.h"
@@ skipping 20 matching lines @@
     HttpStreamFactoryImpl* factory,
     HttpStreamRequest::Delegate* delegate,
     HttpNetworkSession* session,
     JobFactory* job_factory)
     : factory_(factory),
       session_(session),
       job_factory_(job_factory),
       request_(nullptr),
       delegate_(delegate),
       is_preconnect_(false),
+      alternative_service_is_broken_(false),
       job_bound_(false),
       main_job_is_blocked_(false),
       bound_job_(nullptr),
       can_start_alternative_proxy_job_(false),
       ptr_factory_(this) {
   DCHECK(factory);
 }
 
 HttpStreamFactoryImpl::JobController::~JobController() {
   main_job_.reset();
@@ skipping 107 matching lines @@
   if (alternative_job_) {
     alternative_job_->SetPriority(priority);
   }
 }
 
 void HttpStreamFactoryImpl::JobController::OnStreamReady(
     Job* job,
     const SSLConfig& used_ssl_config,
     const ProxyInfo& used_proxy_info) {
   DCHECK(job);
+  MaybeReportAlternativeServiceBroken(job);
 
   if (job_bound_ && bound_job_ != job) {
     // We have bound a job to the associated Request, |job| has been orphaned.
     OnOrphanedJobComplete(job);
     return;
   }
   std::unique_ptr<HttpStream> stream = job->ReleaseStream();
   DCHECK(stream);
 
   MarkRequestComplete(job->was_npn_negotiated(), job->negotiated_protocol(),
                       job->using_spdy());
 
   if (!request_)
     return;
   DCHECK(!factory_->for_websockets_);
   DCHECK_EQ(HttpStreamRequest::HTTP_STREAM, request_->stream_type());
   OnJobSucceeded(job);
   request_->OnStreamReady(used_ssl_config, used_proxy_info, stream.release());
 }
 
 void HttpStreamFactoryImpl::JobController::OnBidirectionalStreamImplReady(
     Job* job,
     const SSLConfig& used_ssl_config,
     const ProxyInfo& used_proxy_info) {
   DCHECK(job);
+  MaybeReportAlternativeServiceBroken(job);
 
   if (job_bound_ && bound_job_ != job) {
     // We have bound a job to the associated Request, |job| has been orphaned.
     OnOrphanedJobComplete(job);
     return;
   }
 
   MarkRequestComplete(job->was_npn_negotiated(), job->negotiated_protocol(),
                       job->using_spdy());
 
   if (!request_)
     return;
   std::unique_ptr<BidirectionalStreamImpl> stream =
       job->ReleaseBidirectionalStream();
   DCHECK(stream);
   DCHECK(!factory_->for_websockets_);
   DCHECK_EQ(HttpStreamRequest::BIDIRECTIONAL_STREAM, request_->stream_type());
 
   OnJobSucceeded(job);
   request_->OnBidirectionalStreamImplReady(used_ssl_config, used_proxy_info,
                                            stream.release());
 }
 
 void HttpStreamFactoryImpl::JobController::OnWebSocketHandshakeStreamReady(
     Job* job,
     const SSLConfig& used_ssl_config,
     const ProxyInfo& used_proxy_info,
     WebSocketHandshakeStreamBase* stream) {
   DCHECK(job);
+  MaybeReportAlternativeServiceBroken(job);
 
   MarkRequestComplete(job->was_npn_negotiated(), job->negotiated_protocol(),
                       job->using_spdy());
 
   if (!request_)
     return;
   DCHECK(factory_->for_websockets_);
   DCHECK_EQ(HttpStreamRequest::HTTP_STREAM, request_->stream_type());
   DCHECK(stream);
 
@@ skipping 17 matching lines @@
   if (!request_)
     return;
   DCHECK_NE(OK, status);
   DCHECK(job);
 
   if (!bound_job_) {
     if (main_job_ && alternative_job_) {
       // Hey, we've got other jobs! Maybe one of them will succeed, let's just
       // ignore this failure.
       factory_->request_map_.erase(job);
-      // Notify all the other jobs that this one failed.
       if (job->job_type() == MAIN) {
-        alternative_job_->MarkOtherJobComplete(*job);
         main_job_.reset();
       } else {
         DCHECK(job->job_type() == ALTERNATIVE);
-        main_job_->MarkOtherJobComplete(*job);
         alternative_job_.reset();
       }
       return;
     } else {
       BindJob(job);
     }
   }
 
   request_->OnStreamFailed(status, used_ssl_config);
 }
@@ skipping 121 matching lines @@
                             base::Unretained(alternative_job_.get()),
                             request_->stream_type()));
 }
 
 void HttpStreamFactoryImpl::JobController::OnNewSpdySessionReady(
     Job* job,
     const base::WeakPtr<SpdySession>& spdy_session,
     bool direct) {
   DCHECK(job);
   DCHECK(job->using_spdy());
+  MaybeReportAlternativeServiceBroken(job);
 
   bool is_job_orphaned = job_bound_ && bound_job_ != job;
 
   // Cache these values in case the job gets deleted.
   const SSLConfig used_ssl_config = job->server_ssl_config();
   const ProxyInfo used_proxy_info = job->proxy_info();
   const bool was_npn_negotiated = job->was_npn_negotiated();
   const NextProto negotiated_protocol = job->negotiated_protocol();
   const bool using_spdy = job->using_spdy();
   const BoundNetLog net_log = job->net_log();
@@ skipping 80 matching lines @@
       base::Bind(&NetLogHttpStreamJobDelayCallback, main_job_wait_time_));
 
   main_job_->Resume();
   main_job_wait_time_ = base::TimeDelta();
 }
 
 void HttpStreamFactoryImpl::JobController::MaybeResumeMainJob(
     Job* job,
     const base::TimeDelta& delay) {
   DCHECK(job == main_job_.get() || job == alternative_job_.get());
+  if (job == alternative_job_.get())
+    MaybeMarkAlternativeServiceBroken(job);
+
   if (!main_job_is_blocked_ || job != alternative_job_.get() || !main_job_)
     return;
 
   main_job_is_blocked_ = false;
 
   if (!main_job_->is_waiting())
     return;
 
   base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
       FROM_HERE,
@@ skipping 208 matching lines @@
     // another Job completes first.
     // TODO(mbelshe): Revisit this when we implement ip connection pooling of
     // SpdySessions. Do we want to orphan the jobs for a different hostname so
     // they complete? Or do we want to prevent connecting a new SpdySession if
     // we've already got one available for a different hostname where the ip
     // address matches up?
     CancelJobs();
     return;
   }
   if (!bound_job_) {
-    if (main_job_ && alternative_job_) {
+    if (main_job_ && alternative_job_)
       job->ReportJobSucceededForRequest();
-      // Notify all the other jobs that this one succeeded.
-      if (job->job_type() == MAIN) {
-        alternative_job_->MarkOtherJobComplete(*job);
-      } else {
-        DCHECK(job->job_type() == ALTERNATIVE);
-        main_job_->MarkOtherJobComplete(*job);
-      }
-    }
     BindJob(job);
     return;
   }
   DCHECK(bound_job_);
 }
 
 void HttpStreamFactoryImpl::JobController::MarkRequestComplete(
     bool was_npn_negotiated,
     NextProto negotiated_protocol,
     bool using_spdy) {
   if (request_)
     request_->Complete(was_npn_negotiated, negotiated_protocol, using_spdy);
 }
 
+void HttpStreamFactoryImpl::JobController::MaybeMarkAlternativeServiceBroken(
+    Job* job) {
+  if (job->job_type() == MAIN || !job->alternative_broken())
+    return;
+
+  alternative_service_is_broken_ = true;
+  if (job->alternative_proxy_server().is_valid()) {
+    broken_alternative_proxy_server_ = job->alternative_proxy_server();
+  } else {
+    DCHECK(!broken_alternative_proxy_server_.is_valid());
+    broken_alternative_service_ = job->alternative_service();
+  }
+}
+
+void HttpStreamFactoryImpl::JobController::MaybeReportAlternativeServiceBroken(
+    Job* job) {
+  // Only report QUIC broken alternative service when main job succeeds.
+  if (job->job_type() == ALTERNATIVE)
+    return;

[Ryan Hamilton, 2016/09/13 23:23:15]

As discussed offline, I think we need to make sure that we mark QUIC broken when
TCP succeed and QUIC fails, regardless of the order they happen in. Perhaps we
need some Controller unit tests to verify this? (And that we don't mark broken
if both fail)

+
+  if (!alternative_service_is_broken_)
+    return;
+
+  if (broken_alternative_proxy_server_.is_valid()) {
+    ProxyDelegate* proxy_delegate = session_->params().proxy_delegate;
+    if (proxy_delegate)
+      proxy_delegate->OnAlternativeProxyBroken(
+          broken_alternative_proxy_server_);
+  } else {
+    HistogramBrokenAlternateProtocolLocation(
+        BROKEN_ALTERNATE_PROTOCOL_LOCATION_HTTP_STREAM_FACTORY_IMPL_JOB_ALT);
+    session_->http_server_properties()->MarkAlternativeServiceBroken(
+        broken_alternative_service_);
+  }
+  session_->quic_stream_factory()->OnTcpJobCompleted(true);
+}
+
 void HttpStreamFactoryImpl::JobController::MaybeNotifyFactoryOfCompletion() {
   if (!request_ && !main_job_ && !alternative_job_) {
     DCHECK(!bound_job_);
     factory_->OnJobControllerComplete(this);
   }
 }
 
 GURL HttpStreamFactoryImpl::JobController::ApplyHostMappingRules(
     const GURL& url,
     HostPortPair* endpoint) {
@@ skipping 234 matching lines @@
     if (!session_->params().enable_quic ||
         session_->quic_stream_factory()->IsQuicDisabled(
             alternative_proxy_server->host_port_pair().port())) {
       return false;
     }
   }
 
   return true;
 }
 }