Determine proxy connection timeout based on current network quality

net/http/http_proxy_client_socket_pool_unittest.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_proxy_client_socket_pool.h"
 
+#include <map>
+#include <string>
 #include <utility>
 
 #include "base/callback.h"
 #include "base/compiler_specific.h"
+#include "base/metrics/field_trial.h"
+#include "base/metrics/field_trial_param_associator.h"
+#include "base/metrics/field_trial_params.h"
+#include "base/strings/string_number_conversions.h"
 #include "base/strings/string_util.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/histogram_tester.h"
 #include "net/base/net_errors.h"
 #include "net/base/proxy_delegate.h"
 #include "net/base/test_completion_callback.h"
 #include "net/base/test_proxy_delegate.h"
 #include "net/http/http_network_session.h"
 #include "net/http/http_proxy_client_socket.h"
 #include "net/http/http_response_headers.h"
@@ skipping 47 matching lines @@
                          NULL /* transport_security_state */,
                          NULL /* cert_transparency_verifier */,
                          NULL /* ct_policy_enforcer */,
                          std::string() /* ssl_session_cache_shard */,
                          session_deps_.socket_factory.get(),
                          &transport_socket_pool_,
                          NULL,
                          NULL,
                          session_deps_.ssl_config_service.get(),
                          NetLogWithSource().net_log()),
