diff options
author | rtenneti@chromium.org <rtenneti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-12-02 07:02:12 +0000 |
---|---|---|
committer | rtenneti@chromium.org <rtenneti@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2013-12-02 07:02:12 +0000 |
commit | e6ccc1a6314548b5cd74fc97513850e375bbcf92 (patch) | |
tree | 964fda3a01d4a583e172bc56f12769b8078408cb /net | |
parent | f79da8da073d7ee29619ac3d0e6f76ec0dc9c624 (diff) | |
download | chromium_src-e6ccc1a6314548b5cd74fc97513850e375bbcf92.zip chromium_src-e6ccc1a6314548b5cd74fc97513850e375bbcf92.tar.gz chromium_src-e6ccc1a6314548b5cd74fc97513850e375bbcf92.tar.bz2 |
Land Recent QUIC Changes.
QUIC refactor to move the receive algorithm from QuicCongestionManager
into the ReceivedPacketManager.
Part of a larger congestion control refactor.
Merge internal change: 57325085
https://codereview.chromium.org/92593003/
Fix bug in cl/57315483 in which TerminateFromPeer(true) was turned into
CloseReadSide(). This looks correct, except that QuicSpdyServerStream
overrides this method so it wasn't being called.
Merge internal change: 57324551
https://codereview.chromium.org/93083002/
Rename QuicStreamSequencer::IsHalfClosed -> IsClosed. The sequencer only
has one path so it doesn't make much sense for it to be "half" closed.
Rename ReliableQuicStream::IsHalfClosed -> IsDoneReading
because the semantics are such that "IsHalfClosed" returned true when
the fin had been read, but before the read side had actually been
closed, which is confusing.
Rename two QUIC methods, no behavior change.
Merge internal change: 57315483
https://codereview.chromium.org/93073002/
The QuicAckNotifier should only be attached to a stream frame if the
stream frame contains data. If we didn't consume any data (e.g. it is a
FIN only frame), then attaching it to the frame results in
use-after-delete.
Fix use-after-delete in QuicAckNotifier.
Merge internal change: 57311555
https://codereview.chromium.org/92813003/
Removed the unused compressed parameter from CreateSynStream,
CreateSynReply and CreateHeaders methods.
Code cleanup, no behavioural change.
Merge internal change: 57308486
https://codereview.chromium.org/92793004/
Removing unnecessary received_truncated_ack_ local variable now that
acks have an explicit truncation flag.
Merge internal change: 57297471
https://codereview.chromium.org/92833003/
Change the Beta value used to reduce the TCP cubic congestion window in
QUIC to 1/12, the same as SPDY.
Merge internal change: 57269049
https://codereview.chromium.org/93033002/
R=rch@chromium.org
Review URL: https://codereview.chromium.org/93523002
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@238044 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'net')
34 files changed, 180 insertions, 210 deletions
diff --git a/net/quic/congestion_control/cubic.cc b/net/quic/congestion_control/cubic.cc index ae6c58b..3d03b9f 100644 --- a/net/quic/congestion_control/cubic.cc +++ b/net/quic/congestion_control/cubic.cc @@ -19,8 +19,10 @@ const int kCubeScale = 40; // 1024*1024^3 (first 1024 is from 0.100^3) // where 0.100 is 100 ms which is the scaling // round trip time. const int kCubeCongestionWindowScale = 410; -const uint64 kCubeFactor = (1ull << kCubeScale) / kCubeCongestionWindowScale; -const uint32 kBeta = 717; // Back off factor after loss. +const uint64 kCubeFactor = (GG_UINT64_C(1) << kCubeScale) / + kCubeCongestionWindowScale; +const uint32 kBetaSPDY = 939; // Back off factor after loss for SPDY, reduces + // the CWND by 1/12th. const uint32 kBetaLastMax = 871; // Additional back off factor after loss for // the stored max value. @@ -129,7 +131,7 @@ QuicTcpCongestionWindow Cubic::CongestionWindowAfterPacketLoss( last_max_congestion_window_ = current_congestion_window; } epoch_ = QuicTime::Zero(); // Reset time. - return (current_congestion_window * kBeta) >> 10; + return (current_congestion_window * kBetaSPDY) >> 10; } QuicTcpCongestionWindow Cubic::CongestionWindowAfterAck( diff --git a/net/quic/congestion_control/cubic_test.cc b/net/quic/congestion_control/cubic_test.cc index 84f2a7b..c01bb05 100644 --- a/net/quic/congestion_control/cubic_test.cc +++ b/net/quic/congestion_control/cubic_test.cc @@ -109,10 +109,10 @@ TEST_F(CubicTest, LossEvents) { clock_.AdvanceTime(one_ms_); EXPECT_EQ(expected_cwnd, cubic_->CongestionWindowAfterAck(current_cwnd, rtt_min)); - expected_cwnd = current_cwnd * 717 / 1024; + expected_cwnd = current_cwnd * 939 / 1024; EXPECT_EQ(expected_cwnd, cubic_->CongestionWindowAfterPacketLoss(current_cwnd)); - expected_cwnd = current_cwnd * 717 / 1024; + expected_cwnd = current_cwnd * 939 / 1024; EXPECT_EQ(expected_cwnd, cubic_->CongestionWindowAfterPacketLoss(current_cwnd)); } @@ -126,7 +126,7 @@ TEST_F(CubicTest, BelowOrgin) { clock_.AdvanceTime(one_ms_); EXPECT_EQ(expected_cwnd, cubic_->CongestionWindowAfterAck(current_cwnd, rtt_min)); - expected_cwnd = current_cwnd * 717 / 1024; + expected_cwnd = current_cwnd * 939 / 1024; EXPECT_EQ(expected_cwnd, cubic_->CongestionWindowAfterPacketLoss(current_cwnd)); current_cwnd = expected_cwnd; @@ -142,7 +142,7 @@ TEST_F(CubicTest, BelowOrgin) { clock_.AdvanceTime(hundred_ms_); current_cwnd = cubic_->CongestionWindowAfterAck(current_cwnd, rtt_min); } - expected_cwnd = 422; + expected_cwnd = 440; EXPECT_EQ(expected_cwnd, current_cwnd); } diff --git a/net/quic/congestion_control/quic_congestion_manager.cc b/net/quic/congestion_control/quic_congestion_manager.cc index 72cc187..c97136a 100644 --- a/net/quic/congestion_control/quic_congestion_manager.cc +++ b/net/quic/congestion_control/quic_congestion_manager.cc @@ -56,10 +56,9 @@ COMPILE_ASSERT(kHistoryPeriodMs >= kBitrateSmoothingPeriodMs, QuicCongestionManager::QuicCongestionManager( const QuicClock* clock, - CongestionFeedbackType type) + CongestionFeedbackType congestion_type) : clock_(clock), - receive_algorithm_(ReceiveAlgorithmInterface::Create(clock, type)), - send_algorithm_(SendAlgorithmInterface::Create(clock, type)), + send_algorithm_(SendAlgorithmInterface::Create(clock, congestion_type)), rtt_sample_(QuicTime::Delta::Infinite()), consecutive_rto_count_(0), using_pacing_(false) { @@ -74,7 +73,7 @@ void QuicCongestionManager::SetFromConfig(const QuicConfig& config, if (config.initial_round_trip_time_us() > 0 && rtt_sample_.IsInfinite()) { // The initial rtt should already be set on the client side. - DLOG_IF(INFO, !is_server) + DVLOG_IF(1, !is_server) << "Client did not set an initial RTT, but did negotiate one."; rtt_sample_ = QuicTime::Delta::FromMicroseconds(config.initial_round_trip_time_us()); @@ -256,20 +255,6 @@ QuicTime::Delta QuicCongestionManager::TimeUntilSend( handshake); } -bool QuicCongestionManager::GenerateCongestionFeedback( - QuicCongestionFeedbackFrame* feedback) { - return receive_algorithm_->GenerateCongestionFeedback(feedback); -} - -void QuicCongestionManager::RecordIncomingPacket( - QuicByteCount bytes, - QuicPacketSequenceNumber sequence_number, - QuicTime timestamp, - bool revived) { - receive_algorithm_->RecordIncomingPacket(bytes, sequence_number, timestamp, - revived); -} - const QuicTime::Delta QuicCongestionManager::DefaultRetransmissionTime() { return QuicTime::Delta::FromMilliseconds(kDefaultRetransmissionTimeMs); } diff --git a/net/quic/congestion_control/quic_congestion_manager.h b/net/quic/congestion_control/quic_congestion_manager.h index 5c8d965..42be9c2 100644 --- a/net/quic/congestion_control/quic_congestion_manager.h +++ b/net/quic/congestion_control/quic_congestion_manager.h @@ -79,24 +79,6 @@ class NET_EXPORT_PRIVATE QuicCongestionManager { HasRetransmittableData retransmittable, IsHandshake handshake); - // Should be called before sending an ACK packet, to decide if we need - // to attach a QuicCongestionFeedbackFrame block. - // Returns false if no QuicCongestionFeedbackFrame block is needed. - // Otherwise fills in feedback and returns true. - virtual bool GenerateCongestionFeedback( - QuicCongestionFeedbackFrame* feedback); - - // Should be called for each incoming packet. - // bytes: the packet size in bytes including Quic Headers. - // sequence_number: the unique sequence number from the QUIC packet header. - // timestamp: the arrival time of the packet. - // revived: true if the packet was lost and then recovered with help of a - // FEC packet. - virtual void RecordIncomingPacket(QuicByteCount bytes, - QuicPacketSequenceNumber sequence_number, - QuicTime timestamp, - bool revived); - const QuicTime::Delta DefaultRetransmissionTime(); // Returns amount of time for delayed ack timer. @@ -132,7 +114,6 @@ class NET_EXPORT_PRIVATE QuicCongestionManager { void CleanupPacketHistory(); const QuicClock* clock_; - scoped_ptr<ReceiveAlgorithmInterface> receive_algorithm_; scoped_ptr<SendAlgorithmInterface> send_algorithm_; // Tracks the send time, size, and nack count of sent packets. Packets are // removed after 5 seconds and they've been removed from pending_packets_. diff --git a/net/quic/congestion_control/receive_algorithm_interface.cc b/net/quic/congestion_control/receive_algorithm_interface.cc index e8dacf0..a49f64f 100644 --- a/net/quic/congestion_control/receive_algorithm_interface.cc +++ b/net/quic/congestion_control/receive_algorithm_interface.cc @@ -11,7 +11,6 @@ namespace net { // Factory for receive side congestion control algorithm. ReceiveAlgorithmInterface* ReceiveAlgorithmInterface::Create( - const QuicClock* clock, CongestionFeedbackType type) { switch (type) { case kTCP: diff --git a/net/quic/congestion_control/receive_algorithm_interface.h b/net/quic/congestion_control/receive_algorithm_interface.h index e2bae4b..17e7793 100644 --- a/net/quic/congestion_control/receive_algorithm_interface.h +++ b/net/quic/congestion_control/receive_algorithm_interface.h @@ -17,8 +17,7 @@ namespace net { class NET_EXPORT_PRIVATE ReceiveAlgorithmInterface { public: - static ReceiveAlgorithmInterface* Create(const QuicClock* clock, - CongestionFeedbackType type); + static ReceiveAlgorithmInterface* Create(CongestionFeedbackType type); virtual ~ReceiveAlgorithmInterface() {} diff --git a/net/quic/quic_ack_notifier_manager.cc b/net/quic/quic_ack_notifier_manager.cc index 2221e30..8676315 100644 --- a/net/quic/quic_ack_notifier_manager.cc +++ b/net/quic/quic_ack_notifier_manager.cc @@ -86,18 +86,18 @@ void AckNotifierManager::OnSerializedPacket( for (QuicFrames::const_iterator it = frames->frames().begin(); it != frames->frames().end(); ++it) { if (it->type == STREAM_FRAME && it->stream_frame->notifier != NULL) { + QuicAckNotifier* notifier = it->stream_frame->notifier; + // The AckNotifier needs to know it is tracking this packet's sequence // number. - it->stream_frame->notifier->AddSequenceNumber( - serialized_packet.sequence_number); - - // Add the AckNotifier to our set of AckNotifiers. - ack_notifiers_.insert(it->stream_frame->notifier); + notifier->AddSequenceNumber(serialized_packet.sequence_number); // Update the mapping in the other direction, from sequence // number to AckNotifier. - ack_notifier_map_[serialized_packet.sequence_number] - .insert(it->stream_frame->notifier); + ack_notifier_map_[serialized_packet.sequence_number].insert(notifier); + + // Take ownership of the AckNotifier. + ack_notifiers_.insert(notifier); } } } diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc index 900dc19..d314451 100644 --- a/net/quic/quic_connection.cc +++ b/net/quic/quic_connection.cc @@ -205,6 +205,7 @@ QuicConnection::QuicConnection(QuicGuid guid, largest_seen_packet_with_ack_(0), pending_version_negotiation_packet_(false), write_blocked_(false), + received_packet_manager_(kTCP), ack_alarm_(helper->CreateAlarm(new AckAlarm(this))), retransmission_alarm_(helper->CreateAlarm(new RetransmissionAlarm(this))), abandon_fec_alarm_(helper->CreateAlarm(new AbandonFecAlarm(this))), @@ -226,7 +227,6 @@ QuicConnection::QuicConnection(QuicGuid guid, version_negotiation_state_(START_NEGOTIATION), is_server_(is_server), connected_(true), - received_truncated_ack_(false), address_migrating_(false) { if (!is_server_) { // Pacing will be enabled if the client negotiates it. @@ -513,8 +513,6 @@ void QuicConnection::ProcessAckFrame(const QuicAckFrame& incoming_ack) { largest_seen_packet_with_ack_ = last_header_.packet_sequence_number; - received_truncated_ack_ = incoming_ack.received_info.is_truncated; - received_packet_manager_.UpdatePacketInformationReceivedByPeer(incoming_ack); received_packet_manager_.UpdatePacketInformationSentByPeer(incoming_ack); // Possibly close any FecGroups which are now irrelevant. @@ -523,8 +521,7 @@ void QuicConnection::ProcessAckFrame(const QuicAckFrame& incoming_ack) { sent_entropy_manager_.ClearEntropyBefore( received_packet_manager_.least_packet_awaited_by_peer() - 1); - sent_packet_manager_.OnPacketAcked(incoming_ack.received_info, - received_truncated_ack_); + sent_packet_manager_.OnPacketAcked(incoming_ack.received_info); // Get the updated least unacked sequence number. QuicPacketSequenceNumber least_unacked_sent_after = @@ -706,11 +703,6 @@ void QuicConnection::OnPacketComplete() { << last_close_frames_.size() << " closes, " << last_stream_frames_.size() << " stream frames for " << last_header_.public_header.guid; - if (!last_packet_revived_) { - congestion_manager_.RecordIncomingPacket( - last_size_, last_header_.packet_sequence_number, - time_of_last_received_packet_, last_packet_revived_); - } // Must called before ack processing, because processing acks removes entries // from unacket_packets_, increasing the least_unacked. @@ -727,8 +719,10 @@ void QuicConnection::OnPacketComplete() { // processing the rest of the packet. if (last_stream_frames_.empty() || visitor_->OnStreamFrames(last_stream_frames_)) { - received_packet_manager_.RecordPacketReceived( - last_header_, time_of_last_received_packet_); + received_packet_manager_.RecordPacketReceived(last_size_, + last_header_, + time_of_last_received_packet_, + last_packet_revived_); for (size_t i = 0; i < last_stream_frames_.size(); ++i) { stats_.stream_bytes_received += last_stream_frames_[i].data.TotalBufferSize(); @@ -874,7 +868,7 @@ QuicConsumedData QuicConnection::SendStreamData( } // This notifier will be owned by the AckNotifierManager (or deleted below if - // no data was consumed). + // no data or FIN was consumed). QuicAckNotifier* notifier = NULL; if (delegate) { notifier = new QuicAckNotifier(delegate); @@ -886,8 +880,9 @@ QuicConsumedData QuicConnection::SendStreamData( QuicConsumedData consumed_data = packet_generator_.ConsumeData(id, data, offset, fin, notifier); - if (notifier && consumed_data.bytes_consumed == 0) { - // No data was consumed, delete the notifier. + if (notifier && + (consumed_data.bytes_consumed == 0 && !consumed_data.fin_consumed)) { + // No data was consumed, nor was a fin consumed, so delete the notifier. delete notifier; } @@ -1482,7 +1477,7 @@ void QuicConnection::SendAck() { // method is invoked. This requires changes SetShouldSendAck // to be a no-arg method, and re-jiggering its implementation. bool send_feedback = false; - if (congestion_manager_.GenerateCongestionFeedback( + if (received_packet_manager_.GenerateCongestionFeedback( &outgoing_congestion_feedback_)) { DVLOG(1) << ENDPOINT << "Sending feedback: " << outgoing_congestion_feedback_; diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h index feafa5d..3c85e66 100644 --- a/net/quic/quic_connection.h +++ b/net/quic/quic_connection.h @@ -775,10 +775,6 @@ class NET_EXPORT_PRIVATE QuicConnection // close. bool connected_; - // True if the last ack received from the peer may have been truncated. False - // otherwise. - bool received_truncated_ack_; - // Set to true if the udp packet headers have a new self or peer address. // This is checked later on validating a data or version negotiation packet. bool address_migrating_; diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc index 123aee9..f429664 100644 --- a/net/quic/quic_connection_test.cc +++ b/net/quic/quic_connection_test.cc @@ -896,8 +896,8 @@ TEST_F(QuicConnectionTest, RejectPacketTooFarOut) { TEST_F(QuicConnectionTest, TruncatedAck) { EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); - int num_packets = 256 * 2 + 1; - for (int i = 0; i < num_packets; ++i) { + QuicPacketSequenceNumber num_packets = 256 * 2 + 1; + for (QuicPacketSequenceNumber i = 0; i < num_packets; ++i) { SendStreamDataToPeer(1, "foo", i * 3, !kFin, NULL); } @@ -913,7 +913,11 @@ TEST_F(QuicConnectionTest, TruncatedAck) { EXPECT_CALL(*send_algorithm_, OnPacketAbandoned(_, _)).Times(10); ProcessAckPacket(&frame); - EXPECT_TRUE(QuicConnectionPeer::GetReceivedTruncatedAck(&connection_)); + QuicReceivedPacketManager* received_packet_manager = + QuicConnectionPeer::GetReceivedPacketManager(&connection_); + // A truncated ack will not have the true largest observed. + EXPECT_GT(num_packets, + received_packet_manager->peer_largest_observed_packet()); frame.received_info.missing_packets.erase(192); @@ -922,7 +926,8 @@ TEST_F(QuicConnectionTest, TruncatedAck) { EXPECT_CALL(*send_algorithm_, OnPacketLost(_, _)).Times(10); EXPECT_CALL(*send_algorithm_, OnPacketAbandoned(_, _)).Times(10); ProcessAckPacket(&frame); - EXPECT_FALSE(QuicConnectionPeer::GetReceivedTruncatedAck(&connection_)); + EXPECT_EQ(num_packets, + received_packet_manager->peer_largest_observed_packet()); } TEST_F(QuicConnectionTest, AckReceiptCausesAckSendBadEntropy) { diff --git a/net/quic/quic_packet_creator_test.cc b/net/quic/quic_packet_creator_test.cc index 27370a9..e8dd174 100644 --- a/net/quic/quic_packet_creator_test.cc +++ b/net/quic/quic_packet_creator_test.cc @@ -502,6 +502,22 @@ TEST_F(QuicPacketCreatorTest, UpdatePacketSequenceNumberLengthBandwidth) { creator_.options()->send_sequence_number_length); } +TEST_F(QuicPacketCreatorTest, CreateStreamFrameWithNotifier) { + // Ensure that if CreateStreamFrame does not consume any data (e.g. a FIN only + // frame) then any QuicAckNotifier that is passed in still gets attached to + // the frame. + MockAckNotifierDelegate delegate; + QuicAckNotifier notifier(&delegate); + QuicFrame frame; + IOVector empty_iovector; + bool fin = true; + size_t consumed_bytes = creator_.CreateStreamFrameWithNotifier( + 1u, empty_iovector, 0u, fin, ¬ifier, &frame); + EXPECT_EQ(0u, consumed_bytes); + EXPECT_EQ(¬ifier, frame.stream_frame->notifier); + delete frame.stream_frame; +} + INSTANTIATE_TEST_CASE_P(ToggleVersionSerialization, QuicPacketCreatorTest, ::testing::Values(false, true)); diff --git a/net/quic/quic_received_packet_manager.cc b/net/quic/quic_received_packet_manager.cc index 3047c64..14e91b2 100644 --- a/net/quic/quic_received_packet_manager.cc +++ b/net/quic/quic_received_packet_manager.cc @@ -13,13 +13,15 @@ using std::min; namespace net { -QuicReceivedPacketManager::QuicReceivedPacketManager() +QuicReceivedPacketManager::QuicReceivedPacketManager( + CongestionFeedbackType congestion_type) : packets_entropy_hash_(0), largest_sequence_number_(0), peer_largest_observed_packet_(0), least_packet_awaited_by_peer_(1), peer_least_packet_awaiting_ack_(0), - time_largest_observed_(QuicTime::Zero()) { + time_largest_observed_(QuicTime::Zero()), + receive_algorithm_(ReceiveAlgorithmInterface::Create(congestion_type)) { received_info_.largest_observed = 0; received_info_.entropy_hash = 0; } @@ -27,8 +29,10 @@ QuicReceivedPacketManager::QuicReceivedPacketManager() QuicReceivedPacketManager::~QuicReceivedPacketManager() {} void QuicReceivedPacketManager::RecordPacketReceived( + QuicByteCount bytes, const QuicPacketHeader& header, - QuicTime receipt_time) { + QuicTime receipt_time, + bool revived) { QuicPacketSequenceNumber sequence_number = header.packet_sequence_number; DCHECK(IsAwaitingPacket(sequence_number)); @@ -48,6 +52,12 @@ void QuicReceivedPacketManager::RecordPacketReceived( time_largest_observed_ = receipt_time; } RecordPacketEntropyHash(sequence_number, header.entropy_hash); + + // Don't update the receive algorithm for revived packets. + if (!revived) { + receive_algorithm_->RecordIncomingPacket( + bytes, sequence_number, receipt_time, revived); + } } bool QuicReceivedPacketManager::IsAwaitingPacket( @@ -94,6 +104,11 @@ void QuicReceivedPacketManager::RecordPacketEntropyHash( << " entropy hash: " << static_cast<int>(entropy_hash); } +bool QuicReceivedPacketManager::GenerateCongestionFeedback( + QuicCongestionFeedbackFrame* feedback) { + return receive_algorithm_->GenerateCongestionFeedback(feedback); +} + QuicPacketEntropyHash QuicReceivedPacketManager::EntropyHash( QuicPacketSequenceNumber sequence_number) const { DCHECK_LE(sequence_number, received_info_.largest_observed); diff --git a/net/quic/quic_received_packet_manager.h b/net/quic/quic_received_packet_manager.h index f25c35f..caa172c 100644 --- a/net/quic/quic_received_packet_manager.h +++ b/net/quic/quic_received_packet_manager.h @@ -8,12 +8,14 @@ #ifndef NET_QUIC_QUIC_RECEIVED_PACKET_MANAGER_H_ #define NET_QUIC_QUIC_RECEIVED_PACKET_MANAGER_H_ +#include "net/quic/congestion_control/receive_algorithm_interface.h" #include "net/quic/quic_framer.h" #include "net/quic/quic_protocol.h" namespace net { namespace test { +class QuicConnectionPeer; class QuicReceivedPacketManagerPeer; } // namespace test @@ -23,12 +25,19 @@ class QuicReceivedPacketManagerPeer; class NET_EXPORT_PRIVATE QuicReceivedPacketManager : public QuicReceivedEntropyHashCalculatorInterface { public: - QuicReceivedPacketManager(); + explicit QuicReceivedPacketManager(CongestionFeedbackType congestion_type); virtual ~QuicReceivedPacketManager(); - // Updates the internal state concerning which packets have been acked. - void RecordPacketReceived(const QuicPacketHeader& header, - QuicTime receipt_time); + // Updates the internal state concerning which packets have been received. + // bytes: the packet size in bytes including Quic Headers. + // header: the packet header. + // timestamp: the arrival time of the packet. + // revived: true if the packet was lost and then recovered with help of a + // FEC packet. + void RecordPacketReceived(QuicByteCount bytes, + const QuicPacketHeader& header, + QuicTime receipt_time, + bool revived); // Checks if we're still waiting for the packet with |sequence_number|. bool IsAwaitingPacket(QuicPacketSequenceNumber sequence_number); @@ -37,6 +46,13 @@ class NET_EXPORT_PRIVATE QuicReceivedPacketManager : void UpdateReceivedPacketInfo(ReceivedPacketInfo* received_info, QuicTime approximate_now); + // Should be called before sending an ACK packet, to decide if we need + // to attach a QuicCongestionFeedbackFrame block. + // Returns false if no QuicCongestionFeedbackFrame block is needed. + // Otherwise fills in feedback and returns true. + virtual bool GenerateCongestionFeedback( + QuicCongestionFeedbackFrame* feedback); + // QuicReceivedEntropyHashCalculatorInterface // Called by QuicFramer, when the outgoing ack gets truncated, to recalculate // the received entropy hash for the truncated ack frame. @@ -66,6 +82,7 @@ class NET_EXPORT_PRIVATE QuicReceivedPacketManager : } private: + friend class test::QuicConnectionPeer; friend class test::QuicReceivedPacketManagerPeer; typedef std::map<QuicPacketSequenceNumber, @@ -120,6 +137,8 @@ class NET_EXPORT_PRIVATE QuicReceivedPacketManager : // no sequence numbers have been received since UpdateReceivedPacketInfo. // Needed for calculating delta_time_largest_observed. QuicTime time_largest_observed_; + + scoped_ptr<ReceiveAlgorithmInterface> receive_algorithm_; }; } // namespace net diff --git a/net/quic/quic_received_packet_manager_test.cc b/net/quic/quic_received_packet_manager_test.cc index dd7d529..9d11129 100644 --- a/net/quic/quic_received_packet_manager_test.cc +++ b/net/quic/quic_received_packet_manager_test.cc @@ -21,12 +21,14 @@ namespace { class QuicReceivedPacketManagerTest : public ::testing::Test { protected: + QuicReceivedPacketManagerTest() : received_manager_(kTCP) { } + void RecordPacketEntropyHash(QuicPacketSequenceNumber sequence_number, QuicPacketEntropyHash entropy_hash) { QuicPacketHeader header; header.packet_sequence_number = sequence_number; header.entropy_hash = entropy_hash; - received_manager_.RecordPacketReceived(header, QuicTime::Zero());; + received_manager_.RecordPacketReceived(0u, header, QuicTime::Zero(), false); } QuicReceivedPacketManager received_manager_; @@ -103,9 +105,9 @@ TEST_F(QuicReceivedPacketManagerTest, RecalculateEntropyHash) { TEST_F(QuicReceivedPacketManagerTest, DontWaitForPacketsBefore) { QuicPacketHeader header; header.packet_sequence_number = 2u; - received_manager_.RecordPacketReceived(header, QuicTime::Zero()); + received_manager_.RecordPacketReceived(0u, header, QuicTime::Zero(), false); header.packet_sequence_number = 7u; - received_manager_.RecordPacketReceived(header, QuicTime::Zero()); + received_manager_.RecordPacketReceived(0u, header, QuicTime::Zero(), false); EXPECT_TRUE(received_manager_.IsAwaitingPacket(3u)); EXPECT_TRUE(received_manager_.IsAwaitingPacket(6u)); EXPECT_TRUE(QuicReceivedPacketManagerPeer::DontWaitForPacketsBefore( @@ -118,7 +120,7 @@ TEST_F(QuicReceivedPacketManagerTest, UpdateReceivedPacketInfo) { QuicPacketHeader header; header.packet_sequence_number = 2u; QuicTime two_ms = QuicTime::Zero().Add(QuicTime::Delta::FromMilliseconds(2)); - received_manager_.RecordPacketReceived(header, two_ms); + received_manager_.RecordPacketReceived(0u, header, two_ms, false); ReceivedPacketInfo info; received_manager_.UpdateReceivedPacketInfo(&info, QuicTime::Zero()); diff --git a/net/quic/quic_sent_packet_manager.cc b/net/quic/quic_sent_packet_manager.cc index 43a733e..104cbf7 100644 --- a/net/quic/quic_sent_packet_manager.cc +++ b/net/quic/quic_sent_packet_manager.cc @@ -117,9 +117,8 @@ void QuicSentPacketManager::OnRetransmittedPacket( } void QuicSentPacketManager::OnPacketAcked( - const ReceivedPacketInfo& received_info, - bool is_truncated_ack) { - HandleAckForSentPackets(received_info, is_truncated_ack); + const ReceivedPacketInfo& received_info) { + HandleAckForSentPackets(received_info); HandleAckForSentFecPackets(received_info); } @@ -129,8 +128,7 @@ void QuicSentPacketManager::DiscardUnackedPacket( } void QuicSentPacketManager::HandleAckForSentPackets( - const ReceivedPacketInfo& received_info, - bool is_truncated_ack) { + const ReceivedPacketInfo& received_info) { // Go through the packets we have not received an ack for and see if this // incoming_ack shows they've been seen by the peer. UnackedPacketMap::iterator it = unacked_packets_.begin(); @@ -159,7 +157,7 @@ void QuicSentPacketManager::HandleAckForSentPackets( // If we have received a trunacted ack, then we need to // clear out some previous transmissions to allow the peer // to actually ACK new packets. - if (is_truncated_ack) { + if (received_info.is_truncated) { size_t num_to_clear = received_info.missing_packets.size() / 2; UnackedPacketMap::iterator it = unacked_packets_.begin(); while (it != unacked_packets_.end() && num_to_clear > 0) { diff --git a/net/quic/quic_sent_packet_manager.h b/net/quic/quic_sent_packet_manager.h index 59823d4..090f318 100644 --- a/net/quic/quic_sent_packet_manager.h +++ b/net/quic/quic_sent_packet_manager.h @@ -71,8 +71,7 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager { QuicPacketSequenceNumber new_sequence_number); // Processes the ReceivedPacketInfo data from the incoming ack. - void OnPacketAcked(const ReceivedPacketInfo& received_info, - bool is_truncated_ack); + void OnPacketAcked(const ReceivedPacketInfo& received_info); // Discards any information for the packet corresponding to |sequence_number|. // If this packet has been retransmitted, information on those packets @@ -165,8 +164,7 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager { PreviousTransmissionMap; // Process the incoming ack looking for newly ack'd data packets. - void HandleAckForSentPackets(const ReceivedPacketInfo& received_info, - bool is_truncated_ack); + void HandleAckForSentPackets(const ReceivedPacketInfo& received_info); // Process the incoming ack looking for newly ack'd FEC packets. void HandleAckForSentFecPackets(const ReceivedPacketInfo& received_info); diff --git a/net/quic/quic_sent_packet_manager_test.cc b/net/quic/quic_sent_packet_manager_test.cc index 156e805..bb3e9fc 100644 --- a/net/quic/quic_sent_packet_manager_test.cc +++ b/net/quic/quic_sent_packet_manager_test.cc @@ -150,7 +150,7 @@ TEST_F(QuicSentPacketManagerTest, RetransmitThenAck) { ReceivedPacketInfo received_info; received_info.largest_observed = 2; received_info.missing_packets.insert(1); - manager_.OnPacketAcked(received_info, false); + manager_.OnPacketAcked(received_info); // No unacked packets remain. VerifyUnackedPackets(NULL, 0); @@ -167,7 +167,7 @@ TEST_F(QuicSentPacketManagerTest, RetransmitThenAckBeforeSend) { // Ack 1. ReceivedPacketInfo received_info; received_info.largest_observed = 1; - manager_.OnPacketAcked(received_info, false); + manager_.OnPacketAcked(received_info); // There should no longer be a pending retransmission. EXPECT_FALSE(manager_.HasPendingRetransmissions()); @@ -187,7 +187,7 @@ TEST_F(QuicSentPacketManagerTest, RetransmitThenAckPrevious) { ReceivedPacketInfo received_info; received_info.largest_observed = 2; received_info.missing_packets.insert(2); - manager_.OnPacketAcked(received_info, false); + manager_.OnPacketAcked(received_info); // 2 remains unacked, but no packets have retransmittable data. QuicPacketSequenceNumber unacked[] = { 2 }; @@ -208,7 +208,8 @@ TEST_F(QuicSentPacketManagerTest, TruncatedAck) { received_info.largest_observed = 2; received_info.missing_packets.insert(1); received_info.missing_packets.insert(2); - manager_.OnPacketAcked(received_info, true); + received_info.is_truncated = true; + manager_.OnPacketAcked(received_info); // High water mark will be raised. QuicPacketSequenceNumber unacked[] = { 2, 3, 4 }; @@ -227,7 +228,7 @@ TEST_F(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) { ReceivedPacketInfo received_info; received_info.largest_observed = 3; received_info.missing_packets.insert(2); - manager_.OnPacketAcked(received_info, false); + manager_.OnPacketAcked(received_info); QuicPacketSequenceNumber unacked[] = { 2 }; VerifyUnackedPackets(unacked, arraysize(unacked)); @@ -244,7 +245,7 @@ TEST_F(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) { received_info.largest_observed = 5; received_info.missing_packets.insert(2); received_info.missing_packets.insert(4); - manager_.OnPacketAcked(received_info, false); + manager_.OnPacketAcked(received_info); QuicPacketSequenceNumber unacked[] = { 2, 4 }; VerifyUnackedPackets(unacked, arraysize(unacked)); @@ -262,7 +263,7 @@ TEST_F(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) { received_info.missing_packets.insert(2); received_info.missing_packets.insert(4); received_info.missing_packets.insert(6); - manager_.OnPacketAcked(received_info, false); + manager_.OnPacketAcked(received_info); QuicPacketSequenceNumber unacked[] = { 2, 4, 6 }; VerifyUnackedPackets(unacked, arraysize(unacked)); @@ -283,7 +284,7 @@ TEST_F(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) { received_info.missing_packets.insert(6); received_info.missing_packets.insert(8); received_info.missing_packets.insert(9); - manager_.OnPacketAcked(received_info, false); + manager_.OnPacketAcked(received_info); QuicPacketSequenceNumber unacked[] = { 2, 4, 6, 8, 9 }; VerifyUnackedPackets(unacked, arraysize(unacked)); @@ -307,7 +308,7 @@ TEST_F(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) { received_info.missing_packets.insert(9); received_info.missing_packets.insert(11); received_info.missing_packets.insert(12); - manager_.OnPacketAcked(received_info, false); + manager_.OnPacketAcked(received_info); QuicPacketSequenceNumber unacked[] = { 2, 4, 6, 8, 9, 11, 12 }; VerifyUnackedPackets(unacked, arraysize(unacked)); @@ -330,7 +331,8 @@ TEST_F(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) { received_info.missing_packets.insert(9); received_info.missing_packets.insert(11); received_info.missing_packets.insert(12); - manager_.OnPacketAcked(received_info, true); + received_info.is_truncated = true; + manager_.OnPacketAcked(received_info); // Truncated ack raises the high water mark by clearing out 2, 4, and 6. QuicPacketSequenceNumber unacked[] = { 8, 9, 11, 12, 14, 15, 16 }; @@ -394,7 +396,7 @@ TEST_F(QuicSentPacketManagerTest, GetLeastUnackedFecPacketAndDiscard) { // Ack 2. ReceivedPacketInfo received_info; received_info.largest_observed = 2; - manager_.OnPacketAcked(received_info, false); + manager_.OnPacketAcked(received_info); EXPECT_EQ(3u, manager_.GetLeastUnackedFecPacket()); diff --git a/net/quic/quic_spdy_compressor.cc b/net/quic/quic_spdy_compressor.cc index 0a1bfb4..55015b0 100644 --- a/net/quic/quic_spdy_compressor.cc +++ b/net/quic/quic_spdy_compressor.cc @@ -40,7 +40,7 @@ string QuicSpdyCompressor::CompressHeadersInternal( // CreateCompressedHeaderBlock method, or some such. SpdyStreamId stream_id = 3; // unused. scoped_ptr<SpdyFrame> frame(spdy_framer_.CreateHeaders( - stream_id, CONTROL_FLAG_NONE, true, &headers)); + stream_id, CONTROL_FLAG_NONE, &headers)); // The size of the spdy HEADER frame's fixed prefix which // needs to be stripped off from the resulting frame. diff --git a/net/quic/quic_stream_sequencer.cc b/net/quic/quic_stream_sequencer.cc index b7f5e46..ea7fb4d 100644 --- a/net/quic/quic_stream_sequencer.cc +++ b/net/quic/quic_stream_sequencer.cc @@ -143,7 +143,7 @@ void QuicStreamSequencer::CloseStreamAtOffset(QuicStreamOffset offset) { } bool QuicStreamSequencer::MaybeCloseStream() { - if (IsHalfClosed()) { + if (IsClosed()) { DVLOG(1) << "Passing up termination, as we've processed " << num_bytes_consumed_ << " of " << close_offset_ << " bytes."; @@ -251,7 +251,7 @@ bool QuicStreamSequencer::HasBytesToRead() const { return it != frames_.end() && it->first == num_bytes_consumed_; } -bool QuicStreamSequencer::IsHalfClosed() const { +bool QuicStreamSequencer::IsClosed() const { return num_bytes_consumed_ >= close_offset_; } diff --git a/net/quic/quic_stream_sequencer.h b/net/quic/quic_stream_sequencer.h index a450bef..e260124 100644 --- a/net/quic/quic_stream_sequencer.h +++ b/net/quic/quic_stream_sequencer.h @@ -65,8 +65,8 @@ class NET_EXPORT_PRIVATE QuicStreamSequencer { // Returns true if the sequncer has bytes available for reading. bool HasBytesToRead() const; - // Returns true if the sequencer has delivered a half close. - bool IsHalfClosed() const; + // Returns true if the sequencer has delivered the fin. + bool IsClosed() const; // Returns true if the sequencer has received this frame before. bool IsDuplicate(const QuicStreamFrame& frame) const; @@ -91,7 +91,7 @@ class NET_EXPORT_PRIVATE QuicStreamSequencer { FrameMap frames_; // sequence number -> frame size_t max_frame_memory_; // the maximum memory the sequencer can buffer. // The offset, if any, we got a stream termination for. When this many bytes - // have been processed, the stream will be half closed. + // have been processed, the sequencer will be closed. QuicStreamOffset close_offset_; }; diff --git a/net/quic/quic_stream_sequencer_test.cc b/net/quic/quic_stream_sequencer_test.cc index b5f0f94..4240cb5 100644 --- a/net/quic/quic_stream_sequencer_test.cc +++ b/net/quic/quic_stream_sequencer_test.cc @@ -465,7 +465,7 @@ TEST_F(QuicStreamSequencerTest, TerminateWithReadv) { sequencer_->OnFinFrame(3, ""); EXPECT_EQ(3u, sequencer_->close_offset()); - EXPECT_FALSE(sequencer_->IsHalfClosed()); + EXPECT_FALSE(sequencer_->IsClosed()); EXPECT_CALL(stream_, ProcessData(StrEq("abc"), 3)).WillOnce(Return(0)); EXPECT_TRUE(sequencer_->OnFrame(0, "abc")); @@ -473,7 +473,7 @@ TEST_F(QuicStreamSequencerTest, TerminateWithReadv) { iovec iov = { &buffer[0], 3 }; int bytes_read = sequencer_->Readv(&iov, 1); EXPECT_EQ(3, bytes_read); - EXPECT_TRUE(sequencer_->IsHalfClosed()); + EXPECT_TRUE(sequencer_->IsClosed()); } TEST_F(QuicStreamSequencerTest, MutipleOffsets) { diff --git a/net/quic/reliable_quic_stream.cc b/net/quic/reliable_quic_stream.cc index 6a33fa6..967a814 100644 --- a/net/quic/reliable_quic_stream.cc +++ b/net/quic/reliable_quic_stream.cc @@ -177,11 +177,11 @@ int ReliableQuicStream::GetReadableRegions(iovec* iov, size_t iov_len) { return 1; } -bool ReliableQuicStream::IsHalfClosed() const { +bool ReliableQuicStream::IsDoneReading() const { if (!headers_decompressed_ || !decompressed_headers_.empty()) { return false; } - return sequencer_.IsHalfClosed(); + return sequencer_.IsClosed(); } bool ReliableQuicStream::HasBytesToRead() const { @@ -445,7 +445,7 @@ void ReliableQuicStream::OnDecompressorAvailable() { // Either the headers are complete, or the all data as been consumed. ProcessHeaderData(); // Unprocessed headers remain in decompressed_headers_. - if (IsHalfClosed()) { + if (IsDoneReading()) { TerminateFromPeer(true); } else if (headers_decompressed_ && decompressed_headers_.empty()) { sequencer_.FlushBufferedFrames(); diff --git a/net/quic/reliable_quic_stream.h b/net/quic/reliable_quic_stream.h index d018818..f3d38ab 100644 --- a/net/quic/reliable_quic_stream.h +++ b/net/quic/reliable_quic_stream.h @@ -96,7 +96,8 @@ class NET_EXPORT_PRIVATE ReliableQuicStream : public // been fully processed. Then they simply delegate to the sequencer. virtual size_t Readv(const struct iovec* iov, size_t iov_len); virtual int GetReadableRegions(iovec* iov, size_t iov_len); - virtual bool IsHalfClosed() const; + // Returns true when all data has been read from the peer, including the fin. + virtual bool IsDoneReading() const; virtual bool HasBytesToRead() const; // Called by the session when a decompression blocked stream diff --git a/net/quic/test_tools/quic_connection_peer.cc b/net/quic/test_tools/quic_connection_peer.cc index 508d5da..e034223 100644 --- a/net/quic/test_tools/quic_connection_peer.cc +++ b/net/quic/test_tools/quic_connection_peer.cc @@ -10,6 +10,7 @@ #include "net/quic/congestion_control/send_algorithm_interface.h" #include "net/quic/quic_connection.h" #include "net/quic/quic_packet_writer.h" +#include "net/quic/quic_received_packet_manager.h" #include "net/quic/test_tools/quic_framer_peer.h" #include "net/quic/test_tools/quic_test_writer.h" @@ -25,7 +26,8 @@ void QuicConnectionPeer::SendAck(QuicConnection* connection) { void QuicConnectionPeer::SetReceiveAlgorithm( QuicConnection* connection, ReceiveAlgorithmInterface* receive_algorithm) { - connection->congestion_manager_.receive_algorithm_.reset(receive_algorithm); + connection->received_packet_manager_.receive_algorithm_.reset( + receive_algorithm); } // static @@ -52,8 +54,10 @@ QuicPacketCreator* QuicConnectionPeer::GetPacketCreator( return &connection->packet_creator_; } -bool QuicConnectionPeer::GetReceivedTruncatedAck(QuicConnection* connection) { - return connection->received_truncated_ack_; +// static +QuicReceivedPacketManager* QuicConnectionPeer::GetReceivedPacketManager( + QuicConnection* connection) { + return &connection->received_packet_manager_; } // static diff --git a/net/quic/test_tools/quic_connection_peer.h b/net/quic/test_tools/quic_connection_peer.h index 5dfe69e..35db76d 100644 --- a/net/quic/test_tools/quic_connection_peer.h +++ b/net/quic/test_tools/quic_connection_peer.h @@ -23,6 +23,7 @@ class QuicFecGroup; class QuicFramer; class QuicPacketCreator; class QuicPacketWriter; +class QuicReceivedPacketManager; class ReceiveAlgorithmInterface; class SendAlgorithmInterface; @@ -48,7 +49,8 @@ class QuicConnectionPeer { static QuicPacketCreator* GetPacketCreator(QuicConnection* connection); - static bool GetReceivedTruncatedAck(QuicConnection* connection); + static QuicReceivedPacketManager* GetReceivedPacketManager( + QuicConnection* connection); static QuicTime::Delta GetNetworkTimeout(QuicConnection* connection); diff --git a/net/spdy/buffered_spdy_framer.cc b/net/spdy/buffered_spdy_framer.cc index 1535f9f..0b70063 100644 --- a/net/spdy/buffered_spdy_framer.cc +++ b/net/spdy/buffered_spdy_framer.cc @@ -251,19 +251,16 @@ SpdyFrame* BufferedSpdyFramer::CreateSynStream( SpdyPriority priority, uint8 credential_slot, SpdyControlFlags flags, - bool compressed, const SpdyHeaderBlock* headers) { return spdy_framer_.CreateSynStream(stream_id, associated_stream_id, priority, - credential_slot, flags, compressed, - headers); + credential_slot, flags, headers); } SpdyFrame* BufferedSpdyFramer::CreateSynReply( SpdyStreamId stream_id, SpdyControlFlags flags, - bool compressed, const SpdyHeaderBlock* headers) { - return spdy_framer_.CreateSynReply(stream_id, flags, compressed, headers); + return spdy_framer_.CreateSynReply(stream_id, flags, headers); } SpdyFrame* BufferedSpdyFramer::CreateRstStream( @@ -291,9 +288,8 @@ SpdyFrame* BufferedSpdyFramer::CreateGoAway( SpdyFrame* BufferedSpdyFramer::CreateHeaders( SpdyStreamId stream_id, SpdyControlFlags flags, - bool compressed, const SpdyHeaderBlock* headers) { - return spdy_framer_.CreateHeaders(stream_id, flags, compressed, headers); + return spdy_framer_.CreateHeaders(stream_id, flags, headers); } SpdyFrame* BufferedSpdyFramer::CreateWindowUpdate( diff --git a/net/spdy/buffered_spdy_framer.h b/net/spdy/buffered_spdy_framer.h index fdd026b..23ab64c 100644 --- a/net/spdy/buffered_spdy_framer.h +++ b/net/spdy/buffered_spdy_framer.h @@ -165,11 +165,9 @@ class NET_EXPORT_PRIVATE BufferedSpdyFramer SpdyPriority priority, uint8 credential_slot, SpdyControlFlags flags, - bool compressed, const SpdyHeaderBlock* headers); SpdyFrame* CreateSynReply(SpdyStreamId stream_id, SpdyControlFlags flags, - bool compressed, const SpdyHeaderBlock* headers); SpdyFrame* CreateRstStream(SpdyStreamId stream_id, SpdyRstStreamStatus status) const; @@ -180,7 +178,6 @@ class NET_EXPORT_PRIVATE BufferedSpdyFramer SpdyGoAwayStatus status) const; SpdyFrame* CreateHeaders(SpdyStreamId stream_id, SpdyControlFlags flags, - bool compressed, const SpdyHeaderBlock* headers); SpdyFrame* CreateWindowUpdate( SpdyStreamId stream_id, diff --git a/net/spdy/buffered_spdy_framer_unittest.cc b/net/spdy/buffered_spdy_framer_unittest.cc index 31a649d..9aa8573 100644 --- a/net/spdy/buffered_spdy_framer_unittest.cc +++ b/net/spdy/buffered_spdy_framer_unittest.cc @@ -220,7 +220,6 @@ TEST_P(BufferedSpdyFramerTest, ReadSynStreamHeaderBlock) { 1, // priority 0, // credential_slot CONTROL_FLAG_NONE, - true, // compress &headers)); EXPECT_TRUE(control_frame.get() != NULL); @@ -243,7 +242,6 @@ TEST_P(BufferedSpdyFramerTest, ReadSynReplyHeaderBlock) { scoped_ptr<SpdyFrame> control_frame( framer.CreateSynReply(1, // stream_id CONTROL_FLAG_NONE, - true, // compress &headers)); EXPECT_TRUE(control_frame.get() != NULL); @@ -266,7 +264,6 @@ TEST_P(BufferedSpdyFramerTest, ReadHeadersHeaderBlock) { scoped_ptr<SpdyFrame> control_frame( framer.CreateHeaders(1, // stream_id CONTROL_FLAG_NONE, - true, // compress &headers)); EXPECT_TRUE(control_frame.get() != NULL); diff --git a/net/spdy/spdy_framer.cc b/net/spdy/spdy_framer.cc index 89a8309..37e6a28 100644 --- a/net/spdy/spdy_framer.cc +++ b/net/spdy/spdy_framer.cc @@ -61,9 +61,9 @@ const size_t SpdyFramer::kControlFrameBufferSize = 18; #ifdef DEBUG_SPDY_STATE_CHANGES #define CHANGE_STATE(newstate) \ do { \ - LOG(INFO) << "Changing state from: " \ - << StateToString(state_) \ - << " to " << StateToString(newstate) << "\n"; \ + DVLOG(1) << "Changing state from: " \ + << StateToString(state_) \ + << " to " << StateToString(newstate) << "\n"; \ DCHECK(state_ != SPDY_ERROR); \ DCHECK_EQ(previous_state_, state_); \ previous_state_ = state_; \ @@ -652,8 +652,8 @@ size_t SpdyFramer::ProcessCommonHeader(const char* data, size_t len) { } else if (version != spdy_version_) { // We check version before we check validity: version can never be // 'invalid', it can only be unsupported. - DLOG(INFO) << "Unsupported SPDY version " << version - << " (expected " << spdy_version_ << ")"; + DVLOG(1) << "Unsupported SPDY version " << version + << " (expected " << spdy_version_ << ")"; set_error(SPDY_UNSUPPORTED_VERSION); } else { ProcessControlFrameHeader(control_frame_type_field); @@ -675,7 +675,7 @@ void SpdyFramer::ProcessControlFrameHeader(uint16 control_frame_type_field) { current_frame_type_ = static_cast<SpdyFrameType>(control_frame_type_field); if (current_frame_type_ == NOOP) { - DLOG(INFO) << "NOOP control frame found. Ignoring."; + DVLOG(1) << "NOOP control frame found. Ignoring."; CHANGE_STATE(SPDY_AUTO_RESET); return; } @@ -1517,13 +1517,13 @@ size_t SpdyFramer::ParseHeaderBlockInBuffer(const char* header_data, if (spdy_version_ < 3) { uint16 temp; if (!reader.ReadUInt16(&temp)) { - DLOG(INFO) << "Unable to read number of headers."; + DVLOG(1) << "Unable to read number of headers."; return 0; } num_headers = temp; } else { if (!reader.ReadUInt32(&num_headers)) { - DLOG(INFO) << "Unable to read number of headers."; + DVLOG(1) << "Unable to read number of headers."; return 0; } } @@ -1535,8 +1535,8 @@ size_t SpdyFramer::ParseHeaderBlockInBuffer(const char* header_data, // Read header name. if ((spdy_version_ < 3) ? !reader.ReadStringPiece16(&temp) : !reader.ReadStringPiece32(&temp)) { - DLOG(INFO) << "Unable to read header name (" << index + 1 << " of " - << num_headers << ")."; + DVLOG(1) << "Unable to read header name (" << index + 1 << " of " + << num_headers << ")."; return 0; } std::string name = temp.as_string(); @@ -1544,16 +1544,16 @@ size_t SpdyFramer::ParseHeaderBlockInBuffer(const char* header_data, // Read header value. if ((spdy_version_ < 3) ? !reader.ReadStringPiece16(&temp) : !reader.ReadStringPiece32(&temp)) { - DLOG(INFO) << "Unable to read header value (" << index + 1 << " of " - << num_headers << ")."; + DVLOG(1) << "Unable to read header value (" << index + 1 << " of " + << num_headers << ")."; return 0; } std::string value = temp.as_string(); // Ensure no duplicates. if (block->find(name) != block->end()) { - DLOG(INFO) << "Duplicate header '" << name << "' (" << index + 1 << " of " - << num_headers << ")."; + DVLOG(1) << "Duplicate header '" << name << "' (" << index + 1 << " of " + << num_headers << ")."; return 0; } @@ -1639,10 +1639,8 @@ SpdyFrame* SpdyFramer::CreateSynStream( SpdyPriority priority, uint8 credential_slot, SpdyControlFlags flags, - bool compressed, const SpdyHeaderBlock* headers) { DCHECK_EQ(0, flags & ~CONTROL_FLAG_FIN & ~CONTROL_FLAG_UNIDIRECTIONAL); - DCHECK_EQ(enable_compression_, compressed); SpdySynStreamIR syn_stream(stream_id); syn_stream.set_associated_to_stream_id(associated_stream_id); @@ -1706,10 +1704,8 @@ SpdySerializedFrame* SpdyFramer::SerializeSynStream( SpdyFrame* SpdyFramer::CreateSynReply( SpdyStreamId stream_id, SpdyControlFlags flags, - bool compressed, const SpdyHeaderBlock* headers) { DCHECK_EQ(0, flags & ~CONTROL_FLAG_FIN); - DCHECK_EQ(enable_compression_, compressed); SpdySynReplyIR syn_reply(stream_id); syn_reply.set_fin(flags & CONTROL_FLAG_FIN); @@ -1884,11 +1880,9 @@ SpdySerializedFrame* SpdyFramer::SerializeGoAway( SpdyFrame* SpdyFramer::CreateHeaders( SpdyStreamId stream_id, SpdyControlFlags flags, - bool compressed, const SpdyHeaderBlock* header_block) { // Basically the same as CreateSynReply(). DCHECK_EQ(0, flags & (!CONTROL_FLAG_FIN)); - DCHECK_EQ(enable_compression_, compressed); SpdyHeadersIR headers(stream_id); headers.set_fin(flags & CONTROL_FLAG_FIN); diff --git a/net/spdy/spdy_framer.h b/net/spdy/spdy_framer.h index 9ae1c89..0447af0 100644 --- a/net/spdy/spdy_framer.h +++ b/net/spdy/spdy_framer.h @@ -380,7 +380,6 @@ class NET_EXPORT_PRIVATE SpdyFramer { SpdyPriority priority, uint8 credential_slot, SpdyControlFlags flags, - bool compressed, const SpdyHeaderBlock* headers); SpdySerializedFrame* SerializeSynStream(const SpdySynStreamIR& syn_stream); @@ -392,7 +391,6 @@ class NET_EXPORT_PRIVATE SpdyFramer { // |headers| is the header block to include in the frame. SpdyFrame* CreateSynReply(SpdyStreamId stream_id, SpdyControlFlags flags, - bool compressed, const SpdyHeaderBlock* headers); SpdySerializedFrame* SerializeSynReply(const SpdySynReplyIR& syn_reply); @@ -424,7 +422,6 @@ class NET_EXPORT_PRIVATE SpdyFramer { // arguments are the same as for CreateSynReply. SpdyFrame* CreateHeaders(SpdyStreamId stream_id, SpdyControlFlags flags, - bool compressed, const SpdyHeaderBlock* headers); SpdySerializedFrame* SerializeHeaders(const SpdyHeadersIR& headers); diff --git a/net/spdy/spdy_framer_test.cc b/net/spdy/spdy_framer_test.cc index d731b3a..f9c2503 100644 --- a/net/spdy/spdy_framer_test.cc +++ b/net/spdy/spdy_framer_test.cc @@ -173,7 +173,6 @@ class SpdyFramerTestUtil { priority, slot, static_cast<SpdyControlFlags>(flags), - false, &null_headers)); ResetBuffer(); memcpy(buffer_.get(), frame->data(), framer.GetSynStreamMinimumSize()); @@ -191,7 +190,6 @@ class SpdyFramerTestUtil { scoped_ptr<SpdyFrame> frame( framer.CreateHeaders(stream_id, static_cast<SpdyControlFlags>(flags), - false, &null_headers)); ResetBuffer(); memcpy(buffer_.get(), frame->data(), framer.GetHeadersMinimumSize()); @@ -226,7 +224,6 @@ class SpdyFramerTestUtil { scoped_ptr<SpdyFrame> frame( framer.CreateHeaders(stream_id, static_cast<SpdyControlFlags>(flags), - false, &null_headers)); ResetBuffer(); memcpy(buffer_.get(), frame->data(), framer.GetHeadersMinimumSize()); @@ -674,7 +671,6 @@ TEST_P(SpdyFramerTest, HeaderBlockInBuffer) { 1, // priority 0, // credential slot CONTROL_FLAG_NONE, - false, // compress &headers)); EXPECT_TRUE(frame.get() != NULL); base::StringPiece serialized_headers = @@ -704,7 +700,6 @@ TEST_P(SpdyFramerTest, UndersizedHeaderBlockInBuffer) { 1, // priority 0, // credential slot CONTROL_FLAG_NONE, - false, // compress &headers)); EXPECT_TRUE(frame.get() != NULL); @@ -772,7 +767,6 @@ TEST_P(SpdyFramerTest, SynStreamWithStreamIdZero) { 1, // priority 0, // credential slot CONTROL_FLAG_NONE, - true, // compress &headers)); ASSERT_TRUE(frame.get() != NULL); @@ -796,7 +790,6 @@ TEST_P(SpdyFramerTest, SynReplyWithStreamIdZero) { scoped_ptr<SpdySerializedFrame> frame( framer.CreateSynReply(0, // stream id CONTROL_FLAG_NONE, - true, // compress &headers)); ASSERT_TRUE(frame.get() != NULL); @@ -820,7 +813,6 @@ TEST_P(SpdyFramerTest, HeadersWithStreamIdZero) { scoped_ptr<SpdySerializedFrame> frame( framer.CreateHeaders(0, // stream id CONTROL_FLAG_NONE, - true, // compress &headers)); ASSERT_TRUE(frame.get() != NULL); @@ -1103,7 +1095,6 @@ TEST_P(SpdyFramerTest, BasicCompression) { 1, // priority 0, // credential slot CONTROL_FLAG_NONE, - true, // compress &headers)); size_t uncompressed_size1 = visitor->last_payload_len_; size_t compressed_size1 = @@ -1129,7 +1120,6 @@ TEST_P(SpdyFramerTest, BasicCompression) { 1, // priority 0, // credential slot CONTROL_FLAG_NONE, - true, // compress &headers)); size_t uncompressed_size2 = visitor->last_payload_len_; size_t compressed_size2 = @@ -1183,7 +1173,6 @@ TEST_P(SpdyFramerTest, BasicCompression) { 1, // priority 0, // credential slot CONTROL_FLAG_NONE, - false, // compress &headers)); CompareFrames("Uncompressed SYN_STREAM", *frame3, *uncompressed_frame); } @@ -1207,7 +1196,6 @@ TEST_P(SpdyFramerTest, CompressEmptyHeaders) { 1, // priority 0, // credential slot CONTROL_FLAG_NONE, - true, // compress &headers)); } @@ -1610,7 +1598,6 @@ TEST_P(SpdyFramerTest, HeaderCompression) { 0, // priority 0, // credential slot flags, - true, // compress &block)); EXPECT_TRUE(syn_frame_2.get() != NULL); @@ -1668,7 +1655,6 @@ TEST_P(SpdyFramerTest, UnclosedStreamDataCompressors) { 0, // priority 0, // credential slot flags, - true, // compress &block)); EXPECT_TRUE(syn_frame.get() != NULL); @@ -1721,7 +1707,6 @@ TEST_P(SpdyFramerTest, UnclosedStreamDataCompressorsOneByteAtATime) { 0, // priority 0, // credential slot flags, - true, // compress &block)); EXPECT_TRUE(syn_frame.get() != NULL); @@ -1999,7 +1984,6 @@ TEST_P(SpdyFramerTest, CreateSynStreamUncompressed) { framer.GetLowestPriority(), kCre, // credential slot CONTROL_FLAG_NONE, - false, // compress &headers)); if (IsSpdy2()) { CompareFrame(kDescription, *frame, kV2FrameData, arraysize(kV2FrameData)); @@ -2064,7 +2048,6 @@ TEST_P(SpdyFramerTest, CreateSynStreamUncompressed) { framer.GetHighestPriority(), 0, // credential slot CONTROL_FLAG_FIN, - false, // compress &headers)); if (IsSpdy2()) { CompareFrame(kDescription, *frame, kV2FrameData, arraysize(kV2FrameData)); @@ -2130,7 +2113,6 @@ TEST_P(SpdyFramerTest, CreateSynStreamUncompressed) { 1, // priority 0, // credential slot CONTROL_FLAG_FIN, - false, // compress &headers)); if (IsSpdy2()) { CompareFrame(kDescription, *frame, kV2FrameData, arraysize(kV2FrameData)); @@ -2217,7 +2199,6 @@ TEST_P(SpdyFramerTest, CreateSynStreamCompressed) { priority, 0, // credential slot CONTROL_FLAG_NONE, - true, // compress &headers)); if (IsSpdy2()) { CompareFrame(kDescription, *frame, kV2FrameData, arraysize(kV2FrameData)); @@ -2278,7 +2259,7 @@ TEST_P(SpdyFramerTest, CreateSynReplyUncompressed) { 0x03, 'b', 'a', 'r' }; scoped_ptr<SpdyFrame> frame(framer.CreateSynReply( - 1, CONTROL_FLAG_NONE, false, &headers)); + 1, CONTROL_FLAG_NONE, &headers)); if (IsSpdy2()) { CompareFrame(kDescription, *frame, kV2FrameData, arraysize(kV2FrameData)); } else if (IsSpdy3()) { @@ -2333,7 +2314,7 @@ TEST_P(SpdyFramerTest, CreateSynReplyUncompressed) { 'r' }; scoped_ptr<SpdyFrame> frame(framer.CreateSynReply( - 0x7fffffff, CONTROL_FLAG_FIN, false, &headers)); + 0x7fffffff, CONTROL_FLAG_FIN, &headers)); if (IsSpdy2()) { CompareFrame(kDescription, *frame, kV2FrameData, arraysize(kV2FrameData)); } else if (IsSpdy3()) { @@ -2388,7 +2369,7 @@ TEST_P(SpdyFramerTest, CreateSynReplyUncompressed) { 0x00 }; scoped_ptr<SpdyFrame> frame(framer.CreateSynReply( - 0x7fffffff, CONTROL_FLAG_FIN, false, &headers)); + 0x7fffffff, CONTROL_FLAG_FIN, &headers)); if (IsSpdy2()) { CompareFrame(kDescription, *frame, kV2FrameData, arraysize(kV2FrameData)); } else if (IsSpdy3()) { @@ -2464,7 +2445,7 @@ TEST_P(SpdyFramerTest, CreateSynReplyCompressed) { 0xff, }; scoped_ptr<SpdyFrame> frame(framer.CreateSynReply( - 1, CONTROL_FLAG_NONE, true, &headers)); + 1, CONTROL_FLAG_NONE, &headers)); if (IsSpdy2()) { CompareFrame(kDescription, *frame, kV2FrameData, arraysize(kV2FrameData)); } else if (IsSpdy3()) { @@ -2800,7 +2781,7 @@ TEST_P(SpdyFramerTest, CreateHeadersUncompressed) { 0x03, 'b', 'a', 'r' }; scoped_ptr<SpdyFrame> frame(framer.CreateHeaders( - 1, CONTROL_FLAG_NONE, false, &headers)); + 1, CONTROL_FLAG_NONE, &headers)); if (IsSpdy2()) { CompareFrame(kDescription, *frame, kV2FrameData, arraysize(kV2FrameData)); } else if (IsSpdy3()) { @@ -2855,7 +2836,7 @@ TEST_P(SpdyFramerTest, CreateHeadersUncompressed) { 'r' }; scoped_ptr<SpdyFrame> frame(framer.CreateHeaders( - 0x7fffffff, CONTROL_FLAG_FIN, false, &headers)); + 0x7fffffff, CONTROL_FLAG_FIN, &headers)); if (IsSpdy2()) { CompareFrame(kDescription, *frame, kV2FrameData, arraysize(kV2FrameData)); } else if (IsSpdy3()) { @@ -2910,7 +2891,7 @@ TEST_P(SpdyFramerTest, CreateHeadersUncompressed) { 0x00 }; scoped_ptr<SpdyFrame> frame(framer.CreateHeaders( - 0x7fffffff, CONTROL_FLAG_FIN, false, &headers)); + 0x7fffffff, CONTROL_FLAG_FIN, &headers)); if (IsSpdy2()) { CompareFrame(kDescription, *frame, kV2FrameData, arraysize(kV2FrameData)); } else if (IsSpdy3()) { @@ -2986,7 +2967,7 @@ TEST_P(SpdyFramerTest, CreateHeadersCompressed) { 0xff }; scoped_ptr<SpdyFrame> frame(framer.CreateHeaders( - 1, CONTROL_FLAG_NONE, true, &headers)); + 1, CONTROL_FLAG_NONE, &headers)); if (IsSpdy2()) { CompareFrame(kDescription, *frame, kV2FrameData, arraysize(kV2FrameData)); } else if (IsSpdy3()) { @@ -3163,7 +3144,6 @@ TEST_P(SpdyFramerTest, ReadCompressedSynStreamHeaderBlock) { 1, // priority 0, // credential_slot CONTROL_FLAG_NONE, - true, // compress &headers)); EXPECT_TRUE(control_frame.get() != NULL); TestSpdyVisitor visitor(spdy_version_); @@ -3183,7 +3163,6 @@ TEST_P(SpdyFramerTest, ReadCompressedSynReplyHeaderBlock) { scoped_ptr<SpdyFrame> control_frame( framer.CreateSynReply(1, // stream_id CONTROL_FLAG_NONE, - true, // compress &headers)); EXPECT_TRUE(control_frame.get() != NULL); TestSpdyVisitor visitor(spdy_version_); @@ -3203,7 +3182,6 @@ TEST_P(SpdyFramerTest, ReadCompressedHeadersHeaderBlock) { scoped_ptr<SpdyFrame> control_frame( framer.CreateHeaders(1, // stream_id CONTROL_FLAG_NONE, - true, // compress &headers)); EXPECT_TRUE(control_frame.get() != NULL); TestSpdyVisitor visitor(spdy_version_); @@ -3230,7 +3208,6 @@ TEST_P(SpdyFramerTest, ReadCompressedHeadersHeaderBlockWithHalfClose) { scoped_ptr<SpdyFrame> control_frame( framer.CreateHeaders(1, // stream_id CONTROL_FLAG_FIN, - true, // compress &headers)); EXPECT_TRUE(control_frame.get() != NULL); TestSpdyVisitor visitor(spdy_version_); @@ -3262,7 +3239,6 @@ TEST_P(SpdyFramerTest, ControlFrameAtMaxSizeLimit) { 1, // priority 0, // credential_slot CONTROL_FLAG_NONE, - false, // compress &headers)); const size_t kBigValueSize = framer.GetControlFrameBufferMaxSize() - control_frame->size(); @@ -3276,7 +3252,6 @@ TEST_P(SpdyFramerTest, ControlFrameAtMaxSizeLimit) { 1, // priority 0, // credential_slot CONTROL_FLAG_NONE, - false, // compress &headers)); EXPECT_TRUE(control_frame.get() != NULL); EXPECT_EQ(framer.GetControlFrameBufferMaxSize(), control_frame->size()); @@ -3306,7 +3281,6 @@ TEST_P(SpdyFramerTest, ControlFrameTooLarge) { 1, // priority 0, // credential_slot CONTROL_FLAG_NONE, - false, // compress &headers)); const size_t kBigValueSize = framer.GetControlFrameBufferMaxSize() - control_frame->size() + 1; @@ -3320,7 +3294,6 @@ TEST_P(SpdyFramerTest, ControlFrameTooLarge) { 1, // priority 0, // credential_slot CONTROL_FLAG_NONE, - false, // compress &headers)); EXPECT_TRUE(control_frame.get() != NULL); EXPECT_EQ(framer.GetControlFrameBufferMaxSize() + 1, @@ -3357,7 +3330,6 @@ TEST_P(SpdyFramerTest, ControlFrameMuchTooLarge) { 1, // priority 0, // credential_slot CONTROL_FLAG_FIN, // half close - true, // compress &headers)); EXPECT_TRUE(control_frame.get() != NULL); TestSpdyVisitor visitor(spdy_version_); @@ -3401,7 +3373,6 @@ TEST_P(SpdyFramerTest, DecompressCorruptHeaderBlock) { 1, // priority 0, // credential_slot CONTROL_FLAG_NONE, - false, // compress &headers)); TestSpdyVisitor visitor(spdy_version_); visitor.use_compression_ = true; @@ -4073,7 +4044,7 @@ TEST_P(SpdyFramerTest, SynStreamFrameFlags) { SpdyHeaderBlock headers; headers["foo"] = "bar"; scoped_ptr<SpdyFrame> frame( - framer.CreateSynStream(8, 3, 1, 0, CONTROL_FLAG_NONE, true, &headers)); + framer.CreateSynStream(8, 3, 1, 0, CONTROL_FLAG_NONE, &headers)); SetFrameFlags(frame.get(), flags, spdy_version_); if (flags & ~(CONTROL_FLAG_FIN | CONTROL_FLAG_UNIDIRECTIONAL)) { @@ -4114,7 +4085,7 @@ TEST_P(SpdyFramerTest, SynReplyFrameFlags) { SpdyHeaderBlock headers; headers["foo"] = "bar"; scoped_ptr<SpdyFrame> frame( - framer.CreateSynReply(37, CONTROL_FLAG_NONE, true, &headers)); + framer.CreateSynReply(37, CONTROL_FLAG_NONE, &headers)); SetFrameFlags(frame.get(), flags, spdy_version_); if (flags & ~CONTROL_FLAG_FIN) { @@ -4252,7 +4223,7 @@ TEST_P(SpdyFramerTest, HeadersFrameFlags) { SpdyHeaderBlock headers; headers["foo"] = "bar"; scoped_ptr<SpdyFrame> frame( - framer.CreateHeaders(57, CONTROL_FLAG_NONE, true, &headers)); + framer.CreateHeaders(57, CONTROL_FLAG_NONE, &headers)); SetFrameFlags(frame.get(), flags, spdy_version_); if (flags & ~CONTROL_FLAG_FIN) { @@ -4431,8 +4402,7 @@ TEST_P(SpdyFramerTest, EmptySynStream) { EXPECT_CALL(debug_visitor, OnSendCompressedFrame(1, SYN_STREAM, _, _)); scoped_ptr<SpdyFrame> - frame(framer.CreateSynStream(1, 0, 1, 0, CONTROL_FLAG_NONE, true, - &headers)); + frame(framer.CreateSynStream(1, 0, 1, 0, CONTROL_FLAG_NONE, &headers)); // Adjust size to remove the name/value block. if (IsSpdy4()) { SetFrameLength( diff --git a/net/spdy/spdy_session.cc b/net/spdy/spdy_session.cc index e7f30fc..4d85567 100644 --- a/net/spdy/spdy_session.cc +++ b/net/spdy/spdy_session.cc @@ -813,7 +813,7 @@ scoped_ptr<SpdyFrame> SpdySession::CreateSynStream( scoped_ptr<SpdyFrame> syn_frame( buffered_spdy_framer_->CreateSynStream( stream_id, 0, spdy_priority, - credential_slot, flags, enable_compression_, &headers)); + credential_slot, flags, &headers)); base::StatsCounter spdy_requests("spdy.requests"); spdy_requests.Increment(); diff --git a/net/spdy/spdy_test_util_common.cc b/net/spdy/spdy_test_util_common.cc index 18e39bd..0050325 100644 --- a/net/spdy/spdy_test_util_common.cc +++ b/net/spdy/spdy_test_util_common.cc @@ -752,19 +752,19 @@ SpdyFrame* SpdyTestUtil::ConstructSpdyFrame( header_info.priority, credential_slot, header_info.control_flags, - header_info.compressed, headers.get()); + headers.get()); } break; case SYN_REPLY: frame = framer.CreateSynReply(header_info.id, header_info.control_flags, - header_info.compressed, headers.get()); + headers.get()); break; case RST_STREAM: frame = framer.CreateRstStream(header_info.id, header_info.status); break; case HEADERS: frame = framer.CreateHeaders(header_info.id, header_info.control_flags, - header_info.compressed, headers.get()); + headers.get()); break; default: ADD_FAILURE(); diff --git a/net/tools/flip_server/spdy_interface.cc b/net/tools/flip_server/spdy_interface.cc index a842437..e533c48 100644 --- a/net/tools/flip_server/spdy_interface.cc +++ b/net/tools/flip_server/spdy_interface.cc @@ -428,7 +428,7 @@ size_t SpdySM::SendSynStreamImpl(uint32 stream_id, CopyHeaders(block, headers); SpdyFrame* fsrcf = buffered_spdy_framer_->CreateSynStream( - stream_id, 0, 0, 0, CONTROL_FLAG_NONE, true, &block); + stream_id, 0, 0, 0, CONTROL_FLAG_NONE, &block); size_t df_size = fsrcf->size(); EnqueueDataFrame(new SpdyFrameDataFrame(fsrcf)); @@ -445,7 +445,7 @@ size_t SpdySM::SendSynReplyImpl(uint32 stream_id, const BalsaHeaders& headers) { block["version"] = headers.response_version().as_string(); SpdyFrame* fsrcf = buffered_spdy_framer_->CreateSynReply( - stream_id, CONTROL_FLAG_NONE, true, &block); + stream_id, CONTROL_FLAG_NONE, &block); size_t df_size = fsrcf->size(); EnqueueDataFrame(new SpdyFrameDataFrame(fsrcf)); |