commit 95734b22628ca59b1663eb0255a6c9e682f206e4
author bnc <bnc@google.com> Wed Jul 17 07:06:53 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Wed Jul 17 07:10:29 2019 -0700
tree 6e53dc27ec58938a4559784d79453445dc94e32f
parent 28bb06cdda73dc9d9fc5b652f05833dc1e64cb33

Refactor QpackEncoder to do two-pass encoding.

gfe-relnote: n/a, change in QUIC v99-only code.
PiperOrigin-RevId: 258561443
Change-Id: If03ac2f76064d2b462646bcda07a9e6e98655482

quic/core/qpack/qpack_encoder.cc

diff --git a/quic/core/qpack/qpack_encoder.cc b/quic/core/qpack/qpack_encoder.cc
index 412b7af..3ffe9d0 100644
--- a/quic/core/qpack/qpack_encoder.cc
+++ b/quic/core/qpack/qpack_encoder.cc
@@ -4,7 +4,7 @@
 
 #include "net/third_party/quiche/src/quic/core/qpack/qpack_encoder.h"
 
-#include <string>
+#include <list>
 
 #include "net/third_party/quiche/src/quic/core/qpack/qpack_constants.h"
 #include "net/third_party/quiche/src/quic/core/qpack/qpack_instruction_encoder.h"
@@ -29,18 +29,10 @@
 std::string QpackEncoder::EncodeHeaderList(
     QuicStreamId /* stream_id */,
     const spdy::SpdyHeaderBlock* header_list) {
-  QpackInstructionEncoder instruction_encoder;
-  std::string encoded_headers;
+  // First pass.
 
-  // TODO(bnc): Implement dynamic entries and set Required Insert Count and
-  // Delta Base accordingly.
-  QpackInstructionEncoder::Values values;
-  values.varint = 0;
-  values.varint2 = 0;
-  values.s_bit = false;
-
-  instruction_encoder.Encode(QpackPrefixInstruction(), values,
-                             &encoded_headers);
+  // Encode into |instructions| which will be serialized during the second pass.
+  std::list<InstructionWithValues> instructions;
 
   for (const auto& header : ValueSplittingHeaderList(header_list)) {
     QuicStringPiece name = header.first;
@@ -56,36 +48,50 @@
       case QpackHeaderTable::MatchType::kNameAndValue:
         DCHECK(is_static) << "Dynamic table entries not supported yet.";
 
-        values.s_bit = is_static;
-        values.varint = index;
-
-        instruction_encoder.Encode(QpackIndexedHeaderFieldInstruction(), values,
-                                   &encoded_headers);
+        instructions.push_back({QpackIndexedHeaderFieldInstruction(), {}});
+        instructions.back().values.s_bit = is_static;
+        instructions.back().values.varint = index;
 
         break;
       case QpackHeaderTable::MatchType::kName:
         DCHECK(is_static) << "Dynamic table entries not supported yet.";
 
-        values.s_bit = is_static;
-        values.varint = index;
-        values.value = value;
-
-        instruction_encoder.Encode(
-            QpackLiteralHeaderFieldNameReferenceInstruction(), values,
-            &encoded_headers);
+        instructions.push_back(
+            {QpackLiteralHeaderFieldNameReferenceInstruction(), {}});
+        instructions.back().values.s_bit = is_static;
+        instructions.back().values.varint = index;
+        instructions.back().values.value = value;
 
         break;
       case QpackHeaderTable::MatchType::kNoMatch:
-        values.name = name;
-        values.value = value;
-
-        instruction_encoder.Encode(QpackLiteralHeaderFieldInstruction(), values,
-                                   &encoded_headers);
+        instructions.push_back({QpackLiteralHeaderFieldInstruction(), {}});
+        instructions.back().values.name = name;
+        instructions.back().values.value = value;
 
         break;
     }
   }
 
+  // Second pass.
+  QpackInstructionEncoder instruction_encoder;
+  std::string encoded_headers;
+
+  // Header block prefix.
+  // TODO(bnc): Implement dynamic entries and set Required Insert Count and
+  // Delta Base accordingly.
+  QpackInstructionEncoder::Values values;
+  values.varint = 0;     // Encoded required insert count.
+  values.varint2 = 0;    // Delta Base.
+  values.s_bit = false;  // Delta Base sign.
+
+  instruction_encoder.Encode(QpackPrefixInstruction(), values,
+                             &encoded_headers);
+
+  for (const auto& instruction : instructions) {
+    instruction_encoder.Encode(instruction.instruction, instruction.values,
+                               &encoded_headers);
+  }
+
   return encoded_headers;
 }
 

quic/core/qpack/qpack_encoder.h

diff --git a/quic/core/qpack/qpack_encoder.h b/quic/core/qpack/qpack_encoder.h
index 7ed5808..39600e8 100644
--- a/quic/core/qpack/qpack_encoder.h
+++ b/quic/core/qpack/qpack_encoder.h
@@ -56,6 +56,17 @@
   void OnErrorDetected(QuicStringPiece error_message) override;
 
  private:
+  // TODO(bnc): Consider moving this class to QpackInstructionEncoder or
+  // qpack_constants, adding factory methods, one for each instruction, and
+  // changing QpackInstructionEncoder::Encoder() to take an
+  // InstructionWithValues struct instead of separate |instruction| and |values|
+  // arguments.
+  struct InstructionWithValues {
+    // |instruction| is not owned.
+    const QpackInstruction* instruction;
+    QpackInstructionEncoder::Values values;
+  };
+
   DecoderStreamErrorDelegate* const decoder_stream_error_delegate_;
   QpackDecoderStreamReceiver decoder_stream_receiver_;
   QpackEncoderStreamSender encoder_stream_sender_;