Extract Proxy Resolution out of HttpStreamFactoryImpl::Job

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/base/privacy_mode.h"
 #include "net/http/http_stream_factory_impl_job.h"
 #include "net/http/http_stream_factory_impl_request.h"
 
 namespace net {
 
 class NetLogWithSource;
 
 // HttpStreamFactoryImpl::JobController manages Request and Job(s).
 class HttpStreamFactoryImpl::JobController
     : public HttpStreamFactoryImpl::Job::Delegate,
       public HttpStreamFactoryImpl::Request::Helper {
  public:
   JobController(HttpStreamFactoryImpl* factory,
                 HttpStreamRequest::Delegate* delegate,
                 HttpNetworkSession* session,
                 JobFactory* job_factory,
                 const HttpRequestInfo& request_info,
                 bool is_preconnect,
                 bool enable_ip_based_pooling,
-                bool enable_alternative_services);
+                bool enable_alternative_services,
+                const SSLConfig& server_ssl_config,
+                const SSLConfig& proxy_ssl_config);
 
   ~JobController() override;
 
   bool for_websockets() override;
 
   // Used in tests only for verification purpose.
   const Job* main_job() const { return main_job_.get(); }
   const Job* alternative_job() const { return alternative_job_.get(); }
 
   GURL ApplyHostMappingRules(const GURL& url, HostPortPair* endpoint);
 
   // Methods below are called by HttpStreamFactoryImpl only.
   // Creates request and hands out to HttpStreamFactoryImpl, this will also
   // create Job(s) and start serving the created request.
-  Request* Start(const HttpRequestInfo& request_info,
-                 HttpStreamRequest::Delegate* delegate,
+  Request* Start(HttpStreamRequest::Delegate* delegate,
                  WebSocketHandshakeStreamBase::CreateHelper*
                      websocket_handshake_stream_create_helper,
                  const NetLogWithSource& source_net_log,
                  HttpStreamRequest::StreamType stream_type,
-                 RequestPriority priority,
-                 const SSLConfig& server_ssl_config,
-                 const SSLConfig& proxy_ssl_config);
+                 RequestPriority priority);
 
-  void Preconnect(int num_streams,
-                  const HttpRequestInfo& request_info,
-                  const SSLConfig& server_ssl_config,
-                  const SSLConfig& proxy_ssl_config);
+  void Preconnect(int num_streams);
 
   // From HttpStreamFactoryImpl::Request::Helper.
   // Returns the LoadState for Request.
   LoadState GetLoadState() const override;
 
   // Called when Request is destructed. Job(s) associated with but not bound to
   // |request_| will be deleted. |request_| and |bound_job_| will be nulled if
   // ever set.
   void OnRequestComplete() override;
 
   // Called to resume the HttpStream creation process when necessary
   // Proxy authentication credentials are collected.
   int RestartTunnelWithProxyAuth() override;
 
+  // Called when we encounter a network error that could be resolved by trying

[Ryan Hamilton, 2017/05/09 19:22:07]

nit: avoid first person in comments.

[xunjieli, 2017/05/10 00:26:25]

Done.

+  // a new proxy configuration.  If there is another proxy configuration to try
+  // then this method sets next_state_ appropriately and returns either OK or

[tbansal1, 2017/05/09 20:38:48]

s/next_state_/|next_state_|/

[xunjieli, 2017/05/10 00:26:25]

Done.

+  // ERR_IO_PENDING depending on whether or not the new proxy configuration is
+  // available synchronously or asynchronously.  Otherwise, the given error
+  // code is simply returned.
+  int ReconsiderProxyAfterError(Job* job, int error);

[tbansal1, 2017/05/09 20:38:48]

Can this be a private method?

[xunjieli, 2017/05/10 00:26:25]

Done. Good point!

+
   // Called when the priority of transaction changes.
   void SetPriority(RequestPriority priority) override;
 
   // From HttpStreamFactoryImpl::Job::Delegate.
   // Invoked when |job| has an HttpStream ready.
   void OnStreamReady(Job* job, const SSLConfig& used_ssl_config) override;
 
   // Invoked when |job| has a BidirectionalStream ready.
   void OnBidirectionalStreamImplReady(
       Job* job,
@@ skipping 33 matching lines @@
 
   // Invoked when |job| needs proxy authentication.
   void OnNeedsProxyAuth(Job* job,
                         const HttpResponseInfo& proxy_response,
                         const SSLConfig& used_ssl_config,
                         const ProxyInfo& used_proxy_info,
                         HttpAuthController* auth_controller) override;
 
   bool OnInitConnection(const ProxyInfo& proxy_info) override;
 
-  void OnResolveProxyComplete(
-      Job* job,
-      const HttpRequestInfo& request_info,
-      RequestPriority priority,
-      const SSLConfig& server_ssl_config,
-      const SSLConfig& proxy_ssl_config,
-      HttpStreamRequest::StreamType stream_type) override;
 
   // Invoked to notify the Request and Factory of the readiness of new
   // SPDY session.
   void OnNewSpdySessionReady(Job* job,
                              const base::WeakPtr<SpdySession>& spdy_session,
                              bool direct) override;
 
   // Invoked when the |job| finishes pre-connecting sockets.
   void OnPreconnectsComplete(Job* job) override;
 
@@ skipping 42 matching lines @@
   // TODO(xunjieli): Added to investigate crbug.com/711721. Remove when no
   // longer needed.
   void LogHistograms() const;
 
   // Returns the estimated memory usage in bytes.
   size_t EstimateMemoryUsage() const;
 
  private:
   friend class JobControllerPeer;
 
+  enum State {
+    STATE_RESOLVE_PROXY,
+    STATE_RESOLVE_PROXY_COMPLETE,
+
+    STATE_CREATE_JOBS,
+    // STATE_CREATE_JOBS_COMPLETE,

[Ryan Hamilton, 2017/05/09 19:22:06]

Should this be removed?

[xunjieli, 2017/05/10 00:26:25]

Done. Ah, Good catch!

+    STATE_NONE
+  };
+
+  void OnIOComplete(int result);
+  void OnResolveProxyError(int error);
+  void RunLoop(int result);
+  int DoLoop(int result);
+  int DoResolveProxy();
+  int DoResolveProxyComplete(int result);
   // Creates Job(s) for |request_|. Job(s) will be owned by |this|.
-  void CreateJobs(const HttpRequestInfo& request_info,
-                  RequestPriority priority,
-                  const SSLConfig& server_ssl_config,
-                  const SSLConfig& proxy_ssl_config,
-                  HttpStreamRequest::Delegate* delegate,
-                  HttpStreamRequest::StreamType stream_type);
+  int DoCreateJobs();
 
   // Called to bind |job| to the |request_| and orphan all other jobs that are
   // still associated with |request_|.
   void BindJob(Job* job);
 
   // Called when |request_| is destructed.
   // Job(s) associated with but not bound to |request_| will be deleted.
   void CancelJobs();
 
   // Called after BindJob() to notify the unbound job that its result should be
   // ignored by JobController. The unbound job can be canceled or continue until
   // completion.
   void OrphanUnboundJob();
 
   // Invoked when the orphaned |job| finishes.
   void OnOrphanedJobComplete(const Job* job);
 
   // Called when a Job succeeds.
   void OnJobSucceeded(Job* job);
 
   // Marks completion of the |request_|.
   void MarkRequestComplete(bool was_alpn_negotiated,
                            NextProto negotiated_protocol,
                            bool using_spdy);
 
   // Must be called when the alternative job fails. |net_error| is the net error
   // of the failed alternative job.
   void OnAlternativeJobFailed(int net_error);
 
+  void OnAlternativeProxyJobFailed(int net_error);
+
   // Called to report to http_server_properties to mark alternative service
   // broken.
   void ReportBrokenAlternativeService();
 
   void MaybeNotifyFactoryOfCompletion();
 
   // Called to resume the main job with delay. Main job is resumed only when
   // |alternative_job_| has failed or |main_job_wait_time_| elapsed.
   void MaybeResumeMainJob(Job* job, const base::TimeDelta& delay);
 
@@ skipping 14 matching lines @@
       HttpStreamRequest::StreamType stream_type);
 
   // Remove session from the SpdySessionRequestMap.
   void RemoveRequestFromSpdySessionRequestMap();
 
   // Returns true if the |request_| can be fetched via an alternative
   // proxy server, and sets |alternative_proxy_server| to the available
   // alternative proxy server. |alternative_proxy_server| should not be null,
   // and is owned by the caller.
   bool ShouldCreateAlternativeProxyServerJob(
-      Job* job,
       const ProxyInfo& proxy_info_,
       const GURL& url,
       ProxyServer* alternative_proxy_server) const;
 
   // Records histogram metrics for the usage of alternative protocol. Must be
   // called when |job| has succeeded and the other job will be orphaned.
   void ReportAlternateProtocolUsage(Job* job) const;
 
   // Starts the |alternative_job_|.
   void StartAlternativeProxyServerJob();
@@ skipping 24 matching lines @@
   const bool enable_alternative_services_;
 
   // |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_;
 
   // Net error code of the failed alternative job. Set to OK by default.
   int alternative_job_net_error_;
-
-  // Either and only one of these records failed alternative service/proxy
-  // server that |alternative_job_| uses.
+  // The alternative service server that |alternative_job_| uses failed.
   AlternativeService failed_alternative_service_;
-  ProxyServer failed_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_;
 
   // True if the main job was blocked and has been resumed in ResumeMainJob().
   bool main_job_is_resumed_;
 
   // 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_;
 
   // Privacy mode that should be used for fetching the resource.
   PrivacyMode privacy_mode_;
 
+  State next_state_;
+  ProxyService::PacRequest* pac_request_;
+
   const NetLogWithSource net_log_;

[tbansal1, 2017/05/09 20:38:48]

nit: |net_log_| should be the last entry (before WeakPtr factory).

[xunjieli, 2017/05/10 00:26:25]

Done.

 
+  CompletionCallback io_callback_;
+  const HttpRequestInfo request_info_;
+  ProxyInfo proxy_info_;
+  const SSLConfig server_ssl_config_;
+  const SSLConfig proxy_ssl_config_;
+  int num_streams_;
+
+  HttpStreamRequest::StreamType stream_type_;
+  RequestPriority priority_;
+
   base::WeakPtrFactory<JobController> ptr_factory_;
 };
 
 }  // namespace net
 
 #endif  // NET_HTTP_HTTP_STREAM_FACTORY_IMPL_JOB_CONTROLLER_H_