Skip to content

Commit

Permalink
Adds ability to disable connection pooling in QUIC.
Browse files Browse the repository at this point in the history
BUG=

Review URL: https://codereview.chromium.org/551903002

Cr-Commit-Position: refs/heads/master@{#293823}
  • Loading branch information
jri authored and Commit bot committed Sep 9, 2014
1 parent 5c27061 commit 584002d
Show file tree
Hide file tree
Showing 8 changed files with 245 additions and 1 deletion.
12 changes: 12 additions & 0 deletions chrome/browser/io_thread.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1038,6 +1038,8 @@ void IOThread::InitializeNetworkSessionParamsFromGlobals(
&params->enable_quic_time_based_loss_detection);
globals.quic_always_require_handshake_confirmation.CopyToIfSet(
&params->quic_always_require_handshake_confirmation);
globals.quic_disable_connection_pooling.CopyToIfSet(
&params->quic_disable_connection_pooling);
globals.enable_quic_port_selection.CopyToIfSet(
&params->enable_quic_port_selection);
globals.quic_max_packet_length.CopyToIfSet(&params->quic_max_packet_length);
Expand Down Expand Up @@ -1153,6 +1155,8 @@ void IOThread::ConfigureQuicGlobals(
quic_trial_params));
globals->quic_always_require_handshake_confirmation.set(
ShouldQuicAlwaysRequireHandshakeConfirmation(quic_trial_params));
globals->quic_disable_connection_pooling.set(
ShouldQuicDisableConnectionPooling(quic_trial_params));
globals->enable_quic_port_selection.set(
ShouldEnableQuicPortSelection(command_line));
globals->quic_connection_options =
Expand Down Expand Up @@ -1336,6 +1340,14 @@ bool IOThread::ShouldQuicAlwaysRequireHandshakeConfirmation(
"true");
}

// static
bool IOThread::ShouldQuicDisableConnectionPooling(
const VariationParameters& quic_trial_params) {
return LowerCaseEqualsASCII(
GetVariationParam(quic_trial_params, "disable_connection_pooling"),
"true");
}

