diff options
author | ckrasic <ckrasic@chromium.org> | 2015-10-30 22:03:27 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-10-31 05:04:15 +0000 |
commit | ea295fe8fd113fa2497c596b7c974a2074ec4e37 (patch) | |
tree | 46934075ddacda4de8edd8df7f3c1377a83209f3 | |
parent | 009b3d08f0973a52d3bd52696ecc26689b2738cd (diff) | |
download | chromium_src-ea295fe8fd113fa2497c596b7c974a2074ec4e37.zip chromium_src-ea295fe8fd113fa2497c596b7c974a2074ec4e37.tar.gz chromium_src-ea295fe8fd113fa2497c596b7c974a2074ec4e37.tar.bz2 |
Landing Recent QUIC changes until 9:17 AM, Oct 26 2015 UTC-7
Unroll QuicPacketCreator::CopyToBuffer and prefetch the next iov when possible.
Merge internal change: 106305702
Fully qualify std::* names being exported internally by using declarations in stl.
These using declarations are a historical artifact and they hinder portability
and toolchain changes.
Merge internal change: 106194954
Change QUIC's SendAlgorithmInterface::OnCongestionEvent to use a QuicByteCount instead of an entire TransmissionInfo, because the other fields were unused. No functional change.
Merge internal change: 106159730
Deprecate use_interval_set_.
Merge internal change: 106158486
(Correct a comment.)
Merge internal change: 106095793
(test only) Use data stream id to process decompressed header and data frames. Always register the QuicSpdyServerStreamPeer in session's dynamic_stream_map, pass ownership to session.
Merge internal change: 106092098
Change the PacketTimeList to be a PacketTimeVector, because it is more memory efficient.
Small functional change to clear all timestamps if
any are too old in sequence number space.
Merge internal change: 106063258
Fix the interaction between QUIC stateless rejects and version negotiation.
Move the code which closes statelessly rejected server connections from
the QuicDispatcher to the QuicCryptoServerStream. This permits the
connection be closed even when the packet which creates the connection
is not the packet which solicits the stateless reject.
Merge internal change: 105994917
Rename a large number of QuicAckListeners to listener from delegate. No functional change.
Merge internal change: 105984563
Remove the ProxyAckNotifierDelegate class from QUIC's ack listener pipeline because it was only a passthrough. No functional change.
Merge internal change: 105972541
Deprecate flag quic_no_ack_notifier and remove the QuicAckNotifier and QuicAckNotifierManager.
Merge internal change: 105964824
Pass QuicIOVector by value; it's too small to be passed by reference. No functional change.
Merge internal change: 105961176
Remove C++11 library features from QuicTestServer.
Merge internal change: 105906226
test fixes, and bugfix to unblock new streams, blocked due to max open limit, as unfinished streams retire.
Remove QUIC_TOO_MANY_UNFINISHED_STREAMS.
Unfinished streams should be counted same as the open streams to prevent
buggy client to initate too many streams. The server will refuse streams
with TOO_MANY_OPEN_STREAMS when the checking for
TOO_MANY_UNFINISHED_STREAMS is removed.
Merge internal change: 105904662
BUG=
Review URL: https://codereview.chromium.org/1423033005
Cr-Commit-Position: refs/heads/master@{#357273}
70 files changed, 633 insertions, 1464 deletions
diff --git a/net/net.gypi b/net/net.gypi index c420a97..753fd24 100644 --- a/net/net.gypi +++ b/net/net.gypi @@ -300,10 +300,6 @@ 'quic/port_suggester.cc', 'quic/port_suggester.h', 'quic/quic_ack_listener_interface.h', - 'quic/quic_ack_notifier.cc', - 'quic/quic_ack_notifier.h', - 'quic/quic_ack_notifier_manager.cc', - 'quic/quic_ack_notifier_manager.h', 'quic/quic_address_mismatch.cc', 'quic/quic_address_mismatch.h', 'quic/quic_alarm.cc', @@ -1519,8 +1515,6 @@ 'quic/network_connection_unittest.cc', 'quic/p2p/quic_p2p_session_test.cc', 'quic/port_suggester_unittest.cc', - 'quic/quic_ack_notifier_manager_test.cc', - 'quic/quic_ack_notifier_test.cc', 'quic/quic_address_mismatch_test.cc', 'quic/quic_alarm_test.cc', 'quic/quic_bandwidth_test.cc', @@ -1578,8 +1572,6 @@ 'quic/test_tools/mock_quic_dispatcher.h', 'quic/test_tools/mock_random.cc', 'quic/test_tools/mock_random.h', - 'quic/test_tools/quic_ack_notifier_manager_peer.cc', - 'quic/test_tools/quic_ack_notifier_manager_peer.h', 'quic/test_tools/quic_chromium_client_session_peer.cc', 'quic/test_tools/quic_chromium_client_session_peer.h', 'quic/test_tools/quic_config_peer.cc', diff --git a/net/quic/congestion_control/send_algorithm_interface.h b/net/quic/congestion_control/send_algorithm_interface.h index 3daee55..f59def5 100644 --- a/net/quic/congestion_control/send_algorithm_interface.h +++ b/net/quic/congestion_control/send_algorithm_interface.h @@ -27,7 +27,7 @@ class RttStats; class NET_EXPORT_PRIVATE SendAlgorithmInterface { public: // A sorted vector of packets. - typedef std::vector<std::pair<QuicPacketNumber, TransmissionInfo>> + typedef std::vector<std::pair<QuicPacketNumber, QuicPacketLength>> CongestionVector; static SendAlgorithmInterface* Create( diff --git a/net/quic/congestion_control/send_algorithm_simulator.cc b/net/quic/congestion_control/send_algorithm_simulator.cc index ff58090..a295708 100644 --- a/net/quic/congestion_control/send_algorithm_simulator.cc +++ b/net/quic/congestion_control/send_algorithm_simulator.cc @@ -278,9 +278,6 @@ void SendAlgorithmSimulator::HandlePendingAck(Transfer* transfer) { list<SentPacket>::iterator it = sent_packets_.begin(); while (sender->last_acked < sender->next_acked) { ++sender->last_acked; - TransmissionInfo info = TransmissionInfo(); - info.bytes_sent = kPacketSize; - info.in_flight = true; // Find the next SentPacket for this transfer. while (it->transfer != transfer) { DCHECK(it != sent_packets_.end()); @@ -290,13 +287,13 @@ void SendAlgorithmSimulator::HandlePendingAck(Transfer* transfer) { if (it->packet_number > sender->last_acked) { DVLOG(1) << "Lost packet:" << sender->last_acked << " dropped by buffer overflow."; - lost_packets.push_back(std::make_pair(sender->last_acked, info)); + lost_packets.push_back(std::make_pair(sender->last_acked, kPacketSize)); continue; } if (it->lost) { - lost_packets.push_back(std::make_pair(sender->last_acked, info)); + lost_packets.push_back(std::make_pair(sender->last_acked, kPacketSize)); } else { - acked_packets.push_back(std::make_pair(sender->last_acked, info)); + acked_packets.push_back(std::make_pair(sender->last_acked, kPacketSize)); } // This packet has been acked or lost, remove it from sent_packets_. largest_observed = *it; diff --git a/net/quic/congestion_control/tcp_cubic_bytes_sender.cc b/net/quic/congestion_control/tcp_cubic_bytes_sender.cc index c123fd2..06288db 100644 --- a/net/quic/congestion_control/tcp_cubic_bytes_sender.cc +++ b/net/quic/congestion_control/tcp_cubic_bytes_sender.cc @@ -129,7 +129,7 @@ void TcpCubicBytesSender::OnCongestionEvent( } for (CongestionVector::const_iterator it = acked_packets.begin(); it != acked_packets.end(); ++it) { - OnPacketAcked(it->first, it->second.bytes_sent, bytes_in_flight); + OnPacketAcked(it->first, it->second, bytes_in_flight); } } diff --git a/net/quic/congestion_control/tcp_cubic_bytes_sender_test.cc b/net/quic/congestion_control/tcp_cubic_bytes_sender_test.cc index ff30bb5..d1f1403 100644 --- a/net/quic/congestion_control/tcp_cubic_bytes_sender_test.cc +++ b/net/quic/congestion_control/tcp_cubic_bytes_sender_test.cc @@ -57,9 +57,7 @@ class TcpCubicBytesSenderTest : public ::testing::Test { sender_(new TcpCubicBytesSenderPeer(&clock_, true)), packet_number_(1), acked_packet_number_(0), - bytes_in_flight_(0) { - standard_packet_.bytes_sent = kDefaultTCPMSS; - } + bytes_in_flight_(0) {} int SendAvailableSendWindow() { // Send as long as TimeUntilSend returns Zero. @@ -86,7 +84,7 @@ class TcpCubicBytesSenderTest : public ::testing::Test { for (int i = 0; i < n; ++i) { ++acked_packet_number_; acked_packets.push_back( - std::make_pair(acked_packet_number_, standard_packet_)); + std::make_pair(acked_packet_number_, kDefaultTCPMSS)); } sender_->OnCongestionEvent(true, bytes_in_flight_, acked_packets, lost_packets); @@ -100,7 +98,7 @@ class TcpCubicBytesSenderTest : public ::testing::Test { for (int i = 0; i < n; ++i) { ++acked_packet_number_; lost_packets.push_back( - std::make_pair(acked_packet_number_, standard_packet_)); + std::make_pair(acked_packet_number_, kDefaultTCPMSS)); } sender_->OnCongestionEvent(false, bytes_in_flight_, acked_packets, lost_packets); @@ -111,7 +109,7 @@ class TcpCubicBytesSenderTest : public ::testing::Test { void LosePacket(QuicPacketNumber packet_number) { SendAlgorithmInterface::CongestionVector acked_packets; SendAlgorithmInterface::CongestionVector lost_packets; - lost_packets.push_back(std::make_pair(packet_number, standard_packet_)); + lost_packets.push_back(std::make_pair(packet_number, kDefaultTCPMSS)); sender_->OnCongestionEvent(false, bytes_in_flight_, acked_packets, lost_packets); bytes_in_flight_ -= kDefaultTCPMSS; @@ -123,7 +121,6 @@ class TcpCubicBytesSenderTest : public ::testing::Test { QuicPacketNumber packet_number_; QuicPacketNumber acked_packet_number_; QuicByteCount bytes_in_flight_; - TransmissionInfo standard_packet_; }; TEST_F(TcpCubicBytesSenderTest, SimpleSender) { diff --git a/net/quic/congestion_control/tcp_cubic_sender.cc b/net/quic/congestion_control/tcp_cubic_sender.cc index a302f51..457dd2e 100644 --- a/net/quic/congestion_control/tcp_cubic_sender.cc +++ b/net/quic/congestion_control/tcp_cubic_sender.cc @@ -145,7 +145,7 @@ void TcpCubicSender::OnCongestionEvent( } for (CongestionVector::const_iterator it = acked_packets.begin(); it != acked_packets.end(); ++it) { - OnPacketAcked(it->first, it->second.bytes_sent, bytes_in_flight); + OnPacketAcked(it->first, it->second, bytes_in_flight); } } diff --git a/net/quic/congestion_control/tcp_cubic_sender_test.cc b/net/quic/congestion_control/tcp_cubic_sender_test.cc index 1bac853..ad19af8 100644 --- a/net/quic/congestion_control/tcp_cubic_sender_test.cc +++ b/net/quic/congestion_control/tcp_cubic_sender_test.cc @@ -71,9 +71,7 @@ class TcpCubicSenderTest : public ::testing::Test { sender_(new TcpCubicSenderPeer(&clock_, true, kMaxCongestionWindow)), packet_number_(1), acked_packet_number_(0), - bytes_in_flight_(0) { - standard_packet_.bytes_sent = kDefaultTCPMSS; - } + bytes_in_flight_(0) {} int SendAvailableSendWindow() { // Send as long as TimeUntilSend returns Zero. @@ -101,7 +99,7 @@ class TcpCubicSenderTest : public ::testing::Test { for (int i = 0; i < n; ++i) { ++acked_packet_number_; acked_packets.push_back( - std::make_pair(acked_packet_number_, standard_packet_)); + std::make_pair(acked_packet_number_, kDefaultTCPMSS)); } sender_->OnCongestionEvent( true, bytes_in_flight_, acked_packets, lost_packets); @@ -115,7 +113,7 @@ class TcpCubicSenderTest : public ::testing::Test { for (int i = 0; i < n; ++i) { ++acked_packet_number_; lost_packets.push_back( - std::make_pair(acked_packet_number_, standard_packet_)); + std::make_pair(acked_packet_number_, kDefaultTCPMSS)); } sender_->OnCongestionEvent( false, bytes_in_flight_, acked_packets, lost_packets); @@ -126,7 +124,7 @@ class TcpCubicSenderTest : public ::testing::Test { void LosePacket(QuicPacketNumber packet_number) { SendAlgorithmInterface::CongestionVector acked_packets; SendAlgorithmInterface::CongestionVector lost_packets; - lost_packets.push_back(std::make_pair(packet_number, standard_packet_)); + lost_packets.push_back(std::make_pair(packet_number, kDefaultTCPMSS)); sender_->OnCongestionEvent( false, bytes_in_flight_, acked_packets, lost_packets); bytes_in_flight_ -= kDefaultTCPMSS; @@ -138,7 +136,6 @@ class TcpCubicSenderTest : public ::testing::Test { QuicPacketNumber packet_number_; QuicPacketNumber acked_packet_number_; QuicByteCount bytes_in_flight_; - TransmissionInfo standard_packet_; }; TEST_F(TcpCubicSenderTest, SimpleSender) { diff --git a/net/quic/congestion_control/tcp_loss_algorithm_test.cc b/net/quic/congestion_control/tcp_loss_algorithm_test.cc index af817b7..3604a61 100644 --- a/net/quic/congestion_control/tcp_loss_algorithm_test.cc +++ b/net/quic/congestion_control/tcp_loss_algorithm_test.cc @@ -9,7 +9,6 @@ #include "base/logging.h" #include "base/stl_util.h" #include "net/quic/congestion_control/rtt_stats.h" -#include "net/quic/quic_ack_notifier_manager.h" #include "net/quic/quic_unacked_packet_map.h" #include "net/quic/test_tools/mock_clock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -25,7 +24,7 @@ const uint32 kDefaultLength = 1000; class TcpLossAlgorithmTest : public ::testing::Test { protected: - TcpLossAlgorithmTest() : unacked_packets_(&ack_notifier_manager_) { + TcpLossAlgorithmTest() { rtt_stats_.UpdateRtt(QuicTime::Delta::FromMilliseconds(100), QuicTime::Delta::Zero(), clock_.Now()); @@ -56,7 +55,6 @@ class TcpLossAlgorithmTest : public ::testing::Test { } vector<QuicEncryptedPacket*> packets_; - AckNotifierManager ack_notifier_manager_; QuicUnackedPacketMap unacked_packets_; TCPLossAlgorithm loss_algorithm_; RttStats rtt_stats_; diff --git a/net/quic/congestion_control/time_loss_algorithm_test.cc b/net/quic/congestion_control/time_loss_algorithm_test.cc index 6798c1a..564c43e 100644 --- a/net/quic/congestion_control/time_loss_algorithm_test.cc +++ b/net/quic/congestion_control/time_loss_algorithm_test.cc @@ -9,7 +9,6 @@ #include "base/logging.h" #include "base/stl_util.h" #include "net/quic/congestion_control/rtt_stats.h" -#include "net/quic/quic_ack_notifier_manager.h" #include "net/quic/quic_unacked_packet_map.h" #include "net/quic/test_tools/mock_clock.h" #include "testing/gtest/include/gtest/gtest.h" @@ -25,7 +24,7 @@ const uint32 kDefaultLength = 1000; class TimeLossAlgorithmTest : public ::testing::Test { protected: - TimeLossAlgorithmTest() : unacked_packets_(&ack_notifier_manager_) { + TimeLossAlgorithmTest() { rtt_stats_.UpdateRtt(QuicTime::Delta::FromMilliseconds(100), QuicTime::Delta::Zero(), clock_.Now()); @@ -56,7 +55,6 @@ class TimeLossAlgorithmTest : public ::testing::Test { } vector<QuicEncryptedPacket*> packets_; - AckNotifierManager ack_notifier_manager_; QuicUnackedPacketMap unacked_packets_; TimeLossAlgorithm loss_algorithm_; RttStats rtt_stats_; diff --git a/net/quic/crypto/crypto_server_test.cc b/net/quic/crypto/crypto_server_test.cc index fa5c589..52b4baa 100644 --- a/net/quic/crypto/crypto_server_test.cc +++ b/net/quic/crypto/crypto_server_test.cc @@ -60,9 +60,10 @@ struct TestParams { friend ostream& operator<<(ostream& os, const TestParams& p) { os << "{ use_early_return_when_verifying_chlo: " - << p.use_early_return_when_verifying_chlo << endl; - os << " enable_stateless_rejects: " << p.enable_stateless_rejects << endl; - os << " use_stateless_rejects: " << p.use_stateless_rejects << endl; + << p.use_early_return_when_verifying_chlo << std::endl; + os << " enable_stateless_rejects: " << p.enable_stateless_rejects + << std::endl; + os << " use_stateless_rejects: " << p.use_stateless_rejects << std::endl; os << " versions: " << QuicVersionVectorToString(p.supported_versions) << " }"; return os; diff --git a/net/quic/quic_ack_listener_interface.h b/net/quic/quic_ack_listener_interface.h index 94e0561..ceb369e 100644 --- a/net/quic/quic_ack_listener_interface.h +++ b/net/quic/quic_ack_listener_interface.h @@ -5,8 +5,6 @@ #ifndef NET_QUIC_QUIC_ACK_LISTENER_INTERFACE_H_ #define NET_QUIC_QUIC_ACK_LISTENER_INTERFACE_H_ -#include "net/quic/quic_ack_listener_interface.h" - #include "base/basictypes.h" #include "base/memory/ref_counted.h" #include "net/quic/quic_protocol.h" @@ -20,14 +18,6 @@ class NET_EXPORT_PRIVATE QuicAckListenerInterface public: QuicAckListenerInterface() {} - // Args: - // num_retransmitted_packets - Number of packets that had to be - // retransmitted. - // num_retransmitted_bytes - Number of bytes that had to be retransmitted. - virtual void OnAckNotification(int num_retransmitted_packets, - int num_retransmitted_bytes, - QuicTime::Delta delta_largest_observed) = 0; - // Called when a packet is acked. Called once per packet. // |acked_bytes| is the number of data bytes acked. virtual void OnPacketAcked(int acked_bytes, diff --git a/net/quic/quic_ack_notifier.cc b/net/quic/quic_ack_notifier.cc deleted file mode 100644 index 997dbb1..0000000 --- a/net/quic/quic_ack_notifier.cc +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright 2013 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "net/quic/quic_ack_notifier.h" - -#include <set> - -#include "base/logging.h" -#include "base/stl_util.h" - -using base::hash_map; -using std::make_pair; - -namespace net { - -QuicAckNotifier::QuicAckNotifier(QuicAckListenerInterface* delegate) - : delegate_(delegate), - unacked_packets_(0), - retransmitted_packet_count_(0), - retransmitted_byte_count_(0) { - DCHECK(delegate); -} - -QuicAckNotifier::~QuicAckNotifier() { -} - -void QuicAckNotifier::OnSerializedPacket() { - ++unacked_packets_; -} - -bool QuicAckNotifier::OnAck(QuicTime::Delta delta_largest_observed) { - if (unacked_packets_ <= 0) { - LOG(DFATAL) << "Acked more packets than were tracked." - << " unacked_packets:" << unacked_packets_; - return true; - } - --unacked_packets_; - if (!HasUnackedPackets()) { - // We have seen all the packet numbers we were waiting for, trigger - // callback notification. - delegate_->OnAckNotification(retransmitted_packet_count_, - retransmitted_byte_count_, - delta_largest_observed); - return true; - } - return false; -} - -bool QuicAckNotifier::OnPacketAbandoned() { - if (unacked_packets_ <= 0) { - LOG(DFATAL) << "Abandoned more packets than were tracked." - << " unacked_packets:" << unacked_packets_; - return true; - } - --unacked_packets_; - return unacked_packets_ == 0; -} - -void QuicAckNotifier::OnPacketRetransmitted(int packet_payload_size) { - ++retransmitted_packet_count_; - retransmitted_byte_count_ += packet_payload_size; -} - -} // namespace net diff --git a/net/quic/quic_ack_notifier.h b/net/quic/quic_ack_notifier.h deleted file mode 100644 index d2b5613..0000000 --- a/net/quic/quic_ack_notifier.h +++ /dev/null @@ -1,65 +0,0 @@ -// Copyright 2013 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef NET_QUIC_QUIC_ACK_NOTIFIER_H_ -#define NET_QUIC_QUIC_ACK_NOTIFIER_H_ - -#include "base/memory/ref_counted.h" -#include "net/quic/quic_protocol.h" -#include "net/quic/quic_time.h" - -namespace net { - -// Used to register with a QuicConnection for notification once a set of packets -// have all been ACKed. -// The connection informs this class of newly ACKed packet numbers, and once -// we have seen ACKs for all the packet numbers we are interested in, we -// trigger a call to a provided Closure. -class NET_EXPORT_PRIVATE QuicAckNotifier { - public: - // QuicAckNotifier is expected to keep its own reference to the delegate. - explicit QuicAckNotifier(QuicAckListenerInterface* delegate); - virtual ~QuicAckNotifier(); - - // Register a serialized packet the notifier should track. - void OnSerializedPacket(); - - // Called on receipt of new ACK frame for an unacked packet. - // Decrements the number of unacked packets and if there are none left, calls - // the stored delegate's OnAckNotification method. - // - // Returns true if the delegate was called, false otherwise. - bool OnAck(QuicTime::Delta delta_largest_observed); - - // Called when we've given up waiting for a packet number, typically when - // the connection is torn down. - // Returns true if there are no more unacked packets being tracked. - bool OnPacketAbandoned(); - - bool HasUnackedPackets() const { return unacked_packets_ > 0; } - - // If a packet is retransmitted by the connection, it will be sent with a - // different packet number. - void OnPacketRetransmitted(int packet_payload_size); - - private: - // The delegate's OnAckNotification() method will be called once we have been - // notified of ACKs for all the packet numbers we are tracking. - // This is not owned by OnAckNotifier and must outlive it. - scoped_refptr<QuicAckListenerInterface> delegate_; - - // The number of unacked packets being tracked. - int unacked_packets_; - - // Number of packets that had to be retransmitted. - int retransmitted_packet_count_; - // Number of bytes that had to be retransmitted. - int retransmitted_byte_count_; - - DISALLOW_COPY_AND_ASSIGN(QuicAckNotifier); -}; - -} // namespace net - -#endif // NET_QUIC_QUIC_ACK_NOTIFIER_H_ diff --git a/net/quic/quic_ack_notifier_manager.cc b/net/quic/quic_ack_notifier_manager.cc deleted file mode 100644 index 6e26abd..0000000 --- a/net/quic/quic_ack_notifier_manager.cc +++ /dev/null @@ -1,118 +0,0 @@ -// Copyright 2013 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "net/quic/quic_ack_notifier_manager.h" - -#include <stddef.h> -#include <list> -#include <map> -#include <utility> -#include <vector> - -#include "base/stl_util.h" -#include "net/quic/quic_ack_listener_interface.h" -#include "net/quic/quic_ack_notifier.h" -#include "net/quic/quic_flags.h" -#include "net/quic/quic_protocol.h" - -namespace net { - -AckNotifierManager::AckNotifierManager() {} - -AckNotifierManager::~AckNotifierManager() { - for (const auto& pair : ack_notifier_map_) { - for (QuicAckNotifier* notifier : pair.second) { - if (notifier->OnPacketAbandoned()) { - delete notifier; - } - } - } -} - -void AckNotifierManager::OnPacketAcked(QuicPacketNumber packet_number, - QuicTime::Delta delta_largest_observed) { - // Inform all the registered AckNotifiers of the new ACK. - auto map_it = ack_notifier_map_.find(packet_number); - if (map_it == ack_notifier_map_.end()) { - // No AckNotifier is interested in this packet number. - return; - } - - // One or more AckNotifiers are registered as interested in this sequence - // number. Iterate through them and call OnAck on each. - for (QuicAckNotifier* ack_notifier : map_it->second) { - if (ack_notifier->OnAck(delta_largest_observed)) { - // If this has resulted in an empty AckNotifer, erase it. - delete ack_notifier; - } - } - - // Remove the packet 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); -} - -void AckNotifierManager::OnPacketRetransmitted( - QuicPacketNumber old_packet_number, - QuicPacketNumber new_packet_number, - int packet_payload_size) { - auto map_it = ack_notifier_map_.find(old_packet_number); - if (map_it == ack_notifier_map_.end()) { - // No AckNotifiers are interested in the old packet number. - return; - } - - // Update the existing QuicAckNotifiers to the new packet number. - AckNotifierList& ack_notifier_list = map_it->second; - for (QuicAckNotifier* ack_notifier : ack_notifier_list) { - ack_notifier->OnPacketRetransmitted(packet_payload_size); - } - - // The old packet number is no longer of interest, copy the updated - // AckNotifiers to the new packet number before deleting the old. - // TODO(rtenneti): use std::move when chromium supports it. - // ack_notifier_map_[new_packet_number] = std::move(ack_notifier_list); - ack_notifier_map_[new_packet_number] = ack_notifier_list; - ack_notifier_map_.erase(map_it); -} - -void AckNotifierManager::OnSerializedPacket( - const SerializedPacket& serialized_packet) { - if (serialized_packet.notifiers.empty()) { - return; - } - // Inform each attached AckNotifier of the packet's serialization. - AckNotifierList& notifier_list = - ack_notifier_map_[serialized_packet.packet_number]; - for (QuicAckNotifier* notifier : serialized_packet.notifiers) { - if (notifier == nullptr) { - LOG(DFATAL) << "AckNotifier should not be nullptr."; - continue; - } - notifier->OnSerializedPacket(); - notifier_list.push_back(notifier); - } -} - -void AckNotifierManager::OnPacketRemoved(QuicPacketNumber packet_number) { - // Determine if there are any notifiers interested in this packet. - auto map_it = ack_notifier_map_.find(packet_number); - if (map_it == ack_notifier_map_.end()) { - return; - } - - // Notify all of the interested notifiers that the packet is abandoned. - for (QuicAckNotifier* ack_notifier : map_it->second) { - DCHECK(ack_notifier); - if (ack_notifier->OnPacketAbandoned()) { - // If this has resulted in an empty AckNotifer, erase it. - delete ack_notifier; - } - } - - // Remove the packet with given packet number from the map. - ack_notifier_map_.erase(map_it); -} - -} // namespace net diff --git a/net/quic/quic_ack_notifier_manager.h b/net/quic/quic_ack_notifier_manager.h deleted file mode 100644 index 79a0b9e..0000000 --- a/net/quic/quic_ack_notifier_manager.h +++ /dev/null @@ -1,75 +0,0 @@ -// Copyright 2013 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef NET_QUIC_QUIC_ACK_NOTIFIER_MANAGER_H_ -#define NET_QUIC_QUIC_ACK_NOTIFIER_MANAGER_H_ - -#include <list> -#include <map> - -#include "base/containers/hash_tables.h" -#include "net/quic/quic_protocol.h" - -namespace net { - -class QuicAckNotifier; - -namespace test { -class AckNotifierManagerPeer; -} - -// The AckNotifierManager is used by the QuicSentPacketManager to keep track of -// all the AckNotifiers currently active. It owns the AckNotifiers which it gets -// from the serialized packets passed into OnSerializedPacket. It maintains both -// a set of AckNotifiers and a map from packet number to AckNotifier the sake -// of efficiency - we can quickly check the map to see if any AckNotifiers are -// interested in a given packet number. -class NET_EXPORT_PRIVATE AckNotifierManager { - public: - AckNotifierManager(); - virtual ~AckNotifierManager(); - - // Called when the connection receives a new AckFrame. If |packet_number| - // exists in ack_notifier_map_ then the corresponding AckNotifiers will have - // their OnAck method called. - void OnPacketAcked(QuicPacketNumber packet_number, - QuicTime::Delta delta_largest_observed); - - // If a packet has been retransmitted with a new packet number, then this - // will be called. It updates the mapping in ack_notifier_map_, and also - // updates the internal set of packet numbers in each matching AckNotifier. - void OnPacketRetransmitted(QuicPacketNumber old_packet_number, - QuicPacketNumber new_packet_number, - int packet_payload_size); - - // This is called after a packet has been serialized, is ready to be sent, and - // contains retransmittable frames (which may have associated AckNotifiers). - // If any of the retransmittable frames included in |serialized_packet| have - // AckNotifiers registered, then add them to our internal map and additionally - // inform the AckNotifier of the packet number which it should track. - void OnSerializedPacket(const SerializedPacket& serialized_packet); - - // This method is invoked when a packet is removed from the list of unacked - // packets, and it is no longer necessary to keep track of the notifier. - void OnPacketRemoved(QuicPacketNumber packet_number); - - private: - friend class test::AckNotifierManagerPeer; - - typedef std::list<QuicAckNotifier*> AckNotifierList; - // TODO(ianswett): Further improvement may come from changing this to a deque. - typedef base::hash_map<QuicPacketNumber, AckNotifierList> AckNotifierMap; - - // Maps from packet number to the AckNotifiers which are registered - // for that packet number. On receipt of an ACK for a given sequence - // number, call OnAck for all mapped AckNotifiers. - // When the last reference is removed from the map, the notifier is deleted. - AckNotifierMap ack_notifier_map_; - - DISALLOW_COPY_AND_ASSIGN(AckNotifierManager); -}; - -} // namespace net - -#endif // NET_QUIC_QUIC_ACK_NOTIFIER_MANAGER_H_ diff --git a/net/quic/quic_ack_notifier_manager_test.cc b/net/quic/quic_ack_notifier_manager_test.cc deleted file mode 100644 index ef9ca2f4..0000000 --- a/net/quic/quic_ack_notifier_manager_test.cc +++ /dev/null @@ -1,179 +0,0 @@ -// Copyright (c) 2015 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "net/quic/quic_ack_notifier_manager.h" - -#include "net/quic/quic_ack_notifier.h" -#include "net/quic/quic_flags.h" -#include "net/quic/quic_protocol.h" -#include "net/quic/test_tools/quic_ack_notifier_manager_peer.h" -#include "net/quic/test_tools/quic_test_utils.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace net { -namespace test { -namespace { - -// Test fixture for testing AckNotifierManager. Instantiates a manager and -// provides shared code for adding notifiers and verifying the contents of the -// manager. -class QuicAckNotifierManagerTest : public ::testing::Test { - protected: - AckNotifierManager manager_; - scoped_refptr<MockAckNotifierDelegate> delegate_; - QuicTime::Delta zero_; - bool save_FLAGS_quic_no_ack_notifier_; - - QuicAckNotifierManagerTest() : zero_(QuicTime::Delta::Zero()) { - delegate_ = new MockAckNotifierDelegate; - // This test is obsolete once this flag is deprecated. - save_FLAGS_quic_no_ack_notifier_ = FLAGS_quic_no_ack_notifier; - FLAGS_quic_no_ack_notifier = false; - } - - ~QuicAckNotifierManagerTest() override { - FLAGS_quic_no_ack_notifier = save_FLAGS_quic_no_ack_notifier_; - } - - size_t CountPackets() const { - return AckNotifierManagerPeer::GetNumberOfRegisteredPackets(&manager_); - } - - // Add a mock packet with specified parameters. The packet with given - // packet number must not exist in the map before. - void AddPacket(QuicPacketNumber packet_number, bool retransmittable) { - // Create a mock packet. - RetransmittableFrames frames(ENCRYPTION_NONE); - SerializedPacket packet(packet_number, PACKET_4BYTE_PACKET_NUMBER, - /*packet=*/nullptr, - /*entropy_hash=*/0, - retransmittable ? &frames : nullptr, - /*has_ack=*/false, - /*has_stop_waiting=*/false); - - // Create and register a notifier. Normally, this would be created by - // QuicPacketGenerator. - QuicAckNotifier* notifier = new QuicAckNotifier(delegate_.get()); - packet.notifiers.push_back(notifier); - - // Ensure that exactly one packet is added. - const size_t old_packet_count = CountPackets(); - - // Actually add the packet. - manager_.OnSerializedPacket(packet); - - // Ensure the change in the number of packets. - EXPECT_EQ(old_packet_count + 1, CountPackets()); - } -}; - -// This test verifies that QuicAckNotifierManager can handle the trivial case of -// received packet notification. -TEST_F(QuicAckNotifierManagerTest, SimpleAck) { - AddPacket(1, false); - AddPacket(2, true); - - EXPECT_CALL(*delegate_, OnAckNotification(0, 0, zero_)).Times(2); - manager_.OnPacketAcked(1, zero_); - EXPECT_EQ(1u, CountPackets()); - manager_.OnPacketAcked(2, zero_); - EXPECT_EQ(0u, CountPackets()); - - manager_.OnPacketRemoved(1); - manager_.OnPacketRemoved(2); - EXPECT_EQ(0u, CountPackets()); -} - -// This test verifies that QuicAckNotifierManager can correctly handle the case -// when some of the packets are lost, which causes retransmission and removal -// from the unacked packet map. -TEST_F(QuicAckNotifierManagerTest, AckWithLosses) { - const size_t retransmitted_packet_size = kDefaultMaxPacketSize; - - // Here, we simulate the following scenario: - // 1. We send packets 1 to 5, where only odd-numbered packets are - // retransmittable. - // 2. The peer acks 1, 2 and 5, but not 3 and 4. - // 3. We retransmit 3 as 6. - // 4. We remove 1 and 2, since we no longer care about them. - // 4. The peer acks 6. - // 5. We remove packets 3 to 6. - - // Step 1: send five packets. - AddPacket(1, true); - AddPacket(2, false); - AddPacket(3, true); - AddPacket(4, false); - AddPacket(5, true); - - // Step 2: handle acks from peer. - EXPECT_CALL(*delegate_, OnAckNotification(0, 0, zero_)).Times(3); - manager_.OnPacketAcked(1, zero_); - EXPECT_EQ(4u, CountPackets()); - manager_.OnPacketAcked(2, zero_); - EXPECT_EQ(3u, CountPackets()); - manager_.OnPacketAcked(5, zero_); - EXPECT_EQ(2u, CountPackets()); - - // Step 3: retransmit 3 as 6. - manager_.OnPacketRetransmitted(3, 6, retransmitted_packet_size); - EXPECT_EQ(2u, CountPackets()); - - // Step 4: remove 1 and 2. - manager_.OnPacketRemoved(1); - manager_.OnPacketRemoved(2); - EXPECT_EQ(2u, CountPackets()); - - // Step 4: ack packet 6. - EXPECT_CALL(*delegate_, - OnAckNotification(1, retransmitted_packet_size, zero_)).Times(1); - manager_.OnPacketAcked(6, zero_); - EXPECT_EQ(1u, CountPackets()); - - // Step 5: remove all packets. This causes packet 4 to be dropped from the - // map. - manager_.OnPacketRemoved(3); - manager_.OnPacketRemoved(4); - manager_.OnPacketRemoved(5); - manager_.OnPacketRemoved(6); - EXPECT_EQ(0u, CountPackets()); -} - -// This test verifies that the QuicAckNotifierManager behaves correctly when -// there are many retransmissions. -TEST_F(QuicAckNotifierManagerTest, RepeatedRetransmission) { - AddPacket(1, true); - - const size_t packet_size = kDefaultMaxPacketSize; - const size_t times_lost = 100; - const size_t total_size_lost = packet_size * times_lost; - const QuicPacketNumber last_packet = times_lost + 1; - - // Retransmit the packet many times. - for (size_t packet_number = 1; packet_number < last_packet; packet_number++) { - manager_.OnPacketRetransmitted(packet_number, packet_number + 1, - packet_size); - EXPECT_EQ(1u, CountPackets()); - } - - // Remove all lost packets. - for (QuicPacketNumber packet = 1; packet < last_packet; packet++) { - manager_.OnPacketRemoved(packet); - } - EXPECT_EQ(1u, CountPackets()); - - // Finally get the packet acknowledged. - EXPECT_CALL(*delegate_, OnAckNotification(times_lost, total_size_lost, zero_)) - .Times(1); - manager_.OnPacketAcked(last_packet, zero_); - EXPECT_EQ(0u, CountPackets()); - - // Remove the last packet. - manager_.OnPacketRemoved(last_packet); - EXPECT_EQ(0u, CountPackets()); -} - -} // namespace -} // namespace test -} // namespace net diff --git a/net/quic/quic_ack_notifier_test.cc b/net/quic/quic_ack_notifier_test.cc deleted file mode 100644 index d57235f..0000000 --- a/net/quic/quic_ack_notifier_test.cc +++ /dev/null @@ -1,88 +0,0 @@ -// Copyright 2013 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "net/quic/quic_ack_notifier.h" - -#include "net/quic/test_tools/quic_test_utils.h" -#include "testing/gmock/include/gmock/gmock.h" -#include "testing/gtest/include/gtest/gtest.h" - -using testing::_; - -namespace net { -namespace test { -namespace { - -class QuicAckNotifierTest : public ::testing::Test { - protected: - QuicAckNotifierTest() : zero_(QuicTime::Delta::Zero()) {} - - void SetUp() override { - delegate_ = new MockAckNotifierDelegate; - notifier_.reset(new QuicAckNotifier(delegate_)); - - notifier_->OnSerializedPacket(); - notifier_->OnSerializedPacket(); - notifier_->OnSerializedPacket(); - } - - MockAckNotifierDelegate* delegate_; - scoped_ptr<QuicAckNotifier> notifier_; - QuicTime::Delta zero_; -}; - -// Should trigger callback when we receive acks for all the registered packet -// numbers. -TEST_F(QuicAckNotifierTest, TriggerCallback) { - EXPECT_CALL(*delegate_, OnAckNotification(0, 0, zero_)).Times(1); - EXPECT_FALSE(notifier_->OnAck(zero_)); - EXPECT_FALSE(notifier_->OnAck(zero_)); - EXPECT_TRUE(notifier_->OnAck(zero_)); -} - -// Should not trigger callback if we never provide all the packet numbers. -TEST_F(QuicAckNotifierTest, DoesNotTrigger) { - // Should not trigger callback as not all packets have been seen. - EXPECT_CALL(*delegate_, OnAckNotification(_, _, _)).Times(0); - EXPECT_FALSE(notifier_->OnAck(zero_)); - EXPECT_FALSE(notifier_->OnAck(zero_)); -} - -// Should not trigger callback if we abandon all three packets. -TEST_F(QuicAckNotifierTest, AbandonDoesNotTrigger) { - // Should not trigger callback as not all packets have been seen. - EXPECT_CALL(*delegate_, OnAckNotification(_, _, _)).Times(0); - EXPECT_FALSE(notifier_->OnPacketAbandoned()); - EXPECT_FALSE(notifier_->OnPacketAbandoned()); - EXPECT_TRUE(notifier_->OnPacketAbandoned()); -} - -// Should trigger even after updating packet numbers and receiving ACKs for -// new packet numbers. -TEST_F(QuicAckNotifierTest, UpdatePacketNumbers) { - // Update a couple of the packet numbers (i.e. retransmitted packets) - notifier_->OnPacketRetransmitted(20); - notifier_->OnPacketRetransmitted(3); - - EXPECT_CALL(*delegate_, OnAckNotification(2, 20 + 3, _)).Times(1); - EXPECT_FALSE(notifier_->OnAck(zero_)); // original - EXPECT_FALSE(notifier_->OnAck(zero_)); // updated - EXPECT_TRUE(notifier_->OnAck(zero_)); // updated -} - -// Make sure the delegate is called with the delta time from the last ACK. -TEST_F(QuicAckNotifierTest, DeltaTime) { - const QuicTime::Delta first_delta = QuicTime::Delta::FromSeconds(5); - const QuicTime::Delta second_delta = QuicTime::Delta::FromSeconds(33); - const QuicTime::Delta third_delta = QuicTime::Delta::FromSeconds(10); - - EXPECT_CALL(*delegate_, OnAckNotification(0, 0, third_delta)).Times(1); - EXPECT_FALSE(notifier_->OnAck(first_delta)); - EXPECT_FALSE(notifier_->OnAck(second_delta)); - EXPECT_TRUE(notifier_->OnAck(third_delta)); -} - -} // namespace -} // namespace test -} // namespace net diff --git a/net/quic/quic_chromium_client_session.cc b/net/quic/quic_chromium_client_session.cc index c5b49a0..b7ac6d1ff 100644 --- a/net/quic/quic_chromium_client_session.cc +++ b/net/quic/quic_chromium_client_session.cc @@ -675,6 +675,11 @@ void QuicChromiumClientSession::OnGoAway(const QuicGoAwayFrame& frame) { NotifyFactoryOfSessionGoingAway(); } +void QuicChromiumClientSession::OnRstStream(const QuicRstStreamFrame& frame) { + QuicSession::OnRstStream(frame); + OnClosedStream(); +} + void QuicChromiumClientSession::OnConnectionClosed(QuicErrorCode error, bool from_peer) { DCHECK(!connection()->connected()); @@ -933,7 +938,7 @@ void QuicChromiumClientSession::NotifyFactoryOfSessionClosedLater() { RecordUnexpectedNotGoingAway(NOTIFY_FACTORY_OF_SESSION_CLOSED_LATER); going_away_ = true; - DCHECK_EQ(0u, GetNumOpenStreams()); + DCHECK_EQ(0u, GetNumActiveStreams()); DCHECK(!connection()->connected()); base::ThreadTaskRunnerHandle::Get()->PostTask( FROM_HERE, @@ -949,7 +954,7 @@ void QuicChromiumClientSession::NotifyFactoryOfSessionClosed() { RecordUnexpectedNotGoingAway(NOTIFY_FACTORY_OF_SESSION_CLOSED); going_away_ = true; - DCHECK_EQ(0u, GetNumOpenStreams()); + DCHECK_EQ(0u, GetNumActiveStreams()); // Will delete |this|. if (stream_factory_) stream_factory_->OnSessionClosed(this); diff --git a/net/quic/quic_chromium_client_session.h b/net/quic/quic_chromium_client_session.h index 3f04dee..b269684 100644 --- a/net/quic/quic_chromium_client_session.h +++ b/net/quic/quic_chromium_client_session.h @@ -158,6 +158,7 @@ class NET_EXPORT_PRIVATE QuicChromiumClientSession void OnCryptoHandshakeMessageReceived( const CryptoHandshakeMessage& message) override; void OnGoAway(const QuicGoAwayFrame& frame) override; + void OnRstStream(const QuicRstStreamFrame& frame) override; // QuicClientSessionBase methods: void OnProofValid(const QuicCryptoClientConfig::CachedState& cached) override; diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc index 8095961..180e196 100644 --- a/net/quic/quic_connection.cc +++ b/net/quic/quic_connection.cc @@ -201,13 +201,6 @@ class MtuDiscoveryAckListener : public QuicAckListenerInterface { MtuDiscoveryAckListener(QuicConnection* connection, QuicByteCount probe_size) : connection_(connection), probe_size_(probe_size) {} - void OnAckNotification(int /*num_retransmittable_packets*/, - int /*num_retransmittable_bytes*/, - QuicTime::Delta /*delta_largest_observed*/) override { - // Since the probe was successful, increase the maximum packet size to that. - MaybeIncreaseMtu(); - } - void OnPacketAcked(int /*acked_bytes*/, QuicTime::Delta /*delta_largest_observed*/) override { // MTU discovery packets are not retransmittable, so it must be acked. @@ -341,9 +334,6 @@ QuicConnection::QuicConnection(QuicConnectionId connection_id, SetMaxPacketLength(perspective_ == Perspective::IS_SERVER ? kDefaultServerMaxPacketSize : kDefaultMaxPacketSize); - const bool no_acknotifier = FLAGS_quic_no_ack_notifier; - packet_generator_.set_no_acknotifier(no_acknotifier); - sent_packet_manager_.set_no_acknotifier(no_acknotifier); } QuicConnection::~QuicConnection() { @@ -1130,11 +1120,11 @@ void QuicConnection::SendVersionNegotiationPacket() { QuicConsumedData QuicConnection::SendStreamData( QuicStreamId id, - const QuicIOVector& iov, + QuicIOVector iov, QuicStreamOffset offset, bool fin, FecProtection fec_protection, - QuicAckListenerInterface* delegate) { + QuicAckListenerInterface* listener) { if (!fin && iov.total_length == 0) { LOG(DFATAL) << "Attempt to send empty stream frame"; return QuicConsumedData(0, false); @@ -1156,7 +1146,7 @@ QuicConsumedData QuicConnection::SendStreamData( ScopedRetransmissionScheduler alarm_delayer(this); ScopedPacketBundler ack_bundler(this, BUNDLE_PENDING_ACK); return packet_generator_.ConsumeData(id, iov, offset, fin, fec_protection, - delegate); + listener); } void QuicConnection::SendRstStream(QuicStreamId id, @@ -1670,7 +1660,6 @@ void QuicConnection::OnSerializedPacket( CloseConnection(QUIC_ENCRYPTION_FAILURE, false); return; } - sent_packet_manager_.OnSerializedPacket(serialized_packet); if (serialized_packet.is_fec_packet && fec_alarm_->IsSet()) { // If an FEC packet is serialized with the FEC alarm set, cancel the alarm. fec_alarm_->Cancel(); diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h index f4f3cb5..8399747 100644 --- a/net/quic/quic_connection.h +++ b/net/quic/quic_connection.h @@ -30,8 +30,6 @@ #include "base/strings/string_piece.h" #include "net/base/ip_endpoint.h" #include "net/quic/crypto/quic_decrypter.h" -#include "net/quic/quic_ack_notifier.h" -#include "net/quic/quic_ack_notifier_manager.h" #include "net/quic/quic_alarm.h" #include "net/quic/quic_blocked_writer_interface.h" #include "net/quic/quic_connection_stats.h" @@ -320,15 +318,15 @@ class NET_EXPORT_PRIVATE QuicConnection // data is to be FEC protected. Note that data that is sent immediately // following MUST_FEC_PROTECT data may get protected by falling within the // same FEC group. - // If |delegate| is provided, then it will be informed once ACKs have been + // If |listener| is provided, then it will be informed once ACKs have been // received for all the packets written in this call. - // The |delegate| is not owned by the QuicConnection and must outlive it. + // The |listener| is not owned by the QuicConnection and must outlive it. QuicConsumedData SendStreamData(QuicStreamId id, - const QuicIOVector& iov, + QuicIOVector iov, QuicStreamOffset offset, bool fin, FecProtection fec_protection, - QuicAckListenerInterface* delegate); + QuicAckListenerInterface* listener); // Send a RST_STREAM frame to the peer. virtual void SendRstStream(QuicStreamId id, diff --git a/net/quic/quic_connection_logger.cc b/net/quic/quic_connection_logger.cc index 22a9aae..c24bb6e 100644 --- a/net/quic/quic_connection_logger.cc +++ b/net/quic/quic_connection_logger.cc @@ -138,8 +138,8 @@ scoped_ptr<base::Value> NetLogQuicAckFrameCallback( base::ListValue* received = new base::ListValue(); dict->Set("received_packet_times", received); - const PacketTimeList& received_times = frame->received_packet_times; - for (PacketTimeList::const_iterator it = received_times.begin(); + const PacketTimeVector& received_times = frame->received_packet_times; + for (PacketTimeVector::const_iterator it = received_times.begin(); it != received_times.end(); ++it) { base::DictionaryValue* info = new base::DictionaryValue(); info->SetInteger("packet_number", static_cast<int>(it->first)); diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc index 7a0b199..2ce6c83 100644 --- a/net/quic/quic_connection_test.cc +++ b/net/quic/quic_connection_test.cc @@ -16,7 +16,6 @@ #include "net/quic/crypto/null_encrypter.h" #include "net/quic/crypto/quic_decrypter.h" #include "net/quic/crypto/quic_encrypter.h" -#include "net/quic/quic_ack_notifier.h" #include "net/quic/quic_flags.h" #include "net/quic/quic_protocol.h" #include "net/quic/quic_utils.h" @@ -470,9 +469,9 @@ class TestConnection : public QuicConnection { StringPiece data, QuicStreamOffset offset, bool fin, - QuicAckListenerInterface* delegate) { + QuicAckListenerInterface* listener) { return SendStreamDataWithStringHelper(id, data, offset, fin, - MAY_FEC_PROTECT, delegate); + MAY_FEC_PROTECT, listener); } QuicConsumedData SendStreamDataWithStringWithFec( @@ -480,9 +479,9 @@ class TestConnection : public QuicConnection { StringPiece data, QuicStreamOffset offset, bool fin, - QuicAckListenerInterface* delegate) { + QuicAckListenerInterface* listener) { return SendStreamDataWithStringHelper(id, data, offset, fin, - MUST_FEC_PROTECT, delegate); + MUST_FEC_PROTECT, listener); } QuicConsumedData SendStreamDataWithStringHelper( @@ -491,11 +490,11 @@ class TestConnection : public QuicConnection { QuicStreamOffset offset, bool fin, FecProtection fec_protection, - QuicAckListenerInterface* delegate) { + QuicAckListenerInterface* listener) { struct iovec iov; QuicIOVector data_iov(MakeIOVector(data, &iov)); return QuicConnection::SendStreamData(id, data_iov, offset, fin, - fec_protection, delegate); + fec_protection, listener); } QuicConsumedData SendStreamData3() { @@ -4462,16 +4461,12 @@ TEST_P(QuicConnectionTest, ConnectionCloseWhenWriteBlocked) { TEST_P(QuicConnectionTest, AckNotifierTriggerCallback) { EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); - // Create a delegate which we expect to be called. - scoped_refptr<MockAckNotifierDelegate> delegate(new MockAckNotifierDelegate); - if (FLAGS_quic_no_ack_notifier) { - EXPECT_CALL(*delegate.get(), OnPacketAcked(_, _)).Times(1); - } else { - EXPECT_CALL(*delegate.get(), OnAckNotification(_, _, _)).Times(1); - } + // Create a listener which we expect to be called. + scoped_refptr<MockAckListener> listener(new MockAckListener); + EXPECT_CALL(*listener, OnPacketAcked(_, _)).Times(1); - // Send some data, which will register the delegate to be notified. - connection_.SendStreamDataWithString(1, "foo", 0, !kFin, delegate.get()); + // Send some data, which will register the listener to be notified. + connection_.SendStreamDataWithString(1, "foo", 0, !kFin, listener.get()); // Process an ACK from the server which should trigger the callback. EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _)); @@ -4482,17 +4477,13 @@ TEST_P(QuicConnectionTest, AckNotifierTriggerCallback) { TEST_P(QuicConnectionTest, AckNotifierFailToTriggerCallback) { EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); - // Create a delegate which we don't expect to be called. - scoped_refptr<MockAckNotifierDelegate> delegate(new MockAckNotifierDelegate); - if (FLAGS_quic_no_ack_notifier) { - EXPECT_CALL(*delegate.get(), OnPacketAcked(_, _)).Times(0); - } else { - EXPECT_CALL(*delegate.get(), OnAckNotification(_, _, _)).Times(0); - } + // Create a listener which we don't expect to be called. + scoped_refptr<MockAckListener> listener(new MockAckListener); + EXPECT_CALL(*listener, OnPacketAcked(_, _)).Times(0); - // Send some data, which will register the delegate to be notified. This will - // not be ACKed and so the delegate should never be called. - connection_.SendStreamDataWithString(1, "foo", 0, !kFin, delegate.get()); + // Send some data, which will register the listener to be notified. This will + // not be ACKed and so the listener should never be called. + connection_.SendStreamDataWithString(1, "foo", 0, !kFin, listener.get()); // Send some other data which we will ACK. connection_.SendStreamDataWithString(1, "foo", 0, !kFin, nullptr); @@ -4513,18 +4504,14 @@ TEST_P(QuicConnectionTest, AckNotifierFailToTriggerCallback) { TEST_P(QuicConnectionTest, AckNotifierCallbackAfterRetransmission) { EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); - // Create a delegate which we expect to be called. - scoped_refptr<MockAckNotifierDelegate> delegate(new MockAckNotifierDelegate); - if (FLAGS_quic_no_ack_notifier) { - EXPECT_CALL(*delegate.get(), OnPacketRetransmitted(3)).Times(1); - EXPECT_CALL(*delegate.get(), OnPacketAcked(3, _)).Times(1); - } else { - EXPECT_CALL(*delegate.get(), OnAckNotification(_, _, _)).Times(1); - } + // Create a listener which we expect to be called. + scoped_refptr<MockAckListener> listener(new MockAckListener); + EXPECT_CALL(*listener, OnPacketRetransmitted(3)).Times(1); + EXPECT_CALL(*listener, OnPacketAcked(3, _)).Times(1); // Send four packets, and register to be notified on ACK of packet 2. connection_.SendStreamDataWithString(3, "foo", 0, !kFin, nullptr); - connection_.SendStreamDataWithString(3, "bar", 0, !kFin, delegate.get()); + connection_.SendStreamDataWithString(3, "bar", 0, !kFin, listener.get()); connection_.SendStreamDataWithString(3, "baz", 0, !kFin, nullptr); connection_.SendStreamDataWithString(3, "qux", 0, !kFin, nullptr); @@ -4552,13 +4539,12 @@ TEST_P(QuicConnectionTest, AckNotifierCallbackAfterRetransmission) { // out and was retransmitted, even though the retransmission has a // different packet number. TEST_P(QuicConnectionTest, AckNotifierCallbackForAckAfterRTO) { - // Create a delegate which we expect to be called. - scoped_refptr<MockAckNotifierDelegate> delegate( - new StrictMock<MockAckNotifierDelegate>); + // Create a listener which we expect to be called. + scoped_refptr<MockAckListener> listener(new StrictMock<MockAckListener>); QuicTime default_retransmission_time = clock_.ApproximateNow().Add( DefaultRetransmissionTime()); - connection_.SendStreamDataWithString(3, "foo", 0, !kFin, delegate.get()); + connection_.SendStreamDataWithString(3, "foo", 0, !kFin, listener.get()); EXPECT_EQ(1u, stop_waiting()->least_unacked); EXPECT_EQ(1u, writer_->header().packet_number); @@ -4566,9 +4552,7 @@ TEST_P(QuicConnectionTest, AckNotifierCallbackForAckAfterRTO) { connection_.GetRetransmissionAlarm()->deadline()); // Simulate the retransmission alarm firing. clock_.AdvanceTime(DefaultRetransmissionTime()); - if (FLAGS_quic_no_ack_notifier) { - EXPECT_CALL(*delegate.get(), OnPacketRetransmitted(3)); - } + EXPECT_CALL(*listener, OnPacketRetransmitted(3)); EXPECT_CALL(*send_algorithm_, OnPacketSent(_, _, 2u, _, _)); connection_.GetRetransmissionAlarm()->Fire(); EXPECT_EQ(2u, writer_->header().packet_number); @@ -4577,16 +4561,12 @@ TEST_P(QuicConnectionTest, AckNotifierCallbackForAckAfterRTO) { // Ack the original packet, which will revert the RTO. EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); - if (FLAGS_quic_no_ack_notifier) { - EXPECT_CALL(*delegate.get(), OnPacketAcked(3, _)); - } else { - EXPECT_CALL(*delegate.get(), OnAckNotification(1, _, _)); - } + EXPECT_CALL(*listener, OnPacketAcked(3, _)); EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _)); QuicAckFrame ack_frame = InitAckFrame(1); ProcessAckPacket(&ack_frame); - // Delegate is not notified again when the retransmit is acked. + // listener is not notified again when the retransmit is acked. EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _)); QuicAckFrame second_ack_frame = InitAckFrame(2); ProcessAckPacket(&second_ack_frame); @@ -4596,13 +4576,12 @@ TEST_P(QuicConnectionTest, AckNotifierCallbackForAckAfterRTO) { // previously nacked, even though the retransmission has a different // packet number. TEST_P(QuicConnectionTest, AckNotifierCallbackForAckOfNackedPacket) { - // Create a delegate which we expect to be called. - scoped_refptr<MockAckNotifierDelegate> delegate( - new StrictMock<MockAckNotifierDelegate>); + // Create a listener which we expect to be called. + scoped_refptr<MockAckListener> listener(new StrictMock<MockAckListener>); // Send four packets, and register to be notified on ACK of packet 2. connection_.SendStreamDataWithString(3, "foo", 0, !kFin, nullptr); - connection_.SendStreamDataWithString(3, "bar", 0, !kFin, delegate.get()); + connection_.SendStreamDataWithString(3, "bar", 0, !kFin, listener.get()); connection_.SendStreamDataWithString(3, "baz", 0, !kFin, nullptr); connection_.SendStreamDataWithString(3, "qux", 0, !kFin, nullptr); @@ -4611,9 +4590,7 @@ TEST_P(QuicConnectionTest, AckNotifierCallbackForAckOfNackedPacket) { NackPacket(2, &frame); PacketNumberSet lost_packets; lost_packets.insert(2); - if (FLAGS_quic_no_ack_notifier) { - EXPECT_CALL(*delegate.get(), OnPacketRetransmitted(_)); - } + EXPECT_CALL(*listener, OnPacketRetransmitted(_)); EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); EXPECT_CALL(*loss_algorithm_, DetectLostPackets(_, _, _, _)) .WillOnce(Return(lost_packets)); @@ -4623,17 +4600,13 @@ TEST_P(QuicConnectionTest, AckNotifierCallbackForAckOfNackedPacket) { // Now we get an ACK for packet 2, which was previously nacked. PacketNumberSet no_lost_packets; - if (FLAGS_quic_no_ack_notifier) { - EXPECT_CALL(*delegate.get(), OnPacketAcked(3, _)); - } else { - EXPECT_CALL(*delegate.get(), OnAckNotification(1, _, _)); - } + EXPECT_CALL(*listener, OnPacketAcked(3, _)); EXPECT_CALL(*loss_algorithm_, DetectLostPackets(_, _, _, _)) .WillOnce(Return(no_lost_packets)); QuicAckFrame second_ack_frame = InitAckFrame(4); ProcessAckPacket(&second_ack_frame); - // Verify that the delegate is not notified again when the + // Verify that the listener is not notified again when the // retransmit is acked. EXPECT_CALL(*loss_algorithm_, DetectLostPackets(_, _, _, _)) .WillOnce(Return(no_lost_packets)); @@ -4645,16 +4618,11 @@ TEST_P(QuicConnectionTest, AckNotifierCallbackForAckOfNackedPacket) { TEST_P(QuicConnectionTest, AckNotifierFECTriggerCallback) { EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); - // Create a delegate which we expect to be called. - scoped_refptr<MockAckNotifierDelegate> delegate(new MockAckNotifierDelegate); - if (FLAGS_quic_no_ack_notifier) { - EXPECT_CALL(*delegate.get(), OnPacketAcked(_, _)).Times(1); - } else { - EXPECT_CALL(*delegate.get(), OnAckNotification(_, _, _)).Times(1); - } - - // Send some data, which will register the delegate to be notified. - connection_.SendStreamDataWithString(1, "foo", 0, !kFin, delegate.get()); + // Create a listener which we expect to be called. + scoped_refptr<MockAckListener> listener(new MockAckListener); + EXPECT_CALL(*listener, OnPacketAcked(_, _)).Times(1); + // Send some data, which will register the listener to be notified. + connection_.SendStreamDataWithString(1, "foo", 0, !kFin, listener.get()); connection_.SendStreamDataWithString(2, "bar", 0, !kFin, nullptr); // Process an ACK from the server with a revived packet, which should trigger @@ -4672,19 +4640,15 @@ TEST_P(QuicConnectionTest, AckNotifierCallbackAfterFECRecovery) { EXPECT_CALL(visitor_, OnSuccessfulVersionNegotiation(_)); EXPECT_CALL(visitor_, OnCanWrite()); - // Create a delegate which we expect to be called. - scoped_refptr<MockAckNotifierDelegate> delegate(new MockAckNotifierDelegate); - if (FLAGS_quic_no_ack_notifier) { - EXPECT_CALL(*delegate.get(), OnPacketAcked(_, _)).Times(1); - } else { - EXPECT_CALL(*delegate.get(), OnAckNotification(_, _, _)).Times(1); - } + // Create a listener which we expect to be called. + scoped_refptr<MockAckListener> listener(new MockAckListener); + EXPECT_CALL(*listener, OnPacketAcked(_, _)).Times(1); // Expect ACKs for 1 packet. EXPECT_CALL(*send_algorithm_, OnCongestionEvent(true, _, _, _)); // Send one packet, and register to be notified on ACK. - connection_.SendStreamDataWithString(1, "foo", 0, !kFin, delegate.get()); + connection_.SendStreamDataWithString(1, "foo", 0, !kFin, listener.get()); // Ack packet gets dropped, but we receive an FEC packet that covers it. // Should recover the Ack packet and trigger the notification callback. @@ -4807,9 +4771,9 @@ TEST_P(QuicConnectionTest, NoDataNoFin) { // Make sure that a call to SendStreamWithData, with no data and no FIN, does // not result in a QuicAckNotifier being used-after-free (fail under ASAN). // Regression test for b/18594622 - scoped_refptr<MockAckNotifierDelegate> delegate(new MockAckNotifierDelegate); + scoped_refptr<MockAckListener> listener(new MockAckListener); EXPECT_DFATAL( - connection_.SendStreamDataWithString(3, "", 0, !kFin, delegate.get()), + connection_.SendStreamDataWithString(3, "", 0, !kFin, listener.get()), "Attempt to send empty stream frame"); } diff --git a/net/quic/quic_crypto_server_stream.cc b/net/quic/quic_crypto_server_stream.cc index 1aa3cf7..d475833 100644 --- a/net/quic/quic_crypto_server_stream.cc +++ b/net/quic/quic_crypto_server_stream.cc @@ -20,13 +20,6 @@ using std::string; namespace net { -void ServerHelloNotifier::OnAckNotification( - int num_retransmitted_packets, - int num_retransmitted_bytes, - QuicTime::Delta delta_largest_observed) { - server_stream_->OnServerHelloAcked(); -} - void ServerHelloNotifier::OnPacketAcked( int acked_bytes, QuicTime::Delta delta_largest_observed) { @@ -116,6 +109,17 @@ void QuicCryptoServerStream::FinishProcessingHandshakeMessage( if (reply.tag() != kSHLO) { SendHandshakeMessage(reply); + + if (FLAGS_enable_quic_stateless_reject_support && reply.tag() == kSREJ) { + DCHECK(use_stateless_rejects_if_peer_supported()); + DCHECK(peer_supports_stateless_rejects()); + DCHECK(!handshake_confirmed()); + DVLOG(1) << "Closing connection " + << session()->connection()->connection_id() + << " because of a stateless reject."; + session()->connection()->CloseConnection( + QUIC_CRYPTO_HANDSHAKE_STATELESS_REJECT, /* from_peer */ false); + } return; } diff --git a/net/quic/quic_crypto_server_stream.h b/net/quic/quic_crypto_server_stream.h index fd1b266..dfd8c95 100644 --- a/net/quic/quic_crypto_server_stream.h +++ b/net/quic/quic_crypto_server_stream.h @@ -33,11 +33,6 @@ class NET_EXPORT_PRIVATE ServerHelloNotifier : public QuicAckListenerInterface { explicit ServerHelloNotifier(QuicCryptoServerStream* stream) : server_stream_(stream) {} - // QuicAckListenerInterface implementation - void OnAckNotification(int num_retransmitted_packets, - int num_retransmitted_bytes, - QuicTime::Delta delta_largest_observed) override; - void OnPacketAcked(int acked_bytes, QuicTime::Delta delta_largest_observed) override; diff --git a/net/quic/quic_crypto_server_stream_test.cc b/net/quic/quic_crypto_server_stream_test.cc index af4ff42..af3bcb3 100644 --- a/net/quic/quic_crypto_server_stream_test.cc +++ b/net/quic/quic_crypto_server_stream_test.cc @@ -236,6 +236,7 @@ TEST_P(QuicCryptoServerStreamTest, StatelessRejectAfterCHLO) { EXPECT_EQ(expected_id, server_designated_connection_id); EXPECT_FALSE(client_state->has_server_designated_connection_id()); ASSERT_TRUE(client_state->IsComplete(QuicWallTime::FromUNIXSeconds(0))); + EXPECT_FALSE(server_connection_->connected()); } TEST_P(QuicCryptoServerStreamTest, ConnectedAfterStatelessHandshake) { diff --git a/net/quic/quic_crypto_stream.cc b/net/quic/quic_crypto_stream.cc index 26c966a..0778760 100644 --- a/net/quic/quic_crypto_stream.cc +++ b/net/quic/quic_crypto_stream.cc @@ -69,12 +69,12 @@ void QuicCryptoStream::SendHandshakeMessage( void QuicCryptoStream::SendHandshakeMessage( const CryptoHandshakeMessage& message, - QuicAckListenerInterface* delegate) { + QuicAckListenerInterface* listener) { DVLOG(1) << ENDPOINT << "Sending " << message.DebugString(); session()->OnCryptoHandshakeMessageSent(message); const QuicData& data = message.GetSerialized(); // TODO(wtc): check the return value. - WriteOrBufferData(string(data.data(), data.length()), false, delegate); + WriteOrBufferData(string(data.data(), data.length()), false, listener); } bool QuicCryptoStream::ExportKeyingMaterial( diff --git a/net/quic/quic_crypto_stream.h b/net/quic/quic_crypto_stream.h index 445ac61..33268a9 100644 --- a/net/quic/quic_crypto_stream.h +++ b/net/quic/quic_crypto_stream.h @@ -47,7 +47,7 @@ class NET_EXPORT_PRIVATE QuicCryptoStream // As above, but registers |delegate| for notification when |message| has been // ACKed by the peer. void SendHandshakeMessage(const CryptoHandshakeMessage& message, - QuicAckListenerInterface* delegate); + QuicAckListenerInterface* listener); // Performs key extraction to derive a new secret of |result_len| bytes // dependent on |label|, |context|, and the stream's negotiated subkey secret. diff --git a/net/quic/quic_flags.cc b/net/quic/quic_flags.cc index bc3f41e..fe97495 100644 --- a/net/quic/quic_flags.cc +++ b/net/quic/quic_flags.cc @@ -103,3 +103,11 @@ bool FLAGS_quic_implement_stop_reading = true; // Invoke the QuicAckListener directly, instead of going through the AckNotifier // and AckNotifierManager. bool FLAGS_quic_no_ack_notifier = true; + +// If true, QuicSession::GetNumOpenStreams will count unfinished +// streams as open streams, QuicSession::PostProcessAfterData will not +// check the quota of unifinished streams. +bool FLAGS_quic_count_unfinished_as_open_streams = true; + +// If true, use the unrolled prefetch path in QuicPacketCreator::CopyToBuffer. +bool FLAGS_quic_packet_creator_prefetch = false; diff --git a/net/quic/quic_flags.h b/net/quic/quic_flags.h index 5deaff4..7b7b0f4 100644 --- a/net/quic/quic_flags.h +++ b/net/quic/quic_flags.h @@ -33,5 +33,7 @@ NET_EXPORT_PRIVATE extern bool FLAGS_quic_disable_pacing; NET_EXPORT_PRIVATE extern bool FLAGS_quic_fix_fin_accounting; NET_EXPORT_PRIVATE extern bool FLAGS_quic_implement_stop_reading; NET_EXPORT_PRIVATE extern bool FLAGS_quic_no_ack_notifier; +NET_EXPORT_PRIVATE extern bool FLAGS_quic_count_unfinished_as_open_streams; +NET_EXPORT_PRIVATE extern bool FLAGS_quic_packet_creator_prefetch; #endif // NET_QUIC_QUIC_FLAGS_H_ diff --git a/net/quic/quic_framer.cc b/net/quic/quic_framer.cc index 41e1498..56aaaad 100644 --- a/net/quic/quic_framer.cc +++ b/net/quic/quic_framer.cc @@ -1413,6 +1413,7 @@ bool QuicFramer::ProcessTimestampsInAckFrame(QuicDataReader* reader, last_timestamp_ = CalculateTimestampFromWire(time_delta_us); + ack_frame->received_packet_times.reserve(num_received_packets); ack_frame->received_packet_times.push_back( std::make_pair(seq_num, creation_time_.Add(last_timestamp_))); @@ -2055,7 +2056,6 @@ bool QuicFramer::AppendTimestampToAckFrame(const QuicAckFrame& frame, } uint8 num_received_packets = frame.received_packet_times.size(); - if (!writer->WriteBytes(&num_received_packets, 1)) { return false; } @@ -2063,7 +2063,7 @@ bool QuicFramer::AppendTimestampToAckFrame(const QuicAckFrame& frame, return true; } - PacketTimeList::const_iterator it = frame.received_packet_times.begin(); + PacketTimeVector::const_iterator it = frame.received_packet_times.begin(); QuicPacketNumber packet_number = it->first; QuicPacketNumber delta_from_largest_observed = frame.largest_observed - packet_number; diff --git a/net/quic/quic_packet_creator.cc b/net/quic/quic_packet_creator.cc index ce354b9..3679d19 100644 --- a/net/quic/quic_packet_creator.cc +++ b/net/quic/quic_packet_creator.cc @@ -9,9 +9,9 @@ #include "base/basictypes.h" #include "base/logging.h" #include "net/quic/crypto/quic_random.h" -#include "net/quic/quic_ack_notifier.h" #include "net/quic/quic_data_writer.h" #include "net/quic/quic_fec_group.h" +#include "net/quic/quic_flags.h" #include "net/quic/quic_utils.h" using base::StringPiece; @@ -261,7 +261,7 @@ size_t QuicPacketCreator::StreamFramePacketOverhead( } size_t QuicPacketCreator::CreateStreamFrame(QuicStreamId id, - const QuicIOVector& iov, + QuicIOVector iov, size_t iov_offset, QuicStreamOffset offset, bool fin, @@ -302,7 +302,7 @@ size_t QuicPacketCreator::CreateStreamFrame(QuicStreamId id, } // static -void QuicPacketCreator::CopyToBuffer(const QuicIOVector& iov, +void QuicPacketCreator::CopyToBuffer(QuicIOVector iov, size_t iov_offset, size_t length, char* buffer) { @@ -311,14 +311,56 @@ void QuicPacketCreator::CopyToBuffer(const QuicIOVector& iov, iov_offset -= iov.iov[iovnum].iov_len; ++iovnum; } - while (iovnum < iov.iov_count && length > 0) { - const size_t copy_len = min(length, iov.iov[iovnum].iov_len - iov_offset); - memcpy(buffer, static_cast<char*>(iov.iov[iovnum].iov_base) + iov_offset, - copy_len); - iov_offset = 0; - length -= copy_len; - buffer += copy_len; - ++iovnum; + DCHECK_LE(iovnum, iov.iov_count); + DCHECK_LE(iov_offset, iov.iov[iovnum].iov_len); + if (FLAGS_quic_packet_creator_prefetch) { + if (iovnum >= iov.iov_count || length == 0) { + return; + } + + // Unroll the first iteration that handles iov_offset. + const size_t iov_available = iov.iov[iovnum].iov_len - iov_offset; + size_t copy_len = min(length, iov_available); + + // Try to prefetch the next iov if there is at least one more after the + // current. Otherwise, it looks like an irregular access that the hardware + // prefetcher won't speculatively prefetch. Only prefetch one iov because + // generally, the iov_offset is not 0, input iov consists of 2K buffers and + // the output buffer is ~1.4K. + if (copy_len == iov_available && iovnum + 1 < iov.iov_count) { + // TODO(ckrasic) - this is unused without prefetch() + // char* next_base = static_cast<char*>(iov.iov[iovnum + 1].iov_base); + // Prefetch 2 cachelines worth of data to get the prefetcher started; + // leave it to the hardware prefetcher after that. + // TODO(ckrasic) - investigate what to do about prefetch directives. + // prefetch(next_base, PREFETCH_HINT_T0); + if (iov.iov[iovnum + 1].iov_len >= 64) { + // TODO(ckrasic) - investigate what to do about prefetch directives. + // prefetch(next_base + CACHELINE_SIZE, PREFETCH_HINT_T0); + } + } + + const char* src = static_cast<char*>(iov.iov[iovnum].iov_base) + iov_offset; + while (true) { + memcpy(buffer, src, copy_len); + length -= copy_len; + buffer += copy_len; + if (length == 0 || ++iovnum >= iov.iov_count) { + break; + } + src = static_cast<char*>(iov.iov[iovnum].iov_base); + copy_len = min(length, iov.iov[iovnum].iov_len); + } + } else { + while (iovnum < iov.iov_count && length > 0) { + const size_t copy_len = min(length, iov.iov[iovnum].iov_len - iov_offset); + memcpy(buffer, static_cast<char*>(iov.iov[iovnum].iov_base) + iov_offset, + copy_len); + iov_offset = 0; + length -= copy_len; + buffer += copy_len; + ++iovnum; + } } LOG_IF(DFATAL, length > 0) << "Failed to copy entire length to buffer."; } diff --git a/net/quic/quic_packet_creator.h b/net/quic/quic_packet_creator.h index 618e00b..da66785 100644 --- a/net/quic/quic_packet_creator.h +++ b/net/quic/quic_packet_creator.h @@ -24,7 +24,6 @@ namespace test { class QuicPacketCreatorPeer; } -class QuicAckNotifier; class QuicRandom; class QuicRandomBoolSource; @@ -84,7 +83,7 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { // fin but return 0. If any data is consumed, it will be copied into a // new buffer that |frame| will point to and will be stored in |buffer|. size_t CreateStreamFrame(QuicStreamId id, - const QuicIOVector& iov, + QuicIOVector iov, size_t iov_offset, QuicStreamOffset offset, bool fin, @@ -240,7 +239,7 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { // Copies |length| bytes from iov starting at offset |iov_offset| into buffer. // |iov| must be at least iov_offset+length total length and buffer must be // at least |length| long. - static void CopyToBuffer(const QuicIOVector& iov, + static void CopyToBuffer(QuicIOVector iov, size_t iov_offset, size_t length, char* buffer); diff --git a/net/quic/quic_packet_creator_test.cc b/net/quic/quic_packet_creator_test.cc index 4e73ba1..8544e89 100644 --- a/net/quic/quic_packet_creator_test.cc +++ b/net/quic/quic_packet_creator_test.cc @@ -10,6 +10,7 @@ #include "net/quic/crypto/null_encrypter.h" #include "net/quic/crypto/quic_decrypter.h" #include "net/quic/crypto/quic_encrypter.h" +#include "net/quic/quic_flags.h" #include "net/quic/quic_utils.h" #include "net/quic/test_tools/mock_random.h" #include "net/quic/test_tools/quic_framer_peer.h" @@ -36,22 +37,25 @@ namespace { struct TestParams { TestParams(QuicVersion version, bool version_serialization, - QuicConnectionIdLength length) + QuicConnectionIdLength length, + bool copy_use_prefetch) : version(version), connection_id_length(length), - version_serialization(version_serialization) { - } + version_serialization(version_serialization), + copy_use_prefetch(copy_use_prefetch) {} friend ostream& operator<<(ostream& os, const TestParams& p) { os << "{ client_version: " << QuicVersionToString(p.version) << " connection id length: " << p.connection_id_length - << " include version: " << p.version_serialization << " }"; + << " include version: " << p.version_serialization + << " copy use prefetch: " << p.copy_use_prefetch << " }"; return os; } QuicVersion version; QuicConnectionIdLength connection_id_length; bool version_serialization; + bool copy_use_prefetch; }; // Constructs various test permutations. @@ -60,15 +64,16 @@ vector<TestParams> GetTestParams() { QuicConnectionIdLength max = PACKET_8BYTE_CONNECTION_ID; QuicVersionVector all_supported_versions = QuicSupportedVersions(); for (size_t i = 0; i < all_supported_versions.size(); ++i) { - params.push_back(TestParams(all_supported_versions[i], true, max)); - params.push_back(TestParams(all_supported_versions[i], false, max)); + params.push_back(TestParams(all_supported_versions[i], true, max, false)); + params.push_back(TestParams(all_supported_versions[i], false, max, false)); } - params.push_back(TestParams( - all_supported_versions[0], true, PACKET_0BYTE_CONNECTION_ID)); - params.push_back(TestParams( - all_supported_versions[0], true, PACKET_1BYTE_CONNECTION_ID)); - params.push_back(TestParams( - all_supported_versions[0], true, PACKET_4BYTE_CONNECTION_ID)); + params.push_back(TestParams(all_supported_versions[0], true, + PACKET_0BYTE_CONNECTION_ID, false)); + params.push_back(TestParams(all_supported_versions[0], true, + PACKET_1BYTE_CONNECTION_ID, false)); + params.push_back(TestParams(all_supported_versions[0], true, + PACKET_4BYTE_CONNECTION_ID, false)); + params.push_back(TestParams(all_supported_versions[0], true, max, true)); return params; } @@ -88,6 +93,7 @@ class QuicPacketCreatorTest : public ::testing::TestWithParam<TestParams> { client_framer_.set_visitor(&framer_visitor_); client_framer_.set_received_entropy_calculator(&entropy_calculator_); server_framer_.set_visitor(&framer_visitor_); + FLAGS_quic_packet_creator_prefetch = GetParam().copy_use_prefetch; } ~QuicPacketCreatorTest() override {} diff --git a/net/quic/quic_packet_generator.cc b/net/quic/quic_packet_generator.cc index 872db01..a5e707e 100644 --- a/net/quic/quic_packet_generator.cc +++ b/net/quic/quic_packet_generator.cc @@ -6,7 +6,6 @@ #include "base/basictypes.h" #include "base/logging.h" -#include "net/quic/quic_ack_notifier.h" #include "net/quic/quic_fec_group.h" #include "net/quic/quic_flags.h" #include "net/quic/quic_utils.h" @@ -51,8 +50,7 @@ QuicPacketGenerator::QuicPacketGenerator(QuicConnectionId connection_id, should_send_stop_waiting_(false), ack_queued_(false), stop_waiting_queued_(false), - max_packet_length_(kDefaultMaxPacketSize), - no_acknotifier_(false) {} + max_packet_length_(kDefaultMaxPacketSize) {} QuicPacketGenerator::~QuicPacketGenerator() { for (QuicFrame& frame : queued_control_frames_) { @@ -125,7 +123,7 @@ void QuicPacketGenerator::AddControlFrame(const QuicFrame& frame) { QuicConsumedData QuicPacketGenerator::ConsumeData( QuicStreamId id, - const QuicIOVector& iov, + QuicIOVector iov, QuicStreamOffset offset, bool fin, FecProtection fec_protection, @@ -148,13 +146,6 @@ QuicConsumedData QuicPacketGenerator::ConsumeData( MaybeStartFecProtection(); } - // This notifier will be owned by the AckNotifierManager (or deleted below) if - // not attached to a packet. - QuicAckNotifier* notifier = nullptr; - if (!no_acknotifier_ && listener != nullptr) { - notifier = new QuicAckNotifier(listener); - } - if (!fin && (iov.total_length == 0)) { LOG(DFATAL) << "Attempt to consume empty data without FIN."; return QuicConsumedData(0, false); @@ -170,20 +161,14 @@ QuicConsumedData QuicPacketGenerator::ConsumeData( &frame, &buffer); ++frames_created; - // We want to track which packet this stream frame ends up in. - if (!no_acknotifier_ && notifier != nullptr) { - ack_notifiers_.push_back(notifier); - } - if (!AddFrame(frame, buffer.Pass(), has_handshake)) { LOG(DFATAL) << "Failed to add stream frame."; // Inability to add a STREAM frame creates an unrecoverable hole in a // the stream, so it's best to close the connection. delegate_->CloseConnection(QUIC_INTERNAL_ERROR, false); - delete notifier; return QuicConsumedData(0, false); } - if (no_acknotifier_ && listener != nullptr) { + if (listener != nullptr) { ack_listeners_.push_back(AckListenerWrapper(listener, bytes_consumed)); } @@ -212,18 +197,13 @@ QuicConsumedData QuicPacketGenerator::ConsumeData( } } - if (notifier != nullptr && frames_created == 0) { - // Safe to delete the AckNotifer as it was never attached to a packet. - delete notifier; - } - // Don't allow the handshake to be bundled with other retransmittable frames. if (has_handshake) { SendQueuedFrames(/*flush=*/true, /*is_fec_timeout=*/false); } // Try to close FEC group since we've either run out of data to send or we're - // blocked. If not in batch mode, force close the group. + // blocked. MaybeSendFecPacketAndCloseGroup(/*force=*/false, /*is_fec_timeout=*/false); DCHECK(InBatchMode() || !packet_creator_.HasPendingFrames()); @@ -235,15 +215,6 @@ void QuicPacketGenerator::GenerateMtuDiscoveryPacket( QuicAckListenerInterface* listener) { // MTU discovery frames must be sent by themselves. DCHECK(!InBatchMode() && !packet_creator_.HasPendingFrames()); - - // If an ack notifier delegate is provided, register it. - if (!no_acknotifier_ && listener != nullptr) { - QuicAckNotifier* ack_notifier = new QuicAckNotifier(listener); - // The notifier manager will take the ownership of the notifier after the - // packet is sent. - ack_notifiers_.push_back(ack_notifier); - } - const QuicByteCount current_mtu = GetMaxPacketLength(); // The MTU discovery frame is allocated on the stack, since it is going to be @@ -254,7 +225,7 @@ void QuicPacketGenerator::GenerateMtuDiscoveryPacket( // Send the probe packet with the new length. SetMaxPacketLength(target_mtu, /*force=*/true); const bool success = AddFrame(frame, nullptr, /*needs_padding=*/true); - if (no_acknotifier_ && listener != nullptr) { + if (listener != nullptr) { ack_listeners_.push_back(AckListenerWrapper(listener, 0)); } SerializeAndSendPacket(); @@ -469,14 +440,9 @@ void QuicPacketGenerator::SerializeAndSendPacket() { return; } - // There may be AckNotifiers interested in this packet. - if (no_acknotifier_) { - serialized_packet.listeners.swap(ack_listeners_); - ack_listeners_.clear(); - } else { - serialized_packet.notifiers.swap(ack_notifiers_); - ack_notifiers_.clear(); - } + // There may be AckListeners interested in this packet. + serialized_packet.listeners.swap(ack_listeners_); + ack_listeners_.clear(); delegate_->OnSerializedPacket(serialized_packet); MaybeSendFecPacketAndCloseGroup(/*force=*/false, /*is_fec_timeout=*/false); diff --git a/net/quic/quic_packet_generator.h b/net/quic/quic_packet_generator.h index dc8775b..07ace04 100644 --- a/net/quic/quic_packet_generator.h +++ b/net/quic/quic_packet_generator.h @@ -55,7 +55,6 @@ #include <list> -#include "net/quic/quic_ack_notifier.h" #include "net/quic/quic_packet_creator.h" #include "net/quic/quic_sent_packet_manager.h" #include "net/quic/quic_types.h" @@ -122,7 +121,7 @@ class NET_EXPORT_PRIVATE QuicPacketGenerator { // |delegate| (if not nullptr) will be informed once all packets sent as a // result of this call are ACKed by the peer. QuicConsumedData ConsumeData(QuicStreamId id, - const QuicIOVector& iov, + QuicIOVector iov, QuicStreamOffset offset, bool fin, FecProtection fec_protection, @@ -222,10 +221,6 @@ class NET_EXPORT_PRIVATE QuicPacketGenerator { fec_send_policy_ = fec_send_policy; } - void set_no_acknotifier(bool no_acknotifier) { - no_acknotifier_ = no_acknotifier; - } - private: friend class test::QuicPacketGeneratorPeer; @@ -307,8 +302,6 @@ class NET_EXPORT_PRIVATE QuicPacketGenerator { bool ack_queued_; bool stop_waiting_queued_; - // Stores notifiers that should be attached to the next serialized packet. - std::list<QuicAckNotifier*> ack_notifiers_; // Stores ack listeners that should be attached to the next packet. std::list<AckListenerWrapper> ack_listeners_; @@ -317,9 +310,6 @@ class NET_EXPORT_PRIVATE QuicPacketGenerator { // packet. QuicByteCount max_packet_length_; - // True if the AckNotifier should not be created. - bool no_acknotifier_; - DISALLOW_COPY_AND_ASSIGN(QuicPacketGenerator); }; diff --git a/net/quic/quic_protocol.cc b/net/quic/quic_protocol.cc index 852b11b..c241f4e 100644 --- a/net/quic/quic_protocol.cc +++ b/net/quic/quic_protocol.cc @@ -233,9 +233,9 @@ QuicStopWaitingFrame::~QuicStopWaitingFrame() {} QuicAckFrame::QuicAckFrame() : entropy_hash(0), + is_truncated(false), largest_observed(0), - delta_time_largest_observed(QuicTime::Delta::Infinite()), - is_truncated(false) {} + delta_time_largest_observed(QuicTime::Delta::Infinite()) {} QuicAckFrame::~QuicAckFrame() {} @@ -301,17 +301,10 @@ ostream& operator<<(ostream& os, const QuicStopWaitingFrame& sent_info) { } PacketNumberQueue::const_iterator::const_iterator( - PacketNumberSet::const_iterator set_iter) - : use_interval_set_(false), set_iter_(set_iter) {} - -PacketNumberQueue::const_iterator::const_iterator( IntervalSet<QuicPacketNumber>::const_iterator interval_set_iter, QuicPacketNumber first, QuicPacketNumber last) - : use_interval_set_(true), - interval_set_iter_(interval_set_iter), - current_(first), - last_(last) {} + : interval_set_iter_(interval_set_iter), current_(first), last_(last) {} PacketNumberQueue::const_iterator::const_iterator(const const_iterator& other) = default; @@ -329,46 +322,30 @@ PacketNumberQueue::const_iterator& PacketNumberQueue::const_iterator::operator=( bool PacketNumberQueue::const_iterator::operator!=( const const_iterator& other) const { - if (use_interval_set_) { - return current_ != other.current_; - } else { - return set_iter_ != other.set_iter_; - } + return current_ != other.current_; } bool PacketNumberQueue::const_iterator::operator==( const const_iterator& other) const { - if (use_interval_set_) { - return current_ == other.current_; - } else { - return set_iter_ == other.set_iter_; - } + return current_ == other.current_; } PacketNumberQueue::const_iterator::value_type PacketNumberQueue::const_iterator:: operator*() const { - if (use_interval_set_) { - return current_; - } else { - return *set_iter_; - } + return current_; } PacketNumberQueue::const_iterator& PacketNumberQueue::const_iterator:: operator++() { - if (use_interval_set_) { - ++current_; - if (current_ < last_) { - if (current_ >= interval_set_iter_->max()) { - ++interval_set_iter_; - current_ = interval_set_iter_->min(); - } - } else { - current_ = last_; + ++current_; + if (current_ < last_) { + if (current_ >= interval_set_iter_->max()) { + ++interval_set_iter_; + current_ = interval_set_iter_->min(); } } else { - ++set_iter_; + current_ = last_; } return *this; } @@ -380,9 +357,7 @@ PacketNumberQueue::const_iterator PacketNumberQueue::const_iterator::operator++( return preincrement; } -PacketNumberQueue::PacketNumberQueue() - : use_interval_set_(FLAGS_quic_packet_queue_use_interval_set) {} - +PacketNumberQueue::PacketNumberQueue() = default; PacketNumberQueue::PacketNumberQueue(const PacketNumberQueue& other) = default; // TODO(rtenneti): on windows RValue reference gives errors. // PacketNumberQueue::PacketNumberQueue(PacketNumberQueue&& other) = default; @@ -395,143 +370,89 @@ PacketNumberQueue& PacketNumberQueue::operator=( // default; void PacketNumberQueue::Add(QuicPacketNumber packet_number) { - if (use_interval_set_) { - packet_number_intervals_.Add(packet_number, packet_number + 1); - } else { - packet_number_set_.insert(packet_number); - } + packet_number_intervals_.Add(packet_number, packet_number + 1); } void PacketNumberQueue::Add(QuicPacketNumber lower, QuicPacketNumber higher) { - if (use_interval_set_) { - packet_number_intervals_.Add(lower, higher); - } else { - for (QuicPacketNumber packet_number = lower; packet_number < higher; - ++packet_number) { - Add(packet_number); - } - } + packet_number_intervals_.Add(lower, higher); } void PacketNumberQueue::Remove(QuicPacketNumber packet_number) { - if (use_interval_set_) { - packet_number_intervals_.Difference(packet_number, packet_number + 1); - } else { - packet_number_set_.erase(packet_number); - } + packet_number_intervals_.Difference(packet_number, packet_number + 1); } bool PacketNumberQueue::RemoveUpTo(QuicPacketNumber higher) { - if (use_interval_set_) { - if (Empty()) { - return false; - } - const QuicPacketNumber old_min = Min(); - packet_number_intervals_.Difference(0, higher); - return Empty() || old_min != Min(); - } else { - const size_t orig_size = packet_number_set_.size(); - packet_number_set_.erase(packet_number_set_.begin(), - packet_number_set_.lower_bound(higher)); - return orig_size != packet_number_set_.size(); + if (Empty()) { + return false; } + const QuicPacketNumber old_min = Min(); + packet_number_intervals_.Difference(0, higher); + return Empty() || old_min != Min(); } bool PacketNumberQueue::Contains(QuicPacketNumber packet_number) const { - if (use_interval_set_) { - return packet_number_intervals_.Contains(packet_number); - } else { - return ContainsKey(packet_number_set_, packet_number); - } + return packet_number_intervals_.Contains(packet_number); } bool PacketNumberQueue::Empty() const { - if (use_interval_set_) { - return packet_number_intervals_.Empty(); - } else { - return packet_number_set_.empty(); - } + return packet_number_intervals_.Empty(); } QuicPacketNumber PacketNumberQueue::Min() const { DCHECK(!Empty()); - if (use_interval_set_) { - return packet_number_intervals_.begin()->min(); - } else { - return *packet_number_set_.begin(); - } + return packet_number_intervals_.begin()->min(); } QuicPacketNumber PacketNumberQueue::Max() const { DCHECK(!Empty()); - if (use_interval_set_) { - return packet_number_intervals_.rbegin()->max() - 1; - } else { - return *packet_number_set_.rbegin(); - } + return packet_number_intervals_.rbegin()->max() - 1; } size_t PacketNumberQueue::NumPacketsSlow() const { - if (use_interval_set_) { - size_t num_packets = 0; - for (const auto& interval : packet_number_intervals_) { - num_packets += interval.Length(); - } - return num_packets; - } else { - return packet_number_set_.size(); + size_t num_packets = 0; + for (const auto& interval : packet_number_intervals_) { + num_packets += interval.Length(); } + return num_packets; } PacketNumberQueue::const_iterator PacketNumberQueue::begin() const { - if (use_interval_set_) { - QuicPacketNumber first; - QuicPacketNumber last; - if (packet_number_intervals_.Empty()) { - first = 0; - last = 0; - } else { - first = packet_number_intervals_.begin()->min(); - last = packet_number_intervals_.rbegin()->max(); - } - return const_iterator(packet_number_intervals_.begin(), first, last); + QuicPacketNumber first; + QuicPacketNumber last; + if (packet_number_intervals_.Empty()) { + first = 0; + last = 0; } else { - return const_iterator(packet_number_set_.begin()); + first = packet_number_intervals_.begin()->min(); + last = packet_number_intervals_.rbegin()->max(); } + return const_iterator(packet_number_intervals_.begin(), first, last); } PacketNumberQueue::const_iterator PacketNumberQueue::end() const { - if (use_interval_set_) { - QuicPacketNumber last = packet_number_intervals_.Empty() - ? 0 - : packet_number_intervals_.rbegin()->max(); - return const_iterator(packet_number_intervals_.end(), last, last); - } else { - return const_iterator(packet_number_set_.end()); - } + QuicPacketNumber last = packet_number_intervals_.Empty() + ? 0 + : packet_number_intervals_.rbegin()->max(); + return const_iterator(packet_number_intervals_.end(), last, last); } PacketNumberQueue::const_iterator PacketNumberQueue::lower_bound( QuicPacketNumber packet_number) const { - if (use_interval_set_) { - QuicPacketNumber first; - QuicPacketNumber last; - if (packet_number_intervals_.Empty()) { - first = 0; - last = 0; - return const_iterator(packet_number_intervals_.begin(), first, last); - } - if (!packet_number_intervals_.Contains(packet_number)) { - return end(); - } - IntervalSet<QuicPacketNumber>::const_iterator it = - packet_number_intervals_.Find(packet_number); - first = packet_number; - last = packet_number_intervals_.rbegin()->max(); - return const_iterator(it, first, last); - } else { - return const_iterator(packet_number_set_.lower_bound(packet_number)); + QuicPacketNumber first; + QuicPacketNumber last; + if (packet_number_intervals_.Empty()) { + first = 0; + last = 0; + return const_iterator(packet_number_intervals_.begin(), first, last); + } + if (!packet_number_intervals_.Contains(packet_number)) { + return end(); } + IntervalSet<QuicPacketNumber>::const_iterator it = + packet_number_intervals_.Find(packet_number); + first = packet_number; + last = packet_number_intervals_.rbegin()->max(); + return const_iterator(it, first, last); } ostream& operator<<(ostream& os, const PacketNumberQueue& q) { @@ -554,10 +475,9 @@ ostream& operator<<(ostream& os, const QuicAckFrame& ack_frame) { os << *it << " "; } os << " ] received_packets: [ "; - for (PacketTimeList::const_iterator it = - ack_frame.received_packet_times.begin(); - it != ack_frame.received_packet_times.end(); ++it) { - os << it->first << " at " << it->second.ToDebuggingValue() << " "; + for (const std::pair<QuicPacketNumber, QuicTime>& p : + ack_frame.received_packet_times) { + os << p.first << " at " << p.second.ToDebuggingValue() << " "; } os << " ]"; return os; diff --git a/net/quic/quic_protocol.h b/net/quic/quic_protocol.h index e393140..aa008c4 100644 --- a/net/quic/quic_protocol.h +++ b/net/quic/quic_protocol.h @@ -34,7 +34,6 @@ namespace net { -class QuicAckNotifier; class QuicPacket; struct QuicPacketHeader; @@ -727,7 +726,7 @@ struct NET_EXPORT_PRIVATE QuicStreamFrame { typedef std::set<QuicPacketNumber> PacketNumberSet; typedef std::list<QuicPacketNumber> PacketNumberList; -typedef std::list<std::pair<QuicPacketNumber, QuicTime>> PacketTimeList; +typedef std::vector<std::pair<QuicPacketNumber, QuicTime>> PacketTimeVector; struct NET_EXPORT_PRIVATE QuicStopWaitingFrame { QuicStopWaitingFrame(); @@ -747,8 +746,8 @@ struct NET_EXPORT_PRIVATE QuicStopWaitingFrame { // larger new packet numbers are added, with the occasional random access. class NET_EXPORT_PRIVATE PacketNumberQueue { public: - // TODO(jdorfman): Once this is completely switched over to IntervalSet, - // remove const_iterator and change the callers to iterate over the intervals. + // TODO(jdorfman): remove const_iterator and change the callers to + // iterate over the intervals. class NET_EXPORT_PRIVATE const_iterator : public std::iterator<std::input_iterator_tag, QuicPacketNumber, @@ -776,8 +775,6 @@ class NET_EXPORT_PRIVATE PacketNumberQueue { const_iterator operator++(int /* postincrement */); private: - bool use_interval_set_; - PacketNumberSet::const_iterator set_iter_; IntervalSet<QuicPacketNumber>::const_iterator interval_set_iter_; QuicPacketNumber current_; QuicPacketNumber last_; @@ -834,8 +831,6 @@ class NET_EXPORT_PRIVATE PacketNumberQueue { const PacketNumberQueue& q); private: - bool use_interval_set_; - PacketNumberSet packet_number_set_; IntervalSet<QuicPacketNumber> packet_number_intervals_; }; @@ -850,6 +845,9 @@ struct NET_EXPORT_PRIVATE QuicAckFrame { // packets. QuicPacketEntropyHash entropy_hash; + // Whether the ack had to be truncated when sent. + bool is_truncated; + // The highest packet number we've observed from the peer. // // In general, this should be the largest packet number we've received. In @@ -864,18 +862,15 @@ struct NET_EXPORT_PRIVATE QuicAckFrame { // sent. QuicTime::Delta delta_time_largest_observed; + // Vector of <packet_number, time> for when packets arrived. + PacketTimeVector received_packet_times; + // The set of packets which we're expecting and have not received. PacketNumberQueue missing_packets; - // Whether the ack had to be truncated when sent. - bool is_truncated; - // Packets which have been revived via FEC. // All of these must also be in missing_packets. PacketNumberSet revived_packets; - - // List of <packet_number, time> for when packets arrived. - PacketTimeList received_packet_times; }; // True if the packet number is greater than largest_observed or is listed @@ -1171,7 +1166,6 @@ struct NET_EXPORT_PRIVATE SerializedPacket { bool has_stop_waiting; // Optional notifiers which will be informed when this packet has been ACKed. - std::list<QuicAckNotifier*> notifiers; std::list<AckListenerWrapper> listeners; }; diff --git a/net/quic/quic_received_packet_manager.cc b/net/quic/quic_received_packet_manager.cc index cf83f4a..f49391c 100644 --- a/net/quic/quic_received_packet_manager.cc +++ b/net/quic/quic_received_packet_manager.cc @@ -176,7 +176,8 @@ void QuicReceivedPacketManager::RecordPacketReceived( } entropy_tracker_.RecordPacketEntropyHash(packet_number, header.entropy_hash); - received_packet_times_.push_back(std::make_pair(packet_number, receipt_time)); + ack_frame_.received_packet_times.push_back( + std::make_pair(packet_number, receipt_time)); ack_frame_.revived_packets.erase(packet_number); } @@ -226,11 +227,23 @@ void QuicReceivedPacketManager::UpdateReceivedPacketInfo( QuicTime::Delta::Zero() : approximate_now.Subtract(time_largest_observed_); - // Remove all packets that are too far from largest_observed to express. - received_packet_times_.remove_if(isTooLarge(ack_frame_.largest_observed)); + // Clear all packet times if any are too far from largest observed. + // It's expected this is extremely rare. + for (PacketTimeVector::iterator it = ack_frame_.received_packet_times.begin(); + it != ack_frame_.received_packet_times.end();) { + if (ack_frame_.largest_observed - it->first >= + numeric_limits<uint8>::max()) { + it = ack_frame_.received_packet_times.erase(it); + } else { + ++it; + } + } + // TODO(ianswett): Instead of transferring all the information over, + // consider giving the QuicPacketGenerator a reference to this ack frame + // and clear it afterwards. ack_frame->received_packet_times.clear(); - ack_frame->received_packet_times.swap(received_packet_times_); + ack_frame->received_packet_times.swap(ack_frame_.received_packet_times); } QuicPacketEntropyHash QuicReceivedPacketManager::EntropyHash( diff --git a/net/quic/quic_received_packet_manager.h b/net/quic/quic_received_packet_manager.h index 1095808..d13cd24 100644 --- a/net/quic/quic_received_packet_manager.h +++ b/net/quic/quic_received_packet_manager.h @@ -166,8 +166,6 @@ class NET_EXPORT_PRIVATE QuicReceivedPacketManager : QuicConnectionStats* stats_; - PacketTimeList received_packet_times_; - DISALLOW_COPY_AND_ASSIGN(QuicReceivedPacketManager); }; diff --git a/net/quic/quic_received_packet_manager_test.cc b/net/quic/quic_received_packet_manager_test.cc index 78472f4..31ff8a0 100644 --- a/net/quic/quic_received_packet_manager_test.cc +++ b/net/quic/quic_received_packet_manager_test.cc @@ -313,7 +313,7 @@ TEST_F(QuicReceivedPacketManagerTest, UpdateReceivedPacketInfo) { // When UpdateReceivedPacketInfo with a time earlier than the time of the // largest observed packet, make sure that the delta is 0, not negative. EXPECT_EQ(QuicTime::Delta::Zero(), ack.delta_time_largest_observed); - EXPECT_FALSE(ack.received_packet_times.empty()); + EXPECT_EQ(1ul, ack.received_packet_times.size()); QuicTime four_ms = QuicTime::Zero().Add(QuicTime::Delta::FromMilliseconds(4)); received_manager_.UpdateReceivedPacketInfo(&ack, four_ms); @@ -321,6 +321,18 @@ TEST_F(QuicReceivedPacketManagerTest, UpdateReceivedPacketInfo) { // the delta should still be accurate. EXPECT_EQ(QuicTime::Delta::FromMilliseconds(2), ack.delta_time_largest_observed); + EXPECT_EQ(0ul, ack.received_packet_times.size()); + + header.packet_number = 999u; + received_manager_.RecordPacketReceived(0u, header, two_ms); + header.packet_number = 4u; + received_manager_.RecordPacketReceived(0u, header, two_ms); + header.packet_number = 1000u; + received_manager_.RecordPacketReceived(0u, header, two_ms); + received_manager_.UpdateReceivedPacketInfo(&ack, two_ms); + // UpdateReceivedPacketInfo should discard any times which can't be + // expressed on the wire. + EXPECT_EQ(2ul, ack.received_packet_times.size()); } TEST_F(QuicReceivedPacketManagerTest, UpdateReceivedConnectionStats) { diff --git a/net/quic/quic_sent_packet_manager.cc b/net/quic/quic_sent_packet_manager.cc index 7d45b8e..3d4fb9b 100644 --- a/net/quic/quic_sent_packet_manager.cc +++ b/net/quic/quic_sent_packet_manager.cc @@ -11,7 +11,6 @@ #include "net/quic/congestion_control/pacing_sender.h" #include "net/quic/crypto/crypto_protocol.h" #include "net/quic/proto/cached_network_parameters.pb.h" -#include "net/quic/quic_ack_notifier_manager.h" #include "net/quic/quic_connection_stats.h" #include "net/quic/quic_flags.h" #include "net/quic/quic_utils_chromium.h" @@ -68,7 +67,7 @@ QuicSentPacketManager::QuicSentPacketManager( QuicConnectionStats* stats, CongestionControlType congestion_control_type, LossDetectionType loss_type) - : unacked_packets_(&ack_notifier_manager_), + : unacked_packets_(), perspective_(perspective), clock_(clock), stats_(stats), @@ -94,8 +93,7 @@ QuicSentPacketManager::QuicSentPacketManager( enable_half_rtt_tail_loss_probe_(false), using_pacing_(false), use_new_rto_(false), - handshake_confirmed_(false), - no_acknotifier_(false) {} + handshake_confirmed_(false) {} QuicSentPacketManager::~QuicSentPacketManager() { } @@ -322,7 +320,7 @@ void QuicSentPacketManager::HandleAckForSentPackets( // If data is associated with the most recent transmission of this // packet, then inform the caller. if (it->in_flight) { - packets_acked_.push_back(std::make_pair(packet_number, *it)); + packets_acked_.push_back(std::make_pair(packet_number, it->bytes_sent)); } MarkPacketHandled(packet_number, *it, delta_largest_observed); } @@ -471,15 +469,10 @@ void QuicSentPacketManager::MarkPacketRevived( // retransmit it, do not retransmit it anymore. pending_retransmissions_.erase(newest_transmission); - // The AckNotifierManager needs to be notified for revived packets, + // The AckListener needs to be notified for revived packets, // since it indicates the packet arrived from the appliction's perspective. - if (no_acknotifier_) { - unacked_packets_.NotifyAndClearListeners(newest_transmission, - delta_largest_observed); - } else { - ack_notifier_manager_.OnPacketAcked(newest_transmission, - delta_largest_observed); - } + unacked_packets_.NotifyAndClearListeners(newest_transmission, + delta_largest_observed); unacked_packets_.RemoveRetransmittability(packet_number); } @@ -494,17 +487,12 @@ void QuicSentPacketManager::MarkPacketHandled( // Remove the most recent packet, if it is pending retransmission. pending_retransmissions_.erase(newest_transmission); - // The AckNotifierManager needs to be notified about the most recent + // The AckListener needs to be notified about the most recent // transmission, since that's the one only one it tracks. - if (no_acknotifier_) { - // TODO(ianswett): An extra retrieval into the UnackedPacketMap is done - // here, so there may be opportunity for optimization. - unacked_packets_.NotifyAndClearListeners(newest_transmission, - delta_largest_observed); - } else { - ack_notifier_manager_.OnPacketAcked(newest_transmission, - delta_largest_observed); - } + // TODO(ianswett): An extra retrieval into the UnackedPacketMap is done + // here, so there may be opportunity for optimization. + unacked_packets_.NotifyAndClearListeners(newest_transmission, + delta_largest_observed); if (newest_transmission != packet_number) { const TransmissionInfo& newest_transmission_info = @@ -559,12 +547,6 @@ bool QuicSentPacketManager::OnPacketSent( << "pending_retransmissions_. packet_number: " << original_packet_number; } - // Inform the ack notifier of retransmissions so it can calculate the - // retransmit rate. - if (!no_acknotifier_) { - ack_notifier_manager_.OnPacketRetransmitted(original_packet_number, - packet_number, bytes); - } } if (pending_timer_transmission_count_ > 0) { @@ -724,7 +706,8 @@ void QuicSentPacketManager::InvokeLossDetection(QuicTime time) { // should be recorded as a loss to the send algorithm, but not retransmitted // until it's known whether the FEC packet arrived. ++stats_->packets_lost; - packets_lost_.push_back(std::make_pair(packet_number, transmission_info)); + packets_lost_.push_back( + std::make_pair(packet_number, transmission_info.bytes_sent)); DVLOG(1) << ENDPOINT << "Lost packet " << packet_number; if (transmission_info.retransmittable_frames != nullptr) { @@ -925,13 +908,6 @@ QuicPacketCount QuicSentPacketManager::GetSlowStartThresholdInTcpMss() const { return send_algorithm_->GetSlowStartThreshold() / kDefaultTCPMSS; } -void QuicSentPacketManager::OnSerializedPacket( - const SerializedPacket& serialized_packet) { - if (!no_acknotifier_) { - ack_notifier_manager_.OnSerializedPacket(serialized_packet); - } -} - void QuicSentPacketManager::CancelRetransmissionsForStream( QuicStreamId stream_id) { unacked_packets_.CancelRetransmissionsForStream(stream_id); diff --git a/net/quic/quic_sent_packet_manager.h b/net/quic/quic_sent_packet_manager.h index e79b201..c37d3c3 100644 --- a/net/quic/quic_sent_packet_manager.h +++ b/net/quic/quic_sent_packet_manager.h @@ -16,7 +16,6 @@ #include "net/quic/congestion_control/loss_detection_interface.h" #include "net/quic/congestion_control/rtt_stats.h" #include "net/quic/congestion_control/send_algorithm_interface.h" -#include "net/quic/quic_ack_notifier_manager.h" #include "net/quic/quic_protocol.h" #include "net/quic/quic_sustained_bandwidth_recorder.h" #include "net/quic/quic_unacked_packet_map.h" @@ -208,9 +207,6 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager { // start threshold and will return 0. QuicPacketCount GetSlowStartThresholdInTcpMss() const; - // Called by the connection every time it receives a serialized packet. - void OnSerializedPacket(const SerializedPacket& serialized_packet); - // No longer retransmit data for |stream_id|. void CancelRetransmissionsForStream(QuicStreamId stream_id); @@ -251,11 +247,6 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager { return consecutive_tlp_count_; } - void set_no_acknotifier(bool no_acknotifier) { - no_acknotifier_ = no_acknotifier; - unacked_packets_.set_no_acknotifier(no_acknotifier); - } - private: friend class test::QuicConnectionPeer; friend class test::QuicSentPacketManagerPeer; @@ -357,11 +348,6 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager { // Tracks if the connection was created by the server or the client. Perspective perspective_; - // An AckNotifier can register to be informed when ACKs have been received for - // all packets that a given block of data was sent in. The AckNotifierManager - // maintains the currently active notifiers. - AckNotifierManager ack_notifier_manager_; - const QuicClock* clock_; QuicConnectionStats* stats_; DebugDelegate* debug_delegate_; @@ -412,9 +398,6 @@ class NET_EXPORT_PRIVATE QuicSentPacketManager { // of time with no loss events. QuicSustainedBandwidthRecorder sustained_bandwidth_recorder_; - // True if the AckNotifierManager should not be called. - bool no_acknotifier_; - DISALLOW_COPY_AND_ASSIGN(QuicSentPacketManager); }; diff --git a/net/quic/quic_session.cc b/net/quic/quic_session.cc index 941a846..3925924 100644 --- a/net/quic/quic_session.cc +++ b/net/quic/quic_session.cc @@ -290,7 +290,7 @@ bool QuicSession::HasOpenDynamicStreams() const { QuicConsumedData QuicSession::WritevData( QuicStreamId id, - const QuicIOVector& iov, + QuicIOVector iov, QuicStreamOffset offset, bool fin, FecProtection fec_protection, @@ -728,11 +728,30 @@ bool QuicSession::IsClosedStream(QuicStreamId id) { } size_t QuicSession::GetNumOpenStreams() const { - if (FLAGS_allow_many_available_streams) { - return dynamic_stream_map_.size() - draining_streams_.size(); + if (FLAGS_quic_count_unfinished_as_open_streams) { + if (FLAGS_allow_many_available_streams) { + return dynamic_stream_map_.size() - draining_streams_.size() + + locally_closed_streams_highest_offset_.size(); + } else { + return dynamic_stream_map_.size() + available_streams_.size() - + draining_streams_.size() + + locally_closed_streams_highest_offset_.size(); + } + } else { + if (FLAGS_allow_many_available_streams) { + return dynamic_stream_map_.size() - draining_streams_.size(); + } else { + return dynamic_stream_map_.size() + available_streams_.size() - + draining_streams_.size(); + } + } +} + +size_t QuicSession::GetNumActiveStreams() const { + if (FLAGS_quic_count_unfinished_as_open_streams) { + return GetNumOpenStreams() - locally_closed_streams_highest_offset_.size(); } else { - return dynamic_stream_map_.size() + available_streams_.size() - - draining_streams_.size(); + return GetNumOpenStreams(); } } @@ -775,7 +794,8 @@ void QuicSession::PostProcessAfterData() { STLDeleteElements(&closed_streams_); // A buggy client may fail to send FIN/RSTs. Don't tolerate this. - if (locally_closed_streams_highest_offset_.size() > max_open_streams_) { + if (!FLAGS_quic_count_unfinished_as_open_streams && + locally_closed_streams_highest_offset_.size() > max_open_streams_) { CloseConnection(QUIC_TOO_MANY_UNFINISHED_STREAMS); } } diff --git a/net/quic/quic_session.h b/net/quic/quic_session.h index 12ac967..8c3f079 100644 --- a/net/quic/quic_session.h +++ b/net/quic/quic_session.h @@ -87,7 +87,7 @@ class NET_EXPORT_PRIVATE QuicSession : public QuicConnectionVisitorInterface { // we have seen ACKs for all packets resulting from this call. virtual QuicConsumedData WritevData( QuicStreamId id, - const QuicIOVector& iov, + QuicIOVector iov, QuicStreamOffset offset, bool fin, FecProtection fec_protection, @@ -156,6 +156,9 @@ class NET_EXPORT_PRIVATE QuicSession : public QuicConnectionVisitorInterface { // headers and crypto streams. virtual size_t GetNumOpenStreams() const; + // Same as GetNumOpenStreams(), but never counting unfinished streams. + virtual size_t GetNumActiveStreams() const; + // Returns the number of "available" streams, the stream ids less than // largest_peer_created_stream_id_ that have not yet been opened. virtual size_t GetNumAvailableStreams() const; diff --git a/net/quic/quic_session_test.cc b/net/quic/quic_session_test.cc index ba88975..244743e 100644 --- a/net/quic/quic_session_test.cc +++ b/net/quic/quic_session_test.cc @@ -153,7 +153,7 @@ class TestSession : public QuicSpdySession { QuicConsumedData WritevData( QuicStreamId id, - const QuicIOVector& data, + QuicIOVector data, QuicStreamOffset offset, bool fin, FecProtection fec_protection, @@ -969,16 +969,17 @@ TEST_P(QuicSessionTestServer, WindowUpdateUnblocksHeadersStream) { } TEST_P(QuicSessionTestServer, TooManyUnfinishedStreamsCauseConnectionClose) { - // If a buggy/malicious peer creates too many streams that are not ended with - // a FIN or RST then we send a connection close. + FLAGS_quic_count_unfinished_as_open_streams = false; + // If a buggy/malicious peer creates too many streams that are not ended + // with a FIN or RST then we send a connection close. EXPECT_CALL(*connection_, SendConnectionClose(QUIC_TOO_MANY_UNFINISHED_STREAMS)); const QuicStreamId kMaxStreams = 5; QuicSessionPeer::SetMaxOpenStreams(&session_, kMaxStreams); - // Create kMaxStreams + 1 data streams, and close them all without receiving a - // FIN or a RST_STREAM from the client. + // Create kMaxStreams + 1 data streams, and close them all without receiving + // a FIN or a RST_STREAM from the client. const QuicStreamId kFirstStreamId = kClientDataStreamId1; const QuicStreamId kFinalStreamId = kClientDataStreamId1 + 2 * kMaxStreams + 1; @@ -990,8 +991,45 @@ TEST_P(QuicSessionTestServer, TooManyUnfinishedStreamsCauseConnectionClose) { session_.CloseStream(i); } - // Called after any new data is received by the session, and triggers the call - // to close the connection. + // Called after any new data is received by the session, and triggers the + // call to close the connection. + session_.PostProcessAfterData(); +} + +TEST_P(QuicSessionTestServer, TooManyUnfinishedStreamsCauseServerRejectStream) { + FLAGS_quic_count_unfinished_as_open_streams = true; + // If a buggy/malicious peer creates too many streams that are not ended + // with a FIN or RST then we send a connection close or an RST to + // refuse streams. + const QuicStreamId kMaxStreams = 5; + QuicSessionPeer::SetMaxOpenStreams(&session_, kMaxStreams); + const QuicStreamId kFirstStreamId = kClientDataStreamId1; + const QuicStreamId kFinalStreamId = kClientDataStreamId1 + 2 * kMaxStreams; + + // Create kMaxStreams data streams, and close them all without receiving a + // FIN or a RST_STREAM from the client. + for (QuicStreamId i = kFirstStreamId; i < kFinalStreamId; i += 2) { + QuicStreamFrame data1(i, false, 0, StringPiece("HT")); + session_.OnStreamFrame(data1); + // EXPECT_EQ(1u, session_.GetNumOpenStreams()); + EXPECT_CALL(*connection_, SendRstStream(i, _, _)); + session_.CloseStream(i); + } + + if (GetParam() <= QUIC_VERSION_27) { + EXPECT_CALL(*connection_, SendConnectionClose(QUIC_TOO_MANY_OPEN_STREAMS)); + EXPECT_CALL(*connection_, SendRstStream(kFinalStreamId, _, _)).Times(0); + } else { + EXPECT_CALL(*connection_, + SendRstStream(kFinalStreamId, QUIC_REFUSED_STREAM, _)) + .Times(1); + } + // Create one more data streams to exceed limit of open stream. + QuicStreamFrame data1(kFinalStreamId, false, 0, StringPiece("HT")); + session_.OnStreamFrame(data1); + + // Called after any new data is received by the session, and triggers the + // call to close the connection. session_.PostProcessAfterData(); } @@ -999,8 +1037,19 @@ TEST_P(QuicSessionTestServer, DrainingStreamsDoNotCountAsOpened) { // Verify that a draining stream (which has received a FIN but not consumed // it) does not count against the open quota (because it is closed from the // protocol point of view). - EXPECT_CALL(*connection_, - SendConnectionClose(QUIC_TOO_MANY_UNFINISHED_STREAMS)).Times(0); + if (FLAGS_quic_count_unfinished_as_open_streams) { + if (GetParam() <= QUIC_VERSION_27) { + EXPECT_CALL(*connection_, SendConnectionClose(QUIC_TOO_MANY_OPEN_STREAMS)) + .Times(0); + } else { + EXPECT_CALL(*connection_, SendRstStream(_, QUIC_REFUSED_STREAM, _)) + .Times(0); + } + } else { + EXPECT_CALL(*connection_, + SendConnectionClose(QUIC_TOO_MANY_UNFINISHED_STREAMS)) + .Times(0); + } const QuicStreamId kMaxStreams = 5; QuicSessionPeer::SetMaxOpenStreams(&session_, kMaxStreams); diff --git a/net/quic/quic_spdy_stream.h b/net/quic/quic_spdy_stream.h index 2fc1448..781b005 100644 --- a/net/quic/quic_spdy_stream.h +++ b/net/quic/quic_spdy_stream.h @@ -19,7 +19,6 @@ #include "net/base/iovec.h" #include "net/base/ip_endpoint.h" #include "net/base/net_export.h" -#include "net/quic/quic_ack_notifier.h" #include "net/quic/quic_protocol.h" #include "net/quic/quic_stream_sequencer.h" #include "net/quic/reliable_quic_stream.h" diff --git a/net/quic/quic_spdy_stream_test.cc b/net/quic/quic_spdy_stream_test.cc index 99d9ff4..da94274 100644 --- a/net/quic/quic_spdy_stream_test.cc +++ b/net/quic/quic_spdy_stream_test.cc @@ -4,7 +4,6 @@ #include "net/quic/quic_spdy_stream.h" -#include "net/quic/quic_ack_notifier.h" #include "net/quic/quic_connection.h" #include "net/quic/quic_utils.h" #include "net/quic/quic_write_blocked_list.h" diff --git a/net/quic/quic_stream_factory.cc b/net/quic/quic_stream_factory.cc index 97595bf..e47f116 100644 --- a/net/quic/quic_stream_factory.cc +++ b/net/quic/quic_stream_factory.cc @@ -1038,7 +1038,7 @@ void QuicStreamFactory::MaybeDisableQuic(QuicChromiumClientSession* session) { } void QuicStreamFactory::OnSessionClosed(QuicChromiumClientSession* session) { - DCHECK_EQ(0u, session->GetNumOpenStreams()); + DCHECK_EQ(0u, session->GetNumActiveStreams()); MaybeDisableQuic(session); OnSessionGoingAway(session); delete session; diff --git a/net/quic/quic_stream_factory_test.cc b/net/quic/quic_stream_factory_test.cc index 8cbf44a..dfbcc0f 100644 --- a/net/quic/quic_stream_factory_test.cc +++ b/net/quic/quic_stream_factory_test.cc @@ -1105,19 +1105,22 @@ TEST_P(QuicStreamFactoryTest, Goaway) { TEST_P(QuicStreamFactoryTest, MaxOpenStream) { Initialize(); - MockRead reads[] = { - MockRead(ASYNC, OK, 0) // EOF - }; QuicStreamId stream_id = kClientDataStreamId1; - scoped_ptr<QuicEncryptedPacket> rst( + scoped_ptr<QuicEncryptedPacket> client_rst( maker_.MakeRstPacket(1, true, stream_id, QUIC_STREAM_CANCELLED)); MockWrite writes[] = { - MockWrite(ASYNC, rst->data(), rst->length(), 1), + MockWrite(ASYNC, client_rst->data(), client_rst->length(), 0), + }; + scoped_ptr<QuicEncryptedPacket> server_rst( + maker_.MakeRstPacket(1, false, stream_id, QUIC_STREAM_CANCELLED)); + MockRead reads[] = { + MockRead(ASYNC, server_rst->data(), server_rst->length(), 1), + MockRead(ASYNC, OK, 2) // EOF }; DeterministicSocketData socket_data(reads, arraysize(reads), writes, arraysize(writes)); socket_factory_.AddSocketDataProvider(&socket_data); - socket_data.StopAfter(1); + socket_data.StopAfter(2); HttpRequestInfo request_info; std::vector<QuicHttpStream*> streams; @@ -1152,13 +1155,23 @@ TEST_P(QuicStreamFactoryTest, MaxOpenStream) { // Close the first stream. streams.front()->Close(false); + // Trigger exchange of RSTs that in turn allow progress for the last + // stream. + socket_data.RunFor(2); ASSERT_TRUE(callback_.have_result()); - EXPECT_EQ(OK, callback_.WaitForResult()); EXPECT_TRUE(socket_data.AllReadDataConsumed()); EXPECT_TRUE(socket_data.AllWriteDataConsumed()); + + // Force close of the connection to suppress the generation of RST + // packets when streams are torn down, which wouldn't be relevant to + // this test anyway. + QuicChromiumClientSession* session = + QuicStreamFactoryPeer::GetActiveSession(factory_.get(), host_port_pair_); + session->connection()->CloseConnection(QUIC_PUBLIC_RESET, true); + STLDeleteElements(&streams); } diff --git a/net/quic/quic_unacked_packet_map.cc b/net/quic/quic_unacked_packet_map.cc index fa0e2ff..ab389ce 100644 --- a/net/quic/quic_unacked_packet_map.cc +++ b/net/quic/quic_unacked_packet_map.cc @@ -6,7 +6,6 @@ #include "base/logging.h" #include "base/stl_util.h" -#include "net/quic/quic_ack_notifier_manager.h" #include "net/quic/quic_connection_stats.h" #include "net/quic/quic_flags.h" #include "net/quic/quic_utils_chromium.h" @@ -15,15 +14,12 @@ using std::max; namespace net { -QuicUnackedPacketMap::QuicUnackedPacketMap( - AckNotifierManager* ack_notifier_manager) +QuicUnackedPacketMap::QuicUnackedPacketMap() : largest_sent_packet_(0), largest_observed_(0), least_unacked_(1), bytes_in_flight_(0), - pending_crypto_packet_count_(0), - ack_notifier_manager_(ack_notifier_manager), - no_acknotifier_(false) {} + pending_crypto_packet_count_(0) {} QuicUnackedPacketMap::~QuicUnackedPacketMap() { QuicPacketNumber index = least_unacked_; @@ -60,9 +56,7 @@ void QuicUnackedPacketMap::AddSentPacket(SerializedPacket* packet, packet->retransmittable_frames->HasCryptoHandshake() == IS_HANDSHAKE) { ++pending_crypto_packet_count_; } - if (no_acknotifier_) { - info.ack_listeners.swap(packet->listeners); - } + info.ack_listeners.swap(packet->listeners); } else { TransferRetransmissionInfo(old_packet_number, packet_number, transmission_type, &info); @@ -81,7 +75,6 @@ void QuicUnackedPacketMap::RemoveObsoletePackets() { if (!IsPacketUseless(least_unacked_, unacked_packets_.front())) { break; } - ack_notifier_manager_->OnPacketRemoved(least_unacked_); unacked_packets_.pop_front(); ++least_unacked_; @@ -107,13 +100,11 @@ void QuicUnackedPacketMap::TransferRetransmissionInfo( &unacked_packets_.at(old_packet_number - least_unacked_); RetransmittableFrames* frames = transmission_info->retransmittable_frames; transmission_info->retransmittable_frames = nullptr; - if (no_acknotifier_) { - for (AckListenerWrapper& wrapper : transmission_info->ack_listeners) { - wrapper.ack_listener->OnPacketRetransmitted(wrapper.length); - } - // Transfer the ack listeners if it's present. - info->ack_listeners.swap(transmission_info->ack_listeners); + for (AckListenerWrapper& wrapper : transmission_info->ack_listeners) { + wrapper.ack_listener->OnPacketRetransmitted(wrapper.length); } + // Transfer the AckListeners if any are present. + info->ack_listeners.swap(transmission_info->ack_listeners); LOG_IF(DFATAL, frames == nullptr) << "Attempt to retransmit packet with no " << "retransmittable frames: " << old_packet_number; diff --git a/net/quic/quic_unacked_packet_map.h b/net/quic/quic_unacked_packet_map.h index 6d842c2..0088f4c 100644 --- a/net/quic/quic_unacked_packet_map.h +++ b/net/quic/quic_unacked_packet_map.h @@ -19,10 +19,7 @@ class AckNotifierManager; // 3) Track sent time of packets to provide RTT measurements from acks. class NET_EXPORT_PRIVATE QuicUnackedPacketMap { public: - // Initialize an instance of UnackedPacketMap. The AckNotifierManager - // provided to the constructor will be notified whenever a packet is removed - // from the map. - explicit QuicUnackedPacketMap(AckNotifierManager* ack_notifier_manager); + QuicUnackedPacketMap(); ~QuicUnackedPacketMap(); // Adds |serialized_packet| to the map and marks it as sent at |sent_time|. @@ -129,10 +126,6 @@ class NET_EXPORT_PRIVATE QuicUnackedPacketMap { // RTT measurement purposes. void RemoveObsoletePackets(); - void set_no_acknotifier(bool no_acknotifier) { - no_acknotifier_ = no_acknotifier; - } - private: // Called when a packet is retransmitted with a new packet number. // |old_packet_number| will remain unacked, but will have no @@ -179,13 +172,6 @@ class NET_EXPORT_PRIVATE QuicUnackedPacketMap { // Number of retransmittable crypto handshake packets. size_t pending_crypto_packet_count_; - // Notifier manager for ACK packets. We notify it every time we abandon a - // packet. - AckNotifierManager* ack_notifier_manager_; - - // True if the AckNotifierManager should not be called. - bool no_acknotifier_; - DISALLOW_COPY_AND_ASSIGN(QuicUnackedPacketMap); }; diff --git a/net/quic/quic_unacked_packet_map_test.cc b/net/quic/quic_unacked_packet_map_test.cc index ad9fc9a..1e302e3 100644 --- a/net/quic/quic_unacked_packet_map_test.cc +++ b/net/quic/quic_unacked_packet_map_test.cc @@ -5,7 +5,6 @@ #include "net/quic/quic_unacked_packet_map.h" #include "base/stl_util.h" -#include "net/quic/quic_ack_notifier_manager.h" #include "net/quic/quic_flags.h" #include "net/quic/quic_utils.h" #include "net/quic/test_tools/quic_test_utils.h" @@ -25,7 +24,7 @@ const uint32 kDefaultLength = 1000; class QuicUnackedPacketMapTest : public ::testing::Test { protected: QuicUnackedPacketMapTest() - : unacked_packets_(&ack_notifier_manager_), + : unacked_packets_(), now_(QuicTime::Zero().Add(QuicTime::Delta::FromMilliseconds(1000))) {} ~QuicUnackedPacketMapTest() override { @@ -116,7 +115,6 @@ class QuicUnackedPacketMapTest : public ::testing::Test { } } vector<QuicEncryptedPacket*> packets_; - AckNotifierManager ack_notifier_manager_; QuicUnackedPacketMap unacked_packets_; QuicTime now_; }; diff --git a/net/quic/reliable_quic_stream.cc b/net/quic/reliable_quic_stream.cc index 87b5b5a..f4e9a8a 100644 --- a/net/quic/reliable_quic_stream.cc +++ b/net/quic/reliable_quic_stream.cc @@ -43,82 +43,10 @@ size_t GetReceivedFlowControlWindow(QuicSession* session) { } // namespace -// Wrapper that aggregates OnAckNotifications for packets sent using -// WriteOrBufferData and delivers them to the original -// QuicAckListenerInterface after all bytes written using -// WriteOrBufferData are acked. This level of indirection is -// necessary because the delegate interface provides no mechanism that -// WriteOrBufferData can use to inform it that the write required -// multiple WritevData calls or that only part of the data has been -// sent out by the time ACKs start arriving. -class ReliableQuicStream::ProxyAckNotifierDelegate - : public QuicAckListenerInterface { - public: - explicit ProxyAckNotifierDelegate(QuicAckListenerInterface* delegate) - : delegate_(delegate), - pending_acks_(0), - wrote_last_data_(false), - num_retransmitted_packets_(0), - num_retransmitted_bytes_(0) {} - - // TODO(ianswett): Remove this class and indirection once OnAckNotification - // goes away because the above class comment is no longer true. - void OnAckNotification(int num_retransmitted_packets, - int num_retransmitted_bytes, - QuicTime::Delta delta_largest_observed) override { - DCHECK_LT(0, pending_acks_); - --pending_acks_; - num_retransmitted_packets_ += num_retransmitted_packets; - num_retransmitted_bytes_ += num_retransmitted_bytes; - - if (wrote_last_data_ && pending_acks_ == 0) { - delegate_->OnAckNotification(num_retransmitted_packets_, - num_retransmitted_bytes_, - delta_largest_observed); - } - } - - void OnPacketAcked(int acked_bytes, - QuicTime::Delta delta_largest_observed) override { - delegate_->OnPacketAcked(acked_bytes, delta_largest_observed); - } - - void OnPacketRetransmitted(int retransmitted_bytes) override { - delegate_->OnPacketRetransmitted(retransmitted_bytes); - } - - void WroteData(bool last_data, size_t bytes_consumed) { - DCHECK(!wrote_last_data_); - ++pending_acks_; - wrote_last_data_ = last_data; - } - - protected: - // Delegates are ref counted. - ~ProxyAckNotifierDelegate() override {} - - private: - // Original delegate. delegate_->OnAckNotification will be called when: - // wrote_last_data_ == true and pending_acks_ == 0 - scoped_refptr<QuicAckListenerInterface> delegate_; - - // Number of outstanding acks. - int pending_acks_; - - // True if no pending writes remain. - bool wrote_last_data_; - - int num_retransmitted_packets_; - int num_retransmitted_bytes_; - - DISALLOW_COPY_AND_ASSIGN(ProxyAckNotifierDelegate); -}; - ReliableQuicStream::PendingData::PendingData( string data_in, - scoped_refptr<ProxyAckNotifierDelegate> delegate_in) - : data(data_in), offset(0), delegate(delegate_in) { -} + QuicAckListenerInterface* ack_listener_in) + : data(data_in), offset(0), ack_listener(ack_listener_in) {} ReliableQuicStream::PendingData::~PendingData() { } @@ -277,7 +205,7 @@ void ReliableQuicStream::CloseConnectionWithDetails(QuicErrorCode error, void ReliableQuicStream::WriteOrBufferData( StringPiece data, bool fin, - QuicAckListenerInterface* ack_notifier_delegate) { + QuicAckListenerInterface* ack_listener) { if (data.empty() && !fin) { LOG(DFATAL) << "data.empty() && !fin"; return; @@ -292,34 +220,20 @@ void ReliableQuicStream::WriteOrBufferData( return; } - scoped_refptr<ProxyAckNotifierDelegate> proxy_delegate; - if (ack_notifier_delegate != nullptr) { - proxy_delegate = new ProxyAckNotifierDelegate(ack_notifier_delegate); - } - QuicConsumedData consumed_data(0, false); fin_buffered_ = fin; if (queued_data_.empty()) { struct iovec iov(MakeIovec(data)); - consumed_data = WritevData(&iov, 1, fin, proxy_delegate.get()); + consumed_data = WritevData(&iov, 1, fin, ack_listener); DCHECK_LE(consumed_data.bytes_consumed, data.length()); } - bool write_completed; // If there's unconsumed data or an unconsumed fin, queue it. if (consumed_data.bytes_consumed < data.length() || (fin && !consumed_data.fin_consumed)) { StringPiece remainder(data.substr(consumed_data.bytes_consumed)); - queued_data_.push_back(PendingData(remainder.as_string(), proxy_delegate)); - write_completed = false; - } else { - write_completed = true; - } - - if ((proxy_delegate.get() != nullptr) && - (consumed_data.bytes_consumed > 0 || consumed_data.fin_consumed)) { - proxy_delegate->WroteData(write_completed, consumed_data.bytes_consumed); + queued_data_.push_back(PendingData(remainder.as_string(), ack_listener)); } } @@ -327,7 +241,7 @@ void ReliableQuicStream::OnCanWrite() { bool fin = false; while (!queued_data_.empty()) { PendingData* pending_data = &queued_data_.front(); - ProxyAckNotifierDelegate* delegate = pending_data->delegate.get(); + QuicAckListenerInterface* ack_listener = pending_data->ack_listener.get(); if (queued_data_.size() == 1 && fin_buffered_) { fin = true; } @@ -344,19 +258,13 @@ void ReliableQuicStream::OnCanWrite() { struct iovec iov = { const_cast<char*>(pending_data->data.data()) + pending_data->offset, remaining_len}; - QuicConsumedData consumed_data = WritevData(&iov, 1, fin, delegate); + QuicConsumedData consumed_data = WritevData(&iov, 1, fin, ack_listener); if (consumed_data.bytes_consumed == remaining_len && fin == consumed_data.fin_consumed) { queued_data_.pop_front(); - if (delegate != nullptr) { - delegate->WroteData(true, consumed_data.bytes_consumed); - } } else { if (consumed_data.bytes_consumed > 0) { pending_data->offset += consumed_data.bytes_consumed; - if (delegate != nullptr) { - delegate->WroteData(false, consumed_data.bytes_consumed); - } } break; } @@ -383,7 +291,7 @@ QuicConsumedData ReliableQuicStream::WritevData( const struct iovec* iov, int iov_count, bool fin, - QuicAckListenerInterface* ack_notifier_delegate) { + QuicAckListenerInterface* ack_listener) { if (write_side_closed_) { DLOG(ERROR) << ENDPOINT << "Attempt to write when the write side is closed"; return QuicConsumedData(0, false); @@ -418,7 +326,7 @@ QuicConsumedData ReliableQuicStream::WritevData( QuicConsumedData consumed_data = session()->WritevData( id(), QuicIOVector(iov, iov_count, write_length), stream_bytes_written_, - fin, GetFecProtection(), ack_notifier_delegate); + fin, GetFecProtection(), ack_listener); stream_bytes_written_ += consumed_data.bytes_consumed; AddBytesSent(consumed_data.bytes_consumed); diff --git a/net/quic/reliable_quic_stream.h b/net/quic/reliable_quic_stream.h index 200647c..1c696af 100644 --- a/net/quic/reliable_quic_stream.h +++ b/net/quic/reliable_quic_stream.h @@ -27,7 +27,6 @@ #include "base/strings/string_piece.h" #include "net/base/iovec.h" #include "net/base/net_export.h" -#include "net/quic/quic_ack_notifier.h" #include "net/quic/quic_flow_controller.h" #include "net/quic/quic_protocol.h" #include "net/quic/quic_stream_sequencer.h" @@ -172,17 +171,17 @@ class NET_EXPORT_PRIVATE ReliableQuicStream { // write_side_closed() becomes true, otherwise fin_buffered_ becomes true. void WriteOrBufferData(base::StringPiece data, bool fin, - QuicAckListenerInterface* ack_notifier_delegate); + QuicAckListenerInterface* ack_listener); // Sends as many bytes in the first |count| buffers of |iov| to the connection // as the connection will consume. - // If |ack_notifier_delegate| is provided, then it will be notified once all + // If |ack_listener| is provided, then it will be notified once all // the ACKs for this write have been received. // Returns the number of bytes consumed by the connection. QuicConsumedData WritevData(const struct iovec* iov, int iov_count, bool fin, - QuicAckListenerInterface* ack_notifier_delegate); + QuicAckListenerInterface* ack_listener); // Close the write side of the socket. Further writes will fail. // Can be called by the subclass or internally. @@ -207,7 +206,6 @@ class NET_EXPORT_PRIVATE ReliableQuicStream { private: friend class test::ReliableQuicStreamPeer; friend class QuicStreamUtils; - class ProxyAckNotifierDelegate; // Close the read side of the socket. May cause the stream to be closed. // Subclasses and consumers should use StopReading to terminate reading early. @@ -217,17 +215,16 @@ class NET_EXPORT_PRIVATE ReliableQuicStream { bool read_side_closed() const { return read_side_closed_; } struct PendingData { - PendingData(std::string data_in, - scoped_refptr<ProxyAckNotifierDelegate> delegate_in); + PendingData(std::string data_in, QuicAckListenerInterface* ack_listener_in); ~PendingData(); // Pending data to be written. std::string data; // Index of the first byte in data still to be written. size_t offset; - // Delegate that should be notified when the pending data is acked. + // AckListener that should be notified when the pending data is acked. // Can be nullptr. - scoped_refptr<ProxyAckNotifierDelegate> delegate; + scoped_refptr<QuicAckListenerInterface> ack_listener; }; // Calls MaybeSendBlocked on the stream's flow controller and the connection diff --git a/net/quic/reliable_quic_stream_test.cc b/net/quic/reliable_quic_stream_test.cc index fdc2e54..8fe2be6 100644 --- a/net/quic/reliable_quic_stream_test.cc +++ b/net/quic/reliable_quic_stream_test.cc @@ -4,7 +4,6 @@ #include "net/quic/reliable_quic_stream.h" -#include "net/quic/quic_ack_notifier.h" #include "net/quic/quic_connection.h" #include "net/quic/quic_flags.h" #include "net/quic/quic_utils.h" @@ -427,17 +426,17 @@ TEST_F(ReliableQuicStreamTest, StreamFlowControlMultipleWindowUpdates) { QuicFlowControllerPeer::SendWindowOffset(stream_->flow_controller())); } -void SaveProxyAckNotifierDelegate( - scoped_refptr<QuicAckListenerInterface>* delegate_out, - QuicAckListenerInterface* delegate) { - *delegate_out = delegate; +// TODO(ianswett): It's not clear this method is still needed now that +// ProxyAckNotifierDelegate has been removed. +void SaveAckListener(scoped_refptr<QuicAckListenerInterface>* listener_out, + QuicAckListenerInterface* listener) { + *listener_out = (listener); } TEST_F(ReliableQuicStreamTest, WriteOrBufferDataWithQuicAckNotifier) { Initialize(kShouldProcessData); - scoped_refptr<MockAckNotifierDelegate> delegate( - new StrictMock<MockAckNotifierDelegate>); + scoped_refptr<MockAckListener> delegate(new StrictMock<MockAckListener>); const int kDataSize = 16 * 1024; const string kData(kDataSize, 'a'); @@ -450,40 +449,30 @@ TEST_F(ReliableQuicStreamTest, WriteOrBufferDataWithQuicAckNotifier) { stream_->flow_controller()->UpdateSendWindowOffset(kDataSize + 1); session_->flow_controller()->UpdateSendWindowOffset(kDataSize + 1); - scoped_refptr<QuicAckListenerInterface> proxy_delegate; + scoped_refptr<QuicAckListenerInterface> ack_listener; EXPECT_CALL(*session_, WritevData(kTestStreamId, _, _, _, _, _)) - .WillOnce(DoAll(WithArgs<5>(Invoke(CreateFunctor( - &SaveProxyAckNotifierDelegate, &proxy_delegate))), - Return(QuicConsumedData(kFirstWriteSize, false)))); + .WillOnce(DoAll( + WithArgs<5>(Invoke(CreateFunctor(SaveAckListener, &ack_listener))), + Return(QuicConsumedData(kFirstWriteSize, false)))); stream_->WriteOrBufferData(kData, false, delegate.get()); EXPECT_TRUE(HasWriteBlockedStreams()); EXPECT_CALL(*session_, - WritevData(kTestStreamId, _, _, _, _, proxy_delegate.get())) + WritevData(kTestStreamId, _, _, _, _, ack_listener.get())) .WillOnce(Return(QuicConsumedData(kSecondWriteSize, false))); stream_->OnCanWrite(); // No ack expected for an empty write. EXPECT_CALL(*session_, - WritevData(kTestStreamId, _, _, _, _, proxy_delegate.get())) + WritevData(kTestStreamId, _, _, _, _, ack_listener.get())) .WillOnce(Return(QuicConsumedData(0, false))); stream_->OnCanWrite(); EXPECT_CALL(*session_, - WritevData(kTestStreamId, _, _, _, _, proxy_delegate.get())) + WritevData(kTestStreamId, _, _, _, _, ack_listener.get())) .WillOnce(Return(QuicConsumedData(kLastWriteSize, false))); stream_->OnCanWrite(); - - // There were two writes, so OnAckNotification is not propagated until the - // third Ack arrives. - proxy_delegate->OnAckNotification(3, 4, zero_); - proxy_delegate->OnAckNotification(30, 40, zero_); - - // The arguments to delegate->OnAckNotification are the sum of the - // arguments to proxy_delegate OnAckNotification calls. - EXPECT_CALL(*delegate.get(), OnAckNotification(333, 444, zero_)); - proxy_delegate->OnAckNotification(300, 400, zero_); } // Verify delegate behavior when packets are acked before the WritevData call @@ -491,8 +480,7 @@ TEST_F(ReliableQuicStreamTest, WriteOrBufferDataWithQuicAckNotifier) { TEST_F(ReliableQuicStreamTest, WriteOrBufferDataAckNotificationBeforeFlush) { Initialize(kShouldProcessData); - scoped_refptr<MockAckNotifierDelegate> delegate( - new StrictMock<MockAckNotifierDelegate>); + scoped_refptr<MockAckListener> ack_listener(new StrictMock<MockAckListener>); const int kDataSize = 16 * 1024; const string kData(kDataSize, 'a'); @@ -506,55 +494,40 @@ TEST_F(ReliableQuicStreamTest, WriteOrBufferDataAckNotificationBeforeFlush) { scoped_refptr<QuicAckListenerInterface> proxy_delegate; EXPECT_CALL(*session_, WritevData(kTestStreamId, _, _, _, _, _)) - .WillOnce(DoAll(WithArgs<5>(Invoke(CreateFunctor( - &SaveProxyAckNotifierDelegate, &proxy_delegate))), - Return(QuicConsumedData(kInitialWriteSize, false)))); - stream_->WriteOrBufferData(kData, false, delegate.get()); + .WillOnce(DoAll( + WithArgs<5>(Invoke(CreateFunctor(SaveAckListener, &proxy_delegate))), + Return(QuicConsumedData(kInitialWriteSize, false)))); + stream_->WriteOrBufferData(kData, false, ack_listener.get()); EXPECT_TRUE(HasWriteBlockedStreams()); - // Handle the ack of the first write. - proxy_delegate->OnAckNotification(3, 4, zero_); - proxy_delegate = nullptr; - EXPECT_CALL(*session_, WritevData(kTestStreamId, _, _, _, _, _)) .WillOnce(DoAll( - WithArgs<5>(Invoke( - CreateFunctor(&SaveProxyAckNotifierDelegate, &proxy_delegate))), + WithArgs<5>(Invoke(CreateFunctor(SaveAckListener, &proxy_delegate))), Return(QuicConsumedData(kDataSize - kInitialWriteSize, false)))); stream_->OnCanWrite(); - - // Handle the ack for the second write. - EXPECT_CALL(*delegate.get(), OnAckNotification(303, 404, zero_)); - proxy_delegate->OnAckNotification(300, 400, zero_); } // Verify delegate behavior when WriteOrBufferData does not buffer. TEST_F(ReliableQuicStreamTest, WriteAndBufferDataWithAckNotiferNoBuffer) { Initialize(kShouldProcessData); - scoped_refptr<MockAckNotifierDelegate> delegate( - new StrictMock<MockAckNotifierDelegate>); + scoped_refptr<MockAckListener> delegate(new StrictMock<MockAckListener>); scoped_refptr<QuicAckListenerInterface> proxy_delegate; EXPECT_CALL(*session_, WritevData(kTestStreamId, _, _, _, _, _)) - .WillOnce(DoAll(WithArgs<5>(Invoke(CreateFunctor( - &SaveProxyAckNotifierDelegate, &proxy_delegate))), - Return(QuicConsumedData(kDataLen, true)))); + .WillOnce(DoAll( + WithArgs<5>(Invoke(CreateFunctor(SaveAckListener, &proxy_delegate))), + Return(QuicConsumedData(kDataLen, true)))); stream_->WriteOrBufferData(kData1, true, delegate.get()); EXPECT_FALSE(HasWriteBlockedStreams()); - - // Handle the ack. - EXPECT_CALL(*delegate.get(), OnAckNotification(3, 4, zero_)); - proxy_delegate->OnAckNotification(3, 4, zero_); } // Verify delegate behavior when WriteOrBufferData buffers all the data. TEST_F(ReliableQuicStreamTest, BufferOnWriteAndBufferDataWithAckNotifer) { Initialize(kShouldProcessData); - scoped_refptr<MockAckNotifierDelegate> delegate( - new StrictMock<MockAckNotifierDelegate>); + scoped_refptr<MockAckListener> delegate(new StrictMock<MockAckListener>); scoped_refptr<QuicAckListenerInterface> proxy_delegate; @@ -564,14 +537,10 @@ TEST_F(ReliableQuicStreamTest, BufferOnWriteAndBufferDataWithAckNotifer) { EXPECT_TRUE(HasWriteBlockedStreams()); EXPECT_CALL(*session_, WritevData(kTestStreamId, _, _, _, _, _)) - .WillOnce(DoAll(WithArgs<5>(Invoke(CreateFunctor( - &SaveProxyAckNotifierDelegate, &proxy_delegate))), - Return(QuicConsumedData(kDataLen, true)))); + .WillOnce(DoAll( + WithArgs<5>(Invoke(CreateFunctor(SaveAckListener, &proxy_delegate))), + Return(QuicConsumedData(kDataLen, true)))); stream_->OnCanWrite(); - - // Handle the ack. - EXPECT_CALL(*delegate.get(), OnAckNotification(3, 4, zero_)); - proxy_delegate->OnAckNotification(3, 4, zero_); } // Verify delegate behavior when WriteOrBufferData when the FIN is @@ -579,28 +548,22 @@ TEST_F(ReliableQuicStreamTest, BufferOnWriteAndBufferDataWithAckNotifer) { TEST_F(ReliableQuicStreamTest, WriteAndBufferDataWithAckNotiferOnlyFinRemains) { Initialize(kShouldProcessData); - scoped_refptr<MockAckNotifierDelegate> delegate( - new StrictMock<MockAckNotifierDelegate>); + scoped_refptr<MockAckListener> delegate(new StrictMock<MockAckListener>); scoped_refptr<QuicAckListenerInterface> proxy_delegate; EXPECT_CALL(*session_, WritevData(kTestStreamId, _, _, _, _, _)) - .WillOnce(DoAll(WithArgs<5>(Invoke(CreateFunctor( - &SaveProxyAckNotifierDelegate, &proxy_delegate))), - Return(QuicConsumedData(kDataLen, false)))); + .WillOnce(DoAll( + WithArgs<5>(Invoke(CreateFunctor(SaveAckListener, &proxy_delegate))), + Return(QuicConsumedData(kDataLen, false)))); stream_->WriteOrBufferData(kData1, true, delegate.get()); EXPECT_TRUE(HasWriteBlockedStreams()); EXPECT_CALL(*session_, WritevData(kTestStreamId, _, _, _, _, _)) - .WillOnce(DoAll(WithArgs<5>(Invoke(CreateFunctor( - &SaveProxyAckNotifierDelegate, &proxy_delegate))), - Return(QuicConsumedData(0, true)))); + .WillOnce(DoAll( + WithArgs<5>(Invoke(CreateFunctor(SaveAckListener, &proxy_delegate))), + Return(QuicConsumedData(0, true)))); stream_->OnCanWrite(); - - // Handle the acks. - proxy_delegate->OnAckNotification(3, 4, zero_); - EXPECT_CALL(*delegate.get(), OnAckNotification(33, 44, zero_)); - proxy_delegate->OnAckNotification(30, 40, zero_); } // Verify that when we receive a packet which violates flow control (i.e. sends diff --git a/net/quic/test_tools/quic_ack_notifier_manager_peer.cc b/net/quic/test_tools/quic_ack_notifier_manager_peer.cc index 3e7f90c..12e1096 100644 --- a/net/quic/test_tools/quic_ack_notifier_manager_peer.cc +++ b/net/quic/test_tools/quic_ack_notifier_manager_peer.cc @@ -2,17 +2,3 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include "net/quic/test_tools/quic_ack_notifier_manager_peer.h" - -#include "net/quic/quic_ack_notifier_manager.h" - -namespace net { -namespace test { - -size_t AckNotifierManagerPeer::GetNumberOfRegisteredPackets( - const AckNotifierManager* manager) { - return manager->ack_notifier_map_.size(); -} - -} // namespace test -} // namespace net diff --git a/net/quic/test_tools/quic_ack_notifier_manager_peer.h b/net/quic/test_tools/quic_ack_notifier_manager_peer.h index ea128d3..12e1096 100644 --- a/net/quic/test_tools/quic_ack_notifier_manager_peer.h +++ b/net/quic/test_tools/quic_ack_notifier_manager_peer.h @@ -2,28 +2,3 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#ifndef NET_QUIC_TEST_TOOLS_QUIC_ACK_NOTIFIER_MANAGER_PEER_H_ -#define NET_QUIC_TEST_TOOLS_QUIC_ACK_NOTIFIER_MANAGER_PEER_H_ - -#include "net/quic/quic_protocol.h" - -namespace net { - -class AckNotifierManager; - -namespace test { - -// Exposes the internal fields of AckNotifierManager for tests. -class AckNotifierManagerPeer { - public: - // Returns total number of packets known to the map. - static size_t GetNumberOfRegisteredPackets(const AckNotifierManager* manager); - - private: - DISALLOW_COPY_AND_ASSIGN(AckNotifierManagerPeer); -}; - -} // namespace test -} // namespace net - -#endif // NET_QUIC_TEST_TOOLS_QUIC_ACK_NOTIFIER_MANAGER_PEER_H_ diff --git a/net/quic/test_tools/quic_test_utils.cc b/net/quic/test_tools/quic_test_utils.cc index e450083..44e2c83b 100644 --- a/net/quic/test_tools/quic_test_utils.cc +++ b/net/quic/test_tools/quic_test_utils.cc @@ -372,9 +372,9 @@ MockLossAlgorithm::MockLossAlgorithm() {} MockLossAlgorithm::~MockLossAlgorithm() {} -MockAckNotifierDelegate::MockAckNotifierDelegate() {} +MockAckListener::MockAckListener() {} -MockAckNotifierDelegate::~MockAckNotifierDelegate() {} +MockAckListener::~MockAckListener() {} MockNetworkChangeVisitor::MockNetworkChangeVisitor() {} diff --git a/net/quic/test_tools/quic_test_utils.h b/net/quic/test_tools/quic_test_utils.h index e8ad9f6..19cc5d6 100644 --- a/net/quic/test_tools/quic_test_utils.h +++ b/net/quic/test_tools/quic_test_utils.h @@ -14,7 +14,6 @@ #include "base/strings/string_piece.h" #include "net/quic/congestion_control/loss_detection_interface.h" #include "net/quic/congestion_control/send_algorithm_interface.h" -#include "net/quic/quic_ack_notifier.h" #include "net/quic/quic_client_session_base.h" #include "net/quic/quic_connection.h" #include "net/quic/quic_framer.h" @@ -433,7 +432,7 @@ class MockQuicSpdySession : public QuicSpdySession { MOCK_METHOD0(CreateOutgoingDynamicStream, QuicSpdyStream*()); MOCK_METHOD6(WritevData, QuicConsumedData(QuicStreamId id, - const QuicIOVector& data, + QuicIOVector data, QuicStreamOffset offset, bool fin, FecProtection fec_protection, @@ -608,14 +607,9 @@ class MockEntropyCalculator : public TestEntropyCalculator { DISALLOW_COPY_AND_ASSIGN(MockEntropyCalculator); }; -class MockAckNotifierDelegate : public QuicAckListenerInterface { +class MockAckListener : public QuicAckListenerInterface { public: - MockAckNotifierDelegate(); - - MOCK_METHOD3(OnAckNotification, - void(int num_retransmitted_packets, - int num_retransmitted_bytes, - QuicTime::Delta delta_largest_observed)); + MockAckListener(); MOCK_METHOD2(OnPacketAcked, void(int acked_bytes, QuicTime::Delta delta_largest_observed)); @@ -624,10 +618,10 @@ class MockAckNotifierDelegate : public QuicAckListenerInterface { protected: // Object is ref counted. - ~MockAckNotifierDelegate() override; + ~MockAckListener() override; private: - DISALLOW_COPY_AND_ASSIGN(MockAckNotifierDelegate); + DISALLOW_COPY_AND_ASSIGN(MockAckListener); }; class MockNetworkChangeVisitor : diff --git a/net/quic/test_tools/reliable_quic_stream_peer.h b/net/quic/test_tools/reliable_quic_stream_peer.h index 7a2f611..848a5c0 100644 --- a/net/quic/test_tools/reliable_quic_stream_peer.h +++ b/net/quic/test_tools/reliable_quic_stream_peer.h @@ -7,7 +7,6 @@ #include "base/basictypes.h" #include "base/strings/string_piece.h" -#include "net/quic/quic_ack_notifier.h" #include "net/quic/quic_protocol.h" namespace net { diff --git a/net/tools/quic/end_to_end_test.cc b/net/tools/quic/end_to_end_test.cc index 2b4e789..b99cf1e 100644 --- a/net/tools/quic/end_to_end_test.cc +++ b/net/tools/quic/end_to_end_test.cc @@ -248,7 +248,7 @@ class EndToEndTest : public ::testing::TestWithParam<TestParams> { server_hostname_("example.com"), server_started_(false), strike_register_no_startup_period_(false), - stream_creator_(nullptr) { + stream_factory_(nullptr) { client_supported_versions_ = GetParam().client_supported_versions; server_supported_versions_ = GetParam().server_supported_versions; negotiated_version_ = GetParam().negotiated_version; @@ -413,9 +413,9 @@ class EndToEndTest : public ::testing::TestWithParam<TestParams> { server_writer_->Initialize( QuicDispatcherPeer::GetHelper(dispatcher), new ServerDelegate(packet_writer_factory, dispatcher)); - if (stream_creator_ != nullptr) { + if (stream_factory_ != nullptr) { static_cast<QuicTestServer*>(server_thread_->server()) - ->SetSpdyStreamCreator(stream_creator_); + ->SetSpdyStreamFactory(stream_factory_); } server_thread_->Start(); @@ -516,9 +516,8 @@ class EndToEndTest : public ::testing::TestWithParam<TestParams> { } // Must be called before Initialize to have effect. - void SetSpdyStreamCreator(QuicTestServer::StreamCreationFunction function) { - // TODO(rtenneti) use std::move when supported. - stream_creator_ = function; + void SetSpdyStreamFactory(QuicTestServer::StreamFactory* factory) { + stream_factory_ = factory; } bool initialized_; @@ -535,7 +534,7 @@ class EndToEndTest : public ::testing::TestWithParam<TestParams> { QuicVersionVector server_supported_versions_; QuicVersion negotiated_version_; bool strike_register_no_startup_period_; - QuicTestServer::StreamCreationFunction stream_creator_; + QuicTestServer::StreamFactory* stream_factory_; }; // Run all end to end tests with all supported versions. @@ -935,7 +934,14 @@ TEST_P(EndToEndTest, StatelessRejectWithPacketLoss) { // TODO(jokulik): Once redundant SREJ support is added, this test // should succeed. server_writer_->set_fake_drop_first_n_packets(1); - ASSERT_EQ(!BothSidesSupportStatelessRejects(), Initialize()); + // If this test will involve version negotiation then the version + // negotiation packet will be dropped, not the SREJ, and since the + // version negotiation packet will be retransmitted the test will + // succeed. + const bool will_succeed = + !BothSidesSupportStatelessRejects() || + negotiated_version_ != client_supported_versions_.front(); + ASSERT_EQ(will_succeed, Initialize()); } TEST_P(EndToEndTest, SetInitialReceivedConnectionOptions) { @@ -1654,19 +1660,11 @@ TEST_P(EndToEndTest, RequestWithNoBodyWillNeverSendStreamFrameWithFIN) { server_thread_->Resume(); } -// A TestAckNotifierDelegate verifies that its OnAckNotification method has been +// A TestAckListener verifies that its OnAckNotification method has been // called exactly once on destruction. -class TestAckNotifierDelegate : public QuicAckListenerInterface { +class TestAckListener : public QuicAckListenerInterface { public: - explicit TestAckNotifierDelegate(int num_packets) - : num_notifications_(num_packets) {} - - void OnAckNotification(int /*num_retransmitted_packets*/, - int /*num_retransmitted_bytes*/, - QuicTime::Delta /*delta_largest_observed*/) override { - ASSERT_LT(0, num_notifications_); - num_notifications_ = 0; - } + explicit TestAckListener(int num_packets) : num_notifications_(num_packets) {} void OnPacketAcked(int /*acked_bytes*/, QuicTime::Delta /*delta_largest_observed*/) override { @@ -1680,7 +1678,7 @@ class TestAckNotifierDelegate : public QuicAckListenerInterface { protected: // Object is ref counted. - ~TestAckNotifierDelegate() override { EXPECT_EQ(0, num_notifications_); } + ~TestAckListener() override { EXPECT_EQ(0, num_notifications_); } private: int num_notifications_; @@ -1708,9 +1706,8 @@ TEST_P(EndToEndTest, AckNotifierWithPacketLossAndBlockedSocket) { request.set_has_complete_message(false); client_->SendMessage(request); - // The TestAckNotifierDelegate will cause a failure if not notified. - scoped_refptr<TestAckNotifierDelegate> delegate( - new TestAckNotifierDelegate(2)); + // The TestAckListener will cause a failure if not notified. + scoped_refptr<TestAckListener> delegate(new TestAckListener(2)); // Test the AckNotifier's ability to track multiple packets by making the // request body exceed the size of a single packet. @@ -1975,6 +1972,22 @@ class ServerStreamWithErrorResponseBody : public QuicSpdyServerStream { string response_body_; }; +class StreamWithErrorFactory : public QuicTestServer::StreamFactory { + public: + explicit StreamWithErrorFactory(string response_body) + : response_body_(response_body) {} + + ~StreamWithErrorFactory() override {} + + QuicSpdyServerStream* CreateStream(QuicStreamId id, + QuicSpdySession* session) override { + return new ServerStreamWithErrorResponseBody(id, session, response_body_); + } + + private: + string response_body_; +}; + TEST_P(EndToEndTest, EarlyResponseFinRecording) { set_smaller_flow_control_receive_window(); @@ -1993,10 +2006,9 @@ TEST_P(EndToEndTest, EarlyResponseFinRecording) { uint32 response_body_size = 2 * client_config_.GetInitialStreamFlowControlWindowToSend(); GenerateBody(&response_body, response_body_size); - SetSpdyStreamCreator([response_body](QuicStreamId id, - QuicSpdySession* session) { - return new ServerStreamWithErrorResponseBody(id, session, response_body); - }); + + StreamWithErrorFactory stream_factory(response_body); + SetSpdyStreamFactory(&stream_factory); ASSERT_TRUE(Initialize()); diff --git a/net/tools/quic/quic_client_session_test.cc b/net/tools/quic/quic_client_session_test.cc index fc3748e..541f290 100644 --- a/net/tools/quic/quic_client_session_test.cc +++ b/net/tools/quic/quic_client_session_test.cc @@ -77,7 +77,7 @@ TEST_P(ToolsQuicClientSessionTest, CryptoConnect) { CompleteCryptoHandshake(); } -TEST_P(ToolsQuicClientSessionTest, MaxNumStreams) { +TEST_P(ToolsQuicClientSessionTest, MaxNumStreamsWithNoFinOrRst) { EXPECT_CALL(*connection_, SendRstStream(_, _, _)).Times(AnyNumber()); session_->config()->SetMaxStreamsPerConnection(1, 1); @@ -89,8 +89,41 @@ TEST_P(ToolsQuicClientSessionTest, MaxNumStreams) { ASSERT_TRUE(stream); EXPECT_FALSE(session_->CreateOutgoingDynamicStream()); - // Close a stream and ensure I can now open a new one. + // Close the stream, but without having received a FIN or a RST_STREAM + // and check that a new one can not be created. session_->CloseStream(stream->id()); + if (FLAGS_quic_count_unfinished_as_open_streams) { + EXPECT_EQ(1u, session_->GetNumOpenStreams()); + } else { + EXPECT_EQ(0u, session_->GetNumOpenStreams()); + } + stream = session_->CreateOutgoingDynamicStream(); + if (FLAGS_quic_count_unfinished_as_open_streams) { + EXPECT_FALSE(stream); + } else { + EXPECT_TRUE(stream); + } +} + +TEST_P(ToolsQuicClientSessionTest, MaxNumStreamsWithRst) { + EXPECT_CALL(*connection_, SendRstStream(_, _, _)).Times(AnyNumber()); + + session_->config()->SetMaxStreamsPerConnection(1, 1); + + // Initialize crypto before the client session will create a stream. + CompleteCryptoHandshake(); + + QuicSpdyClientStream* stream = session_->CreateOutgoingDynamicStream(); + ASSERT_TRUE(stream); + EXPECT_FALSE(session_->CreateOutgoingDynamicStream()); + + // Close the stream and receive an RST frame to remove the unfinished stream + session_->CloseStream(stream->id()); + session_->OnRstStream(QuicRstStreamFrame( + stream->id(), AdjustErrorForVersion(QUIC_RST_ACKNOWLEDGEMENT, GetParam()), + 0)); + // Check that a new one can be created. + EXPECT_EQ(0u, session_->GetNumOpenStreams()); stream = session_->CreateOutgoingDynamicStream(); EXPECT_TRUE(stream); } diff --git a/net/tools/quic/quic_dispatcher.cc b/net/tools/quic/quic_dispatcher.cc index 751d9bc..47d47ff 100644 --- a/net/tools/quic/quic_dispatcher.cc +++ b/net/tools/quic/quic_dispatcher.cc @@ -299,20 +299,9 @@ void QuicDispatcher::OnUnauthenticatedHeader(const QuicPacketHeader& header) { QuicServerSession* session = CreateQuicSession(connection_id, current_client_address_); DVLOG(1) << "Created new session for " << connection_id; - session_map_.insert(make_pair(connection_id, session)); + session_map_.insert(std::make_pair(connection_id, session)); session->connection()->ProcessUdpPacket( current_server_address_, current_client_address_, *current_packet_); - - if (FLAGS_enable_quic_stateless_reject_support && - session->UsingStatelessRejectsIfPeerSupported() && - session->PeerSupportsStatelessRejects() && - !session->IsCryptoHandshakeConfirmed()) { - DVLOG(1) << "Removing new session for " << connection_id - << " because the session is in stateless reject mode and" - << " encryption has not been established."; - session->connection()->CloseConnection( - QUIC_CRYPTO_HANDSHAKE_STATELESS_REJECT, /* from_peer */ false); - } break; } case kFateTimeWait: diff --git a/net/tools/quic/quic_dispatcher_test.cc b/net/tools/quic/quic_dispatcher_test.cc index d414fae..b54ee29 100644 --- a/net/tools/quic/quic_dispatcher_test.cc +++ b/net/tools/quic/quic_dispatcher_test.cc @@ -499,15 +499,18 @@ TEST_P(QuicDispatcherStatelessRejectTest, ParameterizedBasicTest) { CreateSessionBasedOnTestParams(connection_id, client_address))); // Process the first packet for the connection. + ProcessPacket(client_address, connection_id, true, "foo"); if (ExpectStatelessReject()) { - // If this is a stateless reject, we expect the connection to close. + // If this is a stateless reject, the crypto stream will close the + // connection. EXPECT_CALL(*session1_, OnConnectionClosed(_, _)) .Times(1) .WillOnce(WithoutArgs(Invoke( reinterpret_cast<MockServerConnection*>(session1_->connection()), &MockServerConnection::UnregisterOnConnectionClosed))); + session1_->connection()->CloseConnection( + QUIC_CRYPTO_HANDSHAKE_STATELESS_REJECT, /* from_peer */ false); } - ProcessPacket(client_address, connection_id, true, "foo"); // Send a second packet and check the results. If this is a stateless reject, // the existing connection_id will go on the time-wait list. diff --git a/net/tools/quic/quic_spdy_client_stream.cc b/net/tools/quic/quic_spdy_client_stream.cc index df68276..817366c 100644 --- a/net/tools/quic/quic_spdy_client_stream.cc +++ b/net/tools/quic/quic_spdy_client_stream.cc @@ -124,8 +124,8 @@ void QuicSpdyClientStream::SendBody(const string& data, bool fin) { void QuicSpdyClientStream::SendBody(const string& data, bool fin, - QuicAckListenerInterface* delegate) { - WriteOrBufferData(data, fin, delegate); + QuicAckListenerInterface* listener) { + WriteOrBufferData(data, fin, listener); } } // namespace tools diff --git a/net/tools/quic/quic_spdy_client_stream.h b/net/tools/quic/quic_spdy_client_stream.h index a031ded..6aa7824 100644 --- a/net/tools/quic/quic_spdy_client_stream.h +++ b/net/tools/quic/quic_spdy_client_stream.h @@ -49,7 +49,7 @@ class QuicSpdyClientStream : public QuicSpdyStream { // As above, but |delegate| will be notified once |data| is ACKed. void SendBody(const std::string& data, bool fin, - QuicAckListenerInterface* delegate); + QuicAckListenerInterface* listener); // Returns the response data. const std::string& data() { return data_; } diff --git a/net/tools/quic/quic_spdy_server_stream_test.cc b/net/tools/quic/quic_spdy_server_stream_test.cc index 23e507c..0ba92ad 100644 --- a/net/tools/quic/quic_spdy_server_stream_test.cc +++ b/net/tools/quic/quic_spdy_server_stream_test.cc @@ -101,7 +101,9 @@ class QuicSpdyServerStreamTest : public ::testing::Test { kInitialStreamFlowControlWindowForTest); session_.config()->SetInitialSessionFlowControlWindowToSend( kInitialSessionFlowControlWindowForTest); - stream_.reset(new QuicSpdyServerStreamPeer(3, &session_)); + stream_ = new QuicSpdyServerStreamPeer(5, &session_); + // Register stream_ in dynamic_stream_map_ and pass ownership to session_. + session_.ActivateStream(stream_); QuicInMemoryCachePeer::ResetForTests(); @@ -117,9 +119,7 @@ class QuicSpdyServerStreamTest : public ::testing::Test { QuicInMemoryCachePeer::ResetForTests(); } - const string& StreamBody() { - return QuicSpdyServerStreamPeer::body(stream_.get()); - } + const string& StreamBody() { return QuicSpdyServerStreamPeer::body(stream_); } StringPiece StreamHeadersValue(const string& key) { return (*stream_->mutable_headers())[key]; @@ -129,7 +129,7 @@ class QuicSpdyServerStreamTest : public ::testing::Test { MockHelper helper_; StrictMock<MockConnection>* connection_; StrictMock<MockQuicSpdySession> session_; - scoped_ptr<QuicSpdyServerStreamPeer> stream_; + QuicSpdyServerStreamPeer* stream_; // Owned by session_. string headers_string_; string body_; }; @@ -207,11 +207,11 @@ TEST_F(QuicSpdyServerStreamTest, TestSendResponse) { EXPECT_CALL(session_, WritevData(_, _, _, _, _, _)).Times(1). WillOnce(Return(QuicConsumedData(3, true))); - QuicSpdyServerStreamPeer::SendResponse(stream_.get()); + QuicSpdyServerStreamPeer::SendResponse(stream_); if (!FLAGS_quic_implement_stop_reading) { - EXPECT_TRUE(ReliableQuicStreamPeer::read_side_closed(stream_.get())); + EXPECT_TRUE(ReliableQuicStreamPeer::read_side_closed(stream_)); } else { - EXPECT_FALSE(ReliableQuicStreamPeer::read_side_closed(stream_.get())); + EXPECT_FALSE(ReliableQuicStreamPeer::read_side_closed(stream_)); } EXPECT_TRUE(stream_->reading_stopped()); EXPECT_TRUE(stream_->write_side_closed()); @@ -227,11 +227,11 @@ TEST_F(QuicSpdyServerStreamTest, TestSendErrorResponse) { EXPECT_CALL(session_, WritevData(_, _, _, _, _, _)).Times(1). WillOnce(Return(QuicConsumedData(3, true))); - QuicSpdyServerStreamPeer::SendErrorResponse(stream_.get()); + QuicSpdyServerStreamPeer::SendErrorResponse(stream_); if (!FLAGS_quic_implement_stop_reading) { - EXPECT_TRUE(ReliableQuicStreamPeer::read_side_closed(stream_.get())); + EXPECT_TRUE(ReliableQuicStreamPeer::read_side_closed(stream_)); } else { - EXPECT_FALSE(ReliableQuicStreamPeer::read_side_closed(stream_.get())); + EXPECT_FALSE(ReliableQuicStreamPeer::read_side_closed(stream_)); } EXPECT_TRUE(stream_->reading_stopped()); EXPECT_TRUE(stream_->write_side_closed()); @@ -252,9 +252,9 @@ TEST_F(QuicSpdyServerStreamTest, InvalidMultipleContentLength) { stream_->OnStreamHeadersComplete(false, headers_string_.size()); if (!FLAGS_quic_implement_stop_reading) { - EXPECT_TRUE(ReliableQuicStreamPeer::read_side_closed(stream_.get())); + EXPECT_TRUE(ReliableQuicStreamPeer::read_side_closed(stream_)); } else { - EXPECT_FALSE(ReliableQuicStreamPeer::read_side_closed(stream_.get())); + EXPECT_FALSE(ReliableQuicStreamPeer::read_side_closed(stream_)); } EXPECT_TRUE(stream_->reading_stopped()); EXPECT_TRUE(stream_->write_side_closed()); @@ -275,9 +275,9 @@ TEST_F(QuicSpdyServerStreamTest, InvalidLeadingNullContentLength) { stream_->OnStreamHeadersComplete(false, headers_string_.size()); if (!FLAGS_quic_implement_stop_reading) { - EXPECT_TRUE(ReliableQuicStreamPeer::read_side_closed(stream_.get())); + EXPECT_TRUE(ReliableQuicStreamPeer::read_side_closed(stream_)); } else { - EXPECT_FALSE(ReliableQuicStreamPeer::read_side_closed(stream_.get())); + EXPECT_FALSE(ReliableQuicStreamPeer::read_side_closed(stream_)); } EXPECT_TRUE(stream_->reading_stopped()); EXPECT_TRUE(stream_->write_side_closed()); @@ -293,8 +293,8 @@ TEST_F(QuicSpdyServerStreamTest, ValidMultipleContentLength) { stream_->OnStreamHeaders(headers_string_); stream_->OnStreamHeadersComplete(false, headers_string_.size()); - EXPECT_EQ(11, QuicSpdyServerStreamPeer::content_length(stream_.get())); - EXPECT_FALSE(ReliableQuicStreamPeer::read_side_closed(stream_.get())); + EXPECT_EQ(11, QuicSpdyServerStreamPeer::content_length(stream_)); + EXPECT_FALSE(ReliableQuicStreamPeer::read_side_closed(stream_)); EXPECT_FALSE(stream_->reading_stopped()); EXPECT_FALSE(stream_->write_side_closed()); } diff --git a/net/tools/quic/test_tools/quic_test_server.cc b/net/tools/quic/test_tools/quic_test_server.cc index 35bc3d4..8d5bd487 100644 --- a/net/tools/quic/test_tools/quic_test_server.cc +++ b/net/tools/quic/test_tools/quic_test_server.cc @@ -32,20 +32,19 @@ class CustomStreamSession : public QuicServerSession { QuicConnection* connection, QuicServerSessionVisitor* visitor, const QuicCryptoServerConfig* crypto_config, - QuicTestServer::StreamCreationFunction creator) + QuicTestServer::StreamFactory* factory) : QuicServerSession(config, connection, visitor, crypto_config), - // TODO(rtenneti): use std::move when chromium supports it. - stream_creator_(creator) {} + stream_factory_(factory) {} QuicSpdyStream* CreateIncomingDynamicStream(QuicStreamId id) override { if (!ShouldCreateIncomingDynamicStream(id)) { return nullptr; } - return stream_creator_(id, this); + return stream_factory_->CreateStream(id, this); } private: - QuicTestServer::StreamCreationFunction stream_creator_; + QuicTestServer::StreamFactory* stream_factory_; // Not owned. }; class QuicTestDispatcher : public QuicDispatcher { @@ -55,10 +54,13 @@ class QuicTestDispatcher : public QuicDispatcher { const QuicVersionVector& versions, PacketWriterFactory* factory, QuicConnectionHelperInterface* helper) - : QuicDispatcher(config, crypto_config, versions, factory, helper) {} + : QuicDispatcher(config, crypto_config, versions, factory, helper), + session_factory_(nullptr), + stream_factory_(nullptr) {} + QuicServerSession* CreateQuicSession(QuicConnectionId id, const IPEndPoint& client) override { - if (session_creator_ == nullptr && stream_creator_ == nullptr) { + if (session_factory_ == nullptr && stream_factory_ == nullptr) { return QuicDispatcher::CreateQuicSession(id, client); } QuicConnection* connection = new QuicConnection( @@ -66,43 +68,36 @@ class QuicTestDispatcher : public QuicDispatcher { /* owns_writer= */ true, Perspective::IS_SERVER, supported_versions()); QuicServerSession* session = nullptr; - if (stream_creator_ != nullptr) { + if (stream_factory_ != nullptr) { session = new CustomStreamSession(config(), connection, this, - crypto_config(), stream_creator_); + crypto_config(), stream_factory_); } else { - session = session_creator_(config(), connection, this, crypto_config()); + session = session_factory_->CreateSession(config(), connection, this, + crypto_config()); } session->Initialize(); return session; } - void set_session_creator(QuicTestServer::SessionCreationFunction function) { - DCHECK(session_creator_ == nullptr); - DCHECK(stream_creator_ == nullptr); - // TODO(rtenneti): use std::move when chromium supports it. - // session_creator_ = std::move(function); - session_creator_ = function; + void set_session_factory(QuicTestServer::SessionFactory* factory) { + DCHECK(session_factory_ == nullptr); + DCHECK(stream_factory_ == nullptr); + session_factory_ = factory; } - void set_stream_creator(QuicTestServer::StreamCreationFunction function) { - DCHECK(session_creator_ == nullptr); - DCHECK(stream_creator_ == nullptr); - // TODO(rtenneti): use std::move when chromium supports it. - // stream_creator_ = std::move(function); - stream_creator_ = function; + void set_stream_factory(QuicTestServer::StreamFactory* factory) { + DCHECK(session_factory_ == nullptr); + DCHECK(stream_factory_ == nullptr); + stream_factory_ = factory; } - QuicTestServer::SessionCreationFunction session_creator() { - return session_creator_; - } + QuicTestServer::SessionFactory* session_factory() { return session_factory_; } - QuicTestServer::StreamCreationFunction stream_creator() { - return stream_creator_; - } + QuicTestServer::StreamFactory* stream_factory() { return stream_factory_; } private: - QuicTestServer::SessionCreationFunction session_creator_; - QuicTestServer::StreamCreationFunction stream_creator_; + QuicTestServer::SessionFactory* session_factory_; // Not owned. + QuicTestServer::StreamFactory* stream_factory_; // Not owned. }; QuicTestServer::QuicTestServer(ProofSource* proof_source) @@ -120,15 +115,13 @@ QuicDispatcher* QuicTestServer::CreateQuicDispatcher() { new QuicEpollConnectionHelper(epoll_server())); } -void QuicTestServer::SetSessionCreator(SessionCreationFunction function) { - static_cast<QuicTestDispatcher*>(dispatcher()) - ->set_session_creator(std::move(function)); +void QuicTestServer::SetSessionFactory(SessionFactory* factory) { + DCHECK(dispatcher()); + static_cast<QuicTestDispatcher*>(dispatcher())->set_session_factory(factory); } -void QuicTestServer::SetSpdyStreamCreator(StreamCreationFunction function) { - DCHECK(dispatcher()); - static_cast<QuicTestDispatcher*>(dispatcher()) - ->set_stream_creator(std::move(function)); +void QuicTestServer::SetSpdyStreamFactory(StreamFactory* factory) { + static_cast<QuicTestDispatcher*>(dispatcher())->set_stream_factory(factory); } /////////////////////////// TEST SESSIONS /////////////////////////////// diff --git a/net/tools/quic/test_tools/quic_test_server.h b/net/tools/quic/test_tools/quic_test_server.h index ad9704c..6a02763 100644 --- a/net/tools/quic/test_tools/quic_test_server.h +++ b/net/tools/quic/test_tools/quic_test_server.h @@ -26,15 +26,28 @@ namespace test { // Eventually this may be extended to allow custom QuicConnections etc. class QuicTestServer : public QuicServer { public: - typedef std::function<QuicServerSession*( - const QuicConfig& config, - QuicConnection* connection, - QuicServerSessionVisitor* visitor, - const QuicCryptoServerConfig* crypto_config)> SessionCreationFunction; - - typedef std::function<QuicSpdyServerStream*(QuicStreamId id, - QuicSpdySession* session)> - StreamCreationFunction; + // Factory for creating QuicServerSessions. + class SessionFactory { + public: + virtual ~SessionFactory() {} + + // Returns a new session owned by the caller. + virtual QuicServerSession* CreateSession( + const QuicConfig& config, + QuicConnection* connection, + QuicServerSessionVisitor* visitor, + const QuicCryptoServerConfig* crypto_config) = 0; + }; + + // Factory for creating QuicServerStreams. + class StreamFactory { + public: + virtual ~StreamFactory() {} + + // Returns a new stream owned by the caller. + virtual QuicSpdyServerStream* CreateStream(QuicStreamId id, + QuicSpdySession* session) = 0; + }; explicit QuicTestServer(ProofSource* proof_source); QuicTestServer(ProofSource* proof_source, @@ -44,13 +57,13 @@ class QuicTestServer : public QuicServer { // Create a custom dispatcher which creates custom sessions. QuicDispatcher* CreateQuicDispatcher() override; - // Sets a custom session creator, for easy custom session logic. - // This is incompatible with setting a stream creator. - void SetSessionCreator(SessionCreationFunction function); + // Sets a custom session factory, owned by the caller, for easy custom + // session logic. This is incompatible with setting a stream factory. + void SetSessionFactory(SessionFactory* factory); - // Sets a custom stream creator, for easy custom stream logic. - // This is incompatible with setting a session creator. - void SetSpdyStreamCreator(StreamCreationFunction function); + // Sets a custom stream factory, owned by the caller, for easy custom + // stream logic. This is incompatible with setting a session factory. + void SetSpdyStreamFactory(StreamFactory* factory); }; // Useful test sessions for the QuicTestServer. |