net/http/http_proxy_client_socket_pool_unittest.cc - Issue 2932993002: Determine proxy connection timeout based on current network quality

Unified Diff: net/http/http_proxy_client_socket_pool_unittest.cc

Issue 2932993002: Determine proxy connection timeout based on current network quality (Closed)

Patch Set: ps Created 3 years, 6 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View side-by-side diff with in-line comments

Index: net/http/http_proxy_client_socket_pool_unittest.cc

diff --git a/net/http/http_proxy_client_socket_pool_unittest.cc b/net/http/http_proxy_client_socket_pool_unittest.cc

index e05b2038caa83631fcd63cff2bd2ccbf570ab46b..b02ff398f8d01d54e0400c27bf7826afc090a41c 100644

--- a/net/http/http_proxy_client_socket_pool_unittest.cc

+++ b/net/http/http_proxy_client_socket_pool_unittest.cc

@@ -4,10 +4,16 @@

#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"

@@ -75,17 +81,41 @@ class HttpProxyClientSocketPoolTest

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

mmenke 2017/06/14 18:10:01 Sorry, was confused by the ssl_socket_pool_ argume

+ 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 i

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"));

@@ -196,6 +226,8 @@ class HttpProxyClientSocketPoolTest

const base::HistogramTester& histogram_tester() { return histogram_tester_; }

+ TestNetworkQualityEstimator* estimator() { return &estimator_; }

private:

SpdySessionDependencies session_deps_;

@@ -210,11 +242,13 @@ class HttpProxyClientSocketPoolTest

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_;

};

@@ -231,7 +265,7 @@ TEST_P(HttpProxyClientSocketPoolTest, NoTunnel) {

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());

@@ -251,9 +285,10 @@ TEST_P(HttpProxyClientSocketPoolTest, NoTunnel) {

// (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());

}

@@ -293,7 +328,7 @@ TEST_P(HttpProxyClientSocketPoolTest, NeedAuth) {

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());

@@ -342,7 +377,7 @@ TEST_P(HttpProxyClientSocketPoolTest, HaveAuth) {

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());

@@ -393,7 +428,7 @@ TEST_P(HttpProxyClientSocketPoolTest, AsyncHaveAuth) {

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());

@@ -427,10 +462,11 @@ TEST_P(HttpProxyClientSocketPoolTest,

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());

@@ -446,7 +482,7 @@ TEST_P(HttpProxyClientSocketPoolTest, TCPError) {

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());

@@ -479,7 +515,7 @@ TEST_P(HttpProxyClientSocketPoolTest, SSLError) {

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());

@@ -511,7 +547,7 @@ TEST_P(HttpProxyClientSocketPoolTest, SslClientAuth) {

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());

@@ -554,7 +590,7 @@ TEST_P(HttpProxyClientSocketPoolTest, TunnelUnexpectedClose) {

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());

@@ -594,7 +630,7 @@ TEST_P(HttpProxyClientSocketPoolTest, Tunnel1xxResponse) {

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());

@@ -633,7 +669,7 @@ TEST_P(HttpProxyClientSocketPoolTest, TunnelSetupError) {

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());

@@ -690,7 +726,7 @@ TEST_P(HttpProxyClientSocketPoolTest, TunnelSetupRedirect) {

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());

@@ -727,17 +763,47 @@ TEST_P(HttpProxyClientSocketPoolTest, TunnelSetupRedirect) {

}

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

tbansal1 2017/06/13 23:02:48 Done.

// It would be nice to also test the timeouts in HttpProxyClientSocketPool.

} // namespace net

« net/http/http_proxy_client_socket_pool.cc ('K') | « net/http/http_proxy_client_socket_pool.cc ('k') | net/nqe/network_quality_estimator_test_util.h » ('j') | no next file with comments »