// static
size_t IOThread::GetQuicMaxPacketLength(
const CommandLine& command_line,
Expand Down
5 changes: 5 additions & 0 deletions chrome/browser/io_thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ class IOThread : public content::BrowserThreadDelegate {
Optional<bool> enable_quic_time_based_loss_detection;
Optional<bool> enable_quic_port_selection;
Optional<bool> quic_always_require_handshake_confirmation;
Optional<bool> quic_disable_connection_pooling;
Optional<size_t> quic_max_packet_length;
net::QuicTagVector quic_connection_options;
Optional<std::string> quic_user_agent_id;
Expand Down Expand Up @@ -358,6 +359,10 @@ class IOThread : public content::BrowserThreadDelegate {
static bool ShouldQuicAlwaysRequireHandshakeConfirmation(
const VariationParameters& quic_trial_params);

// Returns true if QUIC should disable connection pooling.
static bool ShouldQuicDisableConnectionPooling(
const VariationParameters& quic_trial_params);

// Returns the maximum length for QUIC packets, based on any flags in
// |command_line| or the field trial. Returns 0 if there is an error
// parsing any of the options, or if the default value should be used.
Expand Down
11 changes: 11 additions & 0 deletions chrome/browser/io_thread_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,7 @@ TEST_F(IOThreadTest, EnableQuicFromFieldTrialGroup) {
params.quic_supported_versions);
EXPECT_EQ(net::QuicTagVector(), params.quic_connection_options);
EXPECT_FALSE(params.quic_always_require_handshake_confirmation);
EXPECT_FALSE(params.quic_disable_connection_pooling);
}

TEST_F(IOThreadTest, EnableQuicFromCommandLine) {
Expand Down Expand Up @@ -331,4 +332,14 @@ TEST_F(IOThreadTest,
EXPECT_TRUE(params.quic_always_require_handshake_confirmation);
}

TEST_F(IOThreadTest,
QuicDisableConnectionPoolingFromFieldTrialParams) {
field_trial_group_ = "Enabled";
field_trial_params_["disable_connection_pooling"] = "true";
ConfigureQuicGlobals();
net::HttpNetworkSession::Params params;
InitializeNetworkSessionParams(&params);
EXPECT_TRUE(params.quic_disable_connection_pooling);
}

} // namespace test
2 changes: 2 additions & 0 deletions net/http/http_network_session.cc
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ HttpNetworkSession::Params::Params()
enable_quic_port_selection(true),
enable_quic_time_based_loss_detection(false),
quic_always_require_handshake_confirmation(false),
quic_disable_connection_pooling(false),
quic_clock(NULL),
quic_random(NULL),
quic_max_packet_length(kDefaultMaxPacketSize),
Expand Down Expand Up @@ -132,6 +133,7 @@ HttpNetworkSession::HttpNetworkSession(const Params& params)
params.enable_quic_port_selection,
params.enable_quic_time_based_loss_detection,
params.quic_always_require_handshake_confirmation,
params.quic_disable_connection_pooling,
params.quic_connection_options),
spdy_session_pool_(params.host_resolver,
params.ssl_config_service,
Expand Down
1 change: 1 addition & 0 deletions net/http/http_network_session.h
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ class NET_EXPORT HttpNetworkSession
bool enable_quic_port_selection;
bool enable_quic_time_based_loss_detection;
bool quic_always_require_handshake_confirmation;
bool quic_disable_connection_pooling;
HostPortPair origin_to_force_quic_on;
QuicClock* quic_clock; // Will be owned by QuicStreamFactory.
QuicRandom* quic_random;
Expand Down
5 changes: 5 additions & 0 deletions net/quic/quic_stream_factory.cc
Original file line number Diff line number Diff line change
Expand Up @@ -486,6 +486,7 @@ QuicStreamFactory::QuicStreamFactory(
bool enable_port_selection,
bool enable_time_based_loss_detection,
bool always_require_handshake_confirmation,
bool disable_connection_pooling,
const QuicTagVector& connection_options)
: require_confirmation_(true),
host_resolver_(host_resolver),
Expand All @@ -503,6 +504,7 @@ QuicStreamFactory::QuicStreamFactory(
enable_port_selection_(enable_port_selection),
always_require_handshake_confirmation_(
always_require_handshake_confirmation),
disable_connection_pooling_(disable_connection_pooling),
port_seed_(random_generator_->RandUint64()),
weak_factory_(this) {
DCHECK(transport_security_state_);
Expand Down Expand Up @@ -584,6 +586,9 @@ bool QuicStreamFactory::OnResolution(
const QuicServerId& server_id,
const AddressList& address_list) {
DCHECK(!HasActiveSession(server_id));
if (disable_connection_pooling_) {
return false;
}
for (size_t i = 0; i < address_list.size(); ++i) {
const IPEndPoint& address = address_list[i];
const IpAliasKey ip_alias_key(address, server_id.is_https());
Expand Down
4 changes: 4 additions & 0 deletions net/quic/quic_stream_factory.h
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
bool enable_port_selection,
bool enable_time_based_loss_detection,
bool always_require_handshake_confirmation,
bool disable_connection_pooling,
const QuicTagVector& connection_options);
virtual ~QuicStreamFactory();

Expand Down Expand Up @@ -281,6 +282,9 @@ class NET_EXPORT_PRIVATE QuicStreamFactory
// introduce at least one RTT for the handshake before the client sends data.
bool always_require_handshake_confirmation_;

// Set if we do not want connection pooling.
bool disable_connection_pooling_;

// Each profile will (probably) have a unique port_seed_ value. This value is
// used to help seed a pseudo-random number generator (PortSuggester) so that
// we consistently (within this profile) suggest the same ephemeral port when
Expand Down
206 changes: 205 additions & 1 deletion net/quic/quic_stream_factory_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,10 @@ class QuicStreamFactoryPeer {
}
return false;
}

static void DisableConnectionPooling(QuicStreamFactory* factory) {
factory->disable_connection_pooling_ = true;
}
};

class QuicStreamFactoryTest : public ::testing::TestWithParam<QuicVersion> {
Expand All @@ -110,7 +114,8 @@ class QuicStreamFactoryTest : public ::testing::TestWithParam<QuicVersion> {
SupportedVersions(GetParam()),
/*enable_port_selection=*/true,
/*enable_time_based_loss_detection=*/true,
/*always_require_handshake_confirmation=*/true,
/*always_require_handshake_confirmation=*/false,
/*disable_connection_pooling=*/false,
QuicTagVector()),
host_port_pair_(kDefaultServerHostName, kDefaultServerPort),
is_https_(false),
Expand Down Expand Up @@ -418,6 +423,61 @@ TEST_P(QuicStreamFactoryTest, Pooling) {
EXPECT_TRUE(socket_data.at_write_eof());
}

TEST_P(QuicStreamFactoryTest, NoPoolingIfDisabled) {
MockRead reads[] = {
MockRead(ASYNC, OK, 0) // EOF
};
DeterministicSocketData socket_data1(reads, arraysize(reads), NULL, 0);
DeterministicSocketData socket_data2(reads, arraysize(reads), NULL, 0);
socket_factory_.AddSocketDataProvider(&socket_data1);
socket_factory_.AddSocketDataProvider(&socket_data2);
socket_data1.StopAfter(1);
socket_data2.StopAfter(1);

HostPortPair server2("mail.google.com", kDefaultServerPort);
host_resolver_.set_synchronous_mode(true);
host_resolver_.rules()->AddIPLiteralRule(
kDefaultServerHostName, "192.168.0.1", "");
host_resolver_.rules()->AddIPLiteralRule(
"mail.google.com", "192.168.0.1", "");

// Disable connection pooling.
QuicStreamFactoryPeer::DisableConnectionPooling(&factory_);

QuicStreamRequest request(&factory_);
EXPECT_EQ(OK,
request.Request(host_port_pair_,
is_https_,
privacy_mode_,
"GET",
net_log_,
callback_.callback()));
scoped_ptr<QuicHttpStream> stream = request.ReleaseStream();
EXPECT_TRUE(stream.get());

TestCompletionCallback callback;
QuicStreamRequest request2(&factory_);
EXPECT_EQ(OK,
request2.Request(server2,
is_https_,
privacy_mode_,
"GET",
net_log_,
callback.callback()));
scoped_ptr<QuicHttpStream> stream2 = request2.ReleaseStream();
EXPECT_TRUE(stream2.get());

EXPECT_NE(
QuicStreamFactoryPeer::GetActiveSession(
&factory_, host_port_pair_, is_https_),
QuicStreamFactoryPeer::GetActiveSession(&factory_, server2, is_https_));

EXPECT_TRUE(socket_data1.at_read_eof());
EXPECT_TRUE(socket_data1.at_write_eof());
EXPECT_TRUE(socket_data2.at_read_eof());
EXPECT_TRUE(socket_data2.at_write_eof());
}

TEST_P(QuicStreamFactoryTest, NoPoolingAfterGoAway) {
MockRead reads[] = {
MockRead(ASYNC, OK, 0) // EOF
Expand Down Expand Up @@ -548,6 +608,75 @@ TEST_P(QuicStreamFactoryTest, HttpsPooling) {
EXPECT_TRUE(socket_data.at_write_eof());
}

TEST_P(QuicStreamFactoryTest, NoHttpsPoolingIfDisabled) {
MockRead reads[] = {
MockRead(ASYNC, OK, 0) // EOF
};
DeterministicSocketData socket_data1(reads, arraysize(reads), NULL, 0);
DeterministicSocketData socket_data2(reads, arraysize(reads), NULL, 0);
socket_factory_.AddSocketDataProvider(&socket_data1);
socket_factory_.AddSocketDataProvider(&socket_data2);
socket_data1.StopAfter(1);
socket_data2.StopAfter(1);

HostPortPair server1("www.example.org", 443);
HostPortPair server2("mail.example.org", 443);

// Load a cert that is valid for:
// www.example.org (server1)
// mail.example.org (server2)
// www.example.com
base::FilePath certs_dir = GetTestCertsDirectory();
scoped_refptr<X509Certificate> test_cert(
ImportCertFromFile(certs_dir, "spdy_pooling.pem"));
ASSERT_NE(static_cast<X509Certificate*>(NULL), test_cert.get());
ProofVerifyDetailsChromium verify_details;
verify_details.cert_verify_result.verified_cert = test_cert;
verify_details.cert_verify_result.is_issued_by_known_root = true;
crypto_client_stream_factory_.set_proof_verify_details(&verify_details);

host_resolver_.set_synchronous_mode(true);
host_resolver_.rules()->AddIPLiteralRule(server1.host(), "192.168.0.1", "");
host_resolver_.rules()->AddIPLiteralRule(server2.host(), "192.168.0.1", "");

// Disable connection pooling.
QuicStreamFactoryPeer::DisableConnectionPooling(&factory_);

QuicStreamRequest request(&factory_);
is_https_ = true;
EXPECT_EQ(OK,
request.Request(server1,
is_https_,
privacy_mode_,
"GET",
net_log_,
callback_.callback()));
scoped_ptr<QuicHttpStream> stream = request.ReleaseStream();
EXPECT_TRUE(stream.get());

TestCompletionCallback callback;
QuicStreamRequest request2(&factory_);
EXPECT_EQ(OK,
request2.Request(server2,
is_https_,
privacy_mode_,
"GET",
net_log_,
callback_.callback()));
scoped_ptr<QuicHttpStream> stream2 = request2.ReleaseStream();
EXPECT_TRUE(stream2.get());

EXPECT_NE(QuicStreamFactoryPeer::GetActiveSession(
&factory_, server1, is_https_),
QuicStreamFactoryPeer::GetActiveSession(
&factory_, server2, is_https_));

EXPECT_TRUE(socket_data1.at_read_eof());
EXPECT_TRUE(socket_data1.at_write_eof());
EXPECT_TRUE(socket_data2.at_read_eof());
EXPECT_TRUE(socket_data2.at_write_eof());
}

TEST_P(QuicStreamFactoryTest, NoHttpsPoolingWithCertMismatch) {
MockRead reads[] = {
MockRead(ASYNC, OK, 0) // EOF
Expand Down Expand Up @@ -682,6 +811,81 @@ TEST_P(QuicStreamFactoryTest, HttpsPoolingWithMatchingPins) {
EXPECT_TRUE(socket_data.at_write_eof());
}

TEST_P(QuicStreamFactoryTest, NoHttpsPoolingWithMatchingPinsIfDisabled) {
MockRead reads[] = {
MockRead(ASYNC, OK, 0) // EOF
};
DeterministicSocketData socket_data1(reads, arraysize(reads), NULL, 0);
DeterministicSocketData socket_data2(reads, arraysize(reads), NULL, 0);
socket_factory_.AddSocketDataProvider(&socket_data1);
socket_factory_.AddSocketDataProvider(&socket_data2);
socket_data1.StopAfter(1);
socket_data2.StopAfter(1);

HostPortPair server1("www.example.org", 443);
HostPortPair server2("mail.example.org", 443);
uint8 primary_pin = 1;
uint8 backup_pin = 2;
test::AddPin(&transport_security_state_, "mail.example.org", primary_pin,
backup_pin);

// Load a cert that is valid for:
// www.example.org (server1)
// mail.example.org (server2)
base::FilePath certs_dir = GetTestCertsDirectory();
scoped_refptr<X509Certificate> test_cert(
ImportCertFromFile(certs_dir, "spdy_pooling.pem"));
ASSERT_NE(static_cast<X509Certificate*>(NULL), test_cert.get());
ProofVerifyDetailsChromium verify_details;
verify_details.cert_verify_result.verified_cert = test_cert;
verify_details.cert_verify_result.is_issued_by_known_root = true;
verify_details.cert_verify_result.public_key_hashes.push_back(
test::GetTestHashValue(primary_pin));
crypto_client_stream_factory_.set_proof_verify_details(&verify_details);


host_resolver_.set_synchronous_mode(true);
host_resolver_.rules()->AddIPLiteralRule(server1.host(), "192.168.0.1", "");
host_resolver_.rules()->AddIPLiteralRule(server2.host(), "192.168.0.1", "");

// Disable connection pooling.
QuicStreamFactoryPeer::DisableConnectionPooling(&factory_);

QuicStreamRequest request(&factory_);
is_https_ = true;
EXPECT_EQ(OK,
request.Request(server1,
is_https_,
privacy_mode_,
"GET",
net_log_,
callback_.callback()));
scoped_ptr<QuicHttpStream> stream = request.ReleaseStream();
EXPECT_TRUE(stream.get());

TestCompletionCallback callback;
QuicStreamRequest request2(&factory_);
EXPECT_EQ(OK,
request2.Request(server2,
is_https_,
privacy_mode_,
"GET",
net_log_,
callback_.callback()));
scoped_ptr<QuicHttpStream> stream2 = request2.ReleaseStream();
EXPECT_TRUE(stream2.get());

EXPECT_NE(QuicStreamFactoryPeer::GetActiveSession(
&factory_, server1, is_https_),
QuicStreamFactoryPeer::GetActiveSession(
&factory_, server2, is_https_));

EXPECT_TRUE(socket_data1.at_read_eof());
EXPECT_TRUE(socket_data1.at_write_eof());
EXPECT_TRUE(socket_data2.at_read_eof());
EXPECT_TRUE(socket_data2.at_write_eof());
}

TEST_P(QuicStreamFactoryTest, NoHttpsPoolingWithDifferentPins) {
MockRead reads[] = {
MockRead(ASYNC, OK, 0) // EOF
Expand Down

0 comments on commit 584002d

Please sign in to comment.