-        pool_(kMaxSockets,
-              kMaxSocketsPerGroup,
-              &transport_socket_pool_,
-              &ssl_socket_pool_,
-              &estimator_,
-              NULL) {
+        field_trial_list_(nullptr) {

[mmenke, 2017/06/13 19:52:01]

git cl format.

[tbansal1, 2017/06/13 23:02:48]

umm, this is formatted. 
|field_trial_list_| is at the same indentation level as |ssl_socket_pool_| which
looks correct to me.

[mmenke, 2017/06/14 18:10:01]

Sorry, was confused by the ssl_socket_pool_ arguments.

+    pool_ = base::MakeUnique<HttpProxyClientSocketPool>(
+        kMaxSockets, kMaxSocketsPerGroup, &transport_socket_pool_,
+        &ssl_socket_pool_, &estimator_, NetLogWithSource().net_log());

[mmenke, 2017/06/13 19:52:00]

Why move this?  Can still call MakeUnique in the initializer list.

[tbansal1, 2017/06/13 23:02:48]

Done.

     session_ = CreateNetworkSession();
   }
 
   virtual ~HttpProxyClientSocketPoolTest() {}
 
+  // Initializes the field trial paramters for the field trial that determines
+  // connection timeout based on the network quality.
+  void InitAdaptiveTimeoutFieldTrialWithParams(
+      int transport_rtt_multiplier,
+      base::TimeDelta min_proxy_connection_timeout,
+      base::TimeDelta max_proxy_connection_timeout) {
+    std::string trial_name = "NetAdaptiveProxyConnectionTimeout";
+    std::string group_name = "Enabled";
+
+    std::map<std::string, std::string> params;
+    params["transport_rtt_multiplier"] =
+        base::IntToString(transport_rtt_multiplier);
+    params["min_proxy_connection_timeout_seconds"] =
+        base::IntToString(min_proxy_connection_timeout.InSeconds());
+    params["max_proxy_connection_timeout_seconds"] =
+        base::IntToString(max_proxy_connection_timeout.InSeconds());
+    base::AssociateFieldTrialParams(trial_name, group_name, params);
+    base::FieldTrialList::CreateFieldTrial(trial_name, group_name);
+
+    // Reset |pool_| so that the field trial parameters are read by the
+    // |pool_|.
+    pool_ = base::MakeUnique<HttpProxyClientSocketPool>(
+        kMaxSockets, kMaxSocketsPerGroup, &transport_socket_pool_,
+        &ssl_socket_pool_, &estimator_, NetLogWithSource().net_log());
+  }
+
   void AddAuthToCache() {
     const base::string16 kFoo(base::ASCIIToUTF16("foo"));
     const base::string16 kBar(base::ASCIIToUTF16("bar"));
     GURL proxy_url(GetParam() == HTTP
                        ? (std::string("http://") + kHttpProxyHost)
                        : (std::string("https://") + kHttpsProxyHost));
     session_->http_auth_cache()->Add(proxy_url,
                                      "MyRealm1",
                                      HttpAuth::AUTH_SCHEME_BASIC,
                                      "Basic realm=MyRealm1",
@@ skipping 90 matching lines @@
   std::unique_ptr<HttpNetworkSession> CreateNetworkSession() {
     return SpdySessionDependencies::SpdyCreateSession(&session_deps_);
   }
 
   RequestPriority GetLastTransportRequestPriority() const {
     return transport_socket_pool_.last_request_priority();
   }
 
   const base::HistogramTester& histogram_tester() { return histogram_tester_; }
 
+  TestNetworkQualityEstimator* estimator() { return &estimator_; }
+
  private:
   SpdySessionDependencies session_deps_;
 
   TestNetworkQualityEstimator estimator_;
 
   MockTransportClientSocketPool transport_socket_pool_;
   MockHostResolver host_resolver_;
   std::unique_ptr<CertVerifier> cert_verifier_;
   SSLClientSocketPool ssl_socket_pool_;
 
   std::unique_ptr<HttpNetworkSession> session_;
 
   base::HistogramTester histogram_tester_;
 
+  base::FieldTrialList field_trial_list_;
+
  protected:
   SpdyTestUtil spdy_util_;
   std::unique_ptr<SSLSocketDataProvider> ssl_data_;
   std::unique_ptr<SequencedSocketData> data_;
-  HttpProxyClientSocketPool pool_;
+  std::unique_ptr<HttpProxyClientSocketPool> pool_;
   ClientSocketHandle handle_;
   TestCompletionCallback callback_;
 };
 
 // All tests are run with three different proxy types: HTTP, HTTPS (non-SPDY)
 // and SPDY.
 INSTANTIATE_TEST_CASE_P(HttpProxyType,
                         HttpProxyClientSocketPoolTest,
                         ::testing::Values(HTTP, HTTPS, SPDY));
 
 TEST_P(HttpProxyClientSocketPoolTest, NoTunnel) {
   Initialize(NULL, 0, NULL, 0, NULL, 0, NULL, 0);
 
   std::unique_ptr<TestProxyDelegate> proxy_delegate(new TestProxyDelegate());
   int rv = handle_.Init("a", CreateNoTunnelParams(proxy_delegate.get()), LOW,
                         ClientSocketPool::RespectLimits::ENABLED,
-                        CompletionCallback(), &pool_, NetLogWithSource());
+                        CompletionCallback(), pool_.get(), NetLogWithSource());
   EXPECT_THAT(rv, IsOk());
   EXPECT_TRUE(handle_.is_initialized());
   ASSERT_TRUE(handle_.socket());
   EXPECT_TRUE(handle_.socket()->IsConnected());
   EXPECT_FALSE(proxy_delegate->on_before_tunnel_request_called());
   EXPECT_FALSE(proxy_delegate->on_tunnel_headers_received_called());
   EXPECT_TRUE(proxy_delegate->on_tunnel_request_completed_called());
 
   bool is_secure_proxy = GetParam() == HTTPS || GetParam() == SPDY;
   histogram_tester().ExpectTotalCount(
       "Net.HttpProxy.ConnectLatency.Insecure.Success", is_secure_proxy ? 0 : 1);
   histogram_tester().ExpectTotalCount(
       "Net.HttpProxy.ConnectLatency.Secure.Success", is_secure_proxy ? 1 : 0);
 }
 
 // Make sure that HttpProxyConnectJob passes on its priority to its
 // (non-SSL) socket request on Init.
 TEST_P(HttpProxyClientSocketPoolTest, SetSocketRequestPriorityOnInit) {
   Initialize(NULL, 0, NULL, 0, NULL, 0, NULL, 0);
-  EXPECT_EQ(OK, handle_.Init("a", CreateNoTunnelParams(NULL), HIGHEST,
-                             ClientSocketPool::RespectLimits::ENABLED,
-                             CompletionCallback(), &pool_, NetLogWithSource()));
+  EXPECT_EQ(
+      OK, handle_.Init("a", CreateNoTunnelParams(NULL), HIGHEST,
+                       ClientSocketPool::RespectLimits::ENABLED,
+                       CompletionCallback(), pool_.get(), NetLogWithSource()));
   EXPECT_EQ(HIGHEST, GetLastTransportRequestPriority());
 }
 
 TEST_P(HttpProxyClientSocketPoolTest, NeedAuth) {
   MockWrite writes[] = {
       MockWrite(ASYNC, 0,
                 "CONNECT www.google.com:443 HTTP/1.1\r\n"
                 "Host: www.google.com:443\r\n"
                 "Proxy-Connection: keep-alive\r\n\r\n"),
   };
@@ skipping 19 matching lines @@
       spdy_util_.ConstructSpdyReply(1, std::move(resp_block)));
   MockRead spdy_reads[] = {CreateMockRead(resp, 1, ASYNC),
                            MockRead(ASYNC, 0, 3)};
 
   Initialize(reads, arraysize(reads), writes, arraysize(writes),
              spdy_reads, arraysize(spdy_reads), spdy_writes,
              arraysize(spdy_writes));
 
   int rv = handle_.Init("a", CreateTunnelParams(NULL), LOW,
                         ClientSocketPool::RespectLimits::ENABLED,
-                        callback_.callback(), &pool_, NetLogWithSource());
+                        callback_.callback(), pool_.get(), NetLogWithSource());
   EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
 
   rv = callback_.WaitForResult();
   EXPECT_THAT(rv, IsError(ERR_PROXY_AUTH_REQUESTED));
   EXPECT_TRUE(handle_.is_initialized());
   ASSERT_TRUE(handle_.socket());
   ProxyClientSocket* tunnel_socket =
       static_cast<ProxyClientSocket*>(handle_.socket());
