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_;
