Clean up HpackVarintEncoder internals. This is a follow-up to cl/254505501, which simplifies the API. In the end, Encode() can be made static. gfe-relnote: No functional change; not flag protected. PiperOrigin-RevId: 254804541 Change-Id: Ide3833a1f2452df9b8f444e6aff0fca4217cc485
diff --git a/http2/hpack/tools/hpack_block_builder.cc b/http2/hpack/tools/hpack_block_builder.cc index be62346..5b2ab75 100644 --- a/http2/hpack/tools/hpack_block_builder.cc +++ b/http2/hpack/tools/hpack_block_builder.cc
@@ -17,8 +17,7 @@ EXPECT_LE(3, prefix_length); EXPECT_LE(prefix_length, 8); - HpackVarintEncoder varint_encoder; - varint_encoder.Encode(high_bits, prefix_length, varint, &buffer_); + HpackVarintEncoder::Encode(high_bits, prefix_length, varint, &buffer_); } void HpackBlockBuilder::AppendEntryTypeAndVarint(HpackEntryType entry_type,
diff --git a/http2/hpack/varint/hpack_varint_encoder.cc b/http2/hpack/varint/hpack_varint_encoder.cc index f69c734..63cf289 100644 --- a/http2/hpack/varint/hpack_varint_encoder.cc +++ b/http2/hpack/varint/hpack_varint_encoder.cc
@@ -10,28 +10,11 @@ namespace http2 { -HpackVarintEncoder::HpackVarintEncoder() - : varint_(0), encoding_in_progress_(false) {} - +// static void HpackVarintEncoder::Encode(uint8_t high_bits, uint8_t prefix_length, uint64_t varint, Http2String* output) { - unsigned char first_byte = StartEncoding(high_bits, prefix_length, varint); - output->push_back(first_byte); - if (!IsEncodingInProgress()) { - return; - } - - ResumeEncoding(std::numeric_limits<size_t>::max(), output); - DCHECK(!IsEncodingInProgress()); -} - -unsigned char HpackVarintEncoder::StartEncoding(uint8_t high_bits, - uint8_t prefix_length, - uint64_t varint) { - DCHECK(!encoding_in_progress_); - DCHECK_EQ(0u, varint_); DCHECK_LE(1u, prefix_length); DCHECK_LE(prefix_length, 8u); @@ -43,39 +26,24 @@ if (varint < prefix_mask) { // The integer fits into the prefix in its entirety. - return high_bits | static_cast<unsigned char>(varint); + unsigned char first_byte = high_bits | static_cast<unsigned char>(varint); + output->push_back(first_byte); + return; } - // We need extension bytes. - varint_ = varint - prefix_mask; - encoding_in_progress_ = true; - return high_bits | prefix_mask; -} + // Extension bytes are needed. + unsigned char first_byte = high_bits | prefix_mask; + output->push_back(first_byte); -size_t HpackVarintEncoder::ResumeEncoding(size_t max_encoded_bytes, - Http2String* output) { - DCHECK(encoding_in_progress_); - DCHECK_NE(0u, max_encoded_bytes); - - size_t encoded_bytes = 0; - while (encoded_bytes < max_encoded_bytes) { - ++encoded_bytes; - if (varint_ < 128) { - // Encode final seven bits, with continuation bit set to zero. - output->push_back(varint_); - varint_ = 0; - encoding_in_progress_ = false; - break; - } + varint -= prefix_mask; + while (varint >= 128) { // Encode the next seven bits, with continuation bit set to one. - output->push_back(0b10000000 | (varint_ % 128)); - varint_ >>= 7; + output->push_back(0b10000000 | (varint % 128)); + varint >>= 7; } - return encoded_bytes; -} -bool HpackVarintEncoder::IsEncodingInProgress() const { - return encoding_in_progress_; + // Encode final seven bits, with continuation bit set to zero. + output->push_back(varint); } } // namespace http2
diff --git a/http2/hpack/varint/hpack_varint_encoder.h b/http2/hpack/varint/hpack_varint_encoder.h index a5a9cd8..745222e 100644 --- a/http2/hpack/varint/hpack_varint_encoder.h +++ b/http2/hpack/varint/hpack_varint_encoder.h
@@ -13,47 +13,17 @@ namespace http2 { -// HPACK integer encoder class implementing variable length integer -// representation defined in RFC7541, Section 5.1: +// HPACK integer encoder class with single static method implementing variable +// length integer representation defined in RFC7541, Section 5.1: // https://httpwg.org/specs/rfc7541.html#integer.representation class HTTP2_EXPORT_PRIVATE HpackVarintEncoder { public: - HpackVarintEncoder(); - - // Encode |varint|, appending encoded data to |*output|. Appends between 1 - // and 11 bytes in total. Must not be called when another encoding task - // previously started by StartEncoding() is still in progress. No encoding - // will be in progress after this method finishes. - void Encode(uint8_t high_bits, - uint8_t prefix_length, - uint64_t varint, - Http2String* output); - - private: - // Start encoding an integer. Return the first encoded byte (composed of - // optional high bits and 1 to 8 bit long prefix). It is possible that this - // completes the encoding. Must not be called when previously started - // encoding is still in progress. - unsigned char StartEncoding(uint8_t high_bits, - uint8_t prefix_length, - uint64_t varint); - - // Continue encoding the integer |varint| passed in to StartEncoding(). - // Append the next at most |max_encoded_bytes| encoded octets to |output|. - // Returns the number of encoded octets. Must not be called unless a - // previously started encoding is still in progress. - size_t ResumeEncoding(size_t max_encoded_bytes, Http2String* output); - - // Returns true if encoding an integer has started and is not completed yet. - bool IsEncodingInProgress() const; - - // The original integer shifted to the right by the number of bits already - // encoded. The lower bits shifted away have already been encoded, and - // |varint_| has the higher bits that remain to be encoded. - uint64_t varint_; - - // True when encoding an integer has started and is not completed yet. - bool encoding_in_progress_; + // Encode |varint|, appending encoded data to |*output|. + // Appends between 1 and 11 bytes in total. + static void Encode(uint8_t high_bits, + uint8_t prefix_length, + uint64_t varint, + Http2String* output); }; } // namespace http2
diff --git a/http2/hpack/varint/hpack_varint_encoder_test.cc b/http2/hpack/varint/hpack_varint_encoder_test.cc index 96ade85..fa05948 100644 --- a/http2/hpack/varint/hpack_varint_encoder_test.cc +++ b/http2/hpack/varint/hpack_varint_encoder_test.cc
@@ -30,13 +30,11 @@ // Encode integers that fit in the prefix. TEST(HpackVarintEncoderTest, Short) { - HpackVarintEncoder varint_encoder; - for (size_t i = 0; i < HTTP2_ARRAYSIZE(kShortTestData); ++i) { Http2String output; - varint_encoder.Encode(kShortTestData[i].high_bits, - kShortTestData[i].prefix_length, - kShortTestData[i].value, &output); + HpackVarintEncoder::Encode(kShortTestData[i].high_bits, + kShortTestData[i].prefix_length, + kShortTestData[i].value, &output); ASSERT_EQ(1u, output.size()); EXPECT_EQ(kShortTestData[i].expected_encoding, output[0]); } @@ -102,8 +100,6 @@ // Encode integers that do not fit in the prefix. TEST(HpackVarintEncoderTest, Long) { - HpackVarintEncoder varint_encoder; - // Test encoding byte by byte, also test encoding in // a single ResumeEncoding() call. for (size_t i = 0; i < HTTP2_ARRAYSIZE(kLongTestData); ++i) { @@ -111,9 +107,9 @@ Http2HexDecode(kLongTestData[i].expected_encoding); Http2String output; - varint_encoder.Encode(kLongTestData[i].high_bits, - kLongTestData[i].prefix_length, - kLongTestData[i].value, &output); + HpackVarintEncoder::Encode(kLongTestData[i].high_bits, + kLongTestData[i].prefix_length, + kLongTestData[i].value, &output); EXPECT_EQ(expected_encoding, output); } @@ -133,13 +129,11 @@ // Make sure that the encoder outputs the last byte even when it is zero. This // happens exactly when encoding the value 2^prefix_length - 1. TEST(HpackVarintEncoderTest, LastByteIsZero) { - HpackVarintEncoder varint_encoder; - for (size_t i = 0; i < HTTP2_ARRAYSIZE(kLastByteIsZeroTestData); ++i) { Http2String output; - varint_encoder.Encode(kLastByteIsZeroTestData[i].high_bits, - kLastByteIsZeroTestData[i].prefix_length, - kLastByteIsZeroTestData[i].value, &output); + HpackVarintEncoder::Encode(kLastByteIsZeroTestData[i].high_bits, + kLastByteIsZeroTestData[i].prefix_length, + kLastByteIsZeroTestData[i].value, &output); ASSERT_EQ(2u, output.size()); EXPECT_EQ(kLastByteIsZeroTestData[i].expected_encoding_first_byte, output[0]); @@ -149,17 +143,16 @@ // Test that encoder appends correctly to non-empty string. TEST(HpackVarintEncoderTest, Append) { - HpackVarintEncoder varint_encoder; Http2String output("foo"); EXPECT_EQ(Http2HexDecode("666f6f"), output); - varint_encoder.Encode(0b10011000, 3, 103, &output); + HpackVarintEncoder::Encode(0b10011000, 3, 103, &output); EXPECT_EQ(Http2HexDecode("666f6f9f60"), output); - varint_encoder.Encode(0b10100000, 5, 8, &output); + HpackVarintEncoder::Encode(0b10100000, 5, 8, &output); EXPECT_EQ(Http2HexDecode("666f6f9f60a8"), output); - varint_encoder.Encode(0b10011000, 3, 202147110, &output); + HpackVarintEncoder::Encode(0b10011000, 3, 202147110, &output); EXPECT_EQ(Http2HexDecode("666f6f9f60a89f9f8ab260"), output); }
diff --git a/quic/core/qpack/qpack_instruction_encoder.cc b/quic/core/qpack/qpack_instruction_encoder.cc index b2acdbd..277079b 100644 --- a/quic/core/qpack/qpack_instruction_encoder.cc +++ b/quic/core/qpack/qpack_instruction_encoder.cc
@@ -7,6 +7,7 @@ #include <limits> #include "net/third_party/quiche/src/http2/hpack/huffman/hpack_huffman_encoder.h" +#include "net/third_party/quiche/src/http2/hpack/varint/hpack_varint_encoder.h" #include "net/third_party/quiche/src/quic/platform/api/quic_logging.h" #include "net/third_party/quiche/src/quic/platform/api/quic_string_utils.h" @@ -110,7 +111,8 @@ break; } - varint_encoder_.Encode(byte_, field_->param, integer_to_encode, output); + http2::HpackVarintEncoder::Encode(byte_, field_->param, integer_to_encode, + output); byte_ = 0; if (field_->type == QpackInstructionFieldType::kVarint ||
diff --git a/quic/core/qpack/qpack_instruction_encoder.h b/quic/core/qpack/qpack_instruction_encoder.h index a831a07..c484104 100644 --- a/quic/core/qpack/qpack_instruction_encoder.h +++ b/quic/core/qpack/qpack_instruction_encoder.h
@@ -8,7 +8,6 @@ #include <cstdint> #include <string> -#include "net/third_party/quiche/src/http2/hpack/varint/hpack_varint_encoder.h" #include "net/third_party/quiche/src/quic/core/qpack/qpack_constants.h" #include "net/third_party/quiche/src/quic/platform/api/quic_export.h" #include "net/third_party/quiche/src/quic/platform/api/quic_string_piece.h" @@ -93,9 +92,6 @@ // Field currently being decoded. QpackInstructionFields::const_iterator field_; - - // Decoder instance for decoding integers. - http2::HpackVarintEncoder varint_encoder_; }; } // namespace quic