Refactor GOAWAY-related accessors. In Google QUIC, there is a transport layer GOAWAY frame. IETF QUIC does not have such a frame, because it is defined in the application layer HTTP/3. This method prepends a tranport_ prefix to QuicSession's accessors to reflect that they operate on the transport layer GOAWAY frame only. It also replaces QuicSpdySession's accessors with http3_ prefix with generic ones that access information on the appropriate kind of frame determined by the protocol version. Refactor with no functional change PiperOrigin-RevId: 328719322 Change-Id: I028d8e18117fd05b4800beea8181c69a61c239b9
diff --git a/quic/core/http/quic_receive_control_stream_test.cc b/quic/core/http/quic_receive_control_stream_test.cc index 32cd298..d1db3c4 100644 --- a/quic/core/http/quic_receive_control_stream_test.cc +++ b/quic/core/http/quic_receive_control_stream_test.cc
@@ -292,7 +292,7 @@ std::string data = std::string(buffer.get(), header_length); QuicStreamFrame frame(receive_control_stream_->id(), false, offset, data); - EXPECT_FALSE(session_.http3_goaway_received()); + EXPECT_FALSE(session_.goaway_received()); EXPECT_CALL(debug_visitor, OnGoAwayFrameReceived(goaway)); @@ -306,7 +306,7 @@ receive_control_stream_->OnStreamFrame(frame); if (GetQuicReloadableFlag(quic_http3_goaway_new_behavior) || perspective() == Perspective::IS_CLIENT) { - EXPECT_TRUE(session_.http3_goaway_received()); + EXPECT_TRUE(session_.goaway_received()); } }
diff --git a/quic/core/http/quic_spdy_client_session.cc b/quic/core/http/quic_spdy_client_session.cc index c5ef0c4..d7a7b03 100644 --- a/quic/core/http/quic_spdy_client_session.cc +++ b/quic/core/http/quic_spdy_client_session.cc
@@ -57,10 +57,7 @@ QUIC_DLOG(INFO) << "Encryption not active so no outgoing stream created."; return false; } - bool goaway_received = VersionUsesHttp3(transport_version()) - ? http3_goaway_received() - : QuicSession::goaway_received(); - if (goaway_received && respect_goaway_) { + if (goaway_received() && respect_goaway_) { QUIC_DLOG(INFO) << "Failed to create a new outgoing stream. " << "Already received goaway."; return false; @@ -131,10 +128,7 @@ QUIC_BUG << "ShouldCreateIncomingStream called when disconnected"; return false; } - bool goaway_received = quic::VersionUsesHttp3(transport_version()) - ? http3_goaway_received() - : QuicSession::goaway_received(); - if (goaway_received && respect_goaway_) { + if (goaway_received() && respect_goaway_) { QUIC_DLOG(INFO) << "Failed to create a new outgoing stream. " << "Already received goaway."; return false;
diff --git a/quic/core/http/quic_spdy_session.cc b/quic/core/http/quic_spdy_session.cc index 3345fea..6b3d0c8 100644 --- a/quic/core/http/quic_spdy_session.cc +++ b/quic/core/http/quic_spdy_session.cc
@@ -1403,6 +1403,17 @@ ietf_server_push_enabled_ = true; } +bool QuicSpdySession::goaway_received() const { + return VersionUsesHttp3(transport_version()) + ? last_received_http3_goaway_id_.has_value() + : transport_goaway_received(); +} + +bool QuicSpdySession::goaway_sent() const { + return VersionUsesHttp3(transport_version()) ? http3_goaway_sent_ + : transport_goaway_sent(); +} + bool QuicSpdySession::CanCreatePushStreamWithId(PushId push_id) { DCHECK(VersionUsesHttp3(transport_version()));
diff --git a/quic/core/http/quic_spdy_session.h b/quic/core/http/quic_spdy_session.h index 4c50e7f..466cc96 100644 --- a/quic/core/http/quic_spdy_session.h +++ b/quic/core/http/quic_spdy_session.h
@@ -353,11 +353,12 @@ Http3DebugVisitor* debug_visitor() { return debug_visitor_; } - bool http3_goaway_received() const { - return last_received_http3_goaway_id_.has_value(); - } - - bool http3_goaway_sent() const { return http3_goaway_sent_; } + // When using Google QUIC, return whether a transport layer GOAWAY frame has + // been received or sent. + // When using IETF QUIC, return whether an HTTP/3 GOAWAY frame has been + // received or sent. + bool goaway_received() const; + bool goaway_sent() const; // Log header compression ratio histogram. // |using_qpack| is true for QPACK, false for HPACK.
diff --git a/quic/core/http/quic_spdy_session_test.cc b/quic/core/http/quic_spdy_session_test.cc index fd0c645..0ff48c1 100644 --- a/quic/core/http/quic_spdy_session_test.cc +++ b/quic/core/http/quic_spdy_session_test.cc
@@ -1082,7 +1082,7 @@ .WillOnce(Return(WriteResult(WRITE_STATUS_OK, 0))); EXPECT_CALL(debug_visitor, OnGoAwayFrameSent(_)); session_.SendHttp3GoAway(); - EXPECT_TRUE(session_.http3_goaway_sent()); + EXPECT_TRUE(session_.goaway_sent()); const QuicStreamId kTestStreamId = GetNthClientInitiatedBidirectionalStreamId(transport_version(), 0); @@ -1120,10 +1120,10 @@ return; } - EXPECT_FALSE(session_.http3_goaway_received()); + EXPECT_FALSE(session_.goaway_received()); PushId push_id1 = 0; session_.OnHttp3GoAway(push_id1); - EXPECT_TRUE(session_.http3_goaway_received()); + EXPECT_TRUE(session_.goaway_received()); EXPECT_CALL( *connection_, @@ -2703,11 +2703,11 @@ return; } - EXPECT_FALSE(session_.http3_goaway_received()); + EXPECT_FALSE(session_.goaway_received()); QuicStreamId stream_id1 = GetNthClientInitiatedBidirectionalStreamId(transport_version(), 0); session_.OnHttp3GoAway(stream_id1); - EXPECT_TRUE(session_.http3_goaway_received()); + EXPECT_TRUE(session_.goaway_received()); EXPECT_CALL( *connection_,
diff --git a/quic/core/quic_session.cc b/quic/core/quic_session.cc index c656459..8989269 100644 --- a/quic/core/quic_session.cc +++ b/quic/core/quic_session.cc
@@ -89,8 +89,8 @@ perspective() == Perspective::IS_SERVER, nullptr), currently_writing_stream_id_(0), - goaway_sent_(false), - goaway_received_(false), + transport_goaway_sent_(false), + transport_goaway_received_(false), control_frame_manager_(this), last_message_id_(0), datagram_queue_(this), @@ -345,7 +345,7 @@ QUIC_BUG_IF(version().UsesHttp3()) << "gQUIC GOAWAY received on version " << version(); - goaway_received_ = true; + transport_goaway_received_ = true; } void QuicSession::OnMessageReceived(quiche::QuicheStringPiece message) { @@ -832,10 +832,10 @@ const std::string& reason) { // GOAWAY frame is not supported in v99. DCHECK(!VersionHasIetfQuicFrames(transport_version())); - if (goaway_sent_) { + if (transport_goaway_sent_) { return; } - goaway_sent_ = true; + transport_goaway_sent_ = true; control_frame_manager_.WriteOrBufferGoAway( error_code, stream_id_manager_.largest_peer_created_stream_id(), reason); } @@ -1794,10 +1794,10 @@ return nullptr; } - // TODO(fkastenholz): If we are creating a new stream and we have - // sent a goaway, we should ignore the stream creation. Need to - // add code to A) test if goaway was sent ("if (goaway_sent_)") and - // B) reject stream creation ("return nullptr") + // TODO(fkastenholz): If we are creating a new stream and we have sent a + // goaway, we should ignore the stream creation. Need to add code to A) test + // if goaway was sent ("if (transport_goaway_sent_)") and B) reject stream + // creation ("return nullptr") if (!MaybeIncreaseLargestPeerStreamId(stream_id)) { return nullptr;
diff --git a/quic/core/quic_session.h b/quic/core/quic_session.h index 4e59dc0..bb5107a 100644 --- a/quic/core/quic_session.h +++ b/quic/core/quic_session.h
@@ -366,9 +366,9 @@ // connection ID lengths do not change. QuicPacketLength GetGuaranteedLargestMessagePayload() const; - bool goaway_sent() const { return goaway_sent_; } + bool transport_goaway_sent() const { return transport_goaway_sent_; } - bool goaway_received() const { return goaway_received_; } + bool transport_goaway_received() const { return transport_goaway_received_; } // Returns the Google QUIC error code QuicErrorCode error() const { return on_closed_frame_.quic_error_code; } @@ -804,11 +804,15 @@ // call stack of OnCanWrite. QuicStreamId currently_writing_stream_id_; - // Whether a GoAway has been sent. - bool goaway_sent_; + // Whether a transport layer GOAWAY frame has been sent. + // Such a frame only exists in Google QUIC, therefore |transport_goaway_sent_| + // is always false when using IETF QUIC. + bool transport_goaway_sent_; - // Whether a GoAway has been received. - bool goaway_received_; + // Whether a transport layer GOAWAY frame has been received. + // Such a frame only exists in Google QUIC, therefore + // |transport_goaway_received_| is always false when using IETF QUIC. + bool transport_goaway_received_; QuicControlFrameManager control_frame_manager_;
diff --git a/quic/core/quic_session_test.cc b/quic/core/quic_session_test.cc index 6c2b867..37f1305 100644 --- a/quic/core/quic_session_test.cc +++ b/quic/core/quic_session_test.cc
@@ -1424,7 +1424,7 @@ .WillOnce( Invoke(connection_, &MockQuicConnection::ReallySendControlFrame)); session_.SendGoAway(QUIC_PEER_GOING_AWAY, "Going Away."); - EXPECT_TRUE(session_.goaway_sent()); + EXPECT_TRUE(session_.transport_goaway_sent()); const QuicStreamId kTestStreamId = 5u; EXPECT_CALL(*connection_, SendControlFrame(_)).Times(0); @@ -1442,7 +1442,7 @@ EXPECT_CALL(*connection_, SendControlFrame(_)) .WillOnce(Invoke(&ClearControlFrame)); session_.SendGoAway(QUIC_PEER_GOING_AWAY, "Going Away."); - EXPECT_TRUE(session_.goaway_sent()); + EXPECT_TRUE(session_.transport_goaway_sent()); session_.SendGoAway(QUIC_PEER_GOING_AWAY, "Going Away."); }
diff --git a/quic/test_tools/quic_test_server.cc b/quic/test_tools/quic_test_server.cc index 85a86b7..f311079 100644 --- a/quic/test_tools/quic_test_server.cc +++ b/quic/test_tools/quic_test_server.cc
@@ -252,7 +252,7 @@ QuicSimpleServerSession::OnNewEncryptionKeyAvailable(level, std::move(encrypter)); if (VersionUsesHttp3(transport_version())) { - if (IsEncryptionEstablished() && !http3_goaway_sent()) { + if (IsEncryptionEstablished() && !goaway_sent()) { SendHttp3GoAway(); } }
diff --git a/quic/tools/quic_client_base.cc b/quic/tools/quic_client_base.cc index 2895130..c1d2ea2 100644 --- a/quic/tools/quic_client_base.cc +++ b/quic/tools/quic_client_base.cc
@@ -286,7 +286,7 @@ } bool QuicClientBase::goaway_received() const { - return session_ != nullptr && session_->goaway_received(); + return session_ != nullptr && session_->transport_goaway_received(); } int QuicClientBase::GetNumSentClientHellos() {
diff --git a/quic/tools/quic_client_base.h b/quic/tools/quic_client_base.h index ff600ae..4df3a2b 100644 --- a/quic/tools/quic_client_base.h +++ b/quic/tools/quic_client_base.h
@@ -128,7 +128,7 @@ const QuicSession* session() const; bool connected() const; - bool goaway_received() const; + virtual bool goaway_received() const; const QuicServerId& server_id() const { return server_id_; }
diff --git a/quic/tools/quic_spdy_client_base.cc b/quic/tools/quic_spdy_client_base.cc index 09de813..309928f 100644 --- a/quic/tools/quic_spdy_client_base.cc +++ b/quic/tools/quic_spdy_client_base.cc
@@ -195,6 +195,10 @@ return stream; } +bool QuicSpdyClientBase::goaway_received() const { + return client_session() && client_session()->goaway_received(); +} + bool QuicSpdyClientBase::EarlyDataAccepted() { return client_session()->EarlyDataAccepted(); }
diff --git a/quic/tools/quic_spdy_client_base.h b/quic/tools/quic_spdy_client_base.h index 874aa2d..fdd67fb 100644 --- a/quic/tools/quic_spdy_client_base.h +++ b/quic/tools/quic_spdy_client_base.h
@@ -143,6 +143,8 @@ // TODO(b/151641466): Rename this method. void SetMaxAllowedPushId(PushId max) { max_allowed_push_id_ = max; } + // QuicClientBase methods. + bool goaway_received() const override; bool EarlyDataAccepted() override; bool ReceivedInchoateReject() override;