Remove QUIC silent close negotiation
The ability to negotiate silent close was never used and is not defined in the IETF QUIC specification. This CL removes this negotiation and makes us always use silent close once the handshake is complete. During the handshake we will continue to send connection close packets.
gfe-relnote: n/a, remove unused code
PiperOrigin-RevId: 309320830
Change-Id: Id3ca8e408a46533d434bbb1b69656e0331f9b16f
diff --git a/quic/core/crypto/crypto_handshake_message.cc b/quic/core/crypto/crypto_handshake_message.cc
index e0c2528..2c92923 100644
--- a/quic/core/crypto/crypto_handshake_message.cc
+++ b/quic/core/crypto/crypto_handshake_message.cc
@@ -290,7 +290,6 @@
case kIRTT:
case kMIUS:
case kMIBS:
- case kSCLS:
case kTCID:
case kMAD:
// uint32_t value
diff --git a/quic/core/crypto/crypto_protocol.h b/quic/core/crypto/crypto_protocol.h
index e2a48cb..74b9604 100644
--- a/quic/core/crypto/crypto_protocol.h
+++ b/quic/core/crypto/crypto_protocol.h
@@ -27,7 +27,7 @@
typedef std::string ServerConfigID;
// The following tags have been deprecated and should not be reused:
-// "1CON", "BBQ4", "NCON", "RCID", "SREJ", "TBKP", "TB10"
+// "1CON", "BBQ4", "NCON", "RCID", "SREJ", "TBKP", "TB10", "SCLS"
// clang-format off
const QuicTag kCHLO = TAG('C', 'H', 'L', 'O'); // Client hello
@@ -311,7 +311,6 @@
const QuicTag kCOPT = TAG('C', 'O', 'P', 'T'); // Connection options
const QuicTag kCLOP = TAG('C', 'L', 'O', 'P'); // Client connection options
const QuicTag kICSL = TAG('I', 'C', 'S', 'L'); // Idle network timeout
-const QuicTag kSCLS = TAG('S', 'C', 'L', 'S'); // Silently close on timeout
const QuicTag kMIBS = TAG('M', 'I', 'D', 'S'); // Max incoming bidi streams
const QuicTag kMIUS = TAG('M', 'I', 'U', 'S'); // Max incoming unidi streams
const QuicTag kADE = TAG('A', 'D', 'E', 0); // Ack Delay Exponent (IETF
diff --git a/quic/core/quic_config.cc b/quic/core/quic_config.cc
index 640219a..a9b8440 100644
--- a/quic/core/quic_config.cc
+++ b/quic/core/quic_config.cc
@@ -501,7 +501,6 @@
connection_options_(kCOPT, PRESENCE_OPTIONAL),
client_connection_options_(kCLOP, PRESENCE_OPTIONAL),
idle_network_timeout_seconds_(kICSL, PRESENCE_REQUIRED),
- silent_close_(kSCLS, PRESENCE_OPTIONAL),
max_bidirectional_streams_(kMIBS, PRESENCE_REQUIRED),
max_unidirectional_streams_(kMIUS, PRESENCE_OPTIONAL),
bytes_for_connection_id_(kTCID, PRESENCE_OPTIONAL),
@@ -604,15 +603,6 @@
idle_network_timeout_seconds_.GetUint32());
}
-// TODO(ianswett) Use this for silent close on mobile, or delete.
-QUIC_UNUSED void QuicConfig::SetSilentClose(bool silent_close) {
- silent_close_.set(silent_close ? 1 : 0, silent_close ? 1 : 0);
-}
-
-bool QuicConfig::SilentClose() const {
- return silent_close_.GetUint32() > 0;
-}
-
void QuicConfig::SetMaxBidirectionalStreamsToSend(uint32_t max_streams) {
max_bidirectional_streams_.SetSendValue(max_streams);
}
@@ -923,7 +913,6 @@
void QuicConfig::SetDefaults() {
idle_network_timeout_seconds_.set(kMaximumIdleTimeoutSecs,
kDefaultIdleTimeoutSecs);
- silent_close_.set(1, 0);
SetMaxBidirectionalStreamsToSend(kDefaultMaxStreamsPerConnection);
SetMaxUnidirectionalStreamsToSend(kDefaultMaxStreamsPerConnection);
max_time_before_crypto_handshake_ =
@@ -945,7 +934,6 @@
CryptoHandshakeMessage* out,
QuicTransportVersion transport_version) const {
idle_network_timeout_seconds_.ToHandshakeMessage(out);
- silent_close_.ToHandshakeMessage(out);
// Do not need a version check here, max...bi... will encode
// as "MIDS" -- the max initial dynamic streams tag -- if
// doing some version other than IETF QUIC.
@@ -981,10 +969,6 @@
peer_hello, hello_type, error_details);
}
if (error == QUIC_NO_ERROR) {
- error =
- silent_close_.ProcessPeerHello(peer_hello, hello_type, error_details);
- }
- if (error == QUIC_NO_ERROR) {
error = max_bidirectional_streams_.ProcessPeerHello(peer_hello, hello_type,
error_details);
}
@@ -1099,7 +1083,6 @@
if (!params->google_quic_params) {
params->google_quic_params = std::make_unique<CryptoHandshakeMessage>();
}
- silent_close_.ToHandshakeMessage(params->google_quic_params.get());
initial_round_trip_time_us_.ToHandshakeMessage(
params->google_quic_params.get());
connection_options_.ToHandshakeMessage(params->google_quic_params.get());
@@ -1200,12 +1183,6 @@
const CryptoHandshakeMessage* peer_params = params.google_quic_params.get();
if (peer_params != nullptr) {
- error =
- silent_close_.ProcessPeerHello(*peer_params, hello_type, error_details);
- if (error != QUIC_NO_ERROR) {
- DCHECK(!error_details->empty());
- return error;
- }
error = initial_round_trip_time_us_.ProcessPeerHello(
*peer_params, hello_type, error_details);
if (error != QUIC_NO_ERROR) {
diff --git a/quic/core/quic_config.h b/quic/core/quic_config.h
index eca7a80..96284cb 100644
--- a/quic/core/quic_config.h
+++ b/quic/core/quic_config.h
@@ -338,10 +338,6 @@
QuicTime::Delta IdleNetworkTimeout() const;
- void SetSilentClose(bool silent_close);
-
- bool SilentClose() const;
-
// Sets the max bidirectional stream count that this endpoint supports.
void SetMaxBidirectionalStreamsToSend(uint32_t max_streams);
uint32_t GetMaxBidirectionalStreamsToSend() const;
@@ -561,8 +557,6 @@
// Idle network timeout in seconds.
// Uses the max_idle_timeout transport parameter in IETF QUIC.
QuicNegotiableUint32 idle_network_timeout_seconds_;
- // Whether to use silent close. Defaults to 0 (false) and is otherwise true.
- QuicNegotiableUint32 silent_close_;
// Maximum number of dynamic streams that a Google QUIC connection
// can support or the maximum number of bidirectional streams that
// an IETF QUIC connection can support.
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index b388133..45ce52a 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -407,10 +407,8 @@
// Handshake complete, set handshake timeout to Infinite.
SetNetworkTimeouts(QuicTime::Delta::Infinite(),
config.IdleNetworkTimeout());
- if (config.SilentClose()) {
- idle_timeout_connection_close_behavior_ =
- ConnectionCloseBehavior::SILENT_CLOSE;
- }
+ idle_timeout_connection_close_behavior_ =
+ ConnectionCloseBehavior::SILENT_CLOSE;
} else {
SetNetworkTimeouts(config.max_time_before_crypto_handshake(),
config.max_idle_time_before_crypto_handshake());
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index aa68288..82c354d 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -1380,9 +1380,8 @@
termination_packets_;
// Determines whether or not a connection close packet is sent to the peer
- // after idle timeout due to lack of network activity.
- // This is particularly important on mobile, where waking up the radio is
- // undesirable.
+ // after idle timeout due to lack of network activity. During the handshake,
+ // a connection close packet is sent, but not after.
ConnectionCloseBehavior idle_timeout_connection_close_behavior_;
// When true, close the QUIC connection after 5 RTOs. Due to the min rto of
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 11977e6..f1a2df0 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -5614,12 +5614,11 @@
EXPECT_FALSE(connection_.GetMtuDiscoveryAlarm()->IsSet());
}
-TEST_P(QuicConnectionTest, TimeoutAfterSend) {
+TEST_P(QuicConnectionTest, TimeoutAfterSendDuringHandshake) {
EXPECT_TRUE(connection_.connected());
EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
QuicConfig config;
connection_.SetFromConfig(config);
- EXPECT_FALSE(QuicConnectionPeer::IsSilentCloseEnabled(&connection_));
const QuicTime::Delta initial_idle_timeout =
QuicTime::Delta::FromSeconds(kInitialIdleTimeoutSecs - 1);
@@ -5688,7 +5687,6 @@
EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
QuicConfig config;
connection_.SetFromConfig(config);
- EXPECT_FALSE(QuicConnectionPeer::IsSilentCloseEnabled(&connection_));
const QuicTime start_time = clock_.Now();
const QuicTime::Delta initial_idle_timeout =
@@ -5767,9 +5765,9 @@
TestConnectionCloseQuicErrorCode(QUIC_NETWORK_IDLE_TIMEOUT);
}
-TEST_P(QuicConnectionTest, NewTimeoutAfterSendSilentClose) {
- // Same test as above, but complete a handshake which enables silent close,
- // causing no connection close packet to be sent.
+TEST_P(QuicConnectionTest, TimeoutAfterSendAfterHandshake) {
+ // When the idle timeout fires, verify that by default we do not send any
+ // connection close packets.
EXPECT_TRUE(connection_.connected());
EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
QuicConfig config;
@@ -5791,7 +5789,6 @@
EXPECT_THAT(error, IsQuicNoError());
connection_.SetFromConfig(config);
- EXPECT_TRUE(QuicConnectionPeer::IsSilentCloseEnabled(&connection_));
const QuicTime::Delta default_idle_timeout =
QuicTime::Delta::FromSeconds(kDefaultIdleTimeoutSecs - 1);
@@ -5860,8 +5857,7 @@
if (connection_.PtoEnabled()) {
return;
}
- // Same test as above, but complete a handshake which enables silent close,
- // but sending TLPs causes the connection close to be sent.
+ // Same test as above, but sending TLPs causes a connection close to be sent.
EXPECT_TRUE(connection_.connected());
EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
QuicConfig config;
@@ -5883,7 +5879,6 @@
EXPECT_THAT(error, IsQuicNoError());
connection_.SetFromConfig(config);
- EXPECT_TRUE(QuicConnectionPeer::IsSilentCloseEnabled(&connection_));
const QuicTime::Delta default_idle_timeout =
QuicTime::Delta::FromSeconds(kDefaultIdleTimeoutSecs - 1);
@@ -5923,8 +5918,8 @@
}
TEST_P(QuicConnectionTest, TimeoutAfterSendSilentCloseWithOpenStreams) {
- // Same test as above, but complete a handshake which enables silent close,
- // but having open streams causes the connection close to be sent.
+ // Same test as above, but having open streams causes a connection close
+ // to be sent.
EXPECT_TRUE(connection_.connected());
EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
QuicConfig config;
@@ -5946,7 +5941,6 @@
EXPECT_THAT(error, IsQuicNoError());
connection_.SetFromConfig(config);
- EXPECT_TRUE(QuicConnectionPeer::IsSilentCloseEnabled(&connection_));
const QuicTime::Delta default_idle_timeout =
QuicTime::Delta::FromSeconds(kDefaultIdleTimeoutSecs - 1);
@@ -5989,7 +5983,6 @@
EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
QuicConfig config;
connection_.SetFromConfig(config);
- EXPECT_FALSE(QuicConnectionPeer::IsSilentCloseEnabled(&connection_));
const QuicTime::Delta initial_idle_timeout =
QuicTime::Delta::FromSeconds(kInitialIdleTimeoutSecs - 1);
@@ -6043,7 +6036,6 @@
EXPECT_CALL(*send_algorithm_, SetFromConfig(_, _));
QuicConfig config;
connection_.SetFromConfig(config);
- EXPECT_FALSE(QuicConnectionPeer::IsSilentCloseEnabled(&connection_));
const QuicTime::Delta initial_idle_timeout =
QuicTime::Delta::FromSeconds(kInitialIdleTimeoutSecs - 1);
