Remove QpackProgressiveEncoder. Since encoding is not done incrementally any more, there is no need to keep a lot of state about encoding progress in class members: local variables suffice. This CL inlines QpackProgressiveEncoder's functionality into QpackEncoder and removes QpackProgressiveEncoder. No functional change. gfe-relnote: n/a, QUIC v99-only change. PiperOrigin-RevId: 254377802 Change-Id: I8d7b36748f6c7124f1b6e7a05e1beb0b27ef34ae
diff --git a/quic/core/qpack/qpack_encoder.cc b/quic/core/qpack/qpack_encoder.cc index 67e409e..d5bea0d 100644 --- a/quic/core/qpack/qpack_encoder.cc +++ b/quic/core/qpack/qpack_encoder.cc
@@ -6,7 +6,9 @@ #include <string> -#include "net/third_party/quiche/src/quic/core/qpack/qpack_progressive_encoder.h" +#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" +#include "net/third_party/quiche/src/quic/core/qpack/value_splitting_header_list.h" #include "net/third_party/quiche/src/quic/platform/api/quic_logging.h" #include "net/third_party/quiche/src/quic/platform/api/quic_ptr_util.h" @@ -25,16 +27,69 @@ QpackEncoder::~QpackEncoder() {} std::string QpackEncoder::EncodeHeaderList( - QuicStreamId stream_id, + QuicStreamId /* stream_id */, const spdy::SpdyHeaderBlock* header_list) { + QpackInstructionEncoder instruction_encoder; std::string encoded_headers; - auto progressive_encoder = QuicMakeUnique<QpackProgressiveEncoder>( - stream_id, &header_table_, &encoder_stream_sender_, header_list); - while (progressive_encoder->HasNext()) { - progressive_encoder->Next( - /* max_encoded_bytes = */ std::numeric_limits<size_t>::max(), - &encoded_headers); + + // TODO(bnc): Implement dynamic entries and set Required Insert Count and + // Delta Base accordingly. + instruction_encoder.set_varint(0); + instruction_encoder.set_varint2(0); + instruction_encoder.set_s_bit(false); + + instruction_encoder.Encode(QpackPrefixInstruction()); + DCHECK(instruction_encoder.HasNext()); + instruction_encoder.Next(std::numeric_limits<size_t>::max(), + &encoded_headers); + DCHECK(!instruction_encoder.HasNext()); + + for (const auto& header : ValueSplittingHeaderList(header_list)) { + QuicStringPiece name = header.first; + QuicStringPiece value = header.second; + + bool is_static; + uint64_t index; + + auto match_type = + header_table_.FindHeaderField(name, value, &is_static, &index); + + switch (match_type) { + case QpackHeaderTable::MatchType::kNameAndValue: + DCHECK(is_static) << "Dynamic table entries not supported yet."; + + instruction_encoder.set_s_bit(is_static); + instruction_encoder.set_varint(index); + + instruction_encoder.Encode(QpackIndexedHeaderFieldInstruction()); + + break; + case QpackHeaderTable::MatchType::kName: + DCHECK(is_static) << "Dynamic table entries not supported yet."; + + instruction_encoder.set_s_bit(is_static); + instruction_encoder.set_varint(index); + instruction_encoder.set_value(value); + + instruction_encoder.Encode( + QpackLiteralHeaderFieldNameReferenceInstruction()); + + break; + case QpackHeaderTable::MatchType::kNoMatch: + instruction_encoder.set_name(name); + instruction_encoder.set_value(value); + + instruction_encoder.Encode(QpackLiteralHeaderFieldInstruction()); + + break; + } + + DCHECK(instruction_encoder.HasNext()); + instruction_encoder.Next(std::numeric_limits<size_t>::max(), + &encoded_headers); + DCHECK(!instruction_encoder.HasNext()); } + return encoded_headers; }
diff --git a/quic/core/qpack/qpack_progressive_encoder.cc b/quic/core/qpack/qpack_progressive_encoder.cc deleted file mode 100644 index 731ec10..0000000 --- a/quic/core/qpack/qpack_progressive_encoder.cc +++ /dev/null
@@ -1,138 +0,0 @@ -// Copyright (c) 2018 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#include "net/third_party/quiche/src/quic/core/qpack/qpack_progressive_encoder.h" - -#include <string> - -#include "net/third_party/quiche/src/quic/core/qpack/qpack_constants.h" -#include "net/third_party/quiche/src/quic/core/qpack/qpack_header_table.h" -#include "net/third_party/quiche/src/quic/platform/api/quic_logging.h" -#include "net/third_party/quiche/src/quic/platform/api/quic_string_piece.h" - -namespace quic { - -QpackProgressiveEncoder::QpackProgressiveEncoder( - QuicStreamId stream_id, - QpackHeaderTable* header_table, - QpackEncoderStreamSender* encoder_stream_sender, - const spdy::SpdyHeaderBlock* header_list) - : stream_id_(stream_id), - header_table_(header_table), - encoder_stream_sender_(encoder_stream_sender), - header_list_(header_list), - header_list_iterator_(header_list_.begin()), - prefix_encoded_(false) { - // TODO(bnc): Use |stream_id_| for dynamic table entry management, and - // remove this dummy DCHECK. - DCHECK_LE(0u, stream_id_); - - DCHECK(header_table_); - DCHECK(encoder_stream_sender_); -} - -bool QpackProgressiveEncoder::HasNext() const { - return header_list_iterator_ != header_list_.end() || !prefix_encoded_; -} - -void QpackProgressiveEncoder::Next(size_t max_encoded_bytes, - std::string* output) { - DCHECK_NE(0u, max_encoded_bytes); - DCHECK(HasNext()); - - // Since QpackInstructionEncoder::Next() does not indicate the number of bytes - // written, save the maximum new size of |*output|. - const size_t max_length = output->size() + max_encoded_bytes; - - DCHECK_LT(output->size(), max_length); - - if (!prefix_encoded_ && !instruction_encoder_.HasNext()) { - // TODO(bnc): Implement dynamic entries and set Required Insert Count and - // Delta Base accordingly. - instruction_encoder_.set_varint(0); - instruction_encoder_.set_varint2(0); - instruction_encoder_.set_s_bit(false); - - instruction_encoder_.Encode(QpackPrefixInstruction()); - - DCHECK(instruction_encoder_.HasNext()); - } - - do { - // Call QpackInstructionEncoder::Encode() for |*header_list_iterator_| if it - // has not been called yet. - if (!instruction_encoder_.HasNext()) { - DCHECK(prefix_encoded_); - - // Even after |name| and |value| go out of scope, copies of these - // QuicStringPieces retained by QpackInstructionEncoder are still valid as - // long as |header_list_| is valid. - QuicStringPiece name = header_list_iterator_->first; - QuicStringPiece value = header_list_iterator_->second; - - // |is_static| and |index| are saved by QpackInstructionEncoder by value, - // there are no lifetime concerns. - bool is_static; - uint64_t index; - - auto match_type = - header_table_->FindHeaderField(name, value, &is_static, &index); - - switch (match_type) { - case QpackHeaderTable::MatchType::kNameAndValue: - DCHECK(is_static) << "Dynamic table entries not supported yet."; - - instruction_encoder_.set_s_bit(is_static); - instruction_encoder_.set_varint(index); - - instruction_encoder_.Encode(QpackIndexedHeaderFieldInstruction()); - - break; - case QpackHeaderTable::MatchType::kName: - DCHECK(is_static) << "Dynamic table entries not supported yet."; - - instruction_encoder_.set_s_bit(is_static); - instruction_encoder_.set_varint(index); - instruction_encoder_.set_value(value); - - instruction_encoder_.Encode( - QpackLiteralHeaderFieldNameReferenceInstruction()); - - break; - case QpackHeaderTable::MatchType::kNoMatch: - instruction_encoder_.set_name(name); - instruction_encoder_.set_value(value); - - instruction_encoder_.Encode(QpackLiteralHeaderFieldInstruction()); - - break; - } - } - - DCHECK(instruction_encoder_.HasNext()); - - instruction_encoder_.Next(max_length - output->size(), output); - - if (instruction_encoder_.HasNext()) { - // There was not enough room to completely encode current header field. - DCHECK_EQ(output->size(), max_length); - - return; - } - - // It is possible that the output buffer was just large enough for encoding - // the current header field, hence equality is allowed here. - DCHECK_LE(output->size(), max_length); - - if (prefix_encoded_) { - // Move on to the next header field. - ++header_list_iterator_; - } else { - // Mark prefix as encoded. - prefix_encoded_ = true; - } - } while (HasNext() && output->size() < max_length); -} - -} // namespace quic
diff --git a/quic/core/qpack/qpack_progressive_encoder.h b/quic/core/qpack/qpack_progressive_encoder.h deleted file mode 100644 index 98a3cfe..0000000 --- a/quic/core/qpack/qpack_progressive_encoder.h +++ /dev/null
@@ -1,60 +0,0 @@ -// Copyright (c) 2018 The Chromium Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef QUICHE_QUIC_CORE_QPACK_QPACK_PROGRESSIVE_ENCODER_H_ -#define QUICHE_QUIC_CORE_QPACK_QPACK_PROGRESSIVE_ENCODER_H_ - -#include <cstddef> - -#include "net/third_party/quiche/src/quic/core/qpack/qpack_encoder_stream_sender.h" -#include "net/third_party/quiche/src/quic/core/qpack/qpack_instruction_encoder.h" -#include "net/third_party/quiche/src/quic/core/qpack/value_splitting_header_list.h" -#include "net/third_party/quiche/src/quic/core/quic_types.h" -#include "net/third_party/quiche/src/quic/platform/api/quic_export.h" -#include "net/third_party/quiche/src/spdy/core/hpack/hpack_encoder.h" -#include "net/third_party/quiche/src/spdy/core/spdy_header_block.h" - -namespace quic { - -class QpackHeaderTable; - -// An implementation of ProgressiveEncoder interface that encodes a single -// header block. -class QUIC_EXPORT_PRIVATE QpackProgressiveEncoder - : public spdy::HpackEncoder::ProgressiveEncoder { - public: - QpackProgressiveEncoder() = delete; - // |header_table|, |encoder_stream_sender|, and |header_list| must all outlive - // this object. - QpackProgressiveEncoder(QuicStreamId stream_id, - QpackHeaderTable* header_table, - QpackEncoderStreamSender* encoder_stream_sender, - const spdy::SpdyHeaderBlock* header_list); - QpackProgressiveEncoder(const QpackProgressiveEncoder&) = delete; - QpackProgressiveEncoder& operator=(const QpackProgressiveEncoder&) = delete; - ~QpackProgressiveEncoder() override = default; - - // Returns true iff more remains to encode. - bool HasNext() const override; - - // Encodes up to |max_encoded_bytes| octets, appending to |output|. - void Next(size_t max_encoded_bytes, std::string* output) override; - - private: - const QuicStreamId stream_id_; - QpackInstructionEncoder instruction_encoder_; - const QpackHeaderTable* const header_table_; - QpackEncoderStreamSender* const encoder_stream_sender_; - const ValueSplittingHeaderList header_list_; - - // Header field currently being encoded. - ValueSplittingHeaderList::const_iterator header_list_iterator_; - - // False until prefix is fully encoded. - bool prefix_encoded_; -}; - -} // namespace quic - -#endif // QUICHE_QUIC_CORE_QPACK_QPACK_PROGRESSIVE_ENCODER_H_