Allow QpackStreamSenderDelegate to be set dynamically.
In IETF QUIC, the session is not allowed to open any stream until stream limit is negotiated. This means that when QPACK encoder/decoder are initialized, QPACK streams might not exist yet. Plus the peer may not support QPACK dynamic table, in which case we might choose not to create QPACK streams.
This change allows send stream to be nullptr upon initialization and be set later when QPACK streams are initialized.
gfe-relnote: v99 only, not protected.
PiperOrigin-RevId: 262167938
Change-Id: Ibd9f96dec3abcbde98e43e1199f369a1c951922d
diff --git a/quic/core/http/end_to_end_test.cc b/quic/core/http/end_to_end_test.cc
index 750f371..bbd1892 100644
--- a/quic/core/http/end_to_end_test.cc
+++ b/quic/core/http/end_to_end_test.cc
@@ -2450,8 +2450,9 @@
// Determine size of compressed headers.
NoopDecoderStreamErrorDelegate decoder_stream_error_delegate;
NoopQpackStreamSenderDelegate encoder_stream_sender_delegate;
- QpackEncoder qpack_encoder(&decoder_stream_error_delegate,
- &encoder_stream_sender_delegate);
+ QpackEncoder qpack_encoder(&decoder_stream_error_delegate);
+ qpack_encoder.set_qpack_stream_sender_delegate(
+ &encoder_stream_sender_delegate);
std::string encoded_headers =
qpack_encoder.EncodeHeaderList(/* stream_id = */ 0, &headers);
header_size = encoded_headers.size();
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc
index ad13dc1..87208ee 100644
--- a/quic/core/http/quic_spdy_session.cc
+++ b/quic/core/http/quic_spdy_session.cc
@@ -362,12 +362,14 @@
RegisterStaticStream(std::move(headers_stream),
/*stream_already_counted = */ false);
} else {
- qpack_encoder_ =
- QuicMakeUnique<QpackEncoder>(this, &encoder_stream_sender_delegate_);
+ qpack_encoder_ = QuicMakeUnique<QpackEncoder>(this);
+ qpack_encoder_->set_qpack_stream_sender_delegate(
+ &encoder_stream_sender_delegate_);
qpack_decoder_ =
QuicMakeUnique<QpackDecoder>(kDefaultQpackMaxDynamicTableCapacity,
- /* maximum_blocked_streams = */ 0, this,
- &decoder_stream_sender_delegate_);
+ /* maximum_blocked_streams = */ 0, this);
+ qpack_decoder_->set_qpack_stream_sender_delegate(
+ &decoder_stream_sender_delegate_);
// TODO(b/112770235): Set sensible limit on maximum number of blocked
// streams.
// TODO(b/112770235): Send SETTINGS_QPACK_MAX_TABLE_CAPACITY with value
diff --git a/quic/core/http/quic_spdy_stream_test.cc b/quic/core/http/quic_spdy_stream_test.cc
index 894573f..ef9d6ff 100644
--- a/quic/core/http/quic_spdy_stream_test.cc
+++ b/quic/core/http/quic_spdy_stream_test.cc
@@ -179,8 +179,9 @@
// Return QPACK-encoded header block without using the dynamic table.
std::string EncodeQpackHeaders(const SpdyHeaderBlock& header) {
NoopQpackStreamSenderDelegate encoder_stream_sender_delegate;
- auto qpack_encoder = QuicMakeUnique<QpackEncoder>(
- session_.get(), &encoder_stream_sender_delegate);
+ auto qpack_encoder = QuicMakeUnique<QpackEncoder>(session_.get());
+ qpack_encoder->set_qpack_stream_sender_delegate(
+ &encoder_stream_sender_delegate);
// QpackEncoder does not use the dynamic table by default,
// therefore the value of |stream_id| does not matter.
return qpack_encoder->EncodeHeaderList(/* stream_id = */ 0, &header);
diff --git a/quic/core/qpack/fuzzer/qpack_encoder_stream_sender_fuzzer.cc b/quic/core/qpack/fuzzer/qpack_encoder_stream_sender_fuzzer.cc
index 78c4c20..d5204b3 100644
--- a/quic/core/qpack/fuzzer/qpack_encoder_stream_sender_fuzzer.cc
+++ b/quic/core/qpack/fuzzer/qpack_encoder_stream_sender_fuzzer.cc
@@ -22,7 +22,8 @@
// enabled for cc_fuzz_target target types.
extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size) {
NoopQpackStreamSenderDelegate delegate;
- QpackEncoderStreamSender sender(&delegate);
+ QpackEncoderStreamSender sender;
+ sender.set_qpack_stream_sender_delegate(&delegate);
QuicFuzzedDataProvider provider(data, size);
// Limit string literal length to 2 kB for efficiency.
diff --git a/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc b/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
index fceaac5..a9e9a0e 100644
--- a/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
+++ b/quic/core/qpack/fuzzer/qpack_round_trip_fuzzer.cc
@@ -128,8 +128,8 @@
// Encode header list.
NoopDecoderStreamErrorDelegate decoder_stream_error_delegate;
NoopQpackStreamSenderDelegate encoder_stream_sender_delegate;
- QpackEncoder encoder(&decoder_stream_error_delegate,
- &encoder_stream_sender_delegate);
+ QpackEncoder encoder(&decoder_stream_error_delegate);
+ encoder.set_qpack_stream_sender_delegate(&encoder_stream_sender_delegate);
std::string encoded_header_block =
encoder.EncodeHeaderList(/* stream_id = */ 1, &header_list);
diff --git a/quic/core/qpack/offline/qpack_offline_decoder.cc b/quic/core/qpack/offline/qpack_offline_decoder.cc
index 267c679..b6ad935 100644
--- a/quic/core/qpack/offline/qpack_offline_decoder.cc
+++ b/quic/core/qpack/offline/qpack_offline_decoder.cc
@@ -92,8 +92,9 @@
<< "\" as an integer.";
return false;
}
- qpack_decoder_ = QuicMakeUnique<QpackDecoder>(
- maximum_dynamic_table_capacity, max_blocked_streams_, this,
+ qpack_decoder_ = QuicMakeUnique<QpackDecoder>(maximum_dynamic_table_capacity,
+ max_blocked_streams_, this);
+ qpack_decoder_->set_qpack_stream_sender_delegate(
&decoder_stream_sender_delegate_);
return true;
diff --git a/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc b/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
index ed5a4c0..3f7ae25 100644
--- a/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
+++ b/quic/core/qpack/qpack_decoded_headers_accumulator_test.cc
@@ -51,12 +51,14 @@
QpackDecodedHeadersAccumulatorTest()
: qpack_decoder_(kMaxDynamicTableCapacity,
kMaximumBlockedStreams,
- &encoder_stream_error_delegate_,
- &decoder_stream_sender_delegate_),
+ &encoder_stream_error_delegate_),
accumulator_(kTestStreamId,
&qpack_decoder_,
&visitor_,
- kMaxHeaderListSize) {}
+ kMaxHeaderListSize) {
+ qpack_decoder_.set_qpack_stream_sender_delegate(
+ &decoder_stream_sender_delegate_);
+ }
NoopEncoderStreamErrorDelegate encoder_stream_error_delegate_;
StrictMock<MockQpackStreamSenderDelegate> decoder_stream_sender_delegate_;
diff --git a/quic/core/qpack/qpack_decoder.cc b/quic/core/qpack/qpack_decoder.cc
index ae4d68b..739bc1e 100644
--- a/quic/core/qpack/qpack_decoder.cc
+++ b/quic/core/qpack/qpack_decoder.cc
@@ -14,13 +14,10 @@
QpackDecoder::QpackDecoder(
uint64_t maximum_dynamic_table_capacity,
uint64_t /* maximum_blocked_streams */,
- EncoderStreamErrorDelegate* encoder_stream_error_delegate,
- QpackStreamSenderDelegate* decoder_stream_sender_delegate)
+ EncoderStreamErrorDelegate* encoder_stream_error_delegate)
: encoder_stream_error_delegate_(encoder_stream_error_delegate),
- encoder_stream_receiver_(this),
- decoder_stream_sender_(decoder_stream_sender_delegate) {
+ encoder_stream_receiver_(this) {
DCHECK(encoder_stream_error_delegate_);
- DCHECK(decoder_stream_sender_delegate);
header_table_.SetMaximumDynamicTableCapacity(maximum_dynamic_table_capacity);
}
@@ -28,6 +25,8 @@
QpackDecoder::~QpackDecoder() {}
void QpackDecoder::OnStreamReset(QuicStreamId stream_id) {
+ // TODO(bnc): SendStreamCancellation should not be called if maximum dynamic
+ // table capacity is zero.
decoder_stream_sender_.SendStreamCancellation(stream_id);
}
diff --git a/quic/core/qpack/qpack_decoder.h b/quic/core/qpack/qpack_decoder.h
index 1cc07e4..b3eea59 100644
--- a/quic/core/qpack/qpack_decoder.h
+++ b/quic/core/qpack/qpack_decoder.h
@@ -36,8 +36,7 @@
QpackDecoder(uint64_t maximum_dynamic_table_capacity,
uint64_t maximum_blocked_streams,
- EncoderStreamErrorDelegate* encoder_stream_error_delegate,
- QpackStreamSenderDelegate* decoder_stream_sender_delegate);
+ EncoderStreamErrorDelegate* encoder_stream_error_delegate);
~QpackDecoder() override;
// Signal to the peer's encoder that a stream is reset. This lets the peer's
@@ -79,6 +78,11 @@
void OnSetDynamicTableCapacity(uint64_t capacity) override;
void OnErrorDetected(QuicStringPiece error_message) override;
+ // delegate must be set if dynamic table capacity is not zero.
+ void set_qpack_stream_sender_delegate(QpackStreamSenderDelegate* delegate) {
+ decoder_stream_sender_.set_qpack_stream_sender_delegate(delegate);
+ }
+
private:
// The encoder stream uses relative index (but different from the kind of
// relative index used on a request stream). This method converts relative
diff --git a/quic/core/qpack/qpack_decoder_stream_sender.cc b/quic/core/qpack/qpack_decoder_stream_sender.cc
index 63f1721..7f3f346 100644
--- a/quic/core/qpack/qpack_decoder_stream_sender.cc
+++ b/quic/core/qpack/qpack_decoder_stream_sender.cc
@@ -13,11 +13,7 @@
namespace quic {
-QpackDecoderStreamSender::QpackDecoderStreamSender(
- QpackStreamSenderDelegate* delegate)
- : delegate_(delegate) {
- DCHECK(delegate_);
-}
+QpackDecoderStreamSender::QpackDecoderStreamSender() : delegate_(nullptr) {}
void QpackDecoderStreamSender::SendInsertCountIncrement(uint64_t increment) {
values_.varint = increment;
diff --git a/quic/core/qpack/qpack_decoder_stream_sender.h b/quic/core/qpack/qpack_decoder_stream_sender.h
index 5f17744..6bf466f 100644
--- a/quic/core/qpack/qpack_decoder_stream_sender.h
+++ b/quic/core/qpack/qpack_decoder_stream_sender.h
@@ -19,8 +19,7 @@
// stream.
class QUIC_EXPORT_PRIVATE QpackDecoderStreamSender {
public:
- explicit QpackDecoderStreamSender(QpackStreamSenderDelegate* delegate);
- QpackDecoderStreamSender() = delete;
+ QpackDecoderStreamSender();
QpackDecoderStreamSender(const QpackDecoderStreamSender&) = delete;
QpackDecoderStreamSender& operator=(const QpackDecoderStreamSender&) = delete;
@@ -34,8 +33,13 @@
// 5.3.3 Stream Cancellation
void SendStreamCancellation(QuicStreamId stream_id);
+ // delegate must be set if dynamic table capacity is not zero.
+ void set_qpack_stream_sender_delegate(QpackStreamSenderDelegate* delegate) {
+ delegate_ = delegate;
+ }
+
private:
- QpackStreamSenderDelegate* const delegate_;
+ QpackStreamSenderDelegate* delegate_;
QpackInstructionEncoder instruction_encoder_;
QpackInstructionEncoder::Values values_;
};
diff --git a/quic/core/qpack/qpack_decoder_stream_sender_test.cc b/quic/core/qpack/qpack_decoder_stream_sender_test.cc
index 49483f7..ccb42a3 100644
--- a/quic/core/qpack/qpack_decoder_stream_sender_test.cc
+++ b/quic/core/qpack/qpack_decoder_stream_sender_test.cc
@@ -17,7 +17,9 @@
class QpackDecoderStreamSenderTest : public QuicTest {
protected:
- QpackDecoderStreamSenderTest() : stream_(&delegate_) {}
+ QpackDecoderStreamSenderTest() {
+ stream_.set_qpack_stream_sender_delegate(&delegate_);
+ }
~QpackDecoderStreamSenderTest() override = default;
StrictMock<MockQpackStreamSenderDelegate> delegate_;
diff --git a/quic/core/qpack/qpack_decoder_test.cc b/quic/core/qpack/qpack_decoder_test.cc
index 4b57456..9fe3d8c 100644
--- a/quic/core/qpack/qpack_decoder_test.cc
+++ b/quic/core/qpack/qpack_decoder_test.cc
@@ -35,9 +35,11 @@
: qpack_decoder_(
/* maximum_dynamic_table_capacity = */ 1024,
kMaximumBlockedStreams,
- &encoder_stream_error_delegate_,
- &decoder_stream_sender_delegate_),
- fragment_mode_(GetParam()) {}
+ &encoder_stream_error_delegate_),
+ fragment_mode_(GetParam()) {
+ qpack_decoder_.set_qpack_stream_sender_delegate(
+ &decoder_stream_sender_delegate_);
+ }
~QpackDecoderTest() override = default;
diff --git a/quic/core/qpack/qpack_decoder_test_utils.cc b/quic/core/qpack/qpack_decoder_test_utils.cc
index ce990b5..eaf6664 100644
--- a/quic/core/qpack/qpack_decoder_test_utils.cc
+++ b/quic/core/qpack/qpack_decoder_test_utils.cc
@@ -72,8 +72,8 @@
const FragmentSizeGenerator& fragment_size_generator,
QuicStringPiece data) {
QpackDecoder decoder(maximum_dynamic_table_capacity, maximum_blocked_streams,
- encoder_stream_error_delegate,
- decoder_stream_sender_delegate);
+ encoder_stream_error_delegate);
+ decoder.set_qpack_stream_sender_delegate(decoder_stream_sender_delegate);
auto progressive_decoder =
decoder.CreateProgressiveDecoder(/* stream_id = */ 1, handler);
while (!data.empty()) {
diff --git a/quic/core/qpack/qpack_encoder.cc b/quic/core/qpack/qpack_encoder.cc
index 1daa572..cce36ee 100644
--- a/quic/core/qpack/qpack_encoder.cc
+++ b/quic/core/qpack/qpack_encoder.cc
@@ -16,14 +16,11 @@
namespace quic {
QpackEncoder::QpackEncoder(
- DecoderStreamErrorDelegate* decoder_stream_error_delegate,
- QpackStreamSenderDelegate* encoder_stream_sender_delegate)
+ DecoderStreamErrorDelegate* decoder_stream_error_delegate)
: decoder_stream_error_delegate_(decoder_stream_error_delegate),
decoder_stream_receiver_(this),
- encoder_stream_sender_(encoder_stream_sender_delegate),
maximum_blocked_streams_(0) {
DCHECK(decoder_stream_error_delegate_);
- DCHECK(encoder_stream_sender_delegate);
}
QpackEncoder::~QpackEncoder() {}
diff --git a/quic/core/qpack/qpack_encoder.h b/quic/core/qpack/qpack_encoder.h
index e0bd8d4..d64af56 100644
--- a/quic/core/qpack/qpack_encoder.h
+++ b/quic/core/qpack/qpack_encoder.h
@@ -44,8 +44,7 @@
virtual void OnDecoderStreamError(QuicStringPiece error_message) = 0;
};
- QpackEncoder(DecoderStreamErrorDelegate* decoder_stream_error_delegate,
- QpackStreamSenderDelegate* encoder_stream_sender_delegate);
+ QpackEncoder(DecoderStreamErrorDelegate* decoder_stream_error_delegate);
~QpackEncoder() override;
// Encode a header list.
@@ -69,6 +68,11 @@
void OnStreamCancellation(QuicStreamId stream_id) override;
void OnErrorDetected(QuicStringPiece error_message) override;
+ // delegate must be set if dynamic table capacity is not zero.
+ void set_qpack_stream_sender_delegate(QpackStreamSenderDelegate* delegate) {
+ encoder_stream_sender_.set_qpack_stream_sender_delegate(delegate);
+ }
+
private:
friend class test::QpackEncoderPeer;
diff --git a/quic/core/qpack/qpack_encoder_stream_sender.cc b/quic/core/qpack/qpack_encoder_stream_sender.cc
index dce183a..e6209fd 100644
--- a/quic/core/qpack/qpack_encoder_stream_sender.cc
+++ b/quic/core/qpack/qpack_encoder_stream_sender.cc
@@ -13,11 +13,7 @@
namespace quic {
-QpackEncoderStreamSender::QpackEncoderStreamSender(
- QpackStreamSenderDelegate* delegate)
- : delegate_(delegate) {
- DCHECK(delegate_);
-}
+QpackEncoderStreamSender::QpackEncoderStreamSender() : delegate_(nullptr) {}
void QpackEncoderStreamSender::SendInsertWithNameReference(
bool is_static,
diff --git a/quic/core/qpack/qpack_encoder_stream_sender.h b/quic/core/qpack/qpack_encoder_stream_sender.h
index bf3a79f..3c410d5 100644
--- a/quic/core/qpack/qpack_encoder_stream_sender.h
+++ b/quic/core/qpack/qpack_encoder_stream_sender.h
@@ -17,8 +17,7 @@
// This class serializes instructions for transmission on the encoder stream.
class QUIC_EXPORT_PRIVATE QpackEncoderStreamSender {
public:
- explicit QpackEncoderStreamSender(QpackStreamSenderDelegate* delegate);
- QpackEncoderStreamSender() = delete;
+ QpackEncoderStreamSender();
QpackEncoderStreamSender(const QpackEncoderStreamSender&) = delete;
QpackEncoderStreamSender& operator=(const QpackEncoderStreamSender&) = delete;
@@ -37,8 +36,13 @@
// 5.2.4. Set Dynamic Table Capacity
void SendSetDynamicTableCapacity(uint64_t capacity);
+ // delegate must be set if dynamic table capacity is not zero.
+ void set_qpack_stream_sender_delegate(QpackStreamSenderDelegate* delegate) {
+ delegate_ = delegate;
+ }
+
private:
- QpackStreamSenderDelegate* const delegate_;
+ QpackStreamSenderDelegate* delegate_;
QpackInstructionEncoder instruction_encoder_;
QpackInstructionEncoder::Values values_;
};
diff --git a/quic/core/qpack/qpack_encoder_stream_sender_test.cc b/quic/core/qpack/qpack_encoder_stream_sender_test.cc
index 01f1cbf..4a15d1f 100644
--- a/quic/core/qpack/qpack_encoder_stream_sender_test.cc
+++ b/quic/core/qpack/qpack_encoder_stream_sender_test.cc
@@ -17,7 +17,9 @@
class QpackEncoderStreamSenderTest : public QuicTest {
protected:
- QpackEncoderStreamSenderTest() : stream_(&delegate_) {}
+ QpackEncoderStreamSenderTest() {
+ stream_.set_qpack_stream_sender_delegate(&delegate_);
+ }
~QpackEncoderStreamSenderTest() override = default;
StrictMock<MockQpackStreamSenderDelegate> delegate_;
diff --git a/quic/core/qpack/qpack_encoder_test.cc b/quic/core/qpack/qpack_encoder_test.cc
index 544b670..90caf44 100644
--- a/quic/core/qpack/qpack_encoder_test.cc
+++ b/quic/core/qpack/qpack_encoder_test.cc
@@ -25,8 +25,8 @@
~QpackEncoderTest() override = default;
std::string Encode(const spdy::SpdyHeaderBlock* header_list) {
- QpackEncoder encoder(&decoder_stream_error_delegate_,
- &encoder_stream_sender_delegate_);
+ QpackEncoder encoder(&decoder_stream_error_delegate_);
+ encoder.set_qpack_stream_sender_delegate(&encoder_stream_sender_delegate_);
return encoder.EncodeHeaderList(/* stream_id = */ 1, header_list);
}
@@ -129,8 +129,8 @@
EXPECT_CALL(decoder_stream_error_delegate_,
OnDecoderStreamError(Eq("Encoded integer too large.")));
- QpackEncoder encoder(&decoder_stream_error_delegate_,
- &encoder_stream_sender_delegate_);
+ QpackEncoder encoder(&decoder_stream_error_delegate_);
+ encoder.set_qpack_stream_sender_delegate(&encoder_stream_sender_delegate_);
encoder.DecodeDecoderStreamData(
QuicTextUtils::HexDecode("ffffffffffffffffffffff"));
}
diff --git a/quic/core/qpack/qpack_round_trip_test.cc b/quic/core/qpack/qpack_round_trip_test.cc
index 6ad6003..48249b0 100644
--- a/quic/core/qpack/qpack_round_trip_test.cc
+++ b/quic/core/qpack/qpack_round_trip_test.cc
@@ -29,8 +29,8 @@
const spdy::SpdyHeaderBlock& header_list) {
NoopDecoderStreamErrorDelegate decoder_stream_error_delegate;
NoopQpackStreamSenderDelegate encoder_stream_sender_delegate;
- QpackEncoder encoder(&decoder_stream_error_delegate,
- &encoder_stream_sender_delegate);
+ QpackEncoder encoder(&decoder_stream_error_delegate);
+ encoder.set_qpack_stream_sender_delegate(&encoder_stream_sender_delegate);
std::string encoded_header_block =
encoder.EncodeHeaderList(/* stream_id = */ 1, &header_list);