@@ skipping 28 matching lines @@
     MockRead(SYNCHRONOUS, 1, "HTTP/1.1 200 Connection Established\r\n\r\n"),
   };
 
   Initialize(reads, arraysize(reads), writes, arraysize(writes), NULL, 0,
              NULL, 0);
   AddAuthToCache();
 
   std::unique_ptr<TestProxyDelegate> proxy_delegate(new TestProxyDelegate());
   int rv = handle_.Init("a", CreateTunnelParams(proxy_delegate.get()), LOW,
                         ClientSocketPool::RespectLimits::ENABLED,
-                        callback_.callback(), &pool_, NetLogWithSource());
+                        callback_.callback(), pool_.get(), NetLogWithSource());
   EXPECT_THAT(rv, IsOk());
   EXPECT_TRUE(handle_.is_initialized());
   ASSERT_TRUE(handle_.socket());
   EXPECT_TRUE(handle_.socket()->IsConnected());
   proxy_delegate->VerifyOnTunnelHeadersReceived(
       "www.google.com:443",
       proxy_host_port.c_str(),
       "HTTP/1.1 200 Connection Established");
   proxy_delegate->VerifyOnTunnelRequestCompleted(
       "www.google.com:443",
@@ skipping 30 matching lines @@
   };
 
   Initialize(reads, arraysize(reads), writes, arraysize(writes),
              spdy_reads, arraysize(spdy_reads), spdy_writes,
              arraysize(spdy_writes));
   AddAuthToCache();
 
   std::unique_ptr<TestProxyDelegate> proxy_delegate(new TestProxyDelegate());
   int rv = handle_.Init("a", CreateTunnelParams(proxy_delegate.get()), LOW,
                         ClientSocketPool::RespectLimits::ENABLED,
-                        callback_.callback(), &pool_, NetLogWithSource());
+                        callback_.callback(), pool_.get(), NetLogWithSource());
   EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
 
   EXPECT_THAT(callback_.WaitForResult(), IsOk());
   EXPECT_TRUE(handle_.is_initialized());
   ASSERT_TRUE(handle_.socket());
   EXPECT_TRUE(handle_.socket()->IsConnected());
   proxy_delegate->VerifyOnTunnelRequestCompleted(
       "www.google.com:443",
@@ skipping 13 matching lines @@
   MockWrite spdy_writes[] = {CreateMockWrite(req, 0, ASYNC)};
   SpdySerializedFrame resp(spdy_util_.ConstructSpdyGetReply(NULL, 0, 1));
   MockRead spdy_reads[] = {CreateMockRead(resp, 1, ASYNC),
                            MockRead(ASYNC, 0, 2)};
 
   Initialize(NULL, 0, NULL, 0,
              spdy_reads, arraysize(spdy_reads),
              spdy_writes, arraysize(spdy_writes));
   AddAuthToCache();
 
-  EXPECT_EQ(ERR_IO_PENDING,
-            handle_.Init("a", CreateTunnelParams(NULL), MEDIUM,
-                         ClientSocketPool::RespectLimits::ENABLED,
-                         callback_.callback(), &pool_, NetLogWithSource()));
+  EXPECT_EQ(
+      ERR_IO_PENDING,
+      handle_.Init("a", CreateTunnelParams(NULL), MEDIUM,
+                   ClientSocketPool::RespectLimits::ENABLED,
+                   callback_.callback(), pool_.get(), NetLogWithSource()));
   EXPECT_EQ(MEDIUM, GetLastTransportRequestPriority());
 
   EXPECT_THAT(callback_.WaitForResult(), IsOk());
 }
 
 TEST_P(HttpProxyClientSocketPoolTest, TCPError) {
   if (GetParam() == SPDY)
     return;
   data_.reset(new SequencedSocketData(NULL, 0, NULL, 0));
   data_->set_connect_data(MockConnect(ASYNC, ERR_CONNECTION_CLOSED));
 
   socket_factory()->AddSocketDataProvider(data_.get());
 
   int rv = handle_.Init("a", CreateTunnelParams(NULL), LOW,
                         ClientSocketPool::RespectLimits::ENABLED,
-                        callback_.callback(), &pool_, NetLogWithSource());
+                        callback_.callback(), pool_.get(), NetLogWithSource());
   EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
 
   EXPECT_THAT(callback_.WaitForResult(), IsError(ERR_PROXY_CONNECTION_FAILED));
 
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
 
   bool is_secure_proxy = GetParam() == HTTPS;
