Also test http2::HuffmanEncodeFast() in spdy::HpackEncoderTest. spdy::HpackHuffmanTable::EncodeString() will be deprecated and replaced by http2::HuffmanEncodeFast(). With this change, HpackEncoderTest generates data using both and compares that against HpackEncoder which currently uses spdy::HpackHuffmanTable::EncodeString() but will alternatively be using http2::HuffmanEncodeFast() controlled by a flag, to provide extended test coverage. PiperOrigin-RevId: 335869311 Change-Id: If50b36933c0aec87a66279b54cf70c9452933c90
diff --git a/spdy/core/hpack/hpack_encoder_test.cc b/spdy/core/hpack/hpack_encoder_test.cc index 8e9a2d6..62c7887 100644 --- a/spdy/core/hpack/hpack_encoder_test.cc +++ b/spdy/core/hpack/hpack_encoder_test.cc
@@ -5,8 +5,9 @@ #include "net/third_party/quiche/src/spdy/core/hpack/hpack_encoder.h" #include <cstdint> -#include <map> +#include <utility> +#include "net/third_party/quiche/src/http2/hpack/huffman/hpack_huffman_encoder.h" #include "net/third_party/quiche/src/http2/test_tools/http2_random.h" #include "net/third_party/quiche/src/common/platform/api/quiche_test.h" #include "net/third_party/quiche/src/spdy/core/hpack/hpack_huffman_table.h" @@ -125,14 +126,17 @@ kRepresentations, }; -class HpackEncoderTestBase : public QuicheTest { +class HpackEncoderTest + : public QuicheTestWithParam<std::tuple<EncodeStrategy, bool>> { protected: typedef test::HpackEncoderPeer::Representations Representations; - HpackEncoderTestBase() + HpackEncoderTest() : peer_(&encoder_), static_(peer_.table()->GetByIndex(1)), - headers_storage_(1024 /* block size */) {} + headers_storage_(1024 /* block size */), + strategy_(std::get<0>(GetParam())), + use_fast_huffman_encoder_(std::get<1>(GetParam())) {} void SetUp() override { // Populate dynamic entries into the table fixture. For simplicity each @@ -187,13 +191,19 @@ } void ExpectString(HpackOutputStream* stream, quiche::QuicheStringPiece str) { const HpackHuffmanTable& huffman_table = ObtainHpackHuffmanTable(); - size_t encoded_size = peer_.compression_enabled() - ? huffman_table.EncodedSize(str) - : str.size(); + size_t encoded_size = + peer_.compression_enabled() + ? (use_fast_huffman_encoder_ ? http2::HuffmanSize(str) + : huffman_table.EncodedSize(str)) + : str.size(); if (encoded_size < str.size()) { expected_.AppendPrefix(kStringLiteralHuffmanEncoded); expected_.AppendUint32(encoded_size); - huffman_table.EncodeString(str, stream); + if (use_fast_huffman_encoder_) { + http2::HuffmanEncodeFast(str, encoded_size, stream->MutableString()); + } else { + huffman_table.EncodeString(str, stream); + } } else { expected_.AppendPrefix(kStringLiteralIdentityEncoded); expected_.AppendUint32(str.size()); @@ -255,10 +265,18 @@ headers_observed_; HpackOutputStream expected_; - EncodeStrategy strategy_ = kDefault; + const EncodeStrategy strategy_; + const bool use_fast_huffman_encoder_; }; -TEST_F(HpackEncoderTestBase, EncodeRepresentations) { +using HpackEncoderTestWithDefaultStrategy = HpackEncoderTest; + +INSTANTIATE_TEST_SUITE_P(HpackEncoderTests, + HpackEncoderTestWithDefaultStrategy, + ::testing::Combine(::testing::Values(kDefault), + ::testing::Bool())); + +TEST_P(HpackEncoderTestWithDefaultStrategy, EncodeRepresentations) { encoder_.SetHeaderListener( [this](quiche::QuicheStringPiece name, quiche::QuicheStringPiece value) { this->SaveHeaders(name, value); @@ -290,20 +308,12 @@ Pair("withnul", quiche::QuicheStringPiece("one\0two", 7)))); } -class HpackEncoderTest : public HpackEncoderTestBase, - public ::testing::WithParamInterface<EncodeStrategy> { - protected: - void SetUp() override { - strategy_ = GetParam(); - HpackEncoderTestBase::SetUp(); - } -}; - INSTANTIATE_TEST_SUITE_P(HpackEncoderTests, HpackEncoderTest, - ::testing::Values(kDefault, - kIncremental, - kRepresentations)); + ::testing::Combine(::testing::Values(kDefault, + kIncremental, + kRepresentations), + ::testing::Bool())); TEST_P(HpackEncoderTest, SingleDynamicIndex) { encoder_.SetHeaderListener(
diff --git a/spdy/core/hpack/hpack_output_stream.cc b/spdy/core/hpack/hpack_output_stream.cc index 5c2dd68..c53f07f 100644 --- a/spdy/core/hpack/hpack_output_stream.cc +++ b/spdy/core/hpack/hpack_output_stream.cc
@@ -61,6 +61,12 @@ } AppendBits(static_cast<uint8_t>(I), 8); } + DCHECK_EQ(bit_offset_, 0u); +} + +std::string* HpackOutputStream::MutableString() { + DCHECK_EQ(bit_offset_, 0u); + return &buffer_; } void HpackOutputStream::TakeString(std::string* output) {
diff --git a/spdy/core/hpack/hpack_output_stream.h b/spdy/core/hpack/hpack_output_stream.h index f7e71f1..f99097a 100644 --- a/spdy/core/hpack/hpack_output_stream.h +++ b/spdy/core/hpack/hpack_output_stream.h
@@ -48,6 +48,9 @@ // boundary after this function is called. void AppendUint32(uint32_t I); + // Return pointer to internal buffer. |bit_offset_| needs to be zero. + std::string* MutableString(); + // Swaps the internal buffer with |output|, then resets state. void TakeString(std::string* output);
diff --git a/spdy/core/hpack/hpack_output_stream_test.cc b/spdy/core/hpack/hpack_output_stream_test.cc index 21e6947..20d320c 100644 --- a/spdy/core/hpack/hpack_output_stream_test.cc +++ b/spdy/core/hpack/hpack_output_stream_test.cc
@@ -271,6 +271,20 @@ EXPECT_EQ("\x10", str); } +TEST(HpackOutputStreamTest, MutableString) { + HpackOutputStream output_stream; + + output_stream.AppendBytes("1"); + output_stream.MutableString()->append("2"); + + output_stream.AppendBytes("foo"); + output_stream.MutableString()->append("bar"); + + std::string str; + output_stream.TakeString(&str); + EXPECT_EQ("12foobar", str); +} + } // namespace } // namespace spdy