Move Quic AppendTimestampFrame method out of AppendCongestionFrame.

net/quic/quic_framer.cc

Index: net/quic/quic_framer.cc
diff --git a/net/quic/quic_framer.cc b/net/quic/quic_framer.cc
index 2ef5c7fbdf1d8390ddc3d1d0e72ffe44a84df42b..c21bd6b72a1a19187e5fbfb9be5a65f846d4d0e6 100644
--- a/net/quic/quic_framer.cc
+++ b/net/quic/quic_framer.cc
@@ -972,219 +972,224 @@ QuicFramer::AckFrameInfo QuicFramer::GetAckFrameInfo(
     ++iter;
     for (; iter != frame.missing_packets.end(); ++iter) {
       if (cur_range_length != numeric_limits<uint8>::max() &&
-          *iter == (last_missing + 1)) {
-        ++cur_range_length;
-      } else {
-        ack_info.nack_ranges[last_missing - cur_range_length]
-            = cur_range_length;
-        cur_range_length = 0;
+              *iter == (last_missing + 1)) {
+            ++cur_range_length;
+          } else {
+            ack_info.nack_ranges[last_missing - cur_range_length]
+                = cur_range_length;
+            cur_range_length = 0;
+          }
+          ack_info.max_delta = max(ack_info.max_delta, *iter - last_missing);
+          last_missing = *iter;
+        }
+        // Include the last nack range.
+        ack_info.nack_ranges[last_missing - cur_range_length] =
+            cur_range_length;
+        // Include the range to the largest observed.
+        ack_info.max_delta = max(ack_info.max_delta,
+                                 frame.largest_observed - last_missing);
       }
-      ack_info.max_delta = max(ack_info.max_delta, *iter - last_missing);
-      last_missing = *iter;
+      return ack_info;
     }
-    // Include the last nack range.
-    ack_info.nack_ranges[last_missing - cur_range_length] = cur_range_length;
-    // Include the range to the largest observed.
-    ack_info.max_delta = max(ack_info.max_delta,
-                             frame.largest_observed - last_missing);
-  }
-  return ack_info;
-}
 