@@ skipping 12 matching lines @@
 
   ssl_data_.reset(new SSLSocketDataProvider(ASYNC,
                                             ERR_CERT_AUTHORITY_INVALID));
   if (GetParam() == SPDY) {
     InitializeSpdySsl();
   }
   socket_factory()->AddSSLSocketDataProvider(ssl_data_.get());
 
   int rv = handle_.Init("a", CreateTunnelParams(NULL), LOW,
                         ClientSocketPool::RespectLimits::ENABLED,
-                        callback_.callback(), &pool_, NetLogWithSource());
+                        callback_.callback(), pool_.get(), NetLogWithSource());
   EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
 
   EXPECT_THAT(callback_.WaitForResult(),
               IsError(ERR_PROXY_CERTIFICATE_INVALID));
 
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
   histogram_tester().ExpectTotalCount(
@@ skipping 11 matching lines @@
 
   ssl_data_.reset(new SSLSocketDataProvider(ASYNC,
                                             ERR_SSL_CLIENT_AUTH_CERT_NEEDED));
   if (GetParam() == SPDY) {
     InitializeSpdySsl();
   }
   socket_factory()->AddSSLSocketDataProvider(ssl_data_.get());
 
   int rv = handle_.Init("a", CreateTunnelParams(NULL), LOW,
                         ClientSocketPool::RespectLimits::ENABLED,
-                        callback_.callback(), &pool_, NetLogWithSource());
+                        callback_.callback(), pool_.get(), NetLogWithSource());
   EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
 
   EXPECT_THAT(callback_.WaitForResult(),
               IsError(ERR_SSL_CLIENT_AUTH_CERT_NEEDED));
 
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
   histogram_tester().ExpectTotalCount(
@@ skipping 22 matching lines @@
     MockRead(ASYNC, ERR_CONNECTION_CLOSED, 1),
   };
 
   Initialize(reads, arraysize(reads), writes, arraysize(writes),
              spdy_reads, arraysize(spdy_reads), spdy_writes,
              arraysize(spdy_writes));
   AddAuthToCache();
 
   int rv = handle_.Init("a", CreateTunnelParams(NULL), LOW,
                         ClientSocketPool::RespectLimits::ENABLED,
-                        callback_.callback(), &pool_, NetLogWithSource());
+                        callback_.callback(), pool_.get(), NetLogWithSource());
   EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
 
   if (GetParam() == SPDY) {
     // SPDY cannot process a headers block unless it's complete and so it
     // returns ERR_CONNECTION_CLOSED in this case.
     EXPECT_THAT(callback_.WaitForResult(), IsError(ERR_CONNECTION_CLOSED));
   } else {
     EXPECT_THAT(callback_.WaitForResult(),
@@ skipping 19 matching lines @@
   MockRead reads[] = {
     MockRead(ASYNC, 1, "HTTP/1.1 100 Continue\r\n\r\n"),
     MockRead(ASYNC, 2, "HTTP/1.1 200 Connection Established\r\n\r\n"),
   };
 
   Initialize(reads, arraysize(reads), writes, arraysize(writes),
              NULL, 0, NULL, 0);
 
   int rv = handle_.Init("a", CreateTunnelParams(NULL), LOW,
                         ClientSocketPool::RespectLimits::ENABLED,
-                        callback_.callback(), &pool_, NetLogWithSource());
+                        callback_.callback(), pool_.get(), NetLogWithSource());
   EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
 
   EXPECT_THAT(callback_.WaitForResult(), IsError(ERR_TUNNEL_CONNECTION_FAILED));
 }
 
 TEST_P(HttpProxyClientSocketPoolTest, TunnelSetupError) {
   MockWrite writes[] = {
       MockWrite(ASYNC, 0,
@@ skipping 18 matching lines @@
       CreateMockRead(resp, 1, ASYNC), MockRead(ASYNC, 0, 3),
   };
 
   Initialize(reads, arraysize(reads), writes, arraysize(writes),
              spdy_reads, arraysize(spdy_reads), spdy_writes,
              arraysize(spdy_writes));
   AddAuthToCache();
 
   int rv = handle_.Init("a", CreateTunnelParams(NULL), LOW,
                         ClientSocketPool::RespectLimits::ENABLED,
-                        callback_.callback(), &pool_, NetLogWithSource());
+                        callback_.callback(), pool_.get(), NetLogWithSource());
   EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
 
   rv = callback_.WaitForResult();
   // All Proxy CONNECT responses are not trustworthy
   EXPECT_THAT(rv, IsError(ERR_TUNNEL_CONNECTION_FAILED));
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
 }
