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
