diff options
Diffstat (limited to 'net')
30 files changed, 544 insertions, 219 deletions
diff --git a/net/quic/congestion_control/tcp_cubic_sender.cc b/net/quic/congestion_control/tcp_cubic_sender.cc index a267438..89c240b 100644 --- a/net/quic/congestion_control/tcp_cubic_sender.cc +++ b/net/quic/congestion_control/tcp_cubic_sender.cc @@ -168,6 +168,7 @@ QuicTime::Delta TcpCubicSender::TimeUntilSend( HasRetransmittableData has_retransmittable_data, IsHandshake handshake) { if (transmission_type == TLP_RETRANSMISSION || + transmission_type == HANDSHAKE_RETRANSMISSION || has_retransmittable_data == NO_RETRANSMITTABLE_DATA || handshake == IS_HANDSHAKE) { // For TCP we can always send an ACK immediately. diff --git a/net/quic/congestion_control/tcp_cubic_sender_test.cc b/net/quic/congestion_control/tcp_cubic_sender_test.cc index fe3e4fd..1df3b36 100644 --- a/net/quic/congestion_control/tcp_cubic_sender_test.cc +++ b/net/quic/congestion_control/tcp_cubic_sender_test.cc @@ -9,6 +9,7 @@ #include "net/quic/congestion_control/rtt_stats.h" #include "net/quic/congestion_control/tcp_cubic_sender.h" #include "net/quic/congestion_control/tcp_receiver.h" +#include "net/quic/quic_utils.h" #include "net/quic/test_tools/mock_clock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -120,8 +121,29 @@ TEST_F(TcpCubicSenderTest, SimpleSender) { EXPECT_EQ(kDefaultWindowTCP, sender_->GetCongestionWindow()); // A retransmit should always return 0. - EXPECT_TRUE(sender_->TimeUntilSend(clock_.Now(), - NACK_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA, NOT_HANDSHAKE).IsZero()); + for (int i = FIRST_TRANSMISSION_TYPE; i <= LAST_TRANSMISSION_TYPE; ++i) { + TransmissionType type = static_cast<TransmissionType>(i); + EXPECT_TRUE(sender_->TimeUntilSend(clock_.Now(), + type, + HAS_RETRANSMITTABLE_DATA, + NOT_HANDSHAKE).IsZero()) + << QuicUtils::TransmissionTypeToString(type); + } + + // Fill the send window with data, then verify that we can still + // send handshake and TLP packets. + SendAvailableSendWindow(); + for (int i = FIRST_TRANSMISSION_TYPE; i <= LAST_TRANSMISSION_TYPE; ++i) { + TransmissionType type = static_cast<TransmissionType>(i); + bool expect_can_send = (type == HANDSHAKE_RETRANSMISSION || + type == TLP_RETRANSMISSION); + EXPECT_EQ(expect_can_send, + sender_->TimeUntilSend(clock_.Now(), + type, + HAS_RETRANSMITTABLE_DATA, + NOT_HANDSHAKE).IsZero()) + << QuicUtils::TransmissionTypeToString(type); + } } TEST_F(TcpCubicSenderTest, ExponentialSlowStart) { diff --git a/net/quic/congestion_control/time_loss_algorithm_test.cc b/net/quic/congestion_control/time_loss_algorithm_test.cc index 5c1b383..43969a9 100644 --- a/net/quic/congestion_control/time_loss_algorithm_test.cc +++ b/net/quic/congestion_control/time_loss_algorithm_test.cc @@ -55,7 +55,7 @@ TEST_F(TimeLossAlgorithmTest, NoLossFor500Nacks) { SendDataPacket(i); } unacked_packets_.SetNotPending(2); - for (size_t i = 0; i < 500; ++i) { + for (size_t i = 1; i < 500; ++i) { unacked_packets_.NackPacket(1, i); VerifyLosses(2, NULL, 0); } diff --git a/net/quic/crypto/proof_test.cc b/net/quic/crypto/proof_test.cc index cc9e099..df68dd0 100644 --- a/net/quic/crypto/proof_test.cc +++ b/net/quic/crypto/proof_test.cc @@ -100,13 +100,13 @@ class TestProofVerifierCallback : public ProofVerifierCallback { public: TestProofVerifierCallback(TestCompletionCallback* comp_callback, bool* ok, - std::string* error_details) + string* error_details) : comp_callback_(comp_callback), ok_(ok), error_details_(error_details) {} virtual void Run(bool ok, - const std::string& error_details, + const string& error_details, scoped_ptr<ProofVerifyDetails>* details) OVERRIDE { *ok_ = ok; *error_details_ = error_details; @@ -117,21 +117,21 @@ class TestProofVerifierCallback : public ProofVerifierCallback { private: TestCompletionCallback* const comp_callback_; bool* const ok_; - std::string* const error_details_; + string* const error_details_; }; // RunVerification runs |verifier->VerifyProof| and asserts that the result // matches |expected_ok|. static void RunVerification(ProofVerifier* verifier, - const std::string& hostname, - const std::string& server_config, - const vector<std::string>& certs, - const std::string& proof, + const string& hostname, + const string& server_config, + const vector<string>& certs, + const string& proof, bool expected_ok) { scoped_ptr<ProofVerifyDetails> details; TestCompletionCallback comp_callback; bool ok; - std::string error_details; + string error_details; TestProofVerifierCallback* callback = new TestProofVerifierCallback(&comp_callback, &ok, &error_details); diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc index 01c4020..485d48b 100644 --- a/net/quic/quic_connection_test.cc +++ b/net/quic/quic_connection_test.cc @@ -1148,7 +1148,7 @@ TEST_P(QuicConnectionTest, AckReceiptCausesAckSend) { QuicPacketSequenceNumber retransmission; EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, packet_size - kQuicVersionSize, - NACK_RETRANSMISSION, _)) + LOSS_RETRANSMISSION, _)) .WillOnce(DoAll(SaveArg<1>(&retransmission), Return(true))); ProcessAckPacket(&frame); @@ -1848,7 +1848,7 @@ TEST_P(QuicConnectionTest, RetransmitOnNack) { EXPECT_CALL(*send_algorithm_, OnPacketAbandoned(2, _)).Times(1); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, second_packet_size - kQuicVersionSize, - NACK_RETRANSMISSION, _)).Times(1); + LOSS_RETRANSMISSION, _)).Times(1); ProcessAckPacket(&nack_two); } @@ -1916,7 +1916,7 @@ TEST_P(QuicConnectionTest, RetransmitNackedLargestObserved) { EXPECT_CALL(*send_algorithm_, OnPacketAbandoned(1, _)).Times(1); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, packet_size - kQuicVersionSize, - NACK_RETRANSMISSION, _)); + LOSS_RETRANSMISSION, _)); ProcessAckPacket(&frame); } @@ -2408,7 +2408,7 @@ TEST_P(QuicConnectionTest, RetransmissionCountCalculation) { // retransmissions. (More ack packets). EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, NOT_RETRANSMISSION, _)) .Times(AnyNumber()); - EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, NACK_RETRANSMISSION, _)) + EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, LOSS_RETRANSMISSION, _)) .WillOnce(DoAll(SaveArg<1>(&nack_sequence_number), Return(true))); QuicAckFrame ack = InitAckFrame(rto_sequence_number, 0); // Nack the retransmitted packet. @@ -2632,7 +2632,7 @@ TEST_P(QuicConnectionTest, SendSchedulerForce) { // Test that if we force send a packet, it is not queued. QuicPacket* packet = ConstructDataPacket(1, 0, !kEntropyFlag); EXPECT_CALL(*send_algorithm_, - TimeUntilSend(_, NACK_RETRANSMISSION, _, _)).Times(0); + TimeUntilSend(_, LOSS_RETRANSMISSION, _, _)).Times(0); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)); connection_.SendPacket( ENCRYPTION_NONE, 1, packet, kTestEntropyHash, HAS_RETRANSMITTABLE_DATA); @@ -3391,7 +3391,7 @@ TEST_P(QuicConnectionTest, CheckSendStats) { EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, RTO_RETRANSMISSION, _)).Times(2); EXPECT_CALL(*send_algorithm_, - OnPacketSent(_, _, _, NACK_RETRANSMISSION, _)); + OnPacketSent(_, _, _, LOSS_RETRANSMISSION, _)); EXPECT_CALL(*send_algorithm_, OnPacketAbandoned(_, _)).Times(1); // Retransmit due to RTO. @@ -3679,6 +3679,98 @@ TEST_P(QuicConnectionTest, AckNotifierCallbackAfterRetransmission) { ProcessAckPacket(&second_ack_frame); } +// AckNotifierCallback is triggered by the ack of a packet that timed +// out and was retransmitted, even though the retransmission has a +// different sequence number. +TEST_P(QuicConnectionTest, AckNotifierCallbackForAckAfterRTO) { + InSequence s; + + // Create a delegate which we expect to be called. + scoped_refptr<MockAckNotifierDelegate> delegate( + new StrictMock<MockAckNotifierDelegate>); + + QuicTime default_retransmission_time = clock_.ApproximateNow().Add( + DefaultRetransmissionTime()); + connection_.SendStreamDataWithString(3, "foo", 0, !kFin, delegate.get()); + EXPECT_EQ(1u, outgoing_ack()->sent_info.least_unacked); + + EXPECT_EQ(1u, last_header()->packet_sequence_number); + EXPECT_EQ(default_retransmission_time, + connection_.GetRetransmissionAlarm()->deadline()); + // Simulate the retransmission alarm firing. + clock_.AdvanceTime(DefaultRetransmissionTime()); + EXPECT_CALL(*send_algorithm_, OnRetransmissionTimeout(true)); + EXPECT_CALL(*send_algorithm_, OnPacketSent(_, 2u, _, _, _)); + connection_.GetRetransmissionAlarm()->Fire(); + EXPECT_EQ(2u, last_header()->packet_sequence_number); + // We do not raise the high water mark yet. + EXPECT_EQ(1u, outgoing_ack()->sent_info.least_unacked); + + // Ack the original packet. + EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); + EXPECT_CALL(*send_algorithm_, UpdateRtt(_)); + EXPECT_CALL(*delegate, OnAckNotification(1, _, 1, _)); + QuicAckFrame ack_frame = InitAckFrame(1, 0); + ProcessAckPacket(&ack_frame); + + // Delegate is not notified again when the retransmit is acked. + EXPECT_CALL(*send_algorithm_, UpdateRtt(_)); + EXPECT_CALL(*send_algorithm_, OnPacketAcked(2, _)); + QuicAckFrame second_ack_frame = InitAckFrame(2, 0); + ProcessAckPacket(&second_ack_frame); +} + +// AckNotifierCallback is triggered by the ack of a packet that was +// previously nacked, even though the retransmission has a different +// sequence number. +TEST_P(QuicConnectionTest, AckNotifierCallbackForAckOfNackedPacket) { + InSequence s; + + // Create a delegate which we expect to be called. + scoped_refptr<MockAckNotifierDelegate> delegate( + new StrictMock<MockAckNotifierDelegate>); + + // Send four packets, and register to be notified on ACK of packet 2. + connection_.SendStreamDataWithString(3, "foo", 0, !kFin, NULL); + connection_.SendStreamDataWithString(3, "bar", 0, !kFin, delegate.get()); + connection_.SendStreamDataWithString(3, "baz", 0, !kFin, NULL); + connection_.SendStreamDataWithString(3, "qux", 0, !kFin, NULL); + + // Now we receive ACK for packets 1, 3, and 4 and lose 2. + QuicAckFrame frame = InitAckFrame(4, 0); + NackPacket(2, &frame); + SequenceNumberSet lost_packets; + lost_packets.insert(2); + EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); + EXPECT_CALL(*send_algorithm_, UpdateRtt(_)); + EXPECT_CALL(*send_algorithm_, OnPacketAcked(1, _)); + EXPECT_CALL(*send_algorithm_, OnPacketAcked(3, _)); + EXPECT_CALL(*send_algorithm_, OnPacketAcked(4, _)); + EXPECT_CALL(*loss_algorithm_, DetectLostPackets(_, _, _, _)) + .WillOnce(Return(lost_packets)); + EXPECT_CALL(*send_algorithm_, OnPacketLost(2, _)); + EXPECT_CALL(*send_algorithm_, OnPacketAbandoned(2, _)); + EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)); + ProcessAckPacket(&frame); + + // Now we get an ACK for packet 2, which was previously nacked. + SequenceNumberSet no_lost_packets; + EXPECT_CALL(*delegate, OnAckNotification(1, _, 1, _)); + EXPECT_CALL(*loss_algorithm_, DetectLostPackets(_, _, _, _)) + .WillOnce(Return(no_lost_packets)); + QuicAckFrame second_ack_frame = InitAckFrame(4, 0); + ProcessAckPacket(&second_ack_frame); + + // Verify that the delegate is not notified again when the + // retransmit is acked. + EXPECT_CALL(*send_algorithm_, UpdateRtt(_)); + EXPECT_CALL(*send_algorithm_, OnPacketAcked(5, _)); + EXPECT_CALL(*loss_algorithm_, DetectLostPackets(_, _, _, _)) + .WillOnce(Return(no_lost_packets)); + QuicAckFrame third_ack_frame = InitAckFrame(5, 0); + ProcessAckPacket(&third_ack_frame); +} + // TODO(rjshade): Add a similar test that FEC recovery on peer (and resulting // ACK) triggers notification on our end. TEST_P(QuicConnectionTest, AckNotifierCallbackAfterFECRecovery) { diff --git a/net/quic/quic_crypto_stream.cc b/net/quic/quic_crypto_stream.cc index 4baf236..414c2e2 100644 --- a/net/quic/quic_crypto_stream.cc +++ b/net/quic/quic_crypto_stream.cc @@ -60,7 +60,7 @@ void QuicCryptoStream::SendHandshakeMessage( // any other frames in a single packet. session()->connection()->Flush(); // TODO(wtc): check the return value. - WriteOrBufferData(string(data.data(), data.length()), false); + WriteOrBufferData(string(data.data(), data.length()), false, NULL); session()->connection()->Flush(); } diff --git a/net/quic/quic_data_stream.cc b/net/quic/quic_data_stream.cc index c7824b4..5e0628a 100644 --- a/net/quic/quic_data_stream.cc +++ b/net/quic/quic_data_stream.cc @@ -43,9 +43,12 @@ QuicDataStream::QuicDataStream(QuicStreamId id, QuicDataStream::~QuicDataStream() { } -size_t QuicDataStream::WriteHeaders(const SpdyHeaderBlock& header_block, - bool fin) { - size_t bytes_written = session()->WriteHeaders(id(), header_block, fin); +size_t QuicDataStream::WriteHeaders( + const SpdyHeaderBlock& header_block, + bool fin, + QuicAckNotifier::DelegateInterface* ack_notifier_delegate) { + size_t bytes_written = session()->WriteHeaders( + id(), header_block, fin, ack_notifier_delegate); if (fin) { // TODO(rch): Add test to ensure fin_sent_ is set whenever a fin is sent. set_fin_sent(true); diff --git a/net/quic/quic_data_stream.h b/net/quic/quic_data_stream.h index adbc707..35b485c 100644 --- a/net/quic/quic_data_stream.h +++ b/net/quic/quic_data_stream.h @@ -86,8 +86,10 @@ class NET_EXPORT_PRIVATE QuicDataStream : public ReliableQuicStream { // Writes the headers contained in |header_block| to the dedicated // headers stream. - virtual size_t WriteHeaders(const SpdyHeaderBlock& header_block, - bool fin); + virtual size_t WriteHeaders( + const SpdyHeaderBlock& header_block, + bool fin, + QuicAckNotifier::DelegateInterface* ack_notifier_delegate); // This block of functions wraps the sequencer's functions of the same // name. These methods return uncompressed data until that has diff --git a/net/quic/quic_headers_stream.cc b/net/quic/quic_headers_stream.cc index 1712a2e..f3a28c6 100644 --- a/net/quic/quic_headers_stream.cc +++ b/net/quic/quic_headers_stream.cc @@ -178,9 +178,11 @@ QuicHeadersStream::QuicHeadersStream(QuicSession* session) QuicHeadersStream::~QuicHeadersStream() {} -size_t QuicHeadersStream::WriteHeaders(QuicStreamId stream_id, - const SpdyHeaderBlock& headers, - bool fin) { +size_t QuicHeadersStream::WriteHeaders( + QuicStreamId stream_id, + const SpdyHeaderBlock& headers, + bool fin, + QuicAckNotifier::DelegateInterface* ack_notifier_delegate) { scoped_ptr<SpdySerializedFrame> frame; if (session()->is_server()) { SpdySynReplyIR syn_reply(stream_id); @@ -193,7 +195,8 @@ size_t QuicHeadersStream::WriteHeaders(QuicStreamId stream_id, syn_stream.set_fin(fin); frame.reset(spdy_framer_.SerializeFrame(syn_stream)); } - WriteOrBufferData(StringPiece(frame->data(), frame->size()), false); + WriteOrBufferData(StringPiece(frame->data(), frame->size()), false, + ack_notifier_delegate); return frame->size(); } diff --git a/net/quic/quic_headers_stream.h b/net/quic/quic_headers_stream.h index 21bb61e..c3ccbda 100644 --- a/net/quic/quic_headers_stream.h +++ b/net/quic/quic_headers_stream.h @@ -27,9 +27,11 @@ class NET_EXPORT_PRIVATE QuicHeadersStream : public ReliableQuicStream { // frame to the peer. If |fin| is true, the fin flag will be set on // the SPDY frame. Returns the size, in bytes, of the resulting // SPDY frame. - size_t WriteHeaders(QuicStreamId stream_id, - const SpdyHeaderBlock& headers, - bool fin); + size_t WriteHeaders( + QuicStreamId stream_id, + const SpdyHeaderBlock& headers, + bool fin, + QuicAckNotifier::DelegateInterface* ack_notifier_delegate); // ReliableQuicStream implementation virtual uint32 ProcessRawData(const char* data, uint32 data_len) OVERRIDE; diff --git a/net/quic/quic_headers_stream_test.cc b/net/quic/quic_headers_stream_test.cc index 61ea335..bec7cb8 100644 --- a/net/quic/quic_headers_stream_test.cc +++ b/net/quic/quic_headers_stream_test.cc @@ -121,7 +121,7 @@ class QuicHeadersStreamTest : public ::testing::TestWithParam<bool> { // Write the headers and capture the outgoing data EXPECT_CALL(session_, WritevData(kHeadersStreamId, _, _, false, NULL)) .WillOnce(WithArgs<1>(Invoke(this, &QuicHeadersStreamTest::SaveIov))); - headers_stream_->WriteHeaders(stream_id, headers_, fin); + headers_stream_->WriteHeaders(stream_id, headers_, fin, NULL); // Parse the outgoing data and check that it matches was was written. if (type == SYN_STREAM) { diff --git a/net/quic/quic_http_stream.cc b/net/quic/quic_http_stream.cc index 2ceac95..3c96fbe 100644 --- a/net/quic/quic_http_stream.cc +++ b/net/quic/quic_http_stream.cc @@ -431,7 +431,7 @@ int QuicHttpStream::DoSendHeaders() { bool has_upload_data = request_body_stream_ != NULL; next_state_ = STATE_SEND_HEADERS_COMPLETE; - int rv = stream_->WriteHeaders(request_headers_, !has_upload_data); + int rv = stream_->WriteHeaders(request_headers_, !has_upload_data, NULL); request_headers_.clear(); return rv; } diff --git a/net/quic/quic_protocol.h b/net/quic/quic_protocol.h index 7f4392e..b4666dc 100644 --- a/net/quic/quic_protocol.h +++ b/net/quic/quic_protocol.h @@ -126,9 +126,13 @@ const uint64 kUFloat16MaxValue = // 0x3FFC0000000 enum TransmissionType { NOT_RETRANSMISSION, - NACK_RETRANSMISSION, - RTO_RETRANSMISSION, - TLP_RETRANSMISSION, + FIRST_TRANSMISSION_TYPE = NOT_RETRANSMISSION, + HANDSHAKE_RETRANSMISSION, // Retransmits due to handshake timeouts. + ALL_UNACKED_RETRANSMISSION, // Retransmits of all unacked packets. + LOSS_RETRANSMISSION, // Retransmits due to loss detection. + RTO_RETRANSMISSION, // Retransmits due to retransmit time out. + TLP_RETRANSMISSION, // Tail loss probes. + LAST_TRANSMISSION_TYPE = TLP_RETRANSMISSION, }; enum RetransmissionType { diff --git a/net/quic/quic_reliable_client_stream.cc b/net/quic/quic_reliable_client_stream.cc index 996a5c5..36296c3 100644 --- a/net/quic/quic_reliable_client_stream.cc +++ b/net/quic/quic_reliable_client_stream.cc @@ -69,7 +69,7 @@ int QuicReliableClientStream::WriteStreamData( // We should not have data buffered. DCHECK(!HasBufferedData()); // Writes the data, or buffers it. - WriteOrBufferData(data, fin); + WriteOrBufferData(data, fin, NULL); if (!HasBufferedData()) { return OK; } diff --git a/net/quic/quic_sent_packet_manager.cc b/net/quic/quic_sent_packet_manager.cc index 8592ea7..bd8efb1 100644 --- a/net/quic/quic_sent_packet_manager.cc +++ b/net/quic/quic_sent_packet_manager.cc @@ -4,6 +4,8 @@ #include "net/quic/quic_sent_packet_manager.h" +#include <algorithm> + #include "base/logging.h" #include "base/stl_util.h" #include "net/quic/congestion_control/pacing_sender.h" @@ -177,9 +179,6 @@ void QuicSentPacketManager::HandleAckForSentPackets( // If data is associated with the most recent transmission of this // packet, then inform the caller. it = MarkPacketHandled(sequence_number, RECEIVED_BY_PEER); - - // The AckNotifierManager is informed of every ACKed sequence number. - ack_notifier_manager_.OnPacketAcked(sequence_number); } // Discard any retransmittable frames associated with revived packets. @@ -231,7 +230,7 @@ void QuicSentPacketManager::RetransmitUnackedPackets( if (frames != NULL && (retransmission_type == ALL_PACKETS || frames->encryption_level() == ENCRYPTION_INITIAL)) { OnPacketAbandoned(unacked_it->first); - MarkForRetransmission(unacked_it->first, NACK_RETRANSMISSION); + MarkForRetransmission(unacked_it->first, ALL_UNACKED_RETRANSMISSION); } ++unacked_it; } @@ -304,6 +303,10 @@ QuicSentPacketManager::MarkPacketHandled( ++stats_->packets_spuriously_retransmitted; } + // The AckNotifierManager needs to be notified about the most recent + // transmission, since that's the one only one it tracks. + ack_notifier_manager_.OnPacketAcked(newest_transmission); + bool has_crypto_handshake = HasCryptoHandshake( unacked_packets_.GetTransmissionInfo(newest_transmission)); while (all_transmissions_it != all_transmissions.rend()) { @@ -431,7 +434,7 @@ void QuicSentPacketManager::RetransmitCryptoPackets() { continue; } packet_retransmitted = true; - MarkForRetransmission(sequence_number, TLP_RETRANSMISSION); + MarkForRetransmission(sequence_number, HANDSHAKE_RETRANSMISSION); // Abandon all the crypto retransmissions now so they're not lost later. OnPacketAbandoned(sequence_number); } @@ -538,7 +541,12 @@ void QuicSentPacketManager::MaybeRetransmitOnAckFrame( // Consider it multiple nacks when there is a gap between the missing packet // and the largest observed, since the purpose of a nack threshold is to // tolerate re-ordering. This handles both StretchAcks and Forward Acks. + // The nack count only increases when the largest observed increases. size_t min_nacks = received_info.largest_observed - sequence_number; + // Truncated acks can nack the largest observed, so set the nack count to 1. + if (min_nacks == 0) { + min_nacks = 1; + } unacked_packets_.NackPacket(sequence_number, min_nacks); } @@ -562,7 +570,7 @@ void QuicSentPacketManager::InvokeLossDetection(QuicTime time) { OnPacketAbandoned(sequence_number); if (unacked_packets_.HasRetransmittableFrames(sequence_number)) { - MarkForRetransmission(sequence_number, NACK_RETRANSMISSION); + MarkForRetransmission(sequence_number, LOSS_RETRANSMISSION); } else { // Since we will not retransmit this, we need to remove it from // unacked_packets_. This is either the current transmission of diff --git a/net/quic/quic_sent_packet_manager_test.cc b/net/quic/quic_sent_packet_manager_test.cc index cfd04d2..4e8b9ca 100644 --- a/net/quic/quic_sent_packet_manager_test.cc +++ b/net/quic/quic_sent_packet_manager_test.cc @@ -74,12 +74,13 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam<bool> { void RetransmitPacket(QuicPacketSequenceNumber old_sequence_number, QuicPacketSequenceNumber new_sequence_number) { QuicSentPacketManagerPeer::MarkForRetransmission( - &manager_, old_sequence_number, NACK_RETRANSMISSION); + &manager_, old_sequence_number, LOSS_RETRANSMISSION); EXPECT_TRUE(manager_.HasPendingRetransmissions()); QuicSentPacketManager::PendingRetransmission next_retransmission = manager_.NextPendingRetransmission(); EXPECT_EQ(old_sequence_number, next_retransmission.sequence_number); - EXPECT_EQ(NACK_RETRANSMISSION, next_retransmission.transmission_type); + EXPECT_EQ(LOSS_RETRANSMISSION, + next_retransmission.transmission_type); manager_.OnRetransmittedPacket(old_sequence_number, new_sequence_number); EXPECT_TRUE(QuicSentPacketManagerPeer::IsRetransmission( &manager_, new_sequence_number)); @@ -90,8 +91,11 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam<bool> { RetransmitPacket(old_sequence_number, new_sequence_number); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, new_sequence_number, _, _, _)) .WillOnce(Return(true)); - manager_.OnPacketSent(new_sequence_number, clock_.Now(), - 1000, NACK_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); + manager_.OnPacketSent(new_sequence_number, + clock_.Now(), + 1000, + LOSS_RETRANSMISSION, + HAS_RETRANSMITTABLE_DATA); } SerializedPacket CreateDataPacket(QuicPacketSequenceNumber sequence_number) { @@ -218,7 +222,7 @@ TEST_F(QuicSentPacketManagerTest, RetransmitThenAck) { TEST_F(QuicSentPacketManagerTest, RetransmitThenAckBeforeSend) { SendDataPacket(1); QuicSentPacketManagerPeer::MarkForRetransmission( - &manager_, 1, NACK_RETRANSMISSION); + &manager_, 1, LOSS_RETRANSMISSION); EXPECT_TRUE(manager_.HasPendingRetransmissions()); // Ack 1. @@ -289,7 +293,7 @@ TEST_F(QuicSentPacketManagerTest, RetransmitThenAckPreviousThenNackRetransmit) { EXPECT_CALL(*send_algorithm_, OnPacketSent(_, 2, _, _, _)) .WillOnce(Return(true)); manager_.OnPacketSent(2, clock_.ApproximateNow(), 1000, - NACK_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); + LOSS_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); QuicTime::Delta rtt = QuicTime::Delta::FromMilliseconds(15); clock_.AdvanceTime(rtt); @@ -600,7 +604,7 @@ TEST_F(QuicSentPacketManagerTest, GetSentTime) { EXPECT_EQ(sent_time, QuicSentPacketManagerPeer::GetSentTime(&manager_, 2)); } -TEST_F(QuicSentPacketManagerTest, NackRetransmit2Packets) { +TEST_F(QuicSentPacketManagerTest, FackRetransmit17Packets) { const size_t kNumSentPackets = 25; // Transmit 25 packets. for (QuicPacketSequenceNumber i = 1; i <= kNumSentPackets; ++i) { @@ -628,9 +632,15 @@ TEST_F(QuicSentPacketManagerTest, NackRetransmit2Packets) { EXPECT_EQ(kLargestObserved - i, QuicSentPacketManagerPeer::GetNackCount(&manager_, i)); } + + // Now receive the second packet, out of order, which should lose and + // retransmit nothing, because it does not increase the largest observed. + // No acks are registered, because the packet was already lost. + received_info.missing_packets.erase(2); + manager_.OnIncomingAck(received_info, clock_.Now()); } -TEST_F(QuicSentPacketManagerTest, NackRetransmit2PacketsAlternateAcks) { +TEST_F(QuicSentPacketManagerTest, FackRetransmit14PacketsAlternateAcks) { const size_t kNumSentPackets = 30; // Transmit 15 packets of data and 15 ack packets. The send algorithm will // inform the congestion manager not to save the acks by returning false. diff --git a/net/quic/quic_session.cc b/net/quic/quic_session.cc index ac39c2f..c2d25b7 100644 --- a/net/quic/quic_session.cc +++ b/net/quic/quic_session.cc @@ -306,10 +306,12 @@ QuicConsumedData QuicSession::WritevData( ack_notifier_delegate); } -size_t QuicSession::WriteHeaders(QuicStreamId id, - const SpdyHeaderBlock& headers, - bool fin) { - return headers_stream_->WriteHeaders(id, headers, fin); +size_t QuicSession::WriteHeaders( + QuicStreamId id, + const SpdyHeaderBlock& headers, + bool fin, + QuicAckNotifier::DelegateInterface* ack_notifier_delegate) { + return headers_stream_->WriteHeaders(id, headers, fin, ack_notifier_delegate); } void QuicSession::SendRstStream(QuicStreamId id, diff --git a/net/quic/quic_session.h b/net/quic/quic_session.h index 40086b4..76fb274 100644 --- a/net/quic/quic_session.h +++ b/net/quic/quic_session.h @@ -95,8 +95,7 @@ class NET_EXPORT_PRIVATE QuicSession : public QuicConnectionVisitorInterface { // has been sent on the wire: it may have been turned into a packet and queued // if the socket was unexpectedly blocked. // If provided, |ack_notifier_delegate| will be registered to be notified when - // we have seen ACKs for all packets resulting from this call. Not owned by - // this class. + // we have seen ACKs for all packets resulting from this call. virtual QuicConsumedData WritevData( QuicStreamId id, const IOVector& data, @@ -106,9 +105,13 @@ class NET_EXPORT_PRIVATE QuicSession : public QuicConnectionVisitorInterface { // Writes |headers| for the stream |id| to the dedicated headers stream. // If |fin| is true, then no more data will be sent for the stream |id|. - size_t WriteHeaders(QuicStreamId id, - const SpdyHeaderBlock& headers, - bool fin); + // If provided, |ack_notifier_delegate| will be registered to be notified when + // we have seen ACKs for all packets resulting from this call. + size_t WriteHeaders( + QuicStreamId id, + const SpdyHeaderBlock& headers, + bool fin, + QuicAckNotifier::DelegateInterface* ack_notifier_delegate); // Called by streams when they want to close the stream in both directions. virtual void SendRstStream(QuicStreamId id, diff --git a/net/quic/quic_stream_sequencer.cc b/net/quic/quic_stream_sequencer.cc index 145a377..28328c4 100644 --- a/net/quic/quic_stream_sequencer.cc +++ b/net/quic/quic_stream_sequencer.cc @@ -233,38 +233,6 @@ int QuicStreamSequencer::Readv(const struct iovec* iov, size_t iov_len) { return num_bytes_consumed_ - initial_bytes_consumed; } -void QuicStreamSequencer::MarkConsumed(size_t num_bytes_consumed) { - DCHECK(!blocked_); - size_t end_offset = num_bytes_consumed_ + num_bytes_consumed; - while (!frames_.empty() && end_offset != num_bytes_consumed_) { - FrameMap::iterator it = frames_.begin(); - if (it->first != num_bytes_consumed_) { - LOG(DFATAL) << "Invalid argument to MarkConsumed. " - << " num_bytes_consumed_: " << num_bytes_consumed_ - << " end_offset: " << end_offset - << " offset: " << it->first - << " length: " << it->second.length(); - stream_->Reset(QUIC_ERROR_PROCESSING_STREAM); - return; - } - - if (it->first + it->second.length() <= end_offset) { - num_bytes_consumed_ += it->second.length(); - num_bytes_buffered_ -= it->second.length(); - // This chunk is entirely consumed. - frames_.erase(it); - continue; - } - - // Partially consume this frame. - size_t delta = end_offset - it->first; - RecordBytesConsumed(delta); - frames_.insert(make_pair(end_offset, it->second.substr(delta))); - frames_.erase(it); - break; - } -} - bool QuicStreamSequencer::HasBytesToRead() const { FrameMap::const_iterator it = frames_.begin(); diff --git a/net/quic/quic_stream_sequencer.h b/net/quic/quic_stream_sequencer.h index 18fd552..5cdf8ad 100644 --- a/net/quic/quic_stream_sequencer.h +++ b/net/quic/quic_stream_sequencer.h @@ -57,10 +57,6 @@ class NET_EXPORT_PRIVATE QuicStreamSequencer { // bytes read. Any buffered data no longer in use will be released. int Readv(const struct iovec* iov, size_t iov_len); - // Consumes |num_bytes| data. Used in conjunction with |GetReadableRegions| - // to do zero-copy reads. - void MarkConsumed(size_t num_bytes); - // Returns true if the sequncer has bytes available for reading. bool HasBytesToRead() const; diff --git a/net/quic/quic_stream_sequencer_test.cc b/net/quic/quic_stream_sequencer_test.cc index d151ba9..3cb56fd 100644 --- a/net/quic/quic_stream_sequencer_test.cc +++ b/net/quic/quic_stream_sequencer_test.cc @@ -410,83 +410,6 @@ TEST_F(QuicStreamSequencerTest, OutOfOrderFramesBlockignWithGetReadableRegion) { EXPECT_TRUE(sequencer_->OnFrame(15, "pqr")); } -// Same as above, just using a different method for reading. -TEST_F(QuicStreamSequencerTest, MarkConsumed) { - sequencer_->SetMemoryLimit(9); - - InSequence s; - EXPECT_CALL(stream_, ProcessRawData(StrEq("abc"), 3)).WillOnce(Return(0)); - - EXPECT_TRUE(sequencer_->OnFrame(0, "abc")); - EXPECT_TRUE(sequencer_->OnFrame(3, "def")); - EXPECT_TRUE(sequencer_->OnFrame(6, "ghi")); - - // abcdefghi buffered - EXPECT_EQ(9u, sequencer_->num_bytes_buffered()); - - // Peek into the data. - const char* expected[] = {"abc", "def", "ghi"}; - ASSERT_TRUE(VerifyReadableRegions(expected, arraysize(expected))); - - // Consume 1 byte. - sequencer_->MarkConsumed(1); - // Verify data. - const char* expected2[] = {"bc", "def", "ghi"}; - ASSERT_TRUE(VerifyReadableRegions(expected2, arraysize(expected2))); - EXPECT_EQ(8u, sequencer_->num_bytes_buffered()); - - // Consume 2 bytes. - sequencer_->MarkConsumed(2); - // Verify data. - const char* expected3[] = {"def", "ghi"}; - ASSERT_TRUE(VerifyReadableRegions(expected3, arraysize(expected3))); - EXPECT_EQ(6u, sequencer_->num_bytes_buffered()); - - // Consume 5 bytes. - sequencer_->MarkConsumed(5); - // Verify data. - const char* expected4[] = {"i"}; - ASSERT_TRUE(VerifyReadableRegions(expected4, arraysize(expected4))); - EXPECT_EQ(1u, sequencer_->num_bytes_buffered()); -} - -TEST_F(QuicStreamSequencerTest, MarkConsumedError) { - // TODO(rch): enable when chromium supports EXPECT_DFATAL. - /* - EXPECT_CALL(stream_, ProcessRawData(StrEq("abc"), 3)).WillOnce(Return(0)); - - EXPECT_TRUE(sequencer_->OnFrame(0, "abc")); - EXPECT_TRUE(sequencer_->OnFrame(9, "jklmnopqrstuvwxyz")); - - // Peek into the data. Only the first chunk should be readable - // because of the missing data. - const char* expected[] = {"abc"}; - ASSERT_TRUE(VerifyReadableRegions(expected, arraysize(expected))); - - // Now, attempt to mark consumed more data than was readable - // and expect the stream to be closed. - EXPECT_CALL(stream_, Reset(QUIC_ERROR_PROCESSING_STREAM)); - EXPECT_DFATAL(sequencer_->MarkConsumed(4), - "Invalid argument to MarkConsumed. num_bytes_consumed_: 3 " - "end_offset: 4 offset: 9 length: 17"); - */ -} - -TEST_F(QuicStreamSequencerTest, MarkConsumedWithMissingPacket) { - InSequence s; - EXPECT_CALL(stream_, ProcessRawData(StrEq("abc"), 3)).WillOnce(Return(0)); - - EXPECT_TRUE(sequencer_->OnFrame(0, "abc")); - EXPECT_TRUE(sequencer_->OnFrame(3, "def")); - // Missing packet: 6, ghi - EXPECT_TRUE(sequencer_->OnFrame(9, "jkl")); - - const char* expected[] = {"abc", "def"}; - ASSERT_TRUE(VerifyReadableRegions(expected, arraysize(expected))); - - sequencer_->MarkConsumed(6); -} - TEST_F(QuicStreamSequencerTest, BasicHalfCloseOrdered) { InSequence s; diff --git a/net/quic/quic_unacked_packet_map.cc b/net/quic/quic_unacked_packet_map.cc index b89d73b..e7e5e16 100644 --- a/net/quic/quic_unacked_packet_map.cc +++ b/net/quic/quic_unacked_packet_map.cc @@ -154,7 +154,7 @@ void QuicUnackedPacketMap::NackPacket(QuicPacketSequenceNumber sequence_number, return; } - it->second.nack_count = max(min_nacks, it->second.nack_count + 1); + it->second.nack_count = max(min_nacks, it->second.nack_count); } void QuicUnackedPacketMap::RemovePacket( diff --git a/net/quic/quic_utils.cc b/net/quic/quic_utils.cc index 62524f9..73e19f1 100644 --- a/net/quic/quic_utils.cc +++ b/net/quic/quic_utils.cc @@ -234,7 +234,9 @@ const char* QuicUtils::EncryptionLevelToString(EncryptionLevel level) { const char* QuicUtils::TransmissionTypeToString(TransmissionType type) { switch (type) { RETURN_STRING_LITERAL(NOT_RETRANSMISSION); - RETURN_STRING_LITERAL(NACK_RETRANSMISSION); + RETURN_STRING_LITERAL(HANDSHAKE_RETRANSMISSION); + RETURN_STRING_LITERAL(LOSS_RETRANSMISSION); + RETURN_STRING_LITERAL(ALL_UNACKED_RETRANSMISSION); RETURN_STRING_LITERAL(RTO_RETRANSMISSION); RETURN_STRING_LITERAL(TLP_RETRANSMISSION); } diff --git a/net/quic/reliable_quic_stream.cc b/net/quic/reliable_quic_stream.cc index bc099e2..edcb026 100644 --- a/net/quic/reliable_quic_stream.cc +++ b/net/quic/reliable_quic_stream.cc @@ -25,6 +25,85 @@ struct iovec MakeIovec(StringPiece data) { } // namespace +// Wrapper that aggregates OnAckNotifications for packets sent using +// WriteOrBufferData and delivers them to the original +// QuicAckNotifier::DelegateInterface after all bytes written using +// WriteOrBufferData are acked. This level of indirection is +// necessary because the delegate interface provides no mechanism that +// WriteOrBufferData can use to inform it that the write required +// multiple WritevData calls or that only part of the data has been +// sent out by the time ACKs start arriving. +class ReliableQuicStream::ProxyAckNotifierDelegate + : public QuicAckNotifier::DelegateInterface { + public: + explicit ProxyAckNotifierDelegate(DelegateInterface* delegate) + : delegate_(delegate), + pending_acks_(0), + wrote_last_data_(false), + num_original_packets_(0), + num_original_bytes_(0), + num_retransmitted_packets_(0), + num_retransmitted_bytes_(0) { + } + + virtual void OnAckNotification(int num_original_packets, + int num_original_bytes, + int num_retransmitted_packets, + int num_retransmitted_bytes) OVERRIDE { + DCHECK_LT(0, pending_acks_); + --pending_acks_; + num_original_packets_ += num_original_packets; + num_original_bytes_ += num_original_bytes; + num_retransmitted_packets_ += num_retransmitted_packets; + num_retransmitted_bytes_ += num_retransmitted_bytes; + + if (wrote_last_data_ && pending_acks_ == 0) { + delegate_->OnAckNotification(num_original_packets_, + num_original_bytes_, + num_retransmitted_packets_, + num_retransmitted_bytes_); + } + } + + void WroteData(bool last_data) { + DCHECK(!wrote_last_data_); + ++pending_acks_; + wrote_last_data_ = last_data; + } + + protected: + // Delegates are ref counted. + virtual ~ProxyAckNotifierDelegate() { + } + + private: + // Original delegate. delegate_->OnAckNotification will be called when: + // wrote_last_data_ == true and pending_acks_ == 0 + scoped_refptr<DelegateInterface> delegate_; + + // Number of outstanding acks. + int pending_acks_; + + // True if no pending writes remain. + bool wrote_last_data_; + + // Accumulators. + int num_original_packets_; + int num_original_bytes_; + int num_retransmitted_packets_; + int num_retransmitted_bytes_; + + DISALLOW_COPY_AND_ASSIGN(ProxyAckNotifierDelegate); +}; + +ReliableQuicStream::PendingData::PendingData( + string data_in, scoped_refptr<ProxyAckNotifierDelegate> delegate_in) + : data(data_in), delegate(delegate_in) { +} + +ReliableQuicStream::PendingData::~PendingData() { +} + ReliableQuicStream::ReliableQuicStream(QuicStreamId id, QuicSession* session) : sequencer_(this), @@ -118,7 +197,10 @@ QuicVersion ReliableQuicStream::version() const { return session()->connection()->version(); } -void ReliableQuicStream::WriteOrBufferData(StringPiece data, bool fin) { +void ReliableQuicStream::WriteOrBufferData( + StringPiece data, + bool fin, + QuicAckNotifier::DelegateInterface* ack_notifier_delegate) { if (data.empty() && !fin) { LOG(DFATAL) << "data.empty() && !fin"; return; @@ -129,38 +211,60 @@ void ReliableQuicStream::WriteOrBufferData(StringPiece data, bool fin) { return; } + scoped_refptr<ProxyAckNotifierDelegate> proxy_delegate; + if (ack_notifier_delegate != NULL) { + proxy_delegate = new ProxyAckNotifierDelegate(ack_notifier_delegate); + } + QuicConsumedData consumed_data(0, false); fin_buffered_ = fin; if (queued_data_.empty()) { struct iovec iov(MakeIovec(data)); - consumed_data = WritevData(&iov, 1, fin, NULL); + consumed_data = WritevData(&iov, 1, fin, proxy_delegate.get()); DCHECK_LE(consumed_data.bytes_consumed, data.length()); } + bool write_completed; // If there's unconsumed data or an unconsumed fin, queue it. if (consumed_data.bytes_consumed < data.length() || (fin && !consumed_data.fin_consumed)) { - queued_data_.push_back( - string(data.data() + consumed_data.bytes_consumed, - data.length() - consumed_data.bytes_consumed)); + StringPiece remainder(data.substr(consumed_data.bytes_consumed)); + queued_data_.push_back(PendingData(remainder.as_string(), proxy_delegate)); + write_completed = false; + } else { + write_completed = true; + } + + if ((proxy_delegate.get() != NULL) && + (consumed_data.bytes_consumed > 0 || consumed_data.fin_consumed)) { + proxy_delegate->WroteData(write_completed); } } void ReliableQuicStream::OnCanWrite() { bool fin = false; while (!queued_data_.empty()) { - const string& data = queued_data_.front(); + PendingData* pending_data = &queued_data_.front(); + ProxyAckNotifierDelegate* delegate = pending_data->delegate.get(); if (queued_data_.size() == 1 && fin_buffered_) { fin = true; } - struct iovec iov(MakeIovec(data)); - QuicConsumedData consumed_data = WritevData(&iov, 1, fin, NULL); - if (consumed_data.bytes_consumed == data.size() && + struct iovec iov(MakeIovec(pending_data->data)); + QuicConsumedData consumed_data = WritevData(&iov, 1, fin, delegate); + if (consumed_data.bytes_consumed == pending_data->data.size() && fin == consumed_data.fin_consumed) { queued_data_.pop_front(); + if (delegate != NULL) { + delegate->WroteData(true); + } } else { - queued_data_.front().erase(0, consumed_data.bytes_consumed); + if (consumed_data.bytes_consumed > 0) { + pending_data->data.erase(0, consumed_data.bytes_consumed); + if (delegate != NULL) { + delegate->WroteData(false); + } + } break; } } diff --git a/net/quic/reliable_quic_stream.h b/net/quic/reliable_quic_stream.h index 42848f8..15cd030 100644 --- a/net/quic/reliable_quic_stream.h +++ b/net/quic/reliable_quic_stream.h @@ -12,6 +12,7 @@ #include <list> #include "base/basictypes.h" +#include "base/memory/ref_counted.h" #include "base/strings/string_piece.h" #include "net/base/iovec.h" #include "net/base/net_export.h" @@ -94,7 +95,10 @@ class NET_EXPORT_PRIVATE ReliableQuicStream { protected: // Sends as much of 'data' to the connection as the connection will consume, // and then buffers any remaining data in queued_data_. - void WriteOrBufferData(base::StringPiece data, bool fin); + void WriteOrBufferData( + base::StringPiece data, + bool fin, + QuicAckNotifier::DelegateInterface* ack_notifier_delegate); // Sends as many bytes in the first |count| buffers of |iov| to the connection // as the connection will consume. @@ -126,8 +130,20 @@ class NET_EXPORT_PRIVATE ReliableQuicStream { private: friend class test::ReliableQuicStreamPeer; friend class QuicStreamUtils; + class ProxyAckNotifierDelegate; + + struct PendingData { + PendingData(string data_in, + scoped_refptr<ProxyAckNotifierDelegate> delegate_in); + ~PendingData(); + + string data; + // Delegate that should be notified when the pending data is acked. + // Can be nullptr. + scoped_refptr<ProxyAckNotifierDelegate> delegate; + }; - std::list<string> queued_data_; + std::list<PendingData> queued_data_; QuicStreamSequencer sequencer_; QuicStreamId id_; diff --git a/net/quic/reliable_quic_stream_test.cc b/net/quic/reliable_quic_stream_test.cc index 0576715..9ea4fd6 100644 --- a/net/quic/reliable_quic_stream_test.cc +++ b/net/quic/reliable_quic_stream_test.cc @@ -14,14 +14,18 @@ #include "net/quic/test_tools/reliable_quic_stream_peer.h" #include "net/test/gtest_util.h" #include "testing/gmock/include/gmock/gmock.h" +#include "testing/gmock_mutant.h" using base::StringPiece; using std::min; using testing::_; +using testing::CreateFunctor; using testing::InSequence; +using testing::Invoke; using testing::Return; using testing::SaveArg; using testing::StrictMock; +using testing::WithArgs; namespace net { namespace test { @@ -127,7 +131,7 @@ TEST_F(ReliableQuicStreamTest, WriteAllData) { PACKET_6BYTE_SEQUENCE_NUMBER, NOT_IN_FEC_GROUP); EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( Return(QuicConsumedData(kDataLen, true))); - stream_->WriteOrBufferData(kData1, false); + stream_->WriteOrBufferData(kData1, false, NULL); EXPECT_FALSE(write_blocked_list_->HasWriteBlockedStreams()); } @@ -136,7 +140,7 @@ TEST_F(ReliableQuicStreamTest, NoBlockingIfNoDataOrFin) { // Write no data and no fin. If we consume nothing we should not be write // blocked. - EXPECT_DFATAL(stream_->WriteOrBufferData(StringPiece(), false), ""); + EXPECT_DFATAL(stream_->WriteOrBufferData(StringPiece(), false, NULL), ""); EXPECT_FALSE(write_blocked_list_->HasWriteBlockedStreams()); } @@ -147,7 +151,7 @@ TEST_F(ReliableQuicStreamTest, BlockIfOnlySomeDataConsumed) { // we should be write blocked a not all the data was consumed. EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( Return(QuicConsumedData(1, false))); - stream_->WriteOrBufferData(StringPiece(kData1, 2), false); + stream_->WriteOrBufferData(StringPiece(kData1, 2), false, NULL); ASSERT_EQ(1u, write_blocked_list_->NumBlockedStreams()); } @@ -161,7 +165,7 @@ TEST_F(ReliableQuicStreamTest, BlockIfFinNotConsumedWithData) { // last data) EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( Return(QuicConsumedData(2, false))); - stream_->WriteOrBufferData(StringPiece(kData1, 2), true); + stream_->WriteOrBufferData(StringPiece(kData1, 2), true, NULL); ASSERT_EQ(1u, write_blocked_list_->NumBlockedStreams()); } @@ -172,7 +176,7 @@ TEST_F(ReliableQuicStreamTest, BlockIfSoloFinNotConsumed) { // as the fin was not consumed. EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( Return(QuicConsumedData(0, false))); - stream_->WriteOrBufferData(StringPiece(), true); + stream_->WriteOrBufferData(StringPiece(), true, NULL); ASSERT_EQ(1u, write_blocked_list_->NumBlockedStreams()); } @@ -186,11 +190,11 @@ TEST_F(ReliableQuicStreamTest, WriteOrBufferData) { PACKET_6BYTE_SEQUENCE_NUMBER, NOT_IN_FEC_GROUP); EXPECT_CALL(*session_, WritevData(_, _, _, _, _)).WillOnce( Return(QuicConsumedData(kDataLen - 1, false))); - stream_->WriteOrBufferData(kData1, false); + stream_->WriteOrBufferData(kData1, false, NULL); EXPECT_TRUE(write_blocked_list_->HasWriteBlockedStreams()); // Queue a bytes_consumed write. - stream_->WriteOrBufferData(kData2, false); + stream_->WriteOrBufferData(kData2, false, NULL); // Make sure we get the tail of the first write followed by the bytes_consumed InSequence s; @@ -230,7 +234,7 @@ TEST_F(ReliableQuicStreamTest, RstAlwaysSentIfNoFinSent) { // Write some data, with no FIN. EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( Return(QuicConsumedData(1, false))); - stream_->WriteOrBufferData(StringPiece(kData1, 1), false); + stream_->WriteOrBufferData(StringPiece(kData1, 1), false, NULL); EXPECT_FALSE(fin_sent()); EXPECT_FALSE(rst_sent()); @@ -253,7 +257,7 @@ TEST_F(ReliableQuicStreamTest, RstNotSentIfFinSent) { // Write some data, with FIN. EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( Return(QuicConsumedData(1, true))); - stream_->WriteOrBufferData(StringPiece(kData1, 1), true); + stream_->WriteOrBufferData(StringPiece(kData1, 1), true, NULL); EXPECT_TRUE(fin_sent()); EXPECT_FALSE(rst_sent()); @@ -287,6 +291,172 @@ TEST_F(ReliableQuicStreamTest, OnlySendOneRst) { EXPECT_TRUE(rst_sent()); } +void SaveProxyAckNotifierDelegate( + scoped_refptr<QuicAckNotifier::DelegateInterface>* delegate_out, + QuicAckNotifier::DelegateInterface* delegate) { + *delegate_out = delegate; +} +TEST_F(ReliableQuicStreamTest, WriteOrBufferDataWithQuicAckNotifier) { + Initialize(kShouldProcessData); + + scoped_refptr<MockAckNotifierDelegate> delegate( + new StrictMock<MockAckNotifierDelegate>); + + const int kDataSize = 16 * 1024; + const string kData(kDataSize, 'a'); + + const int kFirstWriteSize = 100; + const int kSecondWriteSize = 50; + const int kLastWriteSize = kDataSize - kFirstWriteSize - kSecondWriteSize; + + scoped_refptr<QuicAckNotifier::DelegateInterface> proxy_delegate; + + EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce(DoAll( + WithArgs<4>(Invoke(CreateFunctor( + &SaveProxyAckNotifierDelegate, &proxy_delegate))), + Return(QuicConsumedData(kFirstWriteSize, false)))); + stream_->WriteOrBufferData(kData, false, delegate.get()); + EXPECT_TRUE(write_blocked_list_->HasWriteBlockedStreams()); + + EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, proxy_delegate.get())). + WillOnce( + Return(QuicConsumedData(kSecondWriteSize, false))); + stream_->OnCanWrite(); + + // No ack expected for an empty write. + EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, proxy_delegate.get())). + WillOnce( + Return(QuicConsumedData(0, false))); + stream_->OnCanWrite(); + + EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, proxy_delegate.get())). + WillOnce( + Return(QuicConsumedData(kLastWriteSize, false))); + stream_->OnCanWrite(); + + // There were two writes, so OnAckNotification is not propagated + // until the third Ack arrives. + proxy_delegate->OnAckNotification(1, 2, 3, 4); + proxy_delegate->OnAckNotification(10, 20, 30, 40); + + // The arguments to delegate->OnAckNotification are the sum of the + // arguments to proxy_delegate OnAckNotification calls. + EXPECT_CALL(*delegate, OnAckNotification(111, 222, 333, 444)); + proxy_delegate->OnAckNotification(100, 200, 300, 400); +} + +// Verify delegate behavior when packets are acked before the +// WritevData call that sends out the last byte. +TEST_F(ReliableQuicStreamTest, WriteOrBufferDataAckNotificationBeforeFlush) { + Initialize(kShouldProcessData); + + scoped_refptr<MockAckNotifierDelegate> delegate( + new StrictMock<MockAckNotifierDelegate>); + + const int kDataSize = 16 * 1024; + const string kData(kDataSize, 'a'); + + const int kInitialWriteSize = 100; + + scoped_refptr<QuicAckNotifier::DelegateInterface> proxy_delegate; + + EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce(DoAll( + WithArgs<4>(Invoke(CreateFunctor( + &SaveProxyAckNotifierDelegate, &proxy_delegate))), + Return(QuicConsumedData(kInitialWriteSize, false)))); + stream_->WriteOrBufferData(kData, false, delegate.get()); + EXPECT_TRUE(write_blocked_list_->HasWriteBlockedStreams()); + + // Handle the ack of the first write. + proxy_delegate->OnAckNotification(1, 2, 3, 4); + proxy_delegate = NULL; + + EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce(DoAll( + WithArgs<4>(Invoke(CreateFunctor( + &SaveProxyAckNotifierDelegate, &proxy_delegate))), + Return(QuicConsumedData(kDataSize - kInitialWriteSize, false)))); + stream_->OnCanWrite(); + + // Handle the ack for the second write. + EXPECT_CALL(*delegate, OnAckNotification(101, 202, 303, 404)); + proxy_delegate->OnAckNotification(100, 200, 300, 400); +} + +// Verify delegate behavior when WriteOrBufferData does not buffer. +TEST_F(ReliableQuicStreamTest, WriteAndBufferDataWithAckNotiferNoBuffer) { + Initialize(kShouldProcessData); + + scoped_refptr<MockAckNotifierDelegate> delegate( + new StrictMock<MockAckNotifierDelegate>); + + scoped_refptr<QuicAckNotifier::DelegateInterface> proxy_delegate; + + EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce(DoAll( + WithArgs<4>(Invoke(CreateFunctor( + &SaveProxyAckNotifierDelegate, &proxy_delegate))), + Return(QuicConsumedData(kDataLen, true)))); + stream_->WriteOrBufferData(kData1, true, delegate.get()); + EXPECT_FALSE(write_blocked_list_->HasWriteBlockedStreams()); + + // Handle the ack. + EXPECT_CALL(*delegate, OnAckNotification(1, 2, 3, 4)); + proxy_delegate->OnAckNotification(1, 2, 3, 4); +} + +// Verify delegate behavior when WriteOrBufferData buffers all the data. +TEST_F(ReliableQuicStreamTest, BufferOnWriteAndBufferDataWithAckNotifer) { + Initialize(kShouldProcessData); + + scoped_refptr<MockAckNotifierDelegate> delegate( + new StrictMock<MockAckNotifierDelegate>); + + scoped_refptr<QuicAckNotifier::DelegateInterface> proxy_delegate; + + EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( + Return(QuicConsumedData(0, false))); + stream_->WriteOrBufferData(kData1, true, delegate.get()); + EXPECT_TRUE(write_blocked_list_->HasWriteBlockedStreams()); + + EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce(DoAll( + WithArgs<4>(Invoke(CreateFunctor( + &SaveProxyAckNotifierDelegate, &proxy_delegate))), + Return(QuicConsumedData(kDataLen, true)))); + stream_->OnCanWrite(); + + // Handle the ack. + EXPECT_CALL(*delegate, OnAckNotification(1, 2, 3, 4)); + proxy_delegate->OnAckNotification(1, 2, 3, 4); +} + +// Verify delegate behavior when WriteOrBufferData when the FIN is +// sent out in a different packet. +TEST_F(ReliableQuicStreamTest, WriteAndBufferDataWithAckNotiferOnlyFinRemains) { + Initialize(kShouldProcessData); + + scoped_refptr<MockAckNotifierDelegate> delegate( + new StrictMock<MockAckNotifierDelegate>); + + scoped_refptr<QuicAckNotifier::DelegateInterface> proxy_delegate; + + EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce(DoAll( + WithArgs<4>(Invoke(CreateFunctor( + &SaveProxyAckNotifierDelegate, &proxy_delegate))), + Return(QuicConsumedData(kDataLen, false)))); + stream_->WriteOrBufferData(kData1, true, delegate.get()); + EXPECT_TRUE(write_blocked_list_->HasWriteBlockedStreams()); + + EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce(DoAll( + WithArgs<4>(Invoke(CreateFunctor( + &SaveProxyAckNotifierDelegate, &proxy_delegate))), + Return(QuicConsumedData(0, true)))); + stream_->OnCanWrite(); + + // Handle the acks. + proxy_delegate->OnAckNotification(1, 2, 3, 4); + EXPECT_CALL(*delegate, OnAckNotification(11, 22, 33, 44)); + proxy_delegate->OnAckNotification(10, 20, 30, 40); +} + } // namespace } // namespace test } // namespace net diff --git a/net/tools/quic/quic_server_session.cc b/net/tools/quic/quic_server_session.cc index 1213495..2fc824d 100644 --- a/net/tools/quic/quic_server_session.cc +++ b/net/tools/quic/quic_server_session.cc @@ -12,10 +12,6 @@ namespace net { namespace tools { -// If true, cancel any asynchronous client hello validation when the connection -// is closed. -bool FLAGS_cancel_crypto_callbacks_on_close = false; - QuicServerSession::QuicServerSession( const QuicConfig& config, QuicConnection* connection, @@ -40,8 +36,7 @@ void QuicServerSession::OnConnectionClosed(QuicErrorCode error, QuicSession::OnConnectionClosed(error, from_peer); // In the unlikely event we get a connection close while doing an asynchronous // crypto event, make sure we cancel the callback. - if (FLAGS_cancel_crypto_callbacks_on_close && - crypto_stream_.get() != NULL) { + if (crypto_stream_.get() != NULL) { crypto_stream_->CancelOutstandingCallbacks(); } visitor_->OnConnectionClosed(connection()->connection_id(), error); diff --git a/net/tools/quic/quic_spdy_client_stream.cc b/net/tools/quic/quic_spdy_client_stream.cc index 1dfee90..8423295 100644 --- a/net/tools/quic/quic_spdy_client_stream.cc +++ b/net/tools/quic/quic_spdy_client_stream.cc @@ -82,11 +82,12 @@ ssize_t QuicSpdyClientStream::SendRequest(const BalsaHeaders& headers, bool send_fin_with_headers = fin && body.empty(); size_t bytes_sent = body.size(); - header_bytes_written_ = WriteHeaders(header_block, send_fin_with_headers); + header_bytes_written_ = WriteHeaders( + header_block, send_fin_with_headers, NULL); bytes_sent += header_bytes_written_; if (!body.empty()) { - WriteOrBufferData(body, fin); + WriteOrBufferData(body, fin, NULL); } return bytes_sent; @@ -119,7 +120,7 @@ int QuicSpdyClientStream::ParseResponseHeaders() { // Sends body data to the server and returns the number of bytes sent. void QuicSpdyClientStream::SendBody(const string& data, bool fin) { - WriteOrBufferData(data, fin); + WriteOrBufferData(data, fin, NULL); } } // namespace tools diff --git a/net/tools/quic/quic_spdy_server_stream.cc b/net/tools/quic/quic_spdy_server_stream.cc index edf493d..a5fc893 100644 --- a/net/tools/quic/quic_spdy_server_stream.cc +++ b/net/tools/quic/quic_spdy_server_stream.cc @@ -133,10 +133,10 @@ void QuicSpdyServerStream:: SendHeadersAndBody( SpdyHeaderBlock header_block = SpdyUtils::ResponseHeadersToSpdyHeaders(response_headers); - WriteHeaders(header_block, body.empty()); + WriteHeaders(header_block, body.empty(), NULL); if (!body.empty()) { - WriteOrBufferData(body, true); + WriteOrBufferData(body, true, NULL); } } diff --git a/net/tools/quic/test_tools/quic_test_client.cc b/net/tools/quic/test_tools/quic_test_client.cc index 00061b5..d88d836f 100644 --- a/net/tools/quic/test_tools/quic_test_client.cc +++ b/net/tools/quic/test_tools/quic_test_client.cc @@ -25,21 +25,23 @@ using net::test::QuicConnectionPeer; using std::string; using std::vector; +namespace net { +namespace tools { +namespace test { namespace { // RecordingProofVerifier accepts any certificate chain and records the common // name of the leaf. -class RecordingProofVerifier : public net::ProofVerifier { +class RecordingProofVerifier : public ProofVerifier { public: // ProofVerifier interface. - virtual net::ProofVerifier::Status VerifyProof( - const string& hostname, - const string& server_config, - const vector<string>& certs, - const string& signature, - string* error_details, - scoped_ptr<net::ProofVerifyDetails>* details, - net::ProofVerifierCallback* callback) OVERRIDE { + virtual Status VerifyProof(const string& hostname, + const string& server_config, + const vector<string>& certs, + const string& signature, + string* error_details, + scoped_ptr<ProofVerifyDetails>* details, + ProofVerifierCallback* callback) OVERRIDE { delete callback; common_name_.clear(); @@ -70,10 +72,6 @@ class RecordingProofVerifier : public net::ProofVerifier { } // anonymous namespace -namespace net { -namespace tools { -namespace test { - BalsaHeaders* MungeHeaders(const BalsaHeaders* const_headers, bool secure) { StringPiece uri = const_headers->request_uri(); |