@@ skipping 36 matching lines @@
       CreateMockRead(resp, 1, ASYNC), MockRead(ASYNC, 0, 2),
   };
 
   Initialize(reads, arraysize(reads), writes, arraysize(writes),
              spdy_reads, arraysize(spdy_reads), spdy_writes,
              arraysize(spdy_writes));
   AddAuthToCache();
 
   int rv = handle_.Init("a", CreateTunnelParams(NULL), LOW,
                         ClientSocketPool::RespectLimits::ENABLED,
-                        callback_.callback(), &pool_, NetLogWithSource());
+                        callback_.callback(), pool_.get(), NetLogWithSource());
   EXPECT_THAT(rv, IsError(ERR_IO_PENDING));
   EXPECT_FALSE(handle_.is_initialized());
   EXPECT_FALSE(handle_.socket());
 
   rv = callback_.WaitForResult();
 
   if (GetParam() == HTTP) {
     // We don't trust 302 responses to CONNECT from HTTP proxies.
     EXPECT_THAT(rv, IsError(ERR_TUNNEL_CONNECTION_FAILED));
     EXPECT_FALSE(handle_.is_initialized());
@@ skipping 16 matching lines @@
     EXPECT_TRUE(headers->HasHeaderValue("content-length", "0"));
 
     // Make sure Location header was included and correct.
     std::string location;
     EXPECT_TRUE(headers->IsRedirect(&location));
     EXPECT_EQ(location, redirectTarget);
   }
 }
 
 TEST_P(HttpProxyClientSocketPoolTest, ProxyPoolTimeout) {
-  ASSERT_LE(base::TimeDelta(), pool_.ConnectionTimeout());
+  EXPECT_LE(base::TimeDelta(), pool_->ConnectionTimeout());
 
   // Test against a large value.
-  ASSERT_GE(base::TimeDelta::FromMinutes(10), pool_.ConnectionTimeout());
+  EXPECT_GE(base::TimeDelta::FromMinutes(10), pool_->ConnectionTimeout());
 
 #if (defined(OS_ANDROID) || defined(OS_IOS))
   // On Android and iOS, the timeout is fixed to 10 seconds.
-  ASSERT_EQ(base::TimeDelta::FromSeconds(10), pool_.ConnectionTimeout());
+  EXPECT_EQ(base::TimeDelta::FromSeconds(10), pool_->ConnectionTimeout());
 #endif
 }
 
