Extract Proxy Resolution out of HttpStreamFactoryImpl::Job

net/http/http_stream_factory_impl.cc

 // Copyright (c) 2012 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.h"
 
 #include <string>
 #include <tuple>
 
 #include "base/logging.h"
@@ skipping 14 matching lines @@
 #include "net/proxy/proxy_info.h"
 #include "net/quic/core/quic_server_id.h"
 #include "net/spdy/chromium/bidirectional_stream_spdy_impl.h"
 #include "net/spdy/chromium/spdy_http_stream.h"
 #include "url/gurl.h"
 #include "url/scheme_host_port.h"
 #include "url/url_constants.h"
 
 namespace net {
 
-namespace {
-// Default JobFactory for creating HttpStreamFactoryImpl::Jobs.
-class DefaultJobFactory : public HttpStreamFactoryImpl::JobFactory {
- public:
-  DefaultJobFactory() {}
-
-  ~DefaultJobFactory() override {}
-
-  HttpStreamFactoryImpl::Job* CreateJob(
-      HttpStreamFactoryImpl::Job::Delegate* delegate,
-      HttpStreamFactoryImpl::JobType job_type,
-      HttpNetworkSession* session,
-      const HttpRequestInfo& request_info,
-      RequestPriority priority,
-      const SSLConfig& server_ssl_config,
-      const SSLConfig& proxy_ssl_config,
-      HostPortPair destination,
-      GURL origin_url,
-      bool enable_ip_based_pooling,
-      NetLog* net_log) override {
-    return new HttpStreamFactoryImpl::Job(
-        delegate, job_type, session, request_info, priority, server_ssl_config,
-        proxy_ssl_config, destination, origin_url, enable_ip_based_pooling,
-        net_log);
-  }
-
-  HttpStreamFactoryImpl::Job* CreateJob(
-      HttpStreamFactoryImpl::Job::Delegate* delegate,
-      HttpStreamFactoryImpl::JobType job_type,
-      HttpNetworkSession* session,
-      const HttpRequestInfo& request_info,
-      RequestPriority priority,
-      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) override {
-    return new HttpStreamFactoryImpl::Job(
-        delegate, job_type, session, request_info, priority, server_ssl_config,
-        proxy_ssl_config, destination, origin_url, alternative_service,
-        ProxyServer(), enable_ip_based_pooling, net_log);
-  }
-
-  HttpStreamFactoryImpl::Job* CreateJob(
-      HttpStreamFactoryImpl::Job::Delegate* delegate,
-      HttpStreamFactoryImpl::JobType job_type,
-      HttpNetworkSession* session,
-      const HttpRequestInfo& request_info,
-      RequestPriority priority,
-      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) override {
-    return new HttpStreamFactoryImpl::Job(
-        delegate, job_type, session, request_info, priority, server_ssl_config,
-        proxy_ssl_config, destination, origin_url, AlternativeService(),
-        alternative_proxy_server, enable_ip_based_pooling, net_log);
-  }
-};
-
-}  // anonymous namespace
-
 HttpStreamFactoryImpl::HttpStreamFactoryImpl(HttpNetworkSession* session,
                                              bool for_websockets)
     : session_(session),
-      job_factory_(new DefaultJobFactory()),
+      job_factory_(new JobFactory()),

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

Well that's a nice cleanup! I wonder what DefaultJobFactory was created for... 

[Zhongyi Shi, 2017/05/09 20:59:30]

Ah, JobFactory used to be an interface, and production code uses
DefaultJobFactory while unittests uses TestJobFactory. 

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

Cherie is right. The interface seems to be created for testing purpose. I am
getting rid of it because I need a real JobFactory instead of TestJobFactory for
JobControllerReconsiderProxyAfterErrorTest. The DefaultJobFactory was a private
class.

       for_websockets_(for_websockets),
       last_logged_job_controller_count_(0) {}
 
 HttpStreamFactoryImpl::~HttpStreamFactoryImpl() {
   DCHECK(spdy_session_request_map_.empty());
   UMA_HISTOGRAM_COUNTS_1M("Net.JobControllerSet.CountOfJobControllerAtShutDown",
                           job_controller_set_.size());
 }
 
 HttpStreamRequest* HttpStreamFactoryImpl::RequestStream(
@@ skipping 58 matching lines @@
         websocket_handshake_stream_create_helper,
     HttpStreamRequest::StreamType stream_type,
     bool enable_ip_based_pooling,
     bool enable_alternative_services,
     const NetLogWithSource& net_log) {
   AddJobControllerCountToHistograms();
 
   auto job_controller = base::MakeUnique<JobController>(
       this, delegate, session_, job_factory_.get(), request_info,
       /* is_preconnect = */ false, enable_ip_based_pooling,
-      enable_alternative_services);
+      enable_alternative_services, server_ssl_config, proxy_ssl_config);
   JobController* job_controller_raw_ptr = job_controller.get();
   job_controller_set_.insert(std::move(job_controller));
   Request* request = job_controller_raw_ptr->Start(
-      request_info, delegate, websocket_handshake_stream_create_helper, net_log,
-      stream_type, priority, server_ssl_config, proxy_ssl_config);
+      delegate, websocket_handshake_stream_create_helper, net_log, stream_type,
+      priority);
 
   return request;
 }
 
 void HttpStreamFactoryImpl::PreconnectStreams(
     int num_streams,
     const HttpRequestInfo& request_info) {
   AddJobControllerCountToHistograms();
 
   SSLConfig server_ssl_config;
   SSLConfig proxy_ssl_config;
   session_->GetSSLConfig(request_info, &server_ssl_config, &proxy_ssl_config);
   // All preconnects should perform EV certificate verification.
   server_ssl_config.verify_ev_cert = true;
   proxy_ssl_config.verify_ev_cert = true;
 
   DCHECK(!for_websockets_);
 
   auto job_controller = base::MakeUnique<JobController>(
       this, nullptr, session_, job_factory_.get(), request_info,
       /* is_preconnect = */ true,
       /* enable_ip_based_pooling = */ true,
-      /* enable_alternative_services = */ true);
+      /* enable_alternative_services = */ true, server_ssl_config,
+      proxy_ssl_config);
   JobController* job_controller_raw_ptr = job_controller.get();
   job_controller_set_.insert(std::move(job_controller));
-  job_controller_raw_ptr->Preconnect(num_streams, request_info,
-                                     server_ssl_config, proxy_ssl_config);
+  job_controller_raw_ptr->Preconnect(num_streams);
 }
 
 const HostMappingRules* HttpStreamFactoryImpl::GetHostMappingRules() const {
   return session_->params().host_mapping_rules;
 }
 
 void HttpStreamFactoryImpl::OnNewSpdySessionReady(
     const base::WeakPtr<SpdySession>& spdy_session,
     bool direct,
     const SSLConfig& used_ssl_config,
@@ skipping 225 matching lines @@
   factory_dump->AddScalar("main_job_count",
                           base::trace_event::MemoryAllocatorDump::kUnitsObjects,
                           main_job_count);
   // The number of preconnect controllers.
   factory_dump->AddScalar("preconnect_count",
                           base::trace_event::MemoryAllocatorDump::kUnitsObjects,
                           num_controllers_for_preconnect);
 }
 
 }  // namespace net