commit a037b8b34eab00e5208e67685ff82c7cf5133c1c
author fkastenholz <fkastenholz@google.com> Tue May 07 06:00:05 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Tue May 07 07:22:45 2019 -0700
tree 33c2a81e845b45c5d079665bfff08eeb003c7ebb
parent e3954c2cc62252cee03ba82a703a1a9ec3fc9dfa

Refactor QuicFramer::Get...ConnectionCloseFrameSize

This CL refactors QUicFramer::Get...ConnectionCloseFrameSize in
preparation for a following CL to add in additional IETF QUIC
CLOSE_CONNECTION functionality.

gfe-relnote: Not flag protected, refactor existing method.
PiperOrigin-RevId: 247005149
Change-Id: Ibb339a6e16f9f16c842cdbc2026563c25f93e44e

quic/core/quic_framer.cc

diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index 0e026c6..67e3447 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -554,31 +554,31 @@
 }
 
 // static
-size_t QuicFramer::GetMinConnectionCloseFrameSize(
+size_t QuicFramer::GetConnectionCloseFrameSize(
     QuicTransportVersion version,
     const QuicConnectionCloseFrame& frame) {
-  if (version == QUIC_VERSION_99) {
-    // TODO(fkastenholz): For complete support of IETF QUIC CONNECTION_CLOSE,
-    // check if the frame is a Transport close and if the frame's
-    // extracted_error_code is not QUIC_IETF_GQUIC_ERROR_MISSING. If so,
-    // extend the error string to include " QuicErrorCode: #"
-    if (frame.close_type == IETF_QUIC_APPLICATION_CONNECTION_CLOSE) {
-      // Application close variant does not include the transport close frame
-      // type field.
-      return QuicDataWriter::GetVarInt62Len(
-                 TruncatedErrorStringSize(frame.error_details)) +
-             kQuicFrameTypeSize + kQuicIetfQuicErrorCodeSize;
-    }
-    QUIC_BUG_IF(frame.close_type != IETF_QUIC_TRANSPORT_CONNECTION_CLOSE)
-        << "IETF QUIC Connection close and QuicConnectionCloseFrame type is "
-           "not IETF ConnectionClose";
-    return QuicDataWriter::GetVarInt62Len(
-               TruncatedErrorStringSize(frame.error_details)) +
-           QuicDataWriter::GetVarInt62Len(frame.transport_close_frame_type) +
-           kQuicFrameTypeSize + kQuicIetfQuicErrorCodeSize;
+  if (version != QUIC_VERSION_99) {
+    // Not version 99/IETF QUIC, return Google QUIC CONNECTION CLOSE frame size.
+    return kQuicFrameTypeSize + kQuicErrorCodeSize +
+           kQuicErrorDetailsLengthSize +
+           TruncatedErrorStringSize(frame.error_details);
   }
-  // Not version 99/IETF QUIC, return Google QUIC CONNECTION CLOSE frame size.
-  return kQuicFrameTypeSize + kQuicErrorCodeSize + kQuicErrorDetailsLengthSize;
+  // TODO(fkastenholz): For complete support of IETF QUIC CONNECTION_CLOSE,
+  // check if the frame is a Transport close and if the frame's
+  // extracted_error_code is not QUIC_IETF_GQUIC_ERROR_MISSING. If so,
+  // extend the error string to include " QuicErrorCode: #"
+  const size_t truncated_error_string_size =
+      TruncatedErrorStringSize(frame.error_details);
+  const size_t frame_size =
+      truncated_error_string_size +
+      QuicDataWriter::GetVarInt62Len(truncated_error_string_size) +
+      kQuicFrameTypeSize + kQuicIetfQuicErrorCodeSize;
+  if (frame.close_type == IETF_QUIC_APPLICATION_CONNECTION_CLOSE) {
+    return frame_size;
+  }
+  // frame includes the transport_close_frame_type, so include its length.
+  return frame_size +
+         QuicDataWriter::GetVarInt62Len(frame.transport_close_frame_type);
 }
 
 // static
@@ -674,10 +674,8 @@
     case RST_STREAM_FRAME:
       return GetRstStreamFrameSize(version, *frame.rst_stream_frame);
     case CONNECTION_CLOSE_FRAME:
-      return GetMinConnectionCloseFrameSize(version,
-                                            *frame.connection_close_frame) +
-             TruncatedErrorStringSize(
-                 frame.connection_close_frame->error_details);
+      return GetConnectionCloseFrameSize(version,
+                                         *frame.connection_close_frame);
     case GOAWAY_FRAME:
       return GetMinGoAwayFrameSize() +
              TruncatedErrorStringSize(frame.goaway_frame->reason_phrase);

quic/core/quic_framer.h

diff --git a/quic/core/quic_framer.h b/quic/core/quic_framer.h
index decb456..17a87fa 100644
--- a/quic/core/quic_framer.h
+++ b/quic/core/quic_framer.h
@@ -301,9 +301,9 @@
   // Size in bytes of all reset stream frame fields.
   static size_t GetRstStreamFrameSize(QuicTransportVersion version,
                                       const QuicRstStreamFrame& frame);
-  // Size in bytes of all connection close frame fields without the error
-  // details and the missing packets from the enclosed ack frame.
-  static size_t GetMinConnectionCloseFrameSize(
+  // Size in bytes of all connection close frame fields, including the error
+  // details.
+  static size_t GetConnectionCloseFrameSize(
       QuicTransportVersion version,
       const QuicConnectionCloseFrame& frame);
   // Size in bytes of all GoAway frame fields without the reason phrase.

quic/core/quic_framer_test.cc

diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index 0b5dca5..ddf8076 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -11558,9 +11558,8 @@
     connection_close.close_type = IETF_QUIC_TRANSPORT_CONNECTION_CLOSE;
   }
 
-  EXPECT_EQ(QuicFramer::GetMinConnectionCloseFrameSize(
-                framer_.transport_version(), connection_close) +
-                256,
+  EXPECT_EQ(QuicFramer::GetConnectionCloseFrameSize(framer_.transport_version(),
+                                                    connection_close),
             QuicFramer::GetRetransmittableControlFrameSize(
                 framer_.transport_version(), QuicFrame(&connection_close)));