Extract Proxy Resolution out of HttpStreamFactoryImpl::Job

net/http/http_stream_factory_impl_job.h

Index: net/http/http_stream_factory_impl_job.h
diff --git a/net/http/http_stream_factory_impl_job.h b/net/http/http_stream_factory_impl_job.h
index 809574ed30b4756c29679858b5e45608f4e30b18..5fcbd3ae1fb0570b285182264dbce6db68a14f5f 100644
--- a/net/http/http_stream_factory_impl_job.h
+++ b/net/http/http_stream_factory_impl_job.h
@@ -102,16 +102,6 @@ class HttpStreamFactoryImpl::Job {
     // contained in |proxy_info| can be skipped.
     virtual bool OnInitConnection(const ProxyInfo& proxy_info) = 0;
 
-    // Invoked when |job| has completed proxy resolution. The delegate may
-    // create an alternative proxy server job to fetch the request.
-    virtual void OnResolveProxyComplete(
-        Job* job,
-        const HttpRequestInfo& request_info,
-        RequestPriority priority,
-        const SSLConfig& server_ssl_config,
-        const SSLConfig& proxy_ssl_config,
-        HttpStreamRequest::StreamType stream_type) = 0;
-
     // Invoked to notify the Request and Factory of the readiness of new
     // SPDY session.
     virtual void OnNewSpdySessionReady(
@@ -163,6 +153,7 @@ class HttpStreamFactoryImpl::Job {
       HttpNetworkSession* session,
       const HttpRequestInfo& request_info,
       RequestPriority priority,
+      const ProxyInfo& proxy_info,
       const SSLConfig& server_ssl_config,
       const SSLConfig& proxy_ssl_config,
       HostPortPair destination,
@@ -183,6 +174,7 @@ class HttpStreamFactoryImpl::Job {
       HttpNetworkSession* session,
       const HttpRequestInfo& request_info,
       RequestPriority priority,
+      const ProxyInfo& proxy_info,
       const SSLConfig& server_ssl_config,
       const SSLConfig& proxy_ssl_config,
       HostPortPair destination,
@@ -251,6 +243,8 @@ class HttpStreamFactoryImpl::Job {
     return using_existing_quic_session_;
   }
 
+  bool should_reconsider_proxy() const { return should_reconsider_proxy_; }
+
   // TODO(xunjieli): Added to investigate crbug.com/711721. Remove when no
   // longer needed.
   void LogHistograms() const;
@@ -260,9 +254,6 @@ class HttpStreamFactoryImpl::Job {
 
   enum State {
     STATE_START,
-    STATE_RESOLVE_PROXY,
-    STATE_RESOLVE_PROXY_COMPLETE,
-
     // The main and alternative jobs are started in parallel.  The main job
     // waits after it finishes proxy resolution.  The alternative job never

[Zhongyi Shi, 2017/05/10 20:28:15]

Could you also update the comments since the proxy resolution is moved out of
Job?

[xunjieli, 2017/05/10 23:24:41]

Done. Good point!

     // waits.
@@ -321,8 +312,6 @@ class HttpStreamFactoryImpl::Job {
   // ERR_IO_PENDING, then the result from OnIOComplete will be passed to the
   // next state method as the result arg.
   int DoStart();
-  int DoResolveProxy();
-  int DoResolveProxyComplete(int result);
   int DoWait();
   int DoWaitComplete(int result);
   int DoInitConnection();
@@ -405,7 +394,7 @@ class HttpStreamFactoryImpl::Job {
 
   const HttpRequestInfo request_info_;
   RequestPriority priority_;
-  ProxyInfo proxy_info_;
+  const ProxyInfo proxy_info_;
   SSLConfig server_ssl_config_;
   SSLConfig proxy_ssl_config_;
   const NetLogWithSource net_log_;
@@ -418,7 +407,6 @@ class HttpStreamFactoryImpl::Job {
   State state_;
 
   State next_state_;
-  ProxyService::PacRequest* pac_request_;
   SSLInfo ssl_info_;
 
   // The server we are trying to reach, could be that of the origin or of the
@@ -453,6 +441,10 @@ class HttpStreamFactoryImpl::Job {
 
   // True if this network transaction is using QUIC instead of HTTP.
   bool using_quic_;
+
+  // True if this job might succeed with a different proxy config.
+  bool should_reconsider_proxy_;
+
   QuicStreamRequest quic_request_;
 
   // True if this job used an existing QUIC session.
@@ -498,51 +490,53 @@ class HttpStreamFactoryImpl::Job {
 // Factory for creating Jobs.
 class HttpStreamFactoryImpl::JobFactory {
  public:
-  virtual ~JobFactory() {}
+  JobFactory();
+
+  virtual ~JobFactory();
 
-  // Creates an alternative service Job.
-  virtual HttpStreamFactoryImpl::Job* CreateJob(
+  virtual HttpStreamFactoryImpl::Job* CreateMainJob(
       HttpStreamFactoryImpl::Job::Delegate* delegate,
       HttpStreamFactoryImpl::JobType job_type,
       HttpNetworkSession* session,
       const HttpRequestInfo& request_info,
       RequestPriority priority,
+      ProxyInfo proxy_info,

[tbansal1, 2017/05/10 21:53:20]

pass as const ref. Same for other ctors below.

[xunjieli, 2017/05/10 23:24:41]

Done. Good point!

       const SSLConfig& server_ssl_config,
       const SSLConfig& proxy_ssl_config,
       HostPortPair destination,
       GURL origin_url,
-      AlternativeService alternative_service,
       bool enable_ip_based_pooling,
-      NetLog* net_log) = 0;
+      NetLog* net_log);
 
-  // Creates an alternative proxy server Job.
-  virtual HttpStreamFactoryImpl::Job* CreateJob(
+  virtual HttpStreamFactoryImpl::Job* CreateAltJob(

[Zhongyi Shi, 2017/05/10 20:28:15]

optional nit: how about CreateAltSvcJob?

[xunjieli, 2017/05/10 23:24:41]

Done.

       HttpStreamFactoryImpl::Job::Delegate* delegate,
       HttpStreamFactoryImpl::JobType job_type,
       HttpNetworkSession* session,
       const HttpRequestInfo& request_info,
       RequestPriority priority,
+      ProxyInfo proxy_info,
       const SSLConfig& server_ssl_config,
       const SSLConfig& proxy_ssl_config,
       HostPortPair destination,
       GURL origin_url,
-      const ProxyServer& alternative_proxy_server,
+      AlternativeService alternative_service,
       bool enable_ip_based_pooling,
-      NetLog* net_log) = 0;
+      NetLog* net_log);
 
-  // Creates a non-alternative Job.
-  virtual HttpStreamFactoryImpl::Job* CreateJob(
+  virtual HttpStreamFactoryImpl::Job* CreateAltProxyJob(
       HttpStreamFactoryImpl::Job::Delegate* delegate,
       HttpStreamFactoryImpl::JobType job_type,
       HttpNetworkSession* session,
       const HttpRequestInfo& request_info,
       RequestPriority priority,
+      ProxyInfo proxy_info,
       const SSLConfig& server_ssl_config,
       const SSLConfig& proxy_ssl_config,
       HostPortPair destination,
       GURL origin_url,
+      const ProxyServer& alternative_proxy_server,
       bool enable_ip_based_pooling,
-      NetLog* net_log) = 0;
+      NetLog* net_log);
 };
 
 }  // namespace net