+TEST_P(HttpProxyClientSocketPoolTest, ProxyPoolTimeoutWithExperiment) {
+  int transport_rtt_multiplier = 2;
+  base::TimeDelta min_timeout = base::TimeDelta::FromSeconds(8);
+  base::TimeDelta max_timeout = base::TimeDelta::FromSeconds(20);
+
+  InitAdaptiveTimeoutFieldTrialWithParams(transport_rtt_multiplier, min_timeout,
+                                          max_timeout);
+  EXPECT_LE(base::TimeDelta(), pool_->ConnectionTimeout());
+
+  // Timeout should be |transport_rtt_multiplier| times the transport RTT
+  // estimate.
+  base::TimeDelta rtt_estimate = base::TimeDelta::FromSeconds(7);
+  estimator()->set_start_time_null_transport_rtt(rtt_estimate);
+  EXPECT_EQ(rtt_estimate + rtt_estimate, pool_->ConnectionTimeout());
+
+  rtt_estimate = base::TimeDelta::FromSeconds(8);
+  estimator()->set_start_time_null_transport_rtt(rtt_estimate);
+  EXPECT_EQ(rtt_estimate + rtt_estimate, pool_->ConnectionTimeout());
+
+  // Connection timeout should not exceed |max_timeout|.
+  rtt_estimate = base::TimeDelta::FromSeconds(25);
+  estimator()->set_start_time_null_transport_rtt(rtt_estimate);
+  EXPECT_EQ(max_timeout, pool_->ConnectionTimeout());
+
+  // Connection timeout should not be less than |min_timeout|.
+  rtt_estimate = base::TimeDelta::FromSeconds(0);
+  estimator()->set_start_time_null_transport_rtt(rtt_estimate);
+  EXPECT_EQ(min_timeout, pool_->ConnectionTimeout());
+}

[mmenke, 2017/06/13 19:52:01]

Suggest a second set of three test with different parameters, just to be
paranoid.

[tbansal1, 2017/06/13 23:02:48]

Done.

+
 // It would be nice to also test the timeouts in HttpProxyClientSocketPool.
 
 }  // namespace net