Factor out QpackInstructionEncoder::Values.
Factor out some QpackInstructionEncoder members into a struct that the caller
can pass to Encode(). Also add DCHECK to Encode() and rename DoStaticBit() to
DoSBit() (since is can be used for either static bit or sign bit).
gfe-relnote: n/a, change to QUIC v99-only code.
PiperOrigin-RevId: 258548817
Change-Id: I1c5d02b16af1e2db4c71813cdfad9548774a8c48
diff --git a/quic/core/qpack/qpack_decoder_stream_sender.cc b/quic/core/qpack/qpack_decoder_stream_sender.cc
index 4586fd3..63f1721 100644
--- a/quic/core/qpack/qpack_decoder_stream_sender.cc
+++ b/quic/core/qpack/qpack_decoder_stream_sender.cc
@@ -20,27 +20,30 @@
}
void QpackDecoderStreamSender::SendInsertCountIncrement(uint64_t increment) {
- instruction_encoder_.set_varint(increment);
+ values_.varint = increment;
std::string output;
- instruction_encoder_.Encode(InsertCountIncrementInstruction(), &output);
+ instruction_encoder_.Encode(InsertCountIncrementInstruction(), values_,
+ &output);
delegate_->WriteStreamData(output);
}
void QpackDecoderStreamSender::SendHeaderAcknowledgement(
QuicStreamId stream_id) {
- instruction_encoder_.set_varint(stream_id);
+ values_.varint = stream_id;
std::string output;
- instruction_encoder_.Encode(HeaderAcknowledgementInstruction(), &output);
+ instruction_encoder_.Encode(HeaderAcknowledgementInstruction(), values_,
+ &output);
delegate_->WriteStreamData(output);
}
void QpackDecoderStreamSender::SendStreamCancellation(QuicStreamId stream_id) {
- instruction_encoder_.set_varint(stream_id);
+ values_.varint = stream_id;
std::string output;
- instruction_encoder_.Encode(StreamCancellationInstruction(), &output);
+ instruction_encoder_.Encode(StreamCancellationInstruction(), values_,
+ &output);
delegate_->WriteStreamData(output);
}
diff --git a/quic/core/qpack/qpack_decoder_stream_sender.h b/quic/core/qpack/qpack_decoder_stream_sender.h
index 558996e..5f17744 100644
--- a/quic/core/qpack/qpack_decoder_stream_sender.h
+++ b/quic/core/qpack/qpack_decoder_stream_sender.h
@@ -37,6 +37,7 @@
private:
QpackStreamSenderDelegate* const delegate_;
QpackInstructionEncoder instruction_encoder_;
+ QpackInstructionEncoder::Values values_;
};
} // namespace quic
diff --git a/quic/core/qpack/qpack_encoder.cc b/quic/core/qpack/qpack_encoder.cc
index c5997b1..412b7af 100644
--- a/quic/core/qpack/qpack_encoder.cc
+++ b/quic/core/qpack/qpack_encoder.cc
@@ -34,11 +34,13 @@
// TODO(bnc): Implement dynamic entries and set Required Insert Count and
// Delta Base accordingly.
- instruction_encoder.set_varint(0);
- instruction_encoder.set_varint2(0);
- instruction_encoder.set_s_bit(false);
+ QpackInstructionEncoder::Values values;
+ values.varint = 0;
+ values.varint2 = 0;
+ values.s_bit = false;
- instruction_encoder.Encode(QpackPrefixInstruction(), &encoded_headers);
+ instruction_encoder.Encode(QpackPrefixInstruction(), values,
+ &encoded_headers);
for (const auto& header : ValueSplittingHeaderList(header_list)) {
QuicStringPiece name = header.first;
@@ -54,30 +56,30 @@
case QpackHeaderTable::MatchType::kNameAndValue:
DCHECK(is_static) << "Dynamic table entries not supported yet.";
- instruction_encoder.set_s_bit(is_static);
- instruction_encoder.set_varint(index);
+ values.s_bit = is_static;
+ values.varint = index;
- instruction_encoder.Encode(QpackIndexedHeaderFieldInstruction(),
+ instruction_encoder.Encode(QpackIndexedHeaderFieldInstruction(), values,
&encoded_headers);
break;
case QpackHeaderTable::MatchType::kName:
DCHECK(is_static) << "Dynamic table entries not supported yet.";
- instruction_encoder.set_s_bit(is_static);
- instruction_encoder.set_varint(index);
- instruction_encoder.set_value(value);
+ values.s_bit = is_static;
+ values.varint = index;
+ values.value = value;
instruction_encoder.Encode(
- QpackLiteralHeaderFieldNameReferenceInstruction(),
+ QpackLiteralHeaderFieldNameReferenceInstruction(), values,
&encoded_headers);
break;
case QpackHeaderTable::MatchType::kNoMatch:
- instruction_encoder.set_name(name);
- instruction_encoder.set_value(value);
+ values.name = name;
+ values.value = value;
- instruction_encoder.Encode(QpackLiteralHeaderFieldInstruction(),
+ instruction_encoder.Encode(QpackLiteralHeaderFieldInstruction(), values,
&encoded_headers);
break;
diff --git a/quic/core/qpack/qpack_encoder_stream_sender.cc b/quic/core/qpack/qpack_encoder_stream_sender.cc
index ca1da4f..dce183a 100644
--- a/quic/core/qpack/qpack_encoder_stream_sender.cc
+++ b/quic/core/qpack/qpack_encoder_stream_sender.cc
@@ -23,39 +23,42 @@
bool is_static,
uint64_t name_index,
QuicStringPiece value) {
- instruction_encoder_.set_s_bit(is_static);
- instruction_encoder_.set_varint(name_index);
- instruction_encoder_.set_value(value);
+ values_.s_bit = is_static;
+ values_.varint = name_index;
+ values_.value = value;
std::string output;
- instruction_encoder_.Encode(InsertWithNameReferenceInstruction(), &output);
+ instruction_encoder_.Encode(InsertWithNameReferenceInstruction(), values_,
+ &output);
delegate_->WriteStreamData(output);
}
void QpackEncoderStreamSender::SendInsertWithoutNameReference(
QuicStringPiece name,
QuicStringPiece value) {
- instruction_encoder_.set_name(name);
- instruction_encoder_.set_value(value);
+ values_.name = name;
+ values_.value = value;
std::string output;
- instruction_encoder_.Encode(InsertWithoutNameReferenceInstruction(), &output);
+ instruction_encoder_.Encode(InsertWithoutNameReferenceInstruction(), values_,
+ &output);
delegate_->WriteStreamData(output);
}
void QpackEncoderStreamSender::SendDuplicate(uint64_t index) {
- instruction_encoder_.set_varint(index);
+ values_.varint = index;
std::string output;
- instruction_encoder_.Encode(DuplicateInstruction(), &output);
+ instruction_encoder_.Encode(DuplicateInstruction(), values_, &output);
delegate_->WriteStreamData(output);
}
void QpackEncoderStreamSender::SendSetDynamicTableCapacity(uint64_t capacity) {
- instruction_encoder_.set_varint(capacity);
+ values_.varint = capacity;
std::string output;
- instruction_encoder_.Encode(SetDynamicTableCapacityInstruction(), &output);
+ instruction_encoder_.Encode(SetDynamicTableCapacityInstruction(), values_,
+ &output);
delegate_->WriteStreamData(output);
}
diff --git a/quic/core/qpack/qpack_encoder_stream_sender.h b/quic/core/qpack/qpack_encoder_stream_sender.h
index e395b6d..bf3a79f 100644
--- a/quic/core/qpack/qpack_encoder_stream_sender.h
+++ b/quic/core/qpack/qpack_encoder_stream_sender.h
@@ -40,6 +40,7 @@
private:
QpackStreamSenderDelegate* const delegate_;
QpackInstructionEncoder instruction_encoder_;
+ QpackInstructionEncoder::Values values_;
};
} // namespace quic
diff --git a/quic/core/qpack/qpack_instruction_encoder.cc b/quic/core/qpack/qpack_instruction_encoder.cc
index 277079b..5845a74 100644
--- a/quic/core/qpack/qpack_instruction_encoder.cc
+++ b/quic/core/qpack/qpack_instruction_encoder.cc
@@ -14,14 +14,10 @@
namespace quic {
QpackInstructionEncoder::QpackInstructionEncoder()
- : s_bit_(false),
- varint_(0),
- varint2_(0),
- byte_(0),
- state_(State::kOpcode),
- instruction_(nullptr) {}
+ : byte_(0), state_(State::kOpcode), instruction_(nullptr) {}
void QpackInstructionEncoder::Encode(const QpackInstruction* instruction,
+ const Values& values,
std::string* output) {
DCHECK(instruction);
@@ -41,19 +37,21 @@
DoStartField();
break;
case State::kSbit:
- DoStaticBit();
+ DoSBit(values.s_bit);
break;
case State::kVarintEncode:
- DoVarintEncode(output);
+ DoVarintEncode(values.varint, values.varint2, output);
break;
case State::kStartString:
- DoStartString();
+ DoStartString(values.name, values.value);
break;
case State::kWriteString:
DoWriteString(output);
break;
}
} while (field_ != instruction_->fields.end());
+
+ DCHECK(state_ == State::kStartField);
}
void QpackInstructionEncoder::DoOpcode() {
@@ -80,10 +78,10 @@
}
}
-void QpackInstructionEncoder::DoStaticBit() {
+void QpackInstructionEncoder::DoSBit(bool s_bit) {
DCHECK(field_->type == QpackInstructionFieldType::kSbit);
- if (s_bit_) {
+ if (s_bit) {
DCHECK_EQ(0, byte_ & field_->param);
byte_ |= field_->param;
@@ -93,7 +91,9 @@
state_ = State::kStartField;
}
-void QpackInstructionEncoder::DoVarintEncode(std::string* output) {
+void QpackInstructionEncoder::DoVarintEncode(uint64_t varint,
+ uint64_t varint2,
+ std::string* output) {
DCHECK(field_->type == QpackInstructionFieldType::kVarint ||
field_->type == QpackInstructionFieldType::kVarint2 ||
field_->type == QpackInstructionFieldType::kName ||
@@ -101,10 +101,10 @@
uint64_t integer_to_encode;
switch (field_->type) {
case QpackInstructionFieldType::kVarint:
- integer_to_encode = varint_;
+ integer_to_encode = varint;
break;
case QpackInstructionFieldType::kVarint2:
- integer_to_encode = varint2_;
+ integer_to_encode = varint2;
break;
default:
integer_to_encode = string_to_write_.size();
@@ -125,12 +125,13 @@
state_ = State::kWriteString;
}
-void QpackInstructionEncoder::DoStartString() {
+void QpackInstructionEncoder::DoStartString(QuicStringPiece name,
+ QuicStringPiece value) {
DCHECK(field_->type == QpackInstructionFieldType::kName ||
field_->type == QpackInstructionFieldType::kValue);
string_to_write_ =
- (field_->type == QpackInstructionFieldType::kName) ? name_ : value_;
+ (field_->type == QpackInstructionFieldType::kName) ? name : value;
http2::HuffmanEncode(string_to_write_, &huffman_encoded_string_);
if (huffman_encoded_string_.size() < string_to_write_.size()) {
diff --git a/quic/core/qpack/qpack_instruction_encoder.h b/quic/core/qpack/qpack_instruction_encoder.h
index c484104..1ca52e6 100644
--- a/quic/core/qpack/qpack_instruction_encoder.h
+++ b/quic/core/qpack/qpack_instruction_encoder.h
@@ -19,20 +19,24 @@
// fields that follow each instruction.
class QUIC_EXPORT_PRIVATE QpackInstructionEncoder {
public:
+ // Storage for field values to be encoded.
+ // The encoded instruction determines which values are actually used.
+ struct Values {
+ bool s_bit;
+ uint64_t varint;
+ uint64_t varint2;
+ QuicStringPiece name;
+ QuicStringPiece value;
+ };
+
QpackInstructionEncoder();
QpackInstructionEncoder(const QpackInstructionEncoder&) = delete;
QpackInstructionEncoder& operator=(const QpackInstructionEncoder&) = delete;
- // Setters for values to be encoded.
- // |name| and |value| must remain valid until the instruction is encoded.
- void set_s_bit(bool s_bit) { s_bit_ = s_bit; }
- void set_varint(uint64_t varint) { varint_ = varint; }
- void set_varint2(uint64_t varint2) { varint2_ = varint2; }
- void set_name(QuicStringPiece name) { name_ = name; }
- void set_value(QuicStringPiece value) { value_ = value; }
-
// Append encoded instruction to |output|.
- void Encode(const QpackInstruction* instruction, std::string* output);
+ void Encode(const QpackInstruction* instruction,
+ const Values& values,
+ std::string* output);
private:
enum class State {
@@ -57,19 +61,11 @@
// Some only change internal state.
void DoOpcode();
void DoStartField();
- void DoStaticBit();
- void DoVarintEncode(std::string* output);
- void DoStartString();
+ void DoSBit(bool s_bit);
+ void DoVarintEncode(uint64_t varint, uint64_t varint2, std::string* output);
+ void DoStartString(QuicStringPiece name, QuicStringPiece value);
void DoWriteString(std::string* output);
- // Storage for field values to be encoded.
- bool s_bit_;
- uint64_t varint_;
- uint64_t varint2_;
- // The caller must keep the string that |name_| and |value_| point to
- // valid until they are encoded.
- QuicStringPiece name_;
- QuicStringPiece value_;
// Storage for the Huffman encoded string literal to be written if Huffman
// encoding is used.
diff --git a/quic/core/qpack/qpack_instruction_encoder_test.cc b/quic/core/qpack/qpack_instruction_encoder_test.cc
index d921fa6..0d172cd 100644
--- a/quic/core/qpack/qpack_instruction_encoder_test.cc
+++ b/quic/core/qpack/qpack_instruction_encoder_test.cc
@@ -20,8 +20,9 @@
~QpackInstructionEncoderTest() override = default;
// Append encoded |instruction| to |output_|.
- void EncodeInstruction(const QpackInstruction* instruction) {
- encoder_.Encode(instruction, &output_);
+ void EncodeInstruction(const QpackInstruction* instruction,
+ const QpackInstructionEncoder::Values& values) {
+ encoder_.Encode(instruction, values, &output_);
}
// Compare substring appended to |output_| since last EncodedSegmentMatches()
@@ -33,9 +34,8 @@
return recently_encoded == expected;
}
- QpackInstructionEncoder encoder_;
-
private:
+ QpackInstructionEncoder encoder_;
std::string output_;
std::string::size_type verified_position_;
};
@@ -44,12 +44,13 @@
const QpackInstruction instruction{QpackInstructionOpcode{0x00, 0x80},
{{QpackInstructionFieldType::kVarint, 7}}};
- encoder_.set_varint(5);
- EncodeInstruction(&instruction);
+ QpackInstructionEncoder::Values values;
+ values.varint = 5;
+ EncodeInstruction(&instruction, values);
EXPECT_TRUE(EncodedSegmentMatches("05"));
- encoder_.set_varint(127);
- EncodeInstruction(&instruction);
+ values.varint = 127;
+ EncodeInstruction(&instruction, values);
EXPECT_TRUE(EncodedSegmentMatches("7f00"));
}
@@ -60,16 +61,17 @@
{QpackInstructionFieldType::kVarint, 5},
{QpackInstructionFieldType::kVarint2, 8}}};
- encoder_.set_s_bit(true);
- encoder_.set_varint(5);
- encoder_.set_varint2(200);
- EncodeInstruction(&instruction);
+ QpackInstructionEncoder::Values values;
+ values.s_bit = true;
+ values.varint = 5;
+ values.varint2 = 200;
+ EncodeInstruction(&instruction, values);
EXPECT_TRUE(EncodedSegmentMatches("a5c8"));
- encoder_.set_s_bit(false);
- encoder_.set_varint(31);
- encoder_.set_varint2(356);
- EncodeInstruction(&instruction);
+ values.s_bit = false;
+ values.varint = 31;
+ values.varint2 = 356;
+ EncodeInstruction(&instruction, values);
EXPECT_TRUE(EncodedSegmentMatches("9f00ff65"));
}
@@ -79,16 +81,17 @@
{QpackInstructionFieldType::kVarint, 5},
{QpackInstructionFieldType::kValue, 7}}};
- encoder_.set_s_bit(true);
- encoder_.set_varint(100);
- encoder_.set_value("foo");
- EncodeInstruction(&instruction);
+ QpackInstructionEncoder::Values values;
+ values.s_bit = true;
+ values.varint = 100;
+ values.value = "foo";
+ EncodeInstruction(&instruction, values);
EXPECT_TRUE(EncodedSegmentMatches("ff458294e7"));
- encoder_.set_s_bit(false);
- encoder_.set_varint(3);
- encoder_.set_value("bar");
- EncodeInstruction(&instruction);
+ values.s_bit = false;
+ values.varint = 3;
+ values.value = "bar";
+ EncodeInstruction(&instruction, values);
EXPECT_TRUE(EncodedSegmentMatches("c303626172"));
}
@@ -96,16 +99,17 @@
const QpackInstruction instruction{QpackInstructionOpcode{0xe0, 0xe0},
{{QpackInstructionFieldType::kName, 4}}};
- encoder_.set_name("");
- EncodeInstruction(&instruction);
+ QpackInstructionEncoder::Values values;
+ values.name = "";
+ EncodeInstruction(&instruction, values);
EXPECT_TRUE(EncodedSegmentMatches("e0"));
- encoder_.set_name("foo");
- EncodeInstruction(&instruction);
+ values.name = "foo";
+ EncodeInstruction(&instruction, values);
EXPECT_TRUE(EncodedSegmentMatches("f294e7"));
- encoder_.set_name("bar");
- EncodeInstruction(&instruction);
+ values.name = "bar";
+ EncodeInstruction(&instruction, values);
EXPECT_TRUE(EncodedSegmentMatches("e3626172"));
}
@@ -113,16 +117,17 @@
const QpackInstruction instruction{QpackInstructionOpcode{0xf0, 0xf0},
{{QpackInstructionFieldType::kValue, 3}}};
- encoder_.set_value("");
- EncodeInstruction(&instruction);
+ QpackInstructionEncoder::Values values;
+ values.value = "";
+ EncodeInstruction(&instruction, values);
EXPECT_TRUE(EncodedSegmentMatches("f0"));
- encoder_.set_value("foo");
- EncodeInstruction(&instruction);
+ values.value = "foo";
+ EncodeInstruction(&instruction, values);
EXPECT_TRUE(EncodedSegmentMatches("fa94e7"));
- encoder_.set_value("bar");
- EncodeInstruction(&instruction);
+ values.value = "bar";
+ EncodeInstruction(&instruction, values);
EXPECT_TRUE(EncodedSegmentMatches("f3626172"));
}
@@ -132,16 +137,17 @@
{QpackInstructionFieldType::kName, 2},
{QpackInstructionFieldType::kValue, 7}}};
- encoder_.set_s_bit(false);
- encoder_.set_name("");
- encoder_.set_value("");
- EncodeInstruction(&instruction);
+ QpackInstructionEncoder::Values values;
+ values.s_bit = false;
+ values.name = "";
+ values.value = "";
+ EncodeInstruction(&instruction, values);
EXPECT_TRUE(EncodedSegmentMatches("f000"));
- encoder_.set_s_bit(true);
- encoder_.set_name("foo");
- encoder_.set_value("bar");
- EncodeInstruction(&instruction);
+ values.s_bit = true;
+ values.name = "foo";
+ values.value = "bar";
+ EncodeInstruction(&instruction, values);
EXPECT_TRUE(EncodedSegmentMatches("fe94e703626172"));
}
