gfe-relnote: In QUIC, clean up !session_decides_what_to_write code path. PiperOrigin-RevId: 276061544 Change-Id: If57489805bb2f6d038d6a2ff2b2d21c141742735
diff --git a/quic/core/congestion_control/general_loss_algorithm_test.cc b/quic/core/congestion_control/general_loss_algorithm_test.cc index dba6d5b..f1ba35a 100644 --- a/quic/core/congestion_control/general_loss_algorithm_test.cc +++ b/quic/core/congestion_control/general_loss_algorithm_test.cc
@@ -41,16 +41,16 @@ PACKET_1BYTE_PACKET_NUMBER, nullptr, kDefaultLength, false, false); packet.retransmittable_frames.push_back(QuicFrame(frame)); - unacked_packets_.AddSentPacket(&packet, QuicPacketNumber(), - NOT_RETRANSMISSION, clock_.Now(), true); + unacked_packets_.AddSentPacket(&packet, NOT_RETRANSMISSION, clock_.Now(), + true); } void SendAckPacket(uint64_t packet_number) { SerializedPacket packet(QuicPacketNumber(packet_number), PACKET_1BYTE_PACKET_NUMBER, nullptr, kDefaultLength, true, false); - unacked_packets_.AddSentPacket(&packet, QuicPacketNumber(), - NOT_RETRANSMISSION, clock_.Now(), false); + unacked_packets_.AddSentPacket(&packet, NOT_RETRANSMISSION, clock_.Now(), + false); } void VerifyLosses(uint64_t largest_newly_acked,
diff --git a/quic/core/congestion_control/uber_loss_algorithm_test.cc b/quic/core/congestion_control/uber_loss_algorithm_test.cc index 8d69c13..79d0ff7 100644 --- a/quic/core/congestion_control/uber_loss_algorithm_test.cc +++ b/quic/core/congestion_control/uber_loss_algorithm_test.cc
@@ -48,8 +48,8 @@ false, false); packet.encryption_level = encryption_level; packet.retransmittable_frames.push_back(QuicFrame(frame)); - unacked_packets_->AddSentPacket(&packet, QuicPacketNumber(), - NOT_RETRANSMISSION, clock_.Now(), true); + unacked_packets_->AddSentPacket(&packet, NOT_RETRANSMISSION, clock_.Now(), + true); } void AckPackets(const std::vector<uint64_t>& packets_acked) {
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc index d46be60..cbb4ca8 100644 --- a/quic/core/http/end_to_end_test.cc +++ b/quic/core/http/end_to_end_test.cc
@@ -3807,9 +3807,6 @@ TEST_P(EndToEndTest, ResetStreamOnTtlExpires) { ASSERT_TRUE(Initialize()); EXPECT_TRUE(client_->client()->WaitForCryptoHandshakeConfirmed()); - if (!GetClientSession()->session_decides_what_to_write()) { - return; - } SetPacketLossPercentage(30); QuicSpdyClientStream* stream = client_->GetOrCreateStream();
diff --git a/quic/core/http/quic_server_session_base_test.cc b/quic/core/http/quic_server_session_base_test.cc index c7ab623..e9c1f9c 100644 --- a/quic/core/http/quic_server_session_base_test.cc +++ b/quic/core/http/quic_server_session_base_test.cc
@@ -570,8 +570,7 @@ SerializedPacket packet( QuicPacketNumber(1) + kMinPacketsBetweenServerConfigUpdates, PACKET_4BYTE_PACKET_NUMBER, nullptr, 1000, false, false); - sent_packet_manager->OnPacketSent(&packet, QuicPacketNumber(), now, - NOT_RETRANSMISSION, + sent_packet_manager->OnPacketSent(&packet, now, NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); // Verify that the proto has exactly the values we expect.
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc index 1dd05d1..d0a30f6 100644 --- a/quic/core/http/quic_spdy_session_test.cc +++ b/quic/core/http/quic_spdy_session_test.cc
@@ -1943,7 +1943,6 @@ } TEST_P(QuicSpdySessionTestServer, OnStreamFrameLost) { - QuicConnectionPeer::SetSessionDecidesWhatToWrite(connection_); InSequence s; // Drive congestion control manually. @@ -2017,7 +2016,6 @@ } TEST_P(QuicSpdySessionTestServer, DonotRetransmitDataOfClosedStreams) { - QuicConnectionPeer::SetSessionDecidesWhatToWrite(connection_); InSequence s; TestStream* stream2 = session_.CreateOutgoingBidirectionalStream(); @@ -2057,7 +2055,6 @@ } TEST_P(QuicSpdySessionTestServer, RetransmitFrames) { - QuicConnectionPeer::SetSessionDecidesWhatToWrite(connection_); MockSendAlgorithm* send_algorithm = new StrictMock<MockSendAlgorithm>; QuicConnectionPeer::SetSendAlgorithm(session_.connection(), send_algorithm); InSequence s;
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc index b9bfbaf..aa4ee06 100644 --- a/quic/core/quic_connection.cc +++ b/quic/core/quic_connection.cc
@@ -25,7 +25,6 @@ #include "net/third_party/quiche/src/quic/core/quic_connection_id.h" #include "net/third_party/quiche/src/quic/core/quic_error_codes.h" #include "net/third_party/quiche/src/quic/core/quic_packet_generator.h" -#include "net/third_party/quiche/src/quic/core/quic_pending_retransmission.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/core/quic_utils.h" #include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h" @@ -357,7 +356,9 @@ ? kDefaultServerMaxPacketSize : kDefaultMaxPacketSize); uber_received_packet_manager_.set_max_ack_ranges(255); - MaybeEnableSessionDecidesWhatToWrite(); + if (version().SupportsAntiAmplificationLimit()) { + sent_packet_manager_.EnableIetfPtoAndLossDetection(); + } MaybeEnableMultiplePacketNumberSpacesSupport(); DCHECK(perspective_ == Perspective::IS_CLIENT || supported_versions.size() == 1); @@ -1668,7 +1669,6 @@ packet_generator_.FlushAllQueuedFrames(); } - sent_packet_manager_.CancelRetransmissionsForStream(id); // Remove all queued packets which only contain data for the reset stream. // TODO(fayang): consider removing this because it should be rarely executed. auto packet_iterator = queued_packets_.begin(); @@ -1881,9 +1881,6 @@ SendAck(); } } - if (!session_decides_what_to_write()) { - WritePendingRetransmissions(); - } WriteNewData(); } @@ -2080,35 +2077,6 @@ } } -void QuicConnection::WritePendingRetransmissions() { - DCHECK(!session_decides_what_to_write()); - // Keep writing as long as there's a pending retransmission which can be - // written. - while (sent_packet_manager_.HasPendingRetransmissions() && - CanWrite(HAS_RETRANSMITTABLE_DATA)) { - const QuicPendingRetransmission pending = - sent_packet_manager_.NextPendingRetransmission(); - - // Re-packetize the frames with a new packet number for retransmission. - // Retransmitted packets use the same packet number length as the - // original. - // Flush the packet generator before making a new packet. - // TODO(ianswett): Implement ReserializeAllFrames as a separate path that - // does not require the creator to be flushed. - // TODO(fayang): FlushAllQueuedFrames should only be called once, and should - // be moved outside of the loop. Also, CanWrite is not checked after the - // generator is flushed. - { - ScopedPacketFlusher flusher(this); - packet_generator_.FlushAllQueuedFrames(); - } - DCHECK(!packet_generator_.HasPendingFrames()); - char buffer[kMaxOutgoingPacketSize]; - packet_generator_.ReserializeAllFrames(pending, buffer, - kMaxOutgoingPacketSize); - } -} - void QuicConnection::SendProbingRetransmissions() { while (sent_packet_manager_.GetSendAlgorithm()->ShouldSendProbingPacket() && CanWrite(HAS_RETRANSMITTABLE_DATA)) { @@ -2117,11 +2085,6 @@ << "Cannot send probing retransmissions: nothing to retransmit."; break; } - - if (!session_decides_what_to_write()) { - DCHECK(sent_packet_manager_.HasPendingRetransmissions()); - WritePendingRetransmissions(); - } } } @@ -2189,8 +2152,7 @@ return false; } - if (session_decides_what_to_write() && - sent_packet_manager_.pending_timer_transmission_count() > 0) { + if (sent_packet_manager_.pending_timer_transmission_count() > 0) { // Force sending the retransmissions for HANDSHAKE, TLP, RTO, PROBING cases. return true; } @@ -2384,8 +2346,8 @@ if (debug_visitor_ != nullptr) { // Pass the write result to the visitor. - debug_visitor_->OnPacketSent(*packet, packet->original_packet_number, - packet->transmission_type, packet_send_time); + debug_visitor_->OnPacketSent(*packet, packet->transmission_type, + packet_send_time); } if (IsRetransmittable(*packet) == HAS_RETRANSMITTABLE_DATA) { if (!is_path_degrading_ && !path_degrading_alarm_->IsSet()) { @@ -2414,8 +2376,8 @@ } const bool in_flight = sent_packet_manager_.OnPacketSent( - packet, packet->original_packet_number, packet_send_time, - packet->transmission_type, IsRetransmittable(*packet)); + packet, packet_send_time, packet->transmission_type, + IsRetransmittable(*packet)); if (in_flight || !retransmission_alarm_->IsSet()) { SetRetransmissionAlarm(); @@ -2544,8 +2506,7 @@ return; } - if (serialized_packet->retransmittable_frames.empty() && - !serialized_packet->original_packet_number.IsInitialized()) { + if (serialized_packet->retransmittable_frames.empty()) { // Increment consecutive_num_packets_with_no_retransmittable_frames_ if // this packet is a new transmission with no retransmittable frames. ++consecutive_num_packets_with_no_retransmittable_frames_; @@ -2714,45 +2675,42 @@ WriteIfNotBlocked(); } - if (session_decides_what_to_write()) { - if (packet_generator_.packet_number() == previous_created_packet_number && - (retransmission_mode == QuicSentPacketManager::TLP_MODE || - retransmission_mode == QuicSentPacketManager::RTO_MODE || - retransmission_mode == QuicSentPacketManager::PTO_MODE) && - !visitor_->WillingAndAbleToWrite()) { - // Send PING if timer fires in RTO or PTO mode but there is no data to - // send. - // When TLP fires, either new data or tail loss probe should be sent. - // There is corner case where TLP fires after RTO because packets get - // acked. Two packets are marked RTO_RETRANSMITTED, but the first packet - // is retransmitted as two packets because of packet number length - // increases (please see QuicConnectionTest.RtoPacketAsTwo). - QUIC_DLOG_IF(WARNING, - retransmission_mode == QuicSentPacketManager::TLP_MODE && - stats_.rto_count == 0) - << "No packet gets sent when timer fires in TLP mode, sending PING"; - DCHECK_LT(0u, sent_packet_manager_.pending_timer_transmission_count()); - visitor_->SendPing(); - } - if (retransmission_mode == QuicSentPacketManager::PTO_MODE) { - sent_packet_manager_.AdjustPendingTimerTransmissions(); - } - if (retransmission_mode != QuicSentPacketManager::LOSS_MODE) { - // When timer fires in TLP or RTO mode, ensure 1) at least one packet is - // created, or there is data to send and available credit (such that - // packets will be sent eventually). - QUIC_BUG_IF( - packet_generator_.packet_number() == previous_created_packet_number && - (!visitor_->WillingAndAbleToWrite() || - sent_packet_manager_.pending_timer_transmission_count() == 0u)) - << "retransmission_mode: " << retransmission_mode - << ", packet_number: " << packet_generator_.packet_number() - << ", session has data to write: " - << visitor_->WillingAndAbleToWrite() - << ", writer is blocked: " << writer_->IsWriteBlocked() - << ", pending_timer_transmission_count: " - << sent_packet_manager_.pending_timer_transmission_count(); - } + if (packet_generator_.packet_number() == previous_created_packet_number && + (retransmission_mode == QuicSentPacketManager::TLP_MODE || + retransmission_mode == QuicSentPacketManager::RTO_MODE || + retransmission_mode == QuicSentPacketManager::PTO_MODE) && + !visitor_->WillingAndAbleToWrite()) { + // Send PING if timer fires in RTO or PTO mode but there is no data to + // send. + // When TLP fires, either new data or tail loss probe should be sent. + // There is corner case where TLP fires after RTO because packets get + // acked. Two packets are marked RTO_RETRANSMITTED, but the first packet + // is retransmitted as two packets because of packet number length + // increases (please see QuicConnectionTest.RtoPacketAsTwo). + QUIC_DLOG_IF(WARNING, + retransmission_mode == QuicSentPacketManager::TLP_MODE && + stats_.rto_count == 0) + << "No packet gets sent when timer fires in TLP mode, sending PING"; + DCHECK_LT(0u, sent_packet_manager_.pending_timer_transmission_count()); + visitor_->SendPing(); + } + if (retransmission_mode == QuicSentPacketManager::PTO_MODE) { + sent_packet_manager_.AdjustPendingTimerTransmissions(); + } + if (retransmission_mode != QuicSentPacketManager::LOSS_MODE) { + // When timer fires in TLP or RTO mode, ensure 1) at least one packet is + // created, or there is data to send and available credit (such that + // packets will be sent eventually). + QUIC_BUG_IF(packet_generator_.packet_number() == + previous_created_packet_number && + (!visitor_->WillingAndAbleToWrite() || + sent_packet_manager_.pending_timer_transmission_count() == 0u)) + << "retransmission_mode: " << retransmission_mode + << ", packet_number: " << packet_generator_.packet_number() + << ", session has data to write: " << visitor_->WillingAndAbleToWrite() + << ", writer is blocked: " << writer_->IsWriteBlocked() + << ", pending_timer_transmission_count: " + << sent_packet_manager_.pending_timer_transmission_count(); } // Ensure the retransmission alarm is always set if there are unacked packets @@ -3322,10 +3280,8 @@ } connection_->packet_generator_.Flush(); connection_->FlushPackets(); - if (connection_->session_decides_what_to_write()) { - // Reset transmission type. - connection_->SetTransmissionType(NOT_RETRANSMISSION); - } + // Reset transmission type. + connection_->SetTransmissionType(NOT_RETRANSMISSION); // Once all transmissions are done, check if there is any outstanding data // to send and notify the congestion controller if not. @@ -3543,15 +3499,13 @@ if (debug_visitor_ != nullptr) { debug_visitor_->OnPacketSent( - *probing_packet, probing_packet->original_packet_number, - probing_packet->transmission_type, packet_send_time); + *probing_packet, probing_packet->transmission_type, packet_send_time); } // Call OnPacketSent regardless of the write result. - sent_packet_manager_.OnPacketSent( - probing_packet.get(), probing_packet->original_packet_number, - packet_send_time, probing_packet->transmission_type, - NO_RETRANSMITTABLE_DATA); + sent_packet_manager_.OnPacketSent(probing_packet.get(), packet_send_time, + probing_packet->transmission_type, + NO_RETRANSMITTABLE_DATA); if (IsWriteBlockedStatus(result.status)) { if (probing_writer == writer_) { @@ -3699,14 +3653,13 @@ } void QuicConnection::CheckIfApplicationLimited() { - if (session_decides_what_to_write() && probing_retransmission_pending_) { + if (probing_retransmission_pending_) { return; } - bool application_limited = - queued_packets_.empty() && buffered_packets_.empty() && - !sent_packet_manager_.HasPendingRetransmissions() && - !visitor_->WillingAndAbleToWrite(); + bool application_limited = queued_packets_.empty() && + buffered_packets_.empty() && + !visitor_->WillingAndAbleToWrite(); if (!application_limited) { return; @@ -3775,14 +3728,6 @@ current_effective_peer_migration_type_ = NO_CHANGE; } -void QuicConnection::MaybeEnableSessionDecidesWhatToWrite() { - sent_packet_manager_.SetSessionDecideWhatToWrite(true); - if (version().SupportsAntiAmplificationLimit()) { - sent_packet_manager_.EnableIetfPtoAndLossDetection(); - } - packet_generator_.SetCanSetTransmissionType(true); -} - void QuicConnection::PostProcessAfterAckFrame(bool send_stop_waiting, bool acked_new_packet) { if (no_stop_waiting_frames_) { @@ -3833,10 +3778,6 @@ packet_generator_.SetTransmissionType(type); } -bool QuicConnection::session_decides_what_to_write() const { - return sent_packet_manager_.session_decides_what_to_write(); -} - void QuicConnection::UpdateReleaseTimeIntoFuture() { DCHECK(supports_release_time_);
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h index e9eb5e1..f333fe8 100644 --- a/quic/core/quic_connection.h +++ b/quic/core/quic_connection.h
@@ -178,7 +178,6 @@ // Called when a packet has been sent. virtual void OnPacketSent(const SerializedPacket& /*serialized_packet*/, - QuicPacketNumber /*original_packet_number*/, TransmissionType /*transmission_type*/, QuicTime /*sent_time*/) {} @@ -829,8 +828,6 @@ defer_send_in_response_to_packets_ = defer; } - bool session_decides_what_to_write() const; - // Sets the current per-packet options for the connection. The QuicConnection // does not take ownership of |options|; |options| must live for as long as // the QuicConnection is in use. @@ -1039,9 +1036,6 @@ // blocked when this is called. void WriteQueuedPackets(); - // Writes as many pending retransmissions as possible. - void WritePendingRetransmissions(); - // Writes new data if congestion control allows. void WriteNewData(); @@ -1109,9 +1103,6 @@ // effective peer address change. void UpdatePacketContent(PacketContent type); - // Enables session decide what to write based on version and flags. - void MaybeEnableSessionDecidesWhatToWrite(); - // Called when last received ack frame has been processed. // |send_stop_waiting| indicates whether a stop waiting needs to be sent. // |acked_new_packet| is true if a previously-unacked packet was acked.
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index e2ef838..1a686d9 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -1016,10 +1016,8 @@ frame1_.stream_id = stream_id; frame2_.stream_id = stream_id; connection_.set_visitor(&visitor_); - if (connection_.session_decides_what_to_write()) { - connection_.SetSessionNotifier(¬ifier_); - connection_.set_notifier(¬ifier_); - } + connection_.SetSessionNotifier(¬ifier_); + connection_.set_notifier(¬ifier_); connection_.SetSendAlgorithm(send_algorithm_); connection_.SetLossAlgorithm(loss_algorithm_.get()); EXPECT_CALL(*send_algorithm_, CanSend(_)).WillRepeatedly(Return(true)); @@ -1039,13 +1037,8 @@ EXPECT_CALL(*send_algorithm_, OnApplicationLimited(_)).Times(AnyNumber()); EXPECT_CALL(visitor_, WillingAndAbleToWrite()).Times(AnyNumber()); EXPECT_CALL(visitor_, HasPendingHandshake()).Times(AnyNumber()); - if (connection_.session_decides_what_to_write()) { - EXPECT_CALL(visitor_, OnCanWrite()) - .WillRepeatedly( - Invoke(¬ifier_, &SimpleSessionNotifier::OnCanWrite)); - } else { - EXPECT_CALL(visitor_, OnCanWrite()).Times(AnyNumber()); - } + EXPECT_CALL(visitor_, OnCanWrite()) + .WillRepeatedly(Invoke(¬ifier_, &SimpleSessionNotifier::OnCanWrite)); EXPECT_CALL(visitor_, ShouldKeepConnectionAlive()) .WillRepeatedly(Return(false)); EXPECT_CALL(visitor_, OnCongestionWindowChange(_)).Times(AnyNumber()); @@ -1329,26 +1322,11 @@ void SendRstStream(QuicStreamId id, QuicRstStreamErrorCode error, QuicStreamOffset bytes_written) { - if (connection_.session_decides_what_to_write()) { - notifier_.WriteOrBufferRstStream(id, error, bytes_written); - connection_.OnStreamReset(id, error); - return; - } - std::unique_ptr<QuicRstStreamFrame> rst_stream = - std::make_unique<QuicRstStreamFrame>(1, id, error, bytes_written); - if (connection_.SendControlFrame(QuicFrame(rst_stream.get()))) { - rst_stream.release(); - } + notifier_.WriteOrBufferRstStream(id, error, bytes_written); connection_.OnStreamReset(id, error); } - void SendPing() { - if (connection_.session_decides_what_to_write()) { - notifier_.WriteOrBufferPing(); - } else { - connection_.SendControlFrame(QuicFrame(QuicPingFrame(1))); - } - } + void SendPing() { notifier_.WriteOrBufferPing(); } void ProcessAckPacket(uint64_t packet_number, QuicAckFrame* frame) { if (packet_number > 1) { @@ -3369,11 +3347,6 @@ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); writer_->SetWritable(); connection_.OnCanWrite(); - if (!connection_.session_decides_what_to_write()) { - // OnCanWrite will cause RST_STREAM be sent again. - connection_.SendControlFrame(QuicFrame(new QuicRstStreamFrame( - 1, stream_id, QUIC_ERROR_PROCESSING_STREAM, 14))); - } size_t padding_frame_count = writer_->padding_frames().size(); EXPECT_EQ(padding_frame_count + 1u, writer_->frame_count()); EXPECT_EQ(1u, writer_->rst_stream_frames().size()); @@ -3405,11 +3378,6 @@ } writer_->SetWritable(); connection_.OnCanWrite(); - if (!connection_.session_decides_what_to_write()) { - // OnCanWrite will cause RST_STREAM be sent again. - connection_.SendControlFrame(QuicFrame( - new QuicRstStreamFrame(1, stream_id, QUIC_STREAM_NO_ERROR, 14))); - } size_t padding_frame_count = writer_->padding_frames().size(); EXPECT_EQ(padding_frame_count + 1u, writer_->frame_count()); EXPECT_EQ(1u, writer_->rst_stream_frames().size()); @@ -3498,11 +3466,7 @@ // Ensure that the data is still in flight, but the retransmission alarm is no // longer set. EXPECT_GT(manager_->GetBytesInFlight(), 0u); - if (connection_.session_decides_what_to_write()) { - EXPECT_TRUE(connection_.GetRetransmissionAlarm()->IsSet()); - } else { - EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet()); - } + EXPECT_TRUE(connection_.GetRetransmissionAlarm()->IsSet()); } TEST_P(QuicConnectionTest, RetransmitForQuicRstStreamNoErrorOnRTO) { @@ -3549,11 +3513,6 @@ EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); writer_->SetWritable(); connection_.OnCanWrite(); - if (!connection_.session_decides_what_to_write()) { - // OnCanWrite will cause this RST_STREAM_FRAME be sent again. - connection_.SendControlFrame(QuicFrame(new QuicRstStreamFrame( - 1, stream_id, QUIC_ERROR_PROCESSING_STREAM, 14))); - } size_t padding_frame_count = writer_->padding_frames().size(); EXPECT_EQ(padding_frame_count + 1u, writer_->frame_count()); ASSERT_EQ(1u, writer_->rst_stream_frames().size()); @@ -3676,8 +3635,7 @@ } TEST_P(QuicConnectionTest, QueueAfterTwoRTOs) { - if (connection_.PtoEnabled() || - !connection_.session_decides_what_to_write()) { + if (connection_.PtoEnabled()) { return; } connection_.SetMaxTailLossProbes(0); @@ -3781,11 +3739,7 @@ writer_->SetWritable(); connection_.OnCanWrite(); - if (connection_.session_decides_what_to_write()) { - EXPECT_TRUE(connection_.GetRetransmissionAlarm()->IsSet()); - } else { - EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet()); - } + EXPECT_TRUE(connection_.GetRetransmissionAlarm()->IsSet()); EXPECT_FALSE(QuicConnectionPeer::HasRetransmittableFrames(&connection_, 2)); } @@ -3907,11 +3861,7 @@ EXPECT_CALL(*loss_algorithm_, DetectLosses(_, _, _, _, _, _)) .WillOnce(SetArgPointee<5>(lost_packets)); EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _)); - if (connection_.session_decides_what_to_write()) { - EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); - } else { - EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(14); - } + EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); ProcessAckPacket(&nack); } @@ -4037,8 +3987,7 @@ } TEST_P(QuicConnectionTest, TailLossProbeDelayForStreamDataInTLPR) { - if (!connection_.session_decides_what_to_write() || - connection_.PtoEnabled()) { + if (connection_.PtoEnabled()) { return; } @@ -4073,8 +4022,7 @@ } TEST_P(QuicConnectionTest, TailLossProbeDelayForNonStreamDataInTLPR) { - if (!connection_.session_decides_what_to_write() || - connection_.PtoEnabled()) { + if (connection_.PtoEnabled()) { return; } @@ -4228,8 +4176,7 @@ // Regression test of b/133771183. TEST_P(QuicConnectionTest, RtoWithNoDataToRetransmit) { - if (!connection_.session_decides_what_to_write() || - connection_.PtoEnabled()) { + if (connection_.PtoEnabled()) { return; } connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); @@ -7301,9 +7248,6 @@ EXPECT_CALL(*loss_algorithm_, DetectLosses(_, _, _, _, _, _)) .WillOnce(SetArgPointee<5>(lost_packets)); EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _, _)); - if (!connection_.session_decides_what_to_write()) { - EXPECT_CALL(visitor_, OnCanWrite()); - } EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); ProcessAckPacket(&nack_three); @@ -7434,10 +7378,10 @@ MockQuicConnectionDebugVisitor debug_visitor; connection_.set_debug_visitor(&debug_visitor); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _)).Times(1); + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1); connection_.SendStreamDataWithString(1, "foo", 0, NO_FIN); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _)).Times(1); + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1); connection_.SendConnectivityProbingPacket(writer_.get(), connection_.peer_address()); } @@ -7566,7 +7510,7 @@ CongestionBlockWrites(); connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _)).Times(1); + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1); EXPECT_CALL(debug_visitor, OnPingSent()).Times(1); connection_.SendControlFrame(QuicFrame(QuicPingFrame(1))); EXPECT_FALSE(connection_.HasQueuedData()); @@ -7578,7 +7522,7 @@ connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _)).Times(1); + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(1); EXPECT_EQ(0u, connection_.GetStats().blocked_frames_sent); connection_.SendControlFrame(QuicFrame(new QuicBlockedFrame(1, 3))); EXPECT_EQ(1u, connection_.GetStats().blocked_frames_sent); @@ -7594,7 +7538,7 @@ QuicBlockedFrame blocked(1, 3); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _)).Times(0); + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(0); EXPECT_EQ(0u, connection_.GetStats().blocked_frames_sent); connection_.SendControlFrame(QuicFrame(&blocked)); EXPECT_EQ(0u, connection_.GetStats().blocked_frames_sent); @@ -8263,9 +8207,8 @@ } // Expect them retransmitted in cyclic order (foo, bar, test, foo, bar...). QuicPacketCount sent_count = 0; - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _)) + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)) .WillRepeatedly(Invoke([this, &sent_count](const SerializedPacket&, - QuicPacketNumber, TransmissionType, QuicTime) { ASSERT_EQ(1u, writer_->stream_frames().size()); // Identify the frames by stream offset (0, 3, 6, 0, 3...). @@ -8295,7 +8238,7 @@ MockQuicConnectionDebugVisitor debug_visitor; connection_.set_debug_visitor(&debug_visitor); - EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _, _)).Times(0); + EXPECT_CALL(debug_visitor, OnPacketSent(_, _, _)).Times(0); EXPECT_CALL(*send_algorithm_, ShouldSendProbingPacket()) .WillRepeatedly(Return(true)); EXPECT_CALL(visitor_, SendProbingData()).WillRepeatedly([this] { @@ -9389,9 +9332,6 @@ // Regresstion test for b/138962304. TEST_P(QuicConnectionTest, RtoAndWriteBlocked) { - if (!connection_.session_decides_what_to_write()) { - return; - } EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet()); QuicStreamId stream_id = 2; @@ -9418,9 +9358,6 @@ // Regresstion test for b/138962304. TEST_P(QuicConnectionTest, TlpAndWriteBlocked) { - if (!connection_.session_decides_what_to_write()) { - return; - } EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet()); connection_.SetMaxTailLossProbes(1); @@ -9451,8 +9388,7 @@ // Regresstion test for b/139375344. TEST_P(QuicConnectionTest, RtoForcesSendingPing) { - if (!connection_.session_decides_what_to_write() || - connection_.PtoEnabled()) { + if (connection_.PtoEnabled()) { return; } EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); @@ -9490,9 +9426,6 @@ } TEST_P(QuicConnectionTest, ProbeTimeout) { - if (!connection_.session_decides_what_to_write()) { - return; - } SetQuicReloadableFlag(quic_enable_pto, true); QuicConfig config; QuicTagVector connection_options; @@ -9521,9 +9454,6 @@ } TEST_P(QuicConnectionTest, CloseConnectionAfter6ClientPTOs) { - if (!connection_.session_decides_what_to_write()) { - return; - } SetQuicReloadableFlag(quic_enable_pto, true); QuicConfig config; QuicTagVector connection_options; @@ -9562,9 +9492,6 @@ } TEST_P(QuicConnectionTest, CloseConnectionAfter7ClientPTOs) { - if (!connection_.session_decides_what_to_write()) { - return; - } SetQuicReloadableFlag(quic_enable_pto, true); QuicConfig config; QuicTagVector connection_options; @@ -9602,9 +9529,6 @@ } TEST_P(QuicConnectionTest, CloseConnectionAfter8ClientPTOs) { - if (!connection_.session_decides_what_to_write()) { - return; - } SetQuicReloadableFlag(quic_enable_pto, true); QuicConfig config; QuicTagVector connection_options; @@ -9754,8 +9678,7 @@ // Regression test for b/137401387 and b/138962304. TEST_P(QuicConnectionTest, RtoPacketAsTwo) { - if (!connection_.session_decides_what_to_write() || - connection_.PtoEnabled()) { + if (connection_.PtoEnabled()) { return; } connection_.SetMaxTailLossProbes(1); @@ -9798,9 +9721,6 @@ } TEST_P(QuicConnectionTest, PtoSkipsPacketNumber) { - if (!connection_.session_decides_what_to_write()) { - return; - } SetQuicReloadableFlag(quic_enable_pto, true); SetQuicReloadableFlag(quic_skip_packet_number_for_pto, true); QuicConfig config;
diff --git a/quic/core/quic_control_frame_manager.cc b/quic/core/quic_control_frame_manager.cc index 11efae8..85b204b 100644 --- a/quic/core/quic_control_frame_manager.cc +++ b/quic/core/quic_control_frame_manager.cc
@@ -281,9 +281,7 @@ void QuicControlFrameManager::WriteBufferedFrames() { while (HasBufferedFrames()) { - if (session_->session_decides_what_to_write()) { - session_->SetTransmissionType(NOT_RETRANSMISSION); - } + session_->SetTransmissionType(NOT_RETRANSMISSION); QuicFrame frame_to_send = control_frames_.at(least_unsent_ - least_unacked_); QuicFrame copy = CopyRetransmittableControlFrame(frame_to_send);
diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc index c93595e..15e9651 100644 --- a/quic/core/quic_packet_creator.cc +++ b/quic/core/quic_packet_creator.cc
@@ -89,7 +89,6 @@ false), pending_padding_bytes_(0), needs_full_padding_(false), - can_set_transmission_type_(false), next_transmission_type_(NOT_RETRANSMISSION), flusher_attached_(false), fully_pad_crypto_handshake_packets_(true), @@ -368,49 +367,6 @@ return true; } -void QuicPacketCreator::ReserializeAllFrames( - const QuicPendingRetransmission& retransmission, - char* buffer, - size_t buffer_len) { - DCHECK(queued_frames_.empty()); - DCHECK_EQ(0, packet_.num_padding_bytes); - QUIC_BUG_IF(retransmission.retransmittable_frames.empty()) - << "Attempt to serialize empty packet"; - const EncryptionLevel default_encryption_level = packet_.encryption_level; - - // Temporarily set the packet number length and change the encryption level. - packet_.packet_number_length = retransmission.packet_number_length; - if (retransmission.num_padding_bytes == -1) { - // Only retransmit padding when original packet needs full padding. Padding - // from pending_padding_bytes_ are not retransmitted. - needs_full_padding_ = true; - } - // Only preserve the original encryption level if it's a handshake packet or - // if we haven't gone forward secure. - if (retransmission.has_crypto_handshake || - packet_.encryption_level != ENCRYPTION_FORWARD_SECURE) { - packet_.encryption_level = retransmission.encryption_level; - } - - // Serialize the packet and restore packet number length state. - for (const QuicFrame& frame : retransmission.retransmittable_frames) { - bool success = AddFrame(frame, false, retransmission.transmission_type); - QUIC_BUG_IF(!success) << " Failed to add frame of type:" << frame.type - << " num_frames:" - << retransmission.retransmittable_frames.size() - << " retransmission.packet_number_length:" - << retransmission.packet_number_length - << " packet_.packet_number_length:" - << packet_.packet_number_length; - } - packet_.transmission_type = retransmission.transmission_type; - SerializePacket(buffer, buffer_len); - packet_.original_packet_number = retransmission.packet_number; - OnSerializedPacket(); - // Restore old values. - packet_.encryption_level = default_encryption_level; -} - void QuicPacketCreator::FlushCurrentPacket() { if (!HasPendingFrames() && pending_padding_bytes_ == 0) { return; @@ -445,7 +401,6 @@ packet_.has_stop_waiting = false; packet_.has_crypto_handshake = NOT_HANDSHAKE; packet_.num_padding_bytes = 0; - packet_.original_packet_number.Clear(); packet_.transmission_type = NOT_RETRANSMISSION; packet_.encrypted_buffer = nullptr; packet_.encrypted_length = 0; @@ -538,9 +493,7 @@ return; } - if (can_set_transmission_type()) { - packet_.transmission_type = transmission_type; - } + packet_.transmission_type = transmission_type; size_t encrypted_length = framer_->EncryptInPlace( packet_.encryption_level, packet_.packet_number, @@ -1289,10 +1242,7 @@ void QuicPacketCreator::SetTransmissionType(TransmissionType type) { DCHECK(combine_generator_and_creator_); - SetTransmissionTypeOfNextPackets(type); - if (can_set_transmission_type()) { - next_transmission_type_ = type; - } + next_transmission_type_ = type; } MessageStatus QuicPacketCreator::AddMessageFrame(QuicMessageId message_id, @@ -1433,8 +1383,7 @@ // Packet transmission type is determined by the last added retransmittable // frame. - if (can_set_transmission_type() && - QuicUtils::IsRetransmittableFrame(frame.type)) { + if (QuicUtils::IsRetransmittableFrame(frame.type)) { packet_.transmission_type = transmission_type; } return true; @@ -1569,19 +1518,6 @@ client_connection_id_ = client_connection_id; } -void QuicPacketCreator::SetTransmissionTypeOfNextPackets( - TransmissionType type) { - DCHECK(can_set_transmission_type_); - - if (!can_set_transmission_type()) { - QUIC_DVLOG_IF(1, type != packet_.transmission_type) - << ENDPOINT << "Setting Transmission type to " - << TransmissionTypeToString(type); - - packet_.transmission_type = type; - } -} - QuicPacketLength QuicPacketCreator::GetCurrentLargestMessagePayload() const { if (!VersionSupportsMessageFrames(framer_->transport_version())) { return 0;
diff --git a/quic/core/quic_packet_creator.h b/quic/core/quic_packet_creator.h index 50bd7f8..3fd877d 100644 --- a/quic/core/quic_packet_creator.h +++ b/quic/core/quic_packet_creator.h
@@ -16,7 +16,6 @@ #include "net/third_party/quiche/src/quic/core/frames/quic_stream_frame.h" #include "net/third_party/quiche/src/quic/core/quic_framer.h" #include "net/third_party/quiche/src/quic/core/quic_packets.h" -#include "net/third_party/quiche/src/quic/core/quic_pending_retransmission.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/platform/api/quic_export.h" @@ -142,12 +141,6 @@ // |length|. bool HasRoomForMessageFrame(QuicByteCount length); - // Re-serializes frames with the original packet's packet number length. - // Used for retransmitting packets to ensure they aren't too long. - void ReserializeAllFrames(const QuicPendingRetransmission& retransmission, - char* buffer, - size_t buffer_len); - // Serializes all added frames into a single packet and invokes the delegate_ // to further process the SerializedPacket. void FlushCurrentPacket(); @@ -280,9 +273,6 @@ // MaybeAddPadding(). void AddPendingPadding(QuicByteCount size); - // Sets transmission type of next constructed packets. - void SetTransmissionTypeOfNextPackets(TransmissionType type); - // Sets the retry token to be sent over the wire in IETF Initial packets. void SetRetryToken(QuicStringPiece retry_token); @@ -368,12 +358,6 @@ debug_delegate_ = debug_delegate; } - void set_can_set_transmission_type(bool can_set_transmission_type) { - can_set_transmission_type_ = can_set_transmission_type; - } - - bool can_set_transmission_type() const { return can_set_transmission_type_; } - QuicByteCount pending_padding_bytes() const { return pending_padding_bytes_; } QuicTransportVersion transport_version() const { @@ -562,10 +546,6 @@ // bytes. bool needs_full_padding_; - // If true, packet_'s transmission type is only set by - // SetPacketTransmissionType and does not get cleared in ClearPacket. - bool can_set_transmission_type_; - // Transmission type of the next serialized packet. TransmissionType next_transmission_type_;
diff --git a/quic/core/quic_packet_creator_test.cc b/quic/core/quic_packet_creator_test.cc index b7ced21..c342235 100644 --- a/quic/core/quic_packet_creator_test.cc +++ b/quic/core/quic_packet_creator_test.cc
@@ -16,7 +16,6 @@ #include "net/third_party/quiche/src/quic/core/crypto/quic_encrypter.h" #include "net/third_party/quiche/src/quic/core/frames/quic_stream_frame.h" #include "net/third_party/quiche/src/quic/core/quic_data_writer.h" -#include "net/third_party/quiche/src/quic/core/quic_pending_retransmission.h" #include "net/third_party/quiche/src/quic/core/quic_simple_buffer_allocator.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/core/quic_utils.h" @@ -264,18 +263,6 @@ /* data_length= */ 0); } - QuicPendingRetransmission CreateRetransmission( - const QuicFrames& retransmittable_frames, - bool has_crypto_handshake, - int num_padding_bytes, - EncryptionLevel encryption_level, - QuicPacketNumberLength packet_number_length) { - return QuicPendingRetransmission(QuicPacketNumber(1u), NOT_RETRANSMISSION, - retransmittable_frames, - has_crypto_handshake, num_padding_bytes, - encryption_level, packet_number_length); - } - bool IsDefaultTestConfiguration() { TestParams p = GetParam(); return p.version == AllSupportedVersions()[0] && p.version_serialization; @@ -362,248 +349,6 @@ } } -TEST_P(QuicPacketCreatorTest, ReserializeFramesWithSequenceNumberLength) { - if (VersionHasIetfInvariantHeader(client_framer_.transport_version())) { - creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE); - } - // If the original packet number length, the current packet number - // length, and the configured send packet number length are different, the - // retransmit must sent with the original length and the others do not change. - QuicPacketCreatorPeer::SetPacketNumberLength(&creator_, - PACKET_2BYTE_PACKET_NUMBER); - QuicFrames frames; - std::string data("a"); - if (!QuicVersionUsesCryptoFrames(client_framer_.transport_version())) { - QuicStreamFrame stream_frame( - QuicUtils::GetCryptoStreamId(client_framer_.transport_version()), - /*fin=*/false, 0u, QuicStringPiece()); - frames.push_back(QuicFrame(stream_frame)); - } else { - producer_.SaveCryptoData(ENCRYPTION_INITIAL, 0, data); - frames.push_back( - QuicFrame(new QuicCryptoFrame(ENCRYPTION_INITIAL, 0, data.length()))); - } - char buffer[kMaxOutgoingPacketSize]; - QuicPendingRetransmission retransmission(CreateRetransmission( - frames, true /* has_crypto_handshake */, -1 /* needs full padding */, - ENCRYPTION_INITIAL, PACKET_4BYTE_PACKET_NUMBER)); - EXPECT_CALL(delegate_, OnSerializedPacket(_)) - .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); - creator_.ReserializeAllFrames(retransmission, buffer, kMaxOutgoingPacketSize); - // The packet number length is updated after every packet is sent, - // so there is no need to restore the old length after sending. - EXPECT_EQ(PACKET_4BYTE_PACKET_NUMBER, - QuicPacketCreatorPeer::GetPacketNumberLength(&creator_)); - EXPECT_EQ(PACKET_4BYTE_PACKET_NUMBER, - serialized_packet_.packet_number_length); - - { - InSequence s; - EXPECT_CALL(framer_visitor_, OnPacket()); - EXPECT_CALL(framer_visitor_, OnUnauthenticatedPublicHeader(_)); - EXPECT_CALL(framer_visitor_, OnUnauthenticatedHeader(_)); - EXPECT_CALL(framer_visitor_, OnDecryptedPacket(_)); - EXPECT_CALL(framer_visitor_, OnPacketHeader(_)); - if (!QuicVersionUsesCryptoFrames(client_framer_.transport_version())) { - EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); - } else { - EXPECT_CALL(framer_visitor_, OnCryptoFrame(_)); - } - EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); - EXPECT_CALL(framer_visitor_, OnPacketComplete()); - } - ProcessPacket(serialized_packet_); - DeleteFrames(&frames); -} - -TEST_P(QuicPacketCreatorTest, ReserializeCryptoFrameWithForwardSecurity) { - QuicFrames frames; - std::string data("a"); - if (!QuicVersionUsesCryptoFrames(client_framer_.transport_version())) { - QuicStreamFrame stream_frame( - QuicUtils::GetCryptoStreamId(client_framer_.transport_version()), - /*fin=*/false, 0u, QuicStringPiece()); - frames.push_back(QuicFrame(stream_frame)); - } else { - producer_.SaveCryptoData(ENCRYPTION_INITIAL, 0, data); - frames.push_back( - QuicFrame(new QuicCryptoFrame(ENCRYPTION_INITIAL, 0, data.length()))); - } - creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE); - char buffer[kMaxOutgoingPacketSize]; - QuicPendingRetransmission retransmission(CreateRetransmission( - frames, true /* has_crypto_handshake */, -1 /* needs full padding */, - ENCRYPTION_INITIAL, - QuicPacketCreatorPeer::GetPacketNumberLength(&creator_))); - EXPECT_CALL(delegate_, OnSerializedPacket(_)) - .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); - creator_.ReserializeAllFrames(retransmission, buffer, kMaxOutgoingPacketSize); - EXPECT_EQ(ENCRYPTION_INITIAL, serialized_packet_.encryption_level); - DeleteFrames(&frames); -} - -TEST_P(QuicPacketCreatorTest, ReserializeFrameWithForwardSecurity) { - QuicStreamFrame stream_frame(0u, /*fin=*/false, 0u, QuicStringPiece()); - QuicFrames frames; - frames.push_back(QuicFrame(stream_frame)); - creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE); - char buffer[kMaxOutgoingPacketSize]; - QuicPendingRetransmission retransmission(CreateRetransmission( - frames, false /* has_crypto_handshake */, 0 /* no padding */, - ENCRYPTION_INITIAL, - QuicPacketCreatorPeer::GetPacketNumberLength(&creator_))); - EXPECT_CALL(delegate_, OnSerializedPacket(_)) - .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); - creator_.ReserializeAllFrames(retransmission, buffer, kMaxOutgoingPacketSize); - EXPECT_EQ(ENCRYPTION_FORWARD_SECURE, serialized_packet_.encryption_level); -} - -TEST_P(QuicPacketCreatorTest, ReserializeFramesWithFullPadding) { - QuicFrame frame; - std::string data = "fake handshake message data"; - if (!QuicVersionUsesCryptoFrames(client_framer_.transport_version())) { - MakeIOVector(data, &iov_); - producer_.SaveStreamData( - QuicUtils::GetCryptoStreamId(client_framer_.transport_version()), &iov_, - 1u, 0u, iov_.iov_len); - QuicPacketCreatorPeer::CreateStreamFrame( - &creator_, - QuicUtils::GetCryptoStreamId(client_framer_.transport_version()), - iov_.iov_len, 0u, false, &frame); - } else { - producer_.SaveCryptoData(ENCRYPTION_INITIAL, 0, data); - EXPECT_TRUE(QuicPacketCreatorPeer::CreateCryptoFrame( - &creator_, ENCRYPTION_INITIAL, data.length(), 0, &frame)); - } - QuicFrames frames; - frames.push_back(frame); - char buffer[kMaxOutgoingPacketSize]; - QuicPendingRetransmission retransmission(CreateRetransmission( - frames, true /* has_crypto_handshake */, -1 /* needs full padding */, - ENCRYPTION_INITIAL, - QuicPacketCreatorPeer::GetPacketNumberLength(&creator_))); - EXPECT_CALL(delegate_, OnSerializedPacket(_)) - .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); - creator_.ReserializeAllFrames(retransmission, buffer, kMaxOutgoingPacketSize); - EXPECT_EQ(kDefaultMaxPacketSize, serialized_packet_.encrypted_length); - DeleteFrames(&frames); -} - -TEST_P(QuicPacketCreatorTest, DoNotRetransmitPendingPadding) { - QuicFrame frame; - std::string data = "fake message data"; - if (!QuicVersionUsesCryptoFrames(client_framer_.transport_version())) { - MakeIOVector(data, &iov_); - producer_.SaveStreamData( - QuicUtils::GetCryptoStreamId(client_framer_.transport_version()), &iov_, - 1u, 0u, iov_.iov_len); - QuicPacketCreatorPeer::CreateStreamFrame( - &creator_, - QuicUtils::GetCryptoStreamId(client_framer_.transport_version()), - iov_.iov_len, 0u, false, &frame); - } else { - producer_.SaveCryptoData(ENCRYPTION_INITIAL, 0, data); - EXPECT_TRUE(QuicPacketCreatorPeer::CreateCryptoFrame( - &creator_, ENCRYPTION_INITIAL, data.length(), 0, &frame)); - } - - const int kNumPaddingBytes1 = 4; - int packet_size = 0; - { - QuicFrames frames; - frames.push_back(frame); - char buffer[kMaxOutgoingPacketSize]; - QuicPendingRetransmission retransmission(CreateRetransmission( - frames, false /* has_crypto_handshake */, - kNumPaddingBytes1 /* padding bytes */, ENCRYPTION_INITIAL, - QuicPacketCreatorPeer::GetPacketNumberLength(&creator_))); - EXPECT_CALL(delegate_, OnSerializedPacket(_)) - .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); - creator_.ReserializeAllFrames(retransmission, buffer, - kMaxOutgoingPacketSize); - packet_size = serialized_packet_.encrypted_length; - } - - { - InSequence s; - EXPECT_CALL(framer_visitor_, OnPacket()); - EXPECT_CALL(framer_visitor_, OnUnauthenticatedPublicHeader(_)); - EXPECT_CALL(framer_visitor_, OnUnauthenticatedHeader(_)); - EXPECT_CALL(framer_visitor_, OnDecryptedPacket(_)); - EXPECT_CALL(framer_visitor_, OnPacketHeader(_)); - if (QuicVersionUsesCryptoFrames(client_framer_.transport_version())) { - EXPECT_CALL(framer_visitor_, OnCryptoFrame(_)); - } else { - EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); - } - // Pending paddings are not retransmitted. - EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)).Times(0); - EXPECT_CALL(framer_visitor_, OnPacketComplete()); - } - ProcessPacket(serialized_packet_); - - const int kNumPaddingBytes2 = 44; - QuicFrames frames; - frames.push_back(frame); - char buffer[kMaxOutgoingPacketSize]; - QuicPendingRetransmission retransmission(CreateRetransmission( - frames, false /* has_crypto_handshake */, - kNumPaddingBytes2 /* padding bytes */, ENCRYPTION_INITIAL, - QuicPacketCreatorPeer::GetPacketNumberLength(&creator_))); - EXPECT_CALL(delegate_, OnSerializedPacket(_)) - .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); - creator_.ReserializeAllFrames(retransmission, buffer, kMaxOutgoingPacketSize); - - EXPECT_EQ(packet_size, serialized_packet_.encrypted_length); - DeleteFrames(&frames); -} - -TEST_P(QuicPacketCreatorTest, ReserializeFramesWithFullPacketAndPadding) { - creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE); - const size_t overhead = - GetPacketHeaderOverhead(client_framer_.transport_version()) + - GetEncryptionOverhead() + - GetStreamFrameOverhead(client_framer_.transport_version()); - size_t capacity = kDefaultMaxPacketSize - overhead; - for (int delta = -5; delta <= 0; ++delta) { - std::string data(capacity + delta, 'A'); - size_t bytes_free = 0 - delta; - - QuicFrame frame; - SimpleDataProducer producer; - QuicPacketCreatorPeer::framer(&creator_)->set_data_producer(&producer); - MakeIOVector(data, &iov_); - QuicStreamId stream_id = QuicUtils::GetFirstBidirectionalStreamId( - client_framer_.transport_version(), Perspective::IS_CLIENT); - producer.SaveStreamData(stream_id, &iov_, 1u, 0u, iov_.iov_len); - QuicPacketCreatorPeer::CreateStreamFrame(&creator_, stream_id, iov_.iov_len, - kOffset, false, &frame); - QuicFrames frames; - frames.push_back(frame); - char buffer[kMaxOutgoingPacketSize]; - QuicPendingRetransmission retransmission(CreateRetransmission( - frames, false /* has_crypto_handshake */, -1 /* needs full padding */, - ENCRYPTION_FORWARD_SECURE, - QuicPacketCreatorPeer::GetPacketNumberLength(&creator_))); - EXPECT_CALL(delegate_, OnSerializedPacket(_)) - .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); - creator_.ReserializeAllFrames(retransmission, buffer, - kMaxOutgoingPacketSize); - - // If there is not enough space in the packet to fit a padding frame - // (1 byte) and to expand the stream frame (another 2 bytes) the packet - // will not be padded. - if (bytes_free < 3) { - EXPECT_EQ(kDefaultMaxPacketSize - bytes_free, - serialized_packet_.encrypted_length); - } else { - EXPECT_EQ(kDefaultMaxPacketSize, serialized_packet_.encrypted_length); - } - - frames_.clear(); - } -} - TEST_P(QuicPacketCreatorTest, SerializeConnectionClose) { QuicConnectionCloseFrame frame(creator_.transport_version(), QUIC_NO_ERROR, "error", @@ -1944,99 +1689,6 @@ EXPECT_EQ(kMaxNumRandomPaddingBytes, creator_.pending_padding_bytes()); } -TEST_P(QuicPacketCreatorTest, SendPendingPaddingInRetransmission) { - creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE); - QuicStreamId stream_id = QuicUtils::GetFirstBidirectionalStreamId( - client_framer_.transport_version(), Perspective::IS_CLIENT); - QuicStreamFrame stream_frame(stream_id, - /*fin=*/false, 0u, QuicStringPiece()); - QuicFrames frames; - frames.push_back(QuicFrame(stream_frame)); - char buffer[kMaxOutgoingPacketSize]; - QuicPendingRetransmission retransmission(CreateRetransmission( - frames, true, /*num_padding_bytes=*/0, ENCRYPTION_FORWARD_SECURE, - QuicPacketCreatorPeer::GetPacketNumberLength(&creator_))); - EXPECT_CALL(delegate_, OnSerializedPacket(_)) - .WillOnce(Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); - creator_.AddPendingPadding(kMaxNumRandomPaddingBytes); - creator_.ReserializeAllFrames(retransmission, buffer, kMaxOutgoingPacketSize); - EXPECT_EQ(0u, creator_.pending_padding_bytes()); - { - InSequence s; - EXPECT_CALL(framer_visitor_, OnPacket()); - EXPECT_CALL(framer_visitor_, OnUnauthenticatedPublicHeader(_)); - EXPECT_CALL(framer_visitor_, OnUnauthenticatedHeader(_)); - EXPECT_CALL(framer_visitor_, OnDecryptedPacket(_)); - EXPECT_CALL(framer_visitor_, OnPacketHeader(_)); - EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); - EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); - EXPECT_CALL(framer_visitor_, OnPacketComplete()); - } - ProcessPacket(serialized_packet_); -} - -TEST_P(QuicPacketCreatorTest, SendPacketAfterFullPaddingRetransmission) { - // Making sure needs_full_padding gets reset after a full padding - // retransmission. - EXPECT_EQ(0u, creator_.pending_padding_bytes()); - creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE); - QuicFrame frame; - std::string data = "fake handshake message data"; - MakeIOVector(data, &iov_); - QuicStreamId stream_id = QuicUtils::GetFirstBidirectionalStreamId( - client_framer_.transport_version(), Perspective::IS_CLIENT); - if (!QuicVersionUsesCryptoFrames(client_framer_.transport_version())) { - stream_id = - QuicUtils::GetCryptoStreamId(client_framer_.transport_version()); - } - producer_.SaveStreamData(stream_id, &iov_, 1u, 0u, iov_.iov_len); - QuicPacketCreatorPeer::CreateStreamFrame(&creator_, stream_id, iov_.iov_len, - 0u, false, &frame); - QuicFrames frames; - frames.push_back(frame); - char buffer[kMaxOutgoingPacketSize]; - QuicPendingRetransmission retransmission(CreateRetransmission( - frames, true, /*num_padding_bytes=*/-1, ENCRYPTION_FORWARD_SECURE, - QuicPacketCreatorPeer::GetPacketNumberLength(&creator_))); - EXPECT_CALL(delegate_, OnSerializedPacket(_)) - .WillRepeatedly( - Invoke(this, &QuicPacketCreatorTest::SaveSerializedPacket)); - creator_.ReserializeAllFrames(retransmission, buffer, kMaxOutgoingPacketSize); - EXPECT_EQ(kDefaultMaxPacketSize, serialized_packet_.encrypted_length); - { - InSequence s; - EXPECT_CALL(framer_visitor_, OnPacket()); - EXPECT_CALL(framer_visitor_, OnUnauthenticatedPublicHeader(_)); - EXPECT_CALL(framer_visitor_, OnUnauthenticatedHeader(_)); - EXPECT_CALL(framer_visitor_, OnDecryptedPacket(_)); - EXPECT_CALL(framer_visitor_, OnPacketHeader(_)); - EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); - // Full padding. - EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)); - EXPECT_CALL(framer_visitor_, OnPacketComplete()); - } - ProcessPacket(serialized_packet_); - - creator_.ConsumeDataToFillCurrentPacket(stream_id, &iov_, 1u, iov_.iov_len, - 0u, 0u, false, false, - NOT_RETRANSMISSION, &frame); - creator_.FlushCurrentPacket(); - { - InSequence s; - EXPECT_CALL(framer_visitor_, OnPacket()); - EXPECT_CALL(framer_visitor_, OnUnauthenticatedPublicHeader(_)); - EXPECT_CALL(framer_visitor_, OnUnauthenticatedHeader(_)); - EXPECT_CALL(framer_visitor_, OnDecryptedPacket(_)); - EXPECT_CALL(framer_visitor_, OnPacketHeader(_)); - EXPECT_CALL(framer_visitor_, OnStreamFrame(_)); - // needs_full_padding gets reset. - EXPECT_CALL(framer_visitor_, OnPaddingFrame(_)).Times(0); - EXPECT_CALL(framer_visitor_, OnPacketComplete()); - } - ProcessPacket(serialized_packet_); - DeleteFrames(&frames); -} - TEST_P(QuicPacketCreatorTest, ConsumeDataAndRandomPadding) { creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE); const QuicByteCount kStreamFramePayloadSize = 100u; @@ -2233,8 +1885,6 @@ TEST_P(QuicPacketCreatorTest, PacketTransmissionType) { creator_.set_encryption_level(ENCRYPTION_FORWARD_SECURE); - creator_.set_can_set_transmission_type(true); - creator_.SetTransmissionTypeOfNextPackets(NOT_RETRANSMISSION); QuicAckFrame temp_ack_frame = InitAckFrame(1); QuicFrame ack_frame(&temp_ack_frame); @@ -2262,13 +1912,9 @@ creator_.FlushCurrentPacket(); ASSERT_TRUE(serialized_packet_.encrypted_buffer); - if (creator_.can_set_transmission_type()) { - // The last retransmittable frame on packet is a stream frame, the packet's - // transmission type should be the same as the stream frame's. - EXPECT_EQ(serialized_packet_.transmission_type, RTO_RETRANSMISSION); - } else { - EXPECT_EQ(serialized_packet_.transmission_type, NOT_RETRANSMISSION); - } + // The last retransmittable frame on packet is a stream frame, the packet's + // transmission type should be the same as the stream frame's. + EXPECT_EQ(serialized_packet_.transmission_type, RTO_RETRANSMISSION); DeleteSerializedPacket(); }
diff --git a/quic/core/quic_packet_generator.cc b/quic/core/quic_packet_generator.cc index fbb7894..de548ac 100644 --- a/quic/core/quic_packet_generator.cc +++ b/quic/core/quic_packet_generator.cc
@@ -352,13 +352,6 @@ payloads, is_padded); } -void QuicPacketGenerator::ReserializeAllFrames( - const QuicPendingRetransmission& retransmission, - char* buffer, - size_t buffer_len) { - packet_creator_.ReserializeAllFrames(retransmission, buffer, buffer_len); -} - void QuicPacketGenerator::UpdatePacketNumberLength( QuicPacketNumber least_packet_awaited_by_peer, QuicPacketCount max_packets_in_flight) { @@ -431,21 +424,13 @@ packet_creator_.SetTransmissionType(type); return; } - packet_creator_.SetTransmissionTypeOfNextPackets(type); - if (packet_creator_.can_set_transmission_type()) { - next_transmission_type_ = type; - } + next_transmission_type_ = type; } void QuicPacketGenerator::SetRetryToken(QuicStringPiece retry_token) { packet_creator_.SetRetryToken(retry_token); } -void QuicPacketGenerator::SetCanSetTransmissionType( - bool can_set_transmission_type) { - packet_creator_.set_can_set_transmission_type(can_set_transmission_type); -} - MessageStatus QuicPacketGenerator::AddMessageFrame(QuicMessageId message_id, QuicMemSliceSpan message) { if (packet_creator_.combine_generator_and_creator()) {
diff --git a/quic/core/quic_packet_generator.h b/quic/core/quic_packet_generator.h index 7d8df7e..2a7f996 100644 --- a/quic/core/quic_packet_generator.h +++ b/quic/core/quic_packet_generator.h
@@ -45,7 +45,6 @@ #include <list> #include "net/third_party/quiche/src/quic/core/quic_packet_creator.h" -#include "net/third_party/quiche/src/quic/core/quic_pending_retransmission.h" #include "net/third_party/quiche/src/quic/core/quic_sent_packet_manager.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/platform/api/quic_export.h" @@ -149,12 +148,6 @@ const QuicDeque<QuicPathFrameBuffer>& payloads, const bool is_padded); - // Re-serializes frames with the original packet's packet number length. - // Used for retransmitting packets to ensure they aren't too long. - void ReserializeAllFrames(const QuicPendingRetransmission& retransmission, - char* buffer, - size_t buffer_len); - // Update the packet number length to use in future packets as soon as it // can be safely changed. void UpdatePacketNumberLength(QuicPacketNumber least_packet_awaited_by_peer, @@ -200,9 +193,6 @@ // Sets the retry token to be sent over the wire in IETF Initial packets. void SetRetryToken(QuicStringPiece retry_token); - // Allow/Disallow setting transmission type of next constructed packets. - void SetCanSetTransmissionType(bool can_set_transmission_type); - // Tries to add a message frame containing |message| and returns the status. MessageStatus AddMessageFrame(QuicMessageId message_id, QuicMemSliceSpan message);
diff --git a/quic/core/quic_packet_generator_test.cc b/quic/core/quic_packet_generator_test.cc index 5749ce0..7a7701e 100644 --- a/quic/core/quic_packet_generator_test.cc +++ b/quic/core/quic_packet_generator_test.cc
@@ -691,7 +691,6 @@ TEST_F(QuicPacketGeneratorTest, ConsumeDataFastPath) { delegate_.SetCanWriteAnything(); - generator_.SetCanSetTransmissionType(true); generator_.SetTransmissionType(LOSS_RETRANSMISSION); // Create a 10000 byte IOVector. @@ -933,7 +932,6 @@ // Regression test of b/120493795. TEST_F(QuicPacketGeneratorTest, PacketTransmissionType) { delegate_.SetCanWriteAnything(); - generator_.SetCanSetTransmissionType(true); // The first ConsumeData will fill the packet without flush. generator_.SetTransmissionType(LOSS_RETRANSMISSION);
diff --git a/quic/core/quic_packets.cc b/quic/core/quic_packets.cc index 68a078e..467d346 100644 --- a/quic/core/quic_packets.cc +++ b/quic/core/quic_packets.cc
@@ -475,7 +475,6 @@ has_ack(other.has_ack), has_stop_waiting(other.has_stop_waiting), transmission_type(other.transmission_type), - original_packet_number(other.original_packet_number), largest_acked(other.largest_acked), has_ack_frame_copy(other.has_ack_frame_copy) { retransmittable_frames.swap(other.retransmittable_frames);
diff --git a/quic/core/quic_packets.h b/quic/core/quic_packets.h index 361cd44..dab0849 100644 --- a/quic/core/quic_packets.h +++ b/quic/core/quic_packets.h
@@ -384,7 +384,6 @@ bool has_ack; bool has_stop_waiting; TransmissionType transmission_type; - QuicPacketNumber original_packet_number; // The largest acked of the AckFrame in this packet if has_ack is true, // 0 otherwise. QuicPacketNumber largest_acked;
diff --git a/quic/core/quic_pending_retransmission.h b/quic/core/quic_pending_retransmission.h deleted file mode 100644 index d1e9657..0000000 --- a/quic/core/quic_pending_retransmission.h +++ /dev/null
@@ -1,54 +0,0 @@ -// Copyright (c) 2016 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef QUICHE_QUIC_CORE_QUIC_PENDING_RETRANSMISSION_H_ -#define QUICHE_QUIC_CORE_QUIC_PENDING_RETRANSMISSION_H_ - -#include "net/third_party/quiche/src/quic/core/frames/quic_frame.h" -#include "net/third_party/quiche/src/quic/core/quic_transmission_info.h" -#include "net/third_party/quiche/src/quic/core/quic_types.h" -#include "net/third_party/quiche/src/quic/platform/api/quic_export.h" - -namespace quic { - -// Struct to store the pending retransmission information. -struct QUIC_EXPORT_PRIVATE QuicPendingRetransmission { - QuicPendingRetransmission(QuicPacketNumber packet_number, - TransmissionType transmission_type, - const QuicFrames& retransmittable_frames, - bool has_crypto_handshake, - int num_padding_bytes, - EncryptionLevel encryption_level, - QuicPacketNumberLength packet_number_length) - : packet_number(packet_number), - retransmittable_frames(retransmittable_frames), - transmission_type(transmission_type), - has_crypto_handshake(has_crypto_handshake), - num_padding_bytes(num_padding_bytes), - encryption_level(encryption_level), - packet_number_length(packet_number_length) {} - - QuicPendingRetransmission(QuicPacketNumber packet_number, - TransmissionType transmission_type, - const QuicTransmissionInfo& tranmission_info) - : packet_number(packet_number), - retransmittable_frames(tranmission_info.retransmittable_frames), - transmission_type(transmission_type), - has_crypto_handshake(tranmission_info.has_crypto_handshake), - num_padding_bytes(tranmission_info.num_padding_bytes), - encryption_level(tranmission_info.encryption_level), - packet_number_length(tranmission_info.packet_number_length) {} - - QuicPacketNumber packet_number; - const QuicFrames& retransmittable_frames; - TransmissionType transmission_type; - bool has_crypto_handshake; - int num_padding_bytes; - EncryptionLevel encryption_level; - QuicPacketNumberLength packet_number_length; -}; - -} // namespace quic - -#endif // QUICHE_QUIC_CORE_QUIC_PENDING_RETRANSMISSION_H_
diff --git a/quic/core/quic_sent_packet_manager.cc b/quic/core/quic_sent_packet_manager.cc index 70ae9ea..13ac894 100644 --- a/quic/core/quic_sent_packet_manager.cc +++ b/quic/core/quic_sent_packet_manager.cc
@@ -12,7 +12,6 @@ #include "net/third_party/quiche/src/quic/core/crypto/crypto_protocol.h" #include "net/third_party/quiche/src/quic/core/proto/cached_network_parameters_proto.h" #include "net/third_party/quiche/src/quic/core/quic_connection_stats.h" -#include "net/third_party/quiche/src/quic/core/quic_pending_retransmission.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" #include "net/third_party/quiche/src/quic/core/quic_utils.h" #include "net/third_party/quiche/src/quic/platform/api/quic_bug_tracker.h" @@ -41,12 +40,6 @@ // per draft RFC draft-dukkipati-tcpm-tcp-loss-probe. static const size_t kDefaultMaxTailLossProbes = 2; -inline bool HasCryptoHandshake(const QuicTransmissionInfo& transmission_info) { - DCHECK(!transmission_info.has_crypto_handshake || - !transmission_info.retransmittable_frames.empty()); - return transmission_info.has_crypto_handshake; -} - // Returns true of retransmissions of the specified type should retransmit // the frames directly (as opposed to resulting in a loss notification). inline bool ShouldForceRetransmission(TransmissionType transmission_type) { @@ -170,8 +163,7 @@ } } - if (GetQuicReloadableFlag(quic_enable_pto) && - session_decides_what_to_write()) { + if (GetQuicReloadableFlag(quic_enable_pto)) { if (config.HasClientSentConnectionOption(k2PTO, perspective)) { pto_enabled_ = true; QUIC_RELOADABLE_FLAG_COUNT_N(quic_enable_pto, 2, 4); @@ -335,10 +327,8 @@ QuicTime ack_receive_time, bool rtt_updated, QuicByteCount prior_bytes_in_flight) { - if (session_decides_what_to_write()) { - unacked_packets_.NotifyAggregatedStreamFrameAcked( - last_ack_frame_.ack_delay_time); - } + unacked_packets_.NotifyAggregatedStreamFrameAcked( + last_ack_frame_.ack_delay_time); InvokeLossDetection(ack_receive_time); // Ignore losses in RTO mode. if (consecutive_rto_count_ > 0 && !use_new_rto_) { @@ -436,29 +426,14 @@ void QuicSentPacketManager::NeuterUnencryptedPackets() { QuicPacketNumber packet_number = unacked_packets_.GetLeastUnacked(); - if (session_decides_what_to_write()) { - for (QuicUnackedPacketMap::const_iterator it = unacked_packets_.begin(); - it != unacked_packets_.end(); ++it, ++packet_number) { - if (!it->retransmittable_frames.empty() && - it->encryption_level == ENCRYPTION_INITIAL) { - // Once the connection swithes to forward secure, no unencrypted packets - // will be sent. The data has been abandoned in the cryto stream. Remove - // it from in flight. - unacked_packets_.RemoveFromInFlight(packet_number); - } - } - return; - } for (QuicUnackedPacketMap::const_iterator it = unacked_packets_.begin(); it != unacked_packets_.end(); ++it, ++packet_number) { - if (it->encryption_level == ENCRYPTION_INITIAL) { - // Once you're forward secure, no unencrypted packets will be sent, - // crypto or otherwise. Unencrypted packets are neutered and abandoned, - // to ensure they are not retransmitted or considered lost from a - // congestion control perspective. - pending_retransmissions_.erase(packet_number); + if (!it->retransmittable_frames.empty() && + it->encryption_level == ENCRYPTION_INITIAL) { + // Once the connection swithes to forward secure, no unencrypted packets + // will be sent. The data has been abandoned in the cryto stream. Remove + // it from in flight. unacked_packets_.RemoveFromInFlight(packet_number); - unacked_packets_.RemoveRetransmittability(packet_number); } } } @@ -467,20 +442,10 @@ QuicPacketNumber packet_number = unacked_packets_.GetLeastUnacked(); for (QuicUnackedPacketMap::const_iterator it = unacked_packets_.begin(); it != unacked_packets_.end(); ++it, ++packet_number) { - if (session_decides_what_to_write()) { - if (!it->retransmittable_frames.empty() && - unacked_packets_.GetPacketNumberSpace(it->encryption_level) == - HANDSHAKE_DATA) { - unacked_packets_.RemoveFromInFlight(packet_number); - } - continue; - } - if (unacked_packets_.GetPacketNumberSpace(it->encryption_level) == - HANDSHAKE_DATA && - unacked_packets_.HasRetransmittableFrames(*it)) { - pending_retransmissions_.erase(packet_number); + if (!it->retransmittable_frames.empty() && + unacked_packets_.GetPacketNumberSpace(it->encryption_level) == + HANDSHAKE_DATA) { unacked_packets_.RemoveFromInFlight(packet_number); - unacked_packets_.RemoveRetransmittability(packet_number); } } } @@ -490,26 +455,15 @@ TransmissionType transmission_type) { QuicTransmissionInfo* transmission_info = unacked_packets_.GetMutableTransmissionInfo(packet_number); - // When session decides what to write, a previous RTO retransmission may cause - // connection close; packets without retransmittable frames can be marked for - // loss retransmissions. - QUIC_BUG_IF((transmission_type != LOSS_RETRANSMISSION && - (!session_decides_what_to_write() || - transmission_type != RTO_RETRANSMISSION)) && + // A previous RTO retransmission may cause connection close; packets without + // retransmittable frames can be marked for loss retransmissions. + QUIC_BUG_IF(transmission_type != LOSS_RETRANSMISSION && + transmission_type != RTO_RETRANSMISSION && !unacked_packets_.HasRetransmittableFrames(*transmission_info)) << "transmission_type: " << TransmissionTypeToString(transmission_type); // Handshake packets should never be sent as probing retransmissions. DCHECK(pto_enabled_ || !transmission_info->has_crypto_handshake || transmission_type != PROBING_RETRANSMISSION); - if (!session_decides_what_to_write()) { - if (!unacked_packets_.HasRetransmittableFrames(*transmission_info)) { - return; - } - if (!QuicContainsKey(pending_retransmissions_, packet_number)) { - pending_retransmissions_[packet_number] = transmission_type; - } - return; - } HandleRetransmission(transmission_type, transmission_info); @@ -521,7 +475,6 @@ void QuicSentPacketManager::HandleRetransmission( TransmissionType transmission_type, QuicTransmissionInfo* transmission_info) { - DCHECK(session_decides_what_to_write()); if (ShouldForceRetransmission(transmission_type)) { // TODO(fayang): Consider to make RTO and PROBING retransmission // strategies be configurable by applications. Today, TLP, RTO and PROBING @@ -565,146 +518,57 @@ void QuicSentPacketManager::RecordSpuriousRetransmissions( const QuicTransmissionInfo& info, QuicPacketNumber acked_packet_number) { - if (session_decides_what_to_write()) { - RecordOneSpuriousRetransmission(info); - if (!detect_spurious_losses_ && - info.transmission_type == LOSS_RETRANSMISSION) { - // Only inform the loss detection of spurious retransmits it caused. - loss_algorithm_->SpuriousRetransmitDetected( - unacked_packets_, clock_->Now(), rtt_stats_, acked_packet_number); - } - return; - } - QuicPacketNumber retransmission = info.retransmission; - while (retransmission.IsInitialized()) { - const QuicTransmissionInfo& retransmit_info = - unacked_packets_.GetTransmissionInfo(retransmission); - retransmission = retransmit_info.retransmission; - RecordOneSpuriousRetransmission(retransmit_info); - } - // Only inform the loss detection of spurious retransmits it caused. - if (unacked_packets_.GetTransmissionInfo(info.retransmission) - .transmission_type == LOSS_RETRANSMISSION) { + RecordOneSpuriousRetransmission(info); + if (!detect_spurious_losses_ && + info.transmission_type == LOSS_RETRANSMISSION) { + // Only inform the loss detection of spurious retransmits it caused. loss_algorithm_->SpuriousRetransmitDetected( - unacked_packets_, clock_->Now(), rtt_stats_, info.retransmission); + unacked_packets_, clock_->Now(), rtt_stats_, acked_packet_number); } } -QuicPendingRetransmission QuicSentPacketManager::NextPendingRetransmission() { - QUIC_BUG_IF(pending_retransmissions_.empty()) - << "Unexpected call to NextPendingRetransmission() with empty pending " - << "retransmission list. Corrupted memory usage imminent."; - QUIC_BUG_IF(session_decides_what_to_write()) - << "Unexpected call to NextPendingRetransmission() when session handles " - "retransmissions"; - QuicPacketNumber packet_number = pending_retransmissions_.begin()->first; - TransmissionType transmission_type = pending_retransmissions_.begin()->second; - if (unacked_packets_.HasPendingCryptoPackets()) { - // Ensure crypto packets are retransmitted before other packets. - for (const auto& pair : pending_retransmissions_) { - if (HasCryptoHandshake( - unacked_packets_.GetTransmissionInfo(pair.first))) { - packet_number = pair.first; - transmission_type = pair.second; - break; - } - } - } - DCHECK(unacked_packets_.IsUnacked(packet_number)) << packet_number; - const QuicTransmissionInfo& transmission_info = - unacked_packets_.GetTransmissionInfo(packet_number); - DCHECK(unacked_packets_.HasRetransmittableFrames(transmission_info)); - - return QuicPendingRetransmission(packet_number, transmission_type, - transmission_info); -} - -QuicPacketNumber QuicSentPacketManager::GetNewestRetransmission( - QuicPacketNumber packet_number, - const QuicTransmissionInfo& transmission_info) const { - if (session_decides_what_to_write()) { - return packet_number; - } - QuicPacketNumber retransmission = transmission_info.retransmission; - while (retransmission.IsInitialized()) { - packet_number = retransmission; - retransmission = - unacked_packets_.GetTransmissionInfo(retransmission).retransmission; - } - return packet_number; -} - void QuicSentPacketManager::MarkPacketHandled(QuicPacketNumber packet_number, QuicTransmissionInfo* info, QuicTime ack_receive_time, QuicTime::Delta ack_delay_time, QuicTime receive_timestamp) { - QuicPacketNumber newest_transmission = - GetNewestRetransmission(packet_number, *info); - // Remove the most recent packet, if it is pending retransmission. - pending_retransmissions_.erase(newest_transmission); - - if (newest_transmission == packet_number) { - // Try to aggregate acked stream frames if acked packet is not a - // retransmission. - const bool fast_path = session_decides_what_to_write() && - info->transmission_type == NOT_RETRANSMISSION; - if (fast_path) { - unacked_packets_.MaybeAggregateAckedStreamFrame(*info, ack_delay_time, - receive_timestamp); - } else { - if (session_decides_what_to_write()) { - unacked_packets_.NotifyAggregatedStreamFrameAcked(ack_delay_time); - } - const bool new_data_acked = unacked_packets_.NotifyFramesAcked( - *info, ack_delay_time, receive_timestamp); - if (session_decides_what_to_write() && !new_data_acked && - info->transmission_type != NOT_RETRANSMISSION) { - // Record as a spurious retransmission if this packet is a - // retransmission and no new data gets acked. - QUIC_DVLOG(1) << "Detect spurious retransmitted packet " - << packet_number << " transmission type: " - << TransmissionTypeToString(info->transmission_type); - RecordSpuriousRetransmissions(*info, packet_number); - } - } - if (detect_spurious_losses_ && session_decides_what_to_write() && - info->state == LOST) { - // Record as a spurious loss as a packet previously declared lost gets - // acked. - QUIC_RELOADABLE_FLAG_COUNT(quic_detect_spurious_loss); - const PacketNumberSpace packet_number_space = - unacked_packets_.GetPacketNumberSpace(info->encryption_level); - const QuicPacketNumber previous_largest_acked = - supports_multiple_packet_number_spaces() - ? unacked_packets_.GetLargestAckedOfPacketNumberSpace( - packet_number_space) - : unacked_packets_.largest_acked(); - QUIC_DVLOG(1) << "Packet " << packet_number - << " was detected lost spuriously, " - "previous_largest_acked: " - << previous_largest_acked; - loss_algorithm_->SpuriousLossDetected(unacked_packets_, rtt_stats_, - ack_receive_time, packet_number, - previous_largest_acked); - } + // Try to aggregate acked stream frames if acked packet is not a + // retransmission. + if (info->transmission_type == NOT_RETRANSMISSION) { + unacked_packets_.MaybeAggregateAckedStreamFrame(*info, ack_delay_time, + receive_timestamp); } else { - DCHECK(!session_decides_what_to_write()); - RecordSpuriousRetransmissions(*info, packet_number); - // Remove the most recent packet from flight if it's a crypto handshake - // packet, since they won't be acked now that one has been processed. - // Other crypto handshake packets won't be in flight, only the newest - // transmission of a crypto packet is in flight at once. - // TODO(ianswett): Instead of handling all crypto packets special, - // only handle null encrypted packets in a special way. - const QuicTransmissionInfo& newest_transmission_info = - unacked_packets_.GetTransmissionInfo(newest_transmission); - unacked_packets_.NotifyFramesAcked(newest_transmission_info, ack_delay_time, - receive_timestamp); - if (HasCryptoHandshake(newest_transmission_info)) { - unacked_packets_.RemoveFromInFlight(newest_transmission); + unacked_packets_.NotifyAggregatedStreamFrameAcked(ack_delay_time); + const bool new_data_acked = unacked_packets_.NotifyFramesAcked( + *info, ack_delay_time, receive_timestamp); + if (!new_data_acked && info->transmission_type != NOT_RETRANSMISSION) { + // Record as a spurious retransmission if this packet is a + // retransmission and no new data gets acked. + QUIC_DVLOG(1) << "Detect spurious retransmitted packet " << packet_number + << " transmission type: " + << TransmissionTypeToString(info->transmission_type); + RecordSpuriousRetransmissions(*info, packet_number); } } + if (detect_spurious_losses_ && info->state == LOST) { + // Record as a spurious loss as a packet previously declared lost gets + // acked. + QUIC_RELOADABLE_FLAG_COUNT(quic_detect_spurious_loss); + const PacketNumberSpace packet_number_space = + unacked_packets_.GetPacketNumberSpace(info->encryption_level); + const QuicPacketNumber previous_largest_acked = + supports_multiple_packet_number_spaces() + ? unacked_packets_.GetLargestAckedOfPacketNumberSpace( + packet_number_space) + : unacked_packets_.largest_acked(); + QUIC_DVLOG(1) << "Packet " << packet_number + << " was detected lost spuriously, " + "previous_largest_acked: " + << previous_largest_acked; + loss_algorithm_->SpuriousLossDetected(unacked_packets_, rtt_stats_, + ack_receive_time, packet_number, + previous_largest_acked); + } if (network_change_visitor_ != nullptr && info->bytes_sent > largest_mtu_acked_) { @@ -718,7 +582,6 @@ bool QuicSentPacketManager::OnPacketSent( SerializedPacket* serialized_packet, - QuicPacketNumber original_packet_number, QuicTime sent_time, TransmissionType transmission_type, HasRetransmittableData has_retransmittable_data) { @@ -727,11 +590,6 @@ DCHECK(!unacked_packets_.IsUnacked(packet_number)); QUIC_BUG_IF(serialized_packet->encrypted_length == 0) << "Cannot send empty packets."; - - if (original_packet_number.IsInitialized()) { - pending_retransmissions_.erase(original_packet_number); - } - if (pending_timer_transmission_count_ > 0) { --pending_timer_transmission_count_; } @@ -747,8 +605,8 @@ serialized_packet->encrypted_length, has_retransmittable_data); } - unacked_packets_.AddSentPacket(serialized_packet, original_packet_number, - transmission_type, sent_time, in_flight); + unacked_packets_.AddSentPacket(serialized_packet, transmission_type, + sent_time, in_flight); // Reset the retransmission timer anytime a pending packet is sent. return in_flight; } @@ -806,25 +664,18 @@ for (QuicUnackedPacketMap::const_iterator it = unacked_packets_.begin(); it != unacked_packets_.end(); ++it, ++packet_number) { // Only retransmit frames which are in flight, and therefore have been sent. - if (!it->in_flight || - (session_decides_what_to_write() && it->state != OUTSTANDING) || + if (!it->in_flight || it->state != OUTSTANDING || !it->has_crypto_handshake || !unacked_packets_.HasRetransmittableFrames(*it)) { continue; } packet_retransmitted = true; - if (session_decides_what_to_write()) { - crypto_retransmissions.push_back(packet_number); - } else { - MarkForRetransmission(packet_number, HANDSHAKE_RETRANSMISSION); - } + crypto_retransmissions.push_back(packet_number); ++pending_timer_transmission_count_; } DCHECK(packet_retransmitted) << "No crypto packets found to retransmit."; - if (session_decides_what_to_write()) { - for (QuicPacketNumber retransmission : crypto_retransmissions) { - MarkForRetransmission(retransmission, HANDSHAKE_RETRANSMISSION); - } + for (QuicPacketNumber retransmission : crypto_retransmissions) { + MarkForRetransmission(retransmission, HANDSHAKE_RETRANSMISSION); } } @@ -844,8 +695,7 @@ for (QuicUnackedPacketMap::const_iterator it = unacked_packets_.begin(); it != unacked_packets_.end(); ++it, ++packet_number) { // Only retransmit frames which are in flight, and therefore have been sent. - if (!it->in_flight || - (session_decides_what_to_write() && it->state != OUTSTANDING) || + if (!it->in_flight || it->state != OUTSTANDING || !unacked_packets_.HasRetransmittableFrames(*it)) { continue; } @@ -866,34 +716,12 @@ std::vector<QuicPacketNumber> retransmissions; for (QuicUnackedPacketMap::const_iterator it = unacked_packets_.begin(); it != unacked_packets_.end(); ++it, ++packet_number) { - if ((!session_decides_what_to_write() || it->state == OUTSTANDING) && + if (it->state == OUTSTANDING && unacked_packets_.HasRetransmittableFrames(*it) && pending_timer_transmission_count_ < max_rto_packets_) { - if (session_decides_what_to_write()) { - retransmissions.push_back(packet_number); - } else { - MarkForRetransmission(packet_number, RTO_RETRANSMISSION); - } + retransmissions.push_back(packet_number); ++pending_timer_transmission_count_; } - // Abandon non-retransmittable data that's in flight to ensure it doesn't - // fill up the congestion window. - bool has_retransmissions = it->retransmission.IsInitialized(); - if (session_decides_what_to_write()) { - has_retransmissions = it->state != OUTSTANDING; - } - if (!session_decides_what_to_write() && it->in_flight && - !has_retransmissions && - !unacked_packets_.HasRetransmittableFrames(*it)) { - // Log only for non-retransmittable data. - // Retransmittable data is marked as lost during loss detection, and will - // be logged later. - unacked_packets_.RemoveFromInFlight(packet_number); - if (debug_delegate_ != nullptr) { - debug_delegate_->OnPacketLoss(packet_number, RTO_RETRANSMISSION, - clock_->Now()); - } - } } if (pending_timer_transmission_count_ > 0) { if (consecutive_rto_count_ == 0) { @@ -901,17 +729,15 @@ } ++consecutive_rto_count_; } - if (session_decides_what_to_write()) { - for (QuicPacketNumber retransmission : retransmissions) { - MarkForRetransmission(retransmission, RTO_RETRANSMISSION); - } - if (retransmissions.empty()) { - QUIC_BUG_IF(pending_timer_transmission_count_ != 0); - // No packets to be RTO retransmitted, raise up a credit to allow - // connection to send. - QUIC_CODE_COUNT(no_packets_to_be_rto_retransmitted); - pending_timer_transmission_count_ = 1; - } + for (QuicPacketNumber retransmission : retransmissions) { + MarkForRetransmission(retransmission, RTO_RETRANSMISSION); + } + if (retransmissions.empty()) { + QUIC_BUG_IF(pending_timer_transmission_count_ != 0); + // No packets to be RTO retransmitted, raise up a credit to allow + // connection to send. + QUIC_CODE_COUNT(no_packets_to_be_rto_retransmitted); + pending_timer_transmission_count_ = 1; } } @@ -951,7 +777,6 @@ } void QuicSentPacketManager::EnableIetfPtoAndLossDetection() { - DCHECK(session_decides_what_to_write()); pto_enabled_ = true; handshake_mode_disabled_ = true; uber_loss_algorithm_.SetLossDetectionType(kIetfLossDetection); @@ -1058,10 +883,6 @@ // Do not set the timer if there is any credit left. return QuicTime::Zero(); } - if (!session_decides_what_to_write() && - !unacked_packets_.HasUnackedRetransmittableFrames()) { - return QuicTime::Zero(); - } switch (GetRetransmissionMode()) { case HANDSHAKE_MODE: return unacked_packets_.GetLastCryptoPacketSentTime() + @@ -1142,9 +963,6 @@ size_t consecutive_tlp_count) const { QuicTime::Delta srtt = rtt_stats_.SmoothedOrInitialRtt(); if (enable_half_rtt_tail_loss_probe_ && consecutive_tlp_count == 0u) { - if (!session_decides_what_to_write()) { - return std::max(min_tlp_timeout_, srtt * 0.5); - } if (unacked_packets().HasUnackedStreamData()) { // Enable TLPR if there are pending data packets. return std::max(min_tlp_timeout_, srtt * 0.5); @@ -1224,22 +1042,6 @@ return send_algorithm_->GetDebugState(); } -void QuicSentPacketManager::CancelRetransmissionsForStream( - QuicStreamId stream_id) { - if (session_decides_what_to_write()) { - return; - } - unacked_packets_.CancelRetransmissionsForStream(stream_id); - auto it = pending_retransmissions_.begin(); - while (it != pending_retransmissions_.end()) { - if (unacked_packets_.HasRetransmittableFrames(it->first)) { - ++it; - continue; - } - it = pending_retransmissions_.erase(it); - } -} - void QuicSentPacketManager::SetSendAlgorithm( CongestionControlType congestion_control_type) { SetSendAlgorithm(SendAlgorithmInterface::Create( @@ -1428,11 +1230,6 @@ rtt_stats_.set_initial_rtt(std::max(min_rtt, std::min(max_rtt, rtt))); } -void QuicSentPacketManager::SetSessionDecideWhatToWrite( - bool session_decides_what_to_write) { - unacked_packets_.SetSessionDecideWhatToWrite(session_decides_what_to_write); -} - void QuicSentPacketManager::EnableMultiplePacketNumberSpacesSupport() { unacked_packets_.EnableMultiplePacketNumberSpacesSupport(); }
diff --git a/quic/core/quic_sent_packet_manager.h b/quic/core/quic_sent_packet_manager.h index eee5e01..a517dec 100644 --- a/quic/core/quic_sent_packet_manager.h +++ b/quic/core/quic_sent_packet_manager.h
@@ -19,7 +19,6 @@ #include "net/third_party/quiche/src/quic/core/congestion_control/uber_loss_algorithm.h" #include "net/third_party/quiche/src/quic/core/proto/cached_network_parameters_proto.h" #include "net/third_party/quiche/src/quic/core/quic_packets.h" -#include "net/third_party/quiche/src/quic/core/quic_pending_retransmission.h" #include "net/third_party/quiche/src/quic/core/quic_sustained_bandwidth_recorder.h" #include "net/third_party/quiche/src/quic/core/quic_transmission_info.h" #include "net/third_party/quiche/src/quic/core/quic_types.h" @@ -165,16 +164,6 @@ // TODO(fayang): Consider replace this function with NeuterHandshakePackets. void NeuterUnencryptedPackets(); - // Returns true if there are pending retransmissions. - // Not const because retransmissions may be cancelled before returning. - bool HasPendingRetransmissions() const { - return !pending_retransmissions_.empty(); - } - - // Retrieves the next pending retransmission. You must ensure that - // there are pending retransmissions prior to calling this function. - QuicPendingRetransmission NextPendingRetransmission(); - // Returns true if there's outstanding crypto data. bool HasUnackedCryptoPackets() const { return unacked_packets_.HasPendingCryptoPackets(); @@ -195,7 +184,6 @@ // the number of bytes sent and if they were retransmitted. Returns true if // the sender should reset the retransmission timer. bool OnPacketSent(SerializedPacket* serialized_packet, - QuicPacketNumber original_packet_number, QuicTime sent_time, TransmissionType transmission_type, HasRetransmittableData has_retransmittable_data); @@ -276,9 +264,6 @@ return unacked_packets_.bytes_in_flight(); } - // No longer retransmit data for |stream_id|. - void CancelRetransmissionsForStream(QuicStreamId stream_id); - // Called when peer address changes and the connection migrates. void OnConnectionMigration(AddressChangeType type); @@ -300,9 +285,6 @@ QuicPacketNumber ack_packet_number, EncryptionLevel ack_decrypted_level); - // Called to enable/disable letting session decide what to write. - void SetSessionDecideWhatToWrite(bool session_decides_what_to_write); - void EnableMultiplePacketNumberSpacesSupport(); void SetDebugDelegate(DebugDelegate* debug_delegate); @@ -365,10 +347,6 @@ bool handshake_confirmed() const { return handshake_confirmed_; } - bool session_decides_what_to_write() const { - return unacked_packets_.session_decides_what_to_write(); - } - size_t pending_timer_transmission_count() const { return pending_timer_transmission_count_; } @@ -421,11 +399,6 @@ friend class test::QuicConnectionPeer; friend class test::QuicSentPacketManagerPeer; - typedef QuicLinkedHashMap<QuicPacketNumber, - TransmissionType, - QuicPacketNumberHash> - PendingRetransmissionMap; - // Returns the current retransmission mode. RetransmissionTimeoutMode GetRetransmissionMode() const; @@ -465,11 +438,6 @@ // Returns the probe timeout. const QuicTime::Delta GetProbeTimeoutDelay() const; - // Returns the newest transmission associated with a packet. - QuicPacketNumber GetNewestRetransmission( - QuicPacketNumber packet_number, - const QuicTransmissionInfo& transmission_info) const; - // Update the RTT if the ack is for the largest acked packet number. // Returns true if the rtt was updated. bool MaybeUpdateRTT(QuicPacketNumber largest_acked, @@ -551,9 +519,6 @@ // set to nullptr. QuicUnackedPacketMap unacked_packets_; - // Pending retransmissions which have not been packetized and sent yet. - PendingRetransmissionMap pending_retransmissions_; - const QuicClock* clock_; QuicRandom* random_; QuicConnectionStats* stats_;
diff --git a/quic/core/quic_sent_packet_manager_test.cc b/quic/core/quic_sent_packet_manager_test.cc index c7b3cfd..7872f74 100644 --- a/quic/core/quic_sent_packet_manager_test.cc +++ b/quic/core/quic_sent_packet_manager_test.cc
@@ -7,7 +7,6 @@ #include <memory> #include <utility> -#include "net/third_party/quiche/src/quic/core/quic_pending_retransmission.h" #include "net/third_party/quiche/src/quic/platform/api/quic_arraysize.h" #include "net/third_party/quiche/src/quic/platform/api/quic_flags.h" #include "net/third_party/quiche/src/quic/platform/api/quic_string_piece.h" @@ -53,7 +52,7 @@ QuicTime detection_time)); }; -class QuicSentPacketManagerTest : public QuicTestWithParam<bool> { +class QuicSentPacketManagerTest : public QuicTest { public: void RetransmitCryptoPacket(uint64_t packet_number) { EXPECT_CALL( @@ -64,8 +63,8 @@ packet.retransmittable_frames.push_back( QuicFrame(QuicStreamFrame(1, false, 0, QuicStringPiece()))); packet.has_crypto_handshake = IS_HANDSHAKE; - manager_.OnPacketSent(&packet, QuicPacketNumber(), clock_.Now(), - HANDSHAKE_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); + manager_.OnPacketSent(&packet, clock_.Now(), HANDSHAKE_RETRANSMISSION, + HAS_RETRANSMITTABLE_DATA); } void RetransmitDataPacket(uint64_t packet_number, @@ -77,7 +76,7 @@ kDefaultLength, HAS_RETRANSMITTABLE_DATA)); SerializedPacket packet(CreatePacket(packet_number, true)); packet.encryption_level = level; - manager_.OnPacketSent(&packet, QuicPacketNumber(), clock_.Now(), type, + manager_.OnPacketSent(&packet, clock_.Now(), type, HAS_RETRANSMITTABLE_DATA); } @@ -102,7 +101,6 @@ clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(1000)); manager_.SetNetworkChangeVisitor(network_change_visitor_.get()); manager_.SetSessionNotifier(¬ifier_); - manager_.SetSessionDecideWhatToWrite(GetParam()); EXPECT_CALL(*send_algorithm_, HasReliableBandwidthEstimate()) .Times(AnyNumber()); @@ -210,53 +208,31 @@ uint64_t new_packet_number, TransmissionType transmission_type) { bool is_lost = false; - if (manager_.session_decides_what_to_write()) { - if (transmission_type == HANDSHAKE_RETRANSMISSION || - transmission_type == TLP_RETRANSMISSION || - transmission_type == RTO_RETRANSMISSION || - transmission_type == PROBING_RETRANSMISSION) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(WithArgs<1>( - Invoke([this, new_packet_number](TransmissionType type) { - RetransmitDataPacket(new_packet_number, type); - }))); - } else { - EXPECT_CALL(notifier_, OnFrameLost(_)).Times(1); - is_lost = true; - } + if (transmission_type == HANDSHAKE_RETRANSMISSION || + transmission_type == TLP_RETRANSMISSION || + transmission_type == RTO_RETRANSMISSION || + transmission_type == PROBING_RETRANSMISSION) { + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>( + Invoke([this, new_packet_number](TransmissionType type) { + RetransmitDataPacket(new_packet_number, type); + }))); + } else { + EXPECT_CALL(notifier_, OnFrameLost(_)).Times(1); + is_lost = true; } QuicSentPacketManagerPeer::MarkForRetransmission( &manager_, old_packet_number, transmission_type); - if (manager_.session_decides_what_to_write()) { - if (!is_lost) { - return; - } - EXPECT_CALL( - *send_algorithm_, - OnPacketSent(_, BytesInFlight(), QuicPacketNumber(new_packet_number), - kDefaultLength, HAS_RETRANSMITTABLE_DATA)); - SerializedPacket packet(CreatePacket(new_packet_number, true)); - manager_.OnPacketSent(&packet, QuicPacketNumber(), clock_.Now(), - transmission_type, HAS_RETRANSMITTABLE_DATA); + if (!is_lost) { return; } - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - QuicPendingRetransmission next_retransmission = - manager_.NextPendingRetransmission(); - EXPECT_EQ(QuicPacketNumber(old_packet_number), - next_retransmission.packet_number); - EXPECT_EQ(transmission_type, next_retransmission.transmission_type); - EXPECT_CALL( *send_algorithm_, OnPacketSent(_, BytesInFlight(), QuicPacketNumber(new_packet_number), kDefaultLength, HAS_RETRANSMITTABLE_DATA)); - SerializedPacket packet(CreatePacket(new_packet_number, false)); - manager_.OnPacketSent(&packet, QuicPacketNumber(old_packet_number), - clock_.Now(), transmission_type, + SerializedPacket packet(CreatePacket(new_packet_number, true)); + manager_.OnPacketSent(&packet, clock_.Now(), transmission_type, HAS_RETRANSMITTABLE_DATA); - EXPECT_TRUE(QuicSentPacketManagerPeer::IsRetransmission(&manager_, - new_packet_number)); } SerializedPacket CreateDataPacket(uint64_t packet_number) { @@ -293,8 +269,8 @@ QuicPacketNumber(packet_number), _, _)); SerializedPacket packet(CreateDataPacket(packet_number)); packet.encryption_level = encryption_level; - manager_.OnPacketSent(&packet, QuicPacketNumber(), clock_.Now(), - NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); + manager_.OnPacketSent(&packet, clock_.Now(), NOT_RETRANSMISSION, + HAS_RETRANSMITTABLE_DATA); } void SendPingPacket(uint64_t packet_number, @@ -304,8 +280,8 @@ QuicPacketNumber(packet_number), _, _)); SerializedPacket packet(CreatePingPacket(packet_number)); packet.encryption_level = encryption_level; - manager_.OnPacketSent(&packet, QuicPacketNumber(), clock_.Now(), - NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); + manager_.OnPacketSent(&packet, clock_.Now(), NOT_RETRANSMISSION, + HAS_RETRANSMITTABLE_DATA); } void SendCryptoPacket(uint64_t packet_number) { @@ -317,12 +293,9 @@ packet.retransmittable_frames.push_back( QuicFrame(QuicStreamFrame(1, false, 0, QuicStringPiece()))); packet.has_crypto_handshake = IS_HANDSHAKE; - manager_.OnPacketSent(&packet, QuicPacketNumber(), clock_.Now(), - NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, HasUnackedCryptoData()) - .WillRepeatedly(Return(true)); - } + manager_.OnPacketSent(&packet, clock_.Now(), NOT_RETRANSMISSION, + HAS_RETRANSMITTABLE_DATA); + EXPECT_CALL(notifier_, HasUnackedCryptoData()).WillRepeatedly(Return(true)); } void SendAckPacket(uint64_t packet_number, uint64_t largest_acked) { @@ -339,26 +312,11 @@ SerializedPacket packet(CreatePacket(packet_number, false)); packet.largest_acked = QuicPacketNumber(largest_acked); packet.encryption_level = level; - manager_.OnPacketSent(&packet, QuicPacketNumber(), clock_.Now(), - NOT_RETRANSMISSION, NO_RETRANSMITTABLE_DATA); - } - - // Based on QuicConnection's WritePendingRetransmissions. - void RetransmitNextPacket(uint64_t retransmission_packet_number) { - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - EXPECT_CALL( - *send_algorithm_, - OnPacketSent(_, _, QuicPacketNumber(retransmission_packet_number), - kDefaultLength, HAS_RETRANSMITTABLE_DATA)); - const QuicPendingRetransmission pending = - manager_.NextPendingRetransmission(); - SerializedPacket packet(CreatePacket(retransmission_packet_number, false)); - manager_.OnPacketSent(&packet, pending.packet_number, clock_.Now(), - pending.transmission_type, HAS_RETRANSMITTABLE_DATA); + manager_.OnPacketSent(&packet, clock_.Now(), NOT_RETRANSMISSION, + NO_RETRANSMITTABLE_DATA); } void EnablePto(QuicTag tag) { - manager_.SetSessionDecideWhatToWrite(true); SetQuicReloadableFlag(quic_enable_pto, true); QuicConfig config; QuicTagVector options; @@ -378,12 +336,7 @@ StrictMock<MockSessionNotifier> notifier_; }; -INSTANTIATE_TEST_SUITE_P(Tests, - QuicSentPacketManagerTest, - ::testing::Bool(), - ::testing::PrintToStringParamName()); - -TEST_P(QuicSentPacketManagerTest, IsUnacked) { +TEST_F(QuicSentPacketManagerTest, IsUnacked) { VerifyUnackedPackets(nullptr, 0); SendDataPacket(1); @@ -394,23 +347,18 @@ QUIC_ARRAYSIZE(retransmittable)); } -TEST_P(QuicSentPacketManagerTest, IsUnAckedRetransmit) { +TEST_F(QuicSentPacketManagerTest, IsUnAckedRetransmit) { SendDataPacket(1); RetransmitAndSendPacket(1, 2); EXPECT_TRUE(QuicSentPacketManagerPeer::IsRetransmission(&manager_, 2)); uint64_t unacked[] = {1, 2}; VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); - std::vector<uint64_t> retransmittable; - if (manager_.session_decides_what_to_write()) { - retransmittable = {1, 2}; - } else { - retransmittable = {2}; - } + std::vector<uint64_t> retransmittable = {1, 2}; VerifyRetransmittablePackets(&retransmittable[0], retransmittable.size()); } -TEST_P(QuicSentPacketManagerTest, RetransmitThenAck) { +TEST_F(QuicSentPacketManagerTest, RetransmitThenAck) { SendDataPacket(1); RetransmitAndSendPacket(1, 2); @@ -422,9 +370,7 @@ EXPECT_EQ(PACKETS_NEWLY_ACKED, manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1), ENCRYPTION_INITIAL)); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); - } + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); // Packet 1 is unacked, pending, but not retransmittable. uint64_t unacked[] = {1}; VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); @@ -432,21 +378,13 @@ VerifyRetransmittablePackets(nullptr, 0); } -TEST_P(QuicSentPacketManagerTest, RetransmitThenAckBeforeSend) { +TEST_F(QuicSentPacketManagerTest, RetransmitThenAckBeforeSend) { SendDataPacket(1); - if (manager_.session_decides_what_to_write()) { - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(WithArgs<1>(Invoke([this](TransmissionType type) { - RetransmitDataPacket(2, type); - }))); - } - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(2, type); }))); QuicSentPacketManagerPeer::MarkForRetransmission(&manager_, 1, TLP_RETRANSMISSION); - if (!manager_.session_decides_what_to_write()) { - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - } // Ack 1. ExpectAck(1); manager_.OnAckFrameStart(QuicPacketNumber(1), QuicTime::Delta::Infinite(), @@ -456,40 +394,21 @@ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1), ENCRYPTION_INITIAL)); - // There should no longer be a pending retransmission. - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); - uint64_t unacked[] = {2}; - VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); - // We do not know packet 2 is a spurious retransmission until it gets acked. - } else { - // No unacked packets remain. - VerifyUnackedPackets(nullptr, 0); - } + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); + uint64_t unacked[] = {2}; + VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); + // We do not know packet 2 is a spurious retransmission until it gets acked. VerifyRetransmittablePackets(nullptr, 0); EXPECT_EQ(0u, stats_.packets_spuriously_retransmitted); } -TEST_P(QuicSentPacketManagerTest, RetransmitThenStopRetransmittingBeforeSend) { +TEST_F(QuicSentPacketManagerTest, RetransmitThenStopRetransmittingBeforeSend) { SendDataPacket(1); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)); QuicSentPacketManagerPeer::MarkForRetransmission(&manager_, 1, TLP_RETRANSMISSION); - if (!manager_.session_decides_what_to_write()) { - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - } - manager_.CancelRetransmissionsForStream(kStreamId); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); - } - - // There should no longer be a pending retransmission. - EXPECT_FALSE(manager_.HasPendingRetransmissions()); + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); uint64_t unacked[] = {1}; VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); @@ -497,7 +416,7 @@ EXPECT_EQ(0u, stats_.packets_spuriously_retransmitted); } -TEST_P(QuicSentPacketManagerTest, RetransmitThenAckPrevious) { +TEST_F(QuicSentPacketManagerTest, RetransmitThenAckPrevious) { SendDataPacket(1); RetransmitAndSendPacket(1, 2); QuicTime::Delta rtt = QuicTime::Delta::FromMilliseconds(15); @@ -511,30 +430,26 @@ EXPECT_EQ(PACKETS_NEWLY_ACKED, manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1), ENCRYPTION_INITIAL)); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); - } + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); // 2 remains unacked, but no packets have retransmittable data. uint64_t unacked[] = {2}; VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); EXPECT_TRUE(manager_.HasInFlightPackets()); VerifyRetransmittablePackets(nullptr, 0); - if (manager_.session_decides_what_to_write()) { - // Ack 2 causes 2 be considered as spurious retransmission. - EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).WillOnce(Return(false)); - ExpectAck(2); - manager_.OnAckFrameStart(QuicPacketNumber(2), QuicTime::Delta::Infinite(), - clock_.Now()); - manager_.OnAckRange(QuicPacketNumber(1), QuicPacketNumber(3)); - EXPECT_EQ(PACKETS_NEWLY_ACKED, - manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(2), - ENCRYPTION_INITIAL)); - } + // Ack 2 causes 2 be considered as spurious retransmission. + EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).WillOnce(Return(false)); + ExpectAck(2); + manager_.OnAckFrameStart(QuicPacketNumber(2), QuicTime::Delta::Infinite(), + clock_.Now()); + manager_.OnAckRange(QuicPacketNumber(1), QuicPacketNumber(3)); + EXPECT_EQ(PACKETS_NEWLY_ACKED, + manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(2), + ENCRYPTION_INITIAL)); EXPECT_EQ(1u, stats_.packets_spuriously_retransmitted); } -TEST_P(QuicSentPacketManagerTest, RetransmitThenAckPreviousThenNackRetransmit) { +TEST_F(QuicSentPacketManagerTest, RetransmitThenAckPreviousThenNackRetransmit) { SendDataPacket(1); RetransmitAndSendPacket(1, 2); QuicTime::Delta rtt = QuicTime::Delta::FromMilliseconds(15); @@ -574,13 +489,11 @@ ENCRYPTION_INITIAL)); ExpectAckAndLoss(true, 5, 2); - if (manager_.session_decides_what_to_write()) { - // Frames in all packets are acked. - EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); - // Notify session that stream frame in packet 2 gets lost although it is - // not outstanding. - EXPECT_CALL(notifier_, OnFrameLost(_)).Times(1); - } + // Frames in all packets are acked. + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); + // Notify session that stream frame in packet 2 gets lost although it is + // not outstanding. + EXPECT_CALL(notifier_, OnFrameLost(_)).Times(1); manager_.OnAckFrameStart(QuicPacketNumber(5), QuicTime::Delta::Infinite(), clock_.Now()); manager_.OnAckRange(QuicPacketNumber(3), QuicPacketNumber(6)); @@ -589,13 +502,8 @@ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(4), ENCRYPTION_INITIAL)); - if (manager_.session_decides_what_to_write()) { - uint64_t unacked[] = {2}; - VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); - } else { - // No packets remain unacked. - VerifyUnackedPackets(nullptr, 0); - } + uint64_t unacked[] = {2}; + VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); EXPECT_FALSE(manager_.HasInFlightPackets()); VerifyRetransmittablePackets(nullptr, 0); @@ -604,7 +512,7 @@ EXPECT_EQ(QuicTime::Zero(), manager_.GetRetransmissionTime()); } -TEST_P(QuicSentPacketManagerTest, +TEST_F(QuicSentPacketManagerTest, DISABLED_RetransmitTwiceThenAckPreviousBeforeSend) { SendDataPacket(1); RetransmitAndSendPacket(1, 2); @@ -613,7 +521,6 @@ EXPECT_CALL(*send_algorithm_, OnRetransmissionTimeout(true)); EXPECT_CALL(*network_change_visitor_, OnCongestionChange()); manager_.OnRetransmissionTimeout(); - EXPECT_TRUE(manager_.HasPendingRetransmissions()); // Ack 1 but not 2, before 2 is able to be sent. // Since 1 has been retransmitted, it has already been lost, and so the @@ -638,17 +545,11 @@ EXPECT_EQ(QuicTime::Zero(), manager_.GetRetransmissionTime()); } -TEST_P(QuicSentPacketManagerTest, RetransmitTwiceThenAckFirst) { +TEST_F(QuicSentPacketManagerTest, RetransmitTwiceThenAckFirst) { StrictMock<MockDebugDelegate> debug_delegate; - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(debug_delegate, OnSpuriousPacketRetransmission( - TLP_RETRANSMISSION, kDefaultLength)) - .Times(1); - } else { - EXPECT_CALL(debug_delegate, OnSpuriousPacketRetransmission( - TLP_RETRANSMISSION, kDefaultLength)) - .Times(2); - } + EXPECT_CALL(debug_delegate, OnSpuriousPacketRetransmission(TLP_RETRANSMISSION, + kDefaultLength)) + .Times(1); manager_.SetDebugDelegate(&debug_delegate); SendDataPacket(1); @@ -665,12 +566,10 @@ EXPECT_EQ(PACKETS_NEWLY_ACKED, manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1), ENCRYPTION_INITIAL)); - if (manager_.session_decides_what_to_write()) { - // Frames in packets 2 and 3 are acked. - EXPECT_CALL(notifier_, IsFrameOutstanding(_)) - .Times(2) - .WillRepeatedly(Return(false)); - } + // Frames in packets 2 and 3 are acked. + EXPECT_CALL(notifier_, IsFrameOutstanding(_)) + .Times(2) + .WillRepeatedly(Return(false)); // 2 and 3 remain unacked, but no packets have retransmittable data. uint64_t unacked[] = {2, 3}; @@ -680,12 +579,10 @@ // Ensure packet 2 is lost when 4 is sent and 3 and 4 are acked. SendDataPacket(4); - if (manager_.session_decides_what_to_write()) { - // No new data gets acked in packet 3. - EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)) - .WillOnce(Return(false)) - .WillRepeatedly(Return(true)); - } + // No new data gets acked in packet 3. + EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)) + .WillOnce(Return(false)) + .WillRepeatedly(Return(true)); uint64_t acked[] = {3, 4}; ExpectAcksAndLosses(true, acked, QUIC_ARRAYSIZE(acked), nullptr, 0); manager_.OnAckFrameStart(QuicPacketNumber(4), QuicTime::Delta::Infinite(), @@ -704,13 +601,11 @@ ExpectAckAndLoss(true, 5, 2); EXPECT_CALL(debug_delegate, OnPacketLoss(QuicPacketNumber(2), LOSS_RETRANSMISSION, _)); - if (manager_.session_decides_what_to_write()) { - // Frames in all packets are acked. - EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); - // Notify session that stream frame in packet 2 gets lost although it is - // not outstanding. - EXPECT_CALL(notifier_, OnFrameLost(_)).Times(1); - } + // Frames in all packets are acked. + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); + // Notify session that stream frame in packet 2 gets lost although it is + // not outstanding. + EXPECT_CALL(notifier_, OnFrameLost(_)).Times(1); manager_.OnAckFrameStart(QuicPacketNumber(5), QuicTime::Delta::Infinite(), clock_.Now()); manager_.OnAckRange(QuicPacketNumber(3), QuicPacketNumber(6)); @@ -719,23 +614,15 @@ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(3), ENCRYPTION_INITIAL)); - if (manager_.session_decides_what_to_write()) { - uint64_t unacked[] = {2}; - VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); - } else { - VerifyUnackedPackets(nullptr, 0); - } + uint64_t unacked3[] = {2}; + VerifyUnackedPackets(unacked3, QUIC_ARRAYSIZE(unacked3)); EXPECT_FALSE(manager_.HasInFlightPackets()); - if (manager_.session_decides_what_to_write()) { - // Spurious retransmission is detected when packet 3 gets acked. We cannot - // know packet 2 is a spurious until it gets acked. - EXPECT_EQ(1u, stats_.packets_spuriously_retransmitted); - } else { - EXPECT_EQ(2u, stats_.packets_spuriously_retransmitted); - } + // Spurious retransmission is detected when packet 3 gets acked. We cannot + // know packet 2 is a spurious until it gets acked. + EXPECT_EQ(1u, stats_.packets_spuriously_retransmitted); } -TEST_P(QuicSentPacketManagerTest, AckOriginalTransmission) { +TEST_F(QuicSentPacketManagerTest, AckOriginalTransmission) { auto loss_algorithm = std::make_unique<MockLossAlgorithm>(); QuicSentPacketManagerPeer::SetLossAlgorithm(&manager_, loss_algorithm.get()); @@ -776,8 +663,7 @@ uint64_t acked[] = {3}; ExpectAcksAndLosses(false, acked, QUIC_ARRAYSIZE(acked), nullptr, 0); EXPECT_CALL(*loss_algorithm, DetectLosses(_, _, _, _, _, _)); - if (GetQuicReloadableFlag(quic_detect_spurious_loss) && - manager_.session_decides_what_to_write()) { + if (GetQuicReloadableFlag(quic_detect_spurious_loss)) { EXPECT_CALL(*loss_algorithm, SpuriousLossDetected(_, _, _, QuicPacketNumber(3), QuicPacketNumber(4))); @@ -792,34 +678,32 @@ EXPECT_EQ(PACKETS_NEWLY_ACKED, manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(3), ENCRYPTION_INITIAL)); - if (manager_.session_decides_what_to_write()) { - // Ack 3 will not cause 5 be considered as a spurious retransmission. Ack - // 5 will cause 5 be considered as a spurious retransmission as no new - // data gets acked. - ExpectAck(5); - EXPECT_CALL(*loss_algorithm, DetectLosses(_, _, _, _, _, _)); - EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).WillOnce(Return(false)); - manager_.OnAckFrameStart(QuicPacketNumber(5), QuicTime::Delta::Infinite(), - clock_.Now()); - manager_.OnAckRange(QuicPacketNumber(3), QuicPacketNumber(6)); - manager_.OnAckRange(QuicPacketNumber(1), QuicPacketNumber(2)); - EXPECT_EQ(PACKETS_NEWLY_ACKED, - manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(4), - ENCRYPTION_INITIAL)); - } + // Ack 3 will not cause 5 be considered as a spurious retransmission. Ack + // 5 will cause 5 be considered as a spurious retransmission as no new + // data gets acked. + ExpectAck(5); + EXPECT_CALL(*loss_algorithm, DetectLosses(_, _, _, _, _, _)); + EXPECT_CALL(notifier_, OnFrameAcked(_, _, _)).WillOnce(Return(false)); + manager_.OnAckFrameStart(QuicPacketNumber(5), QuicTime::Delta::Infinite(), + clock_.Now()); + manager_.OnAckRange(QuicPacketNumber(3), QuicPacketNumber(6)); + manager_.OnAckRange(QuicPacketNumber(1), QuicPacketNumber(2)); + EXPECT_EQ(PACKETS_NEWLY_ACKED, + manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(4), + ENCRYPTION_INITIAL)); } } -TEST_P(QuicSentPacketManagerTest, GetLeastUnacked) { +TEST_F(QuicSentPacketManagerTest, GetLeastUnacked) { EXPECT_EQ(QuicPacketNumber(1u), manager_.GetLeastUnacked()); } -TEST_P(QuicSentPacketManagerTest, GetLeastUnackedUnacked) { +TEST_F(QuicSentPacketManagerTest, GetLeastUnackedUnacked) { SendDataPacket(1); EXPECT_EQ(QuicPacketNumber(1u), manager_.GetLeastUnacked()); } -TEST_P(QuicSentPacketManagerTest, AckAckAndUpdateRtt) { +TEST_F(QuicSentPacketManagerTest, AckAckAndUpdateRtt) { EXPECT_FALSE(manager_.largest_packet_peer_knows_is_acked().IsInitialized()); SendDataPacket(1); SendAckPacket(2, 1); @@ -850,7 +734,7 @@ manager_.largest_packet_peer_knows_is_acked()); } -TEST_P(QuicSentPacketManagerTest, Rtt) { +TEST_F(QuicSentPacketManagerTest, Rtt) { QuicTime::Delta expected_rtt = QuicTime::Delta::FromMilliseconds(20); SendDataPacket(1); clock_.AdvanceTime(expected_rtt); @@ -865,7 +749,7 @@ EXPECT_EQ(expected_rtt, manager_.GetRttStats()->latest_rtt()); } -TEST_P(QuicSentPacketManagerTest, RttWithInvalidDelta) { +TEST_F(QuicSentPacketManagerTest, RttWithInvalidDelta) { // Expect that the RTT is equal to the local time elapsed, since the // ack_delay_time is larger than the local time elapsed // and is hence invalid. @@ -883,7 +767,7 @@ EXPECT_EQ(expected_rtt, manager_.GetRttStats()->latest_rtt()); } -TEST_P(QuicSentPacketManagerTest, RttWithInfiniteDelta) { +TEST_F(QuicSentPacketManagerTest, RttWithInfiniteDelta) { // Expect that the RTT is equal to the local time elapsed, since the // ack_delay_time is infinite, and is hence invalid. QuicTime::Delta expected_rtt = QuicTime::Delta::FromMilliseconds(10); @@ -900,7 +784,7 @@ EXPECT_EQ(expected_rtt, manager_.GetRttStats()->latest_rtt()); } -TEST_P(QuicSentPacketManagerTest, RttZeroDelta) { +TEST_F(QuicSentPacketManagerTest, RttZeroDelta) { // Expect that the RTT is the time between send and receive since the // ack_delay_time is zero. QuicTime::Delta expected_rtt = QuicTime::Delta::FromMilliseconds(10); @@ -917,7 +801,7 @@ EXPECT_EQ(expected_rtt, manager_.GetRttStats()->latest_rtt()); } -TEST_P(QuicSentPacketManagerTest, TailLossProbeTimeout) { +TEST_F(QuicSentPacketManagerTest, TailLossProbeTimeout) { QuicSentPacketManagerPeer::SetMaxTailLossProbes(&manager_, 2); // Send 1 packet. @@ -926,36 +810,20 @@ // The first tail loss probe retransmits 1 packet. manager_.OnRetransmissionTimeout(); EXPECT_EQ(QuicTime::Delta::Zero(), manager_.TimeUntilSend(clock_.Now())); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(WithArgs<1>(Invoke( - [this](TransmissionType type) { RetransmitDataPacket(2, type); }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(2, type); }))); manager_.MaybeRetransmitTailLossProbe(); - if (!manager_.session_decides_what_to_write()) { - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - RetransmitNextPacket(2); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - } // The second tail loss probe retransmits 1 packet. manager_.OnRetransmissionTimeout(); EXPECT_EQ(QuicTime::Delta::Zero(), manager_.TimeUntilSend(clock_.Now())); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(WithArgs<1>(Invoke( - [this](TransmissionType type) { RetransmitDataPacket(3, type); }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(3, type); }))); manager_.MaybeRetransmitTailLossProbe(); - if (!manager_.session_decides_what_to_write()) { - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - RetransmitNextPacket(3); - } EXPECT_CALL(*send_algorithm_, CanSend(_)).WillOnce(Return(false)); EXPECT_EQ(QuicTime::Delta::Infinite(), manager_.TimeUntilSend(clock_.Now())); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); // Ack the third and ensure the first two are still pending. ExpectAck(3); @@ -976,13 +844,11 @@ uint64_t lost[] = {1, 2}; ExpectAcksAndLosses(true, acked, QUIC_ARRAYSIZE(acked), lost, QUIC_ARRAYSIZE(lost)); - if (manager_.session_decides_what_to_write()) { - // Frames in all packets are acked. - EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); - // Notify session that stream frame in packets 1 and 2 get lost although - // they are not outstanding. - EXPECT_CALL(notifier_, OnFrameLost(_)).Times(2); - } + // Frames in all packets are acked. + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); + // Notify session that stream frame in packets 1 and 2 get lost although + // they are not outstanding. + EXPECT_CALL(notifier_, OnFrameLost(_)).Times(2); manager_.OnAckFrameStart(QuicPacketNumber(5), QuicTime::Delta::Infinite(), clock_.Now()); manager_.OnAckRange(QuicPacketNumber(3), QuicPacketNumber(6)); @@ -990,13 +856,12 @@ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(2), ENCRYPTION_INITIAL)); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); EXPECT_FALSE(manager_.HasInFlightPackets()); EXPECT_EQ(2u, stats_.tlp_count); EXPECT_EQ(0u, stats_.rto_count); } -TEST_P(QuicSentPacketManagerTest, TailLossProbeThenRTO) { +TEST_F(QuicSentPacketManagerTest, TailLossProbeThenRTO) { QuicSentPacketManagerPeer::SetMaxTailLossProbes(&manager_, 2); // Send 100 packets. @@ -1011,38 +876,21 @@ // The first tail loss probe retransmits 1 packet. manager_.OnRetransmissionTimeout(); EXPECT_EQ(QuicTime::Delta::Zero(), manager_.TimeUntilSend(clock_.Now())); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(WithArgs<1>(Invoke([this](TransmissionType type) { - RetransmitDataPacket(101, type); - }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(101, type); }))); manager_.MaybeRetransmitTailLossProbe(); - if (!manager_.session_decides_what_to_write()) { - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - RetransmitNextPacket(101); - } EXPECT_CALL(*send_algorithm_, CanSend(_)).WillOnce(Return(false)); EXPECT_EQ(QuicTime::Delta::Infinite(), manager_.TimeUntilSend(clock_.Now())); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); clock_.AdvanceTime(manager_.GetRetransmissionTime() - clock_.Now()); // The second tail loss probe retransmits 1 packet. manager_.OnRetransmissionTimeout(); EXPECT_EQ(QuicTime::Delta::Zero(), manager_.TimeUntilSend(clock_.Now())); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(WithArgs<1>(Invoke([this](TransmissionType type) { - RetransmitDataPacket(102, type); - }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(102, type); }))); EXPECT_TRUE(manager_.MaybeRetransmitTailLossProbe()); - if (!manager_.session_decides_what_to_write()) { - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - RetransmitNextPacket(102); - } EXPECT_CALL(*send_algorithm_, CanSend(_)).WillOnce(Return(false)); EXPECT_EQ(QuicTime::Delta::Infinite(), manager_.TimeUntilSend(clock_.Now())); @@ -1054,43 +902,29 @@ // Advance the time enough to ensure all packets are RTO'd. clock_.AdvanceTime(QuicTime::Delta::FromMilliseconds(1000)); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .Times(2) - .WillOnce(WithArgs<1>(Invoke([this](TransmissionType type) { - RetransmitDataPacket(103, type); - }))) - .WillOnce(WithArgs<1>(Invoke([this](TransmissionType type) { - RetransmitDataPacket(104, type); - }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .Times(2) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(103, type); }))) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(104, type); }))); manager_.OnRetransmissionTimeout(); EXPECT_EQ(2u, stats_.tlp_count); EXPECT_EQ(1u, stats_.rto_count); - if (manager_.session_decides_what_to_write()) { - // There are 2 RTO retransmissions. - EXPECT_EQ(104 * kDefaultLength, manager_.GetBytesInFlight()); - } - if (!manager_.session_decides_what_to_write()) { - // Send and Ack the RTO and ensure OnRetransmissionTimeout is called. - EXPECT_EQ(102 * kDefaultLength, manager_.GetBytesInFlight()); - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - RetransmitNextPacket(103); - } + // There are 2 RTO retransmissions. + EXPECT_EQ(104 * kDefaultLength, manager_.GetBytesInFlight()); QuicPacketNumber largest_acked = QuicPacketNumber(103); EXPECT_CALL(*send_algorithm_, OnRetransmissionTimeout(true)); EXPECT_CALL(*send_algorithm_, OnCongestionEvent( true, _, _, Pointwise(PacketNumberEq(), {largest_acked}), _)); EXPECT_CALL(*network_change_visitor_, OnCongestionChange()); - if (manager_.session_decides_what_to_write()) { - // Although frames in packet 3 gets acked, it would be kept for another - // RTT. - EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(true)); - // Packets [1, 102] are lost, although stream frame in packet 3 is not - // outstanding. - EXPECT_CALL(notifier_, OnFrameLost(_)).Times(102); - } + // Although frames in packet 3 gets acked, it would be kept for another + // RTT. + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(true)); + // Packets [1, 102] are lost, although stream frame in packet 3 is not + // outstanding. + EXPECT_CALL(notifier_, OnFrameLost(_)).Times(102); manager_.OnAckFrameStart(QuicPacketNumber(103), QuicTime::Delta::Infinite(), clock_.Now()); manager_.OnAckRange(QuicPacketNumber(103), QuicPacketNumber(104)); @@ -1098,15 +932,11 @@ manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1), ENCRYPTION_INITIAL)); // All packets before 103 should be lost. - if (manager_.session_decides_what_to_write()) { - // Packet 104 is still in flight. - EXPECT_EQ(1000u, manager_.GetBytesInFlight()); - } else { - EXPECT_EQ(0u, manager_.GetBytesInFlight()); - } + // Packet 104 is still in flight. + EXPECT_EQ(1000u, manager_.GetBytesInFlight()); } -TEST_P(QuicSentPacketManagerTest, CryptoHandshakeTimeout) { +TEST_F(QuicSentPacketManagerTest, CryptoHandshakeTimeout) { // Send 2 crypto packets and 3 data packets. const size_t kNumSentCryptoPackets = 2; for (size_t i = 1; i <= kNumSentCryptoPackets; ++i) { @@ -1120,37 +950,21 @@ EXPECT_EQ(5 * kDefaultLength, manager_.GetBytesInFlight()); // The first retransmits 2 packets. - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .Times(2) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(6); })) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(7); })); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .Times(2) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(6); })) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(7); })); manager_.OnRetransmissionTimeout(); - if (!manager_.session_decides_what_to_write()) { - EXPECT_EQ(QuicTime::Delta::Zero(), manager_.TimeUntilSend(clock_.Now())); - RetransmitNextPacket(6); - RetransmitNextPacket(7); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - } // Expect all 4 handshake packets to be in flight and 3 data packets. EXPECT_EQ(7 * kDefaultLength, manager_.GetBytesInFlight()); EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); // The second retransmits 2 packets. - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .Times(2) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(8); })) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(9); })); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .Times(2) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(8); })) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(9); })); manager_.OnRetransmissionTimeout(); - if (!manager_.session_decides_what_to_write()) { - EXPECT_EQ(QuicTime::Delta::Zero(), manager_.TimeUntilSend(clock_.Now())); - RetransmitNextPacket(8); - RetransmitNextPacket(9); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - } EXPECT_EQ(9 * kDefaultLength, manager_.GetBytesInFlight()); EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); @@ -1161,13 +975,8 @@ uint64_t lost[] = {1, 2, 6}; ExpectAcksAndLosses(true, acked, QUIC_ARRAYSIZE(acked), lost, QUIC_ARRAYSIZE(lost)); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, OnFrameLost(_)).Times(3); - } - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, HasUnackedCryptoData()) - .WillRepeatedly(Return(false)); - } + EXPECT_CALL(notifier_, OnFrameLost(_)).Times(3); + EXPECT_CALL(notifier_, HasUnackedCryptoData()).WillRepeatedly(Return(false)); manager_.OnAckFrameStart(QuicPacketNumber(9), QuicTime::Delta::Infinite(), clock_.Now()); manager_.OnAckRange(QuicPacketNumber(8), QuicPacketNumber(10)); @@ -1179,7 +988,7 @@ EXPECT_FALSE(manager_.HasUnackedCryptoPackets()); } -TEST_P(QuicSentPacketManagerTest, CryptoHandshakeTimeoutVersionNegotiation) { +TEST_F(QuicSentPacketManagerTest, CryptoHandshakeTimeoutVersionNegotiation) { // Send 2 crypto packets and 3 data packets. const size_t kNumSentCryptoPackets = 2; for (size_t i = 1; i <= kNumSentCryptoPackets; ++i) { @@ -1191,51 +1000,26 @@ } EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .Times(2) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(6); })) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(7); })); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .Times(2) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(6); })) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(7); })); manager_.OnRetransmissionTimeout(); - if (!manager_.session_decides_what_to_write()) { - RetransmitNextPacket(6); - RetransmitNextPacket(7); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - } EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); // Now act like a version negotiation packet arrived, which would cause all // unacked packets to be retransmitted. - if (manager_.session_decides_what_to_write()) { - // Mark packets [1, 7] lost. And the frames in 6 and 7 are same as packets 1 - // and 2, respectively. - EXPECT_CALL(notifier_, OnFrameLost(_)).Times(7); - } + // Mark packets [1, 7] lost. And the frames in 6 and 7 are same as packets 1 + // and 2, respectively. + EXPECT_CALL(notifier_, OnFrameLost(_)).Times(7); manager_.RetransmitUnackedPackets(ALL_UNACKED_RETRANSMISSION); // Ensure the first two pending packets are the crypto retransmits. - if (manager_.session_decides_what_to_write()) { - RetransmitCryptoPacket(8); - RetransmitCryptoPacket(9); - RetransmitDataPacket(10, ALL_UNACKED_RETRANSMISSION); - RetransmitDataPacket(11, ALL_UNACKED_RETRANSMISSION); - RetransmitDataPacket(12, ALL_UNACKED_RETRANSMISSION); - } else { - ASSERT_TRUE(manager_.HasPendingRetransmissions()); - EXPECT_EQ(QuicPacketNumber(6u), - manager_.NextPendingRetransmission().packet_number); - RetransmitNextPacket(8); - EXPECT_EQ(QuicPacketNumber(7u), - manager_.NextPendingRetransmission().packet_number); - RetransmitNextPacket(9); - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - // Send 3 more data packets and ensure the least unacked is raised. - RetransmitNextPacket(10); - RetransmitNextPacket(11); - RetransmitNextPacket(12); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - } + RetransmitCryptoPacket(8); + RetransmitCryptoPacket(9); + RetransmitDataPacket(10, ALL_UNACKED_RETRANSMISSION); + RetransmitDataPacket(11, ALL_UNACKED_RETRANSMISSION); + RetransmitDataPacket(12, ALL_UNACKED_RETRANSMISSION); EXPECT_EQ(QuicPacketNumber(1u), manager_.GetLeastUnacked()); // Least unacked isn't raised until an ack is received, so ack the @@ -1248,47 +1032,31 @@ EXPECT_EQ(PACKETS_NEWLY_ACKED, manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1), ENCRYPTION_INITIAL)); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, HasUnackedCryptoData()) - .WillRepeatedly(Return(false)); - } + EXPECT_CALL(notifier_, HasUnackedCryptoData()).WillRepeatedly(Return(false)); EXPECT_EQ(QuicPacketNumber(10u), manager_.GetLeastUnacked()); } -TEST_P(QuicSentPacketManagerTest, CryptoHandshakeSpuriousRetransmission) { +TEST_F(QuicSentPacketManagerTest, CryptoHandshakeSpuriousRetransmission) { // Send 1 crypto packet. SendCryptoPacket(1); EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); // Retransmit the crypto packet as 2. - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(2); })); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(2); })); manager_.OnRetransmissionTimeout(); - if (!manager_.session_decides_what_to_write()) { - RetransmitNextPacket(2); - } // Retransmit the crypto packet as 3. - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(3); })); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(3); })); manager_.OnRetransmissionTimeout(); - if (!manager_.session_decides_what_to_write()) { - RetransmitNextPacket(3); - } // Now ack the second crypto packet, and ensure the first gets removed, but // the third does not. uint64_t acked[] = {2}; ExpectAcksAndLosses(true, acked, QUIC_ARRAYSIZE(acked), nullptr, 0); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, HasUnackedCryptoData()) - .WillRepeatedly(Return(false)); - EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); - } + EXPECT_CALL(notifier_, HasUnackedCryptoData()).WillRepeatedly(Return(false)); + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); manager_.OnAckFrameStart(QuicPacketNumber(2), QuicTime::Delta::Infinite(), clock_.Now()); manager_.OnAckRange(QuicPacketNumber(2), QuicPacketNumber(3)); @@ -1301,7 +1069,7 @@ VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); } -TEST_P(QuicSentPacketManagerTest, CryptoHandshakeTimeoutUnsentDataPacket) { +TEST_F(QuicSentPacketManagerTest, CryptoHandshakeTimeoutUnsentDataPacket) { // Send 2 crypto packets and 1 data packet. const size_t kNumSentCryptoPackets = 2; for (size_t i = 1; i <= kNumSentCryptoPackets; ++i) { @@ -1311,22 +1079,15 @@ EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); // Retransmit 2 crypto packets, but not the serialized packet. - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .Times(2) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(4); })) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(5); })); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .Times(2) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(4); })) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(5); })); manager_.OnRetransmissionTimeout(); - if (!manager_.session_decides_what_to_write()) { - RetransmitNextPacket(4); - RetransmitNextPacket(5); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - } EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); } -TEST_P(QuicSentPacketManagerTest, +TEST_F(QuicSentPacketManagerTest, CryptoHandshakeRetransmissionThenRetransmitAll) { // Send 1 crypto packet. SendCryptoPacket(1); @@ -1334,36 +1095,21 @@ EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); // Retransmit the crypto packet as 2. - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(2); })); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(2); })); manager_.OnRetransmissionTimeout(); - if (!manager_.session_decides_what_to_write()) { - RetransmitNextPacket(2); - } // Now retransmit all the unacked packets, which occurs when there is a // version negotiation. - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, OnFrameLost(_)).Times(2); - } + EXPECT_CALL(notifier_, OnFrameLost(_)).Times(2); manager_.RetransmitUnackedPackets(ALL_UNACKED_RETRANSMISSION); - if (manager_.session_decides_what_to_write()) { - // Both packets 1 and 2 are unackable. - EXPECT_FALSE(manager_.unacked_packets().IsUnacked(QuicPacketNumber(1))); - EXPECT_FALSE(manager_.unacked_packets().IsUnacked(QuicPacketNumber(2))); - } else { - // Packet 2 is useful because it does not get retransmitted and still has - // retransmittable frames. - uint64_t unacked[] = {1, 2}; - VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - } + // Both packets 1 and 2 are unackable. + EXPECT_FALSE(manager_.unacked_packets().IsUnacked(QuicPacketNumber(1))); + EXPECT_FALSE(manager_.unacked_packets().IsUnacked(QuicPacketNumber(2))); EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); EXPECT_FALSE(manager_.HasInFlightPackets()); } -TEST_P(QuicSentPacketManagerTest, +TEST_F(QuicSentPacketManagerTest, CryptoHandshakeRetransmissionThenNeuterAndAck) { // Send 1 crypto packet. SendCryptoPacket(1); @@ -1371,40 +1117,26 @@ EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); // Retransmit the crypto packet as 2. - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(2); })); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(2); })); manager_.OnRetransmissionTimeout(); - if (!manager_.session_decides_what_to_write()) { - RetransmitNextPacket(2); - } EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); // Retransmit the crypto packet as 3. - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(3); })); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(3); })); manager_.OnRetransmissionTimeout(); - if (!manager_.session_decides_what_to_write()) { - RetransmitNextPacket(3); - } EXPECT_TRUE(manager_.HasUnackedCryptoPackets()); // Now neuter all unacked unencrypted packets, which occurs when the // connection goes forward secure. manager_.NeuterUnencryptedPackets(); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, HasUnackedCryptoData()) - .WillRepeatedly(Return(false)); - EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); - } + EXPECT_CALL(notifier_, HasUnackedCryptoData()).WillRepeatedly(Return(false)); + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); EXPECT_FALSE(manager_.HasUnackedCryptoPackets()); uint64_t unacked[] = {1, 2, 3}; VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); VerifyRetransmittablePackets(nullptr, 0); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); EXPECT_FALSE(manager_.HasUnackedCryptoPackets()); EXPECT_FALSE(manager_.HasInFlightPackets()); @@ -1421,7 +1153,7 @@ VerifyRetransmittablePackets(nullptr, 0); } -TEST_P(QuicSentPacketManagerTest, RetransmissionTimeout) { +TEST_F(QuicSentPacketManagerTest, RetransmissionTimeout) { StrictMock<MockDebugDelegate> debug_delegate; manager_.SetDebugDelegate(&debug_delegate); @@ -1432,27 +1164,14 @@ } EXPECT_FALSE(manager_.MaybeRetransmitTailLossProbe()); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .Times(2) - .WillOnce(WithArgs<1>(Invoke([this](TransmissionType type) { - RetransmitDataPacket(101, type); - }))) - .WillOnce(WithArgs<1>(Invoke([this](TransmissionType type) { - RetransmitDataPacket(102, type); - }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .Times(2) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(101, type); }))) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(102, type); }))); manager_.OnRetransmissionTimeout(); - if (manager_.session_decides_what_to_write()) { - EXPECT_EQ(102 * kDefaultLength, manager_.GetBytesInFlight()); - } else { - ASSERT_TRUE(manager_.HasPendingRetransmissions()); - EXPECT_EQ(100 * kDefaultLength, manager_.GetBytesInFlight()); - RetransmitNextPacket(101); - ASSERT_TRUE(manager_.HasPendingRetransmissions()); - RetransmitNextPacket(102); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - } + EXPECT_EQ(102 * kDefaultLength, manager_.GetBytesInFlight()); // Ack a retransmission. // Ensure no packets are lost. @@ -1469,12 +1188,10 @@ EXPECT_CALL(debug_delegate, OnPacketLoss(QuicPacketNumber(i), LOSS_RETRANSMISSION, _)); } - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(true)); - // Packets [1, 99] are considered as lost, although stream frame in packet - // 2 is not outstanding. - EXPECT_CALL(notifier_, OnFrameLost(_)).Times(99); - } + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(true)); + // Packets [1, 99] are considered as lost, although stream frame in packet + // 2 is not outstanding. + EXPECT_CALL(notifier_, OnFrameLost(_)).Times(99); manager_.OnAckFrameStart(QuicPacketNumber(102), QuicTime::Delta::Zero(), clock_.Now()); manager_.OnAckRange(QuicPacketNumber(102), QuicPacketNumber(103)); @@ -1483,7 +1200,7 @@ ENCRYPTION_INITIAL)); } -TEST_P(QuicSentPacketManagerTest, RetransmissionTimeoutOnePacket) { +TEST_F(QuicSentPacketManagerTest, RetransmissionTimeoutOnePacket) { // Set the 1RTO connection option. QuicConfig client_config; QuicTagVector options; @@ -1509,25 +1226,15 @@ } EXPECT_FALSE(manager_.MaybeRetransmitTailLossProbe()); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .Times(1) - .WillOnce(WithArgs<1>(Invoke([this](TransmissionType type) { - RetransmitDataPacket(101, type); - }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .Times(1) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(101, type); }))); manager_.OnRetransmissionTimeout(); - if (manager_.session_decides_what_to_write()) { - EXPECT_EQ(101 * kDefaultLength, manager_.GetBytesInFlight()); - } else { - ASSERT_TRUE(manager_.HasPendingRetransmissions()); - EXPECT_EQ(100 * kDefaultLength, manager_.GetBytesInFlight()); - RetransmitNextPacket(101); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - } + EXPECT_EQ(101 * kDefaultLength, manager_.GetBytesInFlight()); } -TEST_P(QuicSentPacketManagerTest, NewRetransmissionTimeout) { +TEST_F(QuicSentPacketManagerTest, NewRetransmissionTimeout) { QuicConfig client_config; QuicTagVector options; options.push_back(kNRTO); @@ -1550,26 +1257,14 @@ } EXPECT_FALSE(manager_.MaybeRetransmitTailLossProbe()); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .Times(2) - .WillOnce(WithArgs<1>(Invoke([this](TransmissionType type) { - RetransmitDataPacket(101, type); - }))) - .WillOnce(WithArgs<1>(Invoke([this](TransmissionType type) { - RetransmitDataPacket(102, type); - }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .Times(2) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(101, type); }))) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(102, type); }))); manager_.OnRetransmissionTimeout(); - if (manager_.session_decides_what_to_write()) { - EXPECT_EQ(102 * kDefaultLength, manager_.GetBytesInFlight()); - } else { - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - EXPECT_EQ(100 * kDefaultLength, manager_.GetBytesInFlight()); - RetransmitNextPacket(101); - RetransmitNextPacket(102); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - } + EXPECT_EQ(102 * kDefaultLength, manager_.GetBytesInFlight()); // Ack a retransmission and expect no call to OnRetransmissionTimeout. // This will include packets in the lost packet map. @@ -1579,12 +1274,10 @@ Pointwise(PacketNumberEq(), {largest_acked}), /*lost_packets=*/Not(IsEmpty()))); EXPECT_CALL(*network_change_visitor_, OnCongestionChange()); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(true)); - // Packets [1, 99] are considered as lost, although stream frame in packet - // 2 is not outstanding. - EXPECT_CALL(notifier_, OnFrameLost(_)).Times(99); - } + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(true)); + // Packets [1, 99] are considered as lost, although stream frame in packet + // 2 is not outstanding. + EXPECT_CALL(notifier_, OnFrameLost(_)).Times(99); manager_.OnAckFrameStart(QuicPacketNumber(102), QuicTime::Delta::Zero(), clock_.Now()); manager_.OnAckRange(QuicPacketNumber(102), QuicPacketNumber(103)); @@ -1593,40 +1286,22 @@ ENCRYPTION_INITIAL)); } -TEST_P(QuicSentPacketManagerTest, TwoRetransmissionTimeoutsAckSecond) { +TEST_F(QuicSentPacketManagerTest, TwoRetransmissionTimeoutsAckSecond) { // Send 1 packet. SendDataPacket(1); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(WithArgs<1>(Invoke( - [this](TransmissionType type) { RetransmitDataPacket(2, type); }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(2, type); }))); manager_.OnRetransmissionTimeout(); - if (manager_.session_decides_what_to_write()) { - EXPECT_EQ(2 * kDefaultLength, manager_.GetBytesInFlight()); - } else { - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - EXPECT_EQ(kDefaultLength, manager_.GetBytesInFlight()); - RetransmitNextPacket(2); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - } + EXPECT_EQ(2 * kDefaultLength, manager_.GetBytesInFlight()); // Rto a second time. - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(WithArgs<1>(Invoke( - [this](TransmissionType type) { RetransmitDataPacket(3, type); }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(3, type); }))); manager_.OnRetransmissionTimeout(); - if (manager_.session_decides_what_to_write()) { - EXPECT_EQ(3 * kDefaultLength, manager_.GetBytesInFlight()); - } else { - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - EXPECT_EQ(2 * kDefaultLength, manager_.GetBytesInFlight()); - RetransmitNextPacket(3); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - } + EXPECT_EQ(3 * kDefaultLength, manager_.GetBytesInFlight()); // Ack a retransmission and ensure OnRetransmissionTimeout is called. EXPECT_CALL(*send_algorithm_, OnRetransmissionTimeout(true)); @@ -1642,40 +1317,22 @@ EXPECT_EQ(2 * kDefaultLength, manager_.GetBytesInFlight()); } -TEST_P(QuicSentPacketManagerTest, TwoRetransmissionTimeoutsAckFirst) { +TEST_F(QuicSentPacketManagerTest, TwoRetransmissionTimeoutsAckFirst) { // Send 1 packet. SendDataPacket(1); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(WithArgs<1>(Invoke( - [this](TransmissionType type) { RetransmitDataPacket(2, type); }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(2, type); }))); manager_.OnRetransmissionTimeout(); - if (manager_.session_decides_what_to_write()) { - EXPECT_EQ(2 * kDefaultLength, manager_.GetBytesInFlight()); - } else { - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - EXPECT_EQ(kDefaultLength, manager_.GetBytesInFlight()); - RetransmitNextPacket(2); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - } + EXPECT_EQ(2 * kDefaultLength, manager_.GetBytesInFlight()); // Rto a second time. - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(WithArgs<1>(Invoke( - [this](TransmissionType type) { RetransmitDataPacket(3, type); }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(3, type); }))); manager_.OnRetransmissionTimeout(); - if (manager_.session_decides_what_to_write()) { - EXPECT_EQ(3 * kDefaultLength, manager_.GetBytesInFlight()); - } else { - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - EXPECT_EQ(2 * kDefaultLength, manager_.GetBytesInFlight()); - RetransmitNextPacket(3); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - } + EXPECT_EQ(3 * kDefaultLength, manager_.GetBytesInFlight()); // Ack a retransmission and ensure OnRetransmissionTimeout is called. EXPECT_CALL(*send_algorithm_, OnRetransmissionTimeout(true)); @@ -1691,11 +1348,11 @@ EXPECT_EQ(2 * kDefaultLength, manager_.GetBytesInFlight()); } -TEST_P(QuicSentPacketManagerTest, GetTransmissionTime) { +TEST_F(QuicSentPacketManagerTest, GetTransmissionTime) { EXPECT_EQ(QuicTime::Zero(), manager_.GetRetransmissionTime()); } -TEST_P(QuicSentPacketManagerTest, GetTransmissionTimeCryptoHandshake) { +TEST_F(QuicSentPacketManagerTest, GetTransmissionTimeCryptoHandshake) { QuicTime crypto_packet_send_time = clock_.Now(); SendCryptoPacket(1); @@ -1714,16 +1371,11 @@ // Retransmit the packet by invoking the retransmission timeout. clock_.AdvanceTime(1.5 * srtt); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(2); })); - // When session decides what to write, crypto_packet_send_time gets updated. - crypto_packet_send_time = clock_.Now(); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(2); })); + // When session decides what to write, crypto_packet_send_time gets updated. + crypto_packet_send_time = clock_.Now(); manager_.OnRetransmissionTimeout(); - if (!manager_.session_decides_what_to_write()) { - RetransmitNextPacket(2); - } // The retransmission time should now be twice as far in the future. expected_time = crypto_packet_send_time + srtt * 2 * 1.5; @@ -1731,23 +1383,18 @@ // Retransmit the packet for the 2nd time. clock_.AdvanceTime(2 * 1.5 * srtt); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(3); })); - // When session decides what to write, crypto_packet_send_time gets updated. - crypto_packet_send_time = clock_.Now(); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(3); })); + // When session decides what to write, crypto_packet_send_time gets updated. + crypto_packet_send_time = clock_.Now(); manager_.OnRetransmissionTimeout(); - if (!manager_.session_decides_what_to_write()) { - RetransmitNextPacket(3); - } // Verify exponential backoff of the retransmission timeout. expected_time = crypto_packet_send_time + srtt * 4 * 1.5; EXPECT_EQ(expected_time, manager_.GetRetransmissionTime()); } -TEST_P(QuicSentPacketManagerTest, +TEST_F(QuicSentPacketManagerTest, GetConservativeTransmissionTimeCryptoHandshake) { QuicConfig config; QuicTagVector options; @@ -1780,22 +1427,17 @@ // Retransmit the packet by invoking the retransmission timeout. clock_.AdvanceTime(2 * srtt); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(2); })); - crypto_packet_send_time = clock_.Now(); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(InvokeWithoutArgs([this]() { RetransmitCryptoPacket(2); })); + crypto_packet_send_time = clock_.Now(); manager_.OnRetransmissionTimeout(); - if (!manager_.session_decides_what_to_write()) { - RetransmitNextPacket(2); - } // The retransmission time should now be twice as far in the future. expected_time = crypto_packet_send_time + srtt * 2 * 2; EXPECT_EQ(expected_time, manager_.GetRetransmissionTime()); } -TEST_P(QuicSentPacketManagerTest, GetTransmissionTimeTailLossProbe) { +TEST_F(QuicSentPacketManagerTest, GetTransmissionTimeTailLossProbe) { QuicSentPacketManagerPeer::SetMaxTailLossProbes(&manager_, 2); SendDataPacket(1); SendDataPacket(2); @@ -1817,30 +1459,18 @@ clock_.AdvanceTime(expected_tlp_delay); manager_.OnRetransmissionTimeout(); EXPECT_EQ(QuicTime::Delta::Zero(), manager_.TimeUntilSend(clock_.Now())); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(WithArgs<1>(Invoke( - [this](TransmissionType type) { RetransmitDataPacket(3, type); }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(3, type); }))); EXPECT_TRUE(manager_.MaybeRetransmitTailLossProbe()); - if (!manager_.session_decides_what_to_write()) { - EXPECT_TRUE(manager_.HasPendingRetransmissions()); - RetransmitNextPacket(3); - } EXPECT_CALL(*send_algorithm_, CanSend(_)).WillOnce(Return(false)); EXPECT_EQ(QuicTime::Delta::Infinite(), manager_.TimeUntilSend(clock_.Now())); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); expected_time = clock_.Now() + expected_tlp_delay; EXPECT_EQ(expected_time, manager_.GetRetransmissionTime()); } -TEST_P(QuicSentPacketManagerTest, TLPRWithPendingStreamData) { - if (!manager_.session_decides_what_to_write()) { - return; - } - +TEST_F(QuicSentPacketManagerTest, TLPRWithPendingStreamData) { QuicConfig config; QuicTagVector options; @@ -1876,7 +1506,6 @@ clock_.AdvanceTime(expected_tlp_delay); manager_.OnRetransmissionTimeout(); EXPECT_EQ(QuicTime::Delta::Zero(), manager_.TimeUntilSend(clock_.Now())); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); EXPECT_CALL(notifier_, RetransmitFrames(_, _)) .WillOnce(WithArgs<1>(Invoke( [this](TransmissionType type) { RetransmitDataPacket(3, type); }))); @@ -1884,7 +1513,6 @@ EXPECT_CALL(*send_algorithm_, CanSend(_)).WillOnce(Return(false)); EXPECT_EQ(QuicTime::Delta::Infinite(), manager_.TimeUntilSend(clock_.Now())); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); // 2nd TLP. expected_tlp_delay = 2 * srtt; @@ -1892,11 +1520,7 @@ manager_.GetRetransmissionTime() - clock_.Now()); } -TEST_P(QuicSentPacketManagerTest, TLPRWithoutPendingStreamData) { - if (!manager_.session_decides_what_to_write()) { - return; - } - +TEST_F(QuicSentPacketManagerTest, TLPRWithoutPendingStreamData) { QuicConfig config; QuicTagVector options; @@ -1931,14 +1555,12 @@ clock_.AdvanceTime(expected_tlp_delay); manager_.OnRetransmissionTimeout(); EXPECT_EQ(QuicTime::Delta::Zero(), manager_.TimeUntilSend(clock_.Now())); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); EXPECT_CALL(notifier_, RetransmitFrames(_, _)) .WillOnce(WithArgs<1>(Invoke( [this](TransmissionType type) { RetransmitDataPacket(3, type); }))); EXPECT_TRUE(manager_.MaybeRetransmitTailLossProbe()); EXPECT_CALL(*send_algorithm_, CanSend(_)).WillOnce(Return(false)); EXPECT_EQ(QuicTime::Delta::Infinite(), manager_.TimeUntilSend(clock_.Now())); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); // 2nd TLP. expected_tlp_delay = 2 * srtt; @@ -1946,7 +1568,7 @@ manager_.GetRetransmissionTime() - clock_.Now()); } -TEST_P(QuicSentPacketManagerTest, GetTransmissionTimeSpuriousRTO) { +TEST_F(QuicSentPacketManagerTest, GetTransmissionTimeSpuriousRTO) { RttStats* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats()); rtt_stats->UpdateRtt(QuicTime::Delta::FromMilliseconds(100), QuicTime::Delta::Zero(), QuicTime::Zero()); @@ -1963,24 +1585,15 @@ // Retransmit the packet by invoking the retransmission timeout. clock_.AdvanceTime(expected_rto_delay); - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .Times(2) - .WillOnce(WithArgs<1>(Invoke( - [this](TransmissionType type) { RetransmitDataPacket(5, type); }))) - .WillOnce(WithArgs<1>(Invoke( - [this](TransmissionType type) { RetransmitDataPacket(6, type); }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .Times(2) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(5, type); }))) + .WillOnce(WithArgs<1>(Invoke( + [this](TransmissionType type) { RetransmitDataPacket(6, type); }))); manager_.OnRetransmissionTimeout(); - if (!manager_.session_decides_what_to_write()) { - // All packets are still considered inflight. - EXPECT_EQ(4 * kDefaultLength, manager_.GetBytesInFlight()); - RetransmitNextPacket(5); - RetransmitNextPacket(6); - } // All previous packets are inflight, plus two rto retransmissions. EXPECT_EQ(6 * kDefaultLength, manager_.GetBytesInFlight()); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); // The delay should double the second time. expected_time = clock_.Now() + expected_rto_delay + expected_rto_delay; @@ -1997,7 +1610,6 @@ EXPECT_EQ(PACKETS_NEWLY_ACKED, manager_.OnAckFrameEnd(clock_.Now(), QuicPacketNumber(1), ENCRYPTION_INITIAL)); - EXPECT_FALSE(manager_.HasPendingRetransmissions()); EXPECT_EQ(5 * kDefaultLength, manager_.GetBytesInFlight()); // Wait 2RTTs from now for the RTO, since it's the max of the RTO time @@ -2010,7 +1622,7 @@ EXPECT_EQ(expected_time, manager_.GetRetransmissionTime()); } -TEST_P(QuicSentPacketManagerTest, GetTransmissionDelayMin) { +TEST_F(QuicSentPacketManagerTest, GetTransmissionDelayMin) { SendDataPacket(1); // Provide a 1ms RTT sample. const_cast<RttStats*>(manager_.GetRttStats()) @@ -2026,20 +1638,15 @@ EXPECT_EQ(delay, QuicSentPacketManagerPeer::GetRetransmissionDelay(&manager_, i)); delay = delay + delay; - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(WithArgs<1>(Invoke([this, i](TransmissionType type) { - RetransmitDataPacket(i + 2, type); - }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke([this, i](TransmissionType type) { + RetransmitDataPacket(i + 2, type); + }))); manager_.OnRetransmissionTimeout(); - if (!manager_.session_decides_what_to_write()) { - RetransmitNextPacket(i + 2); - } } } -TEST_P(QuicSentPacketManagerTest, GetTransmissionDelayMax) { +TEST_F(QuicSentPacketManagerTest, GetTransmissionDelayMax) { SendDataPacket(1); // Provide a 60s RTT sample. const_cast<RttStats*>(manager_.GetRttStats()) @@ -2052,7 +1659,7 @@ QuicSentPacketManagerPeer::GetRetransmissionDelay(&manager_, 0)); } -TEST_P(QuicSentPacketManagerTest, GetTransmissionDelayExponentialBackoff) { +TEST_F(QuicSentPacketManagerTest, GetTransmissionDelayExponentialBackoff) { SendDataPacket(1); QuicTime::Delta delay = QuicTime::Delta::FromMilliseconds(500); @@ -2063,20 +1670,15 @@ EXPECT_EQ(delay, QuicSentPacketManagerPeer::GetRetransmissionDelay(&manager_, i)); delay = delay + delay; - if (manager_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, RetransmitFrames(_, _)) - .WillOnce(WithArgs<1>(Invoke([this, i](TransmissionType type) { - RetransmitDataPacket(i + 2, type); - }))); - } + EXPECT_CALL(notifier_, RetransmitFrames(_, _)) + .WillOnce(WithArgs<1>(Invoke([this, i](TransmissionType type) { + RetransmitDataPacket(i + 2, type); + }))); manager_.OnRetransmissionTimeout(); - if (!manager_.session_decides_what_to_write()) { - RetransmitNextPacket(i + 2); - } } } -TEST_P(QuicSentPacketManagerTest, RetransmissionDelay) { +TEST_F(QuicSentPacketManagerTest, RetransmissionDelay) { RttStats* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats()); const int64_t kRttMs = 250; const int64_t kDeviationMs = 5; @@ -2114,7 +1716,7 @@ QuicSentPacketManagerPeer::GetRetransmissionDelay(&manager_)); } -TEST_P(QuicSentPacketManagerTest, GetLossDelay) { +TEST_F(QuicSentPacketManagerTest, GetLossDelay) { auto loss_algorithm = std::make_unique<MockLossAlgorithm>(); QuicSentPacketManagerPeer::SetLossAlgorithm(&manager_, loss_algorithm.get()); @@ -2145,7 +1747,7 @@ manager_.OnRetransmissionTimeout(); } -TEST_P(QuicSentPacketManagerTest, NegotiateTimeLossDetectionFromOptions) { +TEST_F(QuicSentPacketManagerTest, NegotiateTimeLossDetectionFromOptions) { EXPECT_EQ(kNack, QuicSentPacketManagerPeer::GetLossAlgorithm(&manager_) ->GetLossDetectionType()); @@ -2161,7 +1763,7 @@ ->GetLossDetectionType()); } -TEST_P(QuicSentPacketManagerTest, NegotiateIetfLossDetectionFromOptions) { +TEST_F(QuicSentPacketManagerTest, NegotiateIetfLossDetectionFromOptions) { SetQuicReloadableFlag(quic_enable_ietf_loss_detection, true); EXPECT_EQ(kNack, QuicSentPacketManagerPeer::GetLossAlgorithm(&manager_) ->GetLossDetectionType()); @@ -2182,7 +1784,7 @@ QuicSentPacketManagerPeer::AdaptiveReorderingThresholdEnabled(&manager_)); } -TEST_P(QuicSentPacketManagerTest, +TEST_F(QuicSentPacketManagerTest, NegotiateIetfLossDetectionOneFourthRttFromOptions) { SetQuicReloadableFlag(quic_enable_ietf_loss_detection, true); EXPECT_EQ(kNack, QuicSentPacketManagerPeer::GetLossAlgorithm(&manager_) @@ -2205,7 +1807,7 @@ QuicSentPacketManagerPeer::AdaptiveReorderingThresholdEnabled(&manager_)); } -TEST_P(QuicSentPacketManagerTest, +TEST_F(QuicSentPacketManagerTest, NegotiateIetfLossDetectionAdaptiveReorderingThreshold) { SetQuicReloadableFlag(quic_enable_ietf_loss_detection, true); SetQuicReloadableFlag(quic_detect_spurious_loss, true); @@ -2230,7 +1832,7 @@ QuicSentPacketManagerPeer::AdaptiveReorderingThresholdEnabled(&manager_)); } -TEST_P(QuicSentPacketManagerTest, +TEST_F(QuicSentPacketManagerTest, NegotiateIetfLossDetectionAdaptiveReorderingThreshold2) { SetQuicReloadableFlag(quic_enable_ietf_loss_detection, true); SetQuicReloadableFlag(quic_detect_spurious_loss, true); @@ -2256,7 +1858,7 @@ QuicSentPacketManagerPeer::AdaptiveReorderingThresholdEnabled(&manager_)); } -TEST_P(QuicSentPacketManagerTest, NegotiateCongestionControlFromOptions) { +TEST_F(QuicSentPacketManagerTest, NegotiateCongestionControlFromOptions) { QuicConfig config; QuicTagVector options; @@ -2292,7 +1894,7 @@ ->GetCongestionControlType()); } -TEST_P(QuicSentPacketManagerTest, NegotiateClientCongestionControlFromOptions) { +TEST_F(QuicSentPacketManagerTest, NegotiateClientCongestionControlFromOptions) { QuicConfig config; QuicTagVector options; @@ -2339,7 +1941,7 @@ ->GetCongestionControlType()); } -TEST_P(QuicSentPacketManagerTest, NegotiateNoMinTLPFromOptionsAtServer) { +TEST_F(QuicSentPacketManagerTest, NegotiateNoMinTLPFromOptionsAtServer) { QuicConfig config; QuicTagVector options; @@ -2374,7 +1976,7 @@ QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_, 0)); } -TEST_P(QuicSentPacketManagerTest, NegotiateNoMinTLPFromOptionsAtClient) { +TEST_F(QuicSentPacketManagerTest, NegotiateNoMinTLPFromOptionsAtClient) { QuicConfig client_config; QuicTagVector options; @@ -2409,7 +2011,7 @@ QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_, 0)); } -TEST_P(QuicSentPacketManagerTest, NegotiateIETFTLPFromOptionsAtServer) { +TEST_F(QuicSentPacketManagerTest, NegotiateIETFTLPFromOptionsAtServer) { if (GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)) { return; } @@ -2440,7 +2042,7 @@ QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_, 0)); } -TEST_P(QuicSentPacketManagerTest, NegotiateIETFTLPFromOptionsAtClient) { +TEST_F(QuicSentPacketManagerTest, NegotiateIETFTLPFromOptionsAtClient) { if (GetQuicReloadableFlag(quic_sent_packet_manager_cleanup)) { return; } @@ -2472,7 +2074,7 @@ QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_, 0)); } -TEST_P(QuicSentPacketManagerTest, NegotiateNoMinRTOFromOptionsAtServer) { +TEST_F(QuicSentPacketManagerTest, NegotiateNoMinRTOFromOptionsAtServer) { QuicConfig config; QuicTagVector options; @@ -2504,7 +2106,7 @@ QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_, 0)); } -TEST_P(QuicSentPacketManagerTest, NegotiateNoMinRTOFromOptionsAtClient) { +TEST_F(QuicSentPacketManagerTest, NegotiateNoMinRTOFromOptionsAtClient) { QuicConfig client_config; QuicTagVector options; @@ -2537,7 +2139,7 @@ QuicSentPacketManagerPeer::GetTailLossProbeDelay(&manager_, 0)); } -TEST_P(QuicSentPacketManagerTest, NegotiateNoTLPFromOptionsAtServer) { +TEST_F(QuicSentPacketManagerTest, NegotiateNoTLPFromOptionsAtServer) { QuicConfig config; QuicTagVector options; @@ -2549,7 +2151,7 @@ EXPECT_EQ(0u, QuicSentPacketManagerPeer::GetMaxTailLossProbes(&manager_)); } -TEST_P(QuicSentPacketManagerTest, NegotiateNoTLPFromOptionsAtClient) { +TEST_F(QuicSentPacketManagerTest, NegotiateNoTLPFromOptionsAtClient) { QuicConfig client_config; QuicTagVector options; @@ -2562,7 +2164,7 @@ EXPECT_EQ(0u, QuicSentPacketManagerPeer::GetMaxTailLossProbes(&manager_)); } -TEST_P(QuicSentPacketManagerTest, Negotiate1TLPFromOptionsAtServer) { +TEST_F(QuicSentPacketManagerTest, Negotiate1TLPFromOptionsAtServer) { QuicConfig config; QuicTagVector options; @@ -2574,7 +2176,7 @@ EXPECT_EQ(1u, QuicSentPacketManagerPeer::GetMaxTailLossProbes(&manager_)); } -TEST_P(QuicSentPacketManagerTest, Negotiate1TLPFromOptionsAtClient) { +TEST_F(QuicSentPacketManagerTest, Negotiate1TLPFromOptionsAtClient) { QuicConfig client_config; QuicTagVector options; @@ -2587,7 +2189,7 @@ EXPECT_EQ(1u, QuicSentPacketManagerPeer::GetMaxTailLossProbes(&manager_)); } -TEST_P(QuicSentPacketManagerTest, NegotiateTLPRttFromOptionsAtServer) { +TEST_F(QuicSentPacketManagerTest, NegotiateTLPRttFromOptionsAtServer) { QuicConfig config; QuicTagVector options; @@ -2600,7 +2202,7 @@ QuicSentPacketManagerPeer::GetEnableHalfRttTailLossProbe(&manager_)); } -TEST_P(QuicSentPacketManagerTest, NegotiateTLPRttFromOptionsAtClient) { +TEST_F(QuicSentPacketManagerTest, NegotiateTLPRttFromOptionsAtClient) { QuicConfig client_config; QuicTagVector options; @@ -2614,7 +2216,7 @@ QuicSentPacketManagerPeer::GetEnableHalfRttTailLossProbe(&manager_)); } -TEST_P(QuicSentPacketManagerTest, NegotiateNewRTOFromOptionsAtServer) { +TEST_F(QuicSentPacketManagerTest, NegotiateNewRTOFromOptionsAtServer) { EXPECT_FALSE(QuicSentPacketManagerPeer::GetUseNewRto(&manager_)); QuicConfig config; QuicTagVector options; @@ -2627,7 +2229,7 @@ EXPECT_TRUE(QuicSentPacketManagerPeer::GetUseNewRto(&manager_)); } -TEST_P(QuicSentPacketManagerTest, NegotiateNewRTOFromOptionsAtClient) { +TEST_F(QuicSentPacketManagerTest, NegotiateNewRTOFromOptionsAtClient) { EXPECT_FALSE(QuicSentPacketManagerPeer::GetUseNewRto(&manager_)); QuicConfig client_config; QuicTagVector options; @@ -2641,7 +2243,7 @@ EXPECT_TRUE(QuicSentPacketManagerPeer::GetUseNewRto(&manager_)); } -TEST_P(QuicSentPacketManagerTest, UseInitialRoundTripTimeToSend) { +TEST_F(QuicSentPacketManagerTest, UseInitialRoundTripTimeToSend) { QuicTime::Delta initial_rtt = QuicTime::Delta::FromMilliseconds(325); EXPECT_NE(initial_rtt, manager_.GetRttStats()->smoothed_rtt()); @@ -2655,7 +2257,7 @@ EXPECT_EQ(initial_rtt, manager_.GetRttStats()->initial_rtt()); } -TEST_P(QuicSentPacketManagerTest, ResumeConnectionState) { +TEST_F(QuicSentPacketManagerTest, ResumeConnectionState) { // The sent packet manager should use the RTT from CachedNetworkParameters if // it is provided. const QuicTime::Delta kRtt = QuicTime::Delta::FromMilliseconds(1234); @@ -2670,7 +2272,7 @@ EXPECT_EQ(kRtt, manager_.GetRttStats()->initial_rtt()); } -TEST_P(QuicSentPacketManagerTest, ConnectionMigrationUnspecifiedChange) { +TEST_F(QuicSentPacketManagerTest, ConnectionMigrationUnspecifiedChange) { RttStats* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats()); QuicTime::Delta default_init_rtt = rtt_stats->initial_rtt(); rtt_stats->set_initial_rtt(default_init_rtt * 2); @@ -2689,7 +2291,7 @@ EXPECT_EQ(0u, manager_.GetConsecutiveTlpCount()); } -TEST_P(QuicSentPacketManagerTest, ConnectionMigrationIPSubnetChange) { +TEST_F(QuicSentPacketManagerTest, ConnectionMigrationIPSubnetChange) { RttStats* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats()); QuicTime::Delta default_init_rtt = rtt_stats->initial_rtt(); rtt_stats->set_initial_rtt(default_init_rtt * 2); @@ -2707,7 +2309,7 @@ EXPECT_EQ(2u, manager_.GetConsecutiveTlpCount()); } -TEST_P(QuicSentPacketManagerTest, ConnectionMigrationPortChange) { +TEST_F(QuicSentPacketManagerTest, ConnectionMigrationPortChange) { RttStats* rtt_stats = const_cast<RttStats*>(manager_.GetRttStats()); QuicTime::Delta default_init_rtt = rtt_stats->initial_rtt(); rtt_stats->set_initial_rtt(default_init_rtt * 2); @@ -2725,13 +2327,13 @@ EXPECT_EQ(2u, manager_.GetConsecutiveTlpCount()); } -TEST_P(QuicSentPacketManagerTest, PathMtuIncreased) { +TEST_F(QuicSentPacketManagerTest, PathMtuIncreased) { EXPECT_CALL(*send_algorithm_, OnPacketSent(_, BytesInFlight(), QuicPacketNumber(1), _, _)); SerializedPacket packet(QuicPacketNumber(1), PACKET_4BYTE_PACKET_NUMBER, nullptr, kDefaultLength + 100, false, false); - manager_.OnPacketSent(&packet, QuicPacketNumber(), clock_.Now(), - NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); + manager_.OnPacketSent(&packet, clock_.Now(), NOT_RETRANSMISSION, + HAS_RETRANSMITTABLE_DATA); // Ack the large packet and expect the path MTU to increase. ExpectAck(1); @@ -2746,7 +2348,7 @@ ENCRYPTION_INITIAL)); } -TEST_P(QuicSentPacketManagerTest, OnAckRangeSlowPath) { +TEST_F(QuicSentPacketManagerTest, OnAckRangeSlowPath) { // Send packets 1 - 20. for (size_t i = 1; i <= 20; ++i) { SendDataPacket(i); @@ -2781,7 +2383,7 @@ ENCRYPTION_INITIAL)); } -TEST_P(QuicSentPacketManagerTest, TolerateReneging) { +TEST_F(QuicSentPacketManagerTest, TolerateReneging) { // Send packets 1 - 20. for (size_t i = 1; i <= 20; ++i) { SendDataPacket(i); @@ -2814,7 +2416,7 @@ EXPECT_EQ(QuicPacketNumber(16), manager_.GetLargestObserved()); } -TEST_P(QuicSentPacketManagerTest, MultiplePacketNumberSpaces) { +TEST_F(QuicSentPacketManagerTest, MultiplePacketNumberSpaces) { manager_.EnableMultiplePacketNumberSpacesSupport(); EXPECT_FALSE( manager_.GetLargestSentPacket(ENCRYPTION_INITIAL).IsInitialized()); @@ -2926,7 +2528,7 @@ manager_.GetLargestAckedPacket(ENCRYPTION_FORWARD_SECURE)); } -TEST_P(QuicSentPacketManagerTest, PacketsGetAckedInWrongPacketNumberSpace) { +TEST_F(QuicSentPacketManagerTest, PacketsGetAckedInWrongPacketNumberSpace) { manager_.EnableMultiplePacketNumberSpacesSupport(); // Send packet 1. SendDataPacket(1, ENCRYPTION_INITIAL); @@ -2943,7 +2545,7 @@ ENCRYPTION_INITIAL)); } -TEST_P(QuicSentPacketManagerTest, PacketsGetAckedInWrongPacketNumberSpace2) { +TEST_F(QuicSentPacketManagerTest, PacketsGetAckedInWrongPacketNumberSpace2) { manager_.EnableMultiplePacketNumberSpacesSupport(); // Send packet 1. SendDataPacket(1, ENCRYPTION_INITIAL); @@ -2960,7 +2562,7 @@ ENCRYPTION_HANDSHAKE)); } -TEST_P(QuicSentPacketManagerTest, +TEST_F(QuicSentPacketManagerTest, ToleratePacketsGetAckedInWrongPacketNumberSpace) { manager_.EnableMultiplePacketNumberSpacesSupport(); // Send packet 1. @@ -2991,10 +2593,7 @@ } // Regression test for b/133771183. -TEST_P(QuicSentPacketManagerTest, PacketInLimbo) { - if (!manager_.session_decides_what_to_write()) { - return; - } +TEST_F(QuicSentPacketManagerTest, PacketInLimbo) { QuicSentPacketManagerPeer::SetMaxTailLossProbes(&manager_, 2); // Send SHLO. SendCryptoPacket(1); @@ -3046,10 +2645,7 @@ ENCRYPTION_INITIAL)); } -TEST_P(QuicSentPacketManagerTest, RtoFiresNoPacketToRetransmit) { - if (!manager_.session_decides_what_to_write()) { - return; - } +TEST_F(QuicSentPacketManagerTest, RtoFiresNoPacketToRetransmit) { // Send 10 packets. for (size_t i = 1; i <= 10; ++i) { SendDataPacket(i); @@ -3073,7 +2669,7 @@ EXPECT_EQ(1u, manager_.pending_timer_transmission_count()); } -TEST_P(QuicSentPacketManagerTest, ComputingProbeTimeout) { +TEST_F(QuicSentPacketManagerTest, ComputingProbeTimeout) { EnablePto(k2PTO); EXPECT_CALL(*send_algorithm_, PacingRate(_)) .WillRepeatedly(Return(QuicBandwidth::Zero())); @@ -3139,7 +2735,7 @@ EXPECT_EQ(sent_time + expected_pto_delay, manager_.GetRetransmissionTime()); } -TEST_P(QuicSentPacketManagerTest, SendOneProbePacket) { +TEST_F(QuicSentPacketManagerTest, SendOneProbePacket) { EnablePto(k1PTO); EXPECT_CALL(*send_algorithm_, PacingRate(_)) .WillRepeatedly(Return(QuicBandwidth::Zero())); @@ -3174,9 +2770,8 @@ manager_.MaybeSendProbePackets(); } -TEST_P(QuicSentPacketManagerTest, DisableHandshakeModeClient) { +TEST_F(QuicSentPacketManagerTest, DisableHandshakeModeClient) { QuicSentPacketManagerPeer::SetPerspective(&manager_, Perspective::IS_CLIENT); - manager_.SetSessionDecideWhatToWrite(true); manager_.EnableIetfPtoAndLossDetection(); // Send CHLO. SendCryptoPacket(1); @@ -3198,8 +2793,7 @@ manager_.OnRetransmissionTimeout()); } -TEST_P(QuicSentPacketManagerTest, DisableHandshakeModeServer) { - manager_.SetSessionDecideWhatToWrite(true); +TEST_F(QuicSentPacketManagerTest, DisableHandshakeModeServer) { manager_.EnableIetfPtoAndLossDetection(); // Send SHLO. SendCryptoPacket(1); @@ -3218,7 +2812,7 @@ EXPECT_EQ(QuicTime::Zero(), manager_.GetRetransmissionTime()); } -TEST_P(QuicSentPacketManagerTest, ForwardSecurePacketAcked) { +TEST_F(QuicSentPacketManagerTest, ForwardSecurePacketAcked) { EXPECT_FALSE(manager_.forward_secure_packet_acked()); SendDataPacket(1, ENCRYPTION_INITIAL); // Ack packet 1.
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index d0b9a84..09e305b 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -540,9 +540,7 @@ "write blocked."; return; } - if (session_decides_what_to_write()) { - SetTransmissionType(NOT_RETRANSMISSION); - } + SetTransmissionType(NOT_RETRANSMISSION); // We limit the number of writes to the number of pending streams. If more // streams become pending, WillingAndAbleToWrite will be true, which will // cause the connection to request resumption before yielding to other @@ -839,9 +837,7 @@ zombie_streams_[stream->id()] = std::move(it->second); } else { // Clean up the stream since it is no longer waiting for acks. - if (session_decides_what_to_write()) { - streams_waiting_for_acks_.erase(stream->id()); - } + streams_waiting_for_acks_.erase(stream->id()); closed_streams_.push_back(std::move(it->second)); // Do not retransmit data of a closed stream. streams_with_pending_retransmission_.erase(stream_id); @@ -1622,9 +1618,7 @@ } void QuicSession::OnStreamDoneWaitingForAcks(QuicStreamId id) { - if (session_decides_what_to_write()) { - streams_waiting_for_acks_.erase(id); - } + streams_waiting_for_acks_.erase(id); auto it = zombie_streams_.find(id); if (it == zombie_streams_.end()) { @@ -1641,10 +1635,6 @@ } void QuicSession::OnStreamWaitingForAcks(QuicStreamId id) { - if (!session_decides_what_to_write()) { - return; - } - // Exclude crypto stream's status since it is counted in HasUnackedCryptoData. if (GetCryptoStream() != nullptr && id == GetCryptoStream()->id()) { return; @@ -1904,14 +1894,12 @@ } void QuicSession::NeuterUnencryptedData() { - if (connection_->session_decides_what_to_write()) { - QuicCryptoStream* crypto_stream = GetMutableCryptoStream(); - crypto_stream->NeuterUnencryptedStreamData(); - if (!crypto_stream->HasPendingRetransmission() && - !QuicVersionUsesCryptoFrames(transport_version())) { - streams_with_pending_retransmission_.erase( - QuicUtils::GetCryptoStreamId(transport_version())); - } + QuicCryptoStream* crypto_stream = GetMutableCryptoStream(); + crypto_stream->NeuterUnencryptedStreamData(); + if (!crypto_stream->HasPendingRetransmission() && + !QuicVersionUsesCryptoFrames(transport_version())) { + streams_with_pending_retransmission_.erase( + QuicUtils::GetCryptoStreamId(transport_version())); } connection_->NeuterUnencryptedPackets(); } @@ -1946,10 +1934,6 @@ closed_streams_.clear(); } -bool QuicSession::session_decides_what_to_write() const { - return connection_->session_decides_what_to_write(); -} - QuicPacketLength QuicSession::GetCurrentLargestMessagePayload() const { return connection_->GetCurrentLargestMessagePayload(); }
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h index 4af037a..498fb57 100644 --- a/quic/core/quic_session.h +++ b/quic/core/quic_session.h
@@ -405,8 +405,6 @@ // Clean up closed_streams_. void CleanUpClosedStreams(); - bool session_decides_what_to_write() const; - const ParsedQuicVersionVector& supported_versions() const { return supported_versions_; }
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index bf1724c..d128f0d 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -2146,7 +2146,6 @@ } TEST_P(QuicSessionTestServer, OnStreamFrameLost) { - QuicConnectionPeer::SetSessionDecidesWhatToWrite(connection_); InSequence s; // Drive congestion control manually. @@ -2223,7 +2222,6 @@ } TEST_P(QuicSessionTestServer, DonotRetransmitDataOfClosedStreams) { - QuicConnectionPeer::SetSessionDecidesWhatToWrite(connection_); InSequence s; TestStream* stream2 = session_.CreateOutgoingBidirectionalStream(); @@ -2263,7 +2261,6 @@ } TEST_P(QuicSessionTestServer, RetransmitFrames) { - QuicConnectionPeer::SetSessionDecidesWhatToWrite(connection_); MockSendAlgorithm* send_algorithm = new StrictMock<MockSendAlgorithm>; QuicConnectionPeer::SetSendAlgorithm(session_.connection(), send_algorithm); InSequence s; @@ -2299,7 +2296,6 @@ TEST_P(QuicSessionTestServer, RetransmitLostDataCausesConnectionClose) { // This test mimics the scenario when a dynamic stream retransmits lost data // and causes connection close. - QuicConnectionPeer::SetSessionDecidesWhatToWrite(connection_); TestStream* stream = session_.CreateOutgoingBidirectionalStream(); QuicStreamFrame frame(stream->id(), false, 0, 9); @@ -2378,8 +2374,6 @@ // Regression test of b/115323618. TEST_P(QuicSessionTestServer, LocallyResetZombieStreams) { - QuicConnectionPeer::SetSessionDecidesWhatToWrite(connection_); - session_.set_writev_consumes_all_data(true); TestStream* stream2 = session_.CreateOutgoingBidirectionalStream(); std::string body(100, '.');
diff --git a/quic/core/quic_stream.cc b/quic/core/quic_stream.cc index d9437fb..4b70bd4 100644 --- a/quic/core/quic_stream.cc +++ b/quic/core/quic_stream.cc
@@ -1051,9 +1051,7 @@ QUIC_DVLOG(1) << "stream " << id() << " shortens write length to " << write_length << " due to flow control"; } - if (session_->session_decides_what_to_write()) { - session_->SetTransmissionType(NOT_RETRANSMISSION); - } + session_->SetTransmissionType(NOT_RETRANSMISSION); StreamSendingState state = fin ? FIN : NO_FIN; if (fin && add_random_padding_after_fin_) { @@ -1176,10 +1174,6 @@ QUIC_DLOG(WARNING) << "Deadline has already been set."; return false; } - if (!session()->session_decides_what_to_write()) { - QUIC_DLOG(WARNING) << "This session does not support stream TTL yet."; - return false; - } QuicTime now = session()->connection()->clock()->ApproximateNow(); deadline_ = now + ttl; return true; @@ -1190,7 +1184,6 @@ // No deadline has been set. return false; } - DCHECK(session()->session_decides_what_to_write()); QuicTime now = session()->connection()->clock()->ApproximateNow(); if (now < deadline_) { return false;
diff --git a/quic/core/quic_trace_visitor.cc b/quic/core/quic_trace_visitor.cc index 42ee003..8295e72 100644 --- a/quic/core/quic_trace_visitor.cc +++ b/quic/core/quic_trace_visitor.cc
@@ -44,7 +44,6 @@ } void QuicTraceVisitor::OnPacketSent(const SerializedPacket& serialized_packet, - QuicPacketNumber /*original_packet_number*/, TransmissionType /*transmission_type*/, QuicTime sent_time) { quic_trace::Event* event = trace_.add_events();
diff --git a/quic/core/quic_trace_visitor.h b/quic/core/quic_trace_visitor.h index 0494d87..124b4b6 100644 --- a/quic/core/quic_trace_visitor.h +++ b/quic/core/quic_trace_visitor.h
@@ -19,7 +19,6 @@ explicit QuicTraceVisitor(const QuicConnection* connection); void OnPacketSent(const SerializedPacket& serialized_packet, - QuicPacketNumber original_packet_number, TransmissionType transmission_type, QuicTime sent_time) override;
diff --git a/quic/core/quic_transmission_info.cc b/quic/core/quic_transmission_info.cc index fb5cf67..163ba38 100644 --- a/quic/core/quic_transmission_info.cc +++ b/quic/core/quic_transmission_info.cc
@@ -8,7 +8,6 @@ QuicTransmissionInfo::QuicTransmissionInfo() : encryption_level(ENCRYPTION_INITIAL), - packet_number_length(PACKET_1BYTE_PACKET_NUMBER), bytes_sent(0), sent_time(QuicTime::Zero()), transmission_type(NOT_RETRANSMISSION), @@ -19,14 +18,12 @@ QuicTransmissionInfo::QuicTransmissionInfo( EncryptionLevel level, - QuicPacketNumberLength packet_number_length, TransmissionType transmission_type, QuicTime sent_time, QuicPacketLength bytes_sent, bool has_crypto_handshake, int num_padding_bytes) : encryption_level(level), - packet_number_length(packet_number_length), bytes_sent(bytes_sent), sent_time(sent_time), transmission_type(transmission_type),
diff --git a/quic/core/quic_transmission_info.h b/quic/core/quic_transmission_info.h index 6bc9674..a4fa762 100644 --- a/quic/core/quic_transmission_info.h +++ b/quic/core/quic_transmission_info.h
@@ -22,7 +22,6 @@ // Constructs a Transmission with a new all_transmissions set // containing |packet_number|. QuicTransmissionInfo(EncryptionLevel level, - QuicPacketNumberLength packet_number_length, TransmissionType transmission_type, QuicTime sent_time, QuicPacketLength bytes_sent, @@ -35,9 +34,6 @@ QuicFrames retransmittable_frames; EncryptionLevel encryption_level; - // TODO(fayang): remove this when clean up !session_decides_what_to_write code - // path. - QuicPacketNumberLength packet_number_length; QuicPacketLength bytes_sent; QuicTime sent_time; // Reason why this packet was transmitted.
diff --git a/quic/core/quic_unacked_packet_map.cc b/quic/core/quic_unacked_packet_map.cc index cc9c13f..c652c8c 100644 --- a/quic/core/quic_unacked_packet_map.cc +++ b/quic/core/quic_unacked_packet_map.cc
@@ -28,11 +28,9 @@ : perspective_(perspective), least_unacked_(FirstSendingPacketNumber()), bytes_in_flight_(0), - pending_crypto_packet_count_(0), last_inflight_packet_sent_time_(QuicTime::Zero()), last_crypto_packet_sent_time_(QuicTime::Zero()), session_notifier_(nullptr), - session_decides_what_to_write_(false), supports_multiple_packet_number_spaces_(false), simple_inflight_time_(GetQuicReloadableFlag(quic_simple_inflight_time)) { if (simple_inflight_time_) { @@ -47,7 +45,6 @@ } void QuicUnackedPacketMap::AddSentPacket(SerializedPacket* packet, - QuicPacketNumber old_packet_number, TransmissionType transmission_type, QuicTime sent_time, bool set_in_flight) { @@ -65,15 +62,11 @@ const bool has_crypto_handshake = packet->has_crypto_handshake == IS_HANDSHAKE; - QuicTransmissionInfo info( - packet->encryption_level, packet->packet_number_length, transmission_type, - sent_time, bytes_sent, has_crypto_handshake, packet->num_padding_bytes); + QuicTransmissionInfo info(packet->encryption_level, transmission_type, + sent_time, bytes_sent, has_crypto_handshake, + packet->num_padding_bytes); info.largest_acked = packet->largest_acked; largest_sent_largest_acked_.UpdateMax(packet->largest_acked); - if (old_packet_number.IsInitialized()) { - TransferRetransmissionInfo(old_packet_number, packet_number, - transmission_type, &info); - } largest_sent_packet_ = packet_number; if (supports_multiple_packet_number_spaces_) { @@ -92,15 +85,12 @@ unacked_packets_.push_back(info); // Swap the retransmittable frames to avoid allocations. // TODO(ianswett): Could use emplace_back when Chromium can. - if (!old_packet_number.IsInitialized()) { - if (has_crypto_handshake) { - ++pending_crypto_packet_count_; - last_crypto_packet_sent_time_ = sent_time; - } - - packet->retransmittable_frames.swap( - unacked_packets_.back().retransmittable_frames); + if (has_crypto_handshake) { + last_crypto_packet_sent_time_ = sent_time; } + + packet->retransmittable_frames.swap( + unacked_packets_.back().retransmittable_frames); } void QuicUnackedPacketMap::RemoveObsoletePackets() { @@ -108,62 +98,12 @@ if (!IsPacketUseless(least_unacked_, unacked_packets_.front())) { break; } - if (session_decides_what_to_write_) { - DeleteFrames(&unacked_packets_.front().retransmittable_frames); - } + DeleteFrames(&unacked_packets_.front().retransmittable_frames); unacked_packets_.pop_front(); ++least_unacked_; } } -void QuicUnackedPacketMap::TransferRetransmissionInfo( - QuicPacketNumber old_packet_number, - QuicPacketNumber new_packet_number, - TransmissionType transmission_type, - QuicTransmissionInfo* info) { - if (old_packet_number < least_unacked_) { - // This can happen when a retransmission packet is queued because of write - // blocked socket, and the original packet gets acked before the - // retransmission gets sent. - return; - } - if (old_packet_number > largest_sent_packet_) { - QUIC_BUG << "Old QuicTransmissionInfo never existed for :" - << old_packet_number << " largest_sent:" << largest_sent_packet_; - return; - } - DCHECK_GE(new_packet_number, least_unacked_ + unacked_packets_.size()); - DCHECK_NE(NOT_RETRANSMISSION, transmission_type); - - QuicTransmissionInfo* transmission_info = - &unacked_packets_.at(old_packet_number - least_unacked_); - QuicFrames* frames = &transmission_info->retransmittable_frames; - if (session_notifier_ != nullptr) { - for (const QuicFrame& frame : *frames) { - if (frame.type == STREAM_FRAME) { - session_notifier_->OnStreamFrameRetransmitted(frame.stream_frame); - } - } - } - - // Swap the frames and preserve num_padding_bytes and has_crypto_handshake. - frames->swap(info->retransmittable_frames); - info->has_crypto_handshake = transmission_info->has_crypto_handshake; - transmission_info->has_crypto_handshake = false; - info->num_padding_bytes = transmission_info->num_padding_bytes; - - // Don't link old transmissions to new ones when version or - // encryption changes. - if (transmission_type == ALL_INITIAL_RETRANSMISSION || - transmission_type == ALL_UNACKED_RETRANSMISSION) { - transmission_info->state = UNACKABLE; - } else { - transmission_info->retransmission = new_packet_number; - } - // Proactively remove obsolete packets so the least unacked can be raised. - RemoveObsoletePackets(); -} - bool QuicUnackedPacketMap::HasRetransmittableFrames( QuicPacketNumber packet_number) const { DCHECK_GE(packet_number, least_unacked_); @@ -174,10 +114,6 @@ bool QuicUnackedPacketMap::HasRetransmittableFrames( const QuicTransmissionInfo& info) const { - if (!session_decides_what_to_write_) { - return !info.retransmittable_frames.empty(); - } - if (!QuicUtils::IsAckable(info.state)) { return false; } @@ -192,24 +128,8 @@ void QuicUnackedPacketMap::RemoveRetransmittability( QuicTransmissionInfo* info) { - if (session_decides_what_to_write_) { - DeleteFrames(&info->retransmittable_frames); - info->retransmission.Clear(); - return; - } - while (info->retransmission.IsInitialized()) { - const QuicPacketNumber retransmission = info->retransmission; - info->retransmission.Clear(); - info = &unacked_packets_[retransmission - least_unacked_]; - } - - if (info->has_crypto_handshake) { - DCHECK(HasRetransmittableFrames(*info)); - DCHECK_LT(0u, pending_crypto_packet_count_); - --pending_crypto_packet_count_; - info->has_crypto_handshake = false; - } DeleteFrames(&info->retransmittable_frames); + info->retransmission.Clear(); } void QuicUnackedPacketMap::RemoveRetransmittability( @@ -250,16 +170,6 @@ bool QuicUnackedPacketMap::IsPacketUsefulForRetransmittableData( const QuicTransmissionInfo& info) const { - if (!session_decides_what_to_write_) { - // Packet may have retransmittable frames, or the data may have been - // retransmitted with a new packet number. - // Allow for an extra 1 RTT before stopping to track old packets. - return (info.retransmission.IsInitialized() && - (!largest_acked_.IsInitialized() || - info.retransmission > largest_acked_)) || - HasRetransmittableFrames(info); - } - // Wait for 1 RTT before giving up on the lost packet. return info.retransmission.IsInitialized() && (!largest_acked_.IsInitialized() || @@ -299,23 +209,6 @@ RemoveFromInFlight(info); } -void QuicUnackedPacketMap::CancelRetransmissionsForStream( - QuicStreamId stream_id) { - DCHECK(!session_decides_what_to_write_); - QuicPacketNumber packet_number = least_unacked_; - for (auto it = unacked_packets_.begin(); it != unacked_packets_.end(); - ++it, ++packet_number) { - QuicFrames* frames = &it->retransmittable_frames; - if (frames->empty()) { - continue; - } - RemoveFramesForStream(frames, stream_id); - if (frames->empty()) { - RemoveRetransmittability(packet_number); - } - } -} - bool QuicUnackedPacketMap::HasInFlightPackets() const { return bytes_in_flight_ > 0; } @@ -381,9 +274,6 @@ } bool QuicUnackedPacketMap::HasPendingCryptoPackets() const { - if (!session_decides_what_to_write_) { - return pending_crypto_packet_count_ > 0; - } return session_notifier_->HasUnackedCryptoData(); } @@ -423,7 +313,6 @@ void QuicUnackedPacketMap::NotifyFramesLost(const QuicTransmissionInfo& info, TransmissionType /*type*/) { - DCHECK(session_decides_what_to_write_); for (const QuicFrame& frame : info.retransmittable_frames) { session_notifier_->OnFrameLost(frame); } @@ -431,7 +320,6 @@ void QuicUnackedPacketMap::RetransmitFrames(const QuicTransmissionInfo& info, TransmissionType type) { - DCHECK(session_decides_what_to_write_); session_notifier_->RetransmitFrames(info.retransmittable_frames, type); } @@ -537,15 +425,6 @@ return largest_sent_retransmittable_packets_[packet_number_space]; } -void QuicUnackedPacketMap::SetSessionDecideWhatToWrite( - bool session_decides_what_to_write) { - if (largest_sent_packet_.IsInitialized()) { - QUIC_BUG << "Cannot change session_decide_what_to_write with packets sent."; - return; - } - session_decides_what_to_write_ = session_decides_what_to_write; -} - void QuicUnackedPacketMap::EnableMultiplePacketNumberSpacesSupport() { if (supports_multiple_packet_number_spaces_) { QUIC_BUG << "Multiple packet number spaces has already been enabled";
diff --git a/quic/core/quic_unacked_packet_map.h b/quic/core/quic_unacked_packet_map.h index 864e485..e079bb8 100644 --- a/quic/core/quic_unacked_packet_map.h +++ b/quic/core/quic_unacked_packet_map.h
@@ -34,12 +34,9 @@ // Marks the packet as in flight if |set_in_flight| is true. // Packets marked as in flight are expected to be marked as missing when they // don't arrive, indicating the need for retransmission. - // |old_packet_number| is the packet number of the previous transmission, - // or 0 if there was none. // Any AckNotifierWrappers in |serialized_packet| are swapped from the // serialized packet into the QuicTransmissionInfo. void AddSentPacket(SerializedPacket* serialized_packet, - QuicPacketNumber old_packet_number, TransmissionType transmission_type, QuicTime sent_time, bool set_in_flight); @@ -68,9 +65,6 @@ // Marks |packet_number| as no longer in flight. void RemoveFromInFlight(QuicPacketNumber packet_number); - // No longer retransmit data for |stream_id|. - void CancelRetransmissionsForStream(QuicStreamId stream_id); - // Returns true if |packet_number| has retransmittable frames. This will // return false if all frames of this packet are either non-retransmittable or // have been acked. @@ -143,13 +137,10 @@ bool HasMultipleInFlightPackets() const; // Returns true if there are any pending crypto packets. - // TODO(fayang): Remove this method and call session_notifier_'s - // HasUnackedCryptoData() when session_decides_what_to_write_ is default true. bool HasPendingCryptoPackets() const; // Returns true if there is any unacked non-crypto stream data. bool HasUnackedStreamData() const { - DCHECK(session_decides_what_to_write()); return session_notifier_->HasUnackedStreamData(); } @@ -210,17 +201,10 @@ QuicPacketNumber GetLargestSentPacketOfPacketNumberSpace( EncryptionLevel encryption_level) const; - // Called to start/stop letting session decide what to write. - void SetSessionDecideWhatToWrite(bool session_decides_what_to_write); - void SetSessionNotifier(SessionNotifierInterface* session_notifier); void EnableMultiplePacketNumberSpacesSupport(); - bool session_decides_what_to_write() const { - return session_decides_what_to_write_; - } - Perspective perspective() const { return perspective_; } bool supports_multiple_packet_number_spaces() const { @@ -232,15 +216,6 @@ private: friend class test::QuicUnackedPacketMapPeer; - // Called when a packet is retransmitted with a new packet number. - // |old_packet_number| will remain unacked, but will have no - // retransmittable data associated with it. Retransmittable frames will be - // transferred to |info| and all_transmissions will be populated. - void TransferRetransmissionInfo(QuicPacketNumber old_packet_number, - QuicPacketNumber new_packet_number, - TransmissionType transmission_type, - QuicTransmissionInfo* info); - // Returns true if packet may be useful for an RTT measurement. bool IsPacketUsefulForMeasuringRtt(QuicPacketNumber packet_number, const QuicTransmissionInfo& info) const; @@ -287,8 +262,6 @@ QuicPacketNumber least_unacked_; QuicByteCount bytes_in_flight_; - // Number of retransmittable crypto handshake packets. - size_t pending_crypto_packet_count_; // Time that the last inflight packet was sent. QuicTime last_inflight_packet_sent_time_; @@ -303,9 +276,6 @@ // Receives notifications of frames being retransmitted or acknowledged. SessionNotifierInterface* session_notifier_; - // If true, let session decides what to write. - bool session_decides_what_to_write_; - // If true, supports multiple packet number spaces. bool supports_multiple_packet_number_spaces_;
diff --git a/quic/core/quic_unacked_packet_map_test.cc b/quic/core/quic_unacked_packet_map_test.cc index ad09639..92e61e3 100644 --- a/quic/core/quic_unacked_packet_map_test.cc +++ b/quic/core/quic_unacked_packet_map_test.cc
@@ -24,43 +24,12 @@ // Default packet length. const uint32_t kDefaultLength = 1000; -struct TestParams { - TestParams(Perspective perspective, bool session_decides_what_to_write) - : perspective(perspective), - session_decides_what_to_write(session_decides_what_to_write) {} - - Perspective perspective; - bool session_decides_what_to_write; -}; - -// Used by ::testing::PrintToStringParamName(). -std::string PrintToString(const TestParams& p) { - return QuicStrCat( - (p.perspective == Perspective::IS_CLIENT ? "Client" : "Server"), - "_Session", - (p.session_decides_what_to_write ? "Decides" : "DoesNotDecide"), - "WhatToWrite"); -} - -std::vector<TestParams> GetTestParams() { - std::vector<TestParams> params; - for (Perspective perspective : - {Perspective::IS_CLIENT, Perspective::IS_SERVER}) { - for (bool session_decides_what_to_write : {true, false}) { - params.push_back(TestParams(perspective, session_decides_what_to_write)); - } - } - return params; -} - -class QuicUnackedPacketMapTest : public QuicTestWithParam<TestParams> { +class QuicUnackedPacketMapTest : public QuicTestWithParam<Perspective> { protected: QuicUnackedPacketMapTest() - : unacked_packets_(GetParam().perspective), + : unacked_packets_(GetParam()), now_(QuicTime::Zero() + QuicTime::Delta::FromMilliseconds(1000)) { unacked_packets_.SetSessionNotifier(¬ifier_); - unacked_packets_.SetSessionDecideWhatToWrite( - GetParam().session_decides_what_to_write); EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(true)); EXPECT_CALL(notifier_, OnStreamFrameRetransmitted(_)) .Times(testing::AnyNumber()); @@ -167,14 +136,6 @@ TransmissionType transmission_type) { DCHECK(unacked_packets_.HasRetransmittableFrames( QuicPacketNumber(old_packet_number))); - if (!unacked_packets_.session_decides_what_to_write()) { - SerializedPacket packet( - CreateNonRetransmittablePacket(new_packet_number)); - unacked_packets_.AddSentPacket(&packet, - QuicPacketNumber(old_packet_number), - transmission_type, now_, true); - return; - } QuicTransmissionInfo* info = unacked_packets_.GetMutableTransmissionInfo( QuicPacketNumber(old_packet_number)); QuicStreamId stream_id = QuicUtils::GetFirstBidirectionalStreamId( @@ -192,8 +153,7 @@ info->retransmission = QuicPacketNumber(new_packet_number); SerializedPacket packet( CreateRetransmittablePacketForStream(new_packet_number, stream_id)); - unacked_packets_.AddSentPacket(&packet, QuicPacketNumber(), - transmission_type, now_, true); + unacked_packets_.AddSentPacket(&packet, transmission_type, now_, true); } QuicUnackedPacketMap unacked_packets_; QuicTime now_; @@ -202,14 +162,14 @@ INSTANTIATE_TEST_SUITE_P(Tests, QuicUnackedPacketMapTest, - ::testing::ValuesIn(GetTestParams()), + ::testing::ValuesIn({Perspective::IS_CLIENT, + Perspective::IS_SERVER}), ::testing::PrintToStringParamName()); TEST_P(QuicUnackedPacketMapTest, RttOnly) { // Acks are only tracked for RTT measurement purposes. SerializedPacket packet(CreateNonRetransmittablePacket(1)); - unacked_packets_.AddSentPacket(&packet, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, false); + unacked_packets_.AddSentPacket(&packet, NOT_RETRANSMISSION, now_, false); uint64_t unacked[] = {1}; VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); @@ -225,8 +185,7 @@ TEST_P(QuicUnackedPacketMapTest, RetransmittableInflightAndRtt) { // Simulate a retransmittable packet being sent and acked. SerializedPacket packet(CreateRetransmittablePacket(1)); - unacked_packets_.AddSentPacket(&packet, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet, NOT_RETRANSMISSION, now_, true); uint64_t unacked[] = {1}; VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); @@ -252,8 +211,7 @@ TEST_P(QuicUnackedPacketMapTest, StopRetransmission) { const QuicStreamId stream_id = 2; SerializedPacket packet(CreateRetransmittablePacketForStream(1, stream_id)); - unacked_packets_.AddSentPacket(&packet, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet, NOT_RETRANSMISSION, now_, true); uint64_t unacked[] = {1}; VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); @@ -262,11 +220,7 @@ VerifyRetransmittablePackets(retransmittable, QUIC_ARRAYSIZE(retransmittable)); - if (unacked_packets_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); - } else { - unacked_packets_.CancelRetransmissionsForStream(stream_id); - } + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); VerifyInFlightPackets(unacked, QUIC_ARRAYSIZE(unacked)); VerifyRetransmittablePackets(nullptr, 0); @@ -275,8 +229,7 @@ TEST_P(QuicUnackedPacketMapTest, StopRetransmissionOnOtherStream) { const QuicStreamId stream_id = 2; SerializedPacket packet(CreateRetransmittablePacketForStream(1, stream_id)); - unacked_packets_.AddSentPacket(&packet, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet, NOT_RETRANSMISSION, now_, true); uint64_t unacked[] = {1}; VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); @@ -285,10 +238,6 @@ VerifyRetransmittablePackets(retransmittable, QUIC_ARRAYSIZE(retransmittable)); - // Stop retransmissions on another stream and verify the packet is unchanged. - if (!unacked_packets_.session_decides_what_to_write()) { - unacked_packets_.CancelRetransmissionsForStream(stream_id + 2); - } VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); VerifyInFlightPackets(unacked, QUIC_ARRAYSIZE(unacked)); VerifyRetransmittablePackets(retransmittable, @@ -298,26 +247,16 @@ TEST_P(QuicUnackedPacketMapTest, StopRetransmissionAfterRetransmission) { const QuicStreamId stream_id = 2; SerializedPacket packet1(CreateRetransmittablePacketForStream(1, stream_id)); - unacked_packets_.AddSentPacket(&packet1, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet1, NOT_RETRANSMISSION, now_, true); RetransmitAndSendPacket(1, 2, LOSS_RETRANSMISSION); uint64_t unacked[] = {1, 2}; VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); VerifyInFlightPackets(unacked, QUIC_ARRAYSIZE(unacked)); - std::vector<uint64_t> retransmittable; - if (unacked_packets_.session_decides_what_to_write()) { - retransmittable = {1, 2}; - } else { - retransmittable = {2}; - } + std::vector<uint64_t> retransmittable = {1, 2}; VerifyRetransmittablePackets(&retransmittable[0], retransmittable.size()); - if (unacked_packets_.session_decides_what_to_write()) { - EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); - } else { - unacked_packets_.CancelRetransmissionsForStream(stream_id); - } + EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); VerifyInFlightPackets(unacked, QUIC_ARRAYSIZE(unacked)); VerifyRetransmittablePackets(nullptr, 0); @@ -327,19 +266,13 @@ // Simulate a retransmittable packet being sent, retransmitted, and the first // transmission being acked. SerializedPacket packet1(CreateRetransmittablePacket(1)); - unacked_packets_.AddSentPacket(&packet1, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet1, NOT_RETRANSMISSION, now_, true); RetransmitAndSendPacket(1, 2, LOSS_RETRANSMISSION); uint64_t unacked[] = {1, 2}; VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); VerifyInFlightPackets(unacked, QUIC_ARRAYSIZE(unacked)); - std::vector<uint64_t> retransmittable; - if (unacked_packets_.session_decides_what_to_write()) { - retransmittable = {1, 2}; - } else { - retransmittable = {2}; - } + std::vector<uint64_t> retransmittable = {1, 2}; VerifyRetransmittablePackets(&retransmittable[0], retransmittable.size()); EXPECT_CALL(notifier_, IsFrameOutstanding(_)).WillRepeatedly(Return(false)); @@ -368,11 +301,9 @@ TEST_P(QuicUnackedPacketMapTest, RetransmitThreeTimes) { // Simulate a retransmittable packet being sent and retransmitted twice. SerializedPacket packet1(CreateRetransmittablePacket(1)); - unacked_packets_.AddSentPacket(&packet1, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet1, NOT_RETRANSMISSION, now_, true); SerializedPacket packet2(CreateRetransmittablePacket(2)); - unacked_packets_.AddSentPacket(&packet2, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet2, NOT_RETRANSMISSION, now_, true); uint64_t unacked[] = {1, 2}; VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); @@ -388,19 +319,13 @@ unacked_packets_.RemoveFromInFlight(QuicPacketNumber(1)); RetransmitAndSendPacket(1, 3, LOSS_RETRANSMISSION); SerializedPacket packet4(CreateRetransmittablePacket(4)); - unacked_packets_.AddSentPacket(&packet4, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet4, NOT_RETRANSMISSION, now_, true); uint64_t unacked2[] = {1, 3, 4}; VerifyUnackedPackets(unacked2, QUIC_ARRAYSIZE(unacked2)); uint64_t pending2[] = {3, 4}; VerifyInFlightPackets(pending2, QUIC_ARRAYSIZE(pending2)); - std::vector<uint64_t> retransmittable2; - if (unacked_packets_.session_decides_what_to_write()) { - retransmittable2 = {1, 3, 4}; - } else { - retransmittable2 = {3, 4}; - } + std::vector<uint64_t> retransmittable2 = {1, 3, 4}; VerifyRetransmittablePackets(&retransmittable2[0], retransmittable2.size()); // Early retransmit 3 (formerly 1) as 5, and remove 1 from unacked. @@ -409,18 +334,10 @@ unacked_packets_.RemoveRetransmittability(QuicPacketNumber(4)); RetransmitAndSendPacket(3, 5, LOSS_RETRANSMISSION); SerializedPacket packet6(CreateRetransmittablePacket(6)); - unacked_packets_.AddSentPacket(&packet6, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet6, NOT_RETRANSMISSION, now_, true); - std::vector<uint64_t> unacked3; - std::vector<uint64_t> retransmittable3; - if (unacked_packets_.session_decides_what_to_write()) { - unacked3 = {3, 5, 6}; - retransmittable3 = {3, 5, 6}; - } else { - unacked3 = {3, 5, 6}; - retransmittable3 = {5, 6}; - } + std::vector<uint64_t> unacked3 = {3, 5, 6}; + std::vector<uint64_t> retransmittable3 = {3, 5, 6}; VerifyUnackedPackets(&unacked3[0], unacked3.size()); VerifyRetransmittablePackets(&retransmittable3[0], retransmittable3.size()); uint64_t pending3[] = {3, 5, 6}; @@ -432,15 +349,8 @@ unacked_packets_.RemoveRetransmittability(QuicPacketNumber(6)); RetransmitAndSendPacket(5, 7, LOSS_RETRANSMISSION); - std::vector<uint64_t> unacked4; - std::vector<uint64_t> retransmittable4; - if (unacked_packets_.session_decides_what_to_write()) { - unacked4 = {3, 5, 7}; - retransmittable4 = {3, 5, 7}; - } else { - unacked4 = {3, 5, 7}; - retransmittable4 = {7}; - } + std::vector<uint64_t> unacked4 = {3, 5, 7}; + std::vector<uint64_t> retransmittable4 = {3, 5, 7}; VerifyUnackedPackets(&unacked4[0], unacked4.size()); VerifyRetransmittablePackets(&retransmittable4[0], retransmittable4.size()); uint64_t pending4[] = {3, 5, 7}; @@ -456,11 +366,9 @@ TEST_P(QuicUnackedPacketMapTest, RetransmitFourTimes) { // Simulate a retransmittable packet being sent and retransmitted twice. SerializedPacket packet1(CreateRetransmittablePacket(1)); - unacked_packets_.AddSentPacket(&packet1, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet1, NOT_RETRANSMISSION, now_, true); SerializedPacket packet2(CreateRetransmittablePacket(2)); - unacked_packets_.AddSentPacket(&packet2, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet2, NOT_RETRANSMISSION, now_, true); uint64_t unacked[] = {1, 2}; VerifyUnackedPackets(unacked, QUIC_ARRAYSIZE(unacked)); @@ -480,30 +388,19 @@ VerifyUnackedPackets(unacked2, QUIC_ARRAYSIZE(unacked2)); uint64_t pending2[] = {3}; VerifyInFlightPackets(pending2, QUIC_ARRAYSIZE(pending2)); - std::vector<uint64_t> retransmittable2; - if (unacked_packets_.session_decides_what_to_write()) { - retransmittable2 = {1, 3}; - } else { - retransmittable2 = {3}; - } + std::vector<uint64_t> retransmittable2 = {1, 3}; VerifyRetransmittablePackets(&retransmittable2[0], retransmittable2.size()); // TLP 3 (formerly 1) as 4, and don't remove 1 from unacked. RetransmitAndSendPacket(3, 4, TLP_RETRANSMISSION); SerializedPacket packet5(CreateRetransmittablePacket(5)); - unacked_packets_.AddSentPacket(&packet5, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet5, NOT_RETRANSMISSION, now_, true); uint64_t unacked3[] = {1, 3, 4, 5}; VerifyUnackedPackets(unacked3, QUIC_ARRAYSIZE(unacked3)); uint64_t pending3[] = {3, 4, 5}; VerifyInFlightPackets(pending3, QUIC_ARRAYSIZE(pending3)); - std::vector<uint64_t> retransmittable3; - if (unacked_packets_.session_decides_what_to_write()) { - retransmittable3 = {1, 3, 4, 5}; - } else { - retransmittable3 = {4, 5}; - } + std::vector<uint64_t> retransmittable3 = {1, 3, 4, 5}; VerifyRetransmittablePackets(&retransmittable3[0], retransmittable3.size()); // Early retransmit 4 as 6 and ensure in flight packet 3 is removed. @@ -514,21 +411,11 @@ unacked_packets_.RemoveFromInFlight(QuicPacketNumber(4)); RetransmitAndSendPacket(4, 6, LOSS_RETRANSMISSION); - std::vector<uint64_t> unacked4; - if (unacked_packets_.session_decides_what_to_write()) { - unacked4 = {4, 6}; - } else { - unacked4 = {4, 6}; - } + std::vector<uint64_t> unacked4 = {4, 6}; VerifyUnackedPackets(&unacked4[0], unacked4.size()); uint64_t pending4[] = {6}; VerifyInFlightPackets(pending4, QUIC_ARRAYSIZE(pending4)); - std::vector<uint64_t> retransmittable4; - if (unacked_packets_.session_decides_what_to_write()) { - retransmittable4 = {4, 6}; - } else { - retransmittable4 = {6}; - } + std::vector<uint64_t> retransmittable4 = {4, 6}; VerifyRetransmittablePackets(&retransmittable4[0], retransmittable4.size()); } @@ -536,11 +423,9 @@ // Simulate a retransmittable packet being sent, retransmitted, and the first // transmission being acked. SerializedPacket packet1(CreateRetransmittablePacket(1)); - unacked_packets_.AddSentPacket(&packet1, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet1, NOT_RETRANSMISSION, now_, true); SerializedPacket packet3(CreateRetransmittablePacket(3)); - unacked_packets_.AddSentPacket(&packet3, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet3, NOT_RETRANSMISSION, now_, true); RetransmitAndSendPacket(3, 5, LOSS_RETRANSMISSION); EXPECT_EQ(QuicPacketNumber(1u), unacked_packets_.GetLeastUnacked()); @@ -685,8 +570,7 @@ // Send packet 1. SerializedPacket packet1(CreateRetransmittablePacket(1)); packet1.encryption_level = ENCRYPTION_INITIAL; - unacked_packets_.AddSentPacket(&packet1, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet1, NOT_RETRANSMISSION, now_, true); EXPECT_EQ(QuicPacketNumber(1u), unacked_packets_.largest_sent_packet()); EXPECT_EQ(QuicPacketNumber(1), unacked_packets_.GetLargestSentPacketOfPacketNumberSpace( @@ -698,8 +582,7 @@ // Send packet 2. SerializedPacket packet2(CreateRetransmittablePacket(2)); packet2.encryption_level = ENCRYPTION_HANDSHAKE; - unacked_packets_.AddSentPacket(&packet2, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet2, NOT_RETRANSMISSION, now_, true); EXPECT_EQ(QuicPacketNumber(2u), unacked_packets_.largest_sent_packet()); EXPECT_EQ(QuicPacketNumber(1), unacked_packets_.GetLargestSentPacketOfPacketNumberSpace( @@ -713,8 +596,7 @@ // Send packet 3. SerializedPacket packet3(CreateRetransmittablePacket(3)); packet3.encryption_level = ENCRYPTION_ZERO_RTT; - unacked_packets_.AddSentPacket(&packet3, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet3, NOT_RETRANSMISSION, now_, true); EXPECT_EQ(QuicPacketNumber(3u), unacked_packets_.largest_sent_packet()); EXPECT_EQ(QuicPacketNumber(1), unacked_packets_.GetLargestSentPacketOfPacketNumberSpace( @@ -734,8 +616,7 @@ // Send packet 4. SerializedPacket packet4(CreateRetransmittablePacket(4)); packet4.encryption_level = ENCRYPTION_FORWARD_SECURE; - unacked_packets_.AddSentPacket(&packet4, QuicPacketNumber(), - NOT_RETRANSMISSION, now_, true); + unacked_packets_.AddSentPacket(&packet4, NOT_RETRANSMISSION, now_, true); EXPECT_EQ(QuicPacketNumber(4u), unacked_packets_.largest_sent_packet()); EXPECT_EQ(QuicPacketNumber(1), unacked_packets_.GetLargestSentPacketOfPacketNumberSpace(
diff --git a/quic/test_tools/quic_connection_peer.cc b/quic/test_tools/quic_connection_peer.cc index 02775ca..6276b2e 100644 --- a/quic/test_tools/quic_connection_peer.cc +++ b/quic/test_tools/quic_connection_peer.cc
@@ -308,13 +308,6 @@ } // static -void QuicConnectionPeer::SetSessionDecidesWhatToWrite( - QuicConnection* connection) { - connection->sent_packet_manager_.SetSessionDecideWhatToWrite(true); - connection->packet_generator_.SetCanSetTransmissionType(true); -} - -// static void QuicConnectionPeer::SetNegotiatedVersion(QuicConnection* connection) { connection->version_negotiated_ = true; }
diff --git a/quic/test_tools/quic_connection_peer.h b/quic/test_tools/quic_connection_peer.h index c5c972f..a0e47ca 100644 --- a/quic/test_tools/quic_connection_peer.h +++ b/quic/test_tools/quic_connection_peer.h
@@ -127,7 +127,6 @@ bool no_stop_waiting_frames); static void SetMaxTrackedPackets(QuicConnection* connection, QuicPacketCount max_tracked_packets); - static void SetSessionDecidesWhatToWrite(QuicConnection* connection); static void SetNegotiatedVersion(QuicConnection* connection); static void SetMaxConsecutiveNumPacketsWithNoRetransmittableFrames( QuicConnection* connection,
diff --git a/quic/test_tools/quic_sent_packet_manager_peer.cc b/quic/test_tools/quic_sent_packet_manager_peer.cc index b680f66..7b3977e 100644 --- a/quic/test_tools/quic_sent_packet_manager_peer.cc +++ b/quic/test_tools/quic_sent_packet_manager_peer.cc
@@ -86,18 +86,9 @@ if (!HasRetransmittableFrames(sent_packet_manager, packet_number)) { return false; } - if (sent_packet_manager->session_decides_what_to_write()) { - return sent_packet_manager->unacked_packets_ - .GetTransmissionInfo(QuicPacketNumber(packet_number)) - .transmission_type != NOT_RETRANSMISSION; - } - for (auto transmission_info : sent_packet_manager->unacked_packets_) { - if (transmission_info.retransmission.IsInitialized() && - transmission_info.retransmission == QuicPacketNumber(packet_number)) { - return true; - } - } - return false; + return sent_packet_manager->unacked_packets_ + .GetTransmissionInfo(QuicPacketNumber(packet_number)) + .transmission_type != NOT_RETRANSMISSION; } // static
diff --git a/quic/test_tools/quic_test_utils.cc b/quic/test_tools/quic_test_utils.cc index ff02392..dbc5963 100644 --- a/quic/test_tools/quic_test_utils.cc +++ b/quic/test_tools/quic_test_utils.cc
@@ -531,7 +531,7 @@ // Transfer ownership of the packet to the SentPacketManager and the // ack notifier to the AckNotifierManager. QuicConnectionPeer::GetSentPacketManager(this)->OnPacketSent( - packet, QuicPacketNumber(), clock_.ApproximateNow(), NOT_RETRANSMISSION, + packet, clock_.ApproximateNow(), NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); }
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h index be6e84d..9f8f23c 100644 --- a/quic/test_tools/quic_test_utils.h +++ b/quic/test_tools/quic_test_utils.h
@@ -1014,11 +1014,8 @@ MOCK_METHOD1(OnFrameAddedToPacket, void(const QuicFrame&)); - MOCK_METHOD4(OnPacketSent, - void(const SerializedPacket&, - QuicPacketNumber, - TransmissionType, - QuicTime)); + MOCK_METHOD3(OnPacketSent, + void(const SerializedPacket&, TransmissionType, QuicTime)); MOCK_METHOD0(OnPingSent, void());
diff --git a/quic/test_tools/simple_session_notifier_test.cc b/quic/test_tools/simple_session_notifier_test.cc index 93f11aa..32dbce4 100644 --- a/quic/test_tools/simple_session_notifier_test.cc +++ b/quic/test_tools/simple_session_notifier_test.cc
@@ -42,7 +42,6 @@ : connection_(&helper_, &alarm_factory_, Perspective::IS_CLIENT), notifier_(&connection_) { connection_.set_visitor(&visitor_); - QuicConnectionPeer::SetSessionDecidesWhatToWrite(&connection_); EXPECT_FALSE(notifier_.WillingToWrite()); EXPECT_EQ(0u, notifier_.StreamBytesSent()); EXPECT_FALSE(notifier_.HasBufferedStreamData());
diff --git a/quic/test_tools/simulator/quic_endpoint.cc b/quic/test_tools/simulator/quic_endpoint.cc index bd01c43..bf84b37 100644 --- a/quic/test_tools/simulator/quic_endpoint.cc +++ b/quic/test_tools/simulator/quic_endpoint.cc
@@ -103,9 +103,7 @@ } connection_.SetDataProducer(&producer_); connection_.SetSessionNotifier(this); - if (connection_.session_decides_what_to_write()) { - notifier_ = std::make_unique<test::SimpleSessionNotifier>(&connection_); - } + notifier_ = std::make_unique<test::SimpleSessionNotifier>(&connection_); // Configure the connection as if it received a handshake. This is important // primarily because