diff options
Diffstat (limited to 'net/quic')
23 files changed, 218 insertions, 383 deletions
diff --git a/net/quic/crypto/aes_128_gcm_12_decrypter.h b/net/quic/crypto/aes_128_gcm_12_decrypter.h index aba610f..5dc12c8 100644 --- a/net/quic/crypto/aes_128_gcm_12_decrypter.h +++ b/net/quic/crypto/aes_128_gcm_12_decrypter.h @@ -60,8 +60,6 @@ class NET_EXPORT_PRIVATE Aes128Gcm12Decrypter : public QuicDecrypter { unsigned char nonce_prefix_[4]; #if defined(USE_OPENSSL) - // TODO(rtenneti): when Chromium's version of OpenSSL has EVP_AEAD_CTX, merge - // internal CL 53267501. ScopedEVPCipherCtx ctx_; #endif }; diff --git a/net/quic/crypto/aes_128_gcm_12_encrypter.h b/net/quic/crypto/aes_128_gcm_12_encrypter.h index abea8ac2..ca9a2b1 100644 --- a/net/quic/crypto/aes_128_gcm_12_encrypter.h +++ b/net/quic/crypto/aes_128_gcm_12_encrypter.h @@ -63,8 +63,6 @@ class NET_EXPORT_PRIVATE Aes128Gcm12Encrypter : public QuicEncrypter { unsigned char nonce_prefix_[4]; #if defined(USE_OPENSSL) - // TODO(rtenneti): when Chromium's version of OpenSSL has EVP_AEAD_CTX, merge - // internal CL 53267501. ScopedEVPCipherCtx ctx_; #endif }; diff --git a/net/quic/crypto/crypto_handshake.cc b/net/quic/crypto/crypto_handshake.cc index f5c7f4d..395f4ae 100644 --- a/net/quic/crypto/crypto_handshake.cc +++ b/net/quic/crypto/crypto_handshake.cc @@ -752,7 +752,7 @@ QuicErrorCode QuicCryptoClientConfig::FillClientHello( if (!channel_id_signer_->Sign(server_hostname, hkdf_input, &key, &signature)) { *error_details = "Channel ID signature failed"; - return QUIC_INVALID_CHANNEL_ID_SIGNATURE; + return QUIC_INTERNAL_ERROR; } cetv.SetStringPiece(kCIDK, key); diff --git a/net/quic/crypto/crypto_server_config.cc b/net/quic/crypto/crypto_server_config.cc index 7c7d0ff..89cea42 100644 --- a/net/quic/crypto/crypto_server_config.cc +++ b/net/quic/crypto/crypto_server_config.cc @@ -46,8 +46,7 @@ const char QuicCryptoServerConfig::TESTING[] = "secret string for testing"; QuicCryptoServerConfig::ConfigOptions::ConfigOptions() : expiry_time(QuicWallTime::Zero()), - channel_id_enabled(false), - p256(false) {} + channel_id_enabled(false) { } QuicCryptoServerConfig::QuicCryptoServerConfig( StringPiece source_address_token_secret, @@ -98,14 +97,9 @@ QuicServerConfigProtobuf* QuicCryptoServerConfig::DefaultConfig( Curve25519KeyExchange::New(curve25519_private_key)); StringPiece curve25519_public_value = curve25519->public_value(); - string p256_private_key; - StringPiece p256_public_value; - scoped_ptr<P256KeyExchange> p256; - if (options.p256) { - p256_private_key = P256KeyExchange::NewPrivateKey(); - p256.reset(P256KeyExchange::New(p256_private_key)); - p256_public_value = p256->public_value(); - } + const string p256_private_key = P256KeyExchange::NewPrivateKey(); + scoped_ptr<P256KeyExchange> p256(P256KeyExchange::New(p256_private_key)); + StringPiece p256_public_value = p256->public_value(); string encoded_public_values; // First three bytes encode the length of the public value. @@ -121,11 +115,7 @@ QuicServerConfigProtobuf* QuicCryptoServerConfig::DefaultConfig( p256_public_value.size()); msg.set_tag(kSCFG); - if (options.p256) { - msg.SetTaglist(kKEXS, kC255, kP256, 0); - } else { - msg.SetTaglist(kKEXS, kC255, 0); - } + msg.SetTaglist(kKEXS, kC255, kP256, 0); msg.SetTaglist(kAEAD, kAESG, 0); msg.SetValue(kVERS, static_cast<uint16>(0)); msg.SetStringPiece(kPUBS, encoded_public_values); @@ -168,12 +158,9 @@ QuicServerConfigProtobuf* QuicCryptoServerConfig::DefaultConfig( QuicServerConfigProtobuf::PrivateKey* curve25519_key = config->add_key(); curve25519_key->set_tag(kC255); curve25519_key->set_private_key(curve25519_private_key); - - if (options.p256) { - QuicServerConfigProtobuf::PrivateKey* p256_key = config->add_key(); - p256_key->set_tag(kP256); - p256_key->set_private_key(p256_private_key); - } + QuicServerConfigProtobuf::PrivateKey* p256_key = config->add_key(); + p256_key->set_tag(kP256); + p256_key->set_private_key(p256_private_key); return config.release(); } diff --git a/net/quic/crypto/crypto_server_config.h b/net/quic/crypto/crypto_server_config.h index 4551425..4255d22 100644 --- a/net/quic/crypto/crypto_server_config.h +++ b/net/quic/crypto/crypto_server_config.h @@ -61,11 +61,6 @@ class NET_EXPORT_PRIVATE QuicCryptoServerConfig { // orbit contains the kOrbitSize bytes of the orbit value for the server // config. If |orbit| is empty then a random orbit is generated. std::string orbit; - // p256 determines whether a P-256 public key will be included in the - // server config. Note that this breaks deterministic server-config - // generation since P-256 key generation doesn't use the QuicRandom given - // to DefaultConfig(). - bool p256; }; // |source_address_token_secret|: secret key material used for encrypting and diff --git a/net/quic/crypto/crypto_server_test.cc b/net/quic/crypto/crypto_server_test.cc index 348b31a..b2cdf82 100644 --- a/net/quic/crypto/crypto_server_test.cc +++ b/net/quic/crypto/crypto_server_test.cc @@ -8,7 +8,6 @@ #include "net/quic/crypto/quic_random.h" #include "net/quic/test_tools/crypto_test_utils.h" #include "net/quic/test_tools/mock_clock.h" -#include "net/quic/test_tools/mock_random.h" #include "testing/gtest/include/gtest/gtest.h" using base::StringPiece; @@ -240,24 +239,6 @@ TEST_F(CryptoServerTest, ReplayProtection) { ASSERT_EQ(kSHLO, out_.tag()); } -TEST(CryptoServerConfigGenerationTest, Determinism) { - // Test that using a deterministic PRNG causes the server-config to be - // deterministic. - - MockRandom rand_a, rand_b; - const QuicCryptoServerConfig::ConfigOptions options; - MockClock clock; - - QuicCryptoServerConfig a(QuicCryptoServerConfig::TESTING, &rand_a); - QuicCryptoServerConfig b(QuicCryptoServerConfig::TESTING, &rand_b); - scoped_ptr<CryptoHandshakeMessage> scfg_a( - a.AddDefaultConfig(&rand_a, &clock, options)); - scoped_ptr<CryptoHandshakeMessage> scfg_b( - b.AddDefaultConfig(&rand_b, &clock, options)); - - ASSERT_EQ(scfg_a->DebugString(), scfg_b->DebugString()); -} - class CryptoServerTestNoConfig : public CryptoServerTest { public: virtual void SetUp() { diff --git a/net/quic/quic_ack_notifier_manager.cc b/net/quic/quic_ack_notifier_manager.cc index 2221e30..901d9d6 100644 --- a/net/quic/quic_ack_notifier_manager.cc +++ b/net/quic/quic_ack_notifier_manager.cc @@ -22,32 +22,40 @@ AckNotifierManager::~AckNotifierManager() { STLDeleteElements(&ack_notifiers_); } -void AckNotifierManager::OnPacketAcked( - QuicPacketSequenceNumber sequence_number) { - // Inform all the registered AckNotifiers of the new ACK. - AckNotifierMap::iterator map_it = ack_notifier_map_.find(sequence_number); - if (map_it == ack_notifier_map_.end()) { - // No AckNotifier is interested in this sequence number. - return; - } +void AckNotifierManager::OnIncomingAck(const SequenceNumberSet& acked_packets) { + // Inform all the registered AckNotifiers of the new ACKs. + for (SequenceNumberSet::const_iterator seq_it = acked_packets.begin(); + seq_it != acked_packets.end(); ++seq_it) { + AckNotifierMap::iterator map_it = ack_notifier_map_.find(*seq_it); + if (map_it == ack_notifier_map_.end()) { + // No AckNotifier is interested in this sequence number. + continue; + } - // One or more AckNotifiers are registered as interested in this sequence - // number. Iterate through them and call OnAck on each. - for (AckNotifierSet::iterator set_it = map_it->second.begin(); - set_it != map_it->second.end(); ++set_it) { - QuicAckNotifier* ack_notifier = *set_it; - ack_notifier->OnAck(sequence_number); - - // If this has resulted in an empty AckNotifer, erase it. - if (ack_notifier->IsEmpty()) { - delete ack_notifier; - ack_notifiers_.erase(ack_notifier); + // One or more AckNotifiers are registered as interested in this sequence + // number. Iterate through them and call OnAck on each. + for (AckNotifierSet::iterator set_it = map_it->second.begin(); + set_it != map_it->second.end(); ++set_it) { + (*set_it)->OnAck(*seq_it); } + + // Remove the sequence number from the map as we have notified all the + // registered AckNotifiers, and we won't see it again. + ack_notifier_map_.erase(map_it); } - // Remove the sequence number from the map as we have notified all the - // registered AckNotifiers, and we won't see it again. - ack_notifier_map_.erase(map_it); + // Clear up any empty AckNotifiers + AckNotifierSet::iterator it = ack_notifiers_.begin(); + while (it != ack_notifiers_.end()) { + if ((*it)->IsEmpty()) { + // The QuicAckNotifier has seen all the ACKs it was interested in, and + // has triggered its callback. No more use for it. + delete *it; + ack_notifiers_.erase(it++); + } else { + ++it; + } + } } void AckNotifierManager::UpdateSequenceNumber( diff --git a/net/quic/quic_ack_notifier_manager.h b/net/quic/quic_ack_notifier_manager.h index 5ddf794..7fe9a46 100644 --- a/net/quic/quic_ack_notifier_manager.h +++ b/net/quic/quic_ack_notifier_manager.h @@ -37,10 +37,11 @@ class NET_EXPORT_PRIVATE AckNotifierManager { AckNotifierManager(); virtual ~AckNotifierManager(); - // Called when the connection receives a new AckFrame. If |sequence_number| - // exists in ack_notifier_map_ then the corresponding AckNotifiers will have - // their OnAck method called. - void OnPacketAcked(QuicPacketSequenceNumber sequence_number); + // Called when the connection receives a new AckFrame. For each packet + // in |acked_packets|, if the packet sequence number exists in + // ack_notifier_map_ then the corresponding AckNotifiers will have their OnAck + // method called. + void OnIncomingAck(const SequenceNumberSet& acked_packets); // If a packet has been retransmitted with a new sequence number, then this // will be called. It updates the mapping in ack_notifier_map_, and also diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc index 2073919..59759f7 100644 --- a/net/quic/quic_connection.cc +++ b/net/quic/quic_connection.cc @@ -52,9 +52,8 @@ namespace { // This will likely have to be tuned. const QuicPacketSequenceNumber kMaxPacketGap = 5000; -// We want to make sure if we get a nack packet which triggers several -// retransmissions, we don't queue up too many packets. 10 is TCP's default -// initial congestion window(RFC 6928). +// We want to make sure if we get a large nack packet, we don't queue up too +// many packets at once. 10 is a common default initial congestion window. const size_t kMaxRetransmissionsPerAck = 10; // TCP retransmits after 2 nacks. We allow for a third in case of out-of-order @@ -216,7 +215,6 @@ QuicConnection::QuicConnection(QuicGuid guid, guid_(guid), peer_address_(address), largest_seen_packet_with_ack_(0), - pending_version_negotiation_packet_(false), write_blocked_(false), ack_alarm_(helper->CreateAlarm(new AckAlarm(this))), retransmission_alarm_(helper->CreateAlarm(new RetransmissionAlarm(this))), @@ -506,15 +504,11 @@ bool QuicConnection::OnAckFrame(const QuicAckFrame& incoming_ack) { } void QuicConnection::ProcessAckFrame(const QuicAckFrame& incoming_ack) { - // Latch current least unacked sequence number. This allows us to reset the - // retransmission and FEC abandonment timers conditionally below. - QuicPacketSequenceNumber least_unacked_sent_before = - sent_packet_manager_.GetLeastUnackedSentPacket(); - largest_seen_packet_with_ack_ = last_header_.packet_sequence_number; - received_truncated_ack_ = incoming_ack.received_info.missing_packets.size() >= - QuicFramer::GetMaxUnackedPackets(last_header_); + received_truncated_ack_ = + incoming_ack.received_info.missing_packets.size() >= + QuicFramer::GetMaxUnackedPackets(last_header_); received_packet_manager_.UpdatePacketInformationReceivedByPeer(incoming_ack); received_packet_manager_.UpdatePacketInformationSentByPeer(incoming_ack); @@ -525,42 +519,35 @@ void QuicConnection::ProcessAckFrame(const QuicAckFrame& incoming_ack) { received_packet_manager_.least_packet_awaited_by_peer() - 1); retransmitted_nacked_packet_count_ = 0; - sent_packet_manager_.OnIncomingAck(incoming_ack.received_info, - received_truncated_ack_); - - // Get the updated least unacked sequence number. - QuicPacketSequenceNumber least_unacked_sent_after = - sent_packet_manager_.GetLeastUnackedSentPacket(); - - // Used to set RTO and FEC alarms. - QuicTime::Delta retransmission_delay = - congestion_manager_.GetRetransmissionDelay( - sent_packet_manager_.GetNumUnackedPackets(), 0); - - // If there are outstanding packets, and the least unacked sequence number - // has increased after processing this latest AckFrame, then reschedule the - // retransmission timer. - if (sent_packet_manager_.HasUnackedPackets() && - least_unacked_sent_before < least_unacked_sent_after) { - DCHECK(retransmission_alarm_->IsSet()); - - retransmission_alarm_->Cancel(); - retransmission_alarm_->Set( - clock_->ApproximateNow().Add(retransmission_delay)); + SequenceNumberSet acked_packets; + sent_packet_manager_.OnIncomingAck( + incoming_ack.received_info, received_truncated_ack_, &acked_packets); + if (acked_packets.size() > 0) { + // Reset the RTO timeout for each packet when an ack is received. + if (retransmission_alarm_->IsSet()) { + retransmission_alarm_->Cancel(); + // Only reschedule the timer if there are outstanding packets. + if (sent_packet_manager_.HasUnackedPackets()) { + QuicTime::Delta retransmission_delay = + congestion_manager_.GetRetransmissionDelay( + sent_packet_manager_.GetNumUnackedPackets(), 0); + retransmission_alarm_->Set(clock_->ApproximateNow().Add( + retransmission_delay)); + } + } + if (abandon_fec_alarm_->IsSet()) { + abandon_fec_alarm_->Cancel(); + // Only reschedule the timer if there are outstanding fec packets. + if (sent_packet_manager_.HasUnackedFecPackets()) { + QuicTime::Delta retransmission_delay = + congestion_manager_.GetRetransmissionDelay( + sent_packet_manager_.GetNumUnackedPackets(), 0); + abandon_fec_alarm_->Set(clock_->ApproximateNow().Add( + retransmission_delay)); + } + } consecutive_rto_count_ = 0; - } else if (!sent_packet_manager_.HasUnackedPackets()) { - retransmission_alarm_->Cancel(); } - - // If there are outstanding FEC packets, and the least unacked sequence number - // has increased after processing this latest AckFrame, then reschedule the - // abandon FEC timer. - abandon_fec_alarm_->Cancel(); - if (sent_packet_manager_.HasUnackedFecPackets() && - least_unacked_sent_before < least_unacked_sent_after) { - abandon_fec_alarm_->Set(clock_->ApproximateNow().Add(retransmission_delay)); - } - congestion_manager_.OnIncomingAckFrame(incoming_ack, time_of_last_received_packet_); } @@ -753,6 +740,7 @@ void QuicConnection::OnPacketComplete() { MaybeSendInResponseToPacket(send_ack_immediately, last_packet_should_instigate_ack); + ClearLastFrames(); } @@ -845,25 +833,12 @@ void QuicConnection::SendVersionNegotiationPacket() { for (size_t i = 0; i < arraysize(kSupportedQuicVersions); ++i) { supported_versions.push_back(kSupportedQuicVersions[i]); } - scoped_ptr<QuicEncryptedPacket> version_packet( - packet_creator_.SerializeVersionNegotiationPacket(supported_versions)); + QuicEncryptedPacket* encrypted = + packet_creator_.SerializeVersionNegotiationPacket(supported_versions); // TODO(satyamshekhar): implement zero server state negotiation. - WriteResult result = - helper_->WritePacketToWire(*version_packet); - if (result.status == WRITE_STATUS_OK || - (result.status == WRITE_STATUS_BLOCKED && - helper_->IsWriteBlockedDataBuffered())) { - pending_version_negotiation_packet_ = false; - return; - } - if (result.status == WRITE_STATUS_ERROR) { - // We can't send an error as the socket is presumably borked. - CloseConnection(QUIC_PACKET_WRITE_ERROR, false); - } - if (result.status == WRITE_STATUS_BLOCKED) { - write_blocked_ = true; - } - pending_version_negotiation_packet_ = true; + int error; + helper_->WritePacketToWire(*encrypted, &error); + delete encrypted; } QuicConsumedData QuicConnection::SendvStreamDataInner( @@ -1074,10 +1049,6 @@ bool QuicConnection::ProcessValidatedPacket() { bool QuicConnection::WriteQueuedPackets() { DCHECK(!write_blocked_); - if (pending_version_negotiation_packet_) { - SendVersionNegotiationPacket(); - } - QueuedPacketList::iterator packet_iterator = queued_packets_.begin(); while (!write_blocked_ && packet_iterator != queued_packets_.end()) { if (WritePacket(packet_iterator->encryption_level, @@ -1232,8 +1203,9 @@ void QuicConnection::SetupRetransmission( return; } - // Do not set the retransmission alarm if we're already handling one, since - // it will be reset when OnRetransmissionTimeout completes. + // Do not set the retransmission alarm if we're already handling the + // retransmission alarm because the retransmission alarm will be reset when + // OnRetransmissionTimeout completes. if (retransmission_alarm_->IsSet()) { return; } @@ -1366,51 +1338,27 @@ bool QuicConnection::WritePacket(EncryptionLevel level, << packet->length() << " " << encrypted->length() << " " << " forced: " << (forced == FORCE ? "yes" : "no"); - DCHECK(pending_write_.get() == NULL); - pending_write_.reset(new PendingWrite(sequence_number, transmission_type, - retransmittable, level, packet)); - - WriteResult result = WritePacketToWire(sequence_number, level, *encrypted); - if (result.status == WRITE_STATUS_BLOCKED) { - // TODO(satyashekhar): It might be more efficient (fewer system calls), if - // all connections share this variable i.e this becomes a part of - // PacketWriterInterface. - write_blocked_ = true; - // If the socket buffers the the data, then the packet should not - // be queued and sent again, which would result in an unnecessary - // duplicate packet being sent. The helper must call OnPacketSent - // when the packet is actually sent. - if (helper_->IsWriteBlockedDataBuffered()) { - return true; + int error; + QuicTime now = clock_->Now(); + if (WritePacketToWire(sequence_number, level, *encrypted, &error) == -1) { + if (helper_->IsWriteBlocked(error)) { + // TODO(satyashekhar): It might be more efficient (fewer system calls), if + // all connections share this variable i.e this becomes a part of + // PacketWriterInterface. + write_blocked_ = true; + // If the socket buffers the the data, then the packet should not + // be queued and sent again, which would result in an unnecessary + // duplicate packet being sent. + if (helper_->IsWriteBlockedDataBuffered()) { + delete packet; + return true; + } + return false; } - pending_write_.reset(); - return false; - } - - return OnPacketSent(result); -} - -bool QuicConnection::OnPacketSent(WriteResult result) { - DCHECK_NE(WRITE_STATUS_BLOCKED, result.status); - if (pending_write_.get() == NULL) { - LOG(DFATAL) << "OnPacketSent called without a pending write."; - return false; - } - - QuicPacketSequenceNumber sequence_number = pending_write_->sequence_number; - TransmissionType transmission_type = pending_write_->transmission_type; - HasRetransmittableData retransmittable = pending_write_->retransmittable; - EncryptionLevel level = pending_write_->level; - QuicPacket* packet = pending_write_->packet; - pending_write_.reset(); - - if (result.status == WRITE_STATUS_ERROR) { // We can't send an error as the socket is presumably borked. CloseConnection(QUIC_PACKET_WRITE_ERROR, false); return false; } - - QuicTime now = clock_->Now(); if (transmission_type == NOT_RETRANSMISSION) { time_of_last_sent_packet_ = now; } @@ -1436,12 +1384,12 @@ bool QuicConnection::OnPacketSent(WriteResult result) { congestion_manager_.OnPacketSent(sequence_number, now, packet->length(), transmission_type, retransmittable); - stats_.bytes_sent += result.bytes_written; + stats_.bytes_sent += encrypted->length(); ++stats_.packets_sent; if (transmission_type == NACK_RETRANSMISSION || transmission_type == RTO_RETRANSMISSION) { - stats_.bytes_retransmitted += result.bytes_written; + stats_.bytes_retransmitted += encrypted->length(); ++stats_.packets_retransmitted; } @@ -1449,16 +1397,18 @@ bool QuicConnection::OnPacketSent(WriteResult result) { return true; } -WriteResult QuicConnection::WritePacketToWire( - QuicPacketSequenceNumber sequence_number, - EncryptionLevel level, - const QuicEncryptedPacket& packet) { - WriteResult result = helper_->WritePacketToWire(packet); +int QuicConnection::WritePacketToWire(QuicPacketSequenceNumber sequence_number, + EncryptionLevel level, + const QuicEncryptedPacket& packet, + int* error) { + int bytes_written = helper_->WritePacketToWire(packet, error); if (debug_visitor_) { - // Pass the write result to the visitor. - debug_visitor_->OnPacketSent(sequence_number, level, packet, result); + // WritePacketToWire returned -1, then |error| will be populated with + // an error code, which we want to pass along to the visitor. + debug_visitor_->OnPacketSent(sequence_number, level, packet, + bytes_written == -1 ? *error : bytes_written); } - return result; + return bytes_written; } bool QuicConnection::OnSerializedPacket( @@ -1556,6 +1506,8 @@ void QuicConnection::OnRetransmissionTimeout() { return; } + // TODO(ianswett): When multiple RTO's fire, the subsequent RTO should send + // the same data packet as the first RTO according to the TCP spec. // TODO(ianswett): When an RTO fires, but the connection has not been // established as forward secure, re-send the client hello first. ++stats_.rto_count; @@ -1813,8 +1765,7 @@ void QuicConnection::Flush() { } bool QuicConnection::HasQueuedData() const { - return pending_version_negotiation_packet_ || - !queued_packets_.empty() || packet_generator_.HasQueuedFrames(); + return !queued_packets_.empty() || packet_generator_.HasQueuedFrames(); } void QuicConnection::SetIdleNetworkTimeout(QuicTime::Delta timeout) { diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h index 2eb7873..2f20351 100644 --- a/net/quic/quic_connection.h +++ b/net/quic/quic_connection.h @@ -104,7 +104,7 @@ class NET_EXPORT_PRIVATE QuicConnectionDebugVisitorInterface virtual void OnPacketSent(QuicPacketSequenceNumber sequence_number, EncryptionLevel level, const QuicEncryptedPacket& packet, - WriteResult result) = 0; + int rv) = 0; // Called when the contents of a packet have been retransmitted as // a new packet. @@ -171,10 +171,10 @@ class NET_EXPORT_PRIVATE QuicConnectionHelperInterface { // Sends the packet out to the peer, possibly simulating packet // loss if FLAGS_fake_packet_loss_percentage is set. If the write - // succeeded, the result's status is WRITE_STATUS_OK and bytes_written is - // populated. If the write failed, the result's status is WRITE_STATUS_BLOCKED - // or WRITE_STATUS_ERROR and error_code will populated. - virtual WriteResult WritePacketToWire(const QuicEncryptedPacket& packet) = 0; + // succeeded, returns the number of bytes written. If the write + // failed, returns -1 and the error code will be copied to |*error|. + virtual int WritePacketToWire(const QuicEncryptedPacket& packet, + int* error) = 0; // Returns true if the helper buffers and subsequently rewrites data // when an attempt to write results in the underlying socket becoming @@ -274,9 +274,6 @@ class NET_EXPORT_PRIVATE QuicConnection // writes to happen. Returns false if the socket has become blocked. virtual bool OnCanWrite() OVERRIDE; - // Called when a packet has been finally sent to the network. - bool OnPacketSent(WriteResult result); - // If the socket is not blocked, this allows queued writes to happen. Returns // false if the socket has become blocked. bool WriteIfNotBlocked(); @@ -458,9 +455,10 @@ class NET_EXPORT_PRIVATE QuicConnection HasRetransmittableData retransmittable, Force force); - WriteResult WritePacketToWire(QuicPacketSequenceNumber sequence_number, - EncryptionLevel level, - const QuicEncryptedPacket& packet); + int WritePacketToWire(QuicPacketSequenceNumber sequence_number, + EncryptionLevel level, + const QuicEncryptedPacket& packet, + int* error); // Make sure an ack we got from our peer is sane. bool ValidateAckFrame(const QuicAckFrame& incoming_ack); @@ -575,25 +573,6 @@ class NET_EXPORT_PRIVATE QuicConnection bool for_fec; }; - struct PendingWrite { - PendingWrite(QuicPacketSequenceNumber sequence_number, - TransmissionType transmission_type, - HasRetransmittableData retransmittable, - EncryptionLevel level, - QuicPacket* packet) - : sequence_number(sequence_number), - transmission_type(transmission_type), - retransmittable(retransmittable), - level(level), - packet(packet) { } - - QuicPacketSequenceNumber sequence_number; - TransmissionType transmission_type; - HasRetransmittableData retransmittable; - EncryptionLevel level; - QuicPacket* packet; - }; - class RetransmissionTimeComparator { public: bool operator()(const RetransmissionTime& lhs, @@ -721,18 +700,11 @@ class NET_EXPORT_PRIVATE QuicConnection // sent with the INITIAL encryption and the CHLO message was lost. std::deque<QuicEncryptedPacket*> undecryptable_packets_; - // When the version negotiation packet could not be sent because the socket - // was not writable, this is set to true. - bool pending_version_negotiation_packet_; - // When packets could not be sent because the socket was not writable, // they are added to this list. All corresponding frames are in // unacked_packets_ if they are to be retransmitted. QueuedPacketList queued_packets_; - // Contains information about the current write in progress, if any. - scoped_ptr<PendingWrite> pending_write_; - // True when the socket becomes unwritable. bool write_blocked_; diff --git a/net/quic/quic_connection_helper.cc b/net/quic/quic_connection_helper.cc index 1de5013..703151b 100644 --- a/net/quic/quic_connection_helper.cc +++ b/net/quic/quic_connection_helper.cc @@ -107,11 +107,13 @@ QuicRandom* QuicConnectionHelper::GetRandomGenerator() { return random_generator_; } -WriteResult QuicConnectionHelper::WritePacketToWire( - const QuicEncryptedPacket& packet) { +int QuicConnectionHelper::WritePacketToWire( + const QuicEncryptedPacket& packet, + int* error) { if (connection_->ShouldSimulateLostPacket()) { DLOG(INFO) << "Dropping packet due to fake packet loss."; - return WriteResult(WRITE_STATUS_OK, packet.length()); + *error = 0; + return packet.length(); } scoped_refptr<StringIOBuffer> buf( @@ -121,17 +123,16 @@ WriteResult QuicConnectionHelper::WritePacketToWire( packet.length(), base::Bind(&QuicConnectionHelper::OnWriteComplete, weak_factory_.GetWeakPtr())); - WriteStatus status = WRITE_STATUS_OK; - if (rv < 0) { + if (rv >= 0) { + *error = 0; + } else { if (rv != ERR_IO_PENDING) { UMA_HISTOGRAM_SPARSE_SLOWLY("Net.QuicSession.WriteError", -rv); - status = WRITE_STATUS_ERROR; - } else { - status = WRITE_STATUS_BLOCKED; } + *error = rv; + rv = -1; } - - return WriteResult(status, rv); + return rv; } bool QuicConnectionHelper::IsWriteBlockedDataBuffered() { diff --git a/net/quic/quic_connection_helper.h b/net/quic/quic_connection_helper.h index e530176..6667b95 100644 --- a/net/quic/quic_connection_helper.h +++ b/net/quic/quic_connection_helper.h @@ -45,8 +45,8 @@ class NET_EXPORT_PRIVATE QuicConnectionHelper virtual void SetConnection(QuicConnection* connection) OVERRIDE; virtual const QuicClock* GetClock() const OVERRIDE; virtual QuicRandom* GetRandomGenerator() OVERRIDE; - virtual WriteResult WritePacketToWire( - const QuicEncryptedPacket& packet) OVERRIDE; + virtual int WritePacketToWire(const QuicEncryptedPacket& packet, + int* error) OVERRIDE; virtual bool IsWriteBlockedDataBuffered() OVERRIDE; virtual bool IsWriteBlocked(int error) OVERRIDE; virtual QuicAlarm* CreateAlarm(QuicAlarm::Delegate* delegate) OVERRIDE; diff --git a/net/quic/quic_connection_helper_test.cc b/net/quic/quic_connection_helper_test.cc index ffc6ee19..e228d12 100644 --- a/net/quic/quic_connection_helper_test.cc +++ b/net/quic/quic_connection_helper_test.cc @@ -10,7 +10,6 @@ #include "net/quic/crypto/quic_decrypter.h" #include "net/quic/crypto/quic_encrypter.h" #include "net/quic/quic_connection.h" -#include "net/quic/quic_protocol.h" #include "net/quic/test_tools/mock_clock.h" #include "net/quic/test_tools/quic_connection_peer.h" #include "net/quic/test_tools/quic_test_utils.h" @@ -410,9 +409,9 @@ TEST_F(QuicConnectionHelperTest, WritePacketToWire) { Initialize(); int len = GetWrite(0)->length(); - WriteResult result = helper_->WritePacketToWire(*GetWrite(0)); - EXPECT_EQ(len, result.bytes_written); - EXPECT_EQ(WRITE_STATUS_OK, result.status); + int error = 0; + EXPECT_EQ(len, helper_->WritePacketToWire(*GetWrite(0), &error)); + EXPECT_EQ(0, error); EXPECT_TRUE(AtEof()); } @@ -421,9 +420,9 @@ TEST_F(QuicConnectionHelperTest, WritePacketToWireAsync) { Initialize(); EXPECT_CALL(visitor_, OnCanWrite()).WillOnce(Return(true)); - WriteResult result = helper_->WritePacketToWire(*GetWrite(0)); - EXPECT_EQ(-1, result.bytes_written); - EXPECT_EQ(WRITE_STATUS_BLOCKED, result.status); + int error = 0; + EXPECT_EQ(-1, helper_->WritePacketToWire(*GetWrite(0), &error)); + EXPECT_EQ(ERR_IO_PENDING, error); base::MessageLoop::current()->RunUntilIdle(); EXPECT_TRUE(AtEof()); } diff --git a/net/quic/quic_connection_logger.cc b/net/quic/quic_connection_logger.cc index 25532e0..5195f43 100644 --- a/net/quic/quic_connection_logger.cc +++ b/net/quic/quic_connection_logger.cc @@ -33,15 +33,15 @@ base::Value* NetLogQuicPacketSentCallback( QuicPacketSequenceNumber sequence_number, EncryptionLevel level, size_t packet_size, - WriteResult result, + int rv, NetLog::LogLevel /* log_level */) { base::DictionaryValue* dict = new base::DictionaryValue(); dict->SetInteger("encryption_level", level); dict->SetString("packet_sequence_number", base::Uint64ToString(sequence_number)); dict->SetInteger("size", packet_size); - if (result.status != WRITE_STATUS_OK) { - dict->SetInteger("net_error", result.error_code); + if (rv < 0) { + dict->SetInteger("net_error", rv); } return dict; } @@ -255,11 +255,11 @@ void QuicConnectionLogger::OnPacketSent( QuicPacketSequenceNumber sequence_number, EncryptionLevel level, const QuicEncryptedPacket& packet, - WriteResult result) { + int rv) { net_log_.AddEvent( NetLog::TYPE_QUIC_SESSION_PACKET_SENT, base::Bind(&NetLogQuicPacketSentCallback, sequence_number, level, - packet.length(), result)); + packet.length(), rv)); } void QuicConnectionLogger:: OnPacketRetransmitted( diff --git a/net/quic/quic_connection_logger.h b/net/quic/quic_connection_logger.h index a7a8747..d498b12 100644 --- a/net/quic/quic_connection_logger.h +++ b/net/quic/quic_connection_logger.h @@ -6,7 +6,6 @@ #define NET_QUIC_QUIC_CONNECTION_LOGGER_H_ #include "net/quic/quic_connection.h" -#include "net/quic/quic_protocol.h" namespace net { @@ -29,7 +28,7 @@ class NET_EXPORT_PRIVATE QuicConnectionLogger virtual void OnPacketSent(QuicPacketSequenceNumber sequence_number, EncryptionLevel level, const QuicEncryptedPacket& packet, - WriteResult result) OVERRIDE; + int rv) OVERRIDE; virtual void OnPacketRetransmitted( QuicPacketSequenceNumber old_sequence_number, QuicPacketSequenceNumber new_sequence_number) OVERRIDE; diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc index f861b1b..102bb33 100644 --- a/net/quic/quic_connection_test.cc +++ b/net/quic/quic_connection_test.cc @@ -243,9 +243,7 @@ class TestConnectionHelper : public QuicConnectionHelperInterface { TestConnectionHelper(MockClock* clock, MockRandom* random_generator) : clock_(clock), random_generator_(random_generator), - last_packet_size_(0), blocked_(false), - is_write_blocked_data_buffered_(false), is_server_(true), use_tagging_decrypter_(false), packets_write_attempts_(0) { @@ -263,8 +261,8 @@ class TestConnectionHelper : public QuicConnectionHelperInterface { return random_generator_; } - virtual WriteResult WritePacketToWire( - const QuicEncryptedPacket& packet) OVERRIDE { + virtual int WritePacketToWire(const QuicEncryptedPacket& packet, + int* error) OVERRIDE { ++packets_write_attempts_; if (packet.length() >= sizeof(final_bytes_of_last_packet_)) { @@ -295,14 +293,16 @@ class TestConnectionHelper : public QuicConnectionHelperInterface { *visitor.version_negotiation_packet())); } if (blocked_) { - return WriteResult(WRITE_STATUS_BLOCKED, -1); + *error = ERR_IO_PENDING; + return -1; } + *error = 0; last_packet_size_ = packet.length(); - return WriteResult(WRITE_STATUS_OK, last_packet_size_); + return last_packet_size_; } virtual bool IsWriteBlockedDataBuffered() OVERRIDE { - return is_write_blocked_data_buffered_; + return false; } virtual bool IsWriteBlocked(int error) OVERRIDE { @@ -335,10 +335,6 @@ class TestConnectionHelper : public QuicConnectionHelperInterface { void set_blocked(bool blocked) { blocked_ = blocked; } - void set_is_write_blocked_data_buffered(bool buffered) { - is_write_blocked_data_buffered_ = buffered; - } - void set_is_server(bool is_server) { is_server_ = is_server; } // final_bytes_of_last_packet_ returns the last four bytes of the previous @@ -364,7 +360,6 @@ class TestConnectionHelper : public QuicConnectionHelperInterface { scoped_ptr<QuicVersionNegotiationPacket> version_negotiation_packet_; size_t last_packet_size_; bool blocked_; - bool is_write_blocked_data_buffered_; bool is_server_; uint32 final_bytes_of_last_packet_; bool use_tagging_decrypter_; @@ -1721,18 +1716,6 @@ TEST_P(QuicConnectionTest, ResumptionAlarmThenWriteBlocked) { EXPECT_TRUE(QuicConnectionPeer::IsWriteBlocked(&connection_)); } -TEST_P(QuicConnectionTest, WriteBlockedThenSent) { - helper_->set_blocked(true); - - helper_->set_is_write_blocked_data_buffered(true); - connection_.SendStreamData(1, "foo", 0, !kFin); - EXPECT_FALSE(connection_.GetRetransmissionAlarm()->IsSet()); - - EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, _, _, _)).Times(1); - connection_.OnPacketSent(WriteResult(WRITE_STATUS_OK, 0)); - EXPECT_TRUE(connection_.GetRetransmissionAlarm()->IsSet()); -} - TEST_P(QuicConnectionTest, LimitPacketsPerNack) { EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); EXPECT_CALL(*send_algorithm_, OnIncomingAck(12, _, _)).Times(1); @@ -2746,49 +2729,6 @@ TEST_P(QuicConnectionTest, ServerSendsVersionNegotiationPacket) { } } -TEST_P(QuicConnectionTest, ServerSendsVersionNegotiationPacketSocketBlocked) { - framer_.set_version_for_tests(QUIC_VERSION_UNSUPPORTED); - - QuicPacketHeader header; - header.public_header.guid = guid_; - header.public_header.reset_flag = false; - header.public_header.version_flag = true; - header.entropy_flag = false; - header.fec_flag = false; - header.packet_sequence_number = 12; - header.fec_group = 0; - - QuicFrames frames; - QuicFrame frame(&frame1_); - frames.push_back(frame); - scoped_ptr<QuicPacket> packet( - framer_.BuildUnsizedDataPacket(header, frames).packet); - scoped_ptr<QuicEncryptedPacket> encrypted( - framer_.EncryptPacket(ENCRYPTION_NONE, 12, *packet)); - - framer_.set_version(QuicVersionMax()); - connection_.set_is_server(true); - helper_->set_blocked(true); - connection_.ProcessUdpPacket(IPEndPoint(), IPEndPoint(), *encrypted); - EXPECT_EQ(0u, helper_->last_packet_size()); - EXPECT_TRUE(connection_.HasQueuedData()); - - helper_->set_blocked(false); - connection_.OnCanWrite(); - EXPECT_TRUE(helper_->version_negotiation_packet() != NULL); - - size_t num_versions = arraysize(kSupportedQuicVersions); - EXPECT_EQ(num_versions, - helper_->version_negotiation_packet()->versions.size()); - - // We expect all versions in kSupportedQuicVersions to be - // included in the packet. - for (size_t i = 0; i < num_versions; ++i) { - EXPECT_EQ(kSupportedQuicVersions[i], - helper_->version_negotiation_packet()->versions[i]); - } -} - TEST_P(QuicConnectionTest, ClientHandlesVersionNegotiation) { // Start out with some unsupported version. QuicConnectionPeer::GetFramer(&connection_)->set_version_for_tests( @@ -3190,7 +3130,7 @@ class MockQuicConnectionDebugVisitor void(QuicPacketSequenceNumber, EncryptionLevel, const QuicEncryptedPacket&, - WriteResult)); + int)); MOCK_METHOD2(OnPacketRetransmitted, void(QuicPacketSequenceNumber, diff --git a/net/quic/quic_protocol.h b/net/quic/quic_protocol.h index c42d1fe..8532a22 100644 --- a/net/quic/quic_protocol.h +++ b/net/quic/quic_protocol.h @@ -377,8 +377,6 @@ enum QuicErrorCode { QUIC_INVALID_CRYPTO_MESSAGE_TYPE = 33, // A crypto message was received with an illegal parameter. QUIC_INVALID_CRYPTO_MESSAGE_PARAMETER = 34, - // An invalid channel id signature was supplied. - QUIC_INVALID_CHANNEL_ID_SIGNATURE = 52, // A crypto message was received with a mandatory parameter missing. QUIC_CRYPTO_MESSAGE_PARAMETER_NOT_FOUND = 35, // A crypto message was received with a parameter that has no overlap @@ -407,7 +405,7 @@ enum QuicErrorCode { QUIC_CRYPTO_SERVER_CONFIG_EXPIRED = 45, // No error. Used as bound while iterating. - QUIC_LAST_ERROR = 53, + QUIC_LAST_ERROR = 52, }; struct NET_EXPORT_PRIVATE QuicPacketPublicHeader { @@ -872,26 +870,6 @@ struct QuicConsumedData { bool fin_consumed; }; -enum WriteStatus { - WRITE_STATUS_OK, - WRITE_STATUS_BLOCKED, - WRITE_STATUS_ERROR, -}; - -// A struct used to return the result of write calls including either the number -// of bytes written or the error code, depending upon the status. -struct NET_EXPORT_PRIVATE WriteResult { - WriteResult(WriteStatus status, int bytes_written_or_error_code) : - status(status), bytes_written(bytes_written_or_error_code) { - } - - WriteStatus status; - union { - int bytes_written; // only valid when status is OK - int error_code; // only valid when status is ERROR - }; -}; - } // namespace net #endif // NET_QUIC_QUIC_PROTOCOL_H_ diff --git a/net/quic/quic_received_packet_manager.cc b/net/quic/quic_received_packet_manager.cc index 1cd261a..81868ac 100644 --- a/net/quic/quic_received_packet_manager.cc +++ b/net/quic/quic_received_packet_manager.cc @@ -98,13 +98,11 @@ QuicPacketEntropyHash QuicReceivedPacketManager::EntropyHash( 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. + // |largest_received_sequence_number_|. // 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 + // LOG_IF_EVERY_N_SEC(WARNING, it != packets_entropy_.end(), 10) + LOG_IF(WARNING, it != packets_entropy_.end()) + << "largest_received: " << received_info_.largest_observed << " sequence_number: " << sequence_number; // TODO(satyamshekhar): Make this O(1). diff --git a/net/quic/quic_sent_packet_manager.cc b/net/quic/quic_sent_packet_manager.cc index 66eea78..a9f26b8 100644 --- a/net/quic/quic_sent_packet_manager.cc +++ b/net/quic/quic_sent_packet_manager.cc @@ -131,9 +131,15 @@ void QuicSentPacketManager::OnRetransmittedPacket( void QuicSentPacketManager::OnIncomingAck( const ReceivedPacketInfo& received_info, - bool is_truncated_ack) { - HandleAckForSentPackets(received_info, is_truncated_ack); - HandleAckForSentFecPackets(received_info); + bool is_truncated_ack, + SequenceNumberSet* acked_packets) { + HandleAckForSentPackets(received_info, is_truncated_ack, acked_packets); + HandleAckForSentFecPackets(received_info, acked_packets); + + if (acked_packets->size() > 0) { + // The AckNotifierManager should be informed of every ACKed sequence number. + ack_notifier_manager_.OnIncomingAck(*acked_packets); + } } void QuicSentPacketManager::DiscardUnackedPacket( @@ -143,7 +149,8 @@ void QuicSentPacketManager::DiscardUnackedPacket( void QuicSentPacketManager::HandleAckForSentPackets( const ReceivedPacketInfo& received_info, - bool is_truncated_ack) { + bool is_truncated_ack, + SequenceNumberSet* acked_packets) { // 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,11 +166,12 @@ void QuicSentPacketManager::HandleAckForSentPackets( DVLOG(1) << ENDPOINT <<"Got an ack for packet " << sequence_number; // If data is associated with the most recent transmission of this // packet, then inform the caller. + QuicPacketSequenceNumber most_recent_transmission = + GetMostRecentTransmission(sequence_number); + if (HasRetransmittableFrames(most_recent_transmission)) { + acked_packets->insert(most_recent_transmission); + } it = MarkPacketReceivedByPeer(sequence_number); - - // The AckNotifierManager is informed of every ACKed sequence number. - ack_notifier_manager_.OnPacketAcked(sequence_number); - continue; } @@ -373,7 +381,8 @@ void QuicSentPacketManager::DiscardPacket( } void QuicSentPacketManager::HandleAckForSentFecPackets( - const ReceivedPacketInfo& received_info) { + const ReceivedPacketInfo& received_info, + SequenceNumberSet* acked_packets) { UnackedFecPacketMap::iterator it = unacked_fec_packets_.begin(); while (it != unacked_fec_packets_.end()) { QuicPacketSequenceNumber sequence_number = it->first; @@ -383,6 +392,7 @@ void QuicSentPacketManager::HandleAckForSentFecPackets( if (!IsAwaitingPacket(received_info, sequence_number)) { DVLOG(1) << ENDPOINT << "Got an ack for fec packet: " << sequence_number; + acked_packets->insert(sequence_number); unacked_fec_packets_.erase(it++); } else { // TODO(rch): treat these packets more consistently. They should diff --git a/net/quic/quic_sent_packet_manager.h b/net/quic/quic_sent_packet_manager.h index cf4edc9..4987bcc 100644 --- a/net/quic/quic_sent_packet_manager.h +++ b/net/quic/quic_sent_packet_manager.h @@ -79,7 +79,8 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager { // Processes the ReceivedPacketInfo data from the incoming ack. void OnIncomingAck(const ReceivedPacketInfo& received_info, - bool is_truncated_ack); + bool is_truncated_ack, + SequenceNumberSet* acked_packets); // Discards any information for the packet corresponding to |sequence_number|. // If this packet has been retransmitted, information on those packets @@ -186,10 +187,12 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager { // Process the incoming ack looking for newly ack'd data packets. void HandleAckForSentPackets(const ReceivedPacketInfo& received_info, - bool is_truncated_ack); + bool is_truncated_ack, + SequenceNumberSet* acked_packets); // Process the incoming ack looking for newly ack'd FEC packets. - void HandleAckForSentFecPackets(const ReceivedPacketInfo& received_info); + void HandleAckForSentFecPackets(const ReceivedPacketInfo& received_info, + SequenceNumberSet* acked_packets); // Marks |sequence_number| as having been seen by the peer. Returns an // iterator to the next remaining unacked packet. diff --git a/net/quic/quic_sent_packet_manager_test.cc b/net/quic/quic_sent_packet_manager_test.cc index 201d9a1..bfd4d4a 100644 --- a/net/quic/quic_sent_packet_manager_test.cc +++ b/net/quic/quic_sent_packet_manager_test.cc @@ -170,11 +170,14 @@ TEST_P(QuicSentPacketManagerTest, RetransmitThenAck) { ReceivedPacketInfo received_info; received_info.largest_observed = 2; received_info.missing_packets.insert(1); - manager_.OnIncomingAck(received_info, false); + SequenceNumberSet acked; + manager_.OnIncomingAck(received_info, false, &acked); // No unacked packets remain. VerifyUnackedPackets(NULL, 0); VerifyRetransmittablePackets(NULL, 0); + QuicPacketSequenceNumber expected_acked[] = { 2 }; + VerifyAckedPackets(expected_acked, arraysize(expected_acked), acked); } TEST_P(QuicSentPacketManagerTest, RetransmitThenAckBeforeSend) { @@ -187,7 +190,8 @@ TEST_P(QuicSentPacketManagerTest, RetransmitThenAckBeforeSend) { // Ack 1. ReceivedPacketInfo received_info; received_info.largest_observed = 1; - manager_.OnIncomingAck(received_info, false); + SequenceNumberSet acked; + manager_.OnIncomingAck(received_info, false, &acked); // There should no longer be a pending retransmission. EXPECT_FALSE(manager_.HasPendingRetransmissions()); @@ -195,6 +199,8 @@ TEST_P(QuicSentPacketManagerTest, RetransmitThenAckBeforeSend) { // No unacked packets remain. VerifyUnackedPackets(NULL, 0); VerifyRetransmittablePackets(NULL, 0); + QuicPacketSequenceNumber expected_acked[] = { 1 }; + VerifyAckedPackets(expected_acked, arraysize(expected_acked), acked); } TEST_P(QuicSentPacketManagerTest, RetransmitThenAckPrevious) { @@ -211,13 +217,16 @@ TEST_P(QuicSentPacketManagerTest, RetransmitThenAckPrevious) { ReceivedPacketInfo received_info; received_info.largest_observed = 2; received_info.missing_packets.insert(2); + SequenceNumberSet acked; EXPECT_CALL(helper_, OnPacketNacked(2, 1)).Times(1); - manager_.OnIncomingAck(received_info, false); + manager_.OnIncomingAck(received_info, false, &acked); // 2 remains unacked, but no packets have retransmittable data. QuicPacketSequenceNumber unacked[] = { 2 }; VerifyUnackedPackets(unacked, arraysize(unacked)); VerifyRetransmittablePackets(NULL, 0); + QuicPacketSequenceNumber expected_acked[] = { 2 }; + VerifyAckedPackets(expected_acked, arraysize(expected_acked), acked); } TEST_P(QuicSentPacketManagerTest, TruncatedAck) { @@ -237,7 +246,8 @@ TEST_P(QuicSentPacketManagerTest, TruncatedAck) { received_info.largest_observed = 2; received_info.missing_packets.insert(1); received_info.missing_packets.insert(2); - manager_.OnIncomingAck(received_info, true); + SequenceNumberSet acked; + manager_.OnIncomingAck(received_info, true, &acked); // High water mark will be raised. QuicPacketSequenceNumber unacked[] = { 2, 3, 4 }; @@ -260,8 +270,9 @@ TEST_P(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) { ReceivedPacketInfo received_info; received_info.largest_observed = 3; received_info.missing_packets.insert(2); + SequenceNumberSet acked; EXPECT_CALL(helper_, OnPacketNacked(2, 1)).Times(1); - manager_.OnIncomingAck(received_info, false); + manager_.OnIncomingAck(received_info, false, &acked); QuicPacketSequenceNumber unacked[] = { 2 }; VerifyUnackedPackets(unacked, arraysize(unacked)); @@ -278,9 +289,10 @@ TEST_P(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) { received_info.largest_observed = 5; received_info.missing_packets.insert(2); received_info.missing_packets.insert(4); + SequenceNumberSet acked; EXPECT_CALL(helper_, OnPacketNacked(2, 2)).Times(1); EXPECT_CALL(helper_, OnPacketNacked(4, 1)).Times(1); - manager_.OnIncomingAck(received_info, false); + manager_.OnIncomingAck(received_info, false, &acked); QuicPacketSequenceNumber unacked[] = { 2, 4 }; VerifyUnackedPackets(unacked, arraysize(unacked)); @@ -298,10 +310,11 @@ TEST_P(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) { received_info.missing_packets.insert(2); received_info.missing_packets.insert(4); received_info.missing_packets.insert(6); + SequenceNumberSet acked; EXPECT_CALL(helper_, OnPacketNacked(2, 3)).Times(1); EXPECT_CALL(helper_, OnPacketNacked(4, 2)).Times(1); EXPECT_CALL(helper_, OnPacketNacked(6, 1)).Times(1); - manager_.OnIncomingAck(received_info, false); + manager_.OnIncomingAck(received_info, false, &acked); QuicPacketSequenceNumber unacked[] = { 2, 4, 6 }; VerifyUnackedPackets(unacked, arraysize(unacked)); @@ -322,11 +335,12 @@ TEST_P(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) { received_info.missing_packets.insert(6); received_info.missing_packets.insert(8); received_info.missing_packets.insert(9); + SequenceNumberSet acked; EXPECT_CALL(helper_, OnPacketNacked(4, 3)).Times(1); EXPECT_CALL(helper_, OnPacketNacked(6, 2)).Times(1); EXPECT_CALL(helper_, OnPacketNacked(8, 1)).Times(1); EXPECT_CALL(helper_, OnPacketNacked(9, 1)).Times(1); - manager_.OnIncomingAck(received_info, false); + manager_.OnIncomingAck(received_info, false, &acked); QuicPacketSequenceNumber unacked[] = { 2, 4, 6, 8, 9 }; VerifyUnackedPackets(unacked, arraysize(unacked)); @@ -350,12 +364,13 @@ TEST_P(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) { received_info.missing_packets.insert(9); received_info.missing_packets.insert(11); received_info.missing_packets.insert(12); + SequenceNumberSet acked; EXPECT_CALL(helper_, OnPacketNacked(6, 3)).Times(1); EXPECT_CALL(helper_, OnPacketNacked(8, 2)).Times(1); EXPECT_CALL(helper_, OnPacketNacked(9, 2)).Times(1); EXPECT_CALL(helper_, OnPacketNacked(11, 1)).Times(1); EXPECT_CALL(helper_, OnPacketNacked(12, 1)).Times(1); - manager_.OnIncomingAck(received_info, false); + manager_.OnIncomingAck(received_info, false, &acked); QuicPacketSequenceNumber unacked[] = { 2, 4, 6, 8, 9, 11, 12 }; VerifyUnackedPackets(unacked, arraysize(unacked)); @@ -378,11 +393,12 @@ TEST_P(QuicSentPacketManagerTest, SendDropAckRetransmitManyPackets) { received_info.missing_packets.insert(9); received_info.missing_packets.insert(11); received_info.missing_packets.insert(12); + SequenceNumberSet acked; EXPECT_CALL(helper_, OnPacketNacked(8, 3)).Times(1); EXPECT_CALL(helper_, OnPacketNacked(9, 3)).Times(1); EXPECT_CALL(helper_, OnPacketNacked(11, 2)).Times(1); EXPECT_CALL(helper_, OnPacketNacked(12, 2)).Times(1); - manager_.OnIncomingAck(received_info, true); + manager_.OnIncomingAck(received_info, true, &acked); // Truncated ack raises the high water mark by clearing out 2, 4, and 6. QuicPacketSequenceNumber unacked[] = { 8, 9, 11, 12, 14, 15, 16 }; @@ -446,7 +462,8 @@ TEST_P(QuicSentPacketManagerTest, GetLeastUnackedFecPacketAndDiscard) { // Ack 2. ReceivedPacketInfo received_info; received_info.largest_observed = 2; - manager_.OnIncomingAck(received_info, false); + SequenceNumberSet acked; + manager_.OnIncomingAck(received_info, false, &acked); EXPECT_EQ(3u, manager_.GetLeastUnackedFecPacket()); diff --git a/net/quic/quic_utils.cc b/net/quic/quic_utils.cc index a5a0eec..cfb3cb7 100644 --- a/net/quic/quic_utils.cc +++ b/net/quic/quic_utils.cc @@ -188,7 +188,6 @@ const char* QuicUtils::ErrorToString(QuicErrorCode error) { RETURN_STRING_LITERAL(QUIC_CRYPTO_DUPLICATE_TAG); RETURN_STRING_LITERAL(QUIC_CRYPTO_ENCRYPTION_LEVEL_INCORRECT); RETURN_STRING_LITERAL(QUIC_CRYPTO_SERVER_CONFIG_EXPIRED); - RETURN_STRING_LITERAL(QUIC_INVALID_CHANNEL_ID_SIGNATURE); RETURN_STRING_LITERAL(QUIC_LAST_ERROR); // Intentionally have no default case, so we'll break the build // if we add errors and don't put them here. diff --git a/net/quic/test_tools/quic_test_utils.h b/net/quic/test_tools/quic_test_utils.h index fd29824..ffb433e 100644 --- a/net/quic/test_tools/quic_test_utils.h +++ b/net/quic/test_tools/quic_test_utils.h @@ -197,8 +197,8 @@ class MockHelper : public QuicConnectionHelperInterface { const QuicClock* GetClock() const; QuicRandom* GetRandomGenerator(); void AdvanceTime(QuicTime::Delta delta); - MOCK_METHOD1(WritePacketToWire, - WriteResult(const QuicEncryptedPacket& packet)); + MOCK_METHOD2(WritePacketToWire, int(const QuicEncryptedPacket& packet, + int* error)); MOCK_METHOD0(IsWriteBlockedDataBuffered, bool()); MOCK_METHOD1(IsWriteBlocked, bool(int stream_id)); virtual QuicAlarm* CreateAlarm(QuicAlarm::Delegate* delegate); |