Remove ack bundling mode.
gfe-relnote: Deprecate gfe2_reloadable_flag_quic_deprecate_ack_bundling_mode.
PiperOrigin-RevId: 253856112
Change-Id: Iaa62b97be5904743c56c5b48b230446a9c4bd870
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index fa60c96..a64b289 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -3612,8 +3612,7 @@
QuicConnectionPeer::GetHelper(client_connection)->GetRandomGenerator();
QuicMemSliceStorage storage(nullptr, 0, nullptr, 0);
{
- QuicConnection::ScopedPacketFlusher flusher(
- client_session->connection(), QuicConnection::SEND_ACK_IF_PENDING);
+ QuicConnection::ScopedPacketFlusher flusher(client_session->connection());
// Verify the largest message gets successfully sent.
EXPECT_EQ(
MessageResult(MESSAGE_STATUS_SUCCESS, 1),
diff --git a/quic/core/http/quic_send_control_stream.cc b/quic/core/http/quic_send_control_stream.cc
index 7003b21..8002b0c 100644
--- a/quic/core/http/quic_send_control_stream.cc
+++ b/quic/core/http/quic_send_control_stream.cc
@@ -25,8 +25,7 @@
void QuicSendControlStream::SendSettingsFrame(const SettingsFrame& settings) {
DCHECK(!settings_sent_);
- QuicConnection::ScopedPacketFlusher flusher(
- session()->connection(), QuicConnection::SEND_ACK_IF_PENDING);
+ QuicConnection::ScopedPacketFlusher flusher(session()->connection());
// Send the stream type on so the peer knows about this stream.
char data[sizeof(kControlStream)];
QuicDataWriter writer(QUIC_ARRAYSIZE(data), data);
diff --git a/quic/core/http/quic_spdy_client_stream.cc b/quic/core/http/quic_spdy_client_stream.cc
index 2903b30..5cbb79d 100644
--- a/quic/core/http/quic_spdy_client_stream.cc
+++ b/quic/core/http/quic_spdy_client_stream.cc
@@ -142,8 +142,7 @@
size_t QuicSpdyClientStream::SendRequest(SpdyHeaderBlock headers,
QuicStringPiece body,
bool fin) {
- QuicConnection::ScopedPacketFlusher flusher(
- session_->connection(), QuicConnection::SEND_ACK_IF_QUEUED);
+ QuicConnection::ScopedPacketFlusher flusher(session_->connection());
bool send_fin_with_headers = fin && body.empty();
size_t bytes_sent = body.size();
header_bytes_written_ =
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index bac53ce..fa43d54 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -218,8 +218,7 @@
SpdyHeaderBlock header_block,
bool fin,
QuicReferenceCountedPointer<QuicAckListenerInterface> ack_listener) {
- QuicConnection::ScopedPacketFlusher flusher(
- spdy_session_->connection(), QuicConnection::SEND_ACK_IF_PENDING);
+ QuicConnection::ScopedPacketFlusher flusher(spdy_session_->connection());
// Send stream type for server push stream
if (VersionHasStreamType(session()->connection()->transport_version()) &&
type() == WRITE_UNIDIRECTIONAL && send_buffer().stream_offset() == 0) {
@@ -256,8 +255,7 @@
WriteOrBufferData(data, fin, nullptr);
return;
}
- QuicConnection::ScopedPacketFlusher flusher(
- spdy_session_->connection(), QuicConnection::SEND_ACK_IF_PENDING);
+ QuicConnection::ScopedPacketFlusher flusher(spdy_session_->connection());
// Write frame header.
std::unique_ptr<char[]> buffer;
@@ -344,8 +342,7 @@
return {0, false};
}
- QuicConnection::ScopedPacketFlusher flusher(
- spdy_session_->connection(), QuicConnection::SEND_ACK_IF_PENDING);
+ QuicConnection::ScopedPacketFlusher flusher(spdy_session_->connection());
// Write frame header.
struct iovec header_iov = {static_cast<void*>(buffer.get()), header_length};
diff --git a/quic/core/qpack/qpack_send_stream.cc b/quic/core/qpack/qpack_send_stream.cc
index 73d62f3..620ab2b 100644
--- a/quic/core/qpack/qpack_send_stream.cc
+++ b/quic/core/qpack/qpack_send_stream.cc
@@ -24,8 +24,7 @@
}
void QpackSendStream::WriteStreamData(QuicStringPiece data) {
- QuicConnection::ScopedPacketFlusher flusher(
- session()->connection(), QuicConnection::SEND_ACK_IF_PENDING);
+ QuicConnection::ScopedPacketFlusher flusher(session()->connection());
if (!stream_type_sent_) {
char type[sizeof(stream_type_)];
QuicDataWriter writer(QUIC_ARRAYSIZE(type), type);
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index e48480c..895227c 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -81,15 +81,12 @@
void OnAlarm() override {
DCHECK(connection_->ack_frame_updated());
- QuicConnection::ScopedPacketFlusher flusher(connection_,
- QuicConnection::SEND_ACK);
- if (connection_->packet_generator().deprecate_ack_bundling_mode()) {
- if (connection_->SupportsMultiplePacketNumberSpaces()) {
- connection_->SendAllPendingAcks();
- } else {
- DCHECK(!connection_->GetUpdatedAckFrame().ack_frame->packets.Empty());
- connection_->SendAck();
- }
+ QuicConnection::ScopedPacketFlusher flusher(connection_);
+ if (connection_->SupportsMultiplePacketNumberSpaces()) {
+ connection_->SendAllPendingAcks();
+ } else {
+ DCHECK(!connection_->GetUpdatedAckFrame().ack_frame->packets.Empty());
+ connection_->SendAck();
}
}
@@ -193,8 +190,7 @@
const ProcessUndecryptablePacketsAlarmDelegate&) = delete;
void OnAlarm() override {
- QuicConnection::ScopedPacketFlusher flusher(connection_,
- QuicConnection::NO_ACK);
+ QuicConnection::ScopedPacketFlusher flusher(connection_);
connection_->MaybeProcessUndecryptablePackets();
}
@@ -263,7 +259,6 @@
close_connection_after_five_rtos_(false),
received_packet_manager_(&stats_),
uber_received_packet_manager_(&stats_),
- ack_queued_(false),
num_retransmittable_packets_received_since_last_ack_sent_(0),
num_packets_received_since_last_ack_sent_(0),
stop_waiting_count_(0),
@@ -366,9 +361,6 @@
supported_versions.size() == 1) {
QUIC_RESTART_FLAG_COUNT(quic_no_server_conn_ver_negotiation2);
}
- if (packet_generator_.deprecate_ack_bundling_mode()) {
- QUIC_RELOADABLE_FLAG_COUNT(quic_deprecate_ack_bundling_mode);
- }
if (received_packet_manager_.decide_when_to_send_acks()) {
QUIC_RELOADABLE_FLAG_COUNT(quic_rpm_decides_when_to_send_acks);
}
@@ -1659,15 +1651,11 @@
sent_packet_manager_.unacked_packets().largest_sent_largest_acked();
if (largest_sent_largest_acked.IsInitialized() &&
last_header_.packet_number < largest_sent_largest_acked) {
- if (packet_generator_.deprecate_ack_bundling_mode()) {
- MaybeSetAckAlarmTo(clock_->ApproximateNow());
- } else {
- ack_queued_ = true;
- }
+ MaybeSetAckAlarmTo(clock_->ApproximateNow());
}
}
- if (should_last_packet_instigate_acks_ && !ack_queued_) {
+ if (should_last_packet_instigate_acks_) {
++num_retransmittable_packets_received_since_last_ack_sent_;
if (ack_mode_ != TCP_ACKING &&
last_header_.packet_number >=
@@ -1677,12 +1665,8 @@
if (!unlimited_ack_decimation_ &&
num_retransmittable_packets_received_since_last_ack_sent_ >=
kMaxRetransmittablePacketsBeforeAck) {
- if (packet_generator_.deprecate_ack_bundling_mode()) {
- MaybeSetAckAlarmTo(clock_->ApproximateNow());
- } else {
- ack_queued_ = true;
- }
- } else if (ShouldSetAckAlarm()) {
+ MaybeSetAckAlarmTo(clock_->ApproximateNow());
+ } else if (!ack_alarm_->IsSet()) {
// Wait for the minimum of the ack decimation delay or the delayed ack
// time before sending an ack.
QuicTime::Delta ack_delay =
@@ -1704,12 +1688,8 @@
// Ack with a timer or every 2 packets by default.
if (num_retransmittable_packets_received_since_last_ack_sent_ >=
ack_frequency_before_ack_decimation_) {
- if (packet_generator_.deprecate_ack_bundling_mode()) {
- MaybeSetAckAlarmTo(clock_->ApproximateNow());
- } else {
- ack_queued_ = true;
- }
- } else if (ShouldSetAckAlarm()) {
+ MaybeSetAckAlarmTo(clock_->ApproximateNow());
+ } else if (!ack_alarm_->IsSet()) {
const QuicTime approximate_now = clock_->ApproximateNow();
if (fast_ack_after_quiescence_ &&
(approximate_now - time_of_previous_received_packet_) >
@@ -1733,15 +1713,11 @@
QuicTime ack_time =
clock_->ApproximateNow() +
0.125 * sent_packet_manager_.GetRttStats()->min_rtt();
- if (ShouldSetAckAlarm() || ack_alarm_->deadline() > ack_time) {
+ if (!ack_alarm_->IsSet() || ack_alarm_->deadline() > ack_time) {
ack_alarm_->Update(ack_time, QuicTime::Delta::Zero());
}
} else {
- if (packet_generator_.deprecate_ack_bundling_mode()) {
- MaybeSetAckAlarmTo(clock_->ApproximateNow());
- } else {
- ack_queued_ = true;
- }
+ MaybeSetAckAlarmTo(clock_->ApproximateNow());
}
}
@@ -1749,10 +1725,6 @@
time_of_previous_received_packet_ = time_of_last_received_packet_;
}
}
-
- if (ack_queued_) {
- ack_alarm_->Cancel();
- }
}
void QuicConnection::ClearLastFrames() {
@@ -1872,7 +1844,7 @@
return 0;
}
- ScopedPacketFlusher flusher(this, SEND_ACK_IF_PENDING);
+ ScopedPacketFlusher flusher(this);
return packet_generator_.ConsumeCryptoData(level, write_length, offset);
}
@@ -1890,7 +1862,7 @@
// which decrypter will be used on an ack packet following a handshake
// packet (a handshake packet from client to server could result in a REJ or a
// SHLO from the server, leading to two different decrypters at the server.)
- ScopedPacketFlusher flusher(this, SEND_ACK_IF_PENDING);
+ ScopedPacketFlusher flusher(this);
return packet_generator_.ConsumeData(id, write_length, offset, state);
}
@@ -1901,7 +1873,7 @@
// Do not check congestion window for ping.
return false;
}
- ScopedPacketFlusher flusher(this, SEND_ACK_IF_PENDING);
+ ScopedPacketFlusher flusher(this);
const bool consumed =
packet_generator_.ConsumeRetransmittableControlFrame(frame);
if (packet_generator_.deprecate_queued_control_frames() && !consumed) {
@@ -1930,7 +1902,7 @@
}
// Flush stream frames of reset stream.
if (packet_generator_.HasPendingStreamFramesOfStream(id)) {
- ScopedPacketFlusher flusher(this, SEND_ACK_IF_PENDING);
+ ScopedPacketFlusher flusher(this);
packet_generator_.FlushAllQueuedFrames();
}
@@ -2039,7 +2011,7 @@
QUIC_DVLOG(1) << ENDPOINT << "time of last received packet: "
<< time_of_last_received_packet_.ToDebuggingValue();
- ScopedPacketFlusher flusher(this, NO_ACK);
+ ScopedPacketFlusher flusher(this);
if (!framer_.ProcessPacket(packet)) {
// If we are unable to decrypt this packet, it might be
// because the CHLO or SHLO packet was lost.
@@ -2100,7 +2072,7 @@
DCHECK(!writer_->IsWriteBlocked());
// Add a flusher to ensure the connection is marked app-limited.
- ScopedPacketFlusher flusher(this, NO_ACK);
+ ScopedPacketFlusher flusher(this);
WriteQueuedPackets();
if (received_packet_manager_.decide_when_to_send_acks()) {
@@ -2123,7 +2095,6 @@
// Send an ACK now because either 1) we were write blocked when we last
// tried to send an ACK, or 2) both ack alarm and send alarm were set to go
// off together.
- DCHECK(packet_generator_.deprecate_ack_bundling_mode());
SendAck();
}
if (!session_decides_what_to_write()) {
@@ -2143,7 +2114,7 @@
}
{
- ScopedPacketFlusher flusher(this, SEND_ACK_IF_QUEUED);
+ ScopedPacketFlusher flusher(this);
visitor_->OnCanWrite();
}
@@ -2166,7 +2137,7 @@
void QuicConnection::WriteAndBundleAcksIfNotBlocked() {
if (!HandleWriteBlocked()) {
- ScopedPacketFlusher flusher(this, SEND_ACK_IF_QUEUED);
+ ScopedPacketFlusher flusher(this);
WriteIfNotBlocked();
}
}
@@ -2338,7 +2309,7 @@
// be moved outside of the loop. Also, CanWrite is not checked after the
// generator is flushed.
{
- ScopedPacketFlusher flusher(this, NO_ACK);
+ ScopedPacketFlusher flusher(this);
packet_generator_.FlushAllQueuedFrames();
}
DCHECK(!packet_generator_.HasQueuedFrames());
@@ -2390,7 +2361,6 @@
}
const QuicFrames QuicConnection::MaybeBundleAckOpportunistically() {
- DCHECK(packet_generator_.deprecate_ack_bundling_mode());
QuicFrames frames;
bool has_pending_ack = false;
if (received_packet_manager_.decide_when_to_send_acks()) {
@@ -2811,8 +2781,7 @@
}
// The client should immediately ack the SHLO to confirm the handshake is
// complete with the server.
- if (perspective_ == Perspective::IS_CLIENT && !ack_queued_ &&
- ack_frame_updated()) {
+ if (perspective_ == Perspective::IS_CLIENT && ack_frame_updated()) {
ack_alarm_->Update(clock_->ApproximateNow(), QuicTime::Delta::Zero());
}
}
@@ -2849,25 +2818,21 @@
ResetAckStates();
}
- if (packet_generator_.deprecate_ack_bundling_mode()) {
- QUIC_DVLOG(1) << ENDPOINT << "Sending an ACK proactively";
- QuicFrames frames;
- frames.push_back(GetUpdatedAckFrame());
- if (!no_stop_waiting_frames_) {
- QuicStopWaitingFrame stop_waiting;
- PopulateStopWaitingFrame(&stop_waiting);
- frames.push_back(QuicFrame(stop_waiting));
+ QUIC_DVLOG(1) << ENDPOINT << "Sending an ACK proactively";
+ QuicFrames frames;
+ frames.push_back(GetUpdatedAckFrame());
+ if (!no_stop_waiting_frames_) {
+ QuicStopWaitingFrame stop_waiting;
+ PopulateStopWaitingFrame(&stop_waiting);
+ frames.push_back(QuicFrame(stop_waiting));
+ }
+ if (received_packet_manager_.decide_when_to_send_acks()) {
+ if (!packet_generator_.FlushAckFrame(frames)) {
+ return;
}
- if (received_packet_manager_.decide_when_to_send_acks()) {
- if (!packet_generator_.FlushAckFrame(frames)) {
- return;
- }
- ResetAckStates();
- } else {
- send_ack_when_on_can_write_ = !packet_generator_.FlushAckFrame(frames);
- }
+ ResetAckStates();
} else {
- packet_generator_.SetShouldSendAck(!no_stop_waiting_frames_);
+ send_ack_when_on_can_write_ = !packet_generator_.FlushAckFrame(frames);
}
if (consecutive_num_packets_with_no_retransmittable_frames_ <
max_consecutive_num_packets_with_no_retransmittable_frames_) {
@@ -2937,7 +2902,7 @@
void QuicConnection::SetDefaultEncryptionLevel(EncryptionLevel level) {
if (level != encryption_level_ && packet_generator_.HasQueuedFrames()) {
// Flush all queued frames when encryption level changes.
- ScopedPacketFlusher flusher(this, NO_ACK);
+ ScopedPacketFlusher flusher(this);
packet_generator_.FlushAllQueuedFrames();
}
encryption_level_ = level;
@@ -3104,12 +3069,11 @@
SetDefaultEncryptionLevel(GetConnectionCloseEncryptionLevel());
ClearQueuedPackets();
// If there was a packet write error, write the smallest close possible.
- AckBundling ack_mode = (error == QUIC_PACKET_WRITE_ERROR) ? NO_ACK : SEND_ACK;
- ScopedPacketFlusher flusher(this, ack_mode);
+ ScopedPacketFlusher flusher(this);
// When multiple packet number spaces is supported, an ACK frame will be
// bundled when connection is not write blocked.
if (!SupportsMultiplePacketNumberSpaces() &&
- packet_generator_.deprecate_ack_bundling_mode() && ack_mode == SEND_ACK &&
+ error != QUIC_PACKET_WRITE_ERROR &&
!GetUpdatedAckFrame().ack_frame->packets.Empty()) {
SendAck();
}
@@ -3344,15 +3308,13 @@
}
void QuicConnection::MaybeSetAckAlarmTo(QuicTime time) {
- DCHECK(packet_generator_.deprecate_ack_bundling_mode());
if (!ack_alarm_->IsSet() || ack_alarm_->deadline() > time) {
ack_alarm_->Update(time, QuicTime::Delta::Zero());
}
}
QuicConnection::ScopedPacketFlusher::ScopedPacketFlusher(
- QuicConnection* connection,
- AckBundling ack_mode)
+ QuicConnection* connection)
: connection_(connection),
flush_and_set_pending_retransmission_alarm_on_delete_(false) {
if (connection_ == nullptr) {
@@ -3363,40 +3325,6 @@
flush_and_set_pending_retransmission_alarm_on_delete_ = true;
connection->packet_generator_.AttachPacketFlusher();
}
- if (connection_->packet_generator_.deprecate_ack_bundling_mode()) {
- return;
- }
-
- // If caller wants us to include an ack, check the delayed-ack timer to see if
- // there's ack info to be sent.
- if (ShouldSendAck(ack_mode)) {
- if (!connection_->GetUpdatedAckFrame().ack_frame->packets.Empty()) {
- QUIC_DVLOG(1) << "Bundling ack with outgoing packet.";
- connection_->SendAck();
- }
- }
-}
-
-bool QuicConnection::ScopedPacketFlusher::ShouldSendAck(
- AckBundling ack_mode) const {
- DCHECK(!connection_->packet_generator_.deprecate_ack_bundling_mode());
- // If the ack alarm is set, make sure the ack has been updated.
- DCHECK(!connection_->ack_alarm_->IsSet() || connection_->ack_frame_updated())
- << "ack_mode:" << ack_mode;
- switch (ack_mode) {
- case SEND_ACK:
- return true;
- case SEND_ACK_IF_QUEUED:
- return connection_->ack_queued();
- case SEND_ACK_IF_PENDING:
- return connection_->ack_alarm_->IsSet() ||
- connection_->stop_waiting_count_ > 1;
- case NO_ACK:
- return false;
- default:
- QUIC_BUG << "Unsupported ack_mode.";
- return true;
- }
}
QuicConnection::ScopedPacketFlusher::~ScopedPacketFlusher() {
@@ -3410,44 +3338,41 @@
}
if (flush_and_set_pending_retransmission_alarm_on_delete_) {
- if (connection_->packet_generator_.deprecate_ack_bundling_mode()) {
- if (connection_->received_packet_manager_.decide_when_to_send_acks()) {
- const QuicTime ack_timeout =
- connection_->use_uber_received_packet_manager_
- ? connection_->uber_received_packet_manager_
- .GetEarliestAckTimeout()
- : connection_->received_packet_manager_.ack_timeout();
- if (ack_timeout.IsInitialized()) {
- if (ack_timeout <= connection_->clock_->ApproximateNow() &&
- !connection_->CanWrite(NO_RETRANSMITTABLE_DATA)) {
- // Cancel ACK alarm if connection is write blocked, and ACK will be
- // sent when connection gets unblocked.
- connection_->ack_alarm_->Cancel();
- } else {
- connection_->MaybeSetAckAlarmTo(ack_timeout);
- }
+ if (connection_->received_packet_manager_.decide_when_to_send_acks()) {
+ const QuicTime ack_timeout =
+ connection_->use_uber_received_packet_manager_
+ ? connection_->uber_received_packet_manager_
+ .GetEarliestAckTimeout()
+ : connection_->received_packet_manager_.ack_timeout();
+ if (ack_timeout.IsInitialized()) {
+ if (ack_timeout <= connection_->clock_->ApproximateNow() &&
+ !connection_->CanWrite(NO_RETRANSMITTABLE_DATA)) {
+ // Cancel ACK alarm if connection is write blocked, and ACK will be
+ // sent when connection gets unblocked.
+ connection_->ack_alarm_->Cancel();
+ } else {
+ connection_->MaybeSetAckAlarmTo(ack_timeout);
}
}
- if (connection_->ack_alarm_->IsSet() &&
- connection_->ack_alarm_->deadline() <=
+ }
+ if (connection_->ack_alarm_->IsSet() &&
+ connection_->ack_alarm_->deadline() <=
+ connection_->clock_->ApproximateNow()) {
+ // An ACK needs to be sent right now. This ACK did not get bundled
+ // because either there was no data to write or packets were marked as
+ // received after frames were queued in the generator.
+ if (connection_->send_alarm_->IsSet() &&
+ connection_->send_alarm_->deadline() <=
connection_->clock_->ApproximateNow()) {
- // An ACK needs to be sent right now. This ACK did not get bundled
- // because either there was no data to write or packets were marked as
- // received after frames were queued in the generator.
- if (connection_->send_alarm_->IsSet() &&
- connection_->send_alarm_->deadline() <=
- connection_->clock_->ApproximateNow()) {
- // If send alarm will go off soon, let send alarm send the ACK.
- connection_->ack_alarm_->Cancel();
- if (!connection_->received_packet_manager_
- .decide_when_to_send_acks()) {
- connection_->send_ack_when_on_can_write_ = true;
- }
- } else if (connection_->SupportsMultiplePacketNumberSpaces()) {
- connection_->SendAllPendingAcks();
- } else {
- connection_->SendAck();
+ // If send alarm will go off soon, let send alarm send the ACK.
+ connection_->ack_alarm_->Cancel();
+ if (!connection_->received_packet_manager_.decide_when_to_send_acks()) {
+ connection_->send_ack_when_on_can_write_ = true;
}
+ } else if (connection_->SupportsMultiplePacketNumberSpaces()) {
+ connection_->SendAllPendingAcks();
+ } else {
+ connection_->SendAck();
}
}
connection_->packet_generator_.Flush();
@@ -3959,7 +3884,6 @@
void QuicConnection::ResetAckStates() {
ack_alarm_->Cancel();
- ack_queued_ = false;
stop_waiting_count_ = 0;
num_retransmittable_packets_received_since_last_ack_sent_ = 0;
num_packets_received_since_last_ack_sent_ = 0;
@@ -3985,7 +3909,7 @@
if (!CanWrite(HAS_RETRANSMITTABLE_DATA)) {
return MESSAGE_STATUS_BLOCKED;
}
- ScopedPacketFlusher flusher(this, SEND_ACK_IF_PENDING);
+ ScopedPacketFlusher flusher(this);
return packet_generator_.AddMessageFrame(message_id, message);
}
@@ -4004,23 +3928,6 @@
return framer_.decrypter()->cipher_id();
}
-bool QuicConnection::ShouldSetAckAlarm() const {
- DCHECK(ack_frame_updated());
- if (ack_alarm_->IsSet()) {
- // ACK alarm has been set.
- return false;
- }
- if (GetQuicReloadableFlag(quic_fix_spurious_ack_alarm) &&
- packet_generator_.should_send_ack()) {
- // If the generator is already configured to send an ACK, then there is no
- // need to schedule the ACK alarm. The updated ACK information will be sent
- // when the generator flushes.
- QUIC_RELOADABLE_FLAG_COUNT(quic_fix_spurious_ack_alarm);
- return false;
- }
- return true;
-}
-
EncryptionLevel QuicConnection::GetConnectionCloseEncryptionLevel() const {
if (perspective_ == Perspective::IS_CLIENT) {
return encryption_level_;
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index dc97b07..186d50f 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -324,19 +324,6 @@
public QuicPacketGenerator::DelegateInterface,
public QuicSentPacketManager::NetworkChangeVisitor {
public:
- // TODO(fayang): Remove this enum when deprecating
- // quic_deprecate_ack_bundling_mode.
- enum AckBundling {
- // Send an ack if it's already queued in the connection.
- SEND_ACK_IF_QUEUED,
- // Always send an ack.
- SEND_ACK,
- // Bundle an ack with outgoing data.
- SEND_ACK_IF_PENDING,
- // Do not send ack.
- NO_ACK,
- };
-
// Constructs a new QuicConnection for |connection_id| and
// |initial_peer_address| using |writer| to write packets. |owns_writer|
// specifies whether the connection takes ownership of |writer|. |helper| must
@@ -526,10 +513,6 @@
bool ShouldGeneratePacket(HasRetransmittableData retransmittable,
IsHandshake handshake) override;
const QuicFrames MaybeBundleAckOpportunistically() override;
- // Please note, this is not a const function. For logging purpose, please use
- // ack_frame().
- const QuicFrame GetUpdatedAckFrame() override;
- void PopulateStopWaitingFrame(QuicStopWaitingFrame* stop_waiting) override;
// QuicPacketCreator::DelegateInterface
char* GetPacketBuffer() override;
@@ -541,6 +524,10 @@
void OnCongestionChange() override;
void OnPathMtuIncreased(QuicPacketLength packet_size) override;
+ // Please note, this is not a const function. For logging purpose, please use
+ // ack_frame().
+ const QuicFrame GetUpdatedAckFrame();
+
// Called by the crypto stream when the handshake completes. In the server's
// case this is when the SHLO has been ACKed. Clients call this on receipt of
// the SHLO.
@@ -707,16 +694,10 @@
// information to be sent.
class QUIC_EXPORT_PRIVATE ScopedPacketFlusher {
public:
- // Setting |include_ack| to true ensures that an ACK frame is
- // opportunistically bundled with the first outgoing packet.
- // TODO(fayang): Remove |ack_mode| when deprecating
- // quic_deprecate_ack_bundling_mode.
- ScopedPacketFlusher(QuicConnection* connection, AckBundling ack_mode);
+ explicit ScopedPacketFlusher(QuicConnection* connection);
~ScopedPacketFlusher();
private:
- bool ShouldSendAck(AckBundling ack_mode) const;
-
QuicConnection* connection_;
// If true, when this flusher goes out of scope, flush connection and set
// retransmission alarm if there is one pending.
@@ -780,8 +761,6 @@
return termination_packets_.get();
}
- bool ack_queued() const { return ack_queued_; }
-
bool ack_frame_updated() const;
QuicConnectionHelperInterface* helper() { return helper_; }
@@ -1116,14 +1095,12 @@
// num_retransmittable_packets_received_since_last_ack_sent_ etc.
void ResetAckStates();
+ void PopulateStopWaitingFrame(QuicStopWaitingFrame* stop_waiting);
+
// Enables multiple packet number spaces support based on handshake protocol
// and flags.
void MaybeEnableMultiplePacketNumberSpacesSupport();
- // Returns true if ack alarm is not set and there is no pending ack in the
- // generator.
- bool ShouldSetAckAlarm() const;
-
// Returns the encryption level the connection close packet should be sent at,
// which is the highest encryption level that peer can guarantee to process.
EncryptionLevel GetConnectionCloseEncryptionLevel() const;
@@ -1276,10 +1253,6 @@
// Used when use_uber_received_packet_manager_ is true.
UberReceivedPacketManager uber_received_packet_manager_;
- // Indicates whether an ack should be sent the next time we try to write.
- // TODO(fayang): Remove ack_queued_ when deprecating
- // quic_deprecate_ack_bundling_mode.
- bool ack_queued_;
// How many retransmittable packets have arrived without sending an ack.
// TODO(fayang): Remove
// num_retransmittable_packets_received_since_last_ack_sent_ when deprecating
@@ -1514,8 +1487,7 @@
// vector to improve performance since it is expected to be very small.
std::vector<QuicConnectionId> incoming_connection_ids_;
- // Indicates whether an ACK needs to be sent in OnCanWrite(). Only used when
- // deprecate_ack_bundling_mode is true.
+ // Indicates whether an ACK needs to be sent in OnCanWrite().
// TODO(fayang): Remove this when ACK sending logic is moved to received
// packet manager, and an ACK timeout would be used to record when an ACK
// needs to be sent.
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 4cc121b..fcefc82 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -647,7 +647,7 @@
size_t total_length,
QuicStreamOffset offset,
StreamSendingState state) {
- ScopedPacketFlusher flusher(this, NO_ACK);
+ ScopedPacketFlusher flusher(this);
producer_.SaveStreamData(id, iov, iov_count, 0u, total_length);
if (notifier_ != nullptr) {
return notifier_->WriteOrBufferData(id, total_length, state);
@@ -659,7 +659,7 @@
QuicStringPiece data,
QuicStreamOffset offset,
StreamSendingState state) {
- ScopedPacketFlusher flusher(this, NO_ACK);
+ ScopedPacketFlusher flusher(this);
if (!QuicUtils::IsCryptoStreamId(transport_version(), id) &&
this->encryption_level() == ENCRYPTION_INITIAL) {
this->SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
@@ -674,7 +674,7 @@
QuicStringPiece data,
QuicStreamOffset offset,
StreamSendingState state) {
- ScopedPacketFlusher flusher(this, NO_ACK);
+ ScopedPacketFlusher flusher(this);
DCHECK(encryption_level >= ENCRYPTION_ZERO_RTT);
SetEncrypter(encryption_level, QuicMakeUnique<TaggingEncrypter>(0x01));
SetDefaultEncryptionLevel(encryption_level);
@@ -1267,8 +1267,7 @@
void SendAckPacketToPeer() {
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
{
- QuicConnection::ScopedPacketFlusher flusher(&connection_,
- QuicConnection::NO_ACK);
+ QuicConnection::ScopedPacketFlusher flusher(&connection_);
connection_.SendAck();
}
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _))
@@ -2930,8 +2929,7 @@
// Send two stream frames in 1 packet by queueing them.
connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
{
- QuicConnection::ScopedPacketFlusher flusher(&connection_,
- QuicConnection::SEND_ACK);
+ QuicConnection::ScopedPacketFlusher flusher(&connection_);
connection_.SendStreamData3();
connection_.SendStreamData5();
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1);
@@ -2964,8 +2962,7 @@
connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
{
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
- QuicConnection::ScopedPacketFlusher flusher(&connection_,
- QuicConnection::SEND_ACK);
+ QuicConnection::ScopedPacketFlusher flusher(&connection_);
connection_.SendStreamData3();
connection_.SendCryptoStreamData();
}
@@ -2990,8 +2987,7 @@
{
connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE);
EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(2);
- QuicConnection::ScopedPacketFlusher flusher(&connection_,
- QuicConnection::SEND_ACK);
+ QuicConnection::ScopedPacketFlusher flusher(&connection_);
connection_.SendCryptoStreamData();
connection_.SendStreamData3();
}
@@ -3702,8 +3698,7 @@
EXPECT_CALL(visitor_, OnWriteBlocked()).Times(0);
{
- QuicConnection::ScopedPacketFlusher flusher(&connection_,
- QuicConnection::NO_ACK);
+ QuicConnection::ScopedPacketFlusher flusher(&connection_);
connection_.CloseConnection(QUIC_PEER_GOING_AWAY, "no reason",
ConnectionCloseBehavior::SILENT_CLOSE);
@@ -3718,8 +3713,7 @@
EXPECT_CALL(visitor_, OnWriteBlocked()).Times(1);
{
- QuicConnection::ScopedPacketFlusher flusher(&connection_,
- QuicConnection::NO_ACK);
+ QuicConnection::ScopedPacketFlusher flusher(&connection_);
// flusher's destructor will call connection_.FlushPackets, which should add
// the connection to the write blocked list.
}
@@ -8015,8 +8009,7 @@
QuicStringPiece message_data(message);
QuicMemSliceStorage storage(nullptr, 0, nullptr, 0);
{
- QuicConnection::ScopedPacketFlusher flusher(&connection_,
- QuicConnection::SEND_ACK);
+ QuicConnection::ScopedPacketFlusher flusher(&connection_);
connection_.SendStreamData3();
// Send a message which cannot fit into current open packet, and 2 packets
// get sent, one contains stream frame, and the other only contains the
diff --git a/quic/core/quic_packet_generator.cc b/quic/core/quic_packet_generator.cc
index 614182c..210e24d 100644
--- a/quic/core/quic_packet_generator.cc
+++ b/quic/core/quic_packet_generator.cc
@@ -26,45 +26,21 @@
packet_creator_(server_connection_id, framer, random_generator, delegate),
next_transmission_type_(NOT_RETRANSMISSION),
flusher_attached_(false),
- should_send_ack_(false),
- should_send_stop_waiting_(false),
random_generator_(random_generator),
fully_pad_crypto_handshake_packets_(true),
- deprecate_ack_bundling_mode_(
- GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)),
deprecate_queued_control_frames_(
- deprecate_ack_bundling_mode_ &&
GetQuicReloadableFlag(quic_deprecate_queued_control_frames)) {}
QuicPacketGenerator::~QuicPacketGenerator() {
DeleteFrames(&queued_control_frames_);
}
-void QuicPacketGenerator::SetShouldSendAck(bool also_send_stop_waiting) {
- DCHECK(!deprecate_ack_bundling_mode_);
- if (packet_creator_.has_ack()) {
- // Ack already queued, nothing to do.
- return;
- }
-
- if (also_send_stop_waiting && packet_creator_.has_stop_waiting()) {
- QUIC_BUG << "Should only ever be one pending stop waiting frame.";
- return;
- }
-
- should_send_ack_ = true;
- should_send_stop_waiting_ = also_send_stop_waiting;
- SendQueuedFrames(/*flush=*/false);
-}
-
bool QuicPacketGenerator::ConsumeRetransmittableControlFrame(
const QuicFrame& frame) {
QUIC_BUG_IF(IsControlFrame(frame.type) && !GetControlFrameId(frame))
<< "Adding a control frame with no control frame id: " << frame;
DCHECK(QuicUtils::IsRetransmittableFrame(frame.type)) << frame;
- if (deprecate_ack_bundling_mode_) {
- MaybeBundleAckOpportunistically();
- }
+ MaybeBundleAckOpportunistically();
if (deprecate_queued_control_frames_) {
QUIC_RELOADABLE_FLAG_COUNT(quic_deprecate_queued_control_frames);
if (packet_creator_.HasPendingFrames()) {
@@ -95,9 +71,7 @@
QuicStreamOffset offset) {
QUIC_BUG_IF(!flusher_attached_) << "Packet flusher is not attached when "
"generator tries to write crypto data.";
- if (deprecate_ack_bundling_mode_) {
- MaybeBundleAckOpportunistically();
- }
+ MaybeBundleAckOpportunistically();
// To make reasoning about crypto frames easier, we don't combine them with
// other retransmittable frames in a single packet.
// TODO(nharper): Once we have separate packet number spaces, everything
@@ -140,9 +114,7 @@
"generator tries to write stream data.";
bool has_handshake =
QuicUtils::IsCryptoStreamId(packet_creator_.transport_version(), id);
- if (deprecate_ack_bundling_mode_) {
- MaybeBundleAckOpportunistically();
- }
+ MaybeBundleAckOpportunistically();
bool fin = state != NO_FIN;
QUIC_BUG_IF(has_handshake && fin)
<< "Handshake packets should never send a fin";
@@ -276,10 +248,8 @@
bool QuicPacketGenerator::CanSendWithNextPendingFrameAddition() const {
DCHECK(HasPendingFrames() || packet_creator_.pending_padding_bytes() > 0);
HasRetransmittableData retransmittable =
- (should_send_ack_ || should_send_stop_waiting_ ||
- packet_creator_.pending_padding_bytes() > 0)
- ? NO_RETRANSMITTABLE_DATA
- : HAS_RETRANSMITTABLE_DATA;
+ packet_creator_.pending_padding_bytes() > 0 ? NO_RETRANSMITTABLE_DATA
+ : HAS_RETRANSMITTABLE_DATA;
if (retransmittable == HAS_RETRANSMITTABLE_DATA) {
DCHECK(!queued_control_frames_.empty()); // These are retransmittable.
}
@@ -294,8 +264,6 @@
if (!AddNextPendingFrame() && first_frame) {
// A single frame cannot fit into the packet, tear down the connection.
QUIC_BUG << "A single frame cannot fit into packet."
- << " should_send_ack: " << should_send_ack_
- << " should_send_stop_waiting: " << should_send_stop_waiting_
<< " number of queued_control_frames: "
<< queued_control_frames_.size();
if (!queued_control_frames_.empty()) {
@@ -353,29 +321,12 @@
}
bool QuicPacketGenerator::HasPendingFrames() const {
- return should_send_ack_ || should_send_stop_waiting_ ||
- !queued_control_frames_.empty();
+ return !queued_control_frames_.empty();
}
bool QuicPacketGenerator::AddNextPendingFrame() {
QUIC_BUG_IF(!flusher_attached_) << "Packet flusher is not attached when "
"generator tries to write control frames.";
- if (should_send_ack_) {
- should_send_ack_ = !packet_creator_.AddSavedFrame(
- delegate_->GetUpdatedAckFrame(), next_transmission_type_);
- return !should_send_ack_;
- }
-
- if (should_send_stop_waiting_) {
- delegate_->PopulateStopWaitingFrame(&pending_stop_waiting_frame_);
- // If we can't this add the frame now, then we still need to do so later.
- should_send_stop_waiting_ = !packet_creator_.AddSavedFrame(
- QuicFrame(pending_stop_waiting_frame_), next_transmission_type_);
- // Return success if we have cleared out this flag (i.e., added the frame).
- // If we still need to send, then the frame is full, and we have failed.
- return !should_send_stop_waiting_;
- }
-
QUIC_BUG_IF(queued_control_frames_.empty())
<< "AddNextPendingFrame called with no queued control frames.";
@@ -512,9 +463,7 @@
QuicMemSliceSpan message) {
QUIC_BUG_IF(!flusher_attached_) << "Packet flusher is not attached when "
"generator tries to add message frame.";
- if (deprecate_ack_bundling_mode_) {
- MaybeBundleAckOpportunistically();
- }
+ MaybeBundleAckOpportunistically();
const QuicByteCount message_length = message.total_length();
if (message_length > GetCurrentLargestMessagePayload()) {
return MESSAGE_STATUS_TOO_LARGE;
@@ -535,7 +484,6 @@
}
void QuicPacketGenerator::MaybeBundleAckOpportunistically() {
- DCHECK(deprecate_ack_bundling_mode_);
if (packet_creator_.has_ack()) {
// Ack already queued, nothing to do.
return;
diff --git a/quic/core/quic_packet_generator.h b/quic/core/quic_packet_generator.h
index 96d2cf2..2f82654 100644
--- a/quic/core/quic_packet_generator.h
+++ b/quic/core/quic_packet_generator.h
@@ -69,11 +69,6 @@
// Called when there is data to be sent. Retrieves updated ACK frame from
// the delegate.
virtual const QuicFrames MaybeBundleAckOpportunistically() = 0;
- // TODO(fayang): Remove these two interfaces when deprecating
- // quic_deprecate_ack_bundling_mode.
- virtual const QuicFrame GetUpdatedAckFrame() = 0;
- virtual void PopulateStopWaitingFrame(
- QuicStopWaitingFrame* stop_waiting) = 0;
};
QuicPacketGenerator(QuicConnectionId server_connection_id,
@@ -85,13 +80,6 @@
~QuicPacketGenerator();
- // Indicates that an ACK frame should be sent.
- // If |also_send_stop_waiting| is true, then it also indicates that a
- // STOP_WAITING frame should be sent as well.
- // The contents of the frame(s) will be generated via a call to the delegate
- // CreateAckFrame() when the packet is serialized.
- void SetShouldSendAck(bool also_send_stop_waiting);
-
// Consumes retransmittable control |frame|. Returns true if the frame is
// successfully consumed. Returns false otherwise.
bool ConsumeRetransmittableControlFrame(const QuicFrame& frame);
@@ -245,8 +233,6 @@
packet_creator_.set_debug_delegate(debug_delegate);
}
- bool should_send_ack() const { return should_send_ack_; }
-
void set_fully_pad_crypto_hadshake_packets(bool new_value) {
fully_pad_crypto_handshake_packets_ = new_value;
}
@@ -255,10 +241,6 @@
return fully_pad_crypto_handshake_packets_;
}
- bool deprecate_ack_bundling_mode() const {
- return deprecate_ack_bundling_mode_;
- }
-
bool deprecate_queued_control_frames() const {
return deprecate_queued_control_frames_;
}
@@ -303,19 +285,6 @@
// True if packet flusher is currently attached.
bool flusher_attached_;
- // Flags to indicate the need for just-in-time construction of a frame.
- // TODO(fayang): Remove these two booleans when deprecating
- // quic_deprecate_ack_bundling_mode.
- bool should_send_ack_;
- bool should_send_stop_waiting_;
- // If we put a non-retransmittable frame in this packet, then we have to hold
- // a reference to it until we flush (and serialize it). Retransmittable frames
- // are referenced elsewhere so that they can later be (optionally)
- // retransmitted.
- // TODO(fayang): Remove this when deprecating
- // quic_deprecate_ack_bundling_mode.
- QuicStopWaitingFrame pending_stop_waiting_frame_;
-
QuicRandom* random_generator_;
// Whether crypto handshake packets should be fully padded.
@@ -326,9 +295,6 @@
// flusher detaches.
QuicPacketNumber write_start_packet_number_;
- // Latched value of quic_deprecate_ack_bundling_mode.
- const bool deprecate_ack_bundling_mode_;
-
// Latched value of quic_deprecate_queued_control_frames.
const bool deprecate_queued_control_frames_;
};
diff --git a/quic/core/quic_packet_generator_test.cc b/quic/core/quic_packet_generator_test.cc
index de2c927..e996c4e 100644
--- a/quic/core/quic_packet_generator_test.cc
+++ b/quic/core/quic_packet_generator_test.cc
@@ -47,8 +47,6 @@
bool(HasRetransmittableData retransmittable,
IsHandshake handshake));
MOCK_METHOD0(MaybeBundleAckOpportunistically, const QuicFrames());
- MOCK_METHOD0(GetUpdatedAckFrame, const QuicFrame());
- MOCK_METHOD1(PopulateStopWaitingFrame, void(QuicStopWaitingFrame*));
MOCK_METHOD0(GetPacketBuffer, char*());
MOCK_METHOD1(OnSerializedPacket, void(SerializedPacket* packet));
MOCK_METHOD2(OnUnrecoverableError, void(QuicErrorCode, const std::string&));
@@ -119,8 +117,7 @@
bool ConsumeRetransmittableControlFrame(const QuicFrame& frame,
bool bundle_ack) {
- if (GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode) &&
- !QuicPacketGeneratorPeer::GetPacketCreator(this)->has_ack()) {
+ if (!QuicPacketGeneratorPeer::GetPacketCreator(this)->has_ack()) {
QuicFrames frames;
if (bundle_ack) {
frames.push_back(QuicFrame(&ack_frame_));
@@ -158,8 +155,7 @@
if (total_length > 0) {
producer_->SaveStreamData(id, iov, iov_count, 0, total_length);
}
- if (GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode) &&
- !QuicPacketGeneratorPeer::GetPacketCreator(this)->has_ack() &&
+ if (!QuicPacketGeneratorPeer::GetPacketCreator(this)->has_ack() &&
delegate_->ShouldGeneratePacket(NO_RETRANSMITTABLE_DATA,
NOT_HANDSHAKE)) {
EXPECT_CALL(*delegate_, MaybeBundleAckOpportunistically()).Times(1);
@@ -169,8 +165,7 @@
MessageStatus AddMessageFrame(QuicMessageId message_id,
QuicMemSliceSpan message) {
- if (GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode) &&
- !QuicPacketGeneratorPeer::GetPacketCreator(this)->has_ack() &&
+ if (!QuicPacketGeneratorPeer::GetPacketCreator(this)->has_ack() &&
delegate_->ShouldGeneratePacket(NO_RETRANSMITTABLE_DATA,
NOT_HANDSHAKE)) {
EXPECT_CALL(*delegate_, MaybeBundleAckOpportunistically()).Times(1);
@@ -182,8 +177,7 @@
QuicStringPiece data,
QuicStreamOffset offset) {
producer_->SaveCryptoData(level, offset, data);
- if (GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode) &&
- !QuicPacketGeneratorPeer::GetPacketCreator(this)->has_ack() &&
+ if (!QuicPacketGeneratorPeer::GetPacketCreator(this)->has_ack() &&
delegate_->ShouldGeneratePacket(NO_RETRANSMITTABLE_DATA,
NOT_HANDSHAKE)) {
EXPECT_CALL(*delegate_, MaybeBundleAckOpportunistically()).Times(1);
@@ -345,78 +339,6 @@
MOCK_METHOD1(OnFrameAddedToPacket, void(const QuicFrame&));
};
-TEST_F(QuicPacketGeneratorTest, ShouldSendAck_NotWritable) {
- if (GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- return;
- }
- delegate_.SetCanNotWrite();
-
- generator_.SetShouldSendAck(false);
- EXPECT_TRUE(generator_.HasQueuedFrames());
- EXPECT_FALSE(generator_.HasRetransmittableFrames());
-}
-
-TEST_F(QuicPacketGeneratorTest, ShouldSendAck_WritableAndShouldNotFlush) {
- if (GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- return;
- }
- StrictMock<MockDebugDelegate> debug_delegate;
-
- generator_.set_debug_delegate(&debug_delegate);
- delegate_.SetCanWriteOnlyNonRetransmittable();
-
- EXPECT_CALL(delegate_, GetUpdatedAckFrame())
- .WillOnce(Return(QuicFrame(&ack_frame_)));
- EXPECT_CALL(debug_delegate, OnFrameAddedToPacket(_)).Times(1);
-
- generator_.SetShouldSendAck(false);
- EXPECT_TRUE(generator_.HasQueuedFrames());
- EXPECT_FALSE(generator_.HasRetransmittableFrames());
-}
-
-TEST_F(QuicPacketGeneratorTest, ShouldSendAck_WritableAndShouldFlush) {
- if (GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- return;
- }
- delegate_.SetCanWriteOnlyNonRetransmittable();
-
- EXPECT_CALL(delegate_, GetUpdatedAckFrame())
- .WillOnce(Return(QuicFrame(&ack_frame_)));
- EXPECT_CALL(delegate_, OnSerializedPacket(_))
- .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket));
-
- generator_.SetShouldSendAck(false);
- generator_.Flush();
- EXPECT_FALSE(generator_.HasQueuedFrames());
- EXPECT_FALSE(generator_.HasRetransmittableFrames());
-
- PacketContents contents;
- contents.num_ack_frames = 1;
- CheckPacketContains(contents, 0);
-}
-
-TEST_F(QuicPacketGeneratorTest, ShouldSendAck_MultipleCalls) {
- if (GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- return;
- }
- // Make sure that calling SetShouldSendAck multiple times does not result in a
- // crash. Previously this would result in multiple QuicFrames queued in the
- // packet generator, with all but the last with internal pointers to freed
- // memory.
- delegate_.SetCanWriteAnything();
-
- // Only one AckFrame should be created.
- EXPECT_CALL(delegate_, GetUpdatedAckFrame())
- .WillOnce(Return(QuicFrame(&ack_frame_)));
- EXPECT_CALL(delegate_, OnSerializedPacket(_))
- .Times(1)
- .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket));
-
- generator_.SetShouldSendAck(false);
- generator_.SetShouldSendAck(false);
- generator_.Flush();
-}
-
TEST_F(QuicPacketGeneratorTest, AddControlFrame_NotWritable) {
delegate_.SetCanNotWrite();
@@ -845,9 +767,6 @@
TEST_F(QuicPacketGeneratorTest, ConsumeDataLargeSendAckFalse) {
delegate_.SetCanNotWrite();
- if (!GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- generator_.SetShouldSendAck(false);
- }
QuicRstStreamFrame* rst_frame = CreateRstStreamFrame();
const bool success =
generator_.ConsumeRetransmittableControlFrame(QuicFrame(rst_frame),
@@ -863,10 +782,6 @@
delegate_.SetCanWriteAnything();
- if (!GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- EXPECT_CALL(delegate_, GetUpdatedAckFrame())
- .WillOnce(Return(QuicFrame(&ack_frame_)));
- }
if (generator_.deprecate_queued_control_frames()) {
generator_.ConsumeRetransmittableControlFrame(QuicFrame(rst_frame),
/*bundle_ack=*/false);
@@ -905,22 +820,8 @@
return;
}
delegate_.SetCanNotWrite();
- if (!GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- generator_.SetShouldSendAck(true /* stop_waiting */);
- }
delegate_.SetCanWriteAnything();
- if (!GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- // Set up frames to write into the creator when control frames are written.
- EXPECT_CALL(delegate_, GetUpdatedAckFrame())
- .WillOnce(Return(QuicFrame(&ack_frame_)));
- EXPECT_CALL(delegate_, PopulateStopWaitingFrame(_));
- // Generator should have queued control frames, and creator should be empty.
- EXPECT_TRUE(generator_.HasQueuedFrames());
- EXPECT_FALSE(generator_.HasRetransmittableFrames());
- EXPECT_FALSE(creator_->HasPendingFrames());
- }
-
// Create a 10000 byte IOVector.
CreateData(10000);
EXPECT_CALL(delegate_, OnSerializedPacket(_))
@@ -947,9 +848,6 @@
TEST_F(QuicPacketGeneratorTest, NotWritableThenBatchOperations) {
delegate_.SetCanNotWrite();
- if (!GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- generator_.SetShouldSendAck(false);
- }
QuicRstStreamFrame* rst_frame = CreateRstStreamFrame();
const bool consumed =
generator_.ConsumeRetransmittableControlFrame(QuicFrame(rst_frame),
@@ -966,12 +864,6 @@
delegate_.SetCanWriteAnything();
- if (!GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- // When the first write operation is invoked, the ack frame will be
- // returned.
- EXPECT_CALL(delegate_, GetUpdatedAckFrame())
- .WillOnce(Return(QuicFrame(&ack_frame_)));
- }
if (generator_.deprecate_queued_control_frames()) {
EXPECT_TRUE(
generator_.ConsumeRetransmittableControlFrame(QuicFrame(rst_frame),
@@ -996,12 +888,8 @@
EXPECT_FALSE(generator_.HasPendingStreamFramesOfStream(3));
PacketContents contents;
- if (GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- // ACK will be flushed by connection.
- contents.num_ack_frames = 0;
- } else {
- contents.num_ack_frames = 1;
- }
+ // ACK will be flushed by connection.
+ contents.num_ack_frames = 0;
if (!VersionHasIetfQuicFrames(framer_.transport_version())) {
contents.num_goaway_frames = 1;
} else {
@@ -1015,9 +903,6 @@
TEST_F(QuicPacketGeneratorTest, NotWritableThenBatchOperations2) {
delegate_.SetCanNotWrite();
- if (!GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- generator_.SetShouldSendAck(false);
- }
QuicRstStreamFrame* rst_frame = CreateRstStreamFrame();
const bool success =
generator_.ConsumeRetransmittableControlFrame(QuicFrame(rst_frame),
@@ -1033,13 +918,6 @@
delegate_.SetCanWriteAnything();
- if (!GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- // When the first write operation is invoked, the ack frame will be
- // returned.
- EXPECT_CALL(delegate_, GetUpdatedAckFrame())
- .WillOnce(Return(QuicFrame(&ack_frame_)));
- }
-
{
InSequence dummy;
// All five frames will be flushed out in a single packet
@@ -1072,12 +950,8 @@
// The first packet should have the queued data and part of the stream data.
PacketContents contents;
- if (GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- // ACK will be sent by connection.
- contents.num_ack_frames = 0;
- } else {
- contents.num_ack_frames = 1;
- }
+ // ACK will be sent by connection.
+ contents.num_ack_frames = 0;
contents.num_rst_stream_frames = 1;
contents.num_stream_frames = 1;
CheckPacketContains(contents, 0);
@@ -1433,22 +1307,8 @@
QuicPacketCreatorPeer::SetPacketNumber(creator_, 1000);
delegate_.SetCanNotWrite();
- if (!GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- generator_.SetShouldSendAck(true);
- }
delegate_.SetCanWriteAnything();
- if (!GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode)) {
- // Set up frames to write into the creator when control frames are written.
- EXPECT_CALL(delegate_, GetUpdatedAckFrame())
- .WillOnce(Return(QuicFrame(&ack_frame_)));
- EXPECT_CALL(delegate_, PopulateStopWaitingFrame(_));
- // Generator should have queued control frames, and creator should be empty.
- EXPECT_TRUE(generator_.HasQueuedFrames());
- EXPECT_FALSE(generator_.HasRetransmittableFrames());
- EXPECT_FALSE(creator_->HasPendingFrames());
- }
-
// This will not serialize any packets, because of the invalid frame.
EXPECT_CALL(delegate_,
OnUnrecoverableError(QUIC_FAILED_TO_SERIALIZE_PACKET, _));
diff --git a/quic/core/quic_received_packet_manager.cc b/quic/core/quic_received_packet_manager.cc
index 4512952..e324c38 100644
--- a/quic/core/quic_received_packet_manager.cc
+++ b/quic/core/quic_received_packet_manager.cc
@@ -62,7 +62,6 @@
time_of_previous_received_packet_(QuicTime::Zero()),
was_last_packet_missing_(false),
decide_when_to_send_acks_(
- GetQuicReloadableFlag(quic_deprecate_ack_bundling_mode) &&
GetQuicReloadableFlag(quic_rpm_decides_when_to_send_acks)) {}
QuicReceivedPacketManager::~QuicReceivedPacketManager() {}
diff --git a/quic/core/quic_received_packet_manager.h b/quic/core/quic_received_packet_manager.h
index 8abe5f9..0a04f63 100644
--- a/quic/core/quic_received_packet_manager.h
+++ b/quic/core/quic_received_packet_manager.h
@@ -186,8 +186,7 @@
// Last sent largest acked, which gets updated when ACK was successfully sent.
QuicPacketNumber last_sent_largest_acked_;
- // Latched value of quic_deprecate_ack_bundling_mode and
- // quic_rpm_decides_when_to_send_acks.
+ // Latched value of quic_rpm_decides_when_to_send_acks.
const bool decide_when_to_send_acks_;
};
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc
index efa3144..754a512 100644
--- a/quic/core/quic_session.cc
+++ b/quic/core/quic_session.cc
@@ -611,8 +611,7 @@
return;
}
- QuicConnection::ScopedPacketFlusher flusher(
- connection_, QuicConnection::SEND_ACK_IF_QUEUED);
+ QuicConnection::ScopedPacketFlusher flusher(connection_);
if (control_frame_manager_.WillingToWrite()) {
control_frame_manager_.OnCanWrite();
}
@@ -754,8 +753,7 @@
// QUIC STOP_SENDING frame. Both sre sent to emulate
// the two-way close that Google QUIC's RST_STREAM does.
if (VersionHasIetfQuicFrames(connection_->transport_version())) {
- QuicConnection::ScopedPacketFlusher flusher(
- connection(), QuicConnection::SEND_ACK_IF_QUEUED);
+ QuicConnection::ScopedPacketFlusher flusher(connection());
control_frame_manager_.WriteOrBufferRstStream(id, error, bytes_written);
control_frame_manager_.WriteOrBufferStopSending(error, id);
} else {
@@ -1700,8 +1698,7 @@
void QuicSession::RetransmitFrames(const QuicFrames& frames,
TransmissionType type) {
- QuicConnection::ScopedPacketFlusher retransmission_flusher(
- connection_, QuicConnection::NO_ACK);
+ QuicConnection::ScopedPacketFlusher retransmission_flusher(connection_);
SetTransmissionType(type);
for (const QuicFrame& frame : frames) {
if (frame.type == MESSAGE_FRAME) {
@@ -1802,8 +1799,7 @@
}
bool QuicSession::RetransmitLostData() {
- QuicConnection::ScopedPacketFlusher retransmission_flusher(
- connection_, QuicConnection::SEND_ACK_IF_QUEUED);
+ QuicConnection::ScopedPacketFlusher retransmission_flusher(connection_);
// Retransmit crypto data first.
bool uses_crypto_frames =
QuicVersionUsesCryptoFrames(connection_->transport_version());
diff --git a/quic/core/quic_versions.cc b/quic/core/quic_versions.cc
index 33faa3c..1989e98 100644
--- a/quic/core/quic_versions.cc
+++ b/quic/core/quic_versions.cc
@@ -429,9 +429,7 @@
// Enable necessary flags.
SetQuicFlag(FLAGS_quic_supports_tls_handshake, true);
- // 60 is the highest multiple of 4 and one-byte variable length integer.
SetQuicFlag(FLAGS_quic_headers_stream_id_in_v99, 60);
- SetQuicReloadableFlag(quic_deprecate_ack_bundling_mode, true);
SetQuicReloadableFlag(quic_rpm_decides_when_to_send_acks, true);
SetQuicReloadableFlag(quic_use_uber_loss_algorithm, true);
SetQuicReloadableFlag(quic_use_uber_received_packet_manager, true);
diff --git a/quic/core/uber_received_packet_manager_test.cc b/quic/core/uber_received_packet_manager_test.cc
index d7d49ee..931db1f 100644
--- a/quic/core/uber_received_packet_manager_test.cc
+++ b/quic/core/uber_received_packet_manager_test.cc
@@ -49,7 +49,6 @@
class UberReceivedPacketManagerTest : public QuicTest {
protected:
UberReceivedPacketManagerTest() {
- SetQuicReloadableFlag(quic_deprecate_ack_bundling_mode, true);
SetQuicReloadableFlag(quic_rpm_decides_when_to_send_acks, true);
manager_ = QuicMakeUnique<UberReceivedPacketManager>(&stats_);
clock_.AdvanceTime(QuicTime::Delta::FromSeconds(1));
