JobController3: Move MarkAlternativeServiceBroken to job controller

net/http/http_stream_factory_impl_job_controller.h

 // 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.
 
 #ifndef NET_HTTP_HTTP_STREAM_FACTORY_IMPL_JOB_CONTROLLER_H_
 #define NET_HTTP_HTTP_STREAM_FACTORY_IMPL_JOB_CONTROLLER_H_
 
 #include "net/base/host_port_pair.h"
 #include "net/http/http_stream_factory_impl_job.h"
 #include "net/http/http_stream_factory_impl_request.h"
@@ skipping 183 matching lines @@
   void OrphanUnboundJob();
 
   // Called when a Job succeeds.
   void OnJobSucceeded(Job* job);
 
   // Marks completion of the |request_|.
   void MarkRequestComplete(bool was_npn_negotiated,
                            NextProto negotiated_protocol,
                            bool using_spdy);
 
+  // Must be called when |alternative_job_| fails.
+  void MarkAlternativeServiceBroken(Job* job);
+
+  // Called to report to http_server_properties to mark alternative service
+  // broken.
+  void ReportAlternativeServiceBroken(Job* job);

[Ryan Hamilton, 2016/09/14 21:45:42]

nit: How about "ReportBrokenAlternativeService", which reads ever so slightly
more like a sentence.

[Zhongyi Shi, 2016/09/14 23:07:07]

Done.

+
   void MaybeNotifyFactoryOfCompletion();
 
   // Called to resume the main job with delay.
   void MaybeResumeMainJob(Job* job, const base::TimeDelta& delay);
 
   void ResumeMainJob();
 
   // Returns true if QUIC is whitelisted for |host|.
   bool IsQuicWhitelistedForHost(const std::string& host);
 
@@ skipping 34 matching lines @@
 
   // True if this JobController is used to preconnect streams.
   bool is_preconnect_;
 
   // |main_job_| is a job waiting to see if |alternative_job_| can reuse a
   // connection. If |alternative_job_| is unable to do so, |this| will notify
   // |main_job_| to proceed and then race the two jobs.
   std::unique_ptr<Job> main_job_;
   std::unique_ptr<Job> alternative_job_;
 
+  // True if |alternative_job_| uses alternative service/proxy server and
+  // discovers it as broken.
+  bool alternative_service_is_broken_;

[Ryan Hamilton, 2016/09/14 21:45:42]

Instead of "broken", let's use "failed" here. Broken is only the state we reach
when alt-svc fails and main succeeds. (and in the members below)

That would also suggest renaming MarkAlternativeServiceBroken to
MarkAlternativeServiceJobFailed. (Or perhaps OnAlternativeServiceJobFailed).

[Zhongyi Shi, 2016/09/14 23:07:07]

Done.

+
+  // Either and only one of these records broken alternative service/proxy
+  // server that |alternative_job_| uses.
+  AlternativeService broken_alternative_service_;
+  ProxyServer broken_alternative_proxy_server_;
+
   // True if a Job has ever been bound to the |request_|.
   bool job_bound_;
 
   // True if the main job has to wait for the alternative job: i.e., the main
   // job must not create a connection until it is resumed.
   bool main_job_is_blocked_;
   // Waiting time for the main job before it is resumed.
   base::TimeDelta main_job_wait_time_;
 
   // At the point where a Job is irrevocably tied to |request_|, we set this.
   // It will be nulled when the |request_| is finished.
   Job* bound_job_;
 
   // True if an alternative proxy server job can be started to fetch |request_|.
   bool can_start_alternative_proxy_job_;
 
   base::WeakPtrFactory<JobController> ptr_factory_;
 };
 
 }  // namespace net
 
 #endif  // NET_HTTP_HTTP_STREAM_FACTORY_IMPL_JOB_CONTROLLER_H_