Removing QUIC's VERSION_6, now that QUIC no longer needs to support the

older STREAM frames.

Merge internal CL: 50558774

Add the QuicVersionMin() function.

Change some unit tests to use QuicVersionMin().

Merge internal CL: 50630692

R=rch@chromium.org
BUG=none
TEST=net_unittests

Review URL: https://chromiumcodereview.appspot.com/22311013

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@217137 0039d316-1c4b-4281-b951-d872f2087c98

net/quic/crypto/proof_source.h

@@ -28,12 +28,12 @@ class NET_EXPORT_PRIVATE ProofSource {
   //
   // The signature uses SHA-256 as the hash function when the key is ECDSA.
   //
-  // If |ecdsa_ok| is true, the signature may use an ECDSA key. Otherwise, the
-  // signature must use an RSA key.
-  //
   // |version| is the QUIC version for the connection. TODO(wtc): Remove once
   // QUIC_VERSION_7 and before are removed.
   //
+  // If |ecdsa_ok| is true, the signature may use an ECDSA key. Otherwise, the
+  // signature must use an RSA key.
+  //
   // |out_certs| is a pointer to a pointer, not a pointer to an array.
   //
   // The number of certificate chains is expected to be small and fixed thus

net/quic/quic_connection_test.cc

@@ -2373,26 +2373,28 @@ TEST_F(QuicConnectionTest, DontProcessFramesIfPacketClosedConnection) {
   connection_.ProcessUdpPacket(IPEndPoint(), IPEndPoint(), *encrypted);
 }
 
-// The QUIC_VERSION_X versions are deliberately set, rather than using all
-// values in kSupportedQuicVersions.
 TEST_F(QuicConnectionTest, SelectMutualVersion) {
-  // Set the connection to speak QUIC_VERSION_6.
-  connection_.set_version(QUIC_VERSION_6);
-  EXPECT_EQ(connection_.version(), QUIC_VERSION_6);
+  // Set the connection to speak the lowest quic version.
+  connection_.set_version(QuicVersionMin());
+  EXPECT_EQ(QuicVersionMin(), connection_.version());
 
   // Pass in available versions which includes a higher mutually supported
   // version.  The higher mutually supported version should be selected.
-  QuicVersionVector available_versions;
-  available_versions.push_back(QUIC_VERSION_6);
-  available_versions.push_back(QUIC_VERSION_7);
-  EXPECT_TRUE(connection_.SelectMutualVersion(available_versions));
-  EXPECT_EQ(connection_.version(), QUIC_VERSION_7);
-
-  // Expect that the lower version is selected.
-  QuicVersionVector lower_version;
-  lower_version.push_back(QUIC_VERSION_6);
-  EXPECT_TRUE(connection_.SelectMutualVersion(lower_version));
-  EXPECT_EQ(connection_.version(), QUIC_VERSION_6);
+  QuicVersionVector supported_versions;
+  for (size_t i = 0; i < arraysize(kSupportedQuicVersions); ++i) {
+    supported_versions.push_back(kSupportedQuicVersions[i]);
+  }
+  EXPECT_TRUE(connection_.SelectMutualVersion(supported_versions));
+  EXPECT_EQ(QuicVersionMax(), connection_.version());
+
+  // Expect that the lowest version is selected.
+  // Ensure the lowest supported version is less than the max, unless they're
+  // the same.
+  EXPECT_LE(QuicVersionMin(), QuicVersionMax());
+  QuicVersionVector lowest_version_vector;
+  lowest_version_vector.push_back(QuicVersionMin());
+  EXPECT_TRUE(connection_.SelectMutualVersion(lowest_version_vector));
+  EXPECT_EQ(QuicVersionMin(), connection_.version());
 
   // Shouldn't be able to find a mutually supported version.
   QuicVersionVector unsupported_version;

net/quic/quic_framer.cc

@@ -103,11 +103,6 @@ size_t QuicFramer::GetMinStreamFrameSize(QuicVersion version,
                                          QuicStreamId stream_id,
                                          QuicStreamOffset offset,
                                          bool last_frame_in_packet) {
-  if (version == QUIC_VERSION_6) {
-    return kQuicFrameTypeSize + kQuicMaxStreamIdSize +
-        kQuicStreamFinSize + kQuicMaxStreamOffsetSize +
-        kQuicStreamPayloadLengthSize;
-  }
   return kQuicFrameTypeSize + GetStreamIdSize(stream_id) +
       GetStreamOffsetSize(offset) +
       (last_frame_in_packet ? 0 : kQuicStreamPayloadLengthSize);
@@ -283,15 +278,9 @@ SerializedPacket QuicFramer::BuildDataPacket(
         writer.WritePadding();
         break;
       case STREAM_FRAME:
-        if (quic_version_ == QUIC_VERSION_6) {
-          if (!AppendV6StreamFramePayload(*frame.stream_frame, &writer)) {
-            return kNoPacket;
-          }
-        } else {
-          if (!AppendStreamFramePayload(
-              *frame.stream_frame, last_frame_in_packet, &writer)) {
-            return kNoPacket;
-          }
+        if (!AppendStreamFramePayload(
+            *frame.stream_frame, last_frame_in_packet, &writer)) {
+          return kNoPacket;
         }
         break;
       case ACK_FRAME:
@@ -898,97 +887,55 @@ bool QuicFramer::ProcessFrameData() {
       return RaiseError(QUIC_INVALID_FRAME_DATA);
     }
 
-    if (quic_version_ >= QUIC_VERSION_7) {
-      if ((frame_type & kQuicFrameType0BitMask) == 0) {
-        QuicStreamFrame frame;
-        if (!ProcessStreamFrame(frame_type, &frame)) {
-          return RaiseError(QUIC_INVALID_FRAME_DATA);
-        }
-        if (!visitor_->OnStreamFrame(frame)) {
-          DLOG(INFO) << "Visitor asked to stop further processing.";
-          // Returning true since there was no parsing error.
-          return true;
-        }
-        continue;
+    if ((frame_type & kQuicFrameType0BitMask) == 0) {
+      QuicStreamFrame frame;
+      if (!ProcessStreamFrame(frame_type, &frame)) {
+        return RaiseError(QUIC_INVALID_FRAME_DATA);
       }
-
-      frame_type >>= 1;
-      if ((frame_type & kQuicFrameType0BitMask) == 0) {
-        QuicAckFrame frame;
-        if (!ProcessAckFrame(&frame)) {
-          return RaiseError(QUIC_INVALID_FRAME_DATA);
-        }
-        if (!visitor_->OnAckFrame(frame)) {
-          DLOG(INFO) << "Visitor asked to stop further processing.";
-          // Returning true since there was no parsing error.
-          return true;
-        }
-        continue;
+      if (!visitor_->OnStreamFrame(frame)) {
+        DLOG(INFO) << "Visitor asked to stop further processing.";
+        // Returning true since there was no parsing error.
+        return true;
       }
+      continue;
+    }
 
-      frame_type >>= 1;
-      if ((frame_type & kQuicFrameType0BitMask) == 0) {
-        QuicCongestionFeedbackFrame frame;
-        if (!ProcessQuicCongestionFeedbackFrame(&frame)) {
-          return RaiseError(QUIC_INVALID_FRAME_DATA);
-        }
-        if (!visitor_->OnCongestionFeedbackFrame(frame)) {
-          DLOG(INFO) << "Visitor asked to stop further processing.";
-          // Returning true since there was no parsing error.
-          return true;
-        }
-        continue;
+    frame_type >>= 1;
+    if ((frame_type & kQuicFrameType0BitMask) == 0) {
+      QuicAckFrame frame;
+      if (!ProcessAckFrame(&frame)) {
+        return RaiseError(QUIC_INVALID_FRAME_DATA);
+      }
+      if (!visitor_->OnAckFrame(frame)) {
+        DLOG(INFO) << "Visitor asked to stop further processing.";
+        // Returning true since there was no parsing error.
+        return true;
       }
+      continue;
+    }
 
-      frame_type >>= 1;
+    frame_type >>= 1;
+    if ((frame_type & kQuicFrameType0BitMask) == 0) {
+      QuicCongestionFeedbackFrame frame;
+      if (!ProcessQuicCongestionFeedbackFrame(&frame)) {
+        return RaiseError(QUIC_INVALID_FRAME_DATA);
+      }
+      if (!visitor_->OnCongestionFeedbackFrame(frame)) {
+        DLOG(INFO) << "Visitor asked to stop further processing.";
+        // Returning true since there was no parsing error.
+        return true;
+      }
+      continue;
     }
 
+    frame_type >>= 1;
+
     switch (frame_type) {
+      // STREAM_FRAME, ACK_FRAME, and CONGESTION_FEEDBACK_FRAME are handled
+      // above.
       case PADDING_FRAME:
         // We're done with the packet
         return true;
-      // STREAM_FRAME, ACK_FRAME, and CONGESTION_FEEDBACK handled above for
-      // QUIC_VERSION_7 and later.
-      case STREAM_FRAME: {
-        QuicStreamFrame frame;
-        if (!ProcessV6StreamFrame(&frame)) {
-          return RaiseError(QUIC_INVALID_FRAME_DATA);
-        }
-        if (!visitor_->OnStreamFrame(frame)) {
-          DLOG(INFO) << "Visitor asked to stop further processing.";
-          // Returning true since there was no parsing error.
-          return true;
-        }
-        continue;
-      }
-
-      case ACK_FRAME: {
-        QuicAckFrame frame;
-        if (!ProcessAckFrame(&frame)) {
-          return RaiseError(QUIC_INVALID_FRAME_DATA);
-        }
-        if (!visitor_->OnAckFrame(frame)) {
-          DLOG(INFO) << "Visitor asked to stop further processing.";
-          // Returning true since there was no parsing error.
-          // TODO(ianswett): Consider continuing to process frames, since there
-          // was not a parsing error.
-          return true;
-        }
-        continue;
-      }
-
-      case CONGESTION_FEEDBACK_FRAME: {
-        QuicCongestionFeedbackFrame frame;
-        if (!ProcessQuicCongestionFeedbackFrame(&frame)) {
-          return RaiseError(QUIC_INVALID_FRAME_DATA);
-        }
-        if (!visitor_->OnCongestionFeedbackFrame(frame)) {
-          DLOG(INFO) << "Visitor asked to stop further processing.";
-          // Returning true since there was no parsing error.
-          return true;
-        }
-        continue;
-      }
 
       case RST_STREAM_FRAME: {
         QuicRstStreamFrame frame;
@@ -1093,36 +1040,6 @@ bool QuicFramer::ProcessStreamFrame(uint8 frame_type,
   return true;
 }
 
-bool QuicFramer::ProcessV6StreamFrame(QuicStreamFrame* frame) {
-  if (!reader_->ReadUInt32(&frame->stream_id)) {
-    set_detailed_error("Unable to read stream_id.");
-    return false;
-  }
-
-  uint8 fin;
-  if (!reader_->ReadBytes(&fin, 1)) {
-    set_detailed_error("Unable to read fin.");
-    return false;
-  }
-  if (fin > 1) {
-    set_detailed_error("Invalid fin value.");
-    return false;
-  }
-  frame->fin = (fin == 1);
-
-  if (!reader_->ReadUInt64(&frame->offset)) {
-    set_detailed_error("Unable to read offset.");
-    return false;
-  }
-
-  if (!reader_->ReadStringPiece16(&frame->data)) {
-    set_detailed_error("Unable to read frame data.");
-    return false;
-  }
-
-  return true;
-}
-
 bool QuicFramer::ProcessAckFrame(QuicAckFrame* frame) {
   if (!ProcessSentInfo(&frame->sent_info)) {
     return false;
@@ -1602,10 +1519,6 @@ size_t QuicFramer::ComputeFrameLength(const QuicFrame& frame,
 bool QuicFramer::AppendTypeByte(const QuicFrame& frame,
                                 bool last_frame_in_packet,
                                 QuicDataWriter* writer) {
-  if (quic_version_ == QUIC_VERSION_6) {
-    return writer->WriteUInt8(frame.type);
-  }
-
   uint8 type_byte = 0;
   switch (frame.type) {
     case STREAM_FRAME: {
@@ -1706,27 +1619,6 @@ bool QuicFramer::AppendStreamFramePayload(
   return true;
 }
 
-bool QuicFramer::AppendV6StreamFramePayload(
-    const QuicStreamFrame& frame,
-    QuicDataWriter* writer) {
-  if (!writer->WriteUInt32(frame.stream_id)) {
-    return false;
-  }
-  if (!writer->WriteUInt8(frame.fin)) {
-    return false;
-  }
-  if (!writer->WriteUInt64(frame.offset)) {
-    return false;
-  }
-  if (!writer->WriteUInt16(frame.data.size())) {
-    return false;
-  }
-  if (!writer->WriteBytes(frame.data.data(), frame.data.size())) {
-    return false;
-  }
-  return true;
-}
-
 QuicPacketSequenceNumber QuicFramer::CalculateLargestObserved(
     const SequenceNumberSet& missing_packets,
     SequenceNumberSet::const_iterator largest_written) {

net/quic/quic_framer.h

@@ -35,9 +35,6 @@ const size_t kQuicErrorDetailsLengthSize = 2;
 
 // Maximum number of bytes reserved for stream id.
 const size_t kQuicMaxStreamIdSize = 4;
-// Number of bytes reserved for fin flag in stream frame.
-// TODO(ianswett): Remove once QUIC_VERSION_6 and before are removed.
-const size_t kQuicStreamFinSize = 1;
 // Maximum number of bytes reserved for byte offset in stream frame.
 const size_t kQuicMaxStreamOffsetSize = 8;
 // Number of bytes reserved to store payload length in stream frame.
@@ -365,7 +362,6 @@ class NET_EXPORT_PRIVATE QuicFramer {
       QuicPacketSequenceNumber* sequence_number);
   bool ProcessFrameData();
   bool ProcessStreamFrame(uint8 frame_type, QuicStreamFrame* frame);
-  bool ProcessV6StreamFrame(QuicStreamFrame* frame);
   bool ProcessAckFrame(QuicAckFrame* frame);
   bool ProcessReceivedInfo(ReceivedPacketInfo* received_info);
   bool ProcessSentInfo(SentPacketInfo* sent_info);
@@ -398,8 +394,6 @@ class NET_EXPORT_PRIVATE QuicFramer {
   bool AppendStreamFramePayload(const QuicStreamFrame& frame,
                                 bool last_frame_in_packet,
                                 QuicDataWriter* builder);
-  bool AppendV6StreamFramePayload(const QuicStreamFrame& frame,
-                                  QuicDataWriter* builder);
   bool AppendAckFramePayload(const QuicAckFrame& frame,
                              QuicDataWriter* builder);
   bool AppendQuicCongestionFeedbackFramePayload(

net/quic/quic_protocol.cc

@@ -123,10 +123,12 @@ uint32 MakeQuicTag(char a, char b, char c, char d) {
 
 QuicVersion QuicVersionMax() { return kSupportedQuicVersions[0]; }
 
+QuicVersion QuicVersionMin() {
+  return kSupportedQuicVersions[arraysize(kSupportedQuicVersions) - 1];
+}
+
 QuicTag QuicVersionToQuicTag(const QuicVersion version) {
   switch (version) {
-    case QUIC_VERSION_6:
-      return MakeQuicTag('Q', '0', '0', '6');
     case QUIC_VERSION_7:
       return MakeQuicTag('Q', '0', '0', '7');
     case QUIC_VERSION_8:
@@ -140,13 +142,10 @@ QuicTag QuicVersionToQuicTag(const QuicVersion version) {
 }
 
 QuicVersion QuicTagToQuicVersion(const QuicTag version_tag) {
-  const QuicTag quic_tag_v6 = MakeQuicTag('Q', '0', '0', '6');
   const QuicTag quic_tag_v7 = MakeQuicTag('Q', '0', '0', '7');
   const QuicTag quic_tag_v8 = MakeQuicTag('Q', '0', '0', '8');
 
-  if (version_tag == quic_tag_v6) {
-    return QUIC_VERSION_6;
-  } else if (version_tag == quic_tag_v7) {
+  if (version_tag == quic_tag_v7) {
     return QUIC_VERSION_7;
   } else if (version_tag == quic_tag_v8) {
     return QUIC_VERSION_8;
@@ -164,7 +163,6 @@ return #x
 
 string QuicVersionToString(const QuicVersion version) {
   switch (version) {
-    RETURN_STRING_LITERAL(QUIC_VERSION_6);
     RETURN_STRING_LITERAL(QUIC_VERSION_7);
     RETURN_STRING_LITERAL(QUIC_VERSION_8);
     default:

net/quic/quic_protocol.h

@@ -187,7 +187,6 @@ enum QuicVersion {
   // Special case to indicate unknown/unsupported QUIC version.
   QUIC_VERSION_UNSUPPORTED = 0,
 
-  QUIC_VERSION_6 = 6,
   QUIC_VERSION_7 = 7,
   QUIC_VERSION_8 = 8,  // Current version.
 };
@@ -197,13 +196,16 @@ enum QuicVersion {
 // element, with subsequent elements in descending order (versions can be
 // skipped as necessary).
 static const QuicVersion kSupportedQuicVersions[] =
-    {QUIC_VERSION_8, QUIC_VERSION_7, QUIC_VERSION_6};
+    {QUIC_VERSION_8, QUIC_VERSION_7};
 
 typedef std::vector<QuicVersion> QuicVersionVector;
 
 // Upper limit on versions we support.
 NET_EXPORT_PRIVATE QuicVersion QuicVersionMax();
 
+// Lower limit on versions we support.
+NET_EXPORT_PRIVATE QuicVersion QuicVersionMin();
+
 // QuicTag is written to and read from the wire, but we prefer to use
 // the more readable QuicVersion at other levels.
 // Helper function which translates from a QuicVersion to a QuicTag. Returns 0

net/quic/test_tools/quic_test_utils.cc

@@ -423,10 +423,10 @@ size_t GetPacketLengthForOneStream(QuicVersion version,
           PACKET_6BYTE_SEQUENCE_NUMBER, is_in_fec_group);
 }
 
+// Size in bytes of the stream frame fields for an arbitrary StreamID and
+// offset and the last frame in a packet.
 size_t GetMinStreamFrameSize(QuicVersion version) {
-  return kQuicFrameTypeSize + kQuicMaxStreamIdSize + kQuicMaxStreamOffsetSize +
-      (version == QUIC_VERSION_6 ?
-          (kQuicStreamFinSize + kQuicStreamPayloadLengthSize) : 0);
+  return kQuicFrameTypeSize + kQuicMaxStreamIdSize + kQuicMaxStreamOffsetSize;
 }
 
 QuicPacketEntropyHash TestEntropyCalculator::EntropyHash(

net/tools/quic/end_to_end_test.cc

@@ -212,10 +212,10 @@ class EndToEndTest : public ::testing::TestWithParam<QuicVersion> {
   QuicVersion version_;
 };
 
-// Run all end to end tests with QUIC version 6 and 7.
+// Run all end to end tests with all supported versions.
 INSTANTIATE_TEST_CASE_P(EndToEndTests,
                         EndToEndTest,
-                        ::testing::Values(QUIC_VERSION_6, QUIC_VERSION_7));
+                        ::testing::ValuesIn(kSupportedQuicVersions));
 
 TEST_P(EndToEndTest, SimpleRequestResponse) {
   // TODO(rtenneti): Delete this when NSS is supported.