-bool QuicFramer::ProcessPacketHeader(
-    QuicPacketHeader* header,
-    const QuicEncryptedPacket& packet) {
-  if (!ProcessPacketSequenceNumber(header->public_header.sequence_number_length,
-                                   &header->packet_sequence_number)) {
-    set_detailed_error("Unable to read sequence number.");
-    return RaiseError(QUIC_INVALID_PACKET_HEADER);
-  }
+    bool QuicFramer::ProcessPacketHeader(
+        QuicPacketHeader* header,
+        const QuicEncryptedPacket& packet) {
+      if (!ProcessPacketSequenceNumber(
+              header->public_header.sequence_number_length,
+              &header->packet_sequence_number)) {
+        set_detailed_error("Unable to read sequence number.");
+        return RaiseError(QUIC_INVALID_PACKET_HEADER);
+      }
 
-  if (header->packet_sequence_number == 0u) {
-    set_detailed_error("Packet sequence numbers cannot be 0.");
-    return RaiseError(QUIC_INVALID_PACKET_HEADER);
-  }
+      if (header->packet_sequence_number == 0u) {
+        set_detailed_error("Packet sequence numbers cannot be 0.");
+        return RaiseError(QUIC_INVALID_PACKET_HEADER);
+      }
 
-  if (!visitor_->OnUnauthenticatedHeader(*header)) {
-    return false;
-  }
+      if (!visitor_->OnUnauthenticatedHeader(*header)) {
+        return false;
+      }
 
-  if (!DecryptPayload(*header, packet)) {
-    set_detailed_error("Unable to decrypt payload.");
-    return RaiseError(QUIC_DECRYPTION_FAILURE);
-  }
+      if (!DecryptPayload(*header, packet)) {
+        set_detailed_error("Unable to decrypt payload.");
+        return RaiseError(QUIC_DECRYPTION_FAILURE);
+      }
 
-  uint8 private_flags;
-  if (!reader_->ReadBytes(&private_flags, 1)) {
-    set_detailed_error("Unable to read private flags.");
-    return RaiseError(QUIC_INVALID_PACKET_HEADER);
-  }
+      uint8 private_flags;
+      if (!reader_->ReadBytes(&private_flags, 1)) {
+        set_detailed_error("Unable to read private flags.");
+        return RaiseError(QUIC_INVALID_PACKET_HEADER);
+      }
 
-  if (private_flags > PACKET_PRIVATE_FLAGS_MAX) {
-    set_detailed_error("Illegal private flags value.");
-    return RaiseError(QUIC_INVALID_PACKET_HEADER);
-  }
+      if (private_flags > PACKET_PRIVATE_FLAGS_MAX) {
+        set_detailed_error("Illegal private flags value.");
+        return RaiseError(QUIC_INVALID_PACKET_HEADER);
+      }
 
-  header->entropy_flag = (private_flags & PACKET_PRIVATE_FLAGS_ENTROPY) != 0;
-  header->fec_flag = (private_flags & PACKET_PRIVATE_FLAGS_FEC) != 0;
+      header->entropy_flag =
+          (private_flags & PACKET_PRIVATE_FLAGS_ENTROPY) != 0;
+      header->fec_flag = (private_flags & PACKET_PRIVATE_FLAGS_FEC) != 0;
+
+      if ((private_flags & PACKET_PRIVATE_FLAGS_FEC_GROUP) != 0) {
+        header->is_in_fec_group = IN_FEC_GROUP;
+        uint8 first_fec_protected_packet_offset;
+        if (!reader_->ReadBytes(&first_fec_protected_packet_offset, 1)) {
+          set_detailed_error(
+              "Unable to read first fec protected packet offset.");
+          return RaiseError(QUIC_INVALID_PACKET_HEADER);
+        }
+        if (first_fec_protected_packet_offset >=
+            header->packet_sequence_number) {
+          set_detailed_error("First fec protected packet offset must be less "
+                             "than the sequence number.");
+          return RaiseError(QUIC_INVALID_PACKET_HEADER);
+        }
+        header->fec_group =
+            header->packet_sequence_number - first_fec_protected_packet_offset;
+      }
 
-  if ((private_flags & PACKET_PRIVATE_FLAGS_FEC_GROUP) != 0) {
-    header->is_in_fec_group = IN_FEC_GROUP;
-    uint8 first_fec_protected_packet_offset;
-    if (!reader_->ReadBytes(&first_fec_protected_packet_offset, 1)) {
-      set_detailed_error("Unable to read first fec protected packet offset.");
-      return RaiseError(QUIC_INVALID_PACKET_HEADER);
-    }
-    if (first_fec_protected_packet_offset >= header->packet_sequence_number) {
-      set_detailed_error("First fec protected packet offset must be less "
-                         "than the sequence number.");
-      return RaiseError(QUIC_INVALID_PACKET_HEADER);
+      header->entropy_hash = GetPacketEntropyHash(*header);
+      // Set the last sequence number after we have decrypted the packet
+      // so we are confident is not attacker controlled.
+      last_sequence_number_ = header->packet_sequence_number;
+      return true;
     }
-    header->fec_group =
-        header->packet_sequence_number - first_fec_protected_packet_offset;
-  }
 
-  header->entropy_hash = GetPacketEntropyHash(*header);
-  // Set the last sequence number after we have decrypted the packet
-  // so we are confident is not attacker controlled.
-  last_sequence_number_ = header->packet_sequence_number;
-  return true;
-}
-
-bool QuicFramer::ProcessPacketSequenceNumber(
-    QuicSequenceNumberLength sequence_number_length,
-    QuicPacketSequenceNumber* sequence_number) {
-  QuicPacketSequenceNumber wire_sequence_number = 0u;
-  if (!reader_->ReadBytes(&wire_sequence_number, sequence_number_length)) {
-    return false;
-  }
-
-  // TODO(ianswett): Explore the usefulness of trying multiple sequence numbers
-  // in case the first guess is incorrect.
-  *sequence_number =
-      CalculatePacketSequenceNumberFromWire(sequence_number_length,
-                                            wire_sequence_number);
-  return true;
-}
+    bool QuicFramer::ProcessPacketSequenceNumber(
+        QuicSequenceNumberLength sequence_number_length,
+        QuicPacketSequenceNumber* sequence_number) {
+      QuicPacketSequenceNumber wire_sequence_number = 0u;
+      if (!reader_->ReadBytes(&wire_sequence_number, sequence_number_length)) {
+        return false;
+      }
 
-bool QuicFramer::ProcessFrameData(const QuicPacketHeader& header) {
-  if (reader_->IsDoneReading()) {
-    set_detailed_error("Packet has no frames.");
-    return RaiseError(QUIC_MISSING_PAYLOAD);
-  }
-  while (!reader_->IsDoneReading()) {
-    uint8 frame_type;
-    if (!reader_->ReadBytes(&frame_type, 1)) {
-      set_detailed_error("Unable to read frame type.");
-      return RaiseError(QUIC_INVALID_FRAME_DATA);
+      // TODO(ianswett): Explore the usefulness of trying multiple sequence
+      // numbers in case the first guess is incorrect.
+      *sequence_number =
+          CalculatePacketSequenceNumberFromWire(sequence_number_length,
+                                                wire_sequence_number);
+      return true;
     }
 
-    if (frame_type & kQuicFrameTypeSpecialMask) {
-      // Stream Frame
-      if (frame_type & kQuicFrameTypeStreamMask) {
-        QuicStreamFrame frame;
-        if (!ProcessStreamFrame(frame_type, &frame)) {
-          return RaiseError(QUIC_INVALID_STREAM_DATA);
-        }
-        if (!visitor_->OnStreamFrame(frame)) {
-          DVLOG(1) << "Visitor asked to stop further processing.";
-          // Returning true since there was no parsing error.
-          return true;
-        }
-        continue;
+    bool QuicFramer::ProcessFrameData(const QuicPacketHeader& header) {
+      if (reader_->IsDoneReading()) {
+        set_detailed_error("Packet has no frames.");
+        return RaiseError(QUIC_MISSING_PAYLOAD);
       }
-
-      // Ack Frame
-      if (frame_type & kQuicFrameTypeAckMask) {
-        QuicAckFrame frame;
-        if (!ProcessAckFrame(frame_type, &frame)) {
-          return RaiseError(QUIC_INVALID_ACK_DATA);
-        }
-        if (!visitor_->OnAckFrame(frame)) {
-          DVLOG(1) << "Visitor asked to stop further processing.";
-          // Returning true since there was no parsing error.
-          return true;
+      while (!reader_->IsDoneReading()) {
+        uint8 frame_type;
+        if (!reader_->ReadBytes(&frame_type, 1)) {
+          set_detailed_error("Unable to read frame type.");
+          return RaiseError(QUIC_INVALID_FRAME_DATA);
         }
-        continue;
-      }
 
-      // Congestion Feedback Frame
-      if (frame_type & kQuicFrameTypeCongestionFeedbackMask) {
-        QuicCongestionFeedbackFrame frame;
-        if (!ProcessQuicCongestionFeedbackFrame(&frame)) {
-          return RaiseError(QUIC_INVALID_CONGESTION_FEEDBACK_DATA);
-        }
-        if (!visitor_->OnCongestionFeedbackFrame(frame)) {
-          DVLOG(1) << "Visitor asked to stop further processing.";
-          // Returning true since there was no parsing error.
-          return true;
-        }
-        continue;
-      }
+        if (frame_type & kQuicFrameTypeSpecialMask) {
+          // Stream Frame
+          if (frame_type & kQuicFrameTypeStreamMask) {
+            QuicStreamFrame frame;
+            if (!ProcessStreamFrame(frame_type, &frame)) {
+              return RaiseError(QUIC_INVALID_STREAM_DATA);
+            }
+            if (!visitor_->OnStreamFrame(frame)) {
+              DVLOG(1) << "Visitor asked to stop further processing.";
+              // Returning true since there was no parsing error.
+              return true;
+            }
+            continue;
+          }
 
-      // This was a special frame type that did not match any
-      // of the known ones. Error.
-      set_detailed_error("Illegal frame type.");
-      DLOG(WARNING) << "Illegal frame type: "
-                    << static_cast<int>(frame_type);
-      return RaiseError(QUIC_INVALID_FRAME_DATA);
-    }
+          // Ack Frame
+          if (frame_type & kQuicFrameTypeAckMask) {
+            QuicAckFrame frame;
+            if (!ProcessAckFrame(frame_type, &frame)) {
+              return RaiseError(QUIC_INVALID_ACK_DATA);
+            }
+            if (!visitor_->OnAckFrame(frame)) {
+              DVLOG(1) << "Visitor asked to stop further processing.";
+              // Returning true since there was no parsing error.
+              return true;
+            }
+            continue;
+          }
 
-    switch (frame_type) {
-      case PADDING_FRAME:
-        // We're done with the packet.
-        return true;
+          // Congestion Feedback Frame
+          if (frame_type & kQuicFrameTypeCongestionFeedbackMask) {
+            QuicCongestionFeedbackFrame frame;
+            if (!ProcessQuicCongestionFeedbackFrame(&frame)) {
+              return RaiseError(QUIC_INVALID_CONGESTION_FEEDBACK_DATA);
+            }
+            if (!visitor_->OnCongestionFeedbackFrame(frame)) {
+              DVLOG(1) << "Visitor asked to stop further processing.";
+              // Returning true since there was no parsing error.
+              return true;
+            }
+            continue;
+          }
 
-      case RST_STREAM_FRAME: {
-        QuicRstStreamFrame frame;
-        if (!ProcessRstStreamFrame(&frame)) {
-          return RaiseError(QUIC_INVALID_RST_STREAM_DATA);
+          // This was a special frame type that did not match any
+          // of the known ones. Error.
+          set_detailed_error("Illegal frame type.");
+          DLOG(WARNING) << "Illegal frame type: "
+                        << static_cast<int>(frame_type);
+          return RaiseError(QUIC_INVALID_FRAME_DATA);
         }
-        if (!visitor_->OnRstStreamFrame(frame)) {
-          DVLOG(1) << "Visitor asked to stop further processing.";
-          // Returning true since there was no parsing error.
-          return true;
-        }
-        continue;
-      }
 
-      case CONNECTION_CLOSE_FRAME: {
-        QuicConnectionCloseFrame frame;
-        if (!ProcessConnectionCloseFrame(&frame)) {
-          return RaiseError(QUIC_INVALID_CONNECTION_CLOSE_DATA);
-        }
+        switch (frame_type) {
+          case PADDING_FRAME:
+            // We're done with the packet.
+            return true;
+
+          case RST_STREAM_FRAME: {
+            QuicRstStreamFrame frame;
+            if (!ProcessRstStreamFrame(&frame)) {
+              return RaiseError(QUIC_INVALID_RST_STREAM_DATA);
+            }
+            if (!visitor_->OnRstStreamFrame(frame)) {
+              DVLOG(1) << "Visitor asked to stop further processing.";
+              // Returning true since there was no parsing error.
+              return true;
+            }
+            continue;
+          }
 
-        if (!visitor_->OnConnectionCloseFrame(frame)) {
-          DVLOG(1) << "Visitor asked to stop further processing.";
-          // Returning true since there was no parsing error.
-          return true;
-        }
-        continue;
-      }
+          case CONNECTION_CLOSE_FRAME: {
+            QuicConnectionCloseFrame frame;
+            if (!ProcessConnectionCloseFrame(&frame)) {
+              return RaiseError(QUIC_INVALID_CONNECTION_CLOSE_DATA);
+            }
+
+            if (!visitor_->OnConnectionCloseFrame(frame)) {
+              DVLOG(1) << "Visitor asked to stop further processing.";
+              // Returning true since there was no parsing error.
+              return true;
+            }
+            continue;
+          }
 
-      case GOAWAY_FRAME: {
-        QuicGoAwayFrame goaway_frame;
-        if (!ProcessGoAwayFrame(&goaway_frame)) {
-          return RaiseError(QUIC_INVALID_GOAWAY_DATA);
-        }
-        if (!visitor_->OnGoAwayFrame(goaway_frame)) {
-          DVLOG(1) << "Visitor asked to stop further processing.";
-          // Returning true since there was no parsing error.
-          return true;
-        }
-        continue;
-      }
+          case GOAWAY_FRAME: {
+            QuicGoAwayFrame goaway_frame;
+            if (!ProcessGoAwayFrame(&goaway_frame)) {
+              return RaiseError(QUIC_INVALID_GOAWAY_DATA);
+            }
+            if (!visitor_->OnGoAwayFrame(goaway_frame)) {
+              DVLOG(1) << "Visitor asked to stop further processing.";
+              // Returning true since there was no parsing error.
+              return true;
+            }
+            continue;
+          }
 
-      case WINDOW_UPDATE_FRAME: {
-        QuicWindowUpdateFrame window_update_frame;
-        if (!ProcessWindowUpdateFrame(&window_update_frame)) {
-          return RaiseError(QUIC_INVALID_WINDOW_UPDATE_DATA);
-        }
-        if (!visitor_->OnWindowUpdateFrame(window_update_frame)) {
-          DVLOG(1) << "Visitor asked to stop further processing.";
-          // Returning true since there was no parsing error.
-          return true;
-        }
+          case WINDOW_UPDATE_FRAME: {
+            QuicWindowUpdateFrame window_update_frame;
+            if (!ProcessWindowUpdateFrame(&window_update_frame)) {
+              return RaiseError(QUIC_INVALID_WINDOW_UPDATE_DATA);
+            }
+            if (!visitor_->OnWindowUpdateFrame(window_update_frame)) {
+              DVLOG(1) << "Visitor asked to stop further processing.";
+              // Returning true since there was no parsing error.
+              return true;
+                    }
         continue;
       }
 
@@ -1417,9 +1422,8 @@ bool QuicFramer::ProcessQuicCongestionFeedbackFrame(
       static_cast<CongestionFeedbackType>(feedback_type);
 
   switch (frame->type) {
-    case kInterArrival: {
-      CongestionFeedbackMessageInterArrival* inter_arrival =
-          &frame->inter_arrival;
+    case kTimestamp: {
+      CongestionFeedbackMessageTimestamp* timestamp = &frame->timestamp;
       uint8 num_received_packets;
       if (!reader_->ReadBytes(&num_received_packets, 1)) {
         set_detailed_error("Unable to read num received packets.");
@@ -1442,7 +1446,7 @@ bool QuicFramer::ProcessQuicCongestionFeedbackFrame(
         QuicTime time_received = creation_time_.Add(
             QuicTime::Delta::FromMicroseconds(time_received_us));
 
-        inter_arrival->received_packet_times.insert(
+        timestamp->received_packet_times.insert(
             make_pair(smallest_received, time_received));
 
         for (uint8 i = 0; i < num_received_packets - 1; ++i) {
@@ -1460,7 +1464,7 @@ bool QuicFramer::ProcessQuicCongestionFeedbackFrame(
             return false;
           }
           QuicPacketSequenceNumber packet = smallest_received + sequence_delta;
-          inter_arrival->received_packet_times.insert(
+          timestamp->received_packet_times.insert(
               make_pair(packet, time_received.Add(
                   QuicTime::Delta::FromMicroseconds(time_delta_us))));
         }
@@ -1469,7 +1473,6 @@ bool QuicFramer::ProcessQuicCongestionFeedbackFrame(
     }
     case kTCP: {
       CongestionFeedbackMessageTCP* tcp = &frame->tcp;
-      // TODO(ianswett): Remove receive window, since it's constant.
       uint16 receive_window = 0;
       if (!reader_->ReadUInt16(&receive_window)) {
         set_detailed_error("Unable to read receive window.");
@@ -1788,16 +1791,16 @@ size_t QuicFramer::ComputeFrameLength(
       len += 1;  // Congestion feedback type.
 
       switch (congestion_feedback.type) {
-        case kInterArrival: {
-          const CongestionFeedbackMessageInterArrival& inter_arrival =
-              congestion_feedback.inter_arrival;
+        case kTimestamp: {
+          const CongestionFeedbackMessageTimestamp& timestamp =
+              congestion_feedback.timestamp;
           len += 1;  // Number received packets.
-          if (inter_arrival.received_packet_times.size() > 0) {
+          if (!timestamp.received_packet_times.empty()) {
             len += PACKET_6BYTE_SEQUENCE_NUMBER;  // Smallest received.
             len += 8;  // Time.
             // 2 bytes per sequence number delta plus 4 bytes per delta time.
             len += PACKET_6BYTE_SEQUENCE_NUMBER *
-                (inter_arrival.received_packet_times.size() - 1);
+                (timestamp.received_packet_times.size() - 1);
           }
           break;
         }
@@ -2094,55 +2097,8 @@ bool QuicFramer::AppendCongestionFeedbackFrame(
   }
 
   switch (frame.type) {
-    case kInterArrival: {
-      const CongestionFeedbackMessageInterArrival& inter_arrival =
-          frame.inter_arrival;
-      DCHECK_GE(numeric_limits<uint8>::max(),
-                inter_arrival.received_packet_times.size());
-      if (inter_arrival.received_packet_times.size() >
-          numeric_limits<uint8>::max()) {
-        return false;
-      }
-      // TODO(ianswett): Make num_received_packets a varint.
-      uint8 num_received_packets =
-          inter_arrival.received_packet_times.size();
-      if (!writer->WriteBytes(&num_received_packets, 1)) {
-        return false;
-      }
-      if (num_received_packets > 0) {
-        TimeMap::const_iterator it =
-            inter_arrival.received_packet_times.begin();
-
-        QuicPacketSequenceNumber lowest_sequence = it->first;
-        if (!AppendPacketSequenceNumber(PACKET_6BYTE_SEQUENCE_NUMBER,
-                                        lowest_sequence, writer)) {
-          return false;
-        }
-
-        QuicTime lowest_time = it->second;
-        if (!writer->WriteUInt64(
-                lowest_time.Subtract(creation_time_).ToMicroseconds())) {
-          return false;
-        }
-
-        for (++it; it != inter_arrival.received_packet_times.end(); ++it) {
-          QuicPacketSequenceNumber sequence_delta = it->first - lowest_sequence;
-          DCHECK_GE(numeric_limits<uint16>::max(), sequence_delta);
-          if (sequence_delta > numeric_limits<uint16>::max()) {
-            return false;
-          }
-          if (!writer->WriteUInt16(static_cast<uint16>(sequence_delta))) {
-            return false;
-          }
-
-          int32 time_delta_us =
-              it->second.Subtract(lowest_time).ToMicroseconds();
-          if (!writer->WriteBytes(&time_delta_us, sizeof(time_delta_us))) {
-            return false;
-          }
-        }
-      }
-      break;
+    case kTimestamp: {
+      return AppendTimestampFrame(frame, writer);
     }
     case kTCP: {
       const CongestionFeedbackMessageTCP& tcp = frame.tcp;
@@ -2161,6 +2117,53 @@ bool QuicFramer::AppendCongestionFeedbackFrame(
   return true;
 }
 
+bool QuicFramer::AppendTimestampFrame(
+    const QuicCongestionFeedbackFrame& frame,
+    QuicDataWriter* writer) {
+  const CongestionFeedbackMessageTimestamp& timestamp = frame.timestamp;
+  DCHECK_GE(numeric_limits<uint8>::max(),
+            timestamp.received_packet_times.size());
+  if (timestamp.received_packet_times.size() > numeric_limits<uint8>::max()) {
+    return false;
+  }
+  uint8 num_received_packets = timestamp.received_packet_times.size();
+  if (!writer->WriteBytes(&num_received_packets, 1)) {
+    return false;
+  }
+  if (num_received_packets > 0) {
+    TimeMap::const_iterator it = timestamp.received_packet_times.begin();
+
+    QuicPacketSequenceNumber lowest_sequence = it->first;
+    if (!AppendPacketSequenceNumber(PACKET_6BYTE_SEQUENCE_NUMBER,
+                                    lowest_sequence, writer)) {
+      return false;
+    }
+
+    QuicTime lowest_time = it->second;
+    if (!writer->WriteUInt64(
+            lowest_time.Subtract(creation_time_).ToMicroseconds())) {
+      return false;
+    }
+
+    for (++it; it != timestamp.received_packet_times.end(); ++it) {
+      QuicPacketSequenceNumber sequence_delta = it->first - lowest_sequence;
+      DCHECK_GE(numeric_limits<uint16>::max(), sequence_delta);
+      if (sequence_delta > numeric_limits<uint16>::max()) {
+        return false;
+      }
+      if (!writer->WriteUInt16(static_cast<uint16>(sequence_delta))) {
+        return false;
+      }
+
+      int32 time_delta_us = it->second.Subtract(lowest_time).ToMicroseconds();
+      if (!writer->WriteBytes(&time_delta_us, sizeof(time_delta_us))) {
+        return false;
+      }
+    }
+  }
+  return true;
+}
+
 bool QuicFramer::AppendStopWaitingFrame(
     const QuicPacketHeader& header,
     const QuicStopWaitingFrame& frame,