Remove QpackDecodedHeadersAccumulatorResetReason. The condition that this extra logging was meant to help debug did not happen since February 3rd. The QUIC_BUGs added at cl/423164997 never triggered in production. It is time to remove the extra member identifying where `qpack_decoded_headers_accumulator_` was reset. The QUIC_BUGs can stay in place indefinitely. PiperOrigin-RevId: 455258985
diff --git a/quiche/quic/core/http/quic_spdy_stream.cc b/quiche/quic/core/http/quic_spdy_stream.cc index 91e264c..fd33936 100644 --- a/quiche/quic/core/http/quic_spdy_stream.cc +++ b/quiche/quic/core/http/quic_spdy_stream.cc
@@ -185,8 +185,6 @@ headers_payload_length_(0), trailers_decompressed_(false), trailers_consumed_(false), - qpack_decoded_headers_accumulator_reset_reason_( - QpackDecodedHeadersAccumulatorResetReason::kUnSet), http_decoder_visitor_(std::make_unique<HttpDecoderVisitor>(this)), decoder_(http_decoder_visitor_.get(), HttpDecoderOptionsForBidiStream(spdy_session)), @@ -223,8 +221,6 @@ headers_payload_length_(0), trailers_decompressed_(false), trailers_consumed_(false), - qpack_decoded_headers_accumulator_reset_reason_( - QpackDecodedHeadersAccumulatorResetReason::kUnSet), http_decoder_visitor_(std::make_unique<HttpDecoderVisitor>(this)), decoder_(http_decoder_visitor_.get()), sequencer_offset_(sequencer()->NumBytesConsumed()), @@ -561,8 +557,6 @@ bool header_list_size_limit_exceeded) { header_list_size_limit_exceeded_ = header_list_size_limit_exceeded; qpack_decoded_headers_accumulator_.reset(); - qpack_decoded_headers_accumulator_reset_reason_ = - QpackDecodedHeadersAccumulatorResetReason::kResetInOnHeadersDecoded; QuicSpdySession::LogHeaderCompressionRatioHistogram( /* using_qpack = */ true, @@ -592,8 +586,6 @@ void QuicSpdyStream::OnHeaderDecodingError(QuicErrorCode error_code, absl::string_view error_message) { qpack_decoded_headers_accumulator_.reset(); - qpack_decoded_headers_accumulator_reset_reason_ = - QpackDecodedHeadersAccumulatorResetReason::kResetInOnHeaderDecodingError; std::string connection_close_error_message = absl::StrCat( "Error decoding ", headers_decompressed_ ? "trailers" : "headers", @@ -741,8 +733,6 @@ spdy_session_->qpack_decoder()) { spdy_session_->qpack_decoder()->OnStreamReset(id()); qpack_decoded_headers_accumulator_.reset(); - qpack_decoded_headers_accumulator_reset_reason_ = - QpackDecodedHeadersAccumulatorResetReason::kResetInOnStreamReset1; } QuicStream::OnStreamReset(frame); @@ -756,8 +746,6 @@ if (!fin_received() && spdy_session_->qpack_decoder()) { spdy_session_->qpack_decoder()->OnStreamReset(id()); qpack_decoded_headers_accumulator_.reset(); - qpack_decoded_headers_accumulator_reset_reason_ = - QpackDecodedHeadersAccumulatorResetReason::kResetInOnStreamReset2; } QuicStream::OnStreamReset(frame); @@ -778,8 +766,6 @@ spdy_session_->qpack_decoder() && web_transport_data_ == nullptr) { spdy_session_->qpack_decoder()->OnStreamReset(id()); qpack_decoded_headers_accumulator_.reset(); - qpack_decoded_headers_accumulator_reset_reason_ = - QpackDecodedHeadersAccumulatorResetReason::kResetInResetWithError; } QuicStream::ResetWithError(error); @@ -881,8 +867,6 @@ QuicStream::OnClose(); qpack_decoded_headers_accumulator_.reset(); - qpack_decoded_headers_accumulator_reset_reason_ = - QpackDecodedHeadersAccumulatorResetReason::kResetInOnClose; if (visitor_) { Visitor* visitor = visitor_; @@ -1076,8 +1060,7 @@ QUICHE_DCHECK(VersionUsesHttp3(transport_version())); if (!qpack_decoded_headers_accumulator_) { - QUIC_BUG(b215142466_OnHeadersFramePayload) - << static_cast<int>(qpack_decoded_headers_accumulator_reset_reason_); + QUIC_BUG(b215142466_OnHeadersFramePayload); OnHeaderDecodingError(QUIC_INTERNAL_ERROR, "qpack_decoded_headers_accumulator_ is nullptr"); return false; @@ -1098,8 +1081,7 @@ QUICHE_DCHECK(VersionUsesHttp3(transport_version())); if (!qpack_decoded_headers_accumulator_) { - QUIC_BUG(b215142466_OnHeadersFrameEnd) - << static_cast<int>(qpack_decoded_headers_accumulator_reset_reason_); + QUIC_BUG(b215142466_OnHeadersFrameEnd); OnHeaderDecodingError(QUIC_INTERNAL_ERROR, "qpack_decoded_headers_accumulator_ is nullptr"); return false;
diff --git a/quiche/quic/core/http/quic_spdy_stream.h b/quiche/quic/core/http/quic_spdy_stream.h index 61cd7e3..1a45682 100644 --- a/quiche/quic/core/http/quic_spdy_stream.h +++ b/quiche/quic/core/http/quic_spdy_stream.h
@@ -336,20 +336,6 @@ WebTransportStreamAdapter adapter; }; - // Reason codes for `qpack_decoded_headers_accumulator_` being nullptr. - enum class QpackDecodedHeadersAccumulatorResetReason { - // `qpack_decoded_headers_accumulator_` was default constructed to nullptr. - kUnSet = 0, - // `qpack_decoded_headers_accumulator_` was reset in the corresponding - // method. - kResetInOnHeadersDecoded = 1, - kResetInOnHeaderDecodingError = 2, - kResetInOnStreamReset1 = 3, - kResetInOnStreamReset2 = 4, - kResetInResetWithError = 5, - kResetInOnClose = 6, - }; - // Called by HttpDecoderVisitor. bool OnDataFrameStart(QuicByteCount header_length, QuicByteCount payload_length); @@ -417,9 +403,6 @@ // Headers accumulator for decoding HEADERS frame payload. std::unique_ptr<QpackDecodedHeadersAccumulator> qpack_decoded_headers_accumulator_; - // Reason for `qpack_decoded_headers_accumulator_` being nullptr. - QpackDecodedHeadersAccumulatorResetReason - qpack_decoded_headers_accumulator_reset_reason_; // Visitor of the HttpDecoder. std::unique_ptr<HttpDecoderVisitor> http_decoder_visitor_; // HttpDecoder for processing raw incoming stream frames.
diff --git a/quiche/quic/core/http/quic_spdy_stream_test.cc b/quiche/quic/core/http/quic_spdy_stream_test.cc index 2bf1ce1..ef695cc 100644 --- a/quiche/quic/core/http/quic_spdy_stream_test.cc +++ b/quiche/quic/core/http/quic_spdy_stream_test.cc
@@ -3195,15 +3195,14 @@ // Resets `qpack_decoded_headers_accumulator_`. stream_->OnHeadersDecoded({}, false); - // This private method should never be called when - // `qpack_decoded_headers_accumulator_` is nullptr. The number 1 identifies - // the site where `qpack_decoded_headers_accumulator_` was last reset. EXPECT_QUIC_BUG( { EXPECT_CALL(*connection_, CloseConnection(_, _, _)); + // This private method should never be called when + // `qpack_decoded_headers_accumulator_` is nullptr. EXPECT_FALSE(QuicSpdyStreamPeer::OnHeadersFrameEnd(stream_)); }, - "b215142466_OnHeadersFrameEnd.?: 1"); + "b215142466_OnHeadersFrameEnd"); } } // namespace