Remove OnApplicationClose upcalls, update OnConnectionClose as needed.
Replaces the OnApplicationClose visitor upcalls with OnConnectionClose. The IETF removes the separate Application Close frame from QUIC, modifying the Connection Close frame to have two flavors; one for Application-initiated reasons, the other for Transport reasons. These were formerly the Application- and Connection- closes.
gfe-relnote: N/A this is only for IETF QUIC (version 99).
PiperOrigin-RevId: 243856589
Change-Id: Ie048aa2a8641f7e3f84b6cd7e4fc6ce61d5683e7
diff --git a/quic/core/chlo_extractor.cc b/quic/core/chlo_extractor.cc
index b2d11e0..4ce5d8c 100644
--- a/quic/core/chlo_extractor.cc
+++ b/quic/core/chlo_extractor.cc
@@ -52,7 +52,6 @@
bool OnPingFrame(const QuicPingFrame& frame) override;
bool OnRstStreamFrame(const QuicRstStreamFrame& frame) override;
bool OnConnectionCloseFrame(const QuicConnectionCloseFrame& frame) override;
- bool OnApplicationCloseFrame(const QuicConnectionCloseFrame& frame) override;
bool OnNewConnectionIdFrame(const QuicNewConnectionIdFrame& frame) override;
bool OnRetireConnectionIdFrame(
const QuicRetireConnectionIdFrame& frame) override;
@@ -211,11 +210,6 @@
return true;
}
-bool ChloFramerVisitor::OnApplicationCloseFrame(
- const QuicConnectionCloseFrame& frame) {
- return true;
-}
-
bool ChloFramerVisitor::OnStopSendingFrame(const QuicStopSendingFrame& frame) {
return true;
}
diff --git a/quic/core/quic_connection.cc b/quic/core/quic_connection.cc
index efc89d3..cb901ba 100644
--- a/quic/core/quic_connection.cc
+++ b/quic/core/quic_connection.cc
@@ -1261,15 +1261,6 @@
return connected_;
}
-bool QuicConnection::OnApplicationCloseFrame(
- const QuicConnectionCloseFrame& frame) {
- // TODO(fkastenholz): Need to figure out what the right thing is to do with
- // this when we get one. Most likely, the correct action is to mimic the
- // OnConnectionCloseFrame actions, with possibly an indication to the
- // application of the ApplicationClose information.
- return true;
-}
-
bool QuicConnection::OnStopSendingFrame(const QuicStopSendingFrame& frame) {
DCHECK(connected_);
@@ -3053,6 +3044,8 @@
}
QuicConnectionCloseFrame* frame =
new QuicConnectionCloseFrame(error, details);
+ // If version99/IETF QUIC set the close type. Default close type is Google
+ // QUIC.
if (transport_version() == QUIC_VERSION_99) {
frame->close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE;
}
diff --git a/quic/core/quic_connection.h b/quic/core/quic_connection.h
index d13458d..c921d1b 100644
--- a/quic/core/quic_connection.h
+++ b/quic/core/quic_connection.h
@@ -250,13 +250,11 @@
// Called when a RstStreamFrame has been parsed.
virtual void OnRstStreamFrame(const QuicRstStreamFrame& frame) {}
- // Called when a ConnectionCloseFrame has been parsed.
+ // Called when a ConnectionCloseFrame has been parsed. All forms
+ // of CONNECTION CLOSE are handled, Google QUIC, IETF QUIC
+ // CONNECTION CLOSE/Transport and IETF QUIC CONNECTION CLOSE/Application
virtual void OnConnectionCloseFrame(const QuicConnectionCloseFrame& frame) {}
- // Called when an IETF QUIC CONNECTION_CLOSE/Application Frame has been
- // parsed.
- virtual void OnApplicationCloseFrame(const QuicConnectionCloseFrame& frame) {}
-
// Called when a WindowUpdate has been parsed.
virtual void OnWindowUpdateFrame(const QuicWindowUpdateFrame& frame,
const QuicTime& receive_time) {}
@@ -498,7 +496,6 @@
bool OnPingFrame(const QuicPingFrame& frame) override;
bool OnRstStreamFrame(const QuicRstStreamFrame& frame) override;
bool OnConnectionCloseFrame(const QuicConnectionCloseFrame& frame) override;
- bool OnApplicationCloseFrame(const QuicConnectionCloseFrame& frame) override;
bool OnStopSendingFrame(const QuicStopSendingFrame& frame) override;
bool OnPathChallengeFrame(const QuicPathChallengeFrame& frame) override;
bool OnPathResponseFrame(const QuicPathResponseFrame& frame) override;
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc
index 4072ecd..56b0453 100644
--- a/quic/core/quic_connection_test.cc
+++ b/quic/core/quic_connection_test.cc
@@ -1368,6 +1368,8 @@
QuicConnectionCloseFrame qccf(QUIC_PEER_GOING_AWAY);
if (peer_framer_.transport_version() == QUIC_VERSION_99) {
+ // Default close-type is Google QUIC. If doing IETF/V99 then
+ // set close type to be IETF CC/T.
qccf.close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE;
}
@@ -7233,6 +7235,8 @@
QuicConnectionCloseFrame qccf(QUIC_PEER_GOING_AWAY);
if (peer_framer_.transport_version() == QUIC_VERSION_99) {
+ // Default close-type is Google QUIC. If doing IETF/V99 then
+ // set close type to be IETF CC/T.
qccf.close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE;
}
diff --git a/quic/core/quic_dispatcher.cc b/quic/core/quic_dispatcher.cc
index 57924ca..75b3c30 100644
--- a/quic/core/quic_dispatcher.cc
+++ b/quic/core/quic_dispatcher.cc
@@ -920,12 +920,6 @@
return false;
}
-bool QuicDispatcher::OnApplicationCloseFrame(
- const QuicConnectionCloseFrame& /*frame*/) {
- DCHECK(false);
- return false;
-}
-
bool QuicDispatcher::OnMaxStreamIdFrame(const QuicMaxStreamIdFrame& frame) {
return true;
}
diff --git a/quic/core/quic_dispatcher.h b/quic/core/quic_dispatcher.h
index 62b7ebb..a6d7e37 100644
--- a/quic/core/quic_dispatcher.h
+++ b/quic/core/quic_dispatcher.h
@@ -165,7 +165,6 @@
bool OnPingFrame(const QuicPingFrame& frame) override;
bool OnRstStreamFrame(const QuicRstStreamFrame& frame) override;
bool OnConnectionCloseFrame(const QuicConnectionCloseFrame& frame) override;
- bool OnApplicationCloseFrame(const QuicConnectionCloseFrame& frame) override;
bool OnStopSendingFrame(const QuicStopSendingFrame& frame) override;
bool OnPathChallengeFrame(const QuicPathChallengeFrame& frame) override;
bool OnPathResponseFrame(const QuicPathResponseFrame& frame) override;
diff --git a/quic/core/quic_error_codes.cc b/quic/core/quic_error_codes.cc
index a5bef73..e2a14ad 100644
--- a/quic/core/quic_error_codes.cc
+++ b/quic/core/quic_error_codes.cc
@@ -135,7 +135,6 @@
RETURN_STRING_LITERAL(QUIC_STREAM_LENGTH_OVERFLOW);
RETURN_STRING_LITERAL(QUIC_CONNECTION_MIGRATION_DISABLED_BY_CONFIG);
RETURN_STRING_LITERAL(QUIC_CONNECTION_MIGRATION_INTERNAL_ERROR);
- RETURN_STRING_LITERAL(QUIC_INVALID_APPLICATION_CLOSE_DATA);
RETURN_STRING_LITERAL(QUIC_INVALID_MAX_DATA_FRAME_DATA);
RETURN_STRING_LITERAL(QUIC_INVALID_MAX_STREAM_DATA_FRAME_DATA);
RETURN_STRING_LITERAL(QUIC_INVALID_STREAM_BLOCKED_DATA);
diff --git a/quic/core/quic_error_codes.h b/quic/core/quic_error_codes.h
index 2004a08..b06cc7f 100644
--- a/quic/core/quic_error_codes.h
+++ b/quic/core/quic_error_codes.h
@@ -286,8 +286,6 @@
// Receive a RST_STREAM with offset larger than kMaxStreamLength.
QUIC_STREAM_LENGTH_OVERFLOW = 98,
- // APPLICATION_CLOSE frame data is malformed.
- QUIC_INVALID_APPLICATION_CLOSE_DATA = 101,
// Received a MAX DATA frame with errors.
QUIC_INVALID_MAX_DATA_FRAME_DATA = 102,
// Received a MAX STREAM DATA frame with errors.
diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index 0e30453..583291e 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -2915,10 +2915,15 @@
}
break;
}
+ case IETF_APPLICATION_CLOSE:
case IETF_CONNECTION_CLOSE: {
QuicConnectionCloseFrame frame;
if (!ProcessIetfConnectionCloseFrame(
- reader, IETF_QUIC_TRANSPORT_CONNECTION_CLOSE, &frame)) {
+ reader,
+ (frame_type == IETF_CONNECTION_CLOSE)
+ ? IETF_QUIC_TRANSPORT_CONNECTION_CLOSE
+ : IETF_QUIC_APPLICATION_CONNECTION_CLOSE,
+ &frame)) {
return RaiseError(QUIC_INVALID_CONNECTION_CLOSE_DATA);
}
if (!visitor_->OnConnectionCloseFrame(frame)) {
@@ -2928,19 +2933,6 @@
}
break;
}
- case IETF_APPLICATION_CLOSE: {
- QuicConnectionCloseFrame frame;
- if (!ProcessIetfConnectionCloseFrame(
- reader, IETF_QUIC_APPLICATION_CONNECTION_CLOSE, &frame)) {
- return RaiseError(QUIC_INVALID_APPLICATION_CLOSE_DATA);
- }
- if (!visitor_->OnApplicationCloseFrame(frame)) {
- QUIC_DVLOG(1) << "Visitor asked to stop further processing.";
- // Returning true since there was no parsing error.
- return true;
- }
- break;
- }
case IETF_MAX_DATA: {
QuicWindowUpdateFrame frame;
if (!ProcessMaxDataFrame(reader, &frame)) {
diff --git a/quic/core/quic_framer.h b/quic/core/quic_framer.h
index 8d2fd93..90ef998 100644
--- a/quic/core/quic_framer.h
+++ b/quic/core/quic_framer.h
@@ -154,17 +154,10 @@
// Called when a RstStreamFrame has been parsed.
virtual bool OnRstStreamFrame(const QuicRstStreamFrame& frame) = 0;
- // Called when a ConnectionCloseFrame has been parsed.
+ // Called when a ConnectionCloseFrame, of any type, has been parsed.
virtual bool OnConnectionCloseFrame(
const QuicConnectionCloseFrame& frame) = 0;
- // Called when an IETF QUIC CONNECTION_CLOSE/Application Frame has been
- // parsed. TEMPORARILY pass the frame as a QuicConnectionCloseFrame.
- // TODO(fkastenholz): remove when V99 fully utilizes the IETF
- // CONNECTION_CLOSE close frame.
- virtual bool OnApplicationCloseFrame(
- const QuicConnectionCloseFrame& frame) = 0;
-
// Called when a StopSendingFrame has been parsed.
virtual bool OnStopSendingFrame(const QuicStopSendingFrame& frame) = 0;
diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index 4cb837e..2ffe54e 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -315,15 +315,6 @@
return true;
}
- // TODO(fkastenholz): Remove when IETF QUIC is completely converted to
- // doing Connection Close.
- bool OnApplicationCloseFrame(const QuicConnectionCloseFrame& frame) override {
- // OnApplicationCloseFrame receives a QuicConnectionCloseFrame as part of
- // migration to IETF QUIC's new model.
- connection_close_frame_ = frame;
- return true;
- }
-
bool OnStopSendingFrame(const QuicStopSendingFrame& frame) override {
stop_sending_frame_ = frame;
return true;
@@ -4452,6 +4443,7 @@
CheckFramingBoundaries(fragments, QUIC_INVALID_CONNECTION_CLOSE_DATA);
}
+// Test the CONNECTION_CLOSE/Application variant.
TEST_P(QuicFramerTest, ApplicationCloseFrame) {
if (framer_.transport_version() != QUIC_VERSION_99) {
// This frame does not exist in versions other than 99.
@@ -4509,7 +4501,7 @@
ASSERT_EQ(0u, visitor_.ack_frames_.size());
- CheckFramingBoundaries(packet99, QUIC_INVALID_APPLICATION_CLOSE_DATA);
+ CheckFramingBoundaries(packet99, QUIC_INVALID_CONNECTION_CLOSE_DATA);
}
TEST_P(QuicFramerTest, GoAwayFrame) {
diff --git a/quic/core/quic_ietf_framer_test.cc b/quic/core/quic_ietf_framer_test.cc
index 987bce9..b0c0125 100644
--- a/quic/core/quic_ietf_framer_test.cc
+++ b/quic/core/quic_ietf_framer_test.cc
@@ -155,12 +155,6 @@
return true;
}
- // TODO(fkastenholz): remove when finishing conversion to new IETF QUIC
- // model.
- bool OnApplicationCloseFrame(const QuicConnectionCloseFrame& frame) override {
- return true;
- }
-
bool OnStopSendingFrame(const QuicStopSendingFrame& frame) override {
return true;
}
diff --git a/quic/core/quic_types.h b/quic/core/quic_types.h
index f90b0fa..e3a40cf 100644
--- a/quic/core/quic_types.h
+++ b/quic/core/quic_types.h
@@ -254,8 +254,7 @@
IETF_PATH_RESPONSE = 0x1b,
// Both of the following are "Connection Close" frames,
// the first signals transport-layer errors, the second application-layer
- // errors.The frame formats and protocol procedures are the same, the only
- // difference is the number space in the frame's error code field.
+ // errors.
IETF_CONNECTION_CLOSE = 0x1c,
IETF_APPLICATION_CLOSE = 0x1d,
diff --git a/quic/test_tools/quic_test_utils.cc b/quic/test_tools/quic_test_utils.cc
index 2de1491..870a7b8 100644
--- a/quic/test_tools/quic_test_utils.cc
+++ b/quic/test_tools/quic_test_utils.cc
@@ -195,9 +195,6 @@
ON_CALL(*this, OnConnectionCloseFrame(_))
.WillByDefault(testing::Return(true));
- ON_CALL(*this, OnApplicationCloseFrame(_))
- .WillByDefault(testing::Return(true));
-
ON_CALL(*this, OnStopSendingFrame(_)).WillByDefault(testing::Return(true));
ON_CALL(*this, OnPathChallengeFrame(_)).WillByDefault(testing::Return(true));
@@ -281,11 +278,6 @@
return true;
}
-bool NoOpFramerVisitor::OnApplicationCloseFrame(
- const QuicConnectionCloseFrame& frame) {
- return true;
-}
-
bool NoOpFramerVisitor::OnNewConnectionIdFrame(
const QuicNewConnectionIdFrame& frame) {
return true;
diff --git a/quic/test_tools/quic_test_utils.h b/quic/test_tools/quic_test_utils.h
index 561362a..c888fef 100644
--- a/quic/test_tools/quic_test_utils.h
+++ b/quic/test_tools/quic_test_utils.h
@@ -269,8 +269,6 @@
MOCK_METHOD1(OnRstStreamFrame, bool(const QuicRstStreamFrame& frame));
MOCK_METHOD1(OnConnectionCloseFrame,
bool(const QuicConnectionCloseFrame& frame));
- MOCK_METHOD1(OnApplicationCloseFrame,
- bool(const QuicConnectionCloseFrame& frame));
MOCK_METHOD1(OnNewConnectionIdFrame,
bool(const QuicNewConnectionIdFrame& frame));
MOCK_METHOD1(OnRetireConnectionIdFrame,
@@ -323,7 +321,6 @@
bool OnPingFrame(const QuicPingFrame& frame) override;
bool OnRstStreamFrame(const QuicRstStreamFrame& frame) override;
bool OnConnectionCloseFrame(const QuicConnectionCloseFrame& frame) override;
- bool OnApplicationCloseFrame(const QuicConnectionCloseFrame& frame) override;
bool OnNewConnectionIdFrame(const QuicNewConnectionIdFrame& frame) override;
bool OnRetireConnectionIdFrame(
const QuicRetireConnectionIdFrame& frame) override;
@@ -1004,8 +1001,6 @@
MOCK_METHOD1(OnConnectionCloseFrame, void(const QuicConnectionCloseFrame&));
- MOCK_METHOD1(OnApplicationCloseFrame, void(const QuicConnectionCloseFrame&));
-
MOCK_METHOD1(OnStopSendingFrame, void(const QuicStopSendingFrame&));
MOCK_METHOD1(OnPathChallengeFrame, void(const QuicPathChallengeFrame&));
diff --git a/quic/test_tools/simple_quic_framer.cc b/quic/test_tools/simple_quic_framer.cc
index 7b6e753..6a90a79 100644
--- a/quic/test_tools/simple_quic_framer.cc
+++ b/quic/test_tools/simple_quic_framer.cc
@@ -125,11 +125,6 @@
return true;
}
- bool OnApplicationCloseFrame(const QuicConnectionCloseFrame& frame) override {
- application_close_frames_.push_back(frame);
- return true;
- }
-
bool OnNewConnectionIdFrame(const QuicNewConnectionIdFrame& frame) override {
new_connection_id_frames_.push_back(frame);
return true;
@@ -207,10 +202,7 @@
const std::vector<QuicConnectionCloseFrame>& connection_close_frames() const {
return connection_close_frames_;
}
- const std::vector<QuicConnectionCloseFrame>& application_close_frames()
- const {
- return application_close_frames_;
- }
+
const std::vector<QuicGoAwayFrame>& goaway_frames() const {
return goaway_frames_;
}
@@ -272,7 +264,6 @@
std::vector<QuicStreamIdBlockedFrame> stream_id_blocked_frames_;
std::vector<QuicMaxStreamIdFrame> max_stream_id_frames_;
std::vector<QuicConnectionCloseFrame> connection_close_frames_;
- std::vector<QuicConnectionCloseFrame> application_close_frames_;
std::vector<QuicStopSendingFrame> stop_sending_frames_;
std::vector<QuicPathChallengeFrame> path_challenge_frames_;
std::vector<QuicPathResponseFrame> path_response_frames_;
diff --git a/quic/tools/quic_packet_printer_bin.cc b/quic/tools/quic_packet_printer_bin.cc
index 25c7b67..c8cfc11 100644
--- a/quic/tools/quic_packet_printer_bin.cc
+++ b/quic/tools/quic_packet_printer_bin.cc
@@ -133,14 +133,10 @@
return true;
}
bool OnConnectionCloseFrame(const QuicConnectionCloseFrame& frame) override {
- std::cerr << "OnConnectionCloseFrame: " << frame;
- return true;
- }
- bool OnApplicationCloseFrame(const QuicConnectionCloseFrame& frame) override {
// The frame printout will indicate whether it's a Google QUIC
// CONNECTION_CLOSE, IETF QUIC CONNECTION_CLOSE/Transport, or IETF QUIC
// CONNECTION_CLOSE/Application frame.
- std::cerr << "OnApplicationCloseFrame: " << frame;
+ std::cerr << "OnConnectionCloseFrame: " << frame;
return true;
}
bool OnNewConnectionIdFrame(const QuicNewConnectionIdFrame& frame) override {
