Blocked decoding part 2: QpackProgressiveDecoder.

gfe-relnote: n/a, QUIC v99-only change.
PiperOrigin-RevId: 257158970
Change-Id: If3aad9939945daf170042038978ab7739078a8bd
diff --git a/quic/core/qpack/qpack_decoder_test.cc b/quic/core/qpack/qpack_decoder_test.cc
index 185aab9..af3a6a2 100644
--- a/quic/core/qpack/qpack_decoder_test.cc
+++ b/quic/core/qpack/qpack_decoder_test.cc
@@ -14,6 +14,7 @@
 #include "net/third_party/quiche/src/spdy/core/spdy_header_block.h"
 
 using ::testing::Eq;
+using ::testing::Mock;
 using ::testing::Sequence;
 using ::testing::StrictMock;
 using ::testing::Values;
@@ -40,17 +41,36 @@
     qpack_decoder_.DecodeEncoderStreamData(data);
   }
 
-  void DecodeHeaderBlock(QuicStringPiece data) {
+  // Set up |progressive_decoder_|.
+  void StartDecoding() {
+    progressive_decoder_ =
+        qpack_decoder_.CreateProgressiveDecoder(/* stream_id = */ 1, &handler_);
+  }
+
+  // Pass header block data to QpackProgressiveDecoder::Decode()
+  // in fragments dictated by |fragment_mode_|.
+  void DecodeData(QuicStringPiece data) {
     auto fragment_size_generator =
         FragmentModeToFragmentSizeGenerator(fragment_mode_);
-    auto progressive_decoder =
-        qpack_decoder_.CreateProgressiveDecoder(/* stream_id = */ 1, &handler_);
     while (!data.empty()) {
       size_t fragment_size = std::min(fragment_size_generator(), data.size());
-      progressive_decoder->Decode(data.substr(0, fragment_size));
+      progressive_decoder_->Decode(data.substr(0, fragment_size));
       data = data.substr(fragment_size);
     }
-    progressive_decoder->EndHeaderBlock();
+  }
+
+  // Signal end of header block to QpackProgressiveDecoder.
+  void EndDecoding() {
+    progressive_decoder_->EndHeaderBlock();
+    // |progressive_decoder_| is kept alive so that it can
+    // handle callbacks later in case of blocked decoding.
+  }
+
+  // Decode an entire header block.
+  void DecodeHeaderBlock(QuicStringPiece data) {
+    StartDecoding();
+    DecodeData(data);
+    EndDecoding();
   }
 
   StrictMock<MockEncoderStreamErrorDelegate> encoder_stream_error_delegate_;
@@ -60,6 +80,7 @@
  private:
   QpackDecoder qpack_decoder_;
   const FragmentMode fragment_mode_;
+  std::unique_ptr<QpackProgressiveDecoder> progressive_decoder_;
 };
 
 INSTANTIATE_TEST_SUITE_P(,
@@ -387,8 +408,8 @@
   // This must cause the entry to be evicted.
   DecodeEncoderStreamData(QuicTextUtils::HexDecode("3f01"));
 
-  EXPECT_CALL(handler_,
-              OnDecodingErrorDetected(Eq("Dynamic table entry not found.")));
+  EXPECT_CALL(handler_, OnDecodingErrorDetected(
+                            Eq("Dynamic table entry already evicted.")));
 
   DecodeHeaderBlock(QuicTextUtils::HexDecode(
       "0200"   // Required Insert Count 1 and Delta Base 0.
@@ -446,6 +467,9 @@
 TEST_P(QpackDecoderTest, InvalidDynamicEntryWhenBaseIsZero) {
   EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Invalid relative index.")));
 
+  // Add literal entry with name "foo" and value "bar".
+  DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
+
   DecodeHeaderBlock(QuicTextUtils::HexDecode(
       "0280"   // Required Insert Count is 1.  Base 1 - 1 - 0 = 0 is explicitly
                // permitted by the spec.
@@ -465,8 +489,36 @@
   // Add literal entry with name "foo" and value "bar".
   DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
 
-  EXPECT_CALL(handler_,
-              OnDecodingErrorDetected(Eq("Dynamic table entry not found.")));
+  EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Invalid relative index.")));
+
+  DecodeHeaderBlock(QuicTextUtils::HexDecode(
+      "0200"   // Required Insert Count 1 and Delta Base 0.
+               // Base is 1 + 0 = 1.
+      "81"));  // Indexed Header Field instruction addressing relative index 1.
+               // This is absolute index -1, which is invalid.
+
+  EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Invalid relative index.")));
+
+  DecodeHeaderBlock(QuicTextUtils::HexDecode(
+      "0200"     // Required Insert Count 1 and Delta Base 0.
+                 // Base is 1 + 0 = 1.
+      "4100"));  // Literal Header Field with Name Reference instruction
+                 // addressing relative index 1.  This is absolute index -1,
+                 // which is invalid.
+}
+
+TEST_P(QpackDecoderTest, EvictedDynamicTableEntry) {
+  // Update dynamic table capacity to 128.
+  DecodeEncoderStreamData(QuicTextUtils::HexDecode("3f61"));
+
+  // Add literal entry with name "foo" and value "bar", size 32 + 3 + 3 = 38.
+  // This fits in the table three times.
+  DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
+  // Duplicate entry four times.  This evicts the first two instances.
+  DecodeEncoderStreamData(QuicTextUtils::HexDecode("00000000"));
+
+  EXPECT_CALL(handler_, OnDecodingErrorDetected(
+                            Eq("Dynamic table entry already evicted.")));
 
   DecodeHeaderBlock(QuicTextUtils::HexDecode(
       "0500"   // Required Insert Count 4 and Delta Base 0.
@@ -474,16 +526,8 @@
       "82"));  // Indexed Header Field instruction addressing relative index 2.
                // This is absolute index 1. Such entry does not exist.
 
-  EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Invalid relative index.")));
-
-  DecodeHeaderBlock(QuicTextUtils::HexDecode(
-      "0500"   // Required Insert Count 4 and Delta Base 0.
-               // Base is 4 + 0 = 4.
-      "84"));  // Indexed Header Field instruction addressing relative index 4.
-               // This is absolute index -1, which is invalid.
-
-  EXPECT_CALL(handler_,
-              OnDecodingErrorDetected(Eq("Dynamic table entry not found.")));
+  EXPECT_CALL(handler_, OnDecodingErrorDetected(
+                            Eq("Dynamic table entry already evicted.")));
 
   DecodeHeaderBlock(QuicTextUtils::HexDecode(
       "0500"     // Required Insert Count 4 and Delta Base 0.
@@ -492,22 +536,8 @@
                  // addressing relative index 2.  This is absolute index 1. Such
                  // entry does not exist.
 
-  EXPECT_CALL(handler_, OnDecodingErrorDetected(Eq("Invalid relative index.")));
-
-  DecodeHeaderBlock(QuicTextUtils::HexDecode(
-      "0500"     // Required Insert Count 4 and Delta Base 0.
-                 // Base is 4 + 0 = 4.
-      "4400"));  // Literal Header Field with Name Reference instruction
-                 // addressing relative index 4.  This is absolute index -1,
-                 // which is invalid.
-}
-
-TEST_P(QpackDecoderTest, InvalidDynamicEntryByPostBaseIndex) {
-  // Add literal entry with name "foo" and value "bar".
-  DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
-
-  EXPECT_CALL(handler_,
-              OnDecodingErrorDetected(Eq("Dynamic table entry not found.")));
+  EXPECT_CALL(handler_, OnDecodingErrorDetected(
+                            Eq("Dynamic table entry already evicted.")));
 
   DecodeHeaderBlock(QuicTextUtils::HexDecode(
       "0380"   // Required Insert Count 2 and Delta Base 0 with sign bit set.
@@ -516,8 +546,8 @@
                // entry with post-base index 0, absolute index 1.  Such entry
                // does not exist.
 
-  EXPECT_CALL(handler_,
-              OnDecodingErrorDetected(Eq("Dynamic table entry not found.")));
+  EXPECT_CALL(handler_, OnDecodingErrorDetected(
+                            Eq("Dynamic table entry already evicted.")));
 
   DecodeHeaderBlock(QuicTextUtils::HexDecode(
       "0380"     // Required Insert Count 2 and Delta Base 0 with sign bit set.
@@ -590,6 +620,9 @@
 }
 
 TEST_P(QpackDecoderTest, NonZeroRequiredInsertCountButNoDynamicEntries) {
+  // Add literal entry with name "foo" and value "bar".
+  DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
+
   EXPECT_CALL(handler_, OnHeaderDecoded(Eq(":method"), Eq("GET")));
   EXPECT_CALL(handler_,
               OnDecodingErrorDetected(Eq("Required Insert Count too large.")));
@@ -600,6 +633,9 @@
 }
 
 TEST_P(QpackDecoderTest, AddressEntryNotAllowedByRequiredInsertCount) {
+  // Add literal entry with name "foo" and value "bar".
+  DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
+
   EXPECT_CALL(
       handler_,
       OnDecodingErrorDetected(
@@ -655,7 +691,9 @@
 TEST_P(QpackDecoderTest, PromisedRequiredInsertCountLargerThanActual) {
   // Add literal entry with name "foo" and value "bar".
   DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
-  // Duplicate entry.
+  // Duplicate entry twice so that decoding of header blocks with Required
+  // Insert Count not exceeding 3 is not blocked.
+  DecodeEncoderStreamData(QuicTextUtils::HexDecode("00"));
   DecodeEncoderStreamData(QuicTextUtils::HexDecode("00"));
 
   EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar")));
@@ -707,6 +745,86 @@
                  // count of 2, even though Required Insert Count is 3.
 }
 
+TEST_P(QpackDecoderTest, BlockedDecoding) {
+  DecodeHeaderBlock(QuicTextUtils::HexDecode(
+      "0200"   // Required Insert Count 1 and Delta Base 0.
+               // Base is 1 + 0 = 1.
+      "80"));  // Indexed Header Field instruction addressing dynamic table
+               // entry with relative index 0, absolute index 0.
+
+  EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar")));
+  EXPECT_CALL(handler_, OnDecodingCompleted());
+  EXPECT_CALL(decoder_stream_sender_delegate_,
+              WriteStreamData(Eq(kHeaderAcknowledgement)));
+
+  // Add literal entry with name "foo" and value "bar".
+  DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
+}
+
+TEST_P(QpackDecoderTest, BlockedDecodingUnblockedBeforeEndOfHeaderBlock) {
+  StartDecoding();
+  DecodeData(QuicTextUtils::HexDecode(
+      "0200"   // Required Insert Count 1 and Delta Base 0.
+               // Base is 1 + 0 = 1.
+      "80"     // Indexed Header Field instruction addressing dynamic table
+               // entry with relative index 0, absolute index 0.
+      "d1"));  // Static table entry with index 17.
+
+  // Add literal entry with name "foo" and value "bar".  Decoding is now
+  // unblocked because dynamic table Insert Count reached the Required Insert
+  // Count of the header block.  |handler_| methods are called immediately for
+  // the already consumed part of the header block.
+  EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar")));
+  EXPECT_CALL(handler_, OnHeaderDecoded(Eq(":method"), Eq("GET")));
+  DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
+  Mock::VerifyAndClearExpectations(&handler_);
+
+  // Rest of header block is processed by QpackProgressiveDecoder
+  // in the unblocked state.
+  EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("bar")));
+  EXPECT_CALL(handler_, OnHeaderDecoded(Eq(":scheme"), Eq("https")));
+  DecodeData(QuicTextUtils::HexDecode(
+      "80"     // Indexed Header Field instruction addressing dynamic table
+               // entry with relative index 0, absolute index 0.
+      "d7"));  // Static table entry with index 23.
+  Mock::VerifyAndClearExpectations(&handler_);
+
+  EXPECT_CALL(handler_, OnDecodingCompleted());
+  EXPECT_CALL(decoder_stream_sender_delegate_,
+              WriteStreamData(Eq(kHeaderAcknowledgement)));
+  EndDecoding();
+}
+
+// Make sure that Required Insert Count is compared to Insert Count,
+// not size of dynamic table.
+TEST_P(QpackDecoderTest, BlockedDecodingAndEvictedEntries) {
+  // Update dynamic table capacity to 128.
+  // 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.
+      "80"));  // Indexed Header Field instruction addressing dynamic table
+               // entry with relative index 0, absolute index 5.
+
+  // Add literal entry with name "foo" and value "bar".
+  DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e703626172"));
+
+  // Duplicate entry four times.  This evicts the first two instances.
+  DecodeEncoderStreamData(QuicTextUtils::HexDecode("00000000"));
+
+  EXPECT_CALL(handler_, OnHeaderDecoded(Eq("foo"), Eq("baz")));
+  EXPECT_CALL(handler_, OnDecodingCompleted());
+  EXPECT_CALL(decoder_stream_sender_delegate_,
+              WriteStreamData(Eq(kHeaderAcknowledgement)));
+
+  // Add literal entry with name "foo" and value "bar".
+  // Insert Count is now 6, reaching Required Insert Count of the header block.
+  DecodeEncoderStreamData(QuicTextUtils::HexDecode("6294e70362617a"));
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace quic