gfe-relnote: In QUIC, clean up !session_decides_what_to_write code path. PiperOrigin-RevId: 276061544 Change-Id: If57489805bb2f6d038d6a2ff2b2d21c141742735
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;