Change the default number of proxy resolver threads used for evaluating PAC scripts from 1 to 4.

Also adds a command line flag to override the default: --num-pac-threads=X

BUG=11079
Review URL: http://codereview.chromium.org/2893005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@52046 0039d316-1c4b-4281-b951-d872f2087c98

2 files changed, 25 insertions, 4 deletions

diff --git a/net/proxy/proxy_service.cc b/net/proxy/proxy_service.cc
index 060745b..e6dc15e 100644
--- a/net/proxy/proxy_service.cc
+++ b/net/proxy/proxy_service.cc
@@ -39,6 +39,7 @@ using base::TimeTicks;
 namespace net {
 
 static const size_t kMaxNumNetLogEntries = 100;
+static const size_t kDefaultNumPacThreads = 4;
 
 // Config getter that fails every time.
 class ProxyConfigServiceNull : public ProxyConfigService {
@@ -264,9 +265,12 @@ ProxyService::ProxyService(ProxyConfigService* config_service,
 ProxyService* ProxyService::Create(
     ProxyConfigService* proxy_config_service,
     bool use_v8_resolver,
+    size_t num_pac_threads,
     URLRequestContext* url_request_context,
     NetLog* net_log,
     MessageLoop* io_loop) {
+  if (num_pac_threads == 0)
+    num_pac_threads = kDefaultNumPacThreads;
 
   ProxyResolverFactory* sync_resolver_factory;
   if (use_v8_resolver) {
@@ -278,10 +282,8 @@ ProxyService* ProxyService::Create(
     sync_resolver_factory = new ProxyResolverFactoryForNonV8();
   }
 
-  const size_t kMaxNumResolverThreads = 1u;
   ProxyResolver* proxy_resolver =
-      new MultiThreadedProxyResolver(sync_resolver_factory,
-                                     kMaxNumResolverThreads);
+      new MultiThreadedProxyResolver(sync_resolver_factory, num_pac_threads);
 
   ProxyService* proxy_service =
       new ProxyService(proxy_config_service, proxy_resolver, net_log);
@@ -298,7 +300,9 @@ ProxyService* ProxyService::Create(
 
 // static
 ProxyService* ProxyService::CreateFixed(const ProxyConfig& pc) {
-  return Create(new ProxyConfigServiceFixed(pc), false, NULL, NULL, NULL);
+  // TODO(eroman): This isn't quite right, won't work if |pc| specifies
+  //               a PAC script.
+  return Create(new ProxyConfigServiceFixed(pc), false, 0, NULL, NULL, NULL);
 }
 
 // static
diff --git a/net/proxy/proxy_service.h b/net/proxy/proxy_service.h
index 5c981f8..a702b38 100644
--- a/net/proxy/proxy_service.h
+++ b/net/proxy/proxy_service.h
@@ -137,6 +137,22 @@ class ProxyService : public base::RefCountedThreadSafe<ProxyService>,
   // the proxy settings change. We take ownership of |proxy_config_service|.
   // Iff |use_v8_resolver| is true, then the V8 implementation is
   // used.
+  //
+  // |num_pac_threads| specifies the maximum number of threads to use for
+  // executing PAC scripts. Threads are created lazily on demand.
+  // If |0| is specified, then a default number of threads will be selected.
+  //
+  // Having more threads avoids stalling proxy resolve requests when the
+  // PAC script takes a while to run. This is particularly a problem when PAC
+  // scripts do synchronous DNS resolutions, since that can take on the order
+  // of seconds.
+  //
+  // However, the disadvantages of using more than 1 thread are:
+  //   (a) can cause compatibility issues for scripts that rely on side effects
+  //       between runs (such scripts should not be common though).
+  //   (b) increases the memory used by proxy resolving, as each thread will
+  //       duplicate its own script context.
+
   // |url_request_context| is only used when use_v8_resolver is true:
   // it specifies the URL request context that will be used if a PAC
   // script needs to be fetched.
@@ -150,6 +166,7 @@ class ProxyService : public base::RefCountedThreadSafe<ProxyService>,
   static ProxyService* Create(
       ProxyConfigService* proxy_config_service,
       bool use_v8_resolver,
+      size_t num_pac_threads,
       URLRequestContext* url_request_context,
       NetLog* net_log,
       MessageLoop* io_loop);