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;
