QuicConnection methods call CloseConnection rather than
TearDownLocalConnectionState
Some of the QuicConnection methods call TearDownLocalConnectionState
rather than CloseConnection. This changes then all to call CloseConnection,
specifying ConnectionCloseBehavior::SILENT_CLOSE.
Some tests modified to expect calls to CloseConnection (preventing test failure due to unexpected calls). Before this CL the connection closing happened via internal calls directly to TearDownLocalConnectionState; now these calls go through CloseConnection and trigger the expect.
This is done in preparation for landing another CL adding IETF QUIC Connection Close functionality.
QuicConnection::OnUnrecoverableError (which is an override of QuicPacketCreator::OnUnrecoverableError) called TearDownLocalConnectionState. It had to be modified to call CloseConnection. OnUnrecoverableError has a ConnectionCloseSource parameter, which CloseConnection does not take. Thus the parameter was removed from OnUnrecoverableError. This is OK since all of the calls to OnUnrecoverableError specified FROM_SELF as the source. CloseConnection uses FROM_SELF as the source (unless the error code is a stateless reject or public reset).
gfe-relnote: No flag protection as it is only moving existing functionality around.
PiperOrigin-RevId: 250341745
Change-Id: Iafe499eda682f3db678d0eddbf7e2e3e4367cebd
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc
index d40ad73..942688a 100644
--- a/quic/core/http/quic_spdy_session_test.cc
+++ b/quic/core/http/quic_spdy_session_test.cc
@@ -433,6 +433,7 @@
}
TEST_P(QuicSpdySessionTestServer, IsCryptoHandshakeConfirmed) {
+ EXPECT_CALL(*connection_, CloseConnection(_, _, _));
EXPECT_FALSE(session_.IsCryptoHandshakeConfirmed());
CryptoHandshakeMessage message;
session_.GetMutableCryptoStream()->OnHandshakeMessage(message);
@@ -1020,6 +1021,7 @@
}
TEST_P(QuicSpdySessionTestServer, IncreasedTimeoutAfterCryptoHandshake) {
+ EXPECT_CALL(*connection_, CloseConnection(_, _, _));
EXPECT_EQ(kInitialIdleTimeoutSecs + 3,
QuicConnectionPeer::GetNetworkTimeout(connection_).ToSeconds());
CryptoHandshakeMessage msg;
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index db717b0..6afc62d 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -632,8 +632,8 @@
if (perspective_ == Perspective::IS_CLIENT) {
const std::string error_details = "Protocol version mismatch.";
QUIC_BUG << ENDPOINT << error_details;
- TearDownLocalConnectionState(QUIC_INTERNAL_ERROR, error_details,
- ConnectionCloseSource::FROM_SELF);
+ CloseConnection(QUIC_INTERNAL_ERROR, error_details,
+ ConnectionCloseBehavior::SILENT_CLOSE);
return false;
}
if (no_version_negotiation_) {
@@ -706,8 +706,8 @@
"Server received version negotiation packet.";
QUIC_BUG << error_details;
QUIC_CODE_COUNT(quic_tear_down_local_connection_on_version_negotiation);
- TearDownLocalConnectionState(QUIC_INTERNAL_ERROR, error_details,
- ConnectionCloseSource::FROM_SELF);
+ CloseConnection(QUIC_INTERNAL_ERROR, error_details,
+ ConnectionCloseBehavior::SILENT_CLOSE);
return;
}
if (debug_visitor_ != nullptr) {
@@ -724,9 +724,8 @@
"Server already supports client's version and should have accepted the "
"connection.";
QUIC_DLOG(WARNING) << error_details;
- TearDownLocalConnectionState(QUIC_INVALID_VERSION_NEGOTIATION_PACKET,
- error_details,
- ConnectionCloseSource::FROM_SELF);
+ CloseConnection(QUIC_INVALID_VERSION_NEGOTIATION_PACKET, error_details,
+ ConnectionCloseBehavior::SILENT_CLOSE);
return;
}
@@ -766,8 +765,8 @@
ParsedQuicVersionToString(original_version) + " and " +
ParsedQuicVersionToString(version()) + " is currently unsupported.";
QUIC_DLOG(WARNING) << error_details;
- TearDownLocalConnectionState(QUIC_INVALID_VERSION, error_details,
- ConnectionCloseSource::FROM_SELF);
+ CloseConnection(QUIC_INVALID_VERSION, error_details,
+ ConnectionCloseBehavior::SILENT_CLOSE);
return;
}
@@ -2735,8 +2734,8 @@
QUIC_CODE_COUNT(
quic_tear_down_local_connection_on_write_error_non_ietf);
}
- TearDownLocalConnectionState(QUIC_PACKET_WRITE_ERROR, error_details,
- ConnectionCloseSource::FROM_SELF);
+ CloseConnection(QUIC_PACKET_WRITE_ERROR, error_details,
+ ConnectionCloseBehavior::SILENT_CLOSE);
}
}
@@ -2747,7 +2746,7 @@
void QuicConnection::OnSerializedPacket(SerializedPacket* serialized_packet) {
if (serialized_packet->encrypted_buffer == nullptr) {
// We failed to serialize the packet, so close the connection.
- // TearDownLocalConnectionState does not send close packet, so no infinite
+ // Specify that the close is silent, that no packet be sent, so no infinite
// loop here.
// TODO(ianswett): This is actually an internal error, not an
// encryption failure.
@@ -2758,10 +2757,9 @@
QUIC_CODE_COUNT(
quic_tear_down_local_connection_on_serialized_packet_non_ietf);
}
- TearDownLocalConnectionState(
- QUIC_ENCRYPTION_FAILURE,
- "Serialized packet does not have an encrypted buffer.",
- ConnectionCloseSource::FROM_SELF);
+ CloseConnection(QUIC_ENCRYPTION_FAILURE,
+ "Serialized packet does not have an encrypted buffer.",
+ ConnectionCloseBehavior::SILENT_CLOSE);
return;
}
@@ -2777,8 +2775,7 @@
}
void QuicConnection::OnUnrecoverableError(QuicErrorCode error,
- const std::string& error_details,
- ConnectionCloseSource source) {
+ const std::string& error_details) {
// The packet creator or generator encountered an unrecoverable error: tear
// down local connection state immediately.
if (transport_version() > QUIC_VERSION_43) {
@@ -2788,7 +2785,7 @@
QUIC_CODE_COUNT(
quic_tear_down_local_connection_on_unrecoverable_error_non_ietf);
}
- TearDownLocalConnectionState(error, error_details, source);
+ CloseConnection(error, error_details, ConnectionCloseBehavior::SILENT_CLOSE);
}
void QuicConnection::OnCongestionChange() {
@@ -3120,6 +3117,7 @@
// Regard stateless rejected connection as closed by server.
source = ConnectionCloseSource::FROM_PEER;
}
+
TearDownLocalConnectionState(error, error_details, source);
}
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index 3126e0b..9392f26 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -535,8 +535,7 @@
char* GetPacketBuffer() override;
void OnSerializedPacket(SerializedPacket* packet) override;
void OnUnrecoverableError(QuicErrorCode error,
- const std::string& error_details,
- ConnectionCloseSource source) override;
+ const std::string& error_details) override;
// QuicSentPacketManager::NetworkChangeVisitor
void OnCongestionChange() override;
@@ -974,7 +973,9 @@
typedef std::list<SerializedPacket> QueuedPacketList;
// Notifies the visitor of the close and marks the connection as disconnected.
- // Does not send a connection close frame to the peer.
+ // Does not send a connection close frame to the peer. It should only be
+ // called by CloseConnection or OnConnectionCloseFrame, OnPublicResetPacket,
+ // and OnAuthenticatedIetfStatelessResetPacket.
void TearDownLocalConnectionState(QuicErrorCode error,
const std::string& details,
ConnectionCloseSource source);
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc
index 0833a27..609c6b6 100644
--- a/quic/core/quic_dispatcher.cc
+++ b/quic/core/quic_dispatcher.cc
@@ -73,8 +73,7 @@
}
void OnUnrecoverableError(QuicErrorCode error,
- const std::string& error_details,
- ConnectionCloseSource source) override {}
+ const std::string& error_details) override {}
void SaveStatelessRejectFrameData(QuicStringPiece reject) {
struct iovec iovec;
diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc
index 9f9a68f..5a53ffd 100644
--- a/quic/core/quic_packet_creator.cc
+++ b/quic/core/quic_packet_creator.cc
@@ -200,8 +200,7 @@
QUIC_BUG << error_details << " Constructed stream frame length: "
<< frame->stream_frame.data_length
<< " CHLO length: " << data_size;
- delegate_->OnUnrecoverableError(QUIC_CRYPTO_CHLO_TOO_LARGE, error_details,
- ConnectionCloseSource::FROM_SELF);
+ delegate_->OnUnrecoverableError(QUIC_CRYPTO_CHLO_TOO_LARGE, error_details);
return false;
}
if (!AddFrame(*frame, /*save_retransmittable_frames=*/true,
@@ -384,8 +383,7 @@
const std::string error_details = "Failed to SerializePacket.";
QUIC_BUG << error_details;
delegate_->OnUnrecoverableError(QUIC_FAILED_TO_SERIALIZE_PACKET,
- error_details,
- ConnectionCloseSource::FROM_SELF);
+ error_details);
return;
}
@@ -897,8 +895,7 @@
QuicUtils::EncryptionLevelToString(packet_.encryption_level));
QUIC_BUG << error_details;
delegate_->OnUnrecoverableError(
- QUIC_ATTEMPT_TO_SEND_UNENCRYPTED_STREAM_DATA, error_details,
- ConnectionCloseSource::FROM_SELF);
+ QUIC_ATTEMPT_TO_SEND_UNENCRYPTED_STREAM_DATA, error_details);
return false;
}
size_t frame_len = framer_->GetSerializedFrameLength(
diff --git a/quic/core/quic_packet_creator.h b/quic/core/quic_packet_creator.h
index 06acfbe..c9e31ac 100644
--- a/quic/core/quic_packet_creator.h
+++ b/quic/core/quic_packet_creator.h
@@ -41,8 +41,7 @@
// Called when an unrecoverable error is encountered.
virtual void OnUnrecoverableError(QuicErrorCode error,
- const std::string& error_details,
- ConnectionCloseSource source) = 0;
+ const std::string& error_details) = 0;
};
// Interface which gets callbacks from the QuicPacketCreator at interesting
diff --git a/quic/core/quic_packet_creator_test.cc b/quic/core/quic_packet_creator_test.cc
index 655d587..bf69563 100644
--- a/quic/core/quic_packet_creator_test.cc
+++ b/quic/core/quic_packet_creator_test.cc
@@ -1402,7 +1402,7 @@
}
creator_.set_encryption_level(ENCRYPTION_INITIAL);
- EXPECT_CALL(delegate_, OnUnrecoverableError(_, _, _));
+ EXPECT_CALL(delegate_, OnUnrecoverableError(_, _));
QuicStreamFrame stream_frame(
QuicUtils::GetHeadersStreamId(client_framer_.transport_version()),
/*fin=*/false, 0u, QuicStringPiece());
@@ -1418,7 +1418,7 @@
}
creator_.set_encryption_level(ENCRYPTION_HANDSHAKE);
- EXPECT_CALL(delegate_, OnUnrecoverableError(_, _, _));
+ EXPECT_CALL(delegate_, OnUnrecoverableError(_, _));
QuicStreamFrame stream_frame(
QuicUtils::GetHeadersStreamId(client_framer_.transport_version()),
/*fin=*/false, 0u, QuicStringPiece());
@@ -1451,8 +1451,7 @@
MakeIOVector(QuicStringPiece(message_data->data(), message_data->length()),
&iov);
QuicFrame frame;
- EXPECT_CALL(delegate_,
- OnUnrecoverableError(QUIC_CRYPTO_CHLO_TOO_LARGE, _, _));
+ EXPECT_CALL(delegate_, OnUnrecoverableError(QUIC_CRYPTO_CHLO_TOO_LARGE, _));
EXPECT_QUIC_BUG(creator_.ConsumeData(QuicUtils::GetCryptoStreamId(
client_framer_.transport_version()),
&iov, 1u, iov.iov_len, 0u, 0u, false,
diff --git a/quic/core/quic_packet_generator.cc b/quic/core/quic_packet_generator.cc
index 6f578f5..d57aad4 100644
--- a/quic/core/quic_packet_generator.cc
+++ b/quic/core/quic_packet_generator.cc
@@ -302,8 +302,7 @@
QUIC_LOG(INFO) << queued_control_frames_[0];
}
delegate_->OnUnrecoverableError(QUIC_FAILED_TO_SERIALIZE_PACKET,
- "Single frame cannot fit into a packet",
- ConnectionCloseSource::FROM_SELF);
+ "Single frame cannot fit into a packet");
return;
}
}
diff --git a/quic/core/quic_packet_generator_test.cc b/quic/core/quic_packet_generator_test.cc
index e1899ff..7212e22 100644
--- a/quic/core/quic_packet_generator_test.cc
+++ b/quic/core/quic_packet_generator_test.cc
@@ -51,8 +51,7 @@
MOCK_METHOD1(PopulateStopWaitingFrame, void(QuicStopWaitingFrame*));
MOCK_METHOD0(GetPacketBuffer, char*());
MOCK_METHOD1(OnSerializedPacket, void(SerializedPacket* packet));
- MOCK_METHOD3(OnUnrecoverableError,
- void(QuicErrorCode, const std::string&, ConnectionCloseSource));
+ MOCK_METHOD2(OnUnrecoverableError, void(QuicErrorCode, const std::string&));
void SetCanWriteAnything() {
EXPECT_CALL(*this, ShouldGeneratePacket(_, _)).WillRepeatedly(Return(true));
@@ -1452,8 +1451,7 @@
// This will not serialize any packets, because of the invalid frame.
EXPECT_CALL(delegate_,
- OnUnrecoverableError(QUIC_FAILED_TO_SERIALIZE_PACKET, _,
- ConnectionCloseSource::FROM_SELF));
+ OnUnrecoverableError(QUIC_FAILED_TO_SERIALIZE_PACKET, _));
EXPECT_QUIC_BUG(generator_.Flush(),
"packet_number_length 1 is too small "
"for least_unacked_delta: 1001");
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h
index a889804..4f6a30a 100644
--- a/quic/test_tools/quic_test_utils.h
+++ b/quic/test_tools/quic_test_utils.h
@@ -1046,10 +1046,7 @@
MOCK_METHOD0(GetPacketBuffer, char*());
MOCK_METHOD1(OnSerializedPacket, void(SerializedPacket* packet));
- MOCK_METHOD3(OnUnrecoverableError,
- void(QuicErrorCode,
- const std::string&,
- ConnectionCloseSource source));
+ MOCK_METHOD2(OnUnrecoverableError, void(QuicErrorCode, const std::string&));
};
class MockSessionNotifier : public SessionNotifierInterface {
diff --git a/quic/test_tools/simple_session_notifier_test.cc b/quic/test_tools/simple_session_notifier_test.cc
index e5298eb..d2aad9f 100644
--- a/quic/test_tools/simple_session_notifier_test.cc
+++ b/quic/test_tools/simple_session_notifier_test.cc
@@ -241,6 +241,7 @@
EXPECT_CALL(connection_, SendCryptoData(ENCRYPTION_INITIAL, 1024, 0))
.WillOnce(Invoke(&connection_,
&MockQuicConnection::QuicConnection_SendCryptoData));
+ EXPECT_CALL(connection_, CloseConnection(QUIC_PACKET_WRITE_ERROR, _, _));
producer.SaveCryptoData(ENCRYPTION_INITIAL, 0, std::string(1024, 'a'));
producer.SaveCryptoData(ENCRYPTION_INITIAL, 500, std::string(524, 'a'));
notifier_.WriteCryptoData(ENCRYPTION_INITIAL, 1024, 0);
