Plumb NQP to context and to http_proxy_client_socket_pool

net/http/http_proxy_client_socket_pool.cc

Index: net/http/http_proxy_client_socket_pool.cc
diff --git a/net/http/http_proxy_client_socket_pool.cc b/net/http/http_proxy_client_socket_pool.cc
index 7375920d8a2daa0b4e83ed0a2447d5243913c7a5..7e68f7de87ddfa01740231cd85a3af7b9a6acbf1 100644
--- a/net/http/http_proxy_client_socket_pool.cc
+++ b/net/http/http_proxy_client_socket_pool.cc
@@ -32,6 +32,36 @@
 
 namespace net {
 
+namespace {
+
+// Configure using net session params.
+base::TimeDelta GetConnectionTimeout(

[mmenke, 2017/06/07 21:31:25]

Why do we have two methods?

[tbansal1, 2017/06/08 05:01:50]

Done.

+    NetworkQualityProvider* network_quality_provider,
+    base::TimeDelta default_connection_timeout) {
+  // TODO(tbansal): https://crbug.com/704339. Use NQE and field trial to
+  // determine the connection timeout.

[mmenke, 2017/06/07 21:31:26]

Should this logic be in the socket pools in in net/nqe?

[tbansal1, 2017/06/08 05:01:50]

The timeout is now dynamic i.e., it can change with time during the coarse of a
single Chrome session.
The timeout would also depend on whether the connection is secure or not. 
Given these two constraints, it seems this is the best place. IIUC, if I put
timeout in socket pool, then neither of these two constraints would be
satisfied.

+  return default_connection_timeout;
+}
+
+base::TimeDelta GetMaxPoolTimeout(TransportClientSocketPool* transport_pool,
+                                  SSLClientSocketPool* ssl_pool) {
+  base::TimeDelta max_pool_timeout = base::TimeDelta();
+
+#if (defined(OS_ANDROID) || defined(OS_IOS))
+  return max_pool_timeout;
+#endif

[mmenke, 2017/06/07 21:31:26]

#else seems a bit more natural here (And I don't trust our tools not to start
complaining about statements unreachable on some platforms)

[tbansal1, 2017/06/08 05:01:50]

Modified this. Hopefully the new one is better.

+
+  if (transport_pool)
+    max_pool_timeout = transport_pool->ConnectionTimeout();
+  if (ssl_pool) {
+    max_pool_timeout =
+        std::max(max_pool_timeout, ssl_pool->ConnectionTimeout());
+  }
+  return max_pool_timeout;
+}
+
+}  // namespace
+
 HttpProxySocketParams::HttpProxySocketParams(
     const scoped_refptr<TransportSocketParams>& transport_params,
     const scoped_refptr<SSLSocketParams>& ssl_params,
@@ -149,29 +179,18 @@ int HttpProxyConnectJob::HandleConnectResult(int result) {
   return result;
 }
 
-HttpProxyClientSocketPool::
-HttpProxyConnectJobFactory::HttpProxyConnectJobFactory(
-    TransportClientSocketPool* transport_pool,
-    SSLClientSocketPool* ssl_pool,
-    NetLog* net_log)
+HttpProxyClientSocketPool::HttpProxyConnectJobFactory::
+    HttpProxyConnectJobFactory(TransportClientSocketPool* transport_pool,
+                               SSLClientSocketPool* ssl_pool,
+                               NetworkQualityProvider* network_quality_provider,
+                               NetLog* net_log)
     : transport_pool_(transport_pool),
       ssl_pool_(ssl_pool),
-      net_log_(net_log) {
-  base::TimeDelta max_pool_timeout = base::TimeDelta();
-
-// TODO(kundaji): Proxy connect timeout should be independent of platform and be
-// based on proxy. Bug http://crbug.com/407446.
-#if (defined(OS_ANDROID) || defined(OS_IOS))
-#else
-  if (transport_pool_)
-    max_pool_timeout = transport_pool_->ConnectionTimeout();
-  if (ssl_pool_)
-    max_pool_timeout = std::max(max_pool_timeout,
-                                ssl_pool_->ConnectionTimeout());
-#endif
-  timeout_ = max_pool_timeout +
-    base::TimeDelta::FromSeconds(kHttpProxyConnectJobTimeoutInSeconds);
-}
+      network_quality_provider_(network_quality_provider),
+      default_connection_timeout_(
+          GetMaxPoolTimeout(transport_pool, ssl_pool) +
+          base::TimeDelta::FromSeconds(kHttpProxyConnectJobTimeoutInSeconds)),
+      net_log_(net_log) {}
 
 std::unique_ptr<ConnectJob>
 HttpProxyClientSocketPool::HttpProxyConnectJobFactory::NewConnectJob(
@@ -187,7 +206,8 @@ HttpProxyClientSocketPool::HttpProxyConnectJobFactory::NewConnectJob(
 base::TimeDelta
 HttpProxyClientSocketPool::HttpProxyConnectJobFactory::ConnectionTimeout(
     ) const {
-  return timeout_;
+  return GetConnectionTimeout(network_quality_provider_,
+                              default_connection_timeout_);
 }
 
 HttpProxyClientSocketPool::HttpProxyClientSocketPool(
@@ -195,6 +215,7 @@ HttpProxyClientSocketPool::HttpProxyClientSocketPool(
     int max_sockets_per_group,
     TransportClientSocketPool* transport_pool,
     SSLClientSocketPool* ssl_pool,
+    NetworkQualityProvider* network_quality_provider,
     NetLog* net_log)
     : transport_pool_(transport_pool),
       ssl_pool_(ssl_pool),
@@ -203,7 +224,10 @@ HttpProxyClientSocketPool::HttpProxyClientSocketPool(
             max_sockets_per_group,
             ClientSocketPool::unused_idle_socket_timeout(),
             ClientSocketPool::used_idle_socket_timeout(),
-            new HttpProxyConnectJobFactory(transport_pool, ssl_pool, net_log)) {
+            new HttpProxyConnectJobFactory(transport_pool,
+                                           ssl_pool,
+                                           network_quality_provider,
+                                           net_log)) {
   // We should always have a |transport_pool_| except in unit tests.
   if (transport_pool_)
     base_.AddLowerLayeredPool(transport_pool_);