Send Insert Count Increment instruction after decoding a header block if possible.
See also https://quicwg.org/base-drafts/draft-ietf-quic-qpack.html#new-table-entries
for justification of not delaying sending Insert Count Increment instructions too long.
gfe-relnote: n/a, change to QUIC v99-only code. Protected by existing disabled gfe2_reloadable_flag_quic_enable_version_99.
PiperOrigin-RevId: 273510751
Change-Id: Ic33afd24c41bc92577b9b75d00f8ecdd94058d60
diff --git a/quic/core/qpack/qpack_decoder.cc b/quic/core/qpack/qpack_decoder.cc
index e2e9d28..d4e5d91 100644
--- a/quic/core/qpack/qpack_decoder.cc
+++ b/quic/core/qpack/qpack_decoder.cc
@@ -16,7 +16,8 @@
EncoderStreamErrorDelegate* encoder_stream_error_delegate)
: encoder_stream_error_delegate_(encoder_stream_error_delegate),
encoder_stream_receiver_(this),
- maximum_blocked_streams_(maximum_blocked_streams) {
+ maximum_blocked_streams_(maximum_blocked_streams),
+ known_received_count_(0) {
DCHECK(encoder_stream_error_delegate_);
header_table_.SetMaximumDynamicTableCapacity(maximum_dynamic_table_capacity);
@@ -41,6 +42,27 @@
DCHECK_EQ(1u, result);
}
+void QpackDecoder::OnDecodingCompleted(QuicStreamId stream_id,
+ uint64_t required_insert_count) {
+ if (required_insert_count > 0) {
+ decoder_stream_sender_.SendHeaderAcknowledgement(stream_id);
+
+ if (known_received_count_ < required_insert_count) {
+ known_received_count_ = required_insert_count;
+ }
+ }
+
+ // Send an Insert Count Increment instruction if not all dynamic table entries
+ // have been acknowledged yet. This is necessary for efficient compression in
+ // case the encoder chooses not to reference unacknowledged dynamic table
+ // entries, otherwise inserted entries would never be acknowledged.
+ if (known_received_count_ < header_table_.inserted_entry_count()) {
+ decoder_stream_sender_.SendInsertCountIncrement(
+ header_table_.inserted_entry_count() - known_received_count_);
+ known_received_count_ = header_table_.inserted_entry_count();
+ }
+}
+
void QpackDecoder::OnInsertWithNameReference(bool is_static,
uint64_t name_index,
QuicStringPiece value) {
@@ -128,8 +150,8 @@
std::unique_ptr<QpackProgressiveDecoder> QpackDecoder::CreateProgressiveDecoder(
QuicStreamId stream_id,
QpackProgressiveDecoder::HeadersHandlerInterface* handler) {
- return std::make_unique<QpackProgressiveDecoder>(
- stream_id, this, &header_table_, &decoder_stream_sender_, handler);
+ return std::make_unique<QpackProgressiveDecoder>(stream_id, this, this,
+ &header_table_, handler);
}
} // namespace quic
diff --git a/quic/core/qpack/qpack_decoder.h b/quic/core/qpack/qpack_decoder.h
index 102fbc4..113c6ea 100644
--- a/quic/core/qpack/qpack_decoder.h
+++ b/quic/core/qpack/qpack_decoder.h
@@ -24,7 +24,8 @@
// list to be encoded.
class QUIC_EXPORT_PRIVATE QpackDecoder
: public QpackEncoderStreamReceiver::Delegate,
- public QpackProgressiveDecoder::BlockedStreamLimitEnforcer {
+ public QpackProgressiveDecoder::BlockedStreamLimitEnforcer,
+ public QpackProgressiveDecoder::DecodingCompletedVisitor {
public:
// Interface for receiving notification that an error has occurred on the
// encoder stream. This MUST be treated as a connection error of type
@@ -63,6 +64,10 @@
bool OnStreamBlocked(QuicStreamId stream_id) override;
void OnStreamUnblocked(QuicStreamId stream_id) override;
+ // QpackProgressiveDecoder::DecodingCompletedVisitor implementation.
+ void OnDecodingCompleted(QuicStreamId stream_id,
+ uint64_t required_insert_count) override;
+
// Factory method to create a QpackProgressiveDecoder for decoding a header
// block. |handler| must remain valid until the returned
// QpackProgressiveDecoder instance is destroyed or the decoder calls
@@ -97,6 +102,13 @@
QpackHeaderTable header_table_;
std::set<QuicStreamId> blocked_streams_;
const uint64_t maximum_blocked_streams_;
+
+ // Known Received Count is the number of insertions the encoder has received
+ // acknowledgement for (through Header Acknowledgement and Insert Count
+ // Increment instructions). The encoder must keep track of it in order to be
+ // able to send Insert Count Increment instructions. See
+ // https://quicwg.org/base-drafts/draft-ietf-quic-qpack.html#known-received-count.
+ uint64_t known_received_count_;
};
} // namespace quic
diff --git a/quic/core/qpack/qpack_decoder_test.cc b/quic/core/qpack/qpack_decoder_test.cc
index 7071913..faa0f9d 100644
--- a/quic/core/qpack/qpack_decoder_test.cc
+++ b/quic/core/qpack/qpack_decoder_test.cc
@@ -816,7 +816,6 @@
// At most three non-empty entries fit in the dynamic table.
DecodeEncoderStreamData(QuicTextUtils::HexDecode("3f61"));
- StartDecoding();
DecodeHeaderBlock(QuicTextUtils::HexDecode(
"0700" // Required Insert Count 6 and Delta Base 0.
// Base is 6 + 0 = 6.
@@ -854,6 +853,29 @@
progressive_decoder2->Decode(data);
}
+TEST_P(QpackDecoderTest, InsertCountIncrement) {
+ DecodeEncoderStreamData(QuicTextUtils::HexDecode(
+ "3fe107" // Set dynamic table capacity to 1024.
+ "6294e703626172" // Add literal entry with name "foo" and value "bar".
+ "00")); // Duplicate entry.
+
+ EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar")));
+ EXPECT_CALL(handler_, OnDecodingCompleted());
+ EXPECT_CALL(decoder_stream_sender_delegate_,
+ WriteStreamData(Eq(kHeaderAcknowledgement)));
+
+ // Decoder received two insertions, but Header Acknowledgement only increases
+ // Known Insert Count to one. Decoder should send an Insert Count Increment
+ // instruction with increment of one to update Known Insert Count to two.
+ EXPECT_CALL(decoder_stream_sender_delegate_,
+ WriteStreamData(Eq(QuicTextUtils::HexDecode("01"))));
+
+ DecodeHeaderBlock(QuicTextUtils::HexDecode(
+ "0200" // Required Insert Count 1 and Delta Base 0.
+ // Base is 1 + 0 = 1.
+ "80")); // Dynamic table entry with relative index 0, absolute index 0.
+}
+
} // namespace
} // namespace test
} // namespace quic
diff --git a/quic/core/qpack/qpack_progressive_decoder.cc b/quic/core/qpack/qpack_progressive_decoder.cc
index 34eb2b3..0a43963 100644
--- a/quic/core/qpack/qpack_progressive_decoder.cc
+++ b/quic/core/qpack/qpack_progressive_decoder.cc
@@ -18,8 +18,8 @@
QpackProgressiveDecoder::QpackProgressiveDecoder(
QuicStreamId stream_id,
BlockedStreamLimitEnforcer* enforcer,
+ DecodingCompletedVisitor* visitor,
QpackHeaderTable* header_table,
- QpackDecoderStreamSender* decoder_stream_sender,
HeadersHandlerInterface* handler)
: stream_id_(stream_id),
prefix_decoder_(
@@ -27,8 +27,8 @@
this)),
instruction_decoder_(QpackRequestStreamLanguage(), this),
enforcer_(enforcer),
+ visitor_(visitor),
header_table_(header_table),
- decoder_stream_sender_(decoder_stream_sender),
handler_(handler),
required_insert_count_(0),
base_(0),
@@ -336,10 +336,7 @@
return;
}
- if (required_insert_count_ > 0) {
- decoder_stream_sender_->SendHeaderAcknowledgement(stream_id_);
- }
-
+ visitor_->OnDecodingCompleted(stream_id_, required_insert_count_);
handler_->OnDecodingCompleted();
}
diff --git a/quic/core/qpack/qpack_progressive_decoder.h b/quic/core/qpack/qpack_progressive_decoder.h
index 91b1e8c..2f306c8 100644
--- a/quic/core/qpack/qpack_progressive_decoder.h
+++ b/quic/core/qpack/qpack_progressive_decoder.h
@@ -9,7 +9,6 @@
#include <memory>
#include <string>
-#include "net/third_party/quiche/src/quic/core/qpack/qpack_decoder_stream_sender.h"
#include "net/third_party/quiche/src/quic/core/qpack/qpack_encoder_stream_receiver.h"
#include "net/third_party/quiche/src/quic/core/qpack/qpack_header_table.h"
#include "net/third_party/quiche/src/quic/core/qpack/qpack_instruction_decoder.h"
@@ -66,11 +65,23 @@
virtual void OnStreamUnblocked(QuicStreamId stream_id) = 0;
};
+ // Visitor to be notified when decoding is completed.
+ class QUIC_EXPORT_PRIVATE DecodingCompletedVisitor {
+ public:
+ virtual ~DecodingCompletedVisitor() = default;
+
+ // Called when decoding is completed, with Required Insert Count of the
+ // decoded header block. Required Insert Count is defined at
+ // https://quicwg.org/base-drafts/draft-ietf-quic-qpack.html#blocked-streams.
+ virtual void OnDecodingCompleted(QuicStreamId stream_id,
+ uint64_t required_insert_count) = 0;
+ };
+
QpackProgressiveDecoder() = delete;
QpackProgressiveDecoder(QuicStreamId stream_id,
BlockedStreamLimitEnforcer* enforcer,
+ DecodingCompletedVisitor* visitor,
QpackHeaderTable* header_table,
- QpackDecoderStreamSender* decoder_stream_sender,
HeadersHandlerInterface* handler);
QpackProgressiveDecoder(const QpackProgressiveDecoder&) = delete;
QpackProgressiveDecoder& operator=(const QpackProgressiveDecoder&) = delete;
@@ -117,8 +128,8 @@
QpackInstructionDecoder instruction_decoder_;
BlockedStreamLimitEnforcer* const enforcer_;
+ DecodingCompletedVisitor* const visitor_;
QpackHeaderTable* const header_table_;
- QpackDecoderStreamSender* const decoder_stream_sender_;
HeadersHandlerInterface* const handler_;
// Required Insert Count and Base are decoded from the Header Data Prefix.
