Makes Http2FrameDecoder a direct member of Http2DecoderAdapter, to avoid an unnecessary pointer dereference.
Also removes the no-argument constructor of Http2FrameDecoder, which is only used in tests.
PiperOrigin-RevId: 435046211
diff --git a/http2/decoder/http2_frame_decoder.h b/http2/decoder/http2_frame_decoder.h
index 295a0b0..b758dc8 100644
--- a/http2/decoder/http2_frame_decoder.h
+++ b/http2/decoder/http2_frame_decoder.h
@@ -50,7 +50,6 @@
class QUICHE_EXPORT_PRIVATE Http2FrameDecoder {
public:
explicit Http2FrameDecoder(Http2FrameDecoderListener* listener);
- Http2FrameDecoder() : Http2FrameDecoder(nullptr) {}
Http2FrameDecoder(const Http2FrameDecoder&) = delete;
Http2FrameDecoder& operator=(const Http2FrameDecoder&) = delete;
@@ -127,7 +126,8 @@
};
friend class test::Http2FrameDecoderPeer;
- friend std::ostream& operator<<(std::ostream& out, State v);
+ QUICHE_EXPORT_PRIVATE friend std::ostream& operator<<(std::ostream& out,
+ State v);
DecodeStatus StartDecodingPayload(DecodeBuffer* db);
DecodeStatus ResumeDecodingPayload(DecodeBuffer* db);
diff --git a/http2/decoder/http2_frame_decoder_test.cc b/http2/decoder/http2_frame_decoder_test.cc
index b3fd723..c628821 100644
--- a/http2/decoder/http2_frame_decoder_test.cc
+++ b/http2/decoder/http2_frame_decoder_test.cc
@@ -34,13 +34,6 @@
class Http2FrameDecoderTest : public RandomDecoderTest {
protected:
- void SetUp() override {
- // On any one run of this suite, we'll always choose the same value for
- // use_default_constructor_ because the random seed is the same for each
- // test case, but across runs the random seed changes.
- use_default_constructor_ = Random().OneIn(2);
- }
-
DecodeStatus StartDecoding(DecodeBuffer* db) override {
HTTP2_DVLOG(2) << "StartDecoding, db->Remaining=" << db->Remaining();
collector_.Reset();
@@ -91,16 +84,8 @@
}
void PrepareDecoder() {
- // Alternate which constructor is used.
- if (use_default_constructor_) {
- decoder_ = std::make_unique<Http2FrameDecoder>();
- decoder_->set_listener(&collector_);
- } else {
- decoder_ = std::make_unique<Http2FrameDecoder>(&collector_);
- }
+ decoder_ = std::make_unique<Http2FrameDecoder>(&collector_);
decoder_->set_maximum_payload_size(maximum_payload_size_);
-
- use_default_constructor_ = !use_default_constructor_;
}
void ResetDecodeSpeedCounters() {
@@ -212,7 +197,6 @@
uint32_t maximum_payload_size_ = Http2SettingsInfo::DefaultMaxFrameSize();
FramePartsCollectorListener collector_;
std::unique_ptr<Http2FrameDecoder> decoder_;
- bool use_default_constructor_;
};
////////////////////////////////////////////////////////////////////////////////
diff --git a/spdy/core/http2_frame_decoder_adapter.cc b/spdy/core/http2_frame_decoder_adapter.cc
index e79234f..f9e0f40 100644
--- a/spdy/core/http2_frame_decoder_adapter.cc
+++ b/spdy/core/http2_frame_decoder_adapter.cc
@@ -242,24 +242,11 @@
return "UNKNOWN_ERROR";
}
-Http2DecoderAdapter::Http2DecoderAdapter() {
+Http2DecoderAdapter::Http2DecoderAdapter() : frame_decoder_(this) {
QUICHE_DVLOG(1) << "Http2DecoderAdapter ctor";
- set_spdy_state(SpdyState::SPDY_READY_FOR_FRAME);
- spdy_framer_error_ = SpdyFramerError::SPDY_NO_ERROR;
-
- decoded_frame_header_ = false;
- has_frame_header_ = false;
- on_headers_called_ = false;
- on_hpack_fragment_called_ = false;
- latched_probable_http_response_ = false;
- has_expected_frame_type_ = false;
-
CorruptFrameHeader(&frame_header_);
CorruptFrameHeader(&hpack_first_frame_header_);
-
- frame_decoder_ = std::make_unique<Http2FrameDecoder>(this);
- hpack_decoder_ = nullptr;
}
Http2DecoderAdapter::~Http2DecoderAdapter() = default;
@@ -322,7 +309,7 @@
void Http2DecoderAdapter::SetMaxFrameSize(size_t max_frame_size) {
max_frame_size_ = max_frame_size;
- frame_decoder_->set_maximum_payload_size(max_frame_size);
+ frame_decoder_.set_maximum_payload_size(max_frame_size);
}
// ===========================================================================
@@ -805,7 +792,7 @@
size_t Http2DecoderAdapter::ProcessInputFrame(const char* data, size_t len) {
QUICHE_DCHECK_NE(spdy_state_, SpdyState::SPDY_ERROR);
DecodeBuffer db(data, len);
- DecodeStatus status = frame_decoder_->DecodeFrame(&db);
+ DecodeStatus status = frame_decoder_.DecodeFrame(&db);
if (spdy_state_ != SpdyState::SPDY_ERROR) {
DetermineSpdyState(status);
} else {
@@ -871,10 +858,11 @@
// Push the Http2FrameDecoder out of state kDiscardPayload now
// since doing so requires no input.
DecodeBuffer tmp("", 0);
- DecodeStatus status = frame_decoder_->DecodeFrame(&tmp);
- if (status != DecodeStatus::kDecodeDone) {
+ DecodeStatus decode_status = frame_decoder_.DecodeFrame(&tmp);
+ if (decode_status != DecodeStatus::kDecodeDone) {
QUICHE_BUG(spdy_bug_1_3)
- << "Expected to be done decoding the frame, not " << status;
+ << "Expected to be done decoding the frame, not "
+ << decode_status;
SetSpdyErrorAndNotify(SPDY_INTERNAL_FRAMER_ERROR, "");
} else if (spdy_framer_error_ != SPDY_NO_ERROR) {
QUICHE_BUG(spdy_bug_1_4)
@@ -915,7 +903,7 @@
QUICHE_DCHECK_NE(error, SpdyFramerError::SPDY_NO_ERROR);
spdy_framer_error_ = error;
set_spdy_state(SpdyState::SPDY_ERROR);
- frame_decoder_->set_listener(&no_op_listener_);
+ frame_decoder_.set_listener(&no_op_listener_);
visitor()->OnError(error, detailed_error);
}
}
@@ -945,9 +933,9 @@
size_t Http2DecoderAdapter::remaining_total_payload() const {
QUICHE_DCHECK(has_frame_header_);
- size_t remaining = frame_decoder_->remaining_payload();
+ size_t remaining = frame_decoder_.remaining_payload();
if (IsPaddable(frame_type()) && frame_header_.IsPadded()) {
- remaining += frame_decoder_->remaining_padding();
+ remaining += frame_decoder_.remaining_padding();
}
return remaining;
}
@@ -959,13 +947,13 @@
}
bool Http2DecoderAdapter::IsSkippingPadding() {
bool result = frame_header_.IsPadded() && opt_pad_length_ &&
- frame_decoder_->remaining_payload() == 0 &&
- frame_decoder_->remaining_padding() > 0;
+ frame_decoder_.remaining_payload() == 0 &&
+ frame_decoder_.remaining_padding() > 0;
QUICHE_DVLOG(2) << "Http2DecoderAdapter::IsSkippingPadding: " << result;
return result;
}
bool Http2DecoderAdapter::IsDiscardingPayload() {
- bool result = decoded_frame_header_ && frame_decoder_->IsDiscardingPayload();
+ bool result = decoded_frame_header_ && frame_decoder_.IsDiscardingPayload();
QUICHE_DVLOG(2) << "Http2DecoderAdapter::IsDiscardingPayload: " << result;
return result;
}
diff --git a/spdy/core/http2_frame_decoder_adapter.h b/spdy/core/http2_frame_decoder_adapter.h
index 3ed18f2..0c21972 100644
--- a/spdy/core/http2_frame_decoder_adapter.h
+++ b/spdy/core/http2_frame_decoder_adapter.h
@@ -290,22 +290,22 @@
// The HPACK decoder to be used for this adapter. User is responsible for
// clearing if the adapter is to be used for another connection.
- std::unique_ptr<spdy::HpackDecoderAdapter> hpack_decoder_ = nullptr;
+ std::unique_ptr<spdy::HpackDecoderAdapter> hpack_decoder_;
// The HTTP/2 frame decoder.
- std::unique_ptr<Http2FrameDecoder> frame_decoder_;
+ Http2FrameDecoder frame_decoder_;
// Next frame type expected. Currently only used for CONTINUATION frames,
// but could be used for detecting whether the first frame is a SETTINGS
// frame.
- // TODO(jamessyng): Provide means to indicate that decoder should require
+ // TODO(jamessynge): Provide means to indicate that decoder should require
// SETTINGS frame as the first frame.
Http2FrameType expected_frame_type_;
// Attempt to duplicate the SpdyState and SpdyFramerError values that
// SpdyFramer sets. Values determined by getting tests to pass.
- SpdyState spdy_state_;
- SpdyFramerError spdy_framer_error_;
+ SpdyState spdy_state_ = SpdyState::SPDY_READY_FOR_FRAME;
+ SpdyFramerError spdy_framer_error_ = SpdyFramerError::SPDY_NO_ERROR;
// The limit on the size of received HTTP/2 payloads as specified in the
// SETTINGS_MAX_FRAME_SIZE advertised to peer.
@@ -326,14 +326,14 @@
// Has OnHeaders() already been called for current HEADERS block? Only
// meaningful between OnHeadersStart and OnHeadersPriority.
- bool on_headers_called_;
+ bool on_headers_called_ = false;
// Has OnHpackFragment() already been called for current HPACK block?
// SpdyFramer will pass an empty buffer to the HPACK decoder if a HEADERS
// or PUSH_PROMISE has no HPACK data in it (e.g. a HEADERS frame with only
// padding). Detect that condition and replicate the behavior using this
// field.
- bool on_hpack_fragment_called_;
+ bool on_hpack_fragment_called_ = false;
// Have we seen a frame header that appears to be an HTTP/1 response?
bool latched_probable_http_response_ = false;
