commit f21c1adbb32768a7df863f1513e3d1050bd58a6f
author bnc <bnc@google.com> Thu Jun 20 20:09:50 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Fri Jun 21 13:04:05 2019 -0700
tree d0df12aba9ac2e3ba6fd2ed00d9964688942c55f
parent 7d4f913c7d5a87fca15fc7d26dc343ee11ff7754

Make QpackEncoder interface not progressive.

When the QPACK implementation was originally written, an
HpackEncoder::ProgressiveEncoder interface was used for encoding.  However, the
way it is used in QuicSpdyStream, it is clear now that encoding can be done in a
single pass into a std::string object.  QuicStream::WriteOrBufferData() does not
lend itself easily to notifying the QPACK encoder that it is ready to take more
data.  Instead it is more natural to encode the entire header block and buffer
that in QuicStreamSendBuffer.  Also note that this should not cost more memory
than doing progressive encoding, because the SpdyHeaderBlock object would need
to be kept around if progressive encoding was used.

This CL introduces no functional change, it just changes the interface, and
moves the code that calls Next() from QuicSpdyStream into QpackEncoder.  A
follow-up CL will clean up QpackEncoder internals.

gfe-relnote: n/a, QUIC v99-only change.
PiperOrigin-RevId: 254323435
Change-Id: Ic3d203ed9f019ad01dba61d407a6e73bde5d12d1

quic/core/qpack/qpack_round_trip_test.cc

diff --git a/quic/core/qpack/qpack_round_trip_test.cc b/quic/core/qpack/qpack_round_trip_test.cc
index 4fc1a38..1e9de2d 100644
--- a/quic/core/qpack/qpack_round_trip_test.cc
+++ b/quic/core/qpack/qpack_round_trip_test.cc
@@ -20,29 +20,25 @@
 namespace test {
 namespace {
 
-class QpackRoundTripTest
-    : public QuicTestWithParam<std::tuple<FragmentMode, FragmentMode>> {
+class QpackRoundTripTest : public QuicTestWithParam<FragmentMode> {
  public:
-  QpackRoundTripTest()
-      : encoding_fragment_mode_(std::get<0>(GetParam())),
-        decoding_fragment_mode_(std::get<1>(GetParam())) {}
+  QpackRoundTripTest() = default;
   ~QpackRoundTripTest() override = default;
 
   spdy::SpdyHeaderBlock EncodeThenDecode(
       const spdy::SpdyHeaderBlock& header_list) {
     NoopDecoderStreamErrorDelegate decoder_stream_error_delegate;
     NoopQpackStreamSenderDelegate encoder_stream_sender_delegate;
-    std::string encoded_header_block = QpackEncode(
-        &decoder_stream_error_delegate, &encoder_stream_sender_delegate,
-        FragmentModeToFragmentSizeGenerator(encoding_fragment_mode_),
-        &header_list);
+    QpackEncoder encoder(&decoder_stream_error_delegate,
+                         &encoder_stream_sender_delegate);
+    std::string encoded_header_block =
+        encoder.EncodeHeaderList(/* stream_id = */ 1, &header_list);
 
     TestHeadersHandler handler;
     NoopEncoderStreamErrorDelegate encoder_stream_error_delegate;
     NoopQpackStreamSenderDelegate decoder_stream_sender_delegate;
     QpackDecode(&encoder_stream_error_delegate, &decoder_stream_sender_delegate,
-                &handler,
-                FragmentModeToFragmentSizeGenerator(decoding_fragment_mode_),
+                &handler, FragmentModeToFragmentSizeGenerator(GetParam()),
                 encoded_header_block);
 
     EXPECT_TRUE(handler.decoding_completed());
@@ -50,17 +46,12 @@
 
     return handler.ReleaseHeaderList();
   }
-
- private:
-  const FragmentMode encoding_fragment_mode_;
-  const FragmentMode decoding_fragment_mode_;
 };
 
-INSTANTIATE_TEST_SUITE_P(
-    ,
-    QpackRoundTripTest,
-    Combine(Values(FragmentMode::kSingleChunk, FragmentMode::kOctetByOctet),
-            Values(FragmentMode::kSingleChunk, FragmentMode::kOctetByOctet)));
+INSTANTIATE_TEST_SUITE_P(,
+                         QpackRoundTripTest,
+                         Values(FragmentMode::kSingleChunk,
+                                FragmentMode::kOctetByOctet));
 
 TEST_P(QpackRoundTripTest, Empty) {
   spdy::SpdyHeaderBlock header_list;