gfe-relnote: Refactor QuicConnectionCloseFrame. Not protected. QuicConnectionCloseFrame::quic_error_code should not be accessed when using IETF QUIC, however, as it is a public member, this is not enforced. When read, it returns the on-the-wire error code (transport or application) cast into a QuicErrorCode. This is bad. Since public unions cannot enforce restrictions on access, I propose to remove the union for clarity. The price paid is type: wire_error_code will now be uint64_t even for Google QUIC error codes which could be QuicErrorCode, and for IETF QUIC transport errors, which could be QuicIetfTransportErrorCodes. Also rename extracted_error_code to quic_error_code for clarity, because for sent frames, it is not extracted. This refactor is necessary for modifying QuicErrorCodeToTransportErrorCode() to return IETF wire error codes. When using Google QUIC, old fields |quic_error_code| and |extracted_error_code| and new fields |wire_error_code| and |quic_error_code| should always have the same value, therefore this refactor should not introduce any functional change. PiperOrigin-RevId: 304902883 Change-Id: I586a92609fb8a6186310f6585c8d29ee1d60af4f
diff --git a/quic/core/quic_connection_test.cc b/quic/core/quic_connection_test.cc index e5faa35..ded114b 100644 --- a/quic/core/quic_connection_test.cc +++ b/quic/core/quic_connection_test.cc
@@ -1670,8 +1670,11 @@ const std::vector<QuicConnectionCloseFrame>& connection_close_frames = writer_->connection_close_frames(); ASSERT_EQ(1u, connection_close_frames.size()); + + EXPECT_EQ(expected_code, connection_close_frames[0].quic_error_code); + if (!VersionHasIetfQuicFrames(version().transport_version)) { - EXPECT_EQ(expected_code, connection_close_frames[0].quic_error_code); + EXPECT_EQ(expected_code, connection_close_frames[0].wire_error_code); EXPECT_EQ(GOOGLE_QUIC_CONNECTION_CLOSE, connection_close_frames[0].close_type); return; @@ -1685,16 +1688,13 @@ EXPECT_EQ(IETF_QUIC_TRANSPORT_CONNECTION_CLOSE, connection_close_frames[0].close_type); EXPECT_EQ(mapping.transport_error_code_, - connection_close_frames[0].transport_error_code); - // TODO(fkastenholz): when the extracted error code CL lands, - // need to test that extracted==expected. + connection_close_frames[0].wire_error_code); } else { // This maps to an application close. - EXPECT_EQ(expected_code, connection_close_frames[0].quic_error_code); EXPECT_EQ(IETF_QUIC_APPLICATION_CONNECTION_CLOSE, connection_close_frames[0].close_type); - // TODO(fkastenholz): when the extracted error code CL lands, - // need to test that extracted==expected. + EXPECT_EQ(mapping.application_error_code_, + connection_close_frames[0].wire_error_code); } } @@ -7621,7 +7621,7 @@ kSelfAddress, kPeerAddress, QuicReceivedPacket(buffer, encrypted_length, QuicTime::Zero(), false)); EXPECT_EQ(1, connection_close_frame_count_); - EXPECT_THAT(saved_connection_close_frame_.extracted_error_code, + EXPECT_THAT(saved_connection_close_frame_.quic_error_code, IsError(QUIC_PEER_GOING_AWAY)); } @@ -10134,7 +10134,7 @@ ASSERT_EQ(1u, connection_close_frames.size()); EXPECT_EQ(IETF_QUIC_TRANSPORT_CONNECTION_CLOSE, connection_close_frames[0].close_type); - EXPECT_EQ(kQuicErrorCode, connection_close_frames[0].extracted_error_code); + EXPECT_EQ(kQuicErrorCode, connection_close_frames[0].quic_error_code); EXPECT_EQ(kTransportCloseFrameType, connection_close_frames[0].transport_close_frame_type); }