Remove HttpDecoder::set_visitor(). Since all call sites have a visitor ready when HttpDecoder is instantiated, pass that in HttpDecoder constructor instead of a set_visitor() method. Also make HttpDecoder::visitor_ member constant. Just one fewer way to hold HttpDecoder wrong. While touching this area already, use |QuicMakeUnique| instead of |new| for initializing unique_ptr<HttpDecoderVisitor> members. And add quic_ptr_util.h include to every file touched that needs it, even if no QuicMakeUnique is added with this CL. Also remove unused QuicReceiveControlStreamTest::decoder_ member. gfe-relnote: n/a, change only affects QUIC v99. PiperOrigin-RevId: 257306155 Change-Id: I6b27704dc6001f69668a36cd6072aaa24a95b389
diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc index ffd5c74..1c4c083 100644 --- a/quic/core/http/http_decoder.cc +++ b/quic/core/http/http_decoder.cc
@@ -29,8 +29,8 @@ } // namespace -HttpDecoder::HttpDecoder() - : visitor_(nullptr), +HttpDecoder::HttpDecoder(Visitor* visitor) + : visitor_(visitor), state_(STATE_READING_FRAME_TYPE), current_frame_type_(0), current_length_field_length_(0), @@ -40,7 +40,9 @@ current_type_field_length_(0), remaining_type_field_length_(0), error_(QUIC_NO_ERROR), - error_detail_("") {} + error_detail_("") { + DCHECK(visitor_); +} HttpDecoder::~HttpDecoder() {}
diff --git a/quic/core/http/http_decoder.h b/quic/core/http/http_decoder.h index 515daae..b13af0e 100644 --- a/quic/core/http/http_decoder.h +++ b/quic/core/http/http_decoder.h
@@ -110,16 +110,11 @@ // to allow callers to handle unknown frames. }; - HttpDecoder(); + // |visitor| must be non-null, and must outlive HttpDecoder. + explicit HttpDecoder(Visitor* visitor); ~HttpDecoder(); - // Set callbacks to be called from the decoder. A visitor must be set, or - // else the decoder will crash. It is acceptable for the visitor to do - // nothing. If this is called multiple times, only the last visitor - // will be used. |visitor| will be owned by the caller. - void set_visitor(Visitor* visitor) { visitor_ = visitor; } - // Processes the input and invokes the appropriate visitor methods, until a // visitor method returns false or an error occurs. Returns the number of // bytes processed. Does not process any input if called after an error. @@ -190,7 +185,7 @@ QuicByteCount MaxFrameLength(uint8_t frame_type); // Visitor to invoke when messages are parsed. - Visitor* visitor_; // Unowned. + Visitor* const visitor_; // Unowned. // Current state of the parsing. HttpDecoderState state_; // Type of the frame currently being parsed.
diff --git a/quic/core/http/http_decoder_test.cc b/quic/core/http/http_decoder_test.cc index 135271b..9229db0 100644 --- a/quic/core/http/http_decoder_test.cc +++ b/quic/core/http/http_decoder_test.cc
@@ -57,7 +57,7 @@ class HttpDecoderTest : public QuicTest { public: - HttpDecoderTest() { + HttpDecoderTest() : decoder_(&visitor_) { ON_CALL(visitor_, OnPriorityFrameStart(_)).WillByDefault(Return(true)); ON_CALL(visitor_, OnPriorityFrame(_)).WillByDefault(Return(true)); ON_CALL(visitor_, OnCancelPushFrame(_)).WillByDefault(Return(true)); @@ -75,7 +75,6 @@ ON_CALL(visitor_, OnPushPromiseFrameStart(_)).WillByDefault(Return(true)); ON_CALL(visitor_, OnPushPromiseFramePayload(_)).WillByDefault(Return(true)); ON_CALL(visitor_, OnPushPromiseFrameEnd()).WillByDefault(Return(true)); - decoder_.set_visitor(&visitor_); } ~HttpDecoderTest() override = default; @@ -112,8 +111,8 @@ return processed_bytes; } - HttpDecoder decoder_; testing::StrictMock<MockVisitor> visitor_; + HttpDecoder decoder_; }; TEST_F(HttpDecoderTest, InitialState) {
diff --git a/quic/core/http/quic_receive_control_stream.cc b/quic/core/http/quic_receive_control_stream.cc index 4529430..9e227f4 100644 --- a/quic/core/http/quic_receive_control_stream.cc +++ b/quic/core/http/quic_receive_control_stream.cc
@@ -6,6 +6,7 @@ #include "net/third_party/quiche/src/quic/core/http/http_decoder.h" #include "net/third_party/quiche/src/quic/core/http/quic_spdy_session.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_ptr_util.h" namespace quic { @@ -134,9 +135,9 @@ : QuicStream(pending, READ_UNIDIRECTIONAL, /*is_static=*/true), current_priority_length_(0), received_settings_length_(0), - http_decoder_visitor_(new HttpDecoderVisitor(this)), + http_decoder_visitor_(QuicMakeUnique<HttpDecoderVisitor>(this)), + decoder_(http_decoder_visitor_.get()), sequencer_offset_(sequencer()->NumBytesConsumed()) { - decoder_.set_visitor(http_decoder_visitor_.get()); sequencer()->set_level_triggered(true); }
diff --git a/quic/core/http/quic_receive_control_stream.h b/quic/core/http/quic_receive_control_stream.h index 3ea53cf..1bd13fe 100644 --- a/quic/core/http/quic_receive_control_stream.h +++ b/quic/core/http/quic_receive_control_stream.h
@@ -41,16 +41,15 @@ // TODO(renjietang): Decode Priority in HTTP/3 style. bool OnPriorityFrame(const PriorityFrame& priority); - HttpDecoder decoder_; - // Track the current priority frame length. QuicByteCount current_priority_length_; // Track the number of settings bytes received. size_t received_settings_length_; - // HttpDecoder's visitor. + // HttpDecoder and its visitor. std::unique_ptr<HttpDecoderVisitor> http_decoder_visitor_; + HttpDecoder decoder_; // Sequencer offset keeping track of how much data HttpDecoder has processed. // Initial value is sequencer()->NumBytesConsumed() at time of
diff --git a/quic/core/http/quic_receive_control_stream_test.cc b/quic/core/http/quic_receive_control_stream_test.cc index 95fe8a2..8a0af4f 100644 --- a/quic/core/http/quic_receive_control_stream_test.cc +++ b/quic/core/http/quic_receive_control_stream_test.cc
@@ -5,6 +5,7 @@ #include "net/third_party/quiche/src/quic/core/http/quic_receive_control_stream.h" #include "net/third_party/quiche/src/quic/core/quic_utils.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_ptr_util.h" #include "net/third_party/quiche/src/quic/test_tools/quic_spdy_session_peer.h" #include "net/third_party/quiche/src/quic/test_tools/quic_test_utils.h" @@ -98,7 +99,6 @@ MockAlarmFactory alarm_factory_; StrictMock<MockQuicConnection>* connection_; StrictMock<MockQuicSpdySession> session_; - HttpDecoder decoder_; std::unique_ptr<QuicReceiveControlStream> receive_control_stream_; TestStream* stream_; };
diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc index 0748d45..6b7c48f 100644 --- a/quic/core/http/quic_spdy_stream.cc +++ b/quic/core/http/quic_spdy_stream.cc
@@ -22,6 +22,7 @@ #include "net/third_party/quiche/src/quic/platform/api/quic_flags.h" #include "net/third_party/quiche/src/quic/platform/api/quic_logging.h" #include "net/third_party/quiche/src/quic/platform/api/quic_mem_slice_storage.h" +#include "net/third_party/quiche/src/quic/platform/api/quic_ptr_util.h" #include "net/third_party/quiche/src/quic/platform/api/quic_string_piece.h" #include "net/third_party/quiche/src/quic/platform/api/quic_text_utils.h" #include "net/third_party/quiche/src/spdy/core/spdy_protocol.h" @@ -170,7 +171,8 @@ trailers_consumed_(false), priority_sent_(false), headers_bytes_to_be_marked_consumed_(0), - http_decoder_visitor_(new HttpDecoderVisitor(this)), + http_decoder_visitor_(QuicMakeUnique<HttpDecoderVisitor>(this)), + decoder_(http_decoder_visitor_.get()), body_buffer_(sequencer()), sequencer_offset_(0), is_decoder_processing_input_(false), @@ -188,7 +190,6 @@ spdy_session_->connection()->transport_version())) { sequencer()->set_level_triggered(true); } - decoder_.set_visitor(http_decoder_visitor_.get()); } QuicSpdyStream::QuicSpdyStream(PendingStream* pending, @@ -206,7 +207,8 @@ trailers_consumed_(false), priority_sent_(false), headers_bytes_to_be_marked_consumed_(0), - http_decoder_visitor_(new HttpDecoderVisitor(this)), + http_decoder_visitor_(QuicMakeUnique<HttpDecoderVisitor>(this)), + decoder_(http_decoder_visitor_.get()), body_buffer_(sequencer()), sequencer_offset_(sequencer()->NumBytesConsumed()), is_decoder_processing_input_(false), @@ -223,7 +225,6 @@ spdy_session_->connection()->transport_version())) { sequencer()->set_level_triggered(true); } - decoder_.set_visitor(http_decoder_visitor_.get()); } QuicSpdyStream::~QuicSpdyStream() {}
diff --git a/quic/core/http/quic_spdy_stream.h b/quic/core/http/quic_spdy_stream.h index 452109a..9cd7827 100644 --- a/quic/core/http/quic_spdy_stream.h +++ b/quic/core/http/quic_spdy_stream.h
@@ -302,13 +302,13 @@ // Http encoder for writing streams. HttpEncoder encoder_; - // Http decoder for processing raw incoming stream frames. - HttpDecoder decoder_; // Headers accumulator for decoding HEADERS frame payload. std::unique_ptr<QpackDecodedHeadersAccumulator> qpack_decoded_headers_accumulator_; // Visitor of the HttpDecoder. std::unique_ptr<HttpDecoderVisitor> http_decoder_visitor_; + // HttpDecoder for processing raw incoming stream frames. + HttpDecoder decoder_; // Buffer that contains decoded data of the stream. QuicSpdyStreamBodyBuffer body_buffer_;