diff options
| author | rtenneti@chromium.org <rtenneti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-05-22 11:13:05 +0000 |
|---|---|---|
| committer | rtenneti@chromium.org <rtenneti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-05-22 11:13:05 +0000 |
| commit | 66ae596f10dfc801b79c5d756c0e99792af1807f (patch) | |
| tree | 5b224ff7568761c1217b5075ec27c9efab9c783f /net/quic | |
| parent | 411f8aef7e6db2fadec7205a220f24ec8b914486 (diff) | |
| download | chromium_src-66ae596f10dfc801b79c5d756c0e99792af1807f.zip chromium_src-66ae596f10dfc801b79c5d756c0e99792af1807f.tar.gz chromium_src-66ae596f10dfc801b79c5d756c0e99792af1807f.tar.bz2 | |
Land Recent QUIC Changes.
A number of tests define data stream IDs: this CL puts these definitions
in one place in quic_test_utils.h
Merge internal change: 67627075
https://codereview.chromium.org/296853003/
Replace NeuterPacket and RemovePacket with RemoveRetransmittibility,
which removes the retransmittibility property from a packet and it's
associated transmissions.
Merge internal change: 67615222
https://codereview.chromium.org/286933010/
Cleanup changes to fix the using:: order.
Merge internal change: 67541632,, 67612445
https://codereview.chromium.org/297773002/
Remove QuicSentPacketManager's DiscardUnackedPacket method now that it's
not necessary.
Merge internal change: 67542370
https://codereview.chromium.org/290333005/
Garbage collect QUIC entropy map as gaps are filled. Not flag protected.
Merge internal change: 67521743
https://codereview.chromium.org/296053006/
Extract entropy tracking from QuicReceivedPacketManager into its own
class in preparation for garbage collecting unneeded entropy history.
Extract QUIC entropy tracking into a separate class, no behavior change.
Merge internal change: 67273904
https://codereview.chromium.org/294143004/
Remove check for an impossible condition. StopWaiting frames with
least_unacked > packet sequence number are illegal. Largest observed is
guaranteed to be >= the StopWaiting packet sequence number.
Remove impossible condition from an if.
Merge internal change: 67268376
https://codereview.chromium.org/292983009/
R=rch@chromium.org
Review URL: https://codereview.chromium.org/297513005
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@272158 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net/quic')
23 files changed, 642 insertions, 423 deletions
diff --git a/net/quic/congestion_control/tcp_loss_algorithm_test.cc b/net/quic/congestion_control/tcp_loss_algorithm_test.cc index 657ab2b..53806b4 100644 --- a/net/quic/congestion_control/tcp_loss_algorithm_test.cc +++ b/net/quic/congestion_control/tcp_loss_algorithm_test.cc @@ -171,7 +171,7 @@ TEST_F(TcpLossAlgorithmTest, DontEarlyRetransmitNeuteredPacket) { // Early retransmit when the final packet gets acked and the first is nacked. unacked_packets_.SetNotPending(2); unacked_packets_.NackPacket(1, 1); - unacked_packets_.NeuterPacket(1); + unacked_packets_.RemoveRetransmittibility(1, 1); VerifyLosses(2, NULL, 0); EXPECT_EQ(QuicTime::Zero(), loss_algorithm_.GetLossTimeout()); } diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc index 523ce6e..851ccd6 100644 --- a/net/quic/quic_connection.cc +++ b/net/quic/quic_connection.cc @@ -1443,16 +1443,6 @@ bool QuicConnection::ShouldDiscardPacket( return true; } - if (encryption_level_ == ENCRYPTION_FORWARD_SECURE && - level == ENCRYPTION_NONE) { - // Drop packets that are NULL encrypted since the peer won't accept them - // anymore. - DVLOG(1) << ENDPOINT << "Dropping packet: " << sequence_number - << " since the packet is NULL encrypted."; - sent_packet_manager_.DiscardUnackedPacket(sequence_number); - return true; - } - // If the packet has been discarded before sending, don't send it. // This occurs if a packet gets serialized, queued, then discarded. if (!sent_packet_manager_.IsUnacked(sequence_number)) { @@ -1461,11 +1451,23 @@ bool QuicConnection::ShouldDiscardPacket( return true; } + if (encryption_level_ == ENCRYPTION_FORWARD_SECURE && + level == ENCRYPTION_NONE) { + // Drop packets that are NULL encrypted since the peer won't accept them + // anymore. + DVLOG(1) << ENDPOINT << "Dropping NULL encrypted packet: " + << sequence_number << " since the connection is forward secure."; + LOG_IF(DFATAL, + sent_packet_manager_.HasRetransmittableFrames(sequence_number)) + << "Once forward secure, all NULL encrypted packets should be " + << "neutered."; + return true; + } + if (retransmittable == HAS_RETRANSMITTABLE_DATA && !sent_packet_manager_.HasRetransmittableFrames(sequence_number)) { - DVLOG(1) << ENDPOINT << "Dropping packet: " << sequence_number - << " since a previous transmission has been acked."; - sent_packet_manager_.DiscardUnackedPacket(sequence_number); + LOG(DFATAL) << ENDPOINT << "Dropping unacked packet: " << sequence_number + << " This should have been removed when it was Neutered."; return true; } diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc index 48cb2db..5e2da31 100644 --- a/net/quic/quic_connection_test.cc +++ b/net/quic/quic_connection_test.cc @@ -59,11 +59,6 @@ const QuicPacketEntropyHash kTestEntropyHash = 76; const int kDefaultRetransmissionTimeMs = 500; const int kMinRetransmissionTimeMs = 200; -// Used by TestConnection::SendStreamData3. -const QuicStreamId kStreamId3 = 3; -// Used by TestConnection::SendStreamData5. -const QuicStreamId kStreamId5 = 5; - class TestReceiveAlgorithm : public ReceiveAlgorithmInterface { public: explicit TestReceiveAlgorithm(QuicCongestionFeedbackFrame* feedback) @@ -473,11 +468,13 @@ class TestConnection : public QuicConnection { } QuicConsumedData SendStreamData3() { - return SendStreamDataWithString(kStreamId3, "food", 0, !kFin, NULL); + return SendStreamDataWithString(kClientDataStreamId1, "food", 0, !kFin, + NULL); } QuicConsumedData SendStreamData5() { - return SendStreamDataWithString(kStreamId5, "food2", 0, !kFin, NULL); + return SendStreamDataWithString(kClientDataStreamId2, "food2", 0, + !kFin, NULL); } // Ensures the connection can write stream data before writing. @@ -1587,8 +1584,8 @@ TEST_P(QuicConnectionTest, FramePacking) { } EXPECT_FALSE(writer_->ack_frames().empty()); EXPECT_EQ(2u, writer_->stream_frames().size()); - EXPECT_EQ(kStreamId3, writer_->stream_frames()[0].stream_id); - EXPECT_EQ(kStreamId5, writer_->stream_frames()[1].stream_id); + EXPECT_EQ(kClientDataStreamId1, writer_->stream_frames()[0].stream_id); + EXPECT_EQ(kClientDataStreamId2, writer_->stream_frames()[1].stream_id); } TEST_P(QuicConnectionTest, FramePackingNonCryptoThenCrypto) { @@ -1640,7 +1637,7 @@ TEST_P(QuicConnectionTest, FramePackingCryptoThenNonCrypto) { // Parse the last packet and ensure it's the stream frame from stream 3. EXPECT_EQ(1u, writer_->frame_count()); EXPECT_EQ(1u, writer_->stream_frames().size()); - EXPECT_EQ(kStreamId3, writer_->stream_frames()[0].stream_id); + EXPECT_EQ(kClientDataStreamId1, writer_->stream_frames()[0].stream_id); } TEST_P(QuicConnectionTest, FramePackingFEC) { @@ -1706,8 +1703,8 @@ TEST_P(QuicConnectionTest, FramePackingAckResponse) { } EXPECT_FALSE(writer_->ack_frames().empty()); ASSERT_EQ(2u, writer_->stream_frames().size()); - EXPECT_EQ(kStreamId3, writer_->stream_frames()[0].stream_id); - EXPECT_EQ(kStreamId5, writer_->stream_frames()[1].stream_id); + EXPECT_EQ(kClientDataStreamId1, writer_->stream_frames()[0].stream_id); + EXPECT_EQ(kClientDataStreamId2, writer_->stream_frames()[1].stream_id); } TEST_P(QuicConnectionTest, FramePackingSendv) { @@ -1794,8 +1791,8 @@ TEST_P(QuicConnectionTest, OnCanWrite) { // two different streams. EXPECT_EQ(2u, writer_->frame_count()); EXPECT_EQ(2u, writer_->stream_frames().size()); - EXPECT_EQ(kStreamId3, writer_->stream_frames()[0].stream_id); - EXPECT_EQ(kStreamId5, writer_->stream_frames()[1].stream_id); + EXPECT_EQ(kClientDataStreamId1, writer_->stream_frames()[0].stream_id); + EXPECT_EQ(kClientDataStreamId2, writer_->stream_frames()[1].stream_id); } TEST_P(QuicConnectionTest, RetransmitOnNack) { @@ -2323,21 +2320,24 @@ TEST_P(QuicConnectionTest, QuicPacketSequenceNumber sequence_number; SendStreamDataToPeer(3, "foo", 0, !kFin, &sequence_number); + // Simulate the retransmission alarm firing and the socket blocking. + BlockOnNextWrite(); + EXPECT_CALL(*send_algorithm_, OnRetransmissionTimeout(true)); + clock_.AdvanceTime(DefaultRetransmissionTime()); + connection_.GetRetransmissionAlarm()->Fire(); + + // Go forward secure. connection_.SetEncrypter(ENCRYPTION_FORWARD_SECURE, new TaggingEncrypter(0x02)); connection_.SetDefaultEncryptionLevel(ENCRYPTION_FORWARD_SECURE); + connection_.NeuterUnencryptedPackets(); - EXPECT_CALL(*send_algorithm_, OnRetransmissionTimeout(true)); - EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0); - - QuicTime default_retransmission_time = clock_.ApproximateNow().Add( - DefaultRetransmissionTime()); - - EXPECT_EQ(default_retransmission_time, + EXPECT_EQ(QuicTime::Zero(), connection_.GetRetransmissionAlarm()->deadline()); - // Simulate the retransmission alarm firing. - clock_.AdvanceTime(DefaultRetransmissionTime()); - connection_.GetRetransmissionAlarm()->Fire(); + // Unblock the socket and ensure that no packets are sent. + EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(0); + writer_->SetWritable(); + connection_.OnCanWrite(); } TEST_P(QuicConnectionTest, RetransmitPacketsWithInitialEncryption) { @@ -2997,7 +2997,8 @@ TEST_P(QuicConnectionTest, NoAckOnOldNacks) { TEST_P(QuicConnectionTest, SendDelayedAckOnOutgoingPacket) { EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); ProcessPacket(1); - connection_.SendStreamDataWithString(kStreamId3, "foo", 0, !kFin, NULL); + connection_.SendStreamDataWithString(kClientDataStreamId1, "foo", 0, + !kFin, NULL); // Check that ack is bundled with outgoing data and that delayed ack // alarm is reset. if (version() > QUIC_VERSION_15) { @@ -3022,8 +3023,10 @@ TEST_P(QuicConnectionTest, SendDelayedAckOnOutgoingCryptoPacket) { TEST_P(QuicConnectionTest, BundleAckWithDataOnIncomingAck) { EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); - connection_.SendStreamDataWithString(kStreamId3, "foo", 0, !kFin, NULL); - connection_.SendStreamDataWithString(kStreamId3, "foo", 3, !kFin, NULL); + connection_.SendStreamDataWithString(kClientDataStreamId1, "foo", 0, + !kFin, NULL); + connection_.SendStreamDataWithString(kClientDataStreamId1, "foo", 3, + !kFin, NULL); // Ack the second packet, which will retransmit the first packet. QuicAckFrame ack = InitAckFrame(2, 0); NackPacket(1, &ack); @@ -3247,7 +3250,7 @@ TEST_P(QuicConnectionTest, EntropyCalculationForTruncatedAck) { QuicPacketEntropyHash entropy[51]; entropy[0] = 0; for (int i = 1; i < 51; ++i) { - bool should_send = i % 10 != 0; + bool should_send = i % 10 != 1; bool entropy_flag = (i & (i - 1)) != 0; if (!should_send) { entropy[i] = entropy[i - 1]; @@ -3260,7 +3263,6 @@ TEST_P(QuicConnectionTest, EntropyCalculationForTruncatedAck) { } ProcessDataPacket(i, 1, entropy_flag); } - // Till 50 since 50th packet is not sent. for (int i = 1; i < 50; ++i) { EXPECT_EQ(entropy[i], QuicConnectionPeer::ReceivedEntropyHash( &connection_, i)); diff --git a/net/quic/quic_data_stream_test.cc b/net/quic/quic_data_stream_test.cc index 164e7e4..a899853 100644 --- a/net/quic/quic_data_stream_test.cc +++ b/net/quic/quic_data_stream_test.cc @@ -30,9 +30,6 @@ namespace net { namespace test { namespace { -// First non-reserved client stream ID. -const QuicStreamId kStreamId = 5; - const bool kIsServer = true; const bool kShouldProcessData = true; @@ -98,10 +95,10 @@ class QuicDataStreamTest : public ::testing::TestWithParam<QuicVersion> { connection_ = new testing::StrictMock<MockConnection>( kIsServer, SupportedVersions(GetParam())); session_.reset(new testing::StrictMock<MockSession>(connection_)); - stream_.reset(new TestStream(kStreamId, session_.get(), - stream_should_process_data)); - stream2_.reset(new TestStream(kStreamId + 2, session_.get(), + stream_.reset(new TestStream(kClientDataStreamId1, session_.get(), stream_should_process_data)); + stream2_.reset(new TestStream(kClientDataStreamId2, session_.get(), + stream_should_process_data)); write_blocked_list_ = QuicSessionPeer::GetWriteblockedStreams(session_.get()); } @@ -140,7 +137,7 @@ TEST_P(QuicDataStreamTest, ProcessHeadersAndBody) { stream_->OnStreamHeaders(headers); EXPECT_EQ(headers, stream_->data()); stream_->OnStreamHeadersComplete(false, headers.size()); - QuicStreamFrame frame(kStreamId, false, 0, MakeIOVector(body)); + QuicStreamFrame frame(kClientDataStreamId1, false, 0, MakeIOVector(body)); stream_->OnStreamFrame(frame); EXPECT_EQ(headers + body, stream_->data()); @@ -165,7 +162,8 @@ TEST_P(QuicDataStreamTest, ProcessHeadersAndBodyFragments) { size_t remaining_data = body.size() - offset; StringPiece fragment(body.data() + offset, min(fragment_size, remaining_data)); - QuicStreamFrame frame(kStreamId, false, offset, MakeIOVector(fragment)); + QuicStreamFrame frame(kClientDataStreamId1, false, offset, + MakeIOVector(fragment)); stream_->OnStreamFrame(frame); } ASSERT_EQ(headers + body, @@ -188,13 +186,14 @@ TEST_P(QuicDataStreamTest, ProcessHeadersAndBodyFragmentsSplit) { stream_->OnStreamHeadersComplete(false, headers.size()); StringPiece fragment1(body.data(), split_point); - QuicStreamFrame frame1(kStreamId, false, 0, MakeIOVector(fragment1)); + QuicStreamFrame frame1(kClientDataStreamId1, false, 0, + MakeIOVector(fragment1)); stream_->OnStreamFrame(frame1); StringPiece fragment2(body.data() + split_point, body.size() - split_point); - QuicStreamFrame frame2( - kStreamId, false, split_point, MakeIOVector(fragment2)); + QuicStreamFrame frame2(kClientDataStreamId1, false, split_point, + MakeIOVector(fragment2)); stream_->OnStreamFrame(frame2); ASSERT_EQ(headers + body, @@ -211,7 +210,7 @@ TEST_P(QuicDataStreamTest, ProcessHeadersAndBodyReadv) { stream_->OnStreamHeaders(headers); EXPECT_EQ(headers, stream_->data()); stream_->OnStreamHeadersComplete(false, headers.size()); - QuicStreamFrame frame(kStreamId, false, 0, MakeIOVector(body)); + QuicStreamFrame frame(kClientDataStreamId1, false, 0, MakeIOVector(body)); stream_->OnStreamFrame(frame); char buffer[2048]; @@ -237,10 +236,9 @@ TEST_P(QuicDataStreamTest, ProcessHeadersAndBodyIncrementalReadv) { stream_->OnStreamHeaders(headers); EXPECT_EQ(headers, stream_->data()); stream_->OnStreamHeadersComplete(false, headers.size()); - QuicStreamFrame frame(kStreamId, false, 0, MakeIOVector(body)); + QuicStreamFrame frame(kClientDataStreamId1, false, 0, MakeIOVector(body)); stream_->OnStreamFrame(frame); - char buffer[1]; struct iovec vec; vec.iov_base = buffer; @@ -262,10 +260,9 @@ TEST_P(QuicDataStreamTest, ProcessHeadersUsingReadvWithMultipleIovecs) { stream_->OnStreamHeaders(headers); EXPECT_EQ(headers, stream_->data()); stream_->OnStreamHeadersComplete(false, headers.size()); - QuicStreamFrame frame(kStreamId, false, 0, MakeIOVector(body)); + QuicStreamFrame frame(kClientDataStreamId1, false, 0, MakeIOVector(body)); stream_->OnStreamFrame(frame); - char buffer1[1]; char buffer2[1]; struct iovec vec[2]; @@ -305,9 +302,9 @@ TEST_P(QuicDataStreamTest, StreamFlowControlBlocked) { const uint64 kOverflow = 15; GenerateBody(&body, kWindow + kOverflow); - EXPECT_CALL(*connection_, SendBlocked(kStreamId)); - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( - Return(QuicConsumedData(kWindow, true))); + EXPECT_CALL(*connection_, SendBlocked(kClientDataStreamId1)); + EXPECT_CALL(*session_, WritevData(kClientDataStreamId1, _, _, _, _)) + .WillOnce(Return(QuicConsumedData(kWindow, true))); stream_->WriteOrBufferData(body, false, NULL); // Should have sent as much as possible, resulting in no send window left. @@ -352,7 +349,7 @@ TEST_P(QuicDataStreamTest, StreamFlowControlNoWindowUpdateIfNotConsumed) { EXPECT_EQ(headers, stream_->data()); stream_->OnStreamHeadersComplete(false, headers.size()); - QuicStreamFrame frame1(kStreamId, false, 0, MakeIOVector(body)); + QuicStreamFrame frame1(kClientDataStreamId1, false, 0, MakeIOVector(body)); stream_->OnStreamFrame(frame1); EXPECT_EQ(kWindow - (kWindow / 3), QuicFlowControllerPeer::ReceiveWindowSize( stream_->flow_controller())); @@ -360,7 +357,8 @@ TEST_P(QuicDataStreamTest, StreamFlowControlNoWindowUpdateIfNotConsumed) { // Now receive another frame which results in the receive window being over // half full. This should all be buffered, decreasing the receive window but // not sending WINDOW_UPDATE. - QuicStreamFrame frame2(kStreamId, false, kWindow / 3, MakeIOVector(body)); + QuicStreamFrame frame2(kClientDataStreamId1, false, kWindow / 3, + MakeIOVector(body)); stream_->OnStreamFrame(frame2); EXPECT_EQ( kWindow - (2 * kWindow / 3), @@ -395,7 +393,7 @@ TEST_P(QuicDataStreamTest, StreamFlowControlWindowUpdate) { EXPECT_EQ(headers, stream_->data()); stream_->OnStreamHeadersComplete(false, headers.size()); - QuicStreamFrame frame1(kStreamId, false, 0, MakeIOVector(body)); + QuicStreamFrame frame1(kClientDataStreamId1, false, 0, MakeIOVector(body)); stream_->OnStreamFrame(frame1); EXPECT_EQ(kWindow - (kWindow / 3), QuicFlowControllerPeer::ReceiveWindowSize( stream_->flow_controller())); @@ -404,12 +402,13 @@ TEST_P(QuicDataStreamTest, StreamFlowControlWindowUpdate) { // half full. This will trigger the stream to increase its receive window // offset and send a WINDOW_UPDATE. The result will be again an available // window of kWindow bytes. - QuicStreamFrame frame2(kStreamId, false, kWindow / 3, MakeIOVector(body)); - EXPECT_CALL( - *connection_, - SendWindowUpdate(kStreamId, QuicFlowControllerPeer::ReceiveWindowOffset( - stream_->flow_controller()) + - 2 * kWindow / 3)); + QuicStreamFrame frame2(kClientDataStreamId1, false, kWindow / 3, + MakeIOVector(body)); + EXPECT_CALL(*connection_, + SendWindowUpdate(kClientDataStreamId1, + QuicFlowControllerPeer::ReceiveWindowOffset( + stream_->flow_controller()) + + 2 * kWindow / 3)); stream_->OnStreamFrame(frame2); EXPECT_EQ(kWindow, QuicFlowControllerPeer::ReceiveWindowSize( stream_->flow_controller())); @@ -453,21 +452,22 @@ TEST_P(QuicDataStreamTest, ConnectionFlowControlWindowUpdate) { // WINDOW_UPDATE for either stream, nor for the connection. string body; GenerateBody(&body, kWindow / 4); - QuicStreamFrame frame1(kStreamId, false, 0, MakeIOVector(body)); + QuicStreamFrame frame1(kClientDataStreamId1, false, 0, MakeIOVector(body)); stream_->OnStreamFrame(frame1); - QuicStreamFrame frame2(kStreamId + 2, false, 0, MakeIOVector(body)); + QuicStreamFrame frame2(kClientDataStreamId2, false, 0, MakeIOVector(body)); stream2_->OnStreamFrame(frame2); // Now receive a further single byte on one stream - again this does not // trigger a stream WINDOW_UPDATE, but now the connection flow control window // is over half full and thus a connection WINDOW_UPDATE is sent. - EXPECT_CALL(*connection_, SendWindowUpdate(kStreamId, _)).Times(0); - EXPECT_CALL(*connection_, SendWindowUpdate(kStreamId + 2, _)).Times(0); + EXPECT_CALL(*connection_, SendWindowUpdate(kClientDataStreamId1, _)).Times(0); + EXPECT_CALL(*connection_, SendWindowUpdate(kClientDataStreamId2, _)).Times(0); EXPECT_CALL(*connection_, SendWindowUpdate(0, QuicFlowControllerPeer::ReceiveWindowOffset( session_->flow_controller()) + 1 + kWindow / 2)); - QuicStreamFrame frame3(kStreamId, false, (kWindow / 4), MakeIOVector("a")); + QuicStreamFrame frame3(kClientDataStreamId1, false, (kWindow / 4), + MakeIOVector("a")); stream_->OnStreamFrame(frame3); } @@ -496,7 +496,7 @@ TEST_P(QuicDataStreamTest, StreamFlowControlViolation) { // Receive data to overflow the window, violating flow control. string body; GenerateBody(&body, kWindow + 1); - QuicStreamFrame frame(kStreamId, false, 0, MakeIOVector(body)); + QuicStreamFrame frame(kClientDataStreamId1, false, 0, MakeIOVector(body)); EXPECT_CALL(*connection_, SendConnectionClose(QUIC_FLOW_CONTROL_ERROR)); stream_->OnStreamFrame(frame); } @@ -532,7 +532,7 @@ TEST_P(QuicDataStreamTest, ConnectionFlowControlViolation) { string body; GenerateBody(&body, kConnectionWindow + 1); EXPECT_LT(body.size(), kStreamWindow); - QuicStreamFrame frame(kStreamId, false, 0, MakeIOVector(body)); + QuicStreamFrame frame(kClientDataStreamId1, false, 0, MakeIOVector(body)); EXPECT_CALL(*connection_, SendConnectionClose(QUIC_FLOW_CONTROL_ERROR)); stream_->OnStreamFrame(frame); @@ -557,9 +557,9 @@ TEST_P(QuicDataStreamTest, StreamFlowControlFinNotBlocked) { string body = ""; bool fin = true; - EXPECT_CALL(*connection_, SendBlocked(kStreamId)).Times(0); - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( - Return(QuicConsumedData(0, fin))); + EXPECT_CALL(*connection_, SendBlocked(kClientDataStreamId1)).Times(0); + EXPECT_CALL(*session_, WritevData(kClientDataStreamId1, _, _, _, _)) + .WillOnce(Return(QuicConsumedData(0, fin))); stream_->WriteOrBufferData(body, fin, NULL); } diff --git a/net/quic/quic_headers_stream_test.cc b/net/quic/quic_headers_stream_test.cc index 4022073..8ce730a 100644 --- a/net/quic/quic_headers_stream_test.cc +++ b/net/quic/quic_headers_stream_test.cc @@ -194,7 +194,8 @@ TEST_P(QuicHeadersStreamTest, EffectivePriority) { } TEST_P(QuicHeadersStreamTest, WriteHeaders) { - for (QuicStreamId stream_id = 5; stream_id < 9; stream_id +=2) { + for (QuicStreamId stream_id = kClientDataStreamId1; + stream_id < kClientDataStreamId3; stream_id += 2) { for (int count = 0; count < 2; ++count) { bool fin = (count == 0); if (is_server()) { @@ -209,7 +210,8 @@ TEST_P(QuicHeadersStreamTest, WriteHeaders) { } TEST_P(QuicHeadersStreamTest, ProcessRawData) { - for (QuicStreamId stream_id = 5; stream_id < 9; stream_id +=2) { + for (QuicStreamId stream_id = kClientDataStreamId1; + stream_id < kClientDataStreamId3; stream_id += 2) { for (int count = 0; count < 2; ++count) { bool fin = (count == 0); for (QuicPriority priority = 0; priority < 7; ++priority) { diff --git a/net/quic/quic_http_stream_test.cc b/net/quic/quic_http_stream_test.cc index 206c00d..d413b66 100644 --- a/net/quic/quic_http_stream_test.cc +++ b/net/quic/quic_http_stream_test.cc @@ -129,7 +129,7 @@ class QuicHttpStreamTest : public ::testing::TestWithParam<QuicVersion> { use_closing_stream_(false), read_buffer_(new IOBufferWithSize(4096)), connection_id_(2), - stream_id_(5), + stream_id_(kClientDataStreamId1), maker_(GetParam(), connection_id_), random_generator_(0) { IPAddressNumber ip; diff --git a/net/quic/quic_network_transaction_unittest.cc b/net/quic/quic_network_transaction_unittest.cc index 3ffa159..c5994a1 100644 --- a/net/quic/quic_network_transaction_unittest.cc +++ b/net/quic/quic_network_transaction_unittest.cc @@ -54,9 +54,6 @@ static const char kQuicAlternateProtocolHttpHeader[] = static const char kQuicAlternateProtocolHttpsHeader[] = "Alternate-Protocol: 443:quic\r\n\r\n"; -// Used by the QuicNetworkTransactionTest unit tests. -const net::QuicStreamId kStreamId5 = 5; - } // namespace namespace net { @@ -324,13 +321,13 @@ TEST_P(QuicNetworkTransactionTest, ForceQuic) { MockQuicData mock_quic_data; mock_quic_data.AddWrite( - ConstructRequestHeadersPacket(1, kStreamId5, true, true, + ConstructRequestHeadersPacket(1, kClientDataStreamId1, true, true, GetRequestHeaders("GET", "http", "/"))); mock_quic_data.AddRead( - ConstructResponseHeadersPacket(1, kStreamId5, false, false, + ConstructResponseHeadersPacket(1, kClientDataStreamId1, false, false, GetResponseHeaders("200 OK"))); mock_quic_data.AddRead( - ConstructDataPacket(2, kStreamId5, false, true, 0, "hello!")); + ConstructDataPacket(2, kClientDataStreamId1, false, true, 0, "hello!")); mock_quic_data.AddWrite(ConstructAckPacket(2, 1)); mock_quic_data.AddRead(SYNCHRONOUS, 0); // EOF @@ -386,13 +383,13 @@ TEST_P(QuicNetworkTransactionTest, QuicProxy) { MockQuicData mock_quic_data; mock_quic_data.AddWrite( - ConstructRequestHeadersPacket(1, kStreamId5, true, true, + ConstructRequestHeadersPacket(1, kClientDataStreamId1, true, true, GetRequestHeaders("GET", "http", "/"))); mock_quic_data.AddRead( - ConstructResponseHeadersPacket(1, kStreamId5, false, false, + ConstructResponseHeadersPacket(1, kClientDataStreamId1, false, false, GetResponseHeaders("200 OK"))); mock_quic_data.AddRead( - ConstructDataPacket(2, kStreamId5, false, true, 0, "hello!")); + ConstructDataPacket(2, kClientDataStreamId1, false, true, 0, "hello!")); mock_quic_data.AddWrite(ConstructAckPacket(2, 1)); mock_quic_data.AddRead(SYNCHRONOUS, 0); // EOF @@ -464,13 +461,13 @@ TEST_P(QuicNetworkTransactionTest, UseAlternateProtocolForQuic) { MockQuicData mock_quic_data; mock_quic_data.AddWrite( - ConstructRequestHeadersPacket(1, kStreamId5, true, true, + ConstructRequestHeadersPacket(1, kClientDataStreamId1, true, true, GetRequestHeaders("GET", "http", "/"))); mock_quic_data.AddRead( - ConstructResponseHeadersPacket(1, kStreamId5, false, false, + ConstructResponseHeadersPacket(1, kClientDataStreamId1, false, false, GetResponseHeaders("200 OK"))); mock_quic_data.AddRead( - ConstructDataPacket(2, kStreamId5, false, true, 0, "hello!")); + ConstructDataPacket(2, kClientDataStreamId1, false, true, 0, "hello!")); mock_quic_data.AddWrite(ConstructAckPacket(2, 1)); mock_quic_data.AddRead(SYNCHRONOUS, 0); // EOF @@ -506,13 +503,13 @@ TEST_P(QuicNetworkTransactionTest, UseAlternateProtocolForQuicForHttps) { MockQuicData mock_quic_data; mock_quic_data.AddWrite( - ConstructRequestHeadersPacket(1, kStreamId5, true, true, + ConstructRequestHeadersPacket(1, kClientDataStreamId1, true, true, GetRequestHeaders("GET", "http", "/"))); mock_quic_data.AddRead( - ConstructResponseHeadersPacket(1, kStreamId5, false, false, + ConstructResponseHeadersPacket(1, kClientDataStreamId1, false, false, GetResponseHeaders("200 OK"))); mock_quic_data.AddRead( - ConstructDataPacket(2, kStreamId5, false, true, 0, "hello!")); + ConstructDataPacket(2, kClientDataStreamId1, false, true, 0, "hello!")); mock_quic_data.AddWrite(ConstructAckPacket(2, 1)); mock_quic_data.AddRead(SYNCHRONOUS, 0); // EOF @@ -592,13 +589,13 @@ TEST_P(QuicNetworkTransactionTest, ZeroRTTWithHttpRace) { MockQuicData mock_quic_data; mock_quic_data.AddWrite( - ConstructRequestHeadersPacket(1, kStreamId5, true, true, + ConstructRequestHeadersPacket(1, kClientDataStreamId1, true, true, GetRequestHeaders("GET", "http", "/"))); mock_quic_data.AddRead( - ConstructResponseHeadersPacket(1, kStreamId5, false, false, + ConstructResponseHeadersPacket(1, kClientDataStreamId1, false, false, GetResponseHeaders("200 OK"))); mock_quic_data.AddRead( - ConstructDataPacket(2, kStreamId5, false, true, 0, "hello!")); + ConstructDataPacket(2, kClientDataStreamId1, false, true, 0, "hello!")); mock_quic_data.AddWrite(ConstructAckPacket(2, 1)); mock_quic_data.AddRead(SYNCHRONOUS, 0); // EOF @@ -618,13 +615,13 @@ TEST_P(QuicNetworkTransactionTest, ZeroRTTWithNoHttpRace) { MockQuicData mock_quic_data; mock_quic_data.AddWrite( - ConstructRequestHeadersPacket(1, kStreamId5, true, true, + ConstructRequestHeadersPacket(1, kClientDataStreamId1, true, true, GetRequestHeaders("GET", "http", "/"))); mock_quic_data.AddRead( - ConstructResponseHeadersPacket(1, kStreamId5, false, false, + ConstructResponseHeadersPacket(1, kClientDataStreamId1, false, false, GetResponseHeaders("200 OK"))); mock_quic_data.AddRead( - ConstructDataPacket(2, kStreamId5, false, true, 0, "hello!")); + ConstructDataPacket(2, kClientDataStreamId1, false, true, 0, "hello!")); mock_quic_data.AddWrite(ConstructAckPacket(2, 1)); mock_quic_data.AddRead(SYNCHRONOUS, 0); // EOF mock_quic_data.AddDelayedSocketDataToFactory(&socket_factory_, 1); @@ -695,13 +692,13 @@ TEST_P(QuicNetworkTransactionTest, ZeroRTTWithConfirmationRequired) { MockQuicData mock_quic_data; mock_quic_data.AddWrite( - ConstructRequestHeadersPacket(1, kStreamId5, true, true, + ConstructRequestHeadersPacket(1, kClientDataStreamId1, true, true, GetRequestHeaders("GET", "http", "/"))); mock_quic_data.AddRead( - ConstructResponseHeadersPacket(1, kStreamId5, false, false, + ConstructResponseHeadersPacket(1, kClientDataStreamId1, false, false, GetResponseHeaders("200 OK"))); mock_quic_data.AddRead( - ConstructDataPacket(2, kStreamId5, false, true, 0, "hello!")); + ConstructDataPacket(2, kClientDataStreamId1, false, true, 0, "hello!")); mock_quic_data.AddWrite(ConstructAckPacket(2, 1)); mock_quic_data.AddRead(SYNCHRONOUS, 0); // EOF mock_quic_data.AddDelayedSocketDataToFactory(&socket_factory_, 1); @@ -902,7 +899,7 @@ TEST_P(QuicNetworkTransactionTest, ConnectionCloseDuringConnect) { MockQuicData mock_quic_data; mock_quic_data.AddRead(ConstructConnectionClosePacket(1)); mock_quic_data.AddWrite( - ConstructRequestHeadersPacket(1, kStreamId5, true, true, + ConstructRequestHeadersPacket(1, kClientDataStreamId1, true, true, GetRequestHeaders("GET", "http", "/"))); mock_quic_data.AddWrite(ConstructAckPacket(2, 1)); mock_quic_data.AddDelayedSocketDataToFactory(&socket_factory_, 0); diff --git a/net/quic/quic_packet_creator_test.cc b/net/quic/quic_packet_creator_test.cc index 72faf7e..3e3b228 100644 --- a/net/quic/quic_packet_creator_test.cc +++ b/net/quic/quic_packet_creator_test.cc @@ -116,10 +116,10 @@ class QuicPacketCreatorTest : public ::testing::TestWithParam<TestParams> { // Returns the number of bytes consumed by the non-data fields of a stream // frame, assuming it is the last frame in the packet size_t GetStreamFrameOverhead(InFecGroup is_in_fec_group) { - return QuicFramer::GetMinStreamFrameSize( - client_framer_.version(), kStreamId, kOffset, true, is_in_fec_group); + return QuicFramer::GetMinStreamFrameSize(client_framer_.version(), + kClientDataStreamId1, kOffset, + true, is_in_fec_group); } - static const QuicStreamId kStreamId = 1u; static const QuicStreamOffset kOffset = 1u; QuicFrames frames_; @@ -503,12 +503,13 @@ TEST_P(QuicPacketCreatorTest, CreateAllFreeBytesForStreamFrames) { creator_.options()->max_packet_length = i; const bool should_have_room = i > overhead + GetStreamFrameOverhead( NOT_IN_FEC_GROUP); - ASSERT_EQ(should_have_room, - creator_.HasRoomForStreamFrame(kStreamId, kOffset)); + ASSERT_EQ(should_have_room, creator_.HasRoomForStreamFrame( + kClientDataStreamId1, kOffset)); if (should_have_room) { QuicFrame frame; size_t bytes_consumed = creator_.CreateStreamFrame( - kStreamId, MakeIOVector("testdata"), kOffset, false, &frame); + kClientDataStreamId1, MakeIOVector("testdata"), kOffset, false, + &frame); EXPECT_LT(0u, bytes_consumed); ASSERT_TRUE(creator_.AddSavedFrame(frame)); SerializedPacket serialized_packet = creator_.SerializePacket(); @@ -530,7 +531,7 @@ TEST_P(QuicPacketCreatorTest, StreamFrameConsumption) { size_t bytes_free = delta > 0 ? 0 : 0 - delta; QuicFrame frame; size_t bytes_consumed = creator_.CreateStreamFrame( - kStreamId, MakeIOVector(data), kOffset, false, &frame); + kClientDataStreamId1, MakeIOVector(data), kOffset, false, &frame); EXPECT_EQ(capacity - bytes_free, bytes_consumed); ASSERT_TRUE(creator_.AddSavedFrame(frame)); @@ -559,7 +560,7 @@ TEST_P(QuicPacketCreatorTest, StreamFrameConsumptionWithFec) { size_t bytes_free = delta > 0 ? 0 : 0 - delta; QuicFrame frame; size_t bytes_consumed = creator_.CreateStreamFrame( - kStreamId, MakeIOVector(data), kOffset, false, &frame); + kClientDataStreamId1, MakeIOVector(data), kOffset, false, &frame); EXPECT_EQ(capacity - bytes_free, bytes_consumed); ASSERT_TRUE(creator_.AddSavedFrame(frame)); @@ -589,7 +590,7 @@ TEST_P(QuicPacketCreatorTest, CryptoStreamFramePacketPadding) { QuicFrame frame; size_t bytes_consumed = creator_.CreateStreamFrame( - kStreamId, MakeIOVector(data), kOffset, false, &frame); + kCryptoStreamId, MakeIOVector(data), kOffset, false, &frame); EXPECT_LT(0u, bytes_consumed); ASSERT_TRUE(creator_.AddSavedFrame(frame)); SerializedPacket serialized_packet = creator_.SerializePacket(); @@ -622,7 +623,7 @@ TEST_P(QuicPacketCreatorTest, NonCryptoStreamFramePacketNonPadding) { QuicFrame frame; size_t bytes_consumed = creator_.CreateStreamFrame( - kStreamId + 2, MakeIOVector(data), kOffset, false, &frame); + kClientDataStreamId1, MakeIOVector(data), kOffset, false, &frame); EXPECT_LT(0u, bytes_consumed); ASSERT_TRUE(creator_.AddSavedFrame(frame)); SerializedPacket serialized_packet = creator_.SerializePacket(); diff --git a/net/quic/quic_received_packet_manager.cc b/net/quic/quic_received_packet_manager.cc index 49a056d5..3b47687 100644 --- a/net/quic/quic_received_packet_manager.cc +++ b/net/quic/quic_received_packet_manager.cc @@ -26,12 +26,110 @@ const size_t kMaxPacketsAfterNewMissing = 4; } +QuicReceivedPacketManager::EntropyTracker::EntropyTracker() + : packets_entropy_hash_(0), + first_gap_(1), + largest_observed_(0) { +} + +QuicReceivedPacketManager::EntropyTracker::~EntropyTracker() {} + +QuicPacketEntropyHash QuicReceivedPacketManager::EntropyTracker::EntropyHash( + QuicPacketSequenceNumber sequence_number) const { + DCHECK_LE(sequence_number, largest_observed_); + if (sequence_number == largest_observed_) { + return packets_entropy_hash_; + } + + DCHECK_GE(sequence_number, first_gap_); + ReceivedEntropyMap::const_iterator it = + packets_entropy_.upper_bound(sequence_number); + // When this map is empty we should only query entropy for + // largest_observed_, since no other entropy can be correctly + // calculated, because we're not storing the entropy for any prior packets. + // TODO(rtenneti): add support for LOG_IF_EVERY_N_SEC to chromium. + // LOG_IF_EVERY_N_SEC(DFATAL, it == packets_entropy_.end(), 10) + LOG_IF(DFATAL, it == packets_entropy_.end()) + << "EntropyHash may be unknown. largest_received: " + << largest_observed_ + << " sequence_number: " << sequence_number; + + // TODO(satyamshekhar): Make this O(1). + QuicPacketEntropyHash hash = packets_entropy_hash_; + for (; it != packets_entropy_.end(); ++it) { + hash ^= it->second; + } + return hash; +} + +void QuicReceivedPacketManager::EntropyTracker::RecordPacketEntropyHash( + QuicPacketSequenceNumber sequence_number, + QuicPacketEntropyHash entropy_hash) { + if (sequence_number < first_gap_) { + DVLOG(1) << "Ignoring received packet entropy for sequence_number:" + << sequence_number << " less than largest_peer_sequence_number:" + << first_gap_; + return; + } + + if (sequence_number > largest_observed_) { + largest_observed_ = sequence_number; + } + + packets_entropy_hash_ ^= entropy_hash; + DVLOG(2) << "setting cumulative received entropy hash to: " + << static_cast<int>(packets_entropy_hash_) + << " updated with sequence number " << sequence_number + << " entropy hash: " << static_cast<int>(entropy_hash); + + packets_entropy_.insert(make_pair(sequence_number, entropy_hash)); + AdvanceFirstGapAndGarbageCollectEntropyMap(); +} + +void QuicReceivedPacketManager::EntropyTracker::SetCumulativeEntropyUpTo( + QuicPacketSequenceNumber sequence_number, + QuicPacketEntropyHash entropy_hash) { + DCHECK_LE(sequence_number, largest_observed_); + if (sequence_number < first_gap_) { + DVLOG(1) << "Ignoring set entropy at:" << sequence_number + << " less than first_gap_:" << first_gap_; + return; + } + // Compute the current entropy based on the hash. + packets_entropy_hash_ = entropy_hash; + ReceivedEntropyMap::iterator it = + packets_entropy_.lower_bound(sequence_number); + // TODO(satyamshekhar): Make this O(1). + for (; it != packets_entropy_.end(); ++it) { + packets_entropy_hash_ ^= it->second; + } + // Update first_gap_ and discard old entropies. + first_gap_ = sequence_number; + packets_entropy_.erase( + packets_entropy_.begin(), + packets_entropy_.lower_bound(sequence_number)); + + // Garbage collect entries from the beginning of the map. + AdvanceFirstGapAndGarbageCollectEntropyMap(); +} + +void QuicReceivedPacketManager::EntropyTracker:: +AdvanceFirstGapAndGarbageCollectEntropyMap() { + while (!packets_entropy_.empty()) { + ReceivedEntropyMap::iterator it = packets_entropy_.begin(); + if (it->first != first_gap_) { + DCHECK_GT(it->first, first_gap_); + break; + } + packets_entropy_.erase(it); + ++first_gap_; + } +} + QuicReceivedPacketManager::QuicReceivedPacketManager( CongestionFeedbackType congestion_type, QuicConnectionStats* stats) - : packets_entropy_hash_(0), - largest_sequence_number_(0), - peer_largest_observed_packet_(0), + : peer_largest_observed_packet_(0), least_packet_awaited_by_peer_(1), peer_least_packet_awaiting_ack_(0), time_largest_observed_(QuicTime::Zero()), @@ -53,9 +151,9 @@ void QuicReceivedPacketManager::RecordPacketReceived( InsertMissingPacketsBetween( &received_info_, max(received_info_.largest_observed + 1, peer_least_packet_awaiting_ack_), - header.packet_sequence_number); + sequence_number); - if (received_info_.largest_observed > header.packet_sequence_number) { + if (received_info_.largest_observed > sequence_number) { // We've gotten one of the out of order packets - remove it from our // "missing packets" list. DVLOG(1) << "Removing " << sequence_number << " from missing list"; @@ -71,11 +169,12 @@ void QuicReceivedPacketManager::RecordPacketReceived( stats_->max_time_reordering_us = max(stats_->max_time_reordering_us, reordering_time_us); } - if (header.packet_sequence_number > received_info_.largest_observed) { - received_info_.largest_observed = header.packet_sequence_number; + if (sequence_number > received_info_.largest_observed) { + received_info_.largest_observed = sequence_number; time_largest_observed_ = receipt_time; } - RecordPacketEntropyHash(sequence_number, header.entropy_hash); + entropy_tracker_.RecordPacketEntropyHash(sequence_number, + header.entropy_hash); receive_algorithm_->RecordIncomingPacket( bytes, sequence_number, receipt_time); @@ -119,23 +218,6 @@ void QuicReceivedPacketManager::UpdateReceivedPacketInfo( approximate_now.Subtract(time_largest_observed_); } -void QuicReceivedPacketManager::RecordPacketEntropyHash( - QuicPacketSequenceNumber sequence_number, - QuicPacketEntropyHash entropy_hash) { - if (sequence_number < largest_sequence_number_) { - DVLOG(1) << "Ignoring received packet entropy for sequence_number:" - << sequence_number << " less than largest_peer_sequence_number:" - << largest_sequence_number_; - return; - } - packets_entropy_.insert(make_pair(sequence_number, entropy_hash)); - packets_entropy_hash_ ^= entropy_hash; - DVLOG(2) << "setting cumulative received entropy hash to: " - << static_cast<int>(packets_entropy_hash_) - << " updated with sequence number " << sequence_number - << " entropy hash: " << static_cast<int>(entropy_hash); -} - bool QuicReceivedPacketManager::GenerateCongestionFeedback( QuicCongestionFeedbackFrame* feedback) { return receive_algorithm_->GenerateCongestionFeedback(feedback); @@ -143,55 +225,7 @@ bool QuicReceivedPacketManager::GenerateCongestionFeedback( QuicPacketEntropyHash QuicReceivedPacketManager::EntropyHash( QuicPacketSequenceNumber sequence_number) const { - DCHECK_LE(sequence_number, received_info_.largest_observed); - DCHECK_GE(sequence_number, largest_sequence_number_); - if (sequence_number == received_info_.largest_observed) { - return packets_entropy_hash_; - } - - ReceivedEntropyMap::const_iterator it = - packets_entropy_.upper_bound(sequence_number); - // When this map is empty we should only query entropy for - // received_info_.largest_observed, since no other entropy can be correctly - // calculated, because we're not storing the entropy for any prior packets. - // TODO(rtenneti): add support for LOG_IF_EVERY_N_SEC to chromium. - // LOG_IF_EVERY_N_SEC(DFATAL, it == packets_entropy_.end(), 10) - LOG_IF(DFATAL, it == packets_entropy_.end()) - << "EntropyHash may be unknown. largest_received: " - << received_info_.largest_observed - << " sequence_number: " << sequence_number; - - // TODO(satyamshekhar): Make this O(1). - QuicPacketEntropyHash hash = packets_entropy_hash_; - for (; it != packets_entropy_.end(); ++it) { - hash ^= it->second; - } - return hash; -} - -void QuicReceivedPacketManager::RecalculateEntropyHash( - QuicPacketSequenceNumber peer_least_unacked, - QuicPacketEntropyHash entropy_hash) { - DCHECK_LE(peer_least_unacked, received_info_.largest_observed); - if (peer_least_unacked < largest_sequence_number_) { - DVLOG(1) << "Ignoring received peer_least_unacked:" << peer_least_unacked - << " less than largest_peer_sequence_number:" - << largest_sequence_number_; - return; - } - largest_sequence_number_ = peer_least_unacked; - packets_entropy_hash_ = entropy_hash; - ReceivedEntropyMap::iterator it = - packets_entropy_.lower_bound(peer_least_unacked); - // TODO(satyamshekhar): Make this O(1). - for (; it != packets_entropy_.end(); ++it) { - packets_entropy_hash_ ^= it->second; - } - // Discard entropies before least unacked. - packets_entropy_.erase( - packets_entropy_.begin(), - packets_entropy_.lower_bound( - min(peer_least_unacked, received_info_.largest_observed))); + return entropy_tracker_.EntropyHash(sequence_number); } void QuicReceivedPacketManager::UpdatePacketInformationReceivedByPeer( @@ -225,13 +259,12 @@ void QuicReceivedPacketManager::UpdatePacketInformationSentByPeer( DCHECK_LE(peer_least_packet_awaiting_ack_, stop_waiting.least_unacked); if (stop_waiting.least_unacked > peer_least_packet_awaiting_ack_) { bool missed_packets = DontWaitForPacketsBefore(stop_waiting.least_unacked); - if (missed_packets || stop_waiting.least_unacked > - received_info_.largest_observed + 1) { + if (missed_packets) { DVLOG(1) << "Updating entropy hashed since we missed packets"; // There were some missing packets that we won't ever get now. Recalculate // the received entropy hash. - RecalculateEntropyHash(stop_waiting.least_unacked, - stop_waiting.entropy_hash); + entropy_tracker_.SetCumulativeEntropyUpTo(stop_waiting.least_unacked, + stop_waiting.entropy_hash); } peer_least_packet_awaiting_ack_ = stop_waiting.least_unacked; } diff --git a/net/quic/quic_received_packet_manager.h b/net/quic/quic_received_packet_manager.h index 05339f1..1b28331 100644 --- a/net/quic/quic_received_packet_manager.h +++ b/net/quic/quic_received_packet_manager.h @@ -15,6 +15,7 @@ namespace net { namespace test { +class EntropyTrackerPeer; class QuicConnectionPeer; class QuicReceivedPacketManagerPeer; } // namespace test @@ -27,6 +28,72 @@ struct QuicConnectionStats; class NET_EXPORT_PRIVATE QuicReceivedPacketManager : public QuicReceivedEntropyHashCalculatorInterface { public: + class NET_EXPORT_PRIVATE EntropyTracker { + public: + EntropyTracker(); + ~EntropyTracker(); + + // Compute the XOR of the entropy of all received packets up to + // and including sequence_number. + // Requires that either: + // sequence_number == largest_observed_ + // or: + // sequence_number > first_gap_ && + // sequence_number < largest_observed_ && + // sequence_number in packets_entropy_ + QuicPacketEntropyHash EntropyHash( + QuicPacketSequenceNumber sequence_number) const; + + // Record the received entropy hash against |sequence_number|. + // Performs garbage collection to advance first_gap_ if + // sequence_number == first_gap_. + void RecordPacketEntropyHash(QuicPacketSequenceNumber sequence_number, + QuicPacketEntropyHash entropy_hash); + + // Sets the entropy hash up to but not including a sequence number based + // on the hash provided by a StopWaiting frame. Clears older packet + // entropy entries and performs garbage collection up to the first gap. + void SetCumulativeEntropyUpTo(QuicPacketSequenceNumber sequence_number, + QuicPacketEntropyHash entropy_hash); + + private: + friend class test::EntropyTrackerPeer; + + typedef std::map<QuicPacketSequenceNumber, + QuicPacketEntropyHash> ReceivedEntropyMap; + + // Recomputes first_gap_ and removes packets_entropy_ entries that are no + // longer needed to compute EntropyHash. + void AdvanceFirstGapAndGarbageCollectEntropyMap(); + + // TODO(satyamshekhar): Can be optimized using an interval set like data + // structure. + // Map of received sequence numbers to their corresponding entropy. + // Stores an entry for every received packet whose sequence_number is larger + // than first_gap_. Packets without the entropy bit set have an entropy + // value of 0. + // TODO(ianswett): When the entropy flag is off, the entropy + // should not be 0. + ReceivedEntropyMap packets_entropy_; + + // Cumulative hash of entropy of all received packets. + QuicPacketEntropyHash packets_entropy_hash_; + + // Sequence number of the first packet that we do not know the entropy of. + // If there are no gaps in the received packet sequence, + // packets_entropy_ will be empty and first_gap_ will be equal to + // 'largest_observed_ + 1' since that's the first packet for which + // entropy is unknown. If there are gaps, packets_entropy_ will + // contain entries for all received packets with sequence_number > + // first_gap_. + QuicPacketSequenceNumber first_gap_; + + // Sequence number of the largest observed packet. + QuicPacketSequenceNumber largest_observed_; + + DISALLOW_COPY_AND_ASSIGN(EntropyTracker); + }; + explicit QuicReceivedPacketManager(CongestionFeedbackType congestion_type, QuicConnectionStats* stats); virtual ~QuicReceivedPacketManager(); @@ -94,40 +161,14 @@ class NET_EXPORT_PRIVATE QuicReceivedPacketManager : friend class test::QuicConnectionPeer; friend class test::QuicReceivedPacketManagerPeer; - typedef std::map<QuicPacketSequenceNumber, - QuicPacketEntropyHash> ReceivedEntropyMap; - - // Record the received entropy hash against |sequence_number|. - void RecordPacketEntropyHash(QuicPacketSequenceNumber sequence_number, - QuicPacketEntropyHash entropy_hash); - - // Recalculate the entropy hash and clears old packet entropies, - // now that the sender sent us the |entropy_hash| for packets up to, - // but not including, |peer_least_unacked|. - void RecalculateEntropyHash(QuicPacketSequenceNumber peer_least_unacked, - QuicPacketEntropyHash entropy_hash); - // Deletes all missing packets before least unacked. The connection won't // process any packets with sequence number before |least_unacked| that it // received after this call. Returns true if there were missing packets before // |least_unacked| unacked, false otherwise. bool DontWaitForPacketsBefore(QuicPacketSequenceNumber least_unacked); - // TODO(satyamshekhar): Can be optimized using an interval set like data - // structure. - // Map of received sequence numbers to their corresponding entropy. - // Every received packet has an entry, and packets without the entropy bit set - // have an entropy value of 0. - // TODO(ianswett): When the entropy flag is off, the entropy should not be 0. - ReceivedEntropyMap packets_entropy_; - - // Cumulative hash of entropy of all received packets. - QuicPacketEntropyHash packets_entropy_hash_; - - // The largest sequence number cleared by RecalculateEntropyHash. - // Received entropy cannot be calculated for numbers less than it. - QuicPacketSequenceNumber largest_sequence_number_; - + // Tracks entropy hashes of received packets. + EntropyTracker entropy_tracker_; // Track some peer state so we can do less bookkeeping. // Largest sequence number that the peer has observed. Mostly received, diff --git a/net/quic/quic_received_packet_manager_test.cc b/net/quic/quic_received_packet_manager_test.cc index 1b893b7..30c84c5 100644 --- a/net/quic/quic_received_packet_manager_test.cc +++ b/net/quic/quic_received_packet_manager_test.cc @@ -18,8 +18,172 @@ using std::vector; namespace net { namespace test { + +class EntropyTrackerPeer { + public: + static QuicPacketSequenceNumber first_gap( + const QuicReceivedPacketManager::EntropyTracker& tracker) { + return tracker.first_gap_; + } + static QuicPacketSequenceNumber largest_observed( + const QuicReceivedPacketManager::EntropyTracker& tracker) { + return tracker.largest_observed_; + } + static int packets_entropy_size( + const QuicReceivedPacketManager::EntropyTracker& tracker) { + return tracker.packets_entropy_.size(); + } + static bool IsTrackingPacket( + const QuicReceivedPacketManager::EntropyTracker& tracker, + QuicPacketSequenceNumber sequence_number) { + return tracker.packets_entropy_.find(sequence_number) != + tracker.packets_entropy_.end(); + } +}; + namespace { +// Entropy of individual packets is not tracked if there are no gaps. +TEST(EntropyTrackerTest, NoGaps) { + QuicReceivedPacketManager::EntropyTracker tracker; + + tracker.RecordPacketEntropyHash(1, 23); + tracker.RecordPacketEntropyHash(2, 42); + + EXPECT_EQ(23 ^ 42, tracker.EntropyHash(2)); + EXPECT_EQ(3u, EntropyTrackerPeer::first_gap(tracker)); + + EXPECT_EQ(2u, EntropyTrackerPeer::largest_observed(tracker)); + EXPECT_EQ(0, EntropyTrackerPeer::packets_entropy_size(tracker)); + EXPECT_FALSE(EntropyTrackerPeer::IsTrackingPacket(tracker, 1)); + EXPECT_FALSE(EntropyTrackerPeer::IsTrackingPacket(tracker, 2)); +} + +// Entropy of individual packets is tracked as long as there are gaps. +// Filling the first gap results in entropy getting garbage collected. +TEST(EntropyTrackerTest, FillGaps) { + QuicReceivedPacketManager::EntropyTracker tracker; + + tracker.RecordPacketEntropyHash(2, 5); + tracker.RecordPacketEntropyHash(5, 17); + tracker.RecordPacketEntropyHash(6, 23); + tracker.RecordPacketEntropyHash(9, 42); + + EXPECT_EQ(1u, EntropyTrackerPeer::first_gap(tracker)); + EXPECT_EQ(9u, EntropyTrackerPeer::largest_observed(tracker)); + EXPECT_EQ(4, EntropyTrackerPeer::packets_entropy_size(tracker)); + + EXPECT_EQ(5, tracker.EntropyHash(2)); + EXPECT_EQ(5 ^ 17, tracker.EntropyHash(5)); + EXPECT_EQ(5 ^ 17 ^ 23, tracker.EntropyHash(6)); + EXPECT_EQ(5 ^ 17 ^ 23 ^ 42, tracker.EntropyHash(9)); + + EXPECT_FALSE(EntropyTrackerPeer::IsTrackingPacket(tracker, 1)); + EXPECT_TRUE(EntropyTrackerPeer::IsTrackingPacket(tracker, 2)); + EXPECT_TRUE(EntropyTrackerPeer::IsTrackingPacket(tracker, 5)); + EXPECT_TRUE(EntropyTrackerPeer::IsTrackingPacket(tracker, 6)); + EXPECT_TRUE(EntropyTrackerPeer::IsTrackingPacket(tracker, 9)); + + // Fill the gap at 1. + tracker.RecordPacketEntropyHash(1, 2); + + EXPECT_EQ(3u, EntropyTrackerPeer::first_gap(tracker)); + EXPECT_EQ(9u, EntropyTrackerPeer::largest_observed(tracker)); + EXPECT_EQ(3, EntropyTrackerPeer::packets_entropy_size(tracker)); + + EXPECT_EQ(2 ^ 5 ^ 17, tracker.EntropyHash(5)); + EXPECT_EQ(2 ^ 5 ^ 17 ^ 23, tracker.EntropyHash(6)); + EXPECT_EQ(2 ^ 5 ^ 17 ^ 23 ^ 42, tracker.EntropyHash(9)); + + EXPECT_FALSE(EntropyTrackerPeer::IsTrackingPacket(tracker, 1)); + EXPECT_FALSE(EntropyTrackerPeer::IsTrackingPacket(tracker, 2)); + EXPECT_TRUE(EntropyTrackerPeer::IsTrackingPacket(tracker, 5)); + EXPECT_TRUE(EntropyTrackerPeer::IsTrackingPacket(tracker, 6)); + EXPECT_TRUE(EntropyTrackerPeer::IsTrackingPacket(tracker, 9)); + + // Fill the gap at 4. + tracker.RecordPacketEntropyHash(4, 2); + + EXPECT_EQ(3u, EntropyTrackerPeer::first_gap(tracker)); + EXPECT_EQ(9u, EntropyTrackerPeer::largest_observed(tracker)); + EXPECT_EQ(4, EntropyTrackerPeer::packets_entropy_size(tracker)); + + EXPECT_EQ(5, tracker.EntropyHash(4)); + EXPECT_EQ(5 ^ 17, tracker.EntropyHash(5)); + EXPECT_EQ(5 ^ 17 ^ 23, tracker.EntropyHash(6)); + EXPECT_EQ(5 ^ 17 ^ 23 ^ 42, tracker.EntropyHash(9)); + + EXPECT_FALSE(EntropyTrackerPeer::IsTrackingPacket(tracker, 3)); + EXPECT_TRUE(EntropyTrackerPeer::IsTrackingPacket(tracker, 4)); + EXPECT_TRUE(EntropyTrackerPeer::IsTrackingPacket(tracker, 5)); + EXPECT_TRUE(EntropyTrackerPeer::IsTrackingPacket(tracker, 6)); + EXPECT_TRUE(EntropyTrackerPeer::IsTrackingPacket(tracker, 9)); + + // Fill the gap at 3. Entropy for packets 3 to 6 are forgotten. + tracker.RecordPacketEntropyHash(3, 2); + + EXPECT_EQ(7u, EntropyTrackerPeer::first_gap(tracker)); + EXPECT_EQ(9u, EntropyTrackerPeer::largest_observed(tracker)); + EXPECT_EQ(1, EntropyTrackerPeer::packets_entropy_size(tracker)); + + EXPECT_EQ(2 ^ 5 ^ 17 ^ 23 ^ 42, tracker.EntropyHash(9)); + + EXPECT_FALSE(EntropyTrackerPeer::IsTrackingPacket(tracker, 3)); + EXPECT_FALSE(EntropyTrackerPeer::IsTrackingPacket(tracker, 4)); + EXPECT_FALSE(EntropyTrackerPeer::IsTrackingPacket(tracker, 5)); + EXPECT_FALSE(EntropyTrackerPeer::IsTrackingPacket(tracker, 6)); + EXPECT_TRUE(EntropyTrackerPeer::IsTrackingPacket(tracker, 9)); + + // Fill in the rest. + tracker.RecordPacketEntropyHash(7, 2); + tracker.RecordPacketEntropyHash(8, 2); + + EXPECT_EQ(10u, EntropyTrackerPeer::first_gap(tracker)); + EXPECT_EQ(9u, EntropyTrackerPeer::largest_observed(tracker)); + EXPECT_EQ(0, EntropyTrackerPeer::packets_entropy_size(tracker)); + + EXPECT_EQ(2 ^ 5 ^ 17 ^ 23 ^ 42, tracker.EntropyHash(9)); +} + +TEST(EntropyTrackerTest, SetCumulativeEntropyUpTo) { + QuicReceivedPacketManager::EntropyTracker tracker; + + tracker.RecordPacketEntropyHash(2, 5); + tracker.RecordPacketEntropyHash(5, 17); + tracker.RecordPacketEntropyHash(6, 23); + tracker.RecordPacketEntropyHash(9, 42); + + EXPECT_EQ(1u, EntropyTrackerPeer::first_gap(tracker)); + EXPECT_EQ(9u, EntropyTrackerPeer::largest_observed(tracker)); + EXPECT_EQ(4, EntropyTrackerPeer::packets_entropy_size(tracker)); + + // Inform the tracker about value of the hash at a gap. + tracker.SetCumulativeEntropyUpTo(3, 7); + EXPECT_EQ(3u, EntropyTrackerPeer::first_gap(tracker)); + EXPECT_EQ(9u, EntropyTrackerPeer::largest_observed(tracker)); + EXPECT_EQ(3, EntropyTrackerPeer::packets_entropy_size(tracker)); + + EXPECT_EQ(7 ^ 17, tracker.EntropyHash(5)); + EXPECT_EQ(7 ^ 17 ^ 23, tracker.EntropyHash(6)); + EXPECT_EQ(7 ^ 17 ^ 23 ^ 42, tracker.EntropyHash(9)); + + // Inform the tracker about value of the hash at a known location. + tracker.SetCumulativeEntropyUpTo(6, 1); + EXPECT_EQ(7u, EntropyTrackerPeer::first_gap(tracker)); + EXPECT_EQ(9u, EntropyTrackerPeer::largest_observed(tracker)); + EXPECT_EQ(1, EntropyTrackerPeer::packets_entropy_size(tracker)); + + EXPECT_EQ(1 ^ 23 ^ 42, tracker.EntropyHash(9)); + + // Inform the tracker about value of the hash at the last location. + tracker.SetCumulativeEntropyUpTo(9, 21); + EXPECT_EQ(10u, EntropyTrackerPeer::first_gap(tracker)); + EXPECT_EQ(9u, EntropyTrackerPeer::largest_observed(tracker)); + EXPECT_EQ(0, EntropyTrackerPeer::packets_entropy_size(tracker)); + + EXPECT_EQ(42 ^ 21, tracker.EntropyHash(9)); +} + class QuicReceivedPacketManagerTest : public ::testing::Test { protected: QuicReceivedPacketManagerTest() : received_manager_(kTCP, &stats_) { } @@ -63,6 +227,7 @@ TEST_F(QuicReceivedPacketManagerTest, ReceivedPacketEntropyHash) { hash ^= entropies[index].second; ++index; } + if (i < 3) continue; EXPECT_EQ(hash, received_manager_.EntropyHash(i)); } // Reorder by 5 when 2 is received after 7. @@ -83,34 +248,34 @@ TEST_F(QuicReceivedPacketManagerTest, EntropyHashAboveLargestObserved) { EXPECT_EQ(0, received_manager_.EntropyHash(3)); } -TEST_F(QuicReceivedPacketManagerTest, RecalculateEntropyHash) { +TEST_F(QuicReceivedPacketManagerTest, SetCumulativeEntropyUpTo) { vector<pair<QuicPacketSequenceNumber, QuicPacketEntropyHash> > entropies; entropies.push_back(make_pair(1, 12)); entropies.push_back(make_pair(2, 1)); entropies.push_back(make_pair(3, 33)); entropies.push_back(make_pair(4, 3)); - entropies.push_back(make_pair(5, 34)); - entropies.push_back(make_pair(6, 29)); + entropies.push_back(make_pair(6, 34)); + entropies.push_back(make_pair(7, 29)); QuicPacketEntropyHash entropy_hash = 0; for (size_t i = 0; i < entropies.size(); ++i) { RecordPacketReceipt(entropies[i].first, entropies[i].second); entropy_hash ^= entropies[i].second; } - EXPECT_EQ(entropy_hash, received_manager_.EntropyHash(6)); + EXPECT_EQ(entropy_hash, received_manager_.EntropyHash(7)); - // Now set the entropy hash up to 4 to be 100. + // Now set the entropy hash up to 5 to be 100. entropy_hash ^= 100; - for (size_t i = 0; i < 3; ++i) { + for (size_t i = 0; i < 4; ++i) { entropy_hash ^= entropies[i].second; } - QuicReceivedPacketManagerPeer::RecalculateEntropyHash( - &received_manager_, 4, 100); - EXPECT_EQ(entropy_hash, received_manager_.EntropyHash(6)); + QuicReceivedPacketManagerPeer::SetCumulativeEntropyUpTo( + &received_manager_, 5, 100); + EXPECT_EQ(entropy_hash, received_manager_.EntropyHash(7)); - QuicReceivedPacketManagerPeer::RecalculateEntropyHash( + QuicReceivedPacketManagerPeer::SetCumulativeEntropyUpTo( &received_manager_, 1, 50); - EXPECT_EQ(entropy_hash, received_manager_.EntropyHash(6)); + EXPECT_EQ(entropy_hash, received_manager_.EntropyHash(7)); // No reordering. EXPECT_EQ(0u, stats_.max_sequence_reordering); diff --git a/net/quic/quic_sent_packet_manager.cc b/net/quic/quic_sent_packet_manager.cc index f2eb6d3..f73bfd3 100644 --- a/net/quic/quic_sent_packet_manager.cc +++ b/net/quic/quic_sent_packet_manager.cc @@ -132,6 +132,13 @@ void QuicSentPacketManager::OnIncomingAck( InvokeLossDetection(ack_receive_time); MaybeInvokeCongestionEvent(largest_observed_acked, bytes_in_flight); + // If we have received a truncated ack, then we need to clear out some + // previous transmissions to allow the peer to actually ACK new packets. + if (received_info.is_truncated) { + unacked_packets_.ClearPreviousRetransmissions( + received_info.missing_packets.size() / 2); + } + // Anytime we are making forward progress and have a new RTT estimate, reset // the backoff counters. if (largest_observed_acked) { @@ -153,11 +160,6 @@ void QuicSentPacketManager::MaybeInvokeCongestionEvent( } } -void QuicSentPacketManager::DiscardUnackedPacket( - QuicPacketSequenceNumber sequence_number) { - MarkPacketHandled(sequence_number, QuicTime::Delta::Zero()); -} - void QuicSentPacketManager::HandleAckForSentPackets( const ReceivedPacketInfo& received_info) { // Go through the packets we have not received an ack for and see if this @@ -176,7 +178,8 @@ void QuicSentPacketManager::HandleAckForSentPackets( // Remove any rtt only packets less than or equal to the largest observed, // since they will not produce an RTT measurement. if (QuicUnackedPacketMap::IsForRttOnly(it->second)) { - it = MarkPacketHandled(sequence_number, delta_largest_observed); + ++it; + unacked_packets_.RemoveRttOnlyPacket(sequence_number); } else { // Consider it multiple nacks when there is a gap between the missing // packet and the largest observed, since the purpose of a nack @@ -210,14 +213,6 @@ void QuicSentPacketManager::HandleAckForSentPackets( revived_it != received_info.revived_packets.end(); ++revived_it) { MarkPacketRevived(*revived_it, delta_largest_observed); } - - // If we have received a truncated ack, then we need to - // clear out some previous transmissions to allow the peer - // to actually ACK new packets. - if (received_info.is_truncated) { - unacked_packets_.ClearPreviousRetransmissions( - received_info.missing_packets.size() / 2); - } } bool QuicSentPacketManager::HasRetransmittableFrames( @@ -264,14 +259,8 @@ void QuicSentPacketManager::NeuterUnencryptedPackets() { // they are not retransmitted or considered lost from a congestion control // perspective. pending_retransmissions_.erase(it->first); + unacked_packets_.RemoveRetransmittibility(it->first, largest_observed_); unacked_packets_.SetNotPending(it->first); - // TODO(ianswett): Clean this up so UnackedPacketMap maintains the correct - // invariants between the various transmissions for NeuterPacket. - SequenceNumberSet all_transmissions = *it->second.all_transmissions; - for (SequenceNumberSet::const_iterator all_it = all_transmissions.begin(); - all_it != all_transmissions.end(); ++all_it) { - unacked_packets_.NeuterPacket(*all_it); - } } } } @@ -317,7 +306,7 @@ QuicSentPacketManager::PendingRetransmission ++it; } while (it != pending_retransmissions_.end()); } - DCHECK(unacked_packets_.IsUnacked(sequence_number)); + DCHECK(unacked_packets_.IsUnacked(sequence_number)) << sequence_number; const TransmissionInfo& transmission_info = unacked_packets_.GetTransmissionInfo(sequence_number); DCHECK(transmission_info.retransmittable_frames); @@ -334,24 +323,23 @@ void QuicSentPacketManager::MarkPacketRevived( if (!unacked_packets_.IsUnacked(sequence_number)) { return; } - // This packet has been revived at the receiver. If we were going to - // retransmit it, do not retransmit it anymore. - pending_retransmissions_.erase(sequence_number); const TransmissionInfo& transmission_info = unacked_packets_.GetTransmissionInfo(sequence_number); + QuicPacketSequenceNumber newest_transmission = + *transmission_info.all_transmissions->rbegin(); + // This packet has been revived at the receiver. If we were going to + // retransmit it, do not retransmit it anymore. + pending_retransmissions_.erase(newest_transmission); + // The AckNotifierManager needs to be notified for revived packets, // since it indicates the packet arrived from the appliction's perspective. if (transmission_info.retransmittable_frames) { ack_notifier_manager_.OnPacketAcked( - sequence_number, delta_largest_observed); + newest_transmission, delta_largest_observed); } - if (!transmission_info.pending) { - unacked_packets_.RemovePacket(sequence_number); - } else { - unacked_packets_.NeuterPacket(sequence_number); - } + unacked_packets_.RemoveRetransmittibility(sequence_number, largest_observed_); } QuicUnackedPacketMap::const_iterator QuicSentPacketManager::MarkPacketHandled( @@ -363,15 +351,9 @@ QuicUnackedPacketMap::const_iterator QuicSentPacketManager::MarkPacketHandled( } const TransmissionInfo& transmission_info = unacked_packets_.GetTransmissionInfo(sequence_number); - // If this packet is pending, remove it. - if (transmission_info.pending) { - unacked_packets_.SetNotPending(sequence_number); - } - SequenceNumberSet all_transmissions = *transmission_info.all_transmissions; - SequenceNumberSet::reverse_iterator all_transmissions_it = - all_transmissions.rbegin(); - QuicPacketSequenceNumber newest_transmission = *all_transmissions_it; + QuicPacketSequenceNumber newest_transmission = + *transmission_info.all_transmissions->rbegin(); // Remove the most recent packet, if it is pending retransmission. pending_retransmissions_.erase(newest_transmission); @@ -389,31 +371,20 @@ QuicUnackedPacketMap::const_iterator QuicSentPacketManager::MarkPacketHandled( ack_notifier_manager_.OnPacketAcked(newest_transmission, delta_largest_observed); + // If it's a crypto handshake packet, discard it and all retransmissions, + // since they won't be acked now that one has been processed. // TODO(ianswett): Instead of handling all crypto packets in a special way, // only handle NULL encrypted packets in a special way. - bool has_crypto_handshake = HasCryptoHandshake( - unacked_packets_.GetTransmissionInfo(newest_transmission)); - while (all_transmissions_it != all_transmissions.rend()) { - QuicPacketSequenceNumber previous_transmission = *all_transmissions_it; - const TransmissionInfo& transmission_info = - unacked_packets_.GetTransmissionInfo(previous_transmission); - DCHECK(!ContainsKey(pending_retransmissions_, previous_transmission)); - if (has_crypto_handshake) { - // If it's a crypto handshake packet, discard it and all retransmissions, - // since they won't be acked now that one has been processed. - unacked_packets_.SetNotPending(previous_transmission); - } - if (!transmission_info.pending) { - unacked_packets_.RemovePacket(previous_transmission); - } else { - unacked_packets_.NeuterPacket(previous_transmission); - } - ++all_transmissions_it; + if (HasCryptoHandshake( + unacked_packets_.GetTransmissionInfo(newest_transmission))) { + unacked_packets_.SetNotPending(newest_transmission); } + unacked_packets_.SetNotPending(sequence_number); + unacked_packets_.RemoveRetransmittibility(sequence_number, largest_observed_); QuicUnackedPacketMap::const_iterator next_unacked = unacked_packets_.begin(); while (next_unacked != unacked_packets_.end() && - next_unacked->first < sequence_number) { + next_unacked->first <= sequence_number) { ++next_unacked; } return next_unacked; @@ -619,7 +590,9 @@ void QuicSentPacketManager::InvokeLossDetection(QuicTime time) { // a packet whose previous transmission has been acked, a packet that has // been TLP retransmitted, or an FEC packet. unacked_packets_.SetNotPending(sequence_number); - unacked_packets_.RemovePacket(sequence_number); + if (QuicUnackedPacketMap::IsForRttOnly(transmission_info)) { + unacked_packets_.RemoveRttOnlyPacket(sequence_number); + } } } } diff --git a/net/quic/quic_sent_packet_manager.h b/net/quic/quic_sent_packet_manager.h index cca7f4c..0c6c407 100644 --- a/net/quic/quic_sent_packet_manager.h +++ b/net/quic/quic_sent_packet_manager.h @@ -82,12 +82,6 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager { void OnIncomingAck(const ReceivedPacketInfo& received_info, QuicTime ack_receive_time); - // Discards any information for the packet corresponding to |sequence_number|. - // If this packet has been retransmitted, information on those packets - // will be discarded as well. Also discards it from the congestion window if - // it is present. - void DiscardUnackedPacket(QuicPacketSequenceNumber sequence_number); - // Returns true if the non-FEC packet |sequence_number| is unacked. bool IsUnacked(QuicPacketSequenceNumber sequence_number) const; diff --git a/net/quic/quic_sent_packet_manager_test.cc b/net/quic/quic_sent_packet_manager_test.cc index aef7c10..f3a9f30 100644 --- a/net/quic/quic_sent_packet_manager_test.cc +++ b/net/quic/quic_sent_packet_manager_test.cc @@ -444,7 +444,7 @@ TEST_F(QuicSentPacketManagerTest, RetransmitTwiceThenAckFirst) { EXPECT_TRUE(QuicSentPacketManagerPeer::HasPendingPackets(&manager_)); VerifyRetransmittablePackets(NULL, 0); - // Ensure packet 2 is lost when 4 and 5 are sent and acked. + // Ensure packet 2 is lost when 4 is sent and 3 and 4 are acked. SendDataPacket(4); received_info.largest_observed = 4; received_info.missing_packets.insert(2); @@ -616,14 +616,6 @@ TEST_F(QuicSentPacketManagerTest, GetLeastUnackedSentPacketUnackedFec) { EXPECT_EQ(1u, manager_.GetLeastUnackedSentPacket()); } -TEST_F(QuicSentPacketManagerTest, GetLeastUnackedSentPacketDiscardUnacked) { - SerializedPacket serialized_packet(CreateDataPacket(1)); - - manager_.OnSerializedPacket(serialized_packet); - manager_.DiscardUnackedPacket(1u); - EXPECT_EQ(0u, manager_.GetLeastUnackedSentPacket()); -} - TEST_F(QuicSentPacketManagerTest, GetLeastUnackedPacketAndDiscard) { VerifyUnackedPackets(NULL, 0); @@ -643,19 +635,12 @@ TEST_F(QuicSentPacketManagerTest, GetLeastUnackedPacketAndDiscard) { VerifyUnackedPackets(unacked, arraysize(unacked)); VerifyRetransmittablePackets(NULL, 0); - manager_.DiscardUnackedPacket(1); - EXPECT_EQ(2u, manager_.GetLeastUnackedSentPacket()); - // Ack 2, which has never been sent, so there's no rtt update. ReceivedPacketInfo received_info; received_info.largest_observed = 2; manager_.OnIncomingAck(received_info, clock_.Now()); EXPECT_EQ(3u, manager_.GetLeastUnackedSentPacket()); - - // Discard the 3rd packet and ensure there are no FEC packets. - manager_.DiscardUnackedPacket(3); - EXPECT_FALSE(manager_.HasUnackedPackets()); } TEST_F(QuicSentPacketManagerTest, GetSentTime) { diff --git a/net/quic/quic_session_test.cc b/net/quic/quic_session_test.cc index 511e345..34025de 100644 --- a/net/quic/quic_session_test.cc +++ b/net/quic/quic_session_test.cc @@ -29,11 +29,11 @@ using base::hash_map; using std::set; using std::vector; -using testing::_; using testing::InSequence; using testing::InvokeWithoutArgs; using testing::Return; using testing::StrictMock; +using testing::_; namespace net { namespace test { @@ -273,8 +273,8 @@ TEST_P(QuicSessionTest, IsClosedStreamLocallyCreated) { } TEST_P(QuicSessionTest, IsClosedStreamPeerCreated) { - QuicStreamId stream_id1 = 5; - QuicStreamId stream_id2 = stream_id1 + 2; + QuicStreamId stream_id1 = kClientDataStreamId1; + QuicStreamId stream_id2 = kClientDataStreamId2; QuicDataStream* stream1 = session_.GetIncomingDataStream(stream_id1); QuicDataStreamPeer::SetHeadersDecompressed(stream1, true); QuicDataStream* stream2 = session_.GetIncomingDataStream(stream_id2); @@ -294,7 +294,7 @@ TEST_P(QuicSessionTest, IsClosedStreamPeerCreated) { } TEST_P(QuicSessionTest, StreamIdTooLarge) { - QuicStreamId stream_id = 5; + QuicStreamId stream_id = kClientDataStreamId1; session_.GetIncomingDataStream(stream_id); EXPECT_CALL(*connection_, SendConnectionClose(QUIC_INVALID_STREAM_ID)); session_.GetIncomingDataStream(stream_id + kMaxStreamIdDelta + 2); @@ -568,7 +568,7 @@ TEST_P(QuicSessionTest, IncreasedTimeoutAfterCryptoHandshake) { } TEST_P(QuicSessionTest, RstStreamBeforeHeadersDecompressed) { - QuicStreamId stream_id1 = 5; + QuicStreamId stream_id1 = kClientDataStreamId1; // Send two bytes of payload. QuicStreamFrame data1(stream_id1, false, 0, MakeIOVector("HT")); vector<QuicStreamFrame> frames; @@ -589,8 +589,7 @@ TEST_P(QuicSessionTest, MultipleRstStreamsCauseSingleConnectionClose) { // multiple connection close frames. // Create valid stream. - const QuicStreamId kStreamId = 5; - QuicStreamFrame data1(kStreamId, false, 0, MakeIOVector("HT")); + QuicStreamFrame data1(kClientDataStreamId1, false, 0, MakeIOVector("HT")); vector<QuicStreamFrame> frames; frames.push_back(data1); session_.OnStreamFrames(frames); diff --git a/net/quic/quic_stream_factory_test.cc b/net/quic/quic_stream_factory_test.cc index 27903e9..8189989 100644 --- a/net/quic/quic_stream_factory_test.cc +++ b/net/quic/quic_stream_factory_test.cc @@ -169,7 +169,7 @@ class QuicStreamFactoryTest : public ::testing::TestWithParam<QuicVersion> { } scoped_ptr<QuicEncryptedPacket> ConstructRstPacket() { - QuicStreamId stream_id = 5; + QuicStreamId stream_id = kClientDataStreamId1; return maker_.MakeRstPacket( 1, true, stream_id, AdjustErrorForVersion(QUIC_RST_FLOW_CONTROL_ACCOUNTING, GetParam())); @@ -663,7 +663,7 @@ TEST_P(QuicStreamFactoryTest, MaxOpenStream) { MockRead reads[] = { MockRead(ASYNC, OK, 0) // EOF }; - QuicStreamId stream_id = 5; + QuicStreamId stream_id = kClientDataStreamId1; scoped_ptr<QuicEncryptedPacket> rst( maker_.MakeRstPacket(1, true, stream_id, QUIC_STREAM_CANCELLED)); MockWrite writes[] = { diff --git a/net/quic/quic_unacked_packet_map.cc b/net/quic/quic_unacked_packet_map.cc index a8fe3b5..92c24f7 100644 --- a/net/quic/quic_unacked_packet_map.cc +++ b/net/quic/quic_unacked_packet_map.cc @@ -90,8 +90,11 @@ void QuicUnackedPacketMap::ClearPreviousRetransmissions(size_t num_to_clear) { break; } + it->second.all_transmissions->erase(sequence_number); + LOG_IF(DFATAL, it->second.all_transmissions->empty()) + << "Previous retransmissions must have a newer transmission."; ++it; - RemovePacket(sequence_number); + unacked_packets_.erase(sequence_number); --num_to_clear; } } @@ -119,38 +122,36 @@ void QuicUnackedPacketMap::NackPacket(QuicPacketSequenceNumber sequence_number, it->second.nack_count = max(min_nacks, it->second.nack_count); } -void QuicUnackedPacketMap::RemovePacket( - QuicPacketSequenceNumber sequence_number) { +void QuicUnackedPacketMap::RemoveRetransmittibility( + QuicPacketSequenceNumber sequence_number, + QuicPacketSequenceNumber largest_observed) { UnackedPacketMap::iterator it = unacked_packets_.find(sequence_number); if (it == unacked_packets_.end()) { LOG(DFATAL) << "packet is not unacked: " << sequence_number; return; } - TransmissionInfo* transmission_info = &it->second; - DCHECK(!transmission_info->pending); - MaybeRemoveRetransmittableFrames(transmission_info); - transmission_info->all_transmissions->erase(sequence_number); - if (transmission_info->all_transmissions->empty()) { - delete transmission_info->all_transmissions; + SequenceNumberSet* all_transmissions = it->second.all_transmissions; + // TODO(ianswett): Consider optimizing this for lone packets. + // TODO(ianswett): Consider adding a check to ensure there are retranmittable + // frames associated with this packet. + for (SequenceNumberSet::reverse_iterator it = all_transmissions->rbegin(); + it != all_transmissions->rend(); ++it) { + TransmissionInfo* transmission_info = FindOrNull(unacked_packets_, *it); + if (transmission_info == NULL) { + LOG(DFATAL) << "All transmissions in all_transmissions must be present " + << "in the unacked packet map."; + continue; + } + MaybeRemoveRetransmittableFrames(transmission_info); + if (sequence_number <= largest_observed && !transmission_info->pending) { + unacked_packets_.erase(*it); + } else { + transmission_info->all_transmissions = new SequenceNumberSet(); + transmission_info->all_transmissions->insert(*it); + } } - unacked_packets_.erase(it); -} -void QuicUnackedPacketMap::NeuterPacket( - QuicPacketSequenceNumber sequence_number) { - UnackedPacketMap::iterator it = unacked_packets_.find(sequence_number); - if (it == unacked_packets_.end()) { - LOG(DFATAL) << "packet is not unacked: " << sequence_number; - return; - } - TransmissionInfo* transmission_info = &it->second; - // TODO(ianswett): Ensure packets are pending before neutering them. - MaybeRemoveRetransmittableFrames(transmission_info); - if (transmission_info->all_transmissions->size() > 1) { - transmission_info->all_transmissions->erase(sequence_number); - transmission_info->all_transmissions = new SequenceNumberSet(); - transmission_info->all_transmissions->insert(sequence_number); - } + delete all_transmissions; } void QuicUnackedPacketMap::MaybeRemoveRetransmittableFrames( @@ -165,6 +166,21 @@ void QuicUnackedPacketMap::MaybeRemoveRetransmittableFrames( } } +void QuicUnackedPacketMap::RemoveRttOnlyPacket( + QuicPacketSequenceNumber sequence_number) { + UnackedPacketMap::iterator it = unacked_packets_.find(sequence_number); + if (it == unacked_packets_.end()) { + LOG(DFATAL) << "packet is not unacked: " << sequence_number; + return; + } + TransmissionInfo* transmission_info = &it->second; + DCHECK(!transmission_info->pending); + DCHECK(transmission_info->retransmittable_frames == NULL); + DCHECK_EQ(1u, transmission_info->all_transmissions->size()); + delete transmission_info->all_transmissions; + unacked_packets_.erase(it); +} + // static bool QuicUnackedPacketMap::IsForRttOnly( const TransmissionInfo& transmission_info) { diff --git a/net/quic/quic_unacked_packet_map.h b/net/quic/quic_unacked_packet_map.h index 12a96a2..4861595 100644 --- a/net/quic/quic_unacked_packet_map.h +++ b/net/quic/quic_unacked_packet_map.h @@ -110,14 +110,20 @@ class NET_EXPORT_PRIVATE QuicUnackedPacketMap { // Returns true if there are any pending crypto packets. bool HasPendingCryptoPackets() const; - // Removes entries from the unacked packet map, and deletes - // the retransmittable frames associated with the packet. - // Does not remove any previous or subsequent transmissions of this packet. - void RemovePacket(QuicPacketSequenceNumber sequence_number); - - // Neuters the specified packet. Deletes any retransmittable - // frames, and sets all_transmissions to only include itself. - void NeuterPacket(QuicPacketSequenceNumber sequence_number); + // Removes any retransmittable frames from this transmission or an associated + // transmission. It removes any nnon-pending transmissions less than or + // equal to |largest_observed|, and disconnects any other packets from other + // transmissions. + // TODO(ianswett): Remove largest_observed_ once the map tracks whether a + // transmission is useful for RTT purposes internally. + void RemoveRetransmittibility(QuicPacketSequenceNumber sequence_number, + QuicPacketSequenceNumber largest_observed_); + + // Removes an entry from the unacked packet map which is not pending, has + // no retransmittable frames, and no associated transmissions. + // TODO(ianswett): Remove or make this method private once the map tracks + // the three reasons for tracking a packet correctly. + void RemoveRttOnlyPacket(QuicPacketSequenceNumber sequence_number); // Returns true if the packet's only purpose is to measure RTT. It must not // be pending, have retransmittable frames, or be linked to transmissions diff --git a/net/quic/quic_write_blocked_list_test.cc b/net/quic/quic_write_blocked_list_test.cc index 1643bc3..0633f63 100644 --- a/net/quic/quic_write_blocked_list_test.cc +++ b/net/quic/quic_write_blocked_list_test.cc @@ -4,6 +4,7 @@ // #include "net/quic/quic_write_blocked_list.h" +#include "net/quic/test_tools/quic_test_utils.h" #include "testing/gtest/include/gtest/gtest.h" namespace net { @@ -93,7 +94,7 @@ TEST(QuicWriteBlockedListTest, NoDuplicateEntries) { // Try to add a stream to the write blocked list multiple times at the same // priority. - const QuicStreamId kBlockedId = 5; + const QuicStreamId kBlockedId = kClientDataStreamId1; write_blocked_list.PushBack(kBlockedId, QuicWriteBlockedList::kHighestPriority); write_blocked_list.PushBack(kBlockedId, diff --git a/net/quic/reliable_quic_stream_test.cc b/net/quic/reliable_quic_stream_test.cc index 48e8616..935554f 100644 --- a/net/quic/reliable_quic_stream_test.cc +++ b/net/quic/reliable_quic_stream_test.cc @@ -37,7 +37,6 @@ namespace { const char kData1[] = "FooAndBar"; const char kData2[] = "EepAndBaz"; const size_t kDataLen = 9; -const QuicStreamId kStreamId = 3; const bool kIsServer = true; const bool kShouldProcessData = true; @@ -119,9 +118,7 @@ class ReliableQuicStreamTest : public ::testing::TestWithParam<bool> { QuicConfigPeer::SetReceivedInitialFlowControlWindow( session_->config(), initial_flow_control_window_bytes_); - stream_.reset(new TestStream(kStreamId, session_.get(), - stream_should_process_data)); - stream2_.reset(new TestStream(kStreamId + 2, session_.get(), + stream_.reset(new TestStream(kHeadersStreamId, session_.get(), stream_should_process_data)); write_blocked_list_ = QuicSessionPeer::GetWriteblockedStreams(session_.get()); @@ -143,7 +140,6 @@ class ReliableQuicStreamTest : public ::testing::TestWithParam<bool> { MockConnection* connection_; scoped_ptr<MockSession> session_; scoped_ptr<TestStream> stream_; - scoped_ptr<TestStream> stream2_; SpdyHeaderBlock headers_; QuicWriteBlockedList* write_blocked_list_; uint32 initial_flow_control_window_bytes_; @@ -156,10 +152,10 @@ TEST_F(ReliableQuicStreamTest, WriteAllData) { connection_->options()->max_packet_length = 1 + QuicPacketCreator::StreamFramePacketOverhead( - connection_->version(), PACKET_8BYTE_CONNECTION_ID, !kIncludeVersion, - PACKET_6BYTE_SEQUENCE_NUMBER, NOT_IN_FEC_GROUP); - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( - Return(QuicConsumedData(kDataLen, true))); + connection_->version(), PACKET_8BYTE_CONNECTION_ID, + !kIncludeVersion, PACKET_6BYTE_SEQUENCE_NUMBER, NOT_IN_FEC_GROUP); + EXPECT_CALL(*session_, WritevData(kHeadersStreamId, _, _, _, _)) + .WillOnce(Return(QuicConsumedData(kDataLen, true))); stream_->WriteOrBufferData(kData1, false, NULL); EXPECT_FALSE(HasWriteBlockedStreams()); } @@ -178,8 +174,8 @@ TEST_F(ReliableQuicStreamTest, BlockIfOnlySomeDataConsumed) { // Write some data and no fin. If we consume some but not all of the data, // we should be write blocked a not all the data was consumed. - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( - Return(QuicConsumedData(1, false))); + EXPECT_CALL(*session_, WritevData(kHeadersStreamId, _, _, _, _)) + .WillOnce(Return(QuicConsumedData(1, false))); stream_->WriteOrBufferData(StringPiece(kData1, 2), false, NULL); ASSERT_EQ(1u, write_blocked_list_->NumBlockedStreams()); } @@ -191,8 +187,8 @@ TEST_F(ReliableQuicStreamTest, BlockIfFinNotConsumedWithData) { // we should be write blocked because the fin was not consumed. // (This should never actually happen as the fin should be sent out with the // last data) - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( - Return(QuicConsumedData(2, false))); + EXPECT_CALL(*session_, WritevData(kHeadersStreamId, _, _, _, _)) + .WillOnce(Return(QuicConsumedData(2, false))); stream_->WriteOrBufferData(StringPiece(kData1, 2), true, NULL); ASSERT_EQ(1u, write_blocked_list_->NumBlockedStreams()); } @@ -202,8 +198,8 @@ TEST_F(ReliableQuicStreamTest, BlockIfSoloFinNotConsumed) { // Write no data and a fin. If we consume nothing we should be write blocked, // as the fin was not consumed. - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( - Return(QuicConsumedData(0, false))); + EXPECT_CALL(*session_, WritevData(kHeadersStreamId, _, _, _, _)) + .WillOnce(Return(QuicConsumedData(0, false))); stream_->WriteOrBufferData(StringPiece(), true, NULL); ASSERT_EQ(1u, write_blocked_list_->NumBlockedStreams()); } @@ -260,8 +256,8 @@ TEST_F(ReliableQuicStreamTest, RstAlwaysSentIfNoFinSent) { EXPECT_FALSE(rst_sent()); // Write some data, with no FIN. - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( - Return(QuicConsumedData(1, false))); + EXPECT_CALL(*session_, WritevData(kHeadersStreamId, _, _, _, _)) + .WillOnce(Return(QuicConsumedData(1, false))); stream_->WriteOrBufferData(StringPiece(kData1, 1), false, NULL); EXPECT_FALSE(fin_sent()); EXPECT_FALSE(rst_sent()); @@ -283,8 +279,8 @@ TEST_F(ReliableQuicStreamTest, RstNotSentIfFinSent) { EXPECT_FALSE(rst_sent()); // Write some data, with FIN. - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( - Return(QuicConsumedData(1, true))); + EXPECT_CALL(*session_, WritevData(kHeadersStreamId, _, _, _, _)) + .WillOnce(Return(QuicConsumedData(1, true))); stream_->WriteOrBufferData(StringPiece(kData1, 1), true, NULL); EXPECT_TRUE(fin_sent()); EXPECT_FALSE(rst_sent()); @@ -404,27 +400,27 @@ TEST_F(ReliableQuicStreamTest, WriteOrBufferDataWithQuicAckNotifier) { scoped_refptr<QuicAckNotifier::DelegateInterface> proxy_delegate; - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce(DoAll( - WithArgs<4>(Invoke(CreateFunctor( - &SaveProxyAckNotifierDelegate, &proxy_delegate))), - Return(QuicConsumedData(kFirstWriteSize, false)))); + EXPECT_CALL(*session_, WritevData(kHeadersStreamId, _, _, _, _)) + .WillOnce(DoAll(WithArgs<4>(Invoke(CreateFunctor( + &SaveProxyAckNotifierDelegate, &proxy_delegate))), + Return(QuicConsumedData(kFirstWriteSize, false)))); stream_->WriteOrBufferData(kData, false, delegate.get()); EXPECT_TRUE(HasWriteBlockedStreams()); - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, proxy_delegate.get())). - WillOnce( - Return(QuicConsumedData(kSecondWriteSize, false))); + EXPECT_CALL(*session_, + WritevData(kHeadersStreamId, _, _, _, 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))); + EXPECT_CALL(*session_, + WritevData(kHeadersStreamId, _, _, _, proxy_delegate.get())) + .WillOnce(Return(QuicConsumedData(0, false))); stream_->OnCanWrite(); - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, proxy_delegate.get())). - WillOnce( - Return(QuicConsumedData(kLastWriteSize, false))); + EXPECT_CALL(*session_, + WritevData(kHeadersStreamId, _, _, _, proxy_delegate.get())) + .WillOnce(Return(QuicConsumedData(kLastWriteSize, false))); stream_->OnCanWrite(); // There were two writes, so OnAckNotification is not propagated @@ -459,10 +455,10 @@ TEST_F(ReliableQuicStreamTest, WriteOrBufferDataAckNotificationBeforeFlush) { scoped_refptr<QuicAckNotifier::DelegateInterface> proxy_delegate; - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce(DoAll( - WithArgs<4>(Invoke(CreateFunctor( - &SaveProxyAckNotifierDelegate, &proxy_delegate))), - Return(QuicConsumedData(kInitialWriteSize, false)))); + EXPECT_CALL(*session_, WritevData(kHeadersStreamId, _, _, _, _)) + .WillOnce(DoAll(WithArgs<4>(Invoke(CreateFunctor( + &SaveProxyAckNotifierDelegate, &proxy_delegate))), + Return(QuicConsumedData(kInitialWriteSize, false)))); stream_->WriteOrBufferData(kData, false, delegate.get()); EXPECT_TRUE(HasWriteBlockedStreams()); @@ -470,10 +466,10 @@ TEST_F(ReliableQuicStreamTest, WriteOrBufferDataAckNotificationBeforeFlush) { proxy_delegate->OnAckNotification(1, 2, 3, 4, zero_); proxy_delegate = NULL; - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce(DoAll( - WithArgs<4>(Invoke(CreateFunctor( - &SaveProxyAckNotifierDelegate, &proxy_delegate))), - Return(QuicConsumedData(kDataSize - kInitialWriteSize, false)))); + EXPECT_CALL(*session_, WritevData(kHeadersStreamId, _, _, _, _)).WillOnce( + DoAll(WithArgs<4>(Invoke(CreateFunctor( + &SaveProxyAckNotifierDelegate, &proxy_delegate))), + Return(QuicConsumedData(kDataSize - kInitialWriteSize, false)))); stream_->OnCanWrite(); // Handle the ack for the second write. @@ -490,10 +486,10 @@ TEST_F(ReliableQuicStreamTest, WriteAndBufferDataWithAckNotiferNoBuffer) { scoped_refptr<QuicAckNotifier::DelegateInterface> proxy_delegate; - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce(DoAll( - WithArgs<4>(Invoke(CreateFunctor( - &SaveProxyAckNotifierDelegate, &proxy_delegate))), - Return(QuicConsumedData(kDataLen, true)))); + EXPECT_CALL(*session_, WritevData(kHeadersStreamId, _, _, _, _)) + .WillOnce(DoAll(WithArgs<4>(Invoke(CreateFunctor( + &SaveProxyAckNotifierDelegate, &proxy_delegate))), + Return(QuicConsumedData(kDataLen, true)))); stream_->WriteOrBufferData(kData1, true, delegate.get()); EXPECT_FALSE(HasWriteBlockedStreams()); @@ -511,15 +507,15 @@ TEST_F(ReliableQuicStreamTest, BufferOnWriteAndBufferDataWithAckNotifer) { scoped_refptr<QuicAckNotifier::DelegateInterface> proxy_delegate; - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce( - Return(QuicConsumedData(0, false))); + EXPECT_CALL(*session_, WritevData(kHeadersStreamId, _, _, _, _)) + .WillOnce(Return(QuicConsumedData(0, false))); stream_->WriteOrBufferData(kData1, true, delegate.get()); EXPECT_TRUE(HasWriteBlockedStreams()); - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce(DoAll( - WithArgs<4>(Invoke(CreateFunctor( - &SaveProxyAckNotifierDelegate, &proxy_delegate))), - Return(QuicConsumedData(kDataLen, true)))); + EXPECT_CALL(*session_, WritevData(kHeadersStreamId, _, _, _, _)) + .WillOnce(DoAll(WithArgs<4>(Invoke(CreateFunctor( + &SaveProxyAckNotifierDelegate, &proxy_delegate))), + Return(QuicConsumedData(kDataLen, true)))); stream_->OnCanWrite(); // Handle the ack. @@ -537,17 +533,17 @@ TEST_F(ReliableQuicStreamTest, WriteAndBufferDataWithAckNotiferOnlyFinRemains) { scoped_refptr<QuicAckNotifier::DelegateInterface> proxy_delegate; - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce(DoAll( - WithArgs<4>(Invoke(CreateFunctor( - &SaveProxyAckNotifierDelegate, &proxy_delegate))), - Return(QuicConsumedData(kDataLen, false)))); + EXPECT_CALL(*session_, WritevData(kHeadersStreamId, _, _, _, _)) + .WillOnce(DoAll(WithArgs<4>(Invoke(CreateFunctor( + &SaveProxyAckNotifierDelegate, &proxy_delegate))), + Return(QuicConsumedData(kDataLen, false)))); stream_->WriteOrBufferData(kData1, true, delegate.get()); EXPECT_TRUE(HasWriteBlockedStreams()); - EXPECT_CALL(*session_, WritevData(kStreamId, _, _, _, _)).WillOnce(DoAll( - WithArgs<4>(Invoke(CreateFunctor( - &SaveProxyAckNotifierDelegate, &proxy_delegate))), - Return(QuicConsumedData(0, true)))); + EXPECT_CALL(*session_, WritevData(kHeadersStreamId, _, _, _, _)) + .WillOnce(DoAll(WithArgs<4>(Invoke(CreateFunctor( + &SaveProxyAckNotifierDelegate, &proxy_delegate))), + Return(QuicConsumedData(0, true)))); stream_->OnCanWrite(); // Handle the acks. diff --git a/net/quic/test_tools/quic_received_packet_manager_peer.cc b/net/quic/test_tools/quic_received_packet_manager_peer.cc index d25a209..2faf84e 100644 --- a/net/quic/test_tools/quic_received_packet_manager_peer.cc +++ b/net/quic/test_tools/quic_received_packet_manager_peer.cc @@ -11,12 +11,12 @@ namespace net { namespace test { // static -void QuicReceivedPacketManagerPeer::RecalculateEntropyHash( +void QuicReceivedPacketManagerPeer::SetCumulativeEntropyUpTo( QuicReceivedPacketManager* received_packet_manager, QuicPacketSequenceNumber peer_least_unacked, QuicPacketEntropyHash entropy_hash) { - received_packet_manager->RecalculateEntropyHash(peer_least_unacked, - entropy_hash); + received_packet_manager->entropy_tracker_.SetCumulativeEntropyUpTo( + peer_least_unacked, entropy_hash); } // static diff --git a/net/quic/test_tools/quic_received_packet_manager_peer.h b/net/quic/test_tools/quic_received_packet_manager_peer.h index 4607a0c..c3cb2d3 100644 --- a/net/quic/test_tools/quic_received_packet_manager_peer.h +++ b/net/quic/test_tools/quic_received_packet_manager_peer.h @@ -15,7 +15,7 @@ namespace test { class QuicReceivedPacketManagerPeer { public: - static void RecalculateEntropyHash( + static void SetCumulativeEntropyUpTo( QuicReceivedPacketManager* received_packet_manager, QuicPacketSequenceNumber peer_least_unacked, QuicPacketEntropyHash entropy_hash); diff --git a/net/quic/test_tools/quic_test_utils.h b/net/quic/test_tools/quic_test_utils.h index 6901824..3bf2bf3 100644 --- a/net/quic/test_tools/quic_test_utils.h +++ b/net/quic/test_tools/quic_test_utils.h @@ -31,6 +31,12 @@ static const QuicConnectionId kTestConnectionId = 42; static const int kTestPort = 123; static const uint32 kInitialFlowControlWindowForTest = 32 * 1024; // 32 KB +// Data stream IDs start at 5: the crypto stream is 1, headers stream is 3. +static const QuicStreamId kClientDataStreamId1 = 5; +static const QuicStreamId kClientDataStreamId2 = 7; +static const QuicStreamId kClientDataStreamId3 = 9; +static const QuicStreamId kClientDataStreamId4 = 11; + // Returns the test peer IP address. IPAddressNumber TestPeerIPAddress(); |