Remove FrameSequence and MetadataSerializer.
FrameSequence is an interface only implemented by MetadataFrameSequence (in
anonymous namespace). Remove the interface and make MetadataFrameSequence
public.
MetadataSerializer is a trivial class with no members and only one factory
method that creates a MetadataFrameSequence on the heap. Remove that class.
Also remove the factory method. Change call sites to instantiate
MetadataFrameSequence on the stack instead of the heap.
PiperOrigin-RevId: 460745625
diff --git a/quiche/spdy/core/metadata_extension.cc b/quiche/spdy/core/metadata_extension.cc
index 0993b99..ec0b2c6 100644
--- a/quiche/spdy/core/metadata_extension.cc
+++ b/quiche/spdy/core/metadata_extension.cc
@@ -9,7 +9,6 @@
#include "quiche/http2/hpack/decoder/hpack_decoder.h"
#include "quiche/common/platform/api/quiche_bug_tracker.h"
#include "quiche/common/platform/api/quiche_logging.h"
-#include "quiche/spdy/core/hpack/hpack_encoder.h"
#include "quiche/spdy/core/http2_header_block_hpack_listener.h"
namespace spdy {
@@ -23,33 +22,19 @@
const size_t kMaxMetadataBlockSize = 1 << 20; // 1 MB
-// This class uses an HpackEncoder to serialize a METADATA block as a series of
-// METADATA frames.
-class MetadataFrameSequence : public MetadataSerializer::FrameSequence {
- public:
- MetadataFrameSequence(SpdyStreamId stream_id, spdy::SpdyHeaderBlock payload)
- : stream_id_(stream_id), payload_(std::move(payload)) {
- // Metadata should not use HPACK compression.
- encoder_.DisableCompression();
- HpackEncoder::Representations r;
- for (const auto& kv_pair : payload_) {
- r.push_back(kv_pair);
- }
- progressive_encoder_ = encoder_.EncodeRepresentations(r);
+} // anonymous namespace
+
+MetadataFrameSequence::MetadataFrameSequence(SpdyStreamId stream_id,
+ spdy::Http2HeaderBlock payload)
+ : stream_id_(stream_id), payload_(std::move(payload)) {
+ // Metadata should not use HPACK compression.
+ encoder_.DisableCompression();
+ HpackEncoder::Representations r;
+ for (const auto& kv_pair : payload_) {
+ r.push_back(kv_pair);
}
-
- // Copies are not allowed.
- MetadataFrameSequence(const MetadataFrameSequence& other) = delete;
- MetadataFrameSequence& operator=(const MetadataFrameSequence& other) = delete;
-
- std::unique_ptr<spdy::SpdyFrameIR> Next() override;
-
- private:
- SpdyStreamId stream_id_;
- SpdyHeaderBlock payload_;
- HpackEncoder encoder_;
- std::unique_ptr<HpackEncoder::ProgressiveEncoder> progressive_encoder_;
-};
+ progressive_encoder_ = encoder_.EncodeRepresentations(r);
+}
std::unique_ptr<spdy::SpdyFrameIR> MetadataFrameSequence::Next() {
if (!progressive_encoder_->HasNext()) {
@@ -65,8 +50,6 @@
std::move(payload));
}
-} // anonymous namespace
-
struct MetadataVisitor::MetadataPayloadState {
MetadataPayloadState(size_t remaining, bool end)
: bytes_remaining(remaining), end_metadata(end) {}
@@ -185,11 +168,4 @@
}
}
-std::unique_ptr<MetadataSerializer::FrameSequence>
-MetadataSerializer::FrameSequenceForPayload(SpdyStreamId stream_id,
- MetadataPayload payload) {
- return absl::make_unique<MetadataFrameSequence>(stream_id,
- std::move(payload));
-}
-
} // namespace spdy
diff --git a/quiche/spdy/core/metadata_extension.h b/quiche/spdy/core/metadata_extension.h
index 4eaf95c..12d817c 100644
--- a/quiche/spdy/core/metadata_extension.h
+++ b/quiche/spdy/core/metadata_extension.h
@@ -7,6 +7,7 @@
#include "absl/container/flat_hash_map.h"
#include "quiche/common/platform/api/quiche_export.h"
+#include "quiche/spdy/core/hpack/hpack_encoder.h"
#include "quiche/spdy/core/http2_frame_decoder_adapter.h"
#include "quiche/spdy/core/spdy_header_block.h"
#include "quiche/spdy/core/spdy_protocol.h"
@@ -90,27 +91,23 @@
MetadataSupportState peer_supports_metadata_;
};
-// A class that serializes metadata blocks as sequences of frames.
-class QUICHE_EXPORT_PRIVATE MetadataSerializer {
+// This class uses an HpackEncoder to serialize a METADATA block as a series of
+// METADATA frames.
+class QUICHE_EXPORT_PRIVATE MetadataFrameSequence {
public:
- using MetadataPayload = spdy::SpdyHeaderBlock;
+ MetadataFrameSequence(SpdyStreamId stream_id, spdy::Http2HeaderBlock payload);
- class QUICHE_EXPORT_PRIVATE FrameSequence {
- public:
- virtual ~FrameSequence() {}
+ // Copies are not allowed.
+ MetadataFrameSequence(const MetadataFrameSequence& other) = delete;
+ MetadataFrameSequence& operator=(const MetadataFrameSequence& other) = delete;
- // Returns nullptr once the sequence has been exhausted.
- virtual std::unique_ptr<spdy::SpdyFrameIR> Next() = 0;
- };
+ std::unique_ptr<spdy::SpdyFrameIR> Next();
- MetadataSerializer() {}
-
- MetadataSerializer(const MetadataSerializer&) = delete;
- MetadataSerializer& operator=(const MetadataSerializer&) = delete;
-
- // Returns nullptr on failure.
- std::unique_ptr<FrameSequence> FrameSequenceForPayload(
- spdy::SpdyStreamId stream_id, MetadataPayload payload);
+ private:
+ SpdyStreamId stream_id_;
+ Http2HeaderBlock payload_;
+ HpackEncoder encoder_;
+ std::unique_ptr<HpackEncoder::ProgressiveEncoder> progressive_encoder_;
};
} // namespace spdy
diff --git a/quiche/spdy/core/metadata_extension_test.cc b/quiche/spdy/core/metadata_extension_test.cc
index 26f98f4..8d9551d 100644
--- a/quiche/spdy/core/metadata_extension_test.cc
+++ b/quiche/spdy/core/metadata_extension_test.cc
@@ -17,9 +17,6 @@
namespace {
using ::absl::bind_front;
-using ::spdy::SpdyFramer;
-using ::spdy::SpdyHeaderBlock;
-using ::spdy::test::MockSpdyFramerVisitor;
using ::testing::_;
using ::testing::ElementsAre;
using ::testing::IsEmpty;
@@ -48,8 +45,8 @@
received_metadata_support_.push_back(peer_supports_metadata);
}
- MetadataSerializer::MetadataPayload PayloadForData(absl::string_view data) {
- SpdyHeaderBlock block;
+ Http2HeaderBlock PayloadForData(absl::string_view data) {
+ Http2HeaderBlock block;
block["example-payload"] = data;
return block;
}
@@ -90,9 +87,7 @@
extension_->OnSetting(MetadataVisitor::kMetadataExtensionId, 1);
ASSERT_TRUE(extension_->PeerSupportsMetadata());
- MetadataSerializer serializer;
- auto sequence = serializer.FrameSequenceForPayload(3, std::move(payload));
- ASSERT_TRUE(sequence != nullptr);
+ MetadataFrameSequence sequence(3, std::move(payload));
http2::Http2DecoderAdapter deframer;
::testing::StrictMock<MockSpdyFramerVisitor> visitor;
@@ -105,7 +100,7 @@
.WillOnce(::testing::Return(true));
SpdyFramer framer(SpdyFramer::ENABLE_COMPRESSION);
- auto frame = sequence->Next();
+ auto frame = sequence.Next();
ASSERT_TRUE(frame != nullptr);
while (frame != nullptr) {
const size_t frame_size = framer.SerializeFrame(*frame, &test_buffer_);
@@ -114,7 +109,7 @@
ASSERT_EQ(frame_size, test_buffer_.Size());
EXPECT_EQ(frame_size, deframer.ProcessInput(kBuffer, frame_size));
test_buffer_.Reset();
- frame = sequence->Next();
+ frame = sequence.Next();
}
EXPECT_FALSE(deframer.HasError());
EXPECT_THAT(received_metadata_support_, ElementsAre(true));
@@ -140,17 +135,13 @@
extension_->OnSetting(MetadataVisitor::kMetadataExtensionId, 1);
ASSERT_TRUE(extension_->PeerSupportsMetadata());
- MetadataSerializer serializer;
- auto sequence =
- serializer.FrameSequenceForPayload(3, payload_block.Clone());
- ASSERT_TRUE(sequence != nullptr);
-
+ MetadataFrameSequence sequence(3, payload_block.Clone());
http2::Http2DecoderAdapter deframer;
::spdy::SpdyNoOpVisitor visitor;
deframer.set_visitor(&visitor);
deframer.set_extension_visitor(extension_.get());
SpdyFramer framer(SpdyFramer::ENABLE_COMPRESSION);
- auto frame = sequence->Next();
+ auto frame = sequence.Next();
ASSERT_TRUE(frame != nullptr);
while (frame != nullptr) {
const size_t frame_size = framer.SerializeFrame(*frame, &test_buffer_);
@@ -159,7 +150,7 @@
ASSERT_EQ(frame_size, test_buffer_.Size());
EXPECT_EQ(frame_size, deframer.ProcessInput(kBuffer, frame_size));
test_buffer_.Reset();
- frame = sequence->Next();
+ frame = sequence.Next();
}
EXPECT_EQ(1u, received_count_);
auto it = received_payload_map_.find(3);
@@ -183,12 +174,8 @@
extension_->OnSetting(MetadataVisitor::kMetadataExtensionId, 1);
ASSERT_TRUE(extension_->PeerSupportsMetadata());
- MetadataSerializer serializer;
- auto sequence1 = serializer.FrameSequenceForPayload(3, payload1.Clone());
- ASSERT_TRUE(sequence1 != nullptr);
-
- auto sequence2 = serializer.FrameSequenceForPayload(5, payload2.Clone());
- ASSERT_TRUE(sequence2 != nullptr);
+ MetadataFrameSequence sequence1(3, payload1.Clone());
+ MetadataFrameSequence sequence2(5, payload2.Clone());
http2::Http2DecoderAdapter deframer;
::spdy::SpdyNoOpVisitor visitor;
@@ -196,9 +183,9 @@
deframer.set_extension_visitor(extension_.get());
SpdyFramer framer(SpdyFramer::ENABLE_COMPRESSION);
- auto frame1 = sequence1->Next();
+ auto frame1 = sequence1.Next();
ASSERT_TRUE(frame1 != nullptr);
- auto frame2 = sequence2->Next();
+ auto frame2 = sequence2.Next();
ASSERT_TRUE(frame2 != nullptr);
while (frame1 != nullptr || frame2 != nullptr) {
for (auto frame : {frame1.get(), frame2.get()}) {
@@ -211,8 +198,8 @@
test_buffer_.Reset();
}
}
- frame1 = sequence1->Next();
- frame2 = sequence2->Next();
+ frame1 = sequence1.Next();
+ frame2 = sequence2.Next();
}
EXPECT_EQ(2u, received_count_);
auto it = received_payload_map_.find(3);
