diff --git a/net/quic/congestion_control/tcp_cubic_sender.cc b/net/quic/congestion_control/tcp_cubic_sender.cc index 30c0be6bc5def4..71830367f76e3e 100644 --- a/net/quic/congestion_control/tcp_cubic_sender.cc +++ b/net/quic/congestion_control/tcp_cubic_sender.cc @@ -60,11 +60,26 @@ TcpCubicSender::~TcpCubicSender() { void TcpCubicSender::SetFromConfig(const QuicConfig& config, Perspective perspective) { if (perspective == Perspective::IS_SERVER) { + if (config.HasReceivedConnectionOptions() && + ContainsQuicTag(config.ReceivedConnectionOptions(), kIW03)) { + // Initial window experiment. + congestion_window_ = 3; + } if (config.HasReceivedConnectionOptions() && ContainsQuicTag(config.ReceivedConnectionOptions(), kIW10)) { // Initial window experiment. congestion_window_ = 10; } + if (config.HasReceivedConnectionOptions() && + ContainsQuicTag(config.ReceivedConnectionOptions(), kIW20)) { + // Initial window experiment. + congestion_window_ = 20; + } + if (config.HasReceivedConnectionOptions() && + ContainsQuicTag(config.ReceivedConnectionOptions(), kIW50)) { + // Initial window experiment. + congestion_window_ = 50; + } if (config.HasReceivedConnectionOptions() && ContainsQuicTag(config.ReceivedConnectionOptions(), kMIN1)) { // Min CWND experiment. diff --git a/net/quic/congestion_control/tcp_cubic_sender_test.cc b/net/quic/congestion_control/tcp_cubic_sender_test.cc index cbcad0f8d52359..804e5611db9215 100644 --- a/net/quic/congestion_control/tcp_cubic_sender_test.cc +++ b/net/quic/congestion_control/tcp_cubic_sender_test.cc @@ -517,12 +517,29 @@ TEST_F(TcpCubicSenderTest, DontTrackAckPackets) { TEST_F(TcpCubicSenderTest, ConfigureInitialWindow) { QuicConfig config; - // Verify that kCOPT: kIW10 forces the congestion window to the default of 10. QuicTagVector options; + options.push_back(kIW03); + QuicConfigPeer::SetReceivedConnectionOptions(&config, options); + sender_->SetFromConfig(config, Perspective::IS_SERVER); + EXPECT_EQ(3u, sender_->congestion_window()); + + options.clear(); options.push_back(kIW10); QuicConfigPeer::SetReceivedConnectionOptions(&config, options); sender_->SetFromConfig(config, Perspective::IS_SERVER); EXPECT_EQ(10u, sender_->congestion_window()); + + options.clear(); + options.push_back(kIW20); + QuicConfigPeer::SetReceivedConnectionOptions(&config, options); + sender_->SetFromConfig(config, Perspective::IS_SERVER); + EXPECT_EQ(20u, sender_->congestion_window()); + + options.clear(); + options.push_back(kIW50); + QuicConfigPeer::SetReceivedConnectionOptions(&config, options); + sender_->SetFromConfig(config, Perspective::IS_SERVER); + EXPECT_EQ(50u, sender_->congestion_window()); } TEST_F(TcpCubicSenderTest, ConfigureMinimumWindow) { diff --git a/net/quic/crypto/aead_base_decrypter.h b/net/quic/crypto/aead_base_decrypter.h index de9e9969b99972..f8d6f1b861a3b4 100644 --- a/net/quic/crypto/aead_base_decrypter.h +++ b/net/quic/crypto/aead_base_decrypter.h @@ -78,13 +78,6 @@ class NET_EXPORT_PRIVATE AeadBaseDecrypter : public QuicDecrypter { #endif // !defined(USE_OPENSSL) private: - bool Decrypt(base::StringPiece nonce, - const base::StringPiece& associated_data, - const base::StringPiece& ciphertext, - uint8* output, - size_t* output_length, - size_t max_output_length); - #if defined(USE_OPENSSL) const EVP_AEAD* const aead_alg_; #else diff --git a/net/quic/crypto/aead_base_decrypter_nss.cc b/net/quic/crypto/aead_base_decrypter_nss.cc index 1ed4f3ab6e2b77..255e88da3f849e 100644 --- a/net/quic/crypto/aead_base_decrypter_nss.cc +++ b/net/quic/crypto/aead_base_decrypter_nss.cc @@ -47,16 +47,22 @@ bool AeadBaseDecrypter::SetNoncePrefix(StringPiece nonce_prefix) { return true; } -bool AeadBaseDecrypter::Decrypt(StringPiece nonce, - const StringPiece& associated_data, - const StringPiece& ciphertext, - uint8* output, - size_t* output_length, - size_t max_output_length) { - if (ciphertext.length() < auth_tag_size_ || - nonce.size() != nonce_prefix_size_ + sizeof(QuicPacketSequenceNumber)) { +bool AeadBaseDecrypter::DecryptPacket(QuicPacketSequenceNumber sequence_number, + const StringPiece& associated_data, + const StringPiece& ciphertext, + char* output, + size_t* output_length, + size_t max_output_length) { + if (ciphertext.length() < auth_tag_size_) { return false; } + + uint8 nonce[sizeof(nonce_prefix_) + sizeof(sequence_number)]; + const size_t nonce_size = nonce_prefix_size_ + sizeof(sequence_number); + DCHECK_LE(nonce_size, sizeof(nonce)); + memcpy(nonce, nonce_prefix_, nonce_prefix_size_); + memcpy(nonce + nonce_prefix_size_, &sequence_number, sizeof(sequence_number)); + // NSS 3.14.x incorrectly requires an output buffer at least as long as // the ciphertext (NSS bug // https://bugzilla.mozilla.org/show_bug.cgi?id= 853674). Fortunately @@ -93,7 +99,8 @@ bool AeadBaseDecrypter::Decrypt(StringPiece nonce, } AeadParams aead_params = {0}; - FillAeadParams(nonce, associated_data, auth_tag_size_, &aead_params); + FillAeadParams(StringPiece(reinterpret_cast(nonce), nonce_size), + associated_data, auth_tag_size_, &aead_params); SECItem param; param.type = siBuffer; @@ -101,8 +108,9 @@ bool AeadBaseDecrypter::Decrypt(StringPiece nonce, param.len = aead_params.len; unsigned int output_len; - if (pk11_decrypt_(aead_key.get(), aead_mechanism_, ¶m, output, - &output_len, max_output_length, + if (pk11_decrypt_(aead_key.get(), aead_mechanism_, ¶m, + reinterpret_cast(output), &output_len, + max_output_length, reinterpret_cast(ciphertext.data()), ciphertext.length()) != SECSuccess) { return false; @@ -116,26 +124,6 @@ bool AeadBaseDecrypter::Decrypt(StringPiece nonce, return true; } -bool AeadBaseDecrypter::DecryptPacket(QuicPacketSequenceNumber sequence_number, - const StringPiece& associated_data, - const StringPiece& ciphertext, - char* output, - size_t* output_length, - size_t max_output_length) { - if (ciphertext.length() < auth_tag_size_) { - return false; - } - - uint8 nonce[sizeof(nonce_prefix_) + sizeof(sequence_number)]; - const size_t nonce_size = nonce_prefix_size_ + sizeof(sequence_number); - DCHECK_LE(nonce_size, sizeof(nonce)); - memcpy(nonce, nonce_prefix_, nonce_prefix_size_); - memcpy(nonce + nonce_prefix_size_, &sequence_number, sizeof(sequence_number)); - return Decrypt(StringPiece(reinterpret_cast(nonce), nonce_size), - associated_data, ciphertext, reinterpret_cast(output), - output_length, max_output_length); -} - StringPiece AeadBaseDecrypter::GetKey() const { return StringPiece(reinterpret_cast(key_), key_size_); } diff --git a/net/quic/crypto/aead_base_decrypter_openssl.cc b/net/quic/crypto/aead_base_decrypter_openssl.cc index 7929388114beb2..95991200a37673 100644 --- a/net/quic/crypto/aead_base_decrypter_openssl.cc +++ b/net/quic/crypto/aead_base_decrypter_openssl.cc @@ -76,32 +76,6 @@ bool AeadBaseDecrypter::SetNoncePrefix(StringPiece nonce_prefix) { return true; } -bool AeadBaseDecrypter::Decrypt(StringPiece nonce, - const StringPiece& associated_data, - const StringPiece& ciphertext, - uint8* output, - size_t* output_length, - size_t max_output_length) { - if (ciphertext.length() < auth_tag_size_ || - nonce.size() != nonce_prefix_size_ + sizeof(QuicPacketSequenceNumber)) { - return false; - } - - if (!EVP_AEAD_CTX_open( - ctx_.get(), output, output_length, max_output_length, - reinterpret_cast(nonce.data()), nonce.size(), - reinterpret_cast(ciphertext.data()), - ciphertext.size(), - reinterpret_cast(associated_data.data()), - associated_data.size())) { - // Because QuicFramer does trial decryption, decryption errors are expected - // when encryption level changes. So we don't log decryption errors. - ClearOpenSslErrors(); - return false; - } - return true; -} - bool AeadBaseDecrypter::DecryptPacket(QuicPacketSequenceNumber sequence_number, const StringPiece& associated_data, const StringPiece& ciphertext, @@ -114,12 +88,21 @@ bool AeadBaseDecrypter::DecryptPacket(QuicPacketSequenceNumber sequence_number, uint8 nonce[sizeof(nonce_prefix_) + sizeof(sequence_number)]; const size_t nonce_size = nonce_prefix_size_ + sizeof(sequence_number); - DCHECK_LE(nonce_size, sizeof(nonce)); memcpy(nonce, nonce_prefix_, nonce_prefix_size_); memcpy(nonce + nonce_prefix_size_, &sequence_number, sizeof(sequence_number)); - return Decrypt(StringPiece(reinterpret_cast(nonce), nonce_size), - associated_data, ciphertext, reinterpret_cast(output), - output_length, max_output_length); + if (!EVP_AEAD_CTX_open( + ctx_.get(), reinterpret_cast(output), output_length, + max_output_length, reinterpret_cast(nonce), + nonce_size, reinterpret_cast(ciphertext.data()), + ciphertext.size(), + reinterpret_cast(associated_data.data()), + associated_data.size())) { + // Because QuicFramer does trial decryption, decryption errors are expected + // when encryption level changes. So we don't log decryption errors. + ClearOpenSslErrors(); + return false; + } + return true; } StringPiece AeadBaseDecrypter::GetKey() const { diff --git a/net/quic/crypto/aead_base_encrypter.h b/net/quic/crypto/aead_base_encrypter.h index 86db32c0253b69..dd8f775caebe19 100644 --- a/net/quic/crypto/aead_base_encrypter.h +++ b/net/quic/crypto/aead_base_encrypter.h @@ -42,10 +42,6 @@ class NET_EXPORT_PRIVATE AeadBaseEncrypter : public QuicEncrypter { // QuicEncrypter implementation bool SetKey(base::StringPiece key) override; bool SetNoncePrefix(base::StringPiece nonce_prefix) override; - bool Encrypt(base::StringPiece nonce, - base::StringPiece associated_data, - base::StringPiece plaintext, - unsigned char* output) override; bool EncryptPacket(QuicPacketSequenceNumber sequence_number, base::StringPiece associated_data, base::StringPiece plaintext, @@ -59,6 +55,13 @@ class NET_EXPORT_PRIVATE AeadBaseEncrypter : public QuicEncrypter { base::StringPiece GetKey() const override; base::StringPiece GetNoncePrefix() const override; + // Necessary so unit tests can explicitly specify a nonce, instead of a + // nonce prefix and sequence number. + bool Encrypt(base::StringPiece nonce, + base::StringPiece associated_data, + base::StringPiece plaintext, + unsigned char* output); + protected: // Make these constants available to the subclasses so that the subclasses // can assert at compile time their key_size_ and nonce_prefix_size_ do not diff --git a/net/quic/crypto/crypto_protocol.h b/net/quic/crypto/crypto_protocol.h index 1f881ea2ea6044..1b4abe95d75f06 100644 --- a/net/quic/crypto/crypto_protocol.h +++ b/net/quic/crypto/crypto_protocol.h @@ -56,7 +56,10 @@ const QuicTag kQBIC = TAG('Q', 'B', 'I', 'C'); // TCP cubic const QuicTag kTBBR = TAG('T', 'B', 'B', 'R'); // Reduced Buffer Bloat TCP const QuicTag kRENO = TAG('R', 'E', 'N', 'O'); // Reno Congestion Control const QuicTag kBYTE = TAG('B', 'Y', 'T', 'E'); // TCP cubic or reno in bytes +const QuicTag kIW03 = TAG('I', 'W', '0', '3'); // Force ICWND to 3 const QuicTag kIW10 = TAG('I', 'W', '1', '0'); // Force ICWND to 10 +const QuicTag kIW20 = TAG('I', 'W', '2', '0'); // Force ICWND to 20 +const QuicTag kIW50 = TAG('I', 'W', '5', '0'); // Force ICWND to 50 const QuicTag k1CON = TAG('1', 'C', 'O', 'N'); // Emulate a single connection const QuicTag kNTLP = TAG('N', 'T', 'L', 'P'); // No tail loss probe const QuicTag kNCON = TAG('N', 'C', 'O', 'N'); // N Connection Congestion Ctrl diff --git a/net/quic/crypto/crypto_secret_boxer.cc b/net/quic/crypto/crypto_secret_boxer.cc index b139a6a50fb8e9..a7898d995a43e5 100644 --- a/net/quic/crypto/crypto_secret_boxer.cc +++ b/net/quic/crypto/crypto_secret_boxer.cc @@ -6,6 +6,8 @@ #include "base/logging.h" #include "base/memory/scoped_ptr.h" +#include "net/quic/crypto/aes_128_gcm_12_decrypter.h" +#include "net/quic/crypto/aes_128_gcm_12_encrypter.h" #include "net/quic/crypto/crypto_protocol.h" #include "net/quic/crypto/quic_decrypter.h" #include "net/quic/crypto/quic_encrypter.h" @@ -41,7 +43,7 @@ void CryptoSecretBoxer::SetKey(StringPiece key) { } string CryptoSecretBoxer::Box(QuicRandom* rand, StringPiece plaintext) const { - scoped_ptr encrypter(QuicEncrypter::Create(kAESG)); + scoped_ptr encrypter(new Aes128Gcm12Encrypter()); if (!encrypter->SetKey(key_)) { DLOG(DFATAL) << "CryptoSecretBoxer's encrypter->SetKey failed."; return string(); @@ -82,7 +84,7 @@ bool CryptoSecretBoxer::Unbox(StringPiece ciphertext, memcpy(&sequence_number, nonce.data() + nonce_prefix.size(), sizeof(sequence_number)); - scoped_ptr decrypter(QuicDecrypter::Create(kAESG)); + scoped_ptr decrypter(new Aes128Gcm12Decrypter()); if (!decrypter->SetKey(key_)) { DLOG(DFATAL) << "CryptoSecretBoxer's decrypter->SetKey failed."; return false; diff --git a/net/quic/crypto/null_encrypter.cc b/net/quic/crypto/null_encrypter.cc index 286694ad7b2488..c1e823e78ccb29 100644 --- a/net/quic/crypto/null_encrypter.cc +++ b/net/quic/crypto/null_encrypter.cc @@ -21,19 +21,6 @@ bool NullEncrypter::SetNoncePrefix(StringPiece nonce_prefix) { return nonce_prefix.empty(); } -bool NullEncrypter::Encrypt( - StringPiece /*nonce*/, - StringPiece associated_data, - StringPiece plaintext, - unsigned char* output) { - string buffer = associated_data.as_string(); - plaintext.AppendToString(&buffer); - uint128 hash = QuicUtils::FNV1a_128_Hash(buffer.data(), buffer.length()); - QuicUtils::SerializeUint128Short(hash, output); - memcpy(output + GetHashLength(), plaintext.data(), plaintext.size()); - return true; -} - bool NullEncrypter::EncryptPacket(QuicPacketSequenceNumber /*sequence_number*/, StringPiece associated_data, StringPiece plaintext, @@ -44,8 +31,12 @@ bool NullEncrypter::EncryptPacket(QuicPacketSequenceNumber /*sequence_number*/, if (max_output_length < len) { return false; } - Encrypt(StringPiece(), associated_data, plaintext, - reinterpret_cast(output)); + uint128 hash = QuicUtils::FNV1a_128_Hash_Two( + associated_data.data(), associated_data.size(), plaintext.data(), + plaintext.size()); + QuicUtils::SerializeUint128Short(hash, + reinterpret_cast(output)); + memcpy(output + GetHashLength(), plaintext.data(), plaintext.length()); *output_length = len; return true; } diff --git a/net/quic/crypto/null_encrypter.h b/net/quic/crypto/null_encrypter.h index 40bcacc703e7c6..66618fb9530a73 100644 --- a/net/quic/crypto/null_encrypter.h +++ b/net/quic/crypto/null_encrypter.h @@ -22,10 +22,6 @@ class NET_EXPORT_PRIVATE NullEncrypter : public QuicEncrypter { // QuicEncrypter implementation bool SetKey(base::StringPiece key) override; bool SetNoncePrefix(base::StringPiece nonce_prefix) override; - bool Encrypt(base::StringPiece nonce, - base::StringPiece associated_data, - base::StringPiece plaintext, - unsigned char* output) override; bool EncryptPacket(QuicPacketSequenceNumber sequence_number, base::StringPiece associated_data, base::StringPiece plaintext, diff --git a/net/quic/crypto/quic_crypto_server_config.h b/net/quic/crypto/quic_crypto_server_config.h index ab7442f0629dda..f7716e25f2b2dc 100644 --- a/net/quic/crypto/quic_crypto_server_config.h +++ b/net/quic/crypto/quic_crypto_server_config.h @@ -215,8 +215,8 @@ class NET_EXPORT_PRIVATE QuicCryptoServerConfig { // ProcessClientHello processes |client_hello| and decides whether to accept // or reject the connection. If the connection is to be accepted, |out| is // set to the contents of the ServerHello, |out_params| is completed and - // QUIC_NO_ERROR is returned. Otherwise |out| is set to be a REJ message and - // an error code is returned. + // QUIC_NO_ERROR is returned. Otherwise |out| is set to be a REJ or SREJ + // message and QUIC_NO_ERROR is returned. // // validate_chlo_result: Output from the asynchronous call to // ValidateClientHello. Contains the client hello message and diff --git a/net/quic/crypto/quic_encrypter.h b/net/quic/crypto/quic_encrypter.h index 2e565c543bc1cf..a44ef2070968d4 100644 --- a/net/quic/crypto/quic_encrypter.h +++ b/net/quic/crypto/quic_encrypter.h @@ -39,16 +39,6 @@ class NET_EXPORT_PRIVATE QuicEncrypter { // packet sequence number, even when retransmitting a lost packet. virtual bool SetNoncePrefix(base::StringPiece nonce_prefix) = 0; - // Encrypt encrypts |plaintext| and writes the ciphertext, plus a MAC over - // both |associated_data| and |plaintext| to |output|, using |nonce| as the - // nonce. |nonce| must be |8+GetNoncePrefixSize()| bytes long and |output| - // must point to a buffer that is at least - // |GetCiphertextSize(plaintext.size()| bytes long. - virtual bool Encrypt(base::StringPiece nonce, - base::StringPiece associated_data, - base::StringPiece plaintext, - unsigned char* output) = 0; - // Returns a newly created QuicData object containing the encrypted // |plaintext| as well as a MAC over both |plaintext| and |associated_data|, // or nullptr if there is an error. |sequence_number| is appended to the diff --git a/net/quic/iovector.h b/net/quic/iovector.h index 22d2cc9a8cd2ee..fcf50e87d57575 100644 --- a/net/quic/iovector.h +++ b/net/quic/iovector.h @@ -89,7 +89,7 @@ class NET_EXPORT_PRIVATE IOVector { // and write, it always takes char*. Clients that writes will need to cast // away the constant of the pointer before appending a block. void Append(char* buffer, size_t length) { - if (buffer != NULL && length > 0) { + if (buffer != nullptr && length > 0) { if (iovec_.size() > 0) { struct iovec& last = iovec_.back(); // If the new block is contiguous with the last block, just extend. @@ -106,7 +106,7 @@ class NET_EXPORT_PRIVATE IOVector { // Same as Append, but doesn't do the tail merge optimization. // Intended for testing. void AppendNoCoalesce(char* buffer, size_t length) { - if (buffer != NULL && length > 0) { + if (buffer != nullptr && length > 0) { struct iovec tmp = {buffer, length}; iovec_.push_back(tmp); } @@ -127,7 +127,7 @@ class NET_EXPORT_PRIVATE IOVector { bytes_to_consume -= iter->iov_len; } iovec_.erase(iovec_.begin(), iter); - if (iovec_.size() > 0 && bytes_to_consume != 0) { + if (!iovec_.empty() && bytes_to_consume != 0) { iovec_[0].iov_base = static_cast(iovec_[0].iov_base) + bytes_to_consume; iovec_[0].iov_len -= bytes_to_consume; @@ -142,6 +142,41 @@ class NET_EXPORT_PRIVATE IOVector { return length - bytes_to_consume; } + // Identical to Consume, but also copies the portion of the buffer being + // consumed into |buffer|. |buffer| must be at least size |length|. If + // the IOVector is less than |length|, the method consumes the entire + // IOVector, logs an error and returns the length consumed. + size_t ConsumeAndCopy(size_t length, char* buffer) { + if (length == 0) + return 0; + + size_t bytes_to_consume = length; + // First consume all the iovecs which can be consumed completely. + std::vector::iterator iter = iovec_.begin(); + std::vector::iterator end = iovec_.end(); + for (; iter < end && bytes_to_consume >= iter->iov_len; ++iter) { + memcpy(buffer, iter->iov_base, iter->iov_len); + bytes_to_consume -= iter->iov_len; + buffer += iter->iov_len; + } + iovec_.erase(iovec_.begin(), iter); + if (bytes_to_consume == 0) { + return length; + } + if (iovec_.empty()) { + LOG_IF(DFATAL, bytes_to_consume > 0) << "Attempting to consume " + << bytes_to_consume + << " non-existent bytes."; + return length - bytes_to_consume; + } + // Partially consume the next iovec. + memcpy(buffer, iovec_[0].iov_base, bytes_to_consume); + iovec_[0].iov_base = + static_cast(iovec_[0].iov_base) + bytes_to_consume; + iovec_[0].iov_len -= bytes_to_consume; + return length; + } + // TODO(joechan): If capacity is large, swap out for a blank one. // Clears the IOVector object to contain no blocks. void Clear() { iovec_.clear(); } diff --git a/net/quic/iovector_test.cc b/net/quic/iovector_test.cc index 37a1523e5ce1d9..f25ae1b5f51570 100644 --- a/net/quic/iovector_test.cc +++ b/net/quic/iovector_test.cc @@ -7,6 +7,8 @@ #include #include "base/logging.h" +#include "base/memory/scoped_ptr.h" +#include "net/test/gtest_util.h" #include "testing/gtest/include/gtest/gtest.h" using std::string; @@ -63,7 +65,7 @@ TEST(IOVectorTest, Append) { for (size_t i = 0; i < arraysize(test_data); ++i) { const int str_len = strlen(test_data[i]); const int append_len = str_len / 2; - // This should append a new block + // This should append a new block. iov.Append(const_cast(test_data[i]), append_len); length += append_len; ASSERT_EQ(i + 1, static_cast(iov.Size())); @@ -140,7 +142,7 @@ TEST(IOVectorTest, ConsumeHalfBlocks) { ASSERT_TRUE(iov2[0].iov_base == test_data[i] + tmp); ASSERT_EQ(iov2[0].iov_len, str_len - tmp); - // Consume the rest of the first block + // Consume the rest of the first block. consumed = iov.Consume(str_len - tmp); ASSERT_EQ(str_len - tmp, consumed); ASSERT_EQ(arraysize(test_data) - i - 1, static_cast(iov.Size())); @@ -197,13 +199,107 @@ TEST(IOVectorTest, ConsumeTooMuch) { } int consumed = 0; - consumed = iov.Consume(length); - // TODO(rtenneti): enable when chromium supports EXPECT_DFATAL. - /* EXPECT_DFATAL( {consumed = iov.Consume(length + 1);}, "Attempting to consume 1 non-existent bytes."); - */ + ASSERT_EQ(length, consumed); + const struct iovec* iov2 = iov.iovec(); + ASSERT_EQ(0u, iov.Size()); + ASSERT_TRUE(iov2 == nullptr); + ASSERT_TRUE(iov.LastBlockEnd() == nullptr); +} + +TEST(IOVectorTest, ConsumeAndCopyHalfBlocks) { + IOVector iov; + int length = 0; + + for (size_t i = 0; i < arraysize(test_data); ++i) { + const int str_len = strlen(test_data[i]); + iov.Append(const_cast(test_data[i]), str_len); + length += str_len; + } + const char* endp = iov.LastBlockEnd(); + for (size_t i = 0; i < arraysize(test_data); ++i) { + const struct iovec* iov2 = iov.iovec(); + const size_t str_len = strlen(test_data[i]); + size_t tmp = str_len / 2; + + ASSERT_TRUE(iov2 != nullptr); + ASSERT_TRUE(iov2[0].iov_base == test_data[i]); + ASSERT_EQ(str_len, iov2[0].iov_len); + + // Consume half of the first block. + scoped_ptr buffer(new char[str_len]); + size_t consumed = iov.ConsumeAndCopy(tmp, buffer.get()); + EXPECT_EQ(0, memcmp(test_data[i], buffer.get(), tmp)); + ASSERT_EQ(tmp, consumed); + ASSERT_EQ(arraysize(test_data) - i, static_cast(iov.Size())); + iov2 = iov.iovec(); + ASSERT_TRUE(iov2 != nullptr); + ASSERT_TRUE(iov2[0].iov_base == test_data[i] + tmp); + ASSERT_EQ(iov2[0].iov_len, str_len - tmp); + + // Consume the rest of the first block. + consumed = iov.ConsumeAndCopy(str_len - tmp, buffer.get()); + ASSERT_EQ(str_len - tmp, consumed); + ASSERT_EQ(arraysize(test_data) - i - 1, static_cast(iov.Size())); + iov2 = iov.iovec(); + if (iov.Size() > 0) { + ASSERT_TRUE(iov2 != nullptr); + ASSERT_TRUE(iov.LastBlockEnd() == endp); + } else { + ASSERT_TRUE(iov2 == nullptr); + ASSERT_TRUE(iov.LastBlockEnd() == nullptr); + } + } +} + +TEST(IOVectorTest, ConsumeAndCopyTwoAndHalfBlocks) { + IOVector iov; + size_t length = 0; + + for (size_t i = 0; i < arraysize(test_data); ++i) { + const int str_len = strlen(test_data[i]); + iov.Append(const_cast(test_data[i]), str_len); + length += str_len; + } + const size_t last_len = strlen(test_data[arraysize(test_data) - 1]); + const size_t half_len = last_len / 2; + + const char* endp = iov.LastBlockEnd(); + scoped_ptr buffer(new char[length]); + size_t consumed = iov.ConsumeAndCopy(length - half_len, buffer.get()); + ASSERT_EQ(length - half_len, consumed); + const struct iovec* iov2 = iov.iovec(); + ASSERT_TRUE(iov2 != nullptr); + ASSERT_EQ(1u, iov.Size()); + ASSERT_TRUE(iov2[0].iov_base == + test_data[arraysize(test_data) - 1] + last_len - half_len); + ASSERT_EQ(half_len, iov2[0].iov_len); + ASSERT_TRUE(iov.LastBlockEnd() == endp); + + consumed = iov.Consume(half_len); + ASSERT_EQ(half_len, consumed); + iov2 = iov.iovec(); + ASSERT_EQ(0u, iov.Size()); + ASSERT_TRUE(iov2 == nullptr); + ASSERT_TRUE(iov.LastBlockEnd() == nullptr); +} + +TEST(IOVectorTest, ConsumeAndCopyTooMuch) { + IOVector iov; + int length = 0; + + for (size_t i = 0; i < arraysize(test_data); ++i) { + const int str_len = strlen(test_data[i]); + iov.Append(const_cast(test_data[i]), str_len); + length += str_len; + } + + int consumed = 0; + scoped_ptr buffer(new char[length + 1]); + EXPECT_DFATAL({ consumed = iov.ConsumeAndCopy(length + 1, buffer.get()); }, + "Attempting to consume 1 non-existent bytes."); ASSERT_EQ(length, consumed); const struct iovec* iov2 = iov.iovec(); ASSERT_EQ(0u, iov.Size()); diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc index 12f929021fa517..5f17af50548f8f 100644 --- a/net/quic/quic_connection.cc +++ b/net/quic/quic_connection.cc @@ -912,9 +912,8 @@ void QuicConnection::OnPacketComplete() { } } - for (size_t i = 0; i < last_stream_frames_.size(); ++i) { - stats_.stream_bytes_received += - last_stream_frames_[i].data.TotalBufferSize(); + for (const QuicStreamFrame& stream_frame : last_stream_frames_) { + stats_.stream_bytes_received += stream_frame.data.size(); } // Process window updates, blocked, stream resets, acks, then congestion // feedback. @@ -1656,6 +1655,14 @@ void QuicConnection::OnSerializedPacket( SendOrQueuePacket(QueuedPacket(serialized_packet, encryption_level_)); } +void QuicConnection::OnResetFecGroup() { + if (!fec_alarm_->IsSet()) { + return; + } + // If an FEC Group is closed with the FEC alarm set, cancel the alarm. + fec_alarm_->Cancel(); +} + void QuicConnection::OnCongestionWindowChange() { packet_generator_.OnCongestionWindowChange( sent_packet_manager_.EstimateMaxPacketsInFlight(max_packet_length())); diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h index d47b8dc9a72ba7..b7bbdd7af191fa 100644 --- a/net/quic/quic_connection.h +++ b/net/quic/quic_connection.h @@ -388,6 +388,7 @@ class NET_EXPORT_PRIVATE QuicConnection void PopulateAckFrame(QuicAckFrame* ack) override; void PopulateStopWaitingFrame(QuicStopWaitingFrame* stop_waiting) override; void OnSerializedPacket(const SerializedPacket& packet) override; + void OnResetFecGroup() override; // QuicSentPacketManager::NetworkChangeVisitor void OnCongestionWindowChange() override; diff --git a/net/quic/quic_connection_logger.cc b/net/quic/quic_connection_logger.cc index 6e23e3da985525..38b257434113b9 100644 --- a/net/quic/quic_connection_logger.cc +++ b/net/quic/quic_connection_logger.cc @@ -100,7 +100,7 @@ scoped_ptr NetLogQuicStreamFrameCallback( dict->SetInteger("stream_id", frame->stream_id); dict->SetBoolean("fin", frame->fin); dict->SetString("offset", base::Uint64ToString(frame->offset)); - dict->SetInteger("length", frame->data.TotalBufferSize()); + dict->SetInteger("length", frame->data.size()); return dict.Pass(); } diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc index 92adfdb648ac2e..9276f4433387d8 100644 --- a/net/quic/quic_connection_test.cc +++ b/net/quic/quic_connection_test.cc @@ -78,16 +78,6 @@ class TaggingEncrypter : public QuicEncrypter { bool SetNoncePrefix(StringPiece nonce_prefix) override { return true; } - bool Encrypt(StringPiece nonce, - StringPiece associated_data, - StringPiece plaintext, - unsigned char* output) override { - memcpy(output, plaintext.data(), plaintext.size()); - output += plaintext.size(); - memset(output, tag_, kTagSize); - return true; - } - bool EncryptPacket(QuicPacketSequenceNumber sequence_number, StringPiece associated_data, StringPiece plaintext, @@ -98,8 +88,9 @@ class TaggingEncrypter : public QuicEncrypter { if (max_output_length < len) { return false; } - Encrypt(StringPiece(), associated_data, plaintext, - reinterpret_cast(output)); + memcpy(output, plaintext.data(), plaintext.size()); + output += plaintext.size(); + memset(output, tag_, kTagSize); *output_length = len; return true; } @@ -618,8 +609,8 @@ class QuicConnectionTest : public ::testing::TestWithParam { creator_(QuicConnectionPeer::GetPacketCreator(&connection_)), generator_(QuicConnectionPeer::GetPacketGenerator(&connection_)), manager_(QuicConnectionPeer::GetSentPacketManager(&connection_)), - frame1_(1, false, 0, MakeIOVector(data1)), - frame2_(1, false, 3, MakeIOVector(data2)), + frame1_(1, false, 0, StringPiece(data1)), + frame2_(1, false, 3, StringPiece(data2)), sequence_number_length_(PACKET_6BYTE_SEQUENCE_NUMBER), connection_id_length_(PACKET_8BYTE_CONNECTION_ID) { connection_.set_visitor(&visitor_); @@ -2091,9 +2082,7 @@ TEST_P(QuicConnectionTest, FramePackingSendv) { EXPECT_EQ(1u, writer_->stream_frames().size()); QuicStreamFrame frame = writer_->stream_frames()[0]; EXPECT_EQ(1u, frame.stream_id); - EXPECT_EQ("ABCD", string(static_cast - (frame.data.iovec()[0].iov_base), - (frame.data.iovec()[0].iov_len))); + EXPECT_EQ("ABCD", frame.data); } TEST_P(QuicConnectionTest, FramePackingSendvQueued) { diff --git a/net/quic/quic_crypto_client_stream_test.cc b/net/quic/quic_crypto_client_stream_test.cc index 9af6d04338dc19..4fc872aa629df3 100644 --- a/net/quic/quic_crypto_client_stream_test.cc +++ b/net/quic/quic_crypto_client_stream_test.cc @@ -271,6 +271,9 @@ TEST_F(QuicCryptoClientStreamStatelessTest, StatelessReject) { InitializeFakeStatelessRejectServer(); AdvanceHandshakeWithFakeServer(); + EXPECT_EQ(1, server_stream_->num_handshake_messages()); + EXPECT_EQ(0, server_stream_->num_handshake_messages_with_server_nonces()); + EXPECT_FALSE(client_stream_->encryption_established()); EXPECT_FALSE(client_stream_->handshake_confirmed()); // Even though the handshake was not complete, the cached client_state is diff --git a/net/quic/quic_crypto_server_stream.cc b/net/quic/quic_crypto_server_stream.cc index ec243c6f19f69b..5c7f5a80e03ec5 100644 --- a/net/quic/quic_crypto_server_stream.cc +++ b/net/quic/quic_crypto_server_stream.cc @@ -34,6 +34,7 @@ QuicCryptoServerStream::QuicCryptoServerStream( crypto_config_(crypto_config), validate_client_hello_cb_(nullptr), num_handshake_messages_(0), + num_handshake_messages_with_server_nonces_(0), num_server_config_update_messages_sent_(0), use_stateless_rejects_if_peer_supported_(false), peer_supports_stateless_rejects_(false) { @@ -227,6 +228,9 @@ QuicErrorCode QuicCryptoServerStream::ProcessClientHello( const ValidateClientHelloResultCallback::Result& result, CryptoHandshakeMessage* reply, string* error_details) { + if (!result.info.server_nonce.empty()) { + ++num_handshake_messages_with_server_nonces_; + } // Store the bandwidth estimate from the client. if (result.cached_network_params.bandwidth_estimate_bytes_per_second() > 0) { previous_cached_network_params_.reset( diff --git a/net/quic/quic_crypto_server_stream.h b/net/quic/quic_crypto_server_stream.h index 27f6f958482045..0a1a61a4f7a1f4 100644 --- a/net/quic/quic_crypto_server_stream.h +++ b/net/quic/quic_crypto_server_stream.h @@ -68,6 +68,10 @@ class NET_EXPORT_PRIVATE QuicCryptoServerStream : public QuicCryptoStream { uint8 num_handshake_messages() const { return num_handshake_messages_; } + uint8 num_handshake_messages_with_server_nonces() const { + return num_handshake_messages_with_server_nonces_; + } + int num_server_config_update_messages_sent() const { return num_server_config_update_messages_sent_; } @@ -167,6 +171,11 @@ class NET_EXPORT_PRIVATE QuicCryptoServerStream : public QuicCryptoStream { // Number of handshake messages received by this stream. uint8 num_handshake_messages_; + // Number of handshake messages received by this stream that contain + // server nonces (indicating that this is a non-zero-RTT handshake + // attempt). + uint8 num_handshake_messages_with_server_nonces_; + // Number of server config update (SCUP) messages sent by this stream. int num_server_config_update_messages_sent_; diff --git a/net/quic/quic_crypto_server_stream_test.cc b/net/quic/quic_crypto_server_stream_test.cc index bb499c2b0b5da2..2a51402d6601d4 100644 --- a/net/quic/quic_crypto_server_stream_test.cc +++ b/net/quic/quic_crypto_server_stream_test.cc @@ -241,6 +241,8 @@ TEST_P(QuicCryptoServerStreamTest, ConnectedAfterStatelessHandshake) { // On the first round, encryption will not be established. EXPECT_FALSE(server_stream_->encryption_established()); EXPECT_FALSE(server_stream_->handshake_confirmed()); + EXPECT_EQ(1, server_stream_->num_handshake_messages()); + EXPECT_EQ(0, server_stream_->num_handshake_messages_with_server_nonces()); // Now check the client state. QuicCryptoClientConfig::CachedState* client_state = @@ -273,6 +275,8 @@ TEST_P(QuicCryptoServerStreamTest, ConnectedAfterStatelessHandshake) { // On the second round, encryption will be established. EXPECT_TRUE(server_stream_->encryption_established()); EXPECT_TRUE(server_stream_->handshake_confirmed()); + EXPECT_EQ(2, server_stream_->num_handshake_messages()); + EXPECT_EQ(1, server_stream_->num_handshake_messages_with_server_nonces()); } TEST_P(QuicCryptoServerStreamTest, NoStatelessRejectIfNoClientSupport) { diff --git a/net/quic/quic_data_stream.cc b/net/quic/quic_data_stream.cc index a817b5b3571284..37013279fb8550 100644 --- a/net/quic/quic_data_stream.cc +++ b/net/quic/quic_data_stream.cc @@ -151,7 +151,7 @@ void QuicDataStream::OnStreamHeadersPriority(QuicPriority priority) { void QuicDataStream::OnStreamHeadersComplete(bool fin, size_t frame_len) { headers_decompressed_ = true; if (fin) { - sequencer()->OnStreamFrame(QuicStreamFrame(id(), fin, 0, IOVector())); + sequencer()->OnStreamFrame(QuicStreamFrame(id(), fin, 0, StringPiece())); } ProcessHeaderData(); if (FinishedReadingHeaders()) { diff --git a/net/quic/quic_data_stream_test.cc b/net/quic/quic_data_stream_test.cc index 55fab6be427415..bafde2f9cbd602 100644 --- a/net/quic/quic_data_stream_test.cc +++ b/net/quic/quic_data_stream_test.cc @@ -138,7 +138,7 @@ TEST_P(QuicDataStreamTest, ProcessHeadersAndBody) { stream_->OnStreamHeaders(headers); EXPECT_EQ(headers, stream_->data()); stream_->OnStreamHeadersComplete(false, headers.size()); - QuicStreamFrame frame(kClientDataStreamId1, false, 0, MakeIOVector(body)); + QuicStreamFrame frame(kClientDataStreamId1, false, 0, StringPiece(body)); stream_->OnStreamFrame(frame); EXPECT_EQ(headers + body, stream_->data()); @@ -165,7 +165,7 @@ TEST_P(QuicDataStreamTest, ProcessHeadersAndBodyFragments) { StringPiece fragment(body.data() + offset, min(fragment_size, remaining_data)); QuicStreamFrame frame(kClientDataStreamId1, false, offset, - MakeIOVector(fragment)); + StringPiece(fragment)); stream_->OnStreamFrame(frame); } ASSERT_EQ(headers + body, @@ -190,13 +190,13 @@ TEST_P(QuicDataStreamTest, ProcessHeadersAndBodyFragmentsSplit) { StringPiece fragment1(body.data(), split_point); QuicStreamFrame frame1(kClientDataStreamId1, false, 0, - MakeIOVector(fragment1)); + StringPiece(fragment1)); stream_->OnStreamFrame(frame1); StringPiece fragment2(body.data() + split_point, body.size() - split_point); QuicStreamFrame frame2(kClientDataStreamId1, false, split_point, - MakeIOVector(fragment2)); + StringPiece(fragment2)); stream_->OnStreamFrame(frame2); ASSERT_EQ(headers + body, @@ -214,7 +214,7 @@ TEST_P(QuicDataStreamTest, ProcessHeadersAndBodyReadv) { stream_->OnStreamHeaders(headers); EXPECT_EQ(headers, stream_->data()); stream_->OnStreamHeadersComplete(false, headers.size()); - QuicStreamFrame frame(kClientDataStreamId1, false, 0, MakeIOVector(body)); + QuicStreamFrame frame(kClientDataStreamId1, false, 0, StringPiece(body)); stream_->OnStreamFrame(frame); char buffer[2048]; @@ -241,7 +241,7 @@ TEST_P(QuicDataStreamTest, ProcessHeadersAndBodyIncrementalReadv) { stream_->OnStreamHeaders(headers); EXPECT_EQ(headers, stream_->data()); stream_->OnStreamHeadersComplete(false, headers.size()); - QuicStreamFrame frame(kClientDataStreamId1, false, 0, MakeIOVector(body)); + QuicStreamFrame frame(kClientDataStreamId1, false, 0, StringPiece(body)); stream_->OnStreamFrame(frame); char buffer[1]; @@ -266,7 +266,7 @@ TEST_P(QuicDataStreamTest, ProcessHeadersUsingReadvWithMultipleIovecs) { stream_->OnStreamHeaders(headers); EXPECT_EQ(headers, stream_->data()); stream_->OnStreamHeadersComplete(false, headers.size()); - QuicStreamFrame frame(kClientDataStreamId1, false, 0, MakeIOVector(body)); + QuicStreamFrame frame(kClientDataStreamId1, false, 0, StringPiece(body)); stream_->OnStreamFrame(frame); char buffer1[1]; @@ -348,7 +348,7 @@ TEST_P(QuicDataStreamTest, StreamFlowControlNoWindowUpdateIfNotConsumed) { EXPECT_EQ(headers, stream_->data()); stream_->OnStreamHeadersComplete(false, headers.size()); - QuicStreamFrame frame1(kClientDataStreamId1, false, 0, MakeIOVector(body)); + QuicStreamFrame frame1(kClientDataStreamId1, false, 0, StringPiece(body)); stream_->OnStreamFrame(frame1); EXPECT_EQ(kWindow - (kWindow / 3), QuicFlowControllerPeer::ReceiveWindowSize( stream_->flow_controller())); @@ -357,7 +357,7 @@ TEST_P(QuicDataStreamTest, StreamFlowControlNoWindowUpdateIfNotConsumed) { // half full. This should all be buffered, decreasing the receive window but // not sending WINDOW_UPDATE. QuicStreamFrame frame2(kClientDataStreamId1, false, kWindow / 3, - MakeIOVector(body)); + StringPiece(body)); stream_->OnStreamFrame(frame2); EXPECT_EQ( kWindow - (2 * kWindow / 3), @@ -388,7 +388,7 @@ TEST_P(QuicDataStreamTest, StreamFlowControlWindowUpdate) { EXPECT_EQ(headers, stream_->data()); stream_->OnStreamHeadersComplete(false, headers.size()); - QuicStreamFrame frame1(kClientDataStreamId1, false, 0, MakeIOVector(body)); + QuicStreamFrame frame1(kClientDataStreamId1, false, 0, StringPiece(body)); stream_->OnStreamFrame(frame1); EXPECT_EQ(kWindow - (kWindow / 3), QuicFlowControllerPeer::ReceiveWindowSize( stream_->flow_controller())); @@ -398,7 +398,7 @@ TEST_P(QuicDataStreamTest, StreamFlowControlWindowUpdate) { // offset and send a WINDOW_UPDATE. The result will be again an available // window of kWindow bytes. QuicStreamFrame frame2(kClientDataStreamId1, false, kWindow / 3, - MakeIOVector(body)); + StringPiece(body)); EXPECT_CALL(*connection_, SendWindowUpdate(kClientDataStreamId1, QuicFlowControllerPeer::ReceiveWindowOffset( @@ -442,9 +442,9 @@ TEST_P(QuicDataStreamTest, ConnectionFlowControlWindowUpdate) { // WINDOW_UPDATE for either stream, nor for the connection. string body; GenerateBody(&body, kWindow / 4); - QuicStreamFrame frame1(kClientDataStreamId1, false, 0, MakeIOVector(body)); + QuicStreamFrame frame1(kClientDataStreamId1, false, 0, StringPiece(body)); stream_->OnStreamFrame(frame1); - QuicStreamFrame frame2(kClientDataStreamId2, false, 0, MakeIOVector(body)); + QuicStreamFrame frame2(kClientDataStreamId2, false, 0, StringPiece(body)); stream2_->OnStreamFrame(frame2); // Now receive a further single byte on one stream - again this does not @@ -457,7 +457,7 @@ TEST_P(QuicDataStreamTest, ConnectionFlowControlWindowUpdate) { session_->flow_controller()) + 1 + kWindow / 2)); QuicStreamFrame frame3(kClientDataStreamId1, false, (kWindow / 4), - MakeIOVector("a")); + StringPiece("a")); stream_->OnStreamFrame(frame3); } @@ -483,7 +483,7 @@ TEST_P(QuicDataStreamTest, StreamFlowControlViolation) { // Receive data to overflow the window, violating flow control. string body; GenerateBody(&body, kWindow + 1); - QuicStreamFrame frame(kClientDataStreamId1, false, 0, MakeIOVector(body)); + QuicStreamFrame frame(kClientDataStreamId1, false, 0, StringPiece(body)); EXPECT_CALL(*connection_, SendConnectionClose(QUIC_FLOW_CONTROL_RECEIVED_TOO_MUCH_DATA)); stream_->OnStreamFrame(frame); @@ -516,7 +516,7 @@ TEST_P(QuicDataStreamTest, ConnectionFlowControlViolation) { string body; GenerateBody(&body, kConnectionWindow + 1); EXPECT_LT(body.size(), kStreamWindow); - QuicStreamFrame frame(kClientDataStreamId1, false, 0, MakeIOVector(body)); + QuicStreamFrame frame(kClientDataStreamId1, false, 0, StringPiece(body)); EXPECT_CALL(*connection_, SendConnectionClose(QUIC_FLOW_CONTROL_RECEIVED_TOO_MUCH_DATA)); diff --git a/net/quic/quic_framer.cc b/net/quic/quic_framer.cc index 61da635de6596b..26eb3140b1cc95 100644 --- a/net/quic/quic_framer.cc +++ b/net/quic/quic_framer.cc @@ -1281,23 +1281,17 @@ bool QuicFramer::ProcessStreamFrame(uint8 frame_type, return false; } - StringPiece frame_data; if (has_data_length) { - if (!reader_->ReadStringPiece16(&frame_data)) { + if (!reader_->ReadStringPiece16(&frame->data)) { set_detailed_error("Unable to read frame data."); return false; } } else { - if (!reader_->ReadStringPiece(&frame_data, reader_->BytesRemaining())) { + if (!reader_->ReadStringPiece(&frame->data, reader_->BytesRemaining())) { set_detailed_error("Unable to read frame data."); return false; } } - // Point frame to the right data. - frame->data.Clear(); - if (!frame_data.empty()) { - frame->data.Append(const_cast(frame_data.data()), frame_data.size()); - } return true; } @@ -1782,9 +1776,8 @@ size_t QuicFramer::ComputeFrameLength( case STREAM_FRAME: return GetMinStreamFrameSize(frame.stream_frame->stream_id, frame.stream_frame->offset, - last_frame_in_packet, - is_in_fec_group) + - frame.stream_frame->data.TotalBufferSize(); + last_frame_in_packet, is_in_fec_group) + + frame.stream_frame->data.length(); case ACK_FRAME: { return GetAckFrameSize(*frame.ack_frame, sequence_number_length); } @@ -1906,15 +1899,14 @@ bool QuicFramer::AppendStreamFrame( return false; } if (!no_stream_frame_length) { - if ((frame.data.TotalBufferSize() > numeric_limits::max()) || - !writer->WriteUInt16( - static_cast(frame.data.TotalBufferSize()))) { + if ((frame.data.size() > numeric_limits::max()) || + !writer->WriteUInt16(static_cast(frame.data.size()))) { LOG(DFATAL) << "Writing stream frame length failed"; return false; } } - if (!writer->WriteIOVector(frame.data)) { + if (!writer->WriteBytes(frame.data.data(), frame.data.size())) { LOG(DFATAL) << "Writing frame data failed."; return false; } diff --git a/net/quic/quic_framer_test.cc b/net/quic/quic_framer_test.cc index 4166801d2a4892..61249c7e74747a 100644 --- a/net/quic/quic_framer_test.cc +++ b/net/quic/quic_framer_test.cc @@ -107,13 +107,6 @@ class TestEncrypter : public QuicEncrypter { ~TestEncrypter() override {} bool SetKey(StringPiece key) override { return true; } bool SetNoncePrefix(StringPiece nonce_prefix) override { return true; } - bool Encrypt(StringPiece nonce, - StringPiece associated_data, - StringPiece plaintext, - unsigned char* output) override { - CHECK(false) << "Not implemented"; - return false; - } bool EncryptPacket(QuicPacketSequenceNumber sequence_number, StringPiece associated_data, StringPiece plaintext, @@ -237,12 +230,12 @@ class TestQuicVisitor : public QuicFramerVisitorInterface { bool OnStreamFrame(const QuicStreamFrame& frame) override { ++frame_count_; // Save a copy of the data so it is valid after the packet is processed. - stream_data_.push_back(frame.GetDataAsString()); + string* string_data = new string(); + frame.data.AppendToString(string_data); + stream_data_.push_back(string_data); QuicStreamFrame* stream_frame = new QuicStreamFrame(frame); // Make sure that the stream frame points to this data. - stream_frame->data.Clear(); - stream_frame->data.Append(const_cast(stream_data_.back()->data()), - stream_data_.back()->size()); + stream_frame->data = StringPiece(*string_data); stream_frames_.push_back(stream_frame); return true; } @@ -435,8 +428,7 @@ class QuicFramerTest : public ::testing::TestWithParam { // Checks if the supplied string matches data in the supplied StreamFrame. void CheckStreamFrameData(string str, QuicStreamFrame* frame) { - scoped_ptr frame_data(frame->GetDataAsString()); - EXPECT_EQ(str, *frame_data); + EXPECT_EQ(str, frame->data); } void CheckStreamFrameBoundaries(unsigned char* packet, @@ -3078,11 +3070,8 @@ TEST_P(QuicFramerTest, BuildStreamFramePacket) { header.packet_sequence_number = UINT64_C(0x77123456789ABC); header.fec_group = 0; - QuicStreamFrame stream_frame; - stream_frame.stream_id = 0x01020304; - stream_frame.fin = true; - stream_frame.offset = UINT64_C(0xBA98FEDC32107654); - stream_frame.data = MakeIOVector("hello world!"); + QuicStreamFrame stream_frame(0x01020304, true, UINT64_C(0xBA98FEDC32107654), + StringPiece("hello world!")); QuicFrames frames; frames.push_back(QuicFrame(&stream_frame)); @@ -3131,11 +3120,8 @@ TEST_P(QuicFramerTest, BuildStreamFramePacketInFecGroup) { header.is_in_fec_group = IN_FEC_GROUP; header.fec_group = UINT64_C(0x77123456789ABC); - QuicStreamFrame stream_frame; - stream_frame.stream_id = 0x01020304; - stream_frame.fin = true; - stream_frame.offset = UINT64_C(0xBA98FEDC32107654); - stream_frame.data = MakeIOVector("hello world!"); + QuicStreamFrame stream_frame(0x01020304, true, UINT64_C(0xBA98FEDC32107654), + StringPiece("hello world!")); QuicFrames frames; frames.push_back(QuicFrame(&stream_frame)); @@ -3180,11 +3166,8 @@ TEST_P(QuicFramerTest, BuildStreamFramePacketWithVersionFlag) { header.packet_sequence_number = UINT64_C(0x77123456789ABC); header.fec_group = 0; - QuicStreamFrame stream_frame; - stream_frame.stream_id = 0x01020304; - stream_frame.fin = true; - stream_frame.offset = UINT64_C(0xBA98FEDC32107654); - stream_frame.data = MakeIOVector("hello world!"); + QuicStreamFrame stream_frame(0x01020304, true, UINT64_C(0xBA98FEDC32107654), + StringPiece("hello world!")); QuicFrames frames; frames.push_back(QuicFrame(&stream_frame)); @@ -4023,7 +4006,6 @@ TEST_P(QuicFramerTest, BuildFecPacket) { header.packet_sequence_number = (UINT64_C(0x123456789ABC)); header.is_in_fec_group = IN_FEC_GROUP; header.fec_group = UINT64_C(0x123456789ABB); - ; QuicFecData fec_data; fec_data.fec_group = 1; @@ -4393,9 +4375,8 @@ TEST_P(QuicFramerTest, StopPacketProcessing) { static char kTestString[] = "At least 20 characters."; static QuicStreamId kTestQuicStreamId = 1; static bool ExpectedStreamFrame(const QuicStreamFrame& frame) { - scoped_ptr data(frame.GetDataAsString()); return frame.stream_id == kTestQuicStreamId && !frame.fin && - frame.offset == 0 && *data == kTestString; + frame.offset == 0 && frame.data == kTestString; // FIN is hard-coded false in ConstructEncryptedPacket. // Offset 0 is hard-coded in ConstructEncryptedPacket. } diff --git a/net/quic/quic_packet_creator.cc b/net/quic/quic_packet_creator.cc index 969ba5cd386634..79182c8a7782d3 100644 --- a/net/quic/quic_packet_creator.cc +++ b/net/quic/quic_packet_creator.cc @@ -248,13 +248,15 @@ size_t QuicPacketCreator::StreamFramePacketOverhead( } size_t QuicPacketCreator::CreateStreamFrame(QuicStreamId id, - const IOVector& data, + IOVector* data, QuicStreamOffset offset, bool fin, - QuicFrame* frame) { + QuicFrame* frame, + scoped_ptr* buffer) { DCHECK_GT(max_packet_length_, StreamFramePacketOverhead( connection_id_length_, kIncludeVersion, PACKET_6BYTE_SEQUENCE_NUMBER, offset, IN_FEC_GROUP)); + DCHECK(buffer); InFecGroup is_in_fec_group = MaybeUpdateLengthsAndStartFec(); @@ -263,25 +265,26 @@ size_t QuicPacketCreator::CreateStreamFrame(QuicStreamId id, << " MinStreamFrameSize: " << QuicFramer::GetMinStreamFrameSize(id, offset, true, is_in_fec_group); - if (data.Empty()) { + if (data->Empty()) { LOG_IF(DFATAL, !fin) << "Creating a stream frame with no data or fin."; // Create a new packet for the fin, if necessary. - *frame = QuicFrame(new QuicStreamFrame(id, true, offset, data)); + *frame = QuicFrame(new QuicStreamFrame(id, true, offset, StringPiece())); return 0; } - const size_t data_size = data.TotalBufferSize(); + const size_t data_size = data->TotalBufferSize(); size_t min_frame_size = QuicFramer::GetMinStreamFrameSize( id, offset, /* last_frame_in_packet= */ true, is_in_fec_group); size_t bytes_consumed = min(BytesFree() - min_frame_size, data_size); bool set_fin = fin && bytes_consumed == data_size; // Last frame. - IOVector frame_data; - frame_data.AppendIovecAtMostBytes(data.iovec(), data.Size(), - bytes_consumed); - DCHECK_EQ(frame_data.TotalBufferSize(), bytes_consumed); - *frame = QuicFrame(new QuicStreamFrame(id, set_fin, offset, frame_data)); + buffer->reset(new char[bytes_consumed]); + size_t bytes_copied = data->ConsumeAndCopy(bytes_consumed, buffer->get()); + LOG_IF(DFATAL, bytes_copied < bytes_consumed) + << "Fewer bytes copied than in IOVector."; + *frame = QuicFrame(new QuicStreamFrame( + id, set_fin, offset, StringPiece(buffer->get(), bytes_consumed))); return bytes_consumed; } @@ -322,7 +325,7 @@ SerializedPacket QuicPacketCreator::SerializeAllFrames(const QuicFrames& frames, LOG_IF(DFATAL, frames.empty()) << "Attempt to serialize empty packet"; for (const QuicFrame& frame : frames) { - bool success = AddFrame(frame, false); + bool success = AddFrame(frame, false, nullptr); DCHECK(success); } SerializedPacket packet = SerializePacket(buffer, buffer_len); @@ -372,7 +375,11 @@ size_t QuicPacketCreator::PacketSize() const { } bool QuicPacketCreator::AddSavedFrame(const QuicFrame& frame) { - return AddFrame(frame, true); + return AddFrame(frame, true, nullptr); +} + +bool QuicPacketCreator::AddSavedFrame(const QuicFrame& frame, char* buffer) { + return AddFrame(frame, true, buffer); } SerializedPacket QuicPacketCreator::SerializePacket( @@ -517,7 +524,8 @@ bool QuicPacketCreator::ShouldRetransmit(const QuicFrame& frame) { } bool QuicPacketCreator::AddFrame(const QuicFrame& frame, - bool save_retransmittable_frames) { + bool save_retransmittable_frames, + char* buffer) { DVLOG(1) << "Adding frame: " << frame; InFecGroup is_in_fec_group = MaybeUpdateLengthsAndStartFec(); @@ -535,13 +543,8 @@ bool QuicPacketCreator::AddFrame(const QuicFrame& frame, queued_retransmittable_frames_.reset( new RetransmittableFrames(encryption_level_)); } - if (frame.type == STREAM_FRAME) { - queued_frames_.push_back( - queued_retransmittable_frames_->AddStreamFrame(frame.stream_frame)); - } else { - queued_frames_.push_back( - queued_retransmittable_frames_->AddNonStreamFrame(frame)); - } + queued_frames_.push_back( + queued_retransmittable_frames_->AddFrame(frame, buffer)); } else { queued_frames_.push_back(frame); } @@ -575,7 +578,7 @@ void QuicPacketCreator::MaybeAddPadding() { } QuicPaddingFrame padding; - bool success = AddFrame(QuicFrame(&padding), false); + bool success = AddFrame(QuicFrame(&padding), false, nullptr); DCHECK(success); } diff --git a/net/quic/quic_packet_creator.h b/net/quic/quic_packet_creator.h index 201720fa67a539..a6257c8eea8548 100644 --- a/net/quic/quic_packet_creator.h +++ b/net/quic/quic_packet_creator.h @@ -81,12 +81,14 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { // Converts a raw payload to a frame which fits into the currently open // packet if there is one. Returns the number of bytes consumed from data. // If data is empty and fin is true, the expected behavior is to consume the - // fin but return 0. + // 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 IOVector& data, + IOVector* data, QuicStreamOffset offset, bool fin, - QuicFrame* frame); + QuicFrame* frame, + scoped_ptr* buffer); // Serializes all frames into a single packet. All frames must fit into a // single packet. Also, sets the entropy hash of the serialized packet to a @@ -149,6 +151,10 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { // Returns false if the frame doesn't fit into the current packet. bool AddSavedFrame(const QuicFrame& frame); + // Identical to AddSavedFrame, but takes ownership of the buffer if it returns + // true. + bool AddSavedFrame(const QuicFrame& frame, char* buffer); + // Serializes all frames which have been added and adds any which should be // retransmitted to |retransmittable_frames| if it's not nullptr. All frames // must fit into a single packet. Sets the entropy hash of the serialized @@ -241,7 +247,9 @@ class NET_EXPORT_PRIVATE QuicPacketCreator { // Allows a frame to be added without creating retransmittable frames. // Particularly useful for retransmits using SerializeAllFrames(). - bool AddFrame(const QuicFrame& frame, bool save_retransmittable_frames); + bool AddFrame(const QuicFrame& frame, + bool save_retransmittable_frames, + char* buffer); // Adds a padding frame to the current packet only if the current packet // contains a handshake message, and there is sufficient room to fit a diff --git a/net/quic/quic_packet_creator_test.cc b/net/quic/quic_packet_creator_test.cc index 113a5837ed8045..c463d9d5b1fb26 100644 --- a/net/quic/quic_packet_creator_test.cc +++ b/net/quic/quic_packet_creator_test.cc @@ -104,8 +104,7 @@ class QuicPacketCreatorTest : public ::testing::TestWithParam { EXPECT_EQ(STREAM_FRAME, frame.type); ASSERT_TRUE(frame.stream_frame); EXPECT_EQ(stream_id, frame.stream_frame->stream_id); - scoped_ptr frame_data(frame.stream_frame->GetDataAsString()); - EXPECT_EQ(data, *frame_data); + EXPECT_EQ(data, frame.stream_frame->data); EXPECT_EQ(offset, frame.stream_frame->offset); EXPECT_EQ(fin, frame.stream_frame->fin); } @@ -162,8 +161,10 @@ INSTANTIATE_TEST_CASE_P(QuicPacketCreatorTests, TEST_P(QuicPacketCreatorTest, SerializeFrames) { frames_.push_back(QuicFrame(new QuicAckFrame(MakeAckFrame(0u)))); - frames_.push_back(QuicFrame(new QuicStreamFrame(0u, false, 0u, IOVector()))); - frames_.push_back(QuicFrame(new QuicStreamFrame(0u, true, 0u, IOVector()))); + frames_.push_back( + QuicFrame(new QuicStreamFrame(0u, false, 0u, StringPiece()))); + frames_.push_back( + QuicFrame(new QuicStreamFrame(0u, true, 0u, StringPiece()))); char buffer[kMaxPacketSize]; SerializedPacket serialized = creator_.SerializeAllFrames(frames_, buffer, kMaxPacketSize); @@ -194,7 +195,8 @@ TEST_P(QuicPacketCreatorTest, SerializeWithFEC) { // trigger an FEC packet. ASSERT_FALSE(creator_.ShouldSendFec(/*force_close=*/false)); - frames_.push_back(QuicFrame(new QuicStreamFrame(0u, false, 0u, IOVector()))); + frames_.push_back( + QuicFrame(new QuicStreamFrame(0u, false, 0u, StringPiece()))); char buffer[kMaxPacketSize]; SerializedPacket serialized = creator_.SerializeAllFrames(frames_, buffer, kMaxPacketSize); @@ -455,9 +457,9 @@ TEST_P(QuicPacketCreatorTest, ReserializeFramesWithSequenceNumberLength) { QuicPacketCreatorPeer::SetSequenceNumberLength(&creator_, PACKET_2BYTE_SEQUENCE_NUMBER); QuicStreamFrame* stream_frame = - new QuicStreamFrame(kCryptoStreamId, /*fin=*/false, 0u, IOVector()); + new QuicStreamFrame(kCryptoStreamId, /*fin=*/false, 0u, StringPiece()); RetransmittableFrames frames(ENCRYPTION_NONE); - frames.AddStreamFrame(stream_frame); + frames.AddFrame(QuicFrame(stream_frame)); char buffer[kMaxPacketSize]; SerializedPacket serialized = creator_.ReserializeAllFrames( frames, PACKET_1BYTE_SEQUENCE_NUMBER, buffer, kMaxPacketSize); @@ -482,11 +484,13 @@ TEST_P(QuicPacketCreatorTest, ReserializeFramesWithSequenceNumberLength) { } TEST_P(QuicPacketCreatorTest, ReserializeFramesWithPadding) { - QuicStreamFrame* stream_frame = - new QuicStreamFrame(kCryptoStreamId, /*fin=*/false, /*offset=*/0, - MakeIOVector("fake handshake message data")); + QuicFrame frame; + IOVector io_vector(MakeIOVector("fake handshake message data")); + scoped_ptr stream_buffer; + creator_.CreateStreamFrame(kCryptoStreamId, &io_vector, 0u, false, &frame, + &stream_buffer); RetransmittableFrames frames(ENCRYPTION_NONE); - frames.AddStreamFrame(stream_frame); + frames.AddFrame(frame); char buffer[kMaxPacketSize]; SerializedPacket serialized = creator_.ReserializeAllFrames( frames, QuicPacketCreatorPeer::NextSequenceNumberLength(&creator_), @@ -503,10 +507,13 @@ TEST_P(QuicPacketCreatorTest, ReserializeFramesWithFullPacketAndPadding) { string data(capacity + delta, 'A'); size_t bytes_free = 0 - delta; - QuicStreamFrame* stream_frame = new QuicStreamFrame( - kCryptoStreamId, /*fin=*/false, kOffset, MakeIOVector(data)); + QuicFrame frame; + IOVector io_vector(MakeIOVector(data)); + scoped_ptr stream_buffer; + creator_.CreateStreamFrame(kCryptoStreamId, &io_vector, kOffset, false, + &frame, &stream_buffer); RetransmittableFrames frames(ENCRYPTION_NONE); - frames.AddStreamFrame(stream_frame); + frames.AddFrame(frame); char buffer[kMaxPacketSize]; SerializedPacket serialized = creator_.ReserializeAllFrames( frames, QuicPacketCreatorPeer::NextSequenceNumberLength(&creator_), @@ -573,7 +580,8 @@ TEST_P(QuicPacketCreatorTest, SwitchFecOnOffWithNoGroup) { TEST_P(QuicPacketCreatorTest, SwitchFecOnOffWithGroupInProgress) { // Enable FEC protection, and send FEC packet every 6 packets. EXPECT_TRUE(SwitchFecProtectionOn(6)); - frames_.push_back(QuicFrame(new QuicStreamFrame(0u, false, 0u, IOVector()))); + frames_.push_back( + QuicFrame(new QuicStreamFrame(0u, false, 0u, StringPiece()))); char buffer[kMaxPacketSize]; SerializedPacket serialized = creator_.SerializeAllFrames(frames_, buffer, kMaxPacketSize); @@ -607,8 +615,10 @@ TEST_P(QuicPacketCreatorTest, SwitchFecOnOffWithGroupInProgress) { TEST_P(QuicPacketCreatorTest, SwitchFecOnWithStreamFrameQueued) { // Add a stream frame to the creator. QuicFrame frame; - size_t consumed = creator_.CreateStreamFrame( - 1u, MakeIOVector("test"), 0u, false, &frame); + IOVector io_vector(MakeIOVector("test")); + scoped_ptr stream_buffer; + size_t consumed = creator_.CreateStreamFrame(1u, &io_vector, 0u, false, + &frame, &stream_buffer); EXPECT_EQ(4u, consumed); ASSERT_TRUE(frame.stream_frame); EXPECT_TRUE(creator_.AddSavedFrame(frame)); @@ -636,26 +646,34 @@ TEST_P(QuicPacketCreatorTest, SwitchFecOnWithStreamFrameQueued) { TEST_P(QuicPacketCreatorTest, CreateStreamFrame) { QuicFrame frame; - size_t consumed = creator_.CreateStreamFrame(1u, MakeIOVector("test"), 0u, - false, &frame); + IOVector io_vector(MakeIOVector("test")); + scoped_ptr stream_buffer; + size_t consumed = creator_.CreateStreamFrame(1u, &io_vector, 0u, false, + &frame, &stream_buffer); EXPECT_EQ(4u, consumed); CheckStreamFrame(frame, 1u, "test", 0u, false); - delete frame.stream_frame; + RetransmittableFrames cleanup_frames(ENCRYPTION_NONE); + cleanup_frames.AddFrame(frame); } TEST_P(QuicPacketCreatorTest, CreateStreamFrameFin) { QuicFrame frame; - size_t consumed = creator_.CreateStreamFrame(1u, MakeIOVector("test"), 10u, - true, &frame); + IOVector io_vector(MakeIOVector("test")); + scoped_ptr stream_buffer; + size_t consumed = creator_.CreateStreamFrame(1u, &io_vector, 10u, true, + &frame, &stream_buffer); EXPECT_EQ(4u, consumed); CheckStreamFrame(frame, 1u, "test", 10u, true); - delete frame.stream_frame; + RetransmittableFrames cleanup_frames(ENCRYPTION_NONE); + cleanup_frames.AddFrame(frame); } TEST_P(QuicPacketCreatorTest, CreateStreamFrameFinOnly) { QuicFrame frame; - size_t consumed = creator_.CreateStreamFrame(1u, IOVector(), 0u, true, - &frame); + IOVector io_vector; + scoped_ptr stream_buffer; + size_t consumed = creator_.CreateStreamFrame(1u, &io_vector, 0u, true, &frame, + &stream_buffer); EXPECT_EQ(0u, consumed); CheckStreamFrame(frame, 1u, string(), 0u, true); delete frame.stream_frame; @@ -672,9 +690,11 @@ TEST_P(QuicPacketCreatorTest, CreateAllFreeBytesForStreamFrames) { kClientDataStreamId1, kOffset)); if (should_have_room) { QuicFrame frame; - size_t bytes_consumed = creator_.CreateStreamFrame( - kClientDataStreamId1, MakeIOVector("testdata"), kOffset, false, - &frame); + IOVector io_vector(MakeIOVector("testdata")); + scoped_ptr stream_buffer; + size_t bytes_consumed = + creator_.CreateStreamFrame(kClientDataStreamId1, &io_vector, kOffset, + false, &frame, &stream_buffer); EXPECT_LT(0u, bytes_consumed); ASSERT_TRUE(creator_.AddSavedFrame(frame)); char buffer[kMaxPacketSize]; @@ -697,8 +717,11 @@ TEST_P(QuicPacketCreatorTest, StreamFrameConsumption) { string data(capacity + delta, 'A'); size_t bytes_free = delta > 0 ? 0 : 0 - delta; QuicFrame frame; - size_t bytes_consumed = creator_.CreateStreamFrame( - kClientDataStreamId1, MakeIOVector(data), kOffset, false, &frame); + IOVector io_vector(MakeIOVector(data)); + scoped_ptr stream_buffer; + size_t bytes_consumed = + creator_.CreateStreamFrame(kClientDataStreamId1, &io_vector, kOffset, + false, &frame, &stream_buffer); EXPECT_EQ(capacity - bytes_free, bytes_consumed); ASSERT_TRUE(creator_.AddSavedFrame(frame)); @@ -728,8 +751,11 @@ TEST_P(QuicPacketCreatorTest, StreamFrameConsumptionWithFec) { string data(capacity + delta, 'A'); size_t bytes_free = delta > 0 ? 0 : 0 - delta; QuicFrame frame; - size_t bytes_consumed = creator_.CreateStreamFrame( - kClientDataStreamId1, MakeIOVector(data), kOffset, false, &frame); + IOVector io_vector(MakeIOVector(data)); + scoped_ptr stream_buffer; + size_t bytes_consumed = + creator_.CreateStreamFrame(kClientDataStreamId1, &io_vector, kOffset, + false, &frame, &stream_buffer); EXPECT_EQ(capacity - bytes_free, bytes_consumed); ASSERT_TRUE(creator_.AddSavedFrame(frame)); @@ -760,8 +786,10 @@ TEST_P(QuicPacketCreatorTest, CryptoStreamFramePacketPadding) { size_t bytes_free = delta > 0 ? 0 : 0 - delta; QuicFrame frame; + IOVector io_vector(MakeIOVector(data)); + scoped_ptr stream_buffer; size_t bytes_consumed = creator_.CreateStreamFrame( - kCryptoStreamId, MakeIOVector(data), kOffset, false, &frame); + kCryptoStreamId, &io_vector, kOffset, false, &frame, &stream_buffer); EXPECT_LT(0u, bytes_consumed); ASSERT_TRUE(creator_.AddSavedFrame(frame)); char buffer[kMaxPacketSize]; @@ -794,8 +822,11 @@ TEST_P(QuicPacketCreatorTest, NonCryptoStreamFramePacketNonPadding) { size_t bytes_free = delta > 0 ? 0 : 0 - delta; QuicFrame frame; - size_t bytes_consumed = creator_.CreateStreamFrame( - kClientDataStreamId1, MakeIOVector(data), kOffset, false, &frame); + IOVector io_vector(MakeIOVector(data)); + scoped_ptr stream_buffer; + size_t bytes_consumed = + creator_.CreateStreamFrame(kClientDataStreamId1, &io_vector, kOffset, + false, &frame, &stream_buffer); EXPECT_LT(0u, bytes_consumed); ASSERT_TRUE(creator_.AddSavedFrame(frame)); char buffer[kMaxPacketSize]; @@ -889,7 +920,8 @@ TEST_P(QuicPacketCreatorTest, SerializeFrame) { if (!GetParam().version_serialization) { creator_.StopSendingVersion(); } - frames_.push_back(QuicFrame(new QuicStreamFrame(0u, false, 0u, IOVector()))); + frames_.push_back( + QuicFrame(new QuicStreamFrame(0u, false, 0u, StringPiece()))); char buffer[kMaxPacketSize]; SerializedPacket serialized = creator_.SerializeAllFrames(frames_, buffer, kMaxPacketSize); @@ -926,12 +958,15 @@ TEST_P(QuicPacketCreatorTest, CreateStreamFrameTooLarge) { PACKET_1BYTE_SEQUENCE_NUMBER, NOT_IN_FEC_GROUP, &payload_length)); QuicFrame frame; const string too_long_payload(payload_length * 2, 'a'); - size_t consumed = creator_.CreateStreamFrame( - 1u, MakeIOVector(too_long_payload), 0u, true, &frame); + IOVector io_vector(MakeIOVector(too_long_payload)); + scoped_ptr stream_buffer; + size_t consumed = creator_.CreateStreamFrame(1u, &io_vector, 0u, true, &frame, + &stream_buffer); EXPECT_EQ(payload_length, consumed); const string payload(payload_length, 'a'); CheckStreamFrame(frame, 1u, payload, 0u, false); - delete frame.stream_frame; + RetransmittableFrames cleanup_frames(ENCRYPTION_NONE); + cleanup_frames.AddFrame(frame); } TEST_P(QuicPacketCreatorTest, AddFrameAndSerialize) { @@ -954,8 +989,10 @@ TEST_P(QuicPacketCreatorTest, AddFrameAndSerialize) { EXPECT_TRUE(creator_.HasPendingFrames()); QuicFrame frame; - size_t consumed = creator_.CreateStreamFrame( - 1u, MakeIOVector("test"), 0u, false, &frame); + IOVector io_vector(MakeIOVector("test")); + scoped_ptr stream_buffer; + size_t consumed = creator_.CreateStreamFrame(1u, &io_vector, 0u, false, + &frame, &stream_buffer); EXPECT_EQ(4u, consumed); ASSERT_TRUE(frame.stream_frame); EXPECT_TRUE(creator_.AddSavedFrame(frame)); @@ -1014,8 +1051,10 @@ TEST_P(QuicPacketCreatorTest, SerializeTruncatedAckFrameWithLargePacketSize) { // Make sure that an additional stream frame can be added to the packet. QuicFrame stream_frame; - size_t consumed = creator_.CreateStreamFrame( - 2u, MakeIOVector("test"), 0u, false, &stream_frame); + IOVector io_vector(MakeIOVector("test")); + scoped_ptr stream_buffer; + size_t consumed = creator_.CreateStreamFrame(2u, &io_vector, 0u, false, + &stream_frame, &stream_buffer); EXPECT_EQ(4u, consumed); ASSERT_TRUE(stream_frame.stream_frame); EXPECT_TRUE(creator_.AddSavedFrame(stream_frame)); @@ -1077,7 +1116,8 @@ TEST_P(QuicPacketCreatorTest, SerializeTruncatedAckFrameWithSmallPacketSize) { TEST_P(QuicPacketCreatorTest, EntropyFlag) { - frames_.push_back(QuicFrame(new QuicStreamFrame(0u, false, 0u, IOVector()))); + frames_.push_back( + QuicFrame(new QuicStreamFrame(0u, false, 0u, StringPiece()))); char buffer[kMaxPacketSize]; for (int i = 0; i < 2; ++i) { @@ -1104,7 +1144,8 @@ TEST_P(QuicPacketCreatorTest, EntropyFlag) { TEST_P(QuicPacketCreatorTest, ResetFecGroup) { // Enable FEC protection, and send FEC packet every 6 packets. EXPECT_TRUE(SwitchFecProtectionOn(6)); - frames_.push_back(QuicFrame(new QuicStreamFrame(0u, false, 0u, IOVector()))); + frames_.push_back( + QuicFrame(new QuicStreamFrame(0u, false, 0u, StringPiece()))); char buffer[kMaxPacketSize]; SerializedPacket serialized = creator_.SerializeAllFrames(frames_, buffer, kMaxPacketSize); @@ -1156,8 +1197,10 @@ TEST_P(QuicPacketCreatorTest, ResetFecGroup) { TEST_P(QuicPacketCreatorTest, ResetFecGroupWithQueuedFrames) { // Add a stream frame to the creator. QuicFrame frame; - size_t consumed = - creator_.CreateStreamFrame(1u, MakeIOVector("test"), 0u, false, &frame); + IOVector io_vector(MakeIOVector("test")); + scoped_ptr stream_buffer; + size_t consumed = creator_.CreateStreamFrame(1u, &io_vector, 0u, false, + &frame, &stream_buffer); EXPECT_EQ(4u, consumed); ASSERT_TRUE(frame.stream_frame); EXPECT_TRUE(creator_.AddSavedFrame(frame)); diff --git a/net/quic/quic_packet_generator.cc b/net/quic/quic_packet_generator.cc index 77f57759857356..eefac37d229c5d 100644 --- a/net/quic/quic_packet_generator.cc +++ b/net/quic/quic_packet_generator.cc @@ -45,6 +45,8 @@ QuicPacketGenerator::QuicPacketGenerator(QuicConnectionId connection_id, batch_mode_(false), fec_timeout_(QuicTime::Delta::Zero()), should_fec_protect_(false), + // TODO(rtenneti): Add the ability to set a different policy. + fec_send_policy_(FEC_ANY_TRIGGER), should_send_ack_(false), should_send_stop_waiting_(false), ack_queued_(false), @@ -114,12 +116,12 @@ void QuicPacketGenerator::SetShouldSendAck(bool also_send_stop_waiting) { should_send_ack_ = true; should_send_stop_waiting_ = also_send_stop_waiting; - SendQueuedFrames(false); + SendQueuedFrames(/*flush=*/false, /*is_fec_timeout=*/false); } void QuicPacketGenerator::AddControlFrame(const QuicFrame& frame) { queued_control_frames_.push_back(frame); - SendQueuedFrames(false); + SendQueuedFrames(/*flush=*/false, /*is_fec_timeout=*/false); } QuicConsumedData QuicPacketGenerator::ConsumeData( @@ -134,7 +136,7 @@ QuicConsumedData QuicPacketGenerator::ConsumeData( // other retransmittable frames in a single packet. const bool flush = has_handshake && packet_creator_.HasPendingRetransmittableFrames(); - SendQueuedFrames(flush); + SendQueuedFrames(flush, /*is_fec_timeout=*/false); size_t total_bytes_consumed = 0; bool fin_consumed = false; @@ -165,8 +167,9 @@ QuicConsumedData QuicPacketGenerator::ConsumeData( while (delegate_->ShouldGeneratePacket( HAS_RETRANSMITTABLE_DATA, has_handshake ? IS_HANDSHAKE : NOT_HANDSHAKE)) { QuicFrame frame; + scoped_ptr buffer; size_t bytes_consumed = packet_creator_.CreateStreamFrame( - id, data, offset + total_bytes_consumed, fin, &frame); + id, &data, offset + total_bytes_consumed, fin, &frame, &buffer); ++frames_created; // We want to track which packet this stream frame ends up in. @@ -174,7 +177,7 @@ QuicConsumedData QuicPacketGenerator::ConsumeData( ack_notifiers_.push_back(notifier); } - if (!AddFrame(frame)) { + if (!AddFrame(frame, buffer.get())) { 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. @@ -182,14 +185,18 @@ QuicConsumedData QuicPacketGenerator::ConsumeData( delete notifier; return QuicConsumedData(0, false); } + // When AddFrame succeeds, it takes ownership of the buffer. + ignore_result(buffer.release()); total_bytes_consumed += bytes_consumed; fin_consumed = fin && total_bytes_consumed == data_size; - data.Consume(bytes_consumed); DCHECK(data.Empty() || packet_creator_.BytesFree() == 0u); // TODO(ianswett): Restore packet reordering. if (!InBatchMode() || !packet_creator_.HasRoomForStreamFrame(id, offset)) { + // TODO(rtenneti): remove MaybeSendFecPacketAndCloseGroup() from inside + // SerializeAndSendPacket() and make it an explicit call here (and + // elsewhere where we call SerializeAndSendPacket?). SerializeAndSendPacket(); } @@ -213,12 +220,12 @@ QuicConsumedData QuicPacketGenerator::ConsumeData( // Don't allow the handshake to be bundled with other retransmittable frames. if (has_handshake) { - SendQueuedFrames(true); + 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. - MaybeSendFecPacketAndCloseGroup(/*force=*/false); + MaybeSendFecPacketAndCloseGroup(/*force=*/false, /*is_fec_timeout=*/false); DCHECK(InBatchMode() || !packet_creator_.HasPendingFrames()); return QuicConsumedData(total_bytes_consumed, fin_consumed); @@ -236,7 +243,7 @@ bool QuicPacketGenerator::CanSendWithNextPendingFrameAddition() const { return delegate_->ShouldGeneratePacket(retransmittable, NOT_HANDSHAKE); } -void QuicPacketGenerator::SendQueuedFrames(bool flush) { +void QuicPacketGenerator::SendQueuedFrames(bool flush, bool is_fec_timeout) { // Only add pending frames if we are SURE we can then send the whole packet. while (HasPendingFrames() && (flush || CanSendWithNextPendingFrameAddition())) { @@ -248,7 +255,7 @@ void QuicPacketGenerator::SendQueuedFrames(bool flush) { if (packet_creator_.HasPendingFrames() && (flush || !InBatchMode())) { SerializeAndSendPacket(); } - MaybeSendFecPacketAndCloseGroup(flush); + MaybeSendFecPacketAndCloseGroup(flush, is_fec_timeout); } void QuicPacketGenerator::MaybeStartFecProtection() { @@ -267,23 +274,31 @@ void QuicPacketGenerator::MaybeStartFecProtection() { // converted to an FEC protected packet, do it. This will require the // generator to check if the resulting expansion still allows the incoming // frame to be added to the packet. - SendQueuedFrames(true); + SendQueuedFrames(/*flush=*/true, /*is_fec_timeout=*/false); } packet_creator_.StartFecProtectingPackets(); DCHECK(packet_creator_.IsFecProtected()); } -void QuicPacketGenerator::MaybeSendFecPacketAndCloseGroup(bool force) { +void QuicPacketGenerator::MaybeSendFecPacketAndCloseGroup(bool force, + bool is_fec_timeout) { if (!ShouldSendFecPacket(force)) { return; } - // TODO(jri): SerializeFec can return a NULL packet, and this should - // cause an early return, with a call to delegate_->OnPacketGenerationError. - char buffer[kMaxPacketSize]; - SerializedPacket serialized_fec = - packet_creator_.SerializeFec(buffer, kMaxPacketSize); - DCHECK(serialized_fec.packet); - delegate_->OnSerializedPacket(serialized_fec); + + // If we want to send FEC packet only when FEC alaram goes off and if it is + // not a FEC timeout then close the group and dont send FEC packet. + if (fec_send_policy_ == FEC_ALARM_TRIGGER && !is_fec_timeout) { + ResetFecGroup(); + } else { + // TODO(jri): SerializeFec can return a NULL packet, and this should + // cause an early return, with a call to delegate_->OnPacketGenerationError. + char buffer[kMaxPacketSize]; + SerializedPacket serialized_fec = + packet_creator_.SerializeFec(buffer, kMaxPacketSize); + DCHECK(serialized_fec.packet); + delegate_->OnSerializedPacket(serialized_fec); + } // Turn FEC protection off if creator's protection is on and the creator // does not have an open FEC group. // Note: We only wait until the frames queued in the creator are flushed; @@ -300,6 +315,12 @@ bool QuicPacketGenerator::ShouldSendFecPacket(bool force) { packet_creator_.ShouldSendFec(force); } +void QuicPacketGenerator::ResetFecGroup() { + DCHECK(packet_creator_.IsFecGroupOpen()); + packet_creator_.ResetFecGroup(); + delegate_->OnResetFecGroup(); +} + void QuicPacketGenerator::OnFecTimeout() { DCHECK(!InBatchMode()); if (!ShouldSendFecPacket(true)) { @@ -308,8 +329,8 @@ void QuicPacketGenerator::OnFecTimeout() { } // Flush out any pending frames in the generator and the creator, and then // send out FEC packet. - SendQueuedFrames(true); - MaybeSendFecPacketAndCloseGroup(/*force=*/true); + SendQueuedFrames(/*flush=*/true, /*is_fec_timeout=*/true); + MaybeSendFecPacketAndCloseGroup(/*force=*/true, /*is_fec_timeout=*/true); } QuicTime::Delta QuicPacketGenerator::GetFecTimeout( @@ -334,11 +355,11 @@ void QuicPacketGenerator::StartBatchOperations() { void QuicPacketGenerator::FinishBatchOperations() { batch_mode_ = false; - SendQueuedFrames(false); + SendQueuedFrames(/*flush=*/false, /*is_fec_timeout=*/false); } void QuicPacketGenerator::FlushAllQueuedFrames() { - SendQueuedFrames(true); + SendQueuedFrames(/*flush=*/true, /*is_fec_timeout=*/false); } bool QuicPacketGenerator::HasQueuedFrames() const { @@ -355,7 +376,7 @@ bool QuicPacketGenerator::AddNextPendingFrame() { delegate_->PopulateAckFrame(&pending_ack_frame_); ack_queued_ = true; // If we can't this add the frame now, then we still need to do so later. - should_send_ack_ = !AddFrame(QuicFrame(&pending_ack_frame_)); + should_send_ack_ = !AddFrame(QuicFrame(&pending_ack_frame_), nullptr); // Return success if we have cleared out this flag (i.e., added the frame). // If we still need to send, then the frame is full, and we have failed. return !should_send_ack_; @@ -366,7 +387,7 @@ bool QuicPacketGenerator::AddNextPendingFrame() { stop_waiting_queued_ = true; // If we can't this add the frame now, then we still need to do so later. should_send_stop_waiting_ = - !AddFrame(QuicFrame(&pending_stop_waiting_frame_)); + !AddFrame(QuicFrame(&pending_stop_waiting_frame_), nullptr); // Return success if we have cleared out this flag (i.e., added the frame). // If we still need to send, then the frame is full, and we have failed. return !should_send_stop_waiting_; @@ -374,7 +395,7 @@ bool QuicPacketGenerator::AddNextPendingFrame() { LOG_IF(DFATAL, queued_control_frames_.empty()) << "AddNextPendingFrame called with no queued control frames."; - if (!AddFrame(queued_control_frames_.back())) { + if (!AddFrame(queued_control_frames_.back(), nullptr)) { // Packet was full. return false; } @@ -382,8 +403,8 @@ bool QuicPacketGenerator::AddNextPendingFrame() { return true; } -bool QuicPacketGenerator::AddFrame(const QuicFrame& frame) { - bool success = packet_creator_.AddSavedFrame(frame); +bool QuicPacketGenerator::AddFrame(const QuicFrame& frame, char* buffer) { + bool success = packet_creator_.AddSavedFrame(frame, buffer); if (success && debug_delegate_) { debug_delegate_->OnFrameAddedToPacket(frame); } @@ -401,7 +422,7 @@ void QuicPacketGenerator::SerializeAndSendPacket() { ack_notifiers_.clear(); delegate_->OnSerializedPacket(serialized_packet); - MaybeSendFecPacketAndCloseGroup(/*force=*/false); + MaybeSendFecPacketAndCloseGroup(/*force=*/false, /*is_fec_timeout=*/false); // The packet has now been serialized, so the frames are no longer queued. ack_queued_ = false; diff --git a/net/quic/quic_packet_generator.h b/net/quic/quic_packet_generator.h index 5db862d07e3779..03141f9c622520 100644 --- a/net/quic/quic_packet_generator.h +++ b/net/quic/quic_packet_generator.h @@ -79,6 +79,8 @@ class NET_EXPORT_PRIVATE QuicPacketGenerator { // Takes ownership of |packet.packet| and |packet.retransmittable_frames|. virtual void OnSerializedPacket(const SerializedPacket& packet) = 0; virtual void CloseConnection(QuicErrorCode error, bool from_peer) = 0; + // Called when a FEC Group is reset (closed). + virtual void OnResetFecGroup() = 0; }; // Interface which gets callbacks from the QuicPacketGenerator at interesting @@ -211,13 +213,19 @@ class NET_EXPORT_PRIVATE QuicPacketGenerator { // creator being ready to send an FEC packet, otherwise FEC packet is sent // as long as one is under construction in the creator. Also tries to turn // off FEC protection in the creator if it's off in the generator. - void MaybeSendFecPacketAndCloseGroup(bool force); + // When |fec_send_policy_| is FEC_SEND_QUIESCENCE, then send FEC + // packet if |is_fec_timeout| is true otherwise close the FEC group. + void MaybeSendFecPacketAndCloseGroup(bool force, bool is_fec_timeout); // Returns true if an FEC packet should be generated based on |force| and // current state of the generator and the creator. bool ShouldSendFecPacket(bool force); - void SendQueuedFrames(bool flush); + // Resets (closes) the FEC group and calls the Delegate's OnResetFecGroup. + // Asserts that FEC group is open. + void ResetFecGroup(); + + void SendQueuedFrames(bool flush, bool is_fec_timeout); // Test to see if we have pending ack, or control frames. bool HasPendingFrames() const; @@ -226,8 +234,9 @@ class NET_EXPORT_PRIVATE QuicPacketGenerator { bool CanSendWithNextPendingFrameAddition() const; // Add exactly one pending frame, preferring ack frames over control frames. bool AddNextPendingFrame(); - - bool AddFrame(const QuicFrame& frame); + // Adds a frame and takes ownership of the underlying buffer if the addition + // was successful. + bool AddFrame(const QuicFrame& frame, char* buffer); void SerializeAndSendPacket(); @@ -248,6 +257,9 @@ class NET_EXPORT_PRIVATE QuicPacketGenerator { // if this variable is false. bool should_fec_protect_; + // FEC policy that specifies when to send FEC packet. + FecSendPolicy fec_send_policy_; + // Flags to indicate the need for just-in-time construction of a frame. bool should_send_ack_; bool should_send_stop_waiting_; diff --git a/net/quic/quic_packet_generator_test.cc b/net/quic/quic_packet_generator_test.cc index 21e186def677c1..6e82b9b380804e 100644 --- a/net/quic/quic_packet_generator_test.cc +++ b/net/quic/quic_packet_generator_test.cc @@ -35,6 +35,11 @@ namespace { const int64 kMinFecTimeoutMs = 5u; +static const FecSendPolicy kFecSendPolicyList[] = { + FEC_ANY_TRIGGER, + FEC_ALARM_TRIGGER, +}; + class MockDelegate : public QuicPacketGenerator::DelegateInterface { public: MockDelegate() {} @@ -47,6 +52,7 @@ class MockDelegate : public QuicPacketGenerator::DelegateInterface { MOCK_METHOD1(PopulateStopWaitingFrame, void(QuicStopWaitingFrame*)); MOCK_METHOD1(OnSerializedPacket, void(const SerializedPacket& packet)); MOCK_METHOD2(CloseConnection, void(QuicErrorCode, bool)); + MOCK_METHOD0(OnResetFecGroup, void()); void SetCanWriteAnything() { EXPECT_CALL(*this, ShouldGeneratePacket(_, _)).WillRepeatedly(Return(true)); @@ -99,14 +105,16 @@ struct PacketContents { } // namespace -class QuicPacketGeneratorTest : public ::testing::Test { +class QuicPacketGeneratorTest : public ::testing::TestWithParam { public: QuicPacketGeneratorTest() : framer_(QuicSupportedVersions(), QuicTime::Zero(), Perspective::IS_CLIENT), generator_(42, &framer_, &random_, &delegate_), - creator_(QuicPacketGeneratorPeer::GetPacketCreator(&generator_)) {} + creator_(QuicPacketGeneratorPeer::GetPacketCreator(&generator_)) { + QuicPacketGeneratorPeer::SetFecSendPolicy(&generator_, GetParam()); + } ~QuicPacketGeneratorTest() override { for (SerializedPacket& packet : packets_) { @@ -215,14 +223,19 @@ class MockDebugDelegate : public QuicPacketGenerator::DebugDelegate { void(const QuicFrame&)); }; -TEST_F(QuicPacketGeneratorTest, ShouldSendAck_NotWritable) { +// Run all end to end tests with all supported FEC send polocies. +INSTANTIATE_TEST_CASE_P(FecSendPolicy, + QuicPacketGeneratorTest, + ::testing::ValuesIn(kFecSendPolicyList)); + +TEST_P(QuicPacketGeneratorTest, ShouldSendAck_NotWritable) { delegate_.SetCanNotWrite(); generator_.SetShouldSendAck(false); EXPECT_TRUE(generator_.HasQueuedFrames()); } -TEST_F(QuicPacketGeneratorTest, ShouldSendAck_WritableAndShouldNotFlush) { +TEST_P(QuicPacketGeneratorTest, ShouldSendAck_WritableAndShouldNotFlush) { StrictMock debug_delegate; generator_.set_debug_delegate(&debug_delegate); @@ -236,7 +249,7 @@ TEST_F(QuicPacketGeneratorTest, ShouldSendAck_WritableAndShouldNotFlush) { EXPECT_TRUE(generator_.HasQueuedFrames()); } -TEST_F(QuicPacketGeneratorTest, ShouldSendAck_WritableAndShouldFlush) { +TEST_P(QuicPacketGeneratorTest, ShouldSendAck_WritableAndShouldFlush) { delegate_.SetCanWriteOnlyNonRetransmittable(); EXPECT_CALL(delegate_, PopulateAckFrame(_)); @@ -251,7 +264,7 @@ TEST_F(QuicPacketGeneratorTest, ShouldSendAck_WritableAndShouldFlush) { CheckPacketContains(contents, 0); } -TEST_F(QuicPacketGeneratorTest, ShouldSendAck_MultipleCalls) { +TEST_P(QuicPacketGeneratorTest, ShouldSendAck_MultipleCalls) { // Make sure that calling SetShouldSendAck multiple times does not result in a // crash. Previously this would result in multiple QuicFrames queued in the // packet generator, with all but the last with internal pointers to freed @@ -270,21 +283,21 @@ TEST_F(QuicPacketGeneratorTest, ShouldSendAck_MultipleCalls) { generator_.FinishBatchOperations(); } -TEST_F(QuicPacketGeneratorTest, AddControlFrame_NotWritable) { +TEST_P(QuicPacketGeneratorTest, AddControlFrame_NotWritable) { delegate_.SetCanNotWrite(); generator_.AddControlFrame(QuicFrame(CreateRstStreamFrame())); EXPECT_TRUE(generator_.HasQueuedFrames()); } -TEST_F(QuicPacketGeneratorTest, AddControlFrame_OnlyAckWritable) { +TEST_P(QuicPacketGeneratorTest, AddControlFrame_OnlyAckWritable) { delegate_.SetCanWriteOnlyNonRetransmittable(); generator_.AddControlFrame(QuicFrame(CreateRstStreamFrame())); EXPECT_TRUE(generator_.HasQueuedFrames()); } -TEST_F(QuicPacketGeneratorTest, AddControlFrame_WritableAndShouldNotFlush) { +TEST_P(QuicPacketGeneratorTest, AddControlFrame_WritableAndShouldNotFlush) { delegate_.SetCanWriteAnything(); generator_.StartBatchOperations(); @@ -292,7 +305,7 @@ TEST_F(QuicPacketGeneratorTest, AddControlFrame_WritableAndShouldNotFlush) { EXPECT_TRUE(generator_.HasQueuedFrames()); } -TEST_F(QuicPacketGeneratorTest, AddControlFrame_NotWritableBatchThenFlush) { +TEST_P(QuicPacketGeneratorTest, AddControlFrame_NotWritableBatchThenFlush) { delegate_.SetCanNotWrite(); generator_.StartBatchOperations(); @@ -311,7 +324,7 @@ TEST_F(QuicPacketGeneratorTest, AddControlFrame_NotWritableBatchThenFlush) { CheckPacketContains(contents, 0); } -TEST_F(QuicPacketGeneratorTest, AddControlFrame_WritableAndShouldFlush) { +TEST_P(QuicPacketGeneratorTest, AddControlFrame_WritableAndShouldFlush) { delegate_.SetCanWriteAnything(); EXPECT_CALL(delegate_, OnSerializedPacket(_)) @@ -325,7 +338,7 @@ TEST_F(QuicPacketGeneratorTest, AddControlFrame_WritableAndShouldFlush) { CheckPacketContains(contents, 0); } -TEST_F(QuicPacketGeneratorTest, ConsumeData_NotWritable) { +TEST_P(QuicPacketGeneratorTest, ConsumeData_NotWritable) { delegate_.SetCanNotWrite(); QuicConsumedData consumed = generator_.ConsumeData( @@ -335,7 +348,7 @@ TEST_F(QuicPacketGeneratorTest, ConsumeData_NotWritable) { EXPECT_FALSE(generator_.HasQueuedFrames()); } -TEST_F(QuicPacketGeneratorTest, ConsumeData_WritableAndShouldNotFlush) { +TEST_P(QuicPacketGeneratorTest, ConsumeData_WritableAndShouldNotFlush) { delegate_.SetCanWriteAnything(); generator_.StartBatchOperations(); @@ -346,7 +359,7 @@ TEST_F(QuicPacketGeneratorTest, ConsumeData_WritableAndShouldNotFlush) { EXPECT_TRUE(generator_.HasQueuedFrames()); } -TEST_F(QuicPacketGeneratorTest, ConsumeData_WritableAndShouldFlush) { +TEST_P(QuicPacketGeneratorTest, ConsumeData_WritableAndShouldFlush) { delegate_.SetCanWriteAnything(); EXPECT_CALL(delegate_, OnSerializedPacket(_)) @@ -362,13 +375,13 @@ TEST_F(QuicPacketGeneratorTest, ConsumeData_WritableAndShouldFlush) { CheckPacketContains(contents, 0); } -TEST_F(QuicPacketGeneratorTest, ConsumeData_EmptyData) { +TEST_P(QuicPacketGeneratorTest, ConsumeData_EmptyData) { EXPECT_DFATAL(generator_.ConsumeData(kHeadersStreamId, MakeIOVector(""), 0, false, MAY_FEC_PROTECT, nullptr), "Attempt to consume empty data without FIN."); } -TEST_F(QuicPacketGeneratorTest, +TEST_P(QuicPacketGeneratorTest, ConsumeDataMultipleTimes_WritableAndShouldNotFlush) { delegate_.SetCanWriteAnything(); generator_.StartBatchOperations(); @@ -382,7 +395,7 @@ TEST_F(QuicPacketGeneratorTest, EXPECT_TRUE(generator_.HasQueuedFrames()); } -TEST_F(QuicPacketGeneratorTest, ConsumeData_BatchOperations) { +TEST_P(QuicPacketGeneratorTest, ConsumeData_BatchOperations) { delegate_.SetCanWriteAnything(); generator_.StartBatchOperations(); @@ -405,7 +418,7 @@ TEST_F(QuicPacketGeneratorTest, ConsumeData_BatchOperations) { CheckPacketContains(contents, 0); } -TEST_F(QuicPacketGeneratorTest, ConsumeDataSendsFecOnMaxGroupSize) { +TEST_P(QuicPacketGeneratorTest, ConsumeDataFecOnMaxGroupSize) { delegate_.SetCanWriteAnything(); // Send FEC every two packets. @@ -417,8 +430,15 @@ TEST_F(QuicPacketGeneratorTest, ConsumeDataSendsFecOnMaxGroupSize) { .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); - EXPECT_CALL(delegate_, OnSerializedPacket(_)) - .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + } else { + // FEC packet is not sent when send policy is FEC_ALARM_TRIGGER, but FEC + // group is closed. + EXPECT_CALL(delegate_, OnResetFecGroup()).Times(1); + } EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); } @@ -434,30 +454,50 @@ TEST_F(QuicPacketGeneratorTest, ConsumeDataSendsFecOnMaxGroupSize) { CheckPacketHasSingleStreamFrame(0); CheckPacketHasSingleStreamFrame(1); - CheckPacketIsFec(2, 1); - CheckPacketHasSingleStreamFrame(3); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + // FEC packet is sent when send policy is FEC_ANY_TRIGGER. + CheckPacketIsFec(2, 1); + CheckPacketHasSingleStreamFrame(3); + } else { + CheckPacketHasSingleStreamFrame(2); + } EXPECT_TRUE(creator_->IsFecProtected()); - // The FEC packet under construction will be sent when one more packet is sent - // (since FEC group size is 2), or when OnFecTimeout is called. Send more data - // with MAY_FEC_PROTECT. This packet should also be protected, and FEC packet - // is sent since FEC group size is reached. + // If FEC send policy is FEC_ANY_TRIGGER, then the FEC packet under + // construction will be sent when one more packet is sent (since FEC group + // size is 2), or when OnFecTimeout is called. Send more data with + // MAY_FEC_PROTECT. This packet should also be protected, and FEC packet is + // sent since FEC group size is reached. + // + // If FEC send policy is FEC_ALARM_TRIGGER, FEC group is closed when the group + // size is reached. FEC packet is not sent. { InSequence dummy; EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); - EXPECT_CALL(delegate_, OnSerializedPacket(_)) - .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + } else { + EXPECT_CALL(delegate_, OnResetFecGroup()).Times(1); + } } consumed = generator_.ConsumeData(5, CreateData(1u), 0, true, MAY_FEC_PROTECT, nullptr); EXPECT_EQ(1u, consumed.bytes_consumed); - CheckPacketHasSingleStreamFrame(4); - CheckPacketIsFec(5, 4); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + CheckPacketHasSingleStreamFrame(4); + CheckPacketIsFec(5, 4); + } else { + CheckPacketHasSingleStreamFrame(3); + } EXPECT_FALSE(creator_->IsFecProtected()); } -TEST_F(QuicPacketGeneratorTest, ConsumeDataSendsFecOnTimeout) { +TEST_P(QuicPacketGeneratorTest, ConsumeDataSendsFecOnTimeout) { delegate_.SetCanWriteAnything(); creator_->set_max_packets_per_fec_group(1000); @@ -515,7 +555,7 @@ TEST_F(QuicPacketGeneratorTest, ConsumeDataSendsFecOnTimeout) { EXPECT_FALSE(creator_->IsFecProtected()); } -TEST_F(QuicPacketGeneratorTest, GetFecTimeoutFiniteOnlyOnFirstPacketInGroup) { +TEST_P(QuicPacketGeneratorTest, GetFecTimeoutFiniteOnlyOnFirstPacketInGroup) { delegate_.SetCanWriteAnything(); creator_->set_max_packets_per_fec_group(6); @@ -608,7 +648,7 @@ TEST_F(QuicPacketGeneratorTest, GetFecTimeoutFiniteOnlyOnFirstPacketInGroup) { generator_.GetFecTimeout(/*sequence_number=*/8u)); } -TEST_F(QuicPacketGeneratorTest, ConsumeData_FramesPreviouslyQueued) { +TEST_P(QuicPacketGeneratorTest, ConsumeData_FramesPreviouslyQueued) { // Set the packet size be enough for two stream frames with 0 stream offset, // but not enough for a stream frame of 0 offset and one with non-zero offset. size_t length = @@ -655,7 +695,7 @@ TEST_F(QuicPacketGeneratorTest, ConsumeData_FramesPreviouslyQueued) { CheckPacketContains(contents, 1); } -TEST_F(QuicPacketGeneratorTest, NoFecPacketSentWhenBatchEnds) { +TEST_P(QuicPacketGeneratorTest, NoFecPacketSentWhenBatchEnds) { delegate_.SetCanWriteAnything(); creator_->set_max_packets_per_fec_group(6); @@ -687,7 +727,7 @@ TEST_F(QuicPacketGeneratorTest, NoFecPacketSentWhenBatchEnds) { CheckPacketIsFec(1, /*fec_group=*/1u); } -TEST_F(QuicPacketGeneratorTest, FecTimeoutOnRttChange) { +TEST_P(QuicPacketGeneratorTest, FecTimeoutOnRttChange) { EXPECT_EQ(QuicTime::Delta::Zero(), QuicPacketGeneratorPeer::GetFecTimeout(&generator_)); generator_.OnRttChange(QuicTime::Delta::FromMilliseconds(300)); @@ -695,7 +735,7 @@ TEST_F(QuicPacketGeneratorTest, FecTimeoutOnRttChange) { QuicPacketGeneratorPeer::GetFecTimeout(&generator_)); } -TEST_F(QuicPacketGeneratorTest, FecGroupSizeOnCongestionWindowChange) { +TEST_P(QuicPacketGeneratorTest, FecGroupSizeOnCongestionWindowChange) { delegate_.SetCanWriteAnything(); creator_->set_max_packets_per_fec_group(50); EXPECT_EQ(50u, creator_->max_packets_per_fec_group()); @@ -714,7 +754,7 @@ TEST_F(QuicPacketGeneratorTest, FecGroupSizeOnCongestionWindowChange) { EXPECT_EQ(2u, creator_->max_packets_per_fec_group()); } -TEST_F(QuicPacketGeneratorTest, FecGroupSizeChangeWithOpenGroup) { +TEST_P(QuicPacketGeneratorTest, FecGroupSizeChangeWithOpenGroup) { delegate_.SetCanWriteAnything(); generator_.StartBatchOperations(); creator_->set_max_packets_per_fec_group(50); @@ -742,25 +782,36 @@ TEST_F(QuicPacketGeneratorTest, FecGroupSizeChangeWithOpenGroup) { generator_.OnCongestionWindowChange(2); EXPECT_EQ(2u, creator_->max_packets_per_fec_group()); - // Send enough data to trigger one unprotected data packet, causing the FEC - // packet to also be sent. + // If FEC send policy is FEC_ANY_TRIGGER, then send enough data to trigger one + // unprotected data packet, causing the FEC packet to also be sent. + // + // If FEC send policy is FEC_ALARM_TRIGGER, FEC group is closed and FEC packet + // is not sent. { InSequence dummy; EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); - EXPECT_CALL(delegate_, OnSerializedPacket(_)) - .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + } else { + EXPECT_CALL(delegate_, OnResetFecGroup()).Times(1); + } } consumed = generator_.ConsumeData(7, CreateData(kDefaultMaxPacketSize), 0, true, MAY_FEC_PROTECT, nullptr); EXPECT_EQ(kDefaultMaxPacketSize, consumed.bytes_consumed); - // Verify that one FEC packet was sent. - CheckPacketIsFec(4, /*fec_group=*/1u); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + // Verify that one FEC packet was sent. + CheckPacketIsFec(4, /*fec_group=*/1u); + } EXPECT_FALSE(creator_->IsFecGroupOpen()); EXPECT_FALSE(creator_->IsFecProtected()); } -TEST_F(QuicPacketGeneratorTest, SwitchFecOnOff) { +TEST_P(QuicPacketGeneratorTest, SwitchFecOnOff) { delegate_.SetCanWriteAnything(); creator_->set_max_packets_per_fec_group(2); EXPECT_FALSE(creator_->IsFecProtected()); @@ -784,8 +835,13 @@ TEST_F(QuicPacketGeneratorTest, SwitchFecOnOff) { .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); - EXPECT_CALL(delegate_, OnSerializedPacket(_)) - .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + } else { + EXPECT_CALL(delegate_, OnResetFecGroup()).Times(1); + } EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); } @@ -796,17 +852,31 @@ TEST_F(QuicPacketGeneratorTest, SwitchFecOnOff) { EXPECT_EQ(data_len, consumed.bytes_consumed); EXPECT_FALSE(generator_.HasQueuedFrames()); - // Verify that packets sent were 3 data and 1 FEC. + // If FEC send policy is FEC_ANY_TRIGGER, verify that packets sent were 3 data + // and 1 FEC. + // + // If FEC send policy is FEC_ALARM_TRIGGER, verify that packets sent were 3 + // data and FEC group is closed. CheckPacketHasSingleStreamFrame(1); CheckPacketHasSingleStreamFrame(2); - CheckPacketIsFec(3, /*fec_group=*/2u); - CheckPacketHasSingleStreamFrame(4); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + CheckPacketIsFec(3, /*fec_group=*/2u); + CheckPacketHasSingleStreamFrame(4); + } else { + CheckPacketHasSingleStreamFrame(3); + } // Calling OnFecTimeout should emit the pending FEC packet. EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); generator_.OnFecTimeout(); - CheckPacketIsFec(5, /*fec_group=*/5u); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + CheckPacketIsFec(5, /*fec_group=*/5u); + } else { + CheckPacketIsFec(4, /*fec_group=*/4u); + } // Send one unprotected data packet. EXPECT_CALL(delegate_, OnSerializedPacket(_)) @@ -817,10 +887,15 @@ TEST_F(QuicPacketGeneratorTest, SwitchFecOnOff) { EXPECT_FALSE(generator_.HasQueuedFrames()); EXPECT_FALSE(creator_->IsFecProtected()); // Verify that one unprotected data packet was sent. - CheckPacketContains(contents, 6); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + CheckPacketContains(contents, 6); + } else { + CheckPacketContains(contents, 5); + } } -TEST_F(QuicPacketGeneratorTest, SwitchFecOnWithPendingFrameInCreator) { +TEST_P(QuicPacketGeneratorTest, SwitchFecOnWithPendingFrameInCreator) { delegate_.SetCanWriteAnything(); // Enable FEC. creator_->set_max_packets_per_fec_group(2); @@ -848,7 +923,7 @@ TEST_F(QuicPacketGeneratorTest, SwitchFecOnWithPendingFrameInCreator) { EXPECT_TRUE(creator_->HasPendingFrames()); } -TEST_F(QuicPacketGeneratorTest, SwitchFecOnWithPendingFramesInGenerator) { +TEST_P(QuicPacketGeneratorTest, SwitchFecOnWithPendingFramesInGenerator) { // Enable FEC. creator_->set_max_packets_per_fec_group(2); @@ -882,7 +957,7 @@ TEST_F(QuicPacketGeneratorTest, SwitchFecOnWithPendingFramesInGenerator) { EXPECT_TRUE(creator_->IsFecProtected()); } -TEST_F(QuicPacketGeneratorTest, SwitchFecOnOffWithSubsequentFramesProtected) { +TEST_P(QuicPacketGeneratorTest, SwitchFecOnOffWithSubsequentFramesProtected) { delegate_.SetCanWriteAnything(); // Enable FEC. @@ -914,7 +989,7 @@ TEST_F(QuicPacketGeneratorTest, SwitchFecOnOffWithSubsequentFramesProtected) { EXPECT_TRUE(creator_->IsFecProtected()); } -TEST_F(QuicPacketGeneratorTest, SwitchFecOnOffWithSubsequentPacketsProtected) { +TEST_P(QuicPacketGeneratorTest, SwitchFecOnOffWithSubsequentPacketsProtected) { delegate_.SetCanWriteAnything(); // Enable FEC. @@ -942,21 +1017,30 @@ TEST_F(QuicPacketGeneratorTest, SwitchFecOnOffWithSubsequentPacketsProtected) { InSequence dummy; EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); - EXPECT_CALL(delegate_, OnSerializedPacket(_)) - .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + } else { + EXPECT_CALL(delegate_, OnResetFecGroup()).Times(1); + } } consumed = generator_.ConsumeData(5, CreateData(1u), 0, true, MAY_FEC_PROTECT, nullptr); EXPECT_EQ(1u, consumed.bytes_consumed); contents.num_stream_frames = 1u; CheckPacketContains(contents, 1); - CheckPacketIsFec(2, /*fec_group=*/1u); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + // FEC packet is sent when send policy is FEC_ANY_TRIGGER. + CheckPacketIsFec(2, /*fec_group=*/1u); + } // FEC protection should be off in creator. EXPECT_FALSE(creator_->IsFecProtected()); } -TEST_F(QuicPacketGeneratorTest, SwitchFecOnOffThenOnWithCreatorProtectionOn) { +TEST_P(QuicPacketGeneratorTest, SwitchFecOnOffThenOnWithCreatorProtectionOn) { delegate_.SetCanWriteAnything(); generator_.StartBatchOperations(); @@ -990,20 +1074,160 @@ TEST_F(QuicPacketGeneratorTest, SwitchFecOnOffThenOnWithCreatorProtectionOn) { InSequence dummy; EXPECT_CALL(delegate_, OnSerializedPacket(_)) .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); - EXPECT_CALL(delegate_, OnSerializedPacket(_)) - .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + } else { + EXPECT_CALL(delegate_, OnResetFecGroup()).Times(1); + } } consumed = generator_.ConsumeData(5, CreateData(data_len), 0, true, MUST_FEC_PROTECT, nullptr); EXPECT_EQ(data_len, consumed.bytes_consumed); CheckPacketContains(contents, 1); - CheckPacketIsFec(2, /*fec_group=*/1u); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + // FEC packet is sent when send policy is FEC_ANY_TRIGGER. + CheckPacketIsFec(2, /*fec_group=*/1u); + } // FEC protection should remain on in creator. EXPECT_TRUE(creator_->IsFecProtected()); } -TEST_F(QuicPacketGeneratorTest, NotWritableThenBatchOperations) { +TEST_P(QuicPacketGeneratorTest, ResetFecGroupNoTimeout) { + delegate_.SetCanWriteAnything(); + // Send FEC packet after 2 packets. + creator_->set_max_packets_per_fec_group(2); + EXPECT_FALSE(creator_->IsFecProtected()); + + // Send two packets so that when this data is consumed, two packets are sent + // out. In FEC_TRIGGER_ANY, this will cause an FEC packet to be sent out and + // with FEC_TRIGGER_ALARM, this will cause a Reset to be called. In both + // cases, the creator's fec protection will be turned off afterwards. + { + InSequence dummy; + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + } else { + // FEC packet is not sent when send policy is FEC_ALARM_TRIGGER, but FEC + // group is closed. + EXPECT_CALL(delegate_, OnResetFecGroup()).Times(1); + } + // Fin Packet. + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + } + size_t data_len = 2 * kDefaultMaxPacketSize; + QuicConsumedData consumed = generator_.ConsumeData( + 5, CreateData(data_len), 0, true, MUST_FEC_PROTECT, nullptr); + EXPECT_EQ(data_len, consumed.bytes_consumed); + EXPECT_TRUE(consumed.fin_consumed); + EXPECT_FALSE(generator_.HasQueuedFrames()); + CheckPacketHasSingleStreamFrame(0); + CheckPacketHasSingleStreamFrame(1); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + // FEC packet is sent after 2 packets and when send policy is + // FEC_ANY_TRIGGER. + CheckPacketIsFec(2, 1); + CheckPacketHasSingleStreamFrame(3); + } else { + CheckPacketHasSingleStreamFrame(2); + } + EXPECT_TRUE(creator_->IsFecProtected()); + + // Do the same send (with MUST_FEC_PROTECT) on a different stream id. + { + InSequence dummy; + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + // FEC packet is sent after 2 packets and when send policy is + // FEC_ANY_TRIGGER. When policy is FEC_ALARM_TRIGGER, FEC group is closed + // and FEC packet is not sent. + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + } else { + EXPECT_CALL(delegate_, OnResetFecGroup()).Times(1); + } + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + // FEC packet is sent after 2 packets and when send policy is + // FEC_ANY_TRIGGER. When policy is FEC_ALARM_TRIGGER, FEC group is closed + // and FEC packet is not sent. + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + } else { + EXPECT_CALL(delegate_, OnResetFecGroup()).Times(1); + } + } + consumed = generator_.ConsumeData(7, CreateData(data_len), 0, true, + MUST_FEC_PROTECT, nullptr); + EXPECT_EQ(data_len, consumed.bytes_consumed); + EXPECT_TRUE(consumed.fin_consumed); + EXPECT_FALSE(generator_.HasQueuedFrames()); + if (QuicPacketGeneratorPeer::GetFecSendPolicy(&generator_) == + FEC_ANY_TRIGGER) { + CheckPacketHasSingleStreamFrame(4); + // FEC packet is sent after 2 packets and when send policy is + // FEC_ANY_TRIGGER. + CheckPacketIsFec(5, 4); + CheckPacketHasSingleStreamFrame(6); + CheckPacketHasSingleStreamFrame(7); + // FEC packet is sent after 2 packets and when send policy is + // FEC_ANY_TRIGGER. + CheckPacketIsFec(8, 7); + } else { + CheckPacketHasSingleStreamFrame(3); + CheckPacketHasSingleStreamFrame(4); + CheckPacketHasSingleStreamFrame(5); + } + EXPECT_TRUE(creator_->IsFecProtected()); +} + +// 1. Create and send one packet with MUST_FEC_PROTECT. +// 2. Call FecTimeout, expect FEC packet is sent. +// 3. Do the same thing over again, with a different stream id. +TEST_P(QuicPacketGeneratorTest, FecPacketSentOnFecTimeout) { + delegate_.SetCanWriteAnything(); + creator_->set_max_packets_per_fec_group(1000); + EXPECT_FALSE(creator_->IsFecProtected()); + + for (int i = 1; i < 4; i = i + 2) { + // Send data with MUST_FEC_PROTECT flag. No FEC packet is emitted, but the + // creator FEC protects all data. + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + QuicConsumedData consumed = generator_.ConsumeData( + i + 2, CreateData(1u), 0, true, MUST_FEC_PROTECT, nullptr); + EXPECT_EQ(1u, consumed.bytes_consumed); + EXPECT_TRUE(consumed.fin_consumed); + CheckPacketHasSingleStreamFrame(0); + EXPECT_TRUE(creator_->IsFecProtected()); + + // Calling OnFecTimeout should cause the FEC packet to be emitted. + EXPECT_CALL(delegate_, OnSerializedPacket(_)) + .WillOnce(Invoke(this, &QuicPacketGeneratorTest::SavePacket)); + generator_.OnFecTimeout(); + CheckPacketIsFec(i, i); + EXPECT_FALSE(creator_->IsFecProtected()); + } +} + +TEST_P(QuicPacketGeneratorTest, NotWritableThenBatchOperations) { delegate_.SetCanNotWrite(); generator_.SetShouldSendAck(false); @@ -1036,7 +1260,7 @@ TEST_F(QuicPacketGeneratorTest, NotWritableThenBatchOperations) { CheckPacketContains(contents, 0); } -TEST_F(QuicPacketGeneratorTest, NotWritableThenBatchOperations2) { +TEST_P(QuicPacketGeneratorTest, NotWritableThenBatchOperations2) { delegate_.SetCanNotWrite(); generator_.SetShouldSendAck(false); @@ -1084,7 +1308,7 @@ TEST_F(QuicPacketGeneratorTest, NotWritableThenBatchOperations2) { CheckPacketContains(contents2, 1); } -TEST_F(QuicPacketGeneratorTest, TestConnectionIdLength) { +TEST_P(QuicPacketGeneratorTest, TestConnectionIdLength) { generator_.SetConnectionIdLength(0); EXPECT_EQ(PACKET_0BYTE_CONNECTION_ID, creator_->connection_id_length()); generator_.SetConnectionIdLength(1); diff --git a/net/quic/quic_protocol.cc b/net/quic/quic_protocol.cc index 7fa153d9c51513..0c325e0492aa7a 100644 --- a/net/quic/quic_protocol.cc +++ b/net/quic/quic_protocol.cc @@ -114,21 +114,10 @@ QuicStreamFrame::QuicStreamFrame(const QuicStreamFrame& frame) QuicStreamFrame::QuicStreamFrame(QuicStreamId stream_id, bool fin, QuicStreamOffset offset, - IOVector data) + StringPiece data) : stream_id(stream_id), fin(fin), offset(offset), data(data) { } -string* QuicStreamFrame::GetDataAsString() const { - string* data_string = new string(); - data_string->reserve(data.TotalBufferSize()); - for (size_t i = 0; i < data.Size(); ++i) { - data_string->append(static_cast(data.iovec()[i].iov_base), - data.iovec()[i].iov_len); - } - DCHECK_EQ(data_string->size(), data.TotalBufferSize()); - return data_string; -} - uint32 MakeQuicTag(char a, char b, char c, char d) { return static_cast(a) | static_cast(b) << 8 | @@ -457,8 +446,7 @@ ostream& operator<<(ostream& os, const QuicStreamFrame& stream_frame) { os << "stream_id { " << stream_frame.stream_id << " } " << "fin { " << stream_frame.fin << " } " << "offset { " << stream_frame.offset << " } " - << "data { " - << QuicUtils::StringToHexASCIIDump(*(stream_frame.GetDataAsString())) + << "data { " << QuicUtils::StringToHexASCIIDump(stream_frame.data) << " }\n"; return os; } @@ -599,27 +587,24 @@ RetransmittableFrames::~RetransmittableFrames() { DCHECK(false) << "Cannot delete type: " << it->type; } } - STLDeleteElements(&stream_data_); -} - -const QuicFrame& RetransmittableFrames::AddStreamFrame( - QuicStreamFrame* stream_frame) { - // Make an owned copy of the stream frame's data. - stream_data_.push_back(stream_frame->GetDataAsString()); - // Ensure the stream frame's IOVector points to the owned copy of the data. - stream_frame->data.Clear(); - stream_frame->data.Append(const_cast(stream_data_.back()->data()), - stream_data_.back()->size()); - frames_.push_back(QuicFrame(stream_frame)); - if (stream_frame->stream_id == kCryptoStreamId) { - has_crypto_handshake_ = IS_HANDSHAKE; + for (const char* buffer : stream_data_) { + delete[] buffer; } - return frames_.back(); } -const QuicFrame& RetransmittableFrames::AddNonStreamFrame( - const QuicFrame& frame) { - DCHECK_NE(frame.type, STREAM_FRAME); +const QuicFrame& RetransmittableFrames::AddFrame(const QuicFrame& frame) { + return AddFrame(frame, nullptr); +} + +const QuicFrame& RetransmittableFrames::AddFrame(const QuicFrame& frame, + char* buffer) { + if (frame.type == STREAM_FRAME && + frame.stream_frame->stream_id == kCryptoStreamId) { + has_crypto_handshake_ = IS_HANDSHAKE; + } + if (buffer != nullptr) { + stream_data_.push_back(buffer); + } frames_.push_back(frame); return frames_.back(); } diff --git a/net/quic/quic_protocol.h b/net/quic/quic_protocol.h index 2ce2a2a9c76df9..3f69ce3082485b 100644 --- a/net/quic/quic_protocol.h +++ b/net/quic/quic_protocol.h @@ -212,6 +212,14 @@ enum FecPolicy { FEC_PROTECT_OPTIONAL // Data in the stream does not need FEC protection. }; +// Indicates FEC policy about when to send FEC packet. +enum FecSendPolicy { + // Send FEC packet when FEC group is full or when FEC alarm goes off. + FEC_ANY_TRIGGER, + // Send FEC packet only when FEC alarm goes off. + FEC_ALARM_TRIGGER +}; + enum QuicFrameType { // Regular frame types. The values set here cannot change without the // introduction of a new QUIC version. @@ -666,19 +674,15 @@ struct NET_EXPORT_PRIVATE QuicStreamFrame { QuicStreamFrame(QuicStreamId stream_id, bool fin, QuicStreamOffset offset, - IOVector data); + base::StringPiece data); NET_EXPORT_PRIVATE friend std::ostream& operator<<( std::ostream& os, const QuicStreamFrame& s); - // Returns a copy of the IOVector |data| as a heap-allocated string. - // Caller must take ownership of the returned string. - std::string* GetDataAsString() const; - QuicStreamId stream_id; bool fin; QuicStreamOffset offset; // Location of this data in the stream. - IOVector data; + base::StringPiece data; }; // TODO(ianswett): Re-evaluate the trade-offs of hash_set vs set when framing @@ -984,12 +988,10 @@ class NET_EXPORT_PRIVATE RetransmittableFrames { explicit RetransmittableFrames(EncryptionLevel level); ~RetransmittableFrames(); - // Allocates a local copy of the referenced StringPiece has QuicStreamFrame - // use it. - // Takes ownership of |stream_frame|. - const QuicFrame& AddStreamFrame(QuicStreamFrame* stream_frame); // Takes ownership of the frame inside |frame|. - const QuicFrame& AddNonStreamFrame(const QuicFrame& frame); + const QuicFrame& AddFrame(const QuicFrame& frame); + // Takes ownership of the frame inside |frame| and |buffer|. + const QuicFrame& AddFrame(const QuicFrame& frame, char* buffer); // Removes all stream frames associated with |stream_id|. void RemoveFramesForStream(QuicStreamId stream_id); @@ -1007,8 +1009,8 @@ class NET_EXPORT_PRIVATE RetransmittableFrames { QuicFrames frames_; const EncryptionLevel encryption_level_; IsHandshake has_crypto_handshake_; - // Data referenced by the StringPiece of a QuicStreamFrame. - std::vector stream_data_; + // Data referenced by the IOVector of a QuicStreamFrame. + std::vector stream_data_; DISALLOW_COPY_AND_ASSIGN(RetransmittableFrames); }; diff --git a/net/quic/quic_reliable_client_stream_test.cc b/net/quic/quic_reliable_client_stream_test.cc index b36e61046fb5ad..7ffead9e969257 100644 --- a/net/quic/quic_reliable_client_stream_test.cc +++ b/net/quic/quic_reliable_client_stream_test.cc @@ -99,8 +99,7 @@ TEST_P(QuicReliableClientStreamTest, OnFinRead) { stream_->OnStreamHeaders(uncompressed_headers); stream_->OnStreamHeadersComplete(false, uncompressed_headers.length()); - IOVector iov; - QuicStreamFrame frame2(kStreamId, true, offset, iov); + QuicStreamFrame frame2(kStreamId, true, offset, StringPiece()); EXPECT_CALL(delegate_, OnClose(QUIC_NO_ERROR)); stream_->OnStreamFrame(frame2); } diff --git a/net/quic/quic_sent_packet_manager_test.cc b/net/quic/quic_sent_packet_manager_test.cc index 2b55382fdd9733..1c78779c54739a 100644 --- a/net/quic/quic_sent_packet_manager_test.cc +++ b/net/quic/quic_sent_packet_manager_test.cc @@ -183,8 +183,8 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam { RetransmittableFrames* frames = nullptr; if (retransmittable) { frames = new RetransmittableFrames(ENCRYPTION_NONE); - frames->AddStreamFrame( - new QuicStreamFrame(kStreamId, false, 0, IOVector())); + frames->AddFrame( + QuicFrame(new QuicStreamFrame(kStreamId, false, 0, StringPiece()))); } return SerializedPacket(sequence_number, PACKET_6BYTE_SEQUENCE_NUMBER, packets_.back(), 0u, frames); @@ -214,8 +214,8 @@ class QuicSentPacketManagerTest : public ::testing::TestWithParam { kDefaultLength, HAS_RETRANSMITTABLE_DATA)) .Times(1).WillOnce(Return(true)); SerializedPacket packet(CreateDataPacket(sequence_number)); - packet.retransmittable_frames->AddStreamFrame( - new QuicStreamFrame(1, false, 0, IOVector())); + packet.retransmittable_frames->AddFrame( + QuicFrame(new QuicStreamFrame(1, false, 0, StringPiece()))); manager_.OnPacketSent(&packet, 0, clock_.Now(), packet.packet->length(), NOT_RETRANSMISSION, HAS_RETRANSMITTABLE_DATA); diff --git a/net/quic/quic_session.cc b/net/quic/quic_session.cc index 61e9b734dd3b3a..1f78b323fba084 100644 --- a/net/quic/quic_session.cc +++ b/net/quic/quic_session.cc @@ -143,8 +143,7 @@ void QuicSession::OnStreamFrames(const vector& frames) { // final stream byte offset sent by the peer. A frame with a FIN can give // us this offset. if (frame.fin) { - QuicStreamOffset final_byte_offset = - frame.offset + frame.data.TotalBufferSize(); + QuicStreamOffset final_byte_offset = frame.offset + frame.data.size(); UpdateFlowControlOnFinalReceivedByteOffset(stream_id, final_byte_offset); } diff --git a/net/quic/quic_session_test.cc b/net/quic/quic_session_test.cc index 206c93c1f9620f..b4df636d49c9c0 100644 --- a/net/quic/quic_session_test.cc +++ b/net/quic/quic_session_test.cc @@ -571,7 +571,7 @@ TEST_P(QuicSessionTestServer, IncreasedTimeoutAfterCryptoHandshake) { TEST_P(QuicSessionTestServer, RstStreamBeforeHeadersDecompressed) { // Send two bytes of payload. - QuicStreamFrame data1(kClientDataStreamId1, false, 0, MakeIOVector("HT")); + QuicStreamFrame data1(kClientDataStreamId1, false, 0, StringPiece("HT")); vector frames; frames.push_back(data1); session_.OnStreamFrames(frames); @@ -591,7 +591,7 @@ TEST_P(QuicSessionTestServer, MultipleRstStreamsCauseSingleConnectionClose) { // multiple connection close frames. // Create valid stream. - QuicStreamFrame data1(kClientDataStreamId1, false, 0, MakeIOVector("HT")); + QuicStreamFrame data1(kClientDataStreamId1, false, 0, StringPiece("HT")); vector frames; frames.push_back(data1); session_.OnStreamFrames(frames); @@ -778,7 +778,7 @@ TEST_P(QuicSessionTestServer, ConnectionFlowControlAccountingFinAndLocalReset) { const QuicStreamOffset kByteOffset = kInitialSessionFlowControlWindowForTest / 2; - QuicStreamFrame frame(stream->id(), true, kByteOffset, IOVector()); + QuicStreamFrame frame(stream->id(), true, kByteOffset, StringPiece()); vector frames; frames.push_back(frame); session_.OnStreamFrames(frames); @@ -820,8 +820,7 @@ TEST_P(QuicSessionTestServer, ConnectionFlowControlAccountingFinAfterRst) { // account the total number of bytes sent by the peer. const QuicStreamOffset kByteOffset = 5678; string body = "hello"; - IOVector data = MakeIOVector(body); - QuicStreamFrame frame(stream->id(), true, kByteOffset, data); + QuicStreamFrame frame(stream->id(), true, kByteOffset, StringPiece(body)); vector frames; frames.push_back(frame); session_.OnStreamFrames(frames); @@ -905,7 +904,7 @@ TEST_P(QuicSessionTestServer, FlowControlWithInvalidFinalOffset) { TestStream* stream = session_.CreateOutgoingDataStream(); EXPECT_CALL(*connection_, SendRstStream(stream->id(), _, _)); stream->Reset(QUIC_STREAM_CANCELLED); - QuicStreamFrame frame(stream->id(), true, kLargeOffset, IOVector()); + QuicStreamFrame frame(stream->id(), true, kLargeOffset, StringPiece()); vector frames; frames.push_back(frame); session_.OnStreamFrames(frames); @@ -954,7 +953,7 @@ TEST_P(QuicSessionTestServer, TooManyUnfinishedStreamsCauseConnectionClose) { const int kFirstStreamId = kClientDataStreamId1; const int kFinalStreamId = kClientDataStreamId1 + 2 * kMaxStreams + 1; for (int i = kFirstStreamId; i < kFinalStreamId; i += 2) { - QuicStreamFrame data1(i, false, 0, MakeIOVector("HT")); + QuicStreamFrame data1(i, false, 0, StringPiece("HT")); vector frames; frames.push_back(data1); session_.OnStreamFrames(frames); diff --git a/net/quic/quic_stream_sequencer.cc b/net/quic/quic_stream_sequencer.cc index 6ca6352f4d54de..4068405e2a1fa6 100644 --- a/net/quic/quic_stream_sequencer.cc +++ b/net/quic/quic_stream_sequencer.cc @@ -45,8 +45,8 @@ void QuicStreamSequencer::OnStreamFrame(const QuicStreamFrame& frame) { return; } - QuicStreamOffset byte_offset = frame.offset; - size_t data_len = frame.data.TotalBufferSize(); + const QuicStreamOffset byte_offset = frame.offset; + const size_t data_len = frame.data.length(); if (data_len == 0 && !frame.fin) { // Stream frames must have data or a fin flag. stream_->CloseConnectionWithDetails(QUIC_INVALID_STREAM_FRAME, @@ -61,23 +61,17 @@ void QuicStreamSequencer::OnStreamFrame(const QuicStreamFrame& frame) { } } - IOVector data; - data.AppendIovec(frame.data.iovec(), frame.data.Size()); - if (byte_offset > num_bytes_consumed_) { ++num_early_frames_received_; } // If the frame has arrived in-order then we can process it immediately, only // buffering if the stream is unable to process it. + size_t bytes_consumed = 0; if (!blocked_ && byte_offset == num_bytes_consumed_) { DVLOG(1) << "Processing byte offset " << byte_offset; - size_t bytes_consumed = 0; - for (size_t i = 0; i < data.Size(); ++i) { - bytes_consumed += stream_->ProcessRawData( - static_cast(data.iovec()[i].iov_base), - data.iovec()[i].iov_len); - } + bytes_consumed = + stream_->ProcessRawData(frame.data.data(), frame.data.length()); num_bytes_consumed_ += bytes_consumed; stream_->AddBytesConsumed(bytes_consumed); @@ -90,24 +84,18 @@ void QuicStreamSequencer::OnStreamFrame(const QuicStreamFrame& frame) { } else if (bytes_consumed == data_len) { FlushBufferedFrames(); return; // it's safe to ack this frame. - } else { - // Set ourselves up to buffer what's left. - data_len -= bytes_consumed; - data.Consume(bytes_consumed); - byte_offset += bytes_consumed; } } // Buffer any remaining data to be consumed by the stream when ready. - for (size_t i = 0; i < data.Size(); ++i) { + if (bytes_consumed < data_len) { DVLOG(1) << "Buffering stream data at offset " << byte_offset; - const iovec& iov = data.iovec()[i]; + const size_t remaining_length = data_len - bytes_consumed; buffered_frames_.insert(std::make_pair( - byte_offset, string(static_cast(iov.iov_base), iov.iov_len))); - byte_offset += iov.iov_len; - num_bytes_buffered_ += iov.iov_len; + byte_offset + bytes_consumed, + string(frame.data.data() + bytes_consumed, remaining_length))); + num_bytes_buffered_ += remaining_length; } - return; } void QuicStreamSequencer::CloseStreamAtOffset(QuicStreamOffset offset) { @@ -227,9 +215,9 @@ bool QuicStreamSequencer::FrameOverlapsBufferedData( // If there is a buffered frame with a higher starting offset, then we check // to see if the new frame runs into the higher frame. if (next_frame != buffered_frames_.end() && - (frame.offset + frame.data.TotalBufferSize()) > next_frame->first) { + (frame.offset + frame.data.size()) > next_frame->first) { DVLOG(1) << "New frame overlaps next frame: " << frame.offset << " + " - << frame.data.TotalBufferSize() << " > " << next_frame->first; + << frame.data.size() << " > " << next_frame->first; return true; } diff --git a/net/quic/quic_stream_sequencer.h b/net/quic/quic_stream_sequencer.h index 3570d3b86cc1a1..0f27ec6868c79f 100644 --- a/net/quic/quic_stream_sequencer.h +++ b/net/quic/quic_stream_sequencer.h @@ -104,7 +104,7 @@ class NET_EXPORT_PRIVATE QuicStreamSequencer { // bytes, and gaps. typedef std::map FrameMap; - // Stores buffered frames (maps from sequence number -> frame data as string). + // Stores buffered frames (maps from byte offset -> frame data as string). FrameMap buffered_frames_; // The offset, if any, we got a stream termination for. When this many bytes diff --git a/net/quic/quic_stream_sequencer_test.cc b/net/quic/quic_stream_sequencer_test.cc index e15f3d3e67d3bf..0f328c96a8125d 100644 --- a/net/quic/quic_stream_sequencer_test.cc +++ b/net/quic/quic_stream_sequencer_test.cc @@ -86,7 +86,7 @@ class QuicStreamSequencerTest : public ::testing::Test { QuicStreamFrame frame; frame.stream_id = 1; frame.offset = byte_offset; - frame.data.Append(const_cast(data), strlen(data)); + frame.data = StringPiece(data); frame.fin = true; sequencer_->OnStreamFrame(frame); } @@ -95,7 +95,7 @@ class QuicStreamSequencerTest : public ::testing::Test { QuicStreamFrame frame; frame.stream_id = 1; frame.offset = byte_offset; - frame.data.Append(const_cast(data), strlen(data)); + frame.data = StringPiece(data); frame.fin = false; sequencer_->OnStreamFrame(frame); } @@ -368,7 +368,7 @@ TEST_F(QuicStreamSequencerTest, FrameOverlapsBufferedData) { const int kBufferedOffset = 10; const int kBufferedDataLength = 3; const int kNewDataLength = 3; - IOVector data = MakeIOVector(string(kNewDataLength, '.')); + StringPiece data(string(kNewDataLength, '.')); // No overlap if no buffered frames. EXPECT_TRUE(buffered_frames_->empty()); @@ -408,10 +408,10 @@ TEST_F(QuicStreamSequencerTest, DontAcceptOverlappingFrames) { // The peer should never send us non-identical stream frames which contain // overlapping byte ranges - if they do, we close the connection. - QuicStreamFrame frame1(kClientDataStreamId1, false, 1, MakeIOVector("hello")); + QuicStreamFrame frame1(kClientDataStreamId1, false, 1, StringPiece("hello")); sequencer_->OnStreamFrame(frame1); - QuicStreamFrame frame2(kClientDataStreamId1, false, 2, MakeIOVector("hello")); + QuicStreamFrame frame2(kClientDataStreamId1, false, 2, StringPiece("hello")); EXPECT_TRUE(sequencer_->FrameOverlapsBufferedData(frame2)); EXPECT_CALL(stream_, CloseConnectionWithDetails(QUIC_INVALID_STREAM_FRAME, _)) .Times(1); diff --git a/net/quic/quic_unacked_packet_map_test.cc b/net/quic/quic_unacked_packet_map_test.cc index 150bebca4c25bf..2b4a0cafd2c34c 100644 --- a/net/quic/quic_unacked_packet_map_test.cc +++ b/net/quic/quic_unacked_packet_map_test.cc @@ -43,7 +43,7 @@ class QuicUnackedPacketMapTest : public ::testing::Test { RetransmittableFrames* frames = new RetransmittableFrames(ENCRYPTION_NONE); QuicStreamFrame* frame = new QuicStreamFrame(); frame->stream_id = stream_id; - frames->AddStreamFrame(frame); + frames->AddFrame(QuicFrame(frame)); return SerializedPacket(sequence_number, PACKET_1BYTE_SEQUENCE_NUMBER, packets_.back(), 0, frames); } diff --git a/net/quic/reliable_quic_stream.cc b/net/quic/reliable_quic_stream.cc index 54bf25efc1e460..d64f01091133e3 100644 --- a/net/quic/reliable_quic_stream.cc +++ b/net/quic/reliable_quic_stream.cc @@ -158,7 +158,7 @@ void ReliableQuicStream::OnStreamFrame(const QuicStreamFrame& frame) { } // This count includes duplicate data received. - size_t frame_payload_size = frame.data.TotalBufferSize(); + size_t frame_payload_size = frame.data.size(); stream_bytes_read_ += frame_payload_size; // Flow control is interested in tracking highest received offset. diff --git a/net/quic/reliable_quic_stream_test.cc b/net/quic/reliable_quic_stream_test.cc index b17def0d1b4fd7..982bb139443767 100644 --- a/net/quic/reliable_quic_stream_test.cc +++ b/net/quic/reliable_quic_stream_test.cc @@ -603,7 +603,7 @@ TEST_F(ReliableQuicStreamTest, // higher than the receive window offset. QuicStreamFrame frame(stream_->id(), false, kInitialSessionFlowControlWindowForTest + 1, - MakeIOVector(".")); + StringPiece(".")); EXPECT_GT(frame.offset, QuicFlowControllerPeer::ReceiveWindowOffset( stream_->flow_controller())); @@ -619,12 +619,12 @@ TEST_F(ReliableQuicStreamTest, FinalByteOffsetFromFin) { EXPECT_FALSE(stream_->HasFinalReceivedByteOffset()); QuicStreamFrame stream_frame_no_fin(stream_->id(), false, 1234, - MakeIOVector(".")); + StringPiece(".")); stream_->OnStreamFrame(stream_frame_no_fin); EXPECT_FALSE(stream_->HasFinalReceivedByteOffset()); QuicStreamFrame stream_frame_with_fin(stream_->id(), true, 1234, - MakeIOVector(".")); + StringPiece(".")); stream_->OnStreamFrame(stream_frame_with_fin); EXPECT_TRUE(stream_->HasFinalReceivedByteOffset()); } diff --git a/net/quic/test_tools/crypto_test_utils.cc b/net/quic/test_tools/crypto_test_utils.cc index f402256e112710..45d94d5c318a89 100644 --- a/net/quic/test_tools/crypto_test_utils.cc +++ b/net/quic/test_tools/crypto_test_utils.cc @@ -90,11 +90,8 @@ void MovePackets(PacketSavingConnection* source_conn, break; } - for (vector::const_iterator - i = framer.stream_frames().begin(); - i != framer.stream_frames().end(); ++i) { - scoped_ptr frame_data(i->GetDataAsString()); - ASSERT_TRUE(crypto_framer.ProcessInput(*frame_data)); + for (const QuicStreamFrame& stream_frame : framer.stream_frames()) { + ASSERT_TRUE(crypto_framer.ProcessInput(stream_frame.data)); ASSERT_FALSE(crypto_visitor.error()); } } @@ -104,10 +101,8 @@ void MovePackets(PacketSavingConnection* source_conn, ASSERT_EQ(0u, crypto_framer.InputBytesRemaining()); - for (vector::const_iterator - i = crypto_visitor.messages().begin(); - i != crypto_visitor.messages().end(); ++i) { - dest_stream->OnHandshakeMessage(*i); + for (const CryptoHandshakeMessage& message : crypto_visitor.messages()) { + dest_stream->OnHandshakeMessage(message); } } diff --git a/net/quic/test_tools/quic_packet_generator_peer.cc b/net/quic/test_tools/quic_packet_generator_peer.cc index c6ef40261869d6..5f5e231fc6fa91 100644 --- a/net/quic/test_tools/quic_packet_generator_peer.cc +++ b/net/quic/test_tools/quic_packet_generator_peer.cc @@ -22,5 +22,17 @@ QuicTime::Delta QuicPacketGeneratorPeer::GetFecTimeout( return generator->fec_timeout_; } +// static +FecSendPolicy QuicPacketGeneratorPeer::GetFecSendPolicy( + QuicPacketGenerator* generator) { + return generator->fec_send_policy_; +} + +// static +void QuicPacketGeneratorPeer::SetFecSendPolicy(QuicPacketGenerator* generator, + FecSendPolicy fec_send_policy) { + generator->fec_send_policy_ = fec_send_policy; +} + } // namespace test } // namespace net diff --git a/net/quic/test_tools/quic_packet_generator_peer.h b/net/quic/test_tools/quic_packet_generator_peer.h index fba8dbee70fbe8..9e2078d58eb998 100644 --- a/net/quic/test_tools/quic_packet_generator_peer.h +++ b/net/quic/test_tools/quic_packet_generator_peer.h @@ -18,6 +18,9 @@ class QuicPacketGeneratorPeer { public: static QuicPacketCreator* GetPacketCreator(QuicPacketGenerator* generator); static QuicTime::Delta GetFecTimeout(QuicPacketGenerator* generator); + static FecSendPolicy GetFecSendPolicy(QuicPacketGenerator* generator); + static void SetFecSendPolicy(QuicPacketGenerator* generator, + FecSendPolicy fec_send_policy); private: DISALLOW_COPY_AND_ASSIGN(QuicPacketGeneratorPeer); diff --git a/net/quic/test_tools/quic_test_packet_maker.cc b/net/quic/test_tools/quic_test_packet_maker.cc index 2ac6b405d271b0..3d6ce0ed0df84b 100644 --- a/net/quic/test_tools/quic_test_packet_maker.cc +++ b/net/quic/test_tools/quic_test_packet_maker.cc @@ -167,7 +167,7 @@ scoped_ptr QuicTestPacketMaker::MakeDataPacket( QuicStreamOffset offset, base::StringPiece data) { InitializeHeader(sequence_number, should_include_version); - QuicStreamFrame frame(stream_id, fin, offset, MakeIOVector(data)); + QuicStreamFrame frame(stream_id, fin, offset, data); return MakePacket(header_, QuicFrame(&frame)); } @@ -194,9 +194,9 @@ scoped_ptr QuicTestPacketMaker::MakeRequestHeadersPacket( headers_frame.set_has_priority(true); spdy_frame.reset(spdy_request_framer_.SerializeFrame(headers_frame)); } - QuicStreamFrame frame(kHeadersStreamId, false, 0, - MakeIOVector(base::StringPiece(spdy_frame->data(), - spdy_frame->size()))); + QuicStreamFrame frame( + kHeadersStreamId, false, 0, + base::StringPiece(spdy_frame->data(), spdy_frame->size())); return MakePacket(header_, QuicFrame(&frame)); } @@ -219,9 +219,9 @@ scoped_ptr QuicTestPacketMaker::MakeResponseHeadersPacket( headers_frame.set_fin(fin); spdy_frame.reset(spdy_request_framer_.SerializeFrame(headers_frame)); } - QuicStreamFrame frame(kHeadersStreamId, false, 0, - MakeIOVector(base::StringPiece(spdy_frame->data(), - spdy_frame->size()))); + QuicStreamFrame frame( + kHeadersStreamId, false, 0, + base::StringPiece(spdy_frame->data(), spdy_frame->size())); return MakePacket(header_, QuicFrame(&frame)); } diff --git a/net/quic/test_tools/quic_test_utils.cc b/net/quic/test_tools/quic_test_utils.cc index 5f687b29cb1050..549dae5821b8e2 100644 --- a/net/quic/test_tools/quic_test_utils.cc +++ b/net/quic/test_tools/quic_test_utils.cc @@ -538,7 +538,7 @@ QuicEncryptedPacket* ConstructEncryptedPacket( header.fec_flag = false; header.is_in_fec_group = NOT_IN_FEC_GROUP; header.fec_group = 0; - QuicStreamFrame stream_frame(1, false, 0, MakeIOVector(data)); + QuicStreamFrame stream_frame(1, false, 0, StringPiece(data)); QuicFrame frame(&stream_frame); QuicFrames frames; frames.push_back(frame); @@ -576,7 +576,7 @@ QuicEncryptedPacket* ConstructMisFramedEncryptedPacket( header.fec_flag = false; header.is_in_fec_group = NOT_IN_FEC_GROUP; header.fec_group = 0; - QuicStreamFrame stream_frame(1, false, 0, MakeIOVector(data)); + QuicStreamFrame stream_frame(1, false, 0, StringPiece(data)); QuicFrame frame(&stream_frame); QuicFrames frames; frames.push_back(frame); @@ -663,7 +663,7 @@ static QuicPacket* ConstructPacketFromHandshakeMessage( header.fec_group = 0; QuicStreamFrame stream_frame(kCryptoStreamId, false, 0, - MakeIOVector(data->AsStringPiece())); + data->AsStringPiece()); QuicFrame frame(&stream_frame); QuicFrames frames; diff --git a/net/quic/test_tools/simple_quic_framer.cc b/net/quic/test_tools/simple_quic_framer.cc index 639a2f6c40d48f..7784695d035392 100644 --- a/net/quic/test_tools/simple_quic_framer.cc +++ b/net/quic/test_tools/simple_quic_framer.cc @@ -56,12 +56,12 @@ class SimpleFramerVisitor : public QuicFramerVisitorInterface { bool OnStreamFrame(const QuicStreamFrame& frame) override { // Save a copy of the data so it is valid after the packet is processed. - stream_data_.push_back(frame.GetDataAsString()); + string* string_data = new string(); + frame.data.AppendToString(string_data); + stream_data_.push_back(string_data); QuicStreamFrame stream_frame(frame); // Make sure that the stream frame points to this data. - stream_frame.data.Clear(); - stream_frame.data.Append(const_cast(stream_data_.back()->data()), - stream_data_.back()->size()); + stream_frame.data = StringPiece(*string_data); stream_frames_.push_back(stream_frame); return true; } diff --git a/net/tools/quic/quic_client_bin.cc b/net/tools/quic/quic_client_bin.cc index 45ef898f6d0798..b5a1761b11dd15 100644 --- a/net/tools/quic/quic_client_bin.cc +++ b/net/tools/quic/quic_client_bin.cc @@ -192,7 +192,7 @@ int main(int argc, char *argv[]) { // Build the client, and try to connect. bool is_https = (FLAGS_port == 443); net::EpollServer epoll_server; - net::QuicServerId server_id(host, FLAGS_port, is_https, + net::QuicServerId server_id(url.host(), FLAGS_port, is_https, net::PRIVACY_MODE_DISABLED); net::QuicVersionVector versions = net::QuicSupportedVersions(); if (FLAGS_quic_version != -1) { diff --git a/net/tools/quic/quic_server_session_test.cc b/net/tools/quic/quic_server_session_test.cc index b48021b9b6a565..4716e81f62018a 100644 --- a/net/tools/quic/quic_server_session_test.cc +++ b/net/tools/quic/quic_server_session_test.cc @@ -118,7 +118,7 @@ INSTANTIATE_TEST_CASE_P(Tests, QuicServerSessionTest, TEST_P(QuicServerSessionTest, CloseStreamDueToReset) { // Open a stream, then reset it. // Send two bytes of payload to open it. - QuicStreamFrame data1(kClientDataStreamId1, false, 0, MakeIOVector("HT")); + QuicStreamFrame data1(kClientDataStreamId1, false, 0, StringPiece("HT")); vector frames; frames.push_back(data1); session_->OnStreamFrames(frames); @@ -148,7 +148,7 @@ TEST_P(QuicServerSessionTest, NeverOpenStreamDueToReset) { EXPECT_EQ(0u, session_->GetNumOpenStreams()); // Send two bytes of payload. - QuicStreamFrame data1(kClientDataStreamId1, false, 0, MakeIOVector("HT")); + QuicStreamFrame data1(kClientDataStreamId1, false, 0, StringPiece("HT")); vector frames; frames.push_back(data1); visitor_->OnStreamFrames(frames); @@ -162,9 +162,9 @@ TEST_P(QuicServerSessionTest, AcceptClosedStream) { vector frames; // Send (empty) compressed headers followed by two bytes of data. frames.push_back(QuicStreamFrame(kClientDataStreamId1, false, 0, - MakeIOVector("\1\0\0\0\0\0\0\0HT"))); + StringPiece("\1\0\0\0\0\0\0\0HT"))); frames.push_back(QuicStreamFrame(kClientDataStreamId2, false, 0, - MakeIOVector("\2\0\0\0\0\0\0\0HT"))); + StringPiece("\2\0\0\0\0\0\0\0HT"))); visitor_->OnStreamFrames(frames); EXPECT_EQ(2u, session_->GetNumOpenStreams()); @@ -179,9 +179,9 @@ TEST_P(QuicServerSessionTest, AcceptClosedStream) { // data on the floor, but accept the packet because it has data for stream 5. frames.clear(); frames.push_back( - QuicStreamFrame(kClientDataStreamId1, false, 2, MakeIOVector("TP"))); + QuicStreamFrame(kClientDataStreamId1, false, 2, StringPiece("TP"))); frames.push_back( - QuicStreamFrame(kClientDataStreamId2, false, 2, MakeIOVector("TP"))); + QuicStreamFrame(kClientDataStreamId2, false, 2, StringPiece("TP"))); visitor_->OnStreamFrames(frames); // The stream should never be opened, now that the reset is received. EXPECT_EQ(1u, session_->GetNumOpenStreams()); diff --git a/net/tools/quic/quic_spdy_client_stream_test.cc b/net/tools/quic/quic_spdy_client_stream_test.cc index 901bf8e2fc3d6a..7e30c92bee3d76 100644 --- a/net/tools/quic/quic_spdy_client_stream_test.cc +++ b/net/tools/quic/quic_spdy_client_stream_test.cc @@ -17,6 +17,7 @@ using net::test::DefaultQuicConfig; using net::test::MockConnection; using net::test::SupportedVersions; +using net::test::kClientDataStreamId1; using std::string; using testing::StrictMock; using testing::TestWithParam; @@ -50,7 +51,7 @@ class QuicSpdyClientStreamTest : public TestWithParam { kInitialStreamFlowControlWindowForTest); session_.config()->SetInitialSessionFlowControlWindowToSend( kInitialSessionFlowControlWindowForTest); - stream_.reset(new QuicSpdyClientStream(3, &session_)); + stream_.reset(new QuicSpdyClientStream(kClientDataStreamId1, &session_)); } StrictMock* connection_; @@ -113,7 +114,7 @@ TEST_P(QuicSpdyClientStreamTest, DISABLED_TestFramingExtraData) { } TEST_P(QuicSpdyClientStreamTest, TestNoBidirectionalStreaming) { - QuicStreamFrame frame(3, false, 3, MakeIOVector("asd")); + QuicStreamFrame frame(kClientDataStreamId1, false, 3, StringPiece("asd")); EXPECT_FALSE(stream_->write_side_closed()); stream_->OnStreamFrame(frame); diff --git a/net/tools/quic/quic_spdy_server_stream_test.cc b/net/tools/quic/quic_spdy_server_stream_test.cc index b32a194f541436..9290a79a212b1c 100644 --- a/net/tools/quic/quic_spdy_server_stream_test.cc +++ b/net/tools/quic/quic_spdy_server_stream_test.cc @@ -243,7 +243,7 @@ TEST_P(QuicSpdyServerStreamTest, InvalidHeadersWithFin) { 0x31, 0x2e, 0x31, // 1.1 }; StringPiece data(arr, arraysize(arr)); - QuicStreamFrame frame(stream_->id(), true, 0, MakeIOVector(data)); + QuicStreamFrame frame(stream_->id(), true, 0, data); // Verify that we don't crash when we get a invalid headers in stream frame. stream_->OnStreamFrame(frame); } diff --git a/net/tools/quic/quic_time_wait_list_manager_test.cc b/net/tools/quic/quic_time_wait_list_manager_test.cc index 8f12c7eb675071..423bd9530609bf 100644 --- a/net/tools/quic/quic_time_wait_list_manager_test.cc +++ b/net/tools/quic/quic_time_wait_list_manager_test.cc @@ -162,7 +162,7 @@ class QuicTimeWaitListManagerTest : public ::testing::Test { header.fec_flag = false; header.is_in_fec_group = NOT_IN_FEC_GROUP; header.fec_group = 0; - QuicStreamFrame stream_frame(1, false, 0, MakeIOVector("data")); + QuicStreamFrame stream_frame(1, false, 0, StringPiece("data")); QuicFrame frame(&stream_frame); QuicFrames frames; frames.push_back(frame);