Queueing QUIC frames to be resent instead of packets and packing RST frames with acks and congestion info frames.

Merge internal change: 40834055


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

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

13 files changed, 210 insertions, 176 deletions

diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc
index 30df462..debc017 100644
--- a/net/quic/quic_connection.cc
+++ b/net/quic/quic_connection.cc
@@ -58,6 +58,21 @@ bool Near(QuicPacketSequenceNumber a, QuicPacketSequenceNumber b) {
   return delta <= kMaxPacketGap;
 }
 
+QuicConnection::UnackedPacket::UnackedPacket(QuicFrames unacked_frames)
+    : frames(unacked_frames),
+      number_nacks(0) {
+}
+
+QuicConnection::UnackedPacket::UnackedPacket(QuicFrames unacked_frames,
+                                             std::string data)
+    : frames(unacked_frames),
+      number_nacks(0),
+      data(data) {
+}
+
+QuicConnection::UnackedPacket::~UnackedPacket() {
+}
+
 QuicConnection::QuicConnection(QuicGuid guid,
                                IPEndPoint address,
                                QuicConnectionHelperInterface* helper)
@@ -99,19 +114,53 @@ QuicConnection::QuicConnection(QuicGuid guid,
 }
 
 QuicConnection::~QuicConnection() {
-  for (UnackedPacketMap::iterator it = unacked_packets_.begin();
-       it != unacked_packets_.end(); ++it) {
-    delete it->second.packet;
+  for (UnackedPacketMap::iterator u = unacked_packets_.begin();
+       u != unacked_packets_.end(); ++u) {
+    DeleteUnackedPacket(u->second);
   }
+  STLDeleteValues(&unacked_packets_);
   STLDeleteValues(&group_map_);
-  // Queued packets that are not to be resent are owned
-  // by the packet queue.
   for (QueuedPacketList::iterator q = queued_packets_.begin();
        q != queued_packets_.end(); ++q) {
-    if (!q->resend) delete q->packet;
+    delete q->packet;
   }
 }
 
+void QuicConnection::DeleteEnclosedFrame(QuicFrame* frame) {
+  switch (frame->type) {
+    case STREAM_FRAME:
+      delete frame->stream_frame;
+      break;
+    case ACK_FRAME:
+      delete frame->ack_frame;
+      break;
+    case CONGESTION_FEEDBACK_FRAME:
+      delete frame->congestion_feedback_frame;
+      break;
+    case RST_STREAM_FRAME:
+      delete frame->rst_stream_frame;
+      break;
+    case CONNECTION_CLOSE_FRAME:
+      delete frame->connection_close_frame;
+      break;
+    case PDU_FRAME:  // Fall through.
+    case NUM_FRAME_TYPES:
+      DCHECK(false) << "Cannot delete type: " << frame->type;
+  }
+}
+
+void QuicConnection::DeleteUnackedPacket(UnackedPacket* unacked) {
+  for (QuicFrames::iterator it = unacked->frames.begin();
+       it != unacked->frames.end(); ++it) {
+    DCHECK(ShouldResend(*it));
+    DeleteEnclosedFrame(&(*it));
+  }
+}
+
+bool QuicConnection::ShouldResend(const QuicFrame& frame) {
+  return frame.type != ACK_FRAME && frame.type != CONGESTION_FEEDBACK_FRAME;
+}
+
 void QuicConnection::OnError(QuicFramer* framer) {
   SendConnectionClose(framer->error());
 }
@@ -270,6 +319,10 @@ void QuicConnection::UpdatePacketInformationReceivedByPeer(
       // Packet was acked, so remove it from our unacked packet list.
       DVLOG(1) << "Got an ack for " << it->first;
       // TODO(rch): This is inefficient and should be sped up.
+      // TODO(ianswett): Ensure this inner loop is applicable now that we're
+      // always sending packets with new sequence numbers.  I believe it may
+      // only be relevant for the first crypto connect packet, which doesn't
+      // get a new packet sequence number.
       // The acked packet might be queued (if a resend had been attempted).
       for (QueuedPacketList::iterator q = queued_packets_.begin();
            q != queued_packets_.end(); ++q) {
@@ -279,7 +332,8 @@ void QuicConnection::UpdatePacketInformationReceivedByPeer(
         }
       }
       acked_packets.insert(it->first);
-      delete it->second.packet;
+      DeleteUnackedPacket(it->second);
+      delete it->second;
       UnackedPacketMap::iterator it_tmp = it;
       ++it;
       unacked_packets_.erase(it_tmp);
@@ -298,8 +352,8 @@ void QuicConnection::UpdatePacketInformationReceivedByPeer(
       if (it->first < peer_largest_observed_packet_) {
         // The peer got packets after this sequence number.  This is an explicit
         // nack.
-        ++(it->second.number_nacks);
-        if (it->second.number_nacks >= kNumberOfNacksBeforeResend &&
+        ++(it->second->number_nacks);
+        if (it->second->number_nacks >= kNumberOfNacksBeforeResend &&
             resent_packets < kMaxResendPerAck) {
           resend_number = it->first;
         }
@@ -424,8 +478,9 @@ QuicConsumedData QuicConnection::SendStreamData(
 
   while (queued_packets_.empty()) {
     vector<PacketPair> packets;
+    QuicFrames frames;
     size_t bytes_consumed =
-        packet_creator_.DataToStream(id, data, offset, fin, &packets);
+        packet_creator_.DataToStream(id, data, offset, fin, &packets, &frames);
     total_bytes_consumed += bytes_consumed;
     offset += bytes_consumed;
     fin_consumed = fin && bytes_consumed == data.size();
@@ -433,14 +488,24 @@ QuicConsumedData QuicConnection::SendStreamData(
     DCHECK_LT(0u, packets.size());
 
     for (size_t i = 0; i < packets.size(); ++i) {
+      // Resend is false for FEC packets.
+      bool should_resend = !packets[i].second->IsFecPacket();
       SendPacket(packets[i].first,
                  packets[i].second,
-                 // Resend is false for FEC packets.
-                 !packets[i].second->IsFecPacket(),
+                 should_resend,
                  false,
                  false);
-      unacked_packets_.insert(make_pair(packets[i].first,
-                                        UnackedPacket(packets[i].second)));
+      if (should_resend) {
+        QuicFrames unacked_frames;
+        unacked_frames.push_back(frames[i]);
+        UnackedPacket* unacked = new UnackedPacket(
+            unacked_frames, frames[i].stream_frame->data.as_string());
+        // Ensure the string piece points to the owned copy of the data.
+        unacked->frames[0].stream_frame->data = StringPiece(unacked->data);
+        unacked_packets_.insert(make_pair(packets[i].first, unacked));
+      } else {
+        DeleteEnclosedFrame(&frames[i]);
+      }
     }
 
     if (last_packet != NULL) {
@@ -459,13 +524,13 @@ QuicConsumedData QuicConnection::SendStreamData(
 void QuicConnection::SendRstStream(QuicStreamId id,
                                    QuicErrorCode error,
                                    QuicStreamOffset offset) {
-  // TODO(ianswett): Queue the frame and use WriteData instead of SendPacket
-  // once we support re-sending frames instead of packets.
-  PacketPair packetpair = packet_creator_.ResetStream(id, offset, error);
+  queued_control_frames_.push_back(QuicFrame(
+      new QuicRstStreamFrame(id, offset, error)));
 
-  SendPacket(packetpair.first, packetpair.second, true, false, false);
-  unacked_packets_.insert(make_pair(packetpair.first,
-                                    UnackedPacket(packetpair.second)));
+  // Try to write immediately if possible.
+  if (CanWrite(false)) {
+    WriteData();
+  }
 }
 
 void QuicConnection::ProcessUdpPacket(const IPEndPoint& self_address,
@@ -506,19 +571,33 @@ bool QuicConnection::OnCanWrite() {
 bool QuicConnection::WriteData() {
   DCHECK_EQ(false, write_blocked_);
   // Serialize the ack and congestion frames before draining the pending queue.
-  QuicFrames frames;
   if (should_send_ack_) {
-    frames.push_back(QuicFrame(&outgoing_ack_));
+    queued_control_frames_.push_back(QuicFrame(&outgoing_ack_));
   }
   if (should_send_congestion_feedback_) {
-    frames.push_back(QuicFrame(&outgoing_congestion_feedback_));
+    queued_control_frames_.push_back(QuicFrame(&outgoing_congestion_feedback_));
   }
-  while (!frames.empty()) {
+  while (!queued_control_frames_.empty()) {
     size_t num_serialized;
-    PacketPair pair = packet_creator_.SerializeFrames(frames, &num_serialized);
+    PacketPair pair = packet_creator_.SerializeFrames(
+        queued_control_frames_, &num_serialized);
+    // If any serialized frames need to be resent, add them to unacked_packets.
+    QuicFrames unacked_frames;
+    for (QuicFrames::const_iterator iter = queued_control_frames_.begin();
+         iter != queued_control_frames_.begin() + num_serialized; ++iter) {
+      if (ShouldResend(*iter)) {
+        unacked_frames.push_back(*iter);
+      }
+    }
+    if (!unacked_frames.empty()) {
+      unacked_packets_.insert(make_pair(pair.first,
+                                        new UnackedPacket(unacked_frames)));
+    }
     queued_packets_.push_back(QueuedPacket(
-        pair.first, pair.second, false, false));
-    frames.erase(frames.begin(), frames.begin() + num_serialized);
+        pair.first, pair.second, !unacked_frames.empty(), false));
+    queued_control_frames_.erase(
+        queued_control_frames_.begin(),
+        queued_control_frames_.begin() + num_serialized);
   }
   should_send_ack_ = false;
   should_send_congestion_feedback_ = false;
@@ -566,22 +645,20 @@ void QuicConnection::MaybeResendPacket(
   UnackedPacketMap::iterator it = unacked_packets_.find(sequence_number);
 
   if (it != unacked_packets_.end()) {
-    QuicPacket* packet = it->second.packet;
+    UnackedPacket* unacked = it->second;
+    // TODO(ianswett): Never change the sequence number of the connect packet.
     unacked_packets_.erase(it);
-    // Re-frame the packet with a new sequence number for resend.
-    QuicPacketSequenceNumber new_sequence_number  =
-        packet_creator_.SetNewSequenceNumber(packet);
+    // Re-packetize the frames with a new sequence number for resend.
+    // Resent data packets do not use FEC, even when it's enabled.
+    PacketPair packetpair = packet_creator_.SerializeAllFrames(unacked->frames);
     DVLOG(1) << "Resending unacked packet " << sequence_number << " as "
-             << new_sequence_number;
-    // Clear the FEC group.
-    framer_.WriteFecGroup(0u, packet);
-    unacked_packets_.insert(make_pair(new_sequence_number,
-                                      UnackedPacket(packet)));
+             << packetpair.first;
+    unacked_packets_.insert(make_pair(packetpair.first, unacked));
 
     // Make sure if this was our least unacked packet, that we update our
     // outgoing ack.  If this wasn't the least unacked, this is a no-op.
     UpdateLeastUnacked(sequence_number);
-    SendPacket(new_sequence_number, packet, true, false, true);
+    SendPacket(packetpair.first, packetpair.second, true, false, true);
   } else {
     DVLOG(2) << "alarm fired for " << sequence_number
              << " but it has been acked";
@@ -658,7 +735,7 @@ bool QuicConnection::SendPacket(QuicPacketSequenceNumber sequence_number,
   DVLOG(1) << "last packet: " << time_of_last_packet_.ToMicroseconds();
 
   scheduler_->SentPacket(sequence_number, packet->length(), is_retransmit);
-  if (!should_resend) delete packet;
+  delete packet;
   return true;
 }
 
diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h
index d69a3a2..1e3bf81 100644
--- a/net/quic/quic_connection.h
+++ b/net/quic/quic_connection.h
@@ -136,6 +136,7 @@ class NET_EXPORT_PRIVATE QuicConnection : public QuicFramerVisitorInterface {
                  QuicConnectionHelperInterface* helper);
   virtual ~QuicConnection();
 
+  static void DeleteEnclosedFrame(QuicFrame* frame);
 
   // Send the data payload to the peer.
   // Returns a pair with the number of bytes consumed from data, and a boolean
@@ -233,7 +234,8 @@ class NET_EXPORT_PRIVATE QuicConnection : public QuicFramerVisitorInterface {
   // data, and contents will be resent with a new sequence number if we don't
   // get an ack.  If force is true, then the packet will be sent immediately and
   // the send scheduler will not be consulted.  If is_retransmit is true, this
-  // packet is being retransmitted with a new sequence number.
+  // packet is being retransmitted with a new sequence number.  Always takes
+  // ownership of packet.
   // TODO(wtc): none of the callers check the return value.
   virtual bool SendPacket(QuicPacketSequenceNumber number,
                           QuicPacket* packet,
@@ -285,17 +287,19 @@ class NET_EXPORT_PRIVATE QuicConnection : public QuicFramerVisitorInterface {
   };
 
   struct UnackedPacket {
-    explicit UnackedPacket(QuicPacket* packet)
-        : packet(packet),
-          number_nacks(0) {
-    }
-    QuicPacket* packet;
+    explicit UnackedPacket(QuicFrames unacked_frames);
+    UnackedPacket(QuicFrames unacked_frames, std::string data);
+    ~UnackedPacket();
+
+    QuicFrames frames;
     uint8 number_nacks;
+    // Data referenced by the StringPiece of a QuicStreamFrame.
+    std::string data;
   };
 
   typedef std::list<QueuedPacket> QueuedPacketList;
   typedef base::hash_map<QuicPacketSequenceNumber,
-      UnackedPacket> UnackedPacketMap;
+      UnackedPacket*> UnackedPacketMap;
   typedef std::map<QuicFecGroupNumber, QuicFecGroup*> FecGroupMap;
 
   // The amount of time we wait before resending a packet.
@@ -303,6 +307,9 @@ class NET_EXPORT_PRIVATE QuicConnection : public QuicFramerVisitorInterface {
     return QuicTime::Delta::FromMilliseconds(500);
   }
 
+  static void DeleteUnackedPacket(UnackedPacket* unacked);
+  static bool ShouldResend(const QuicFrame& frame);
+
   // Checks if a packet can be written now, and sets the timer if necessary.
   bool CanWrite(bool is_retransmit);
 
@@ -350,15 +357,17 @@ class NET_EXPORT_PRIVATE QuicConnection : public QuicFramerVisitorInterface {
   QuicPacketSequenceNumber peer_least_packet_awaiting_ack_;
 
   // When new packets are created which may be resent, they are added
-  // to this map, which contains owning pointers.
+  // to this map, which contains owning pointers to the contained frames.
   UnackedPacketMap unacked_packets_;
 
   // When packets could not be sent because the socket was not writable,
-  // they are added to this list.  For packets that are not resendable, this
-  // list contains owning pointers, since they are not added to
-  // unacked_packets_.
+  // they are added to this list.  All corresponding frames are in
+  // unacked_packets_ if they are to be resent.
   QueuedPacketList queued_packets_;
 
+  // Pending control frames, besides the ack and congestion control frames.
+  QuicFrames queued_control_frames_;
+
   // True when the socket becomes unwritable.
   bool write_blocked_;
 
diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc
index c85a15e..4f0bf6e 100644
--- a/net/quic/quic_connection_test.cc
+++ b/net/quic/quic_connection_test.cc
@@ -903,50 +903,50 @@ TEST_F(QuicConnectionTest, TimeoutAfterSend) {
 // TODO(ianswett): Add scheduler tests when resend is false.
 TEST_F(QuicConnectionTest, SendScheduler) {
   // Test that if we send a packet without delay, it is not queued.
-  scoped_ptr<QuicPacket> packet(ConstructDataPacket(1, 0));
+  QuicPacket* packet = ConstructDataPacket(1, 0);
   EXPECT_CALL(*scheduler_, TimeUntilSend(false)).WillOnce(testing::Return(
       QuicTime::Delta()));
   EXPECT_CALL(*scheduler_, SentPacket(_, _, _));
-  connection_.SendPacket(1, packet.get(), true, false, false);
+  connection_.SendPacket(1, packet, true, false, false);
   EXPECT_EQ(0u, connection_.NumQueuedPackets());
 }
 
 TEST_F(QuicConnectionTest, SendSchedulerDelay) {
   // Test that if we send a packet with a delay, it ends up queued.
-  scoped_ptr<QuicPacket> packet(ConstructDataPacket(1, 0));
+  QuicPacket* packet = ConstructDataPacket(1, 0);
   EXPECT_CALL(*scheduler_, TimeUntilSend(false)).WillOnce(testing::Return(
       QuicTime::Delta::FromMicroseconds(1)));
   EXPECT_CALL(*scheduler_, SentPacket(1, _, _)).Times(0);
-  connection_.SendPacket(1, packet.get(), true, false, false);
+  connection_.SendPacket(1, packet, true, false, false);
   EXPECT_EQ(1u, connection_.NumQueuedPackets());
 }
 
 TEST_F(QuicConnectionTest, SendSchedulerForce) {
   // Test that if we force send a packet, it is not queued.
-  scoped_ptr<QuicPacket> packet(ConstructDataPacket(1, 0));
+  QuicPacket* packet = ConstructDataPacket(1, 0);
   EXPECT_CALL(*scheduler_, TimeUntilSend(true)).Times(0);
   EXPECT_CALL(*scheduler_, SentPacket(_, _, _));
-  connection_.SendPacket(1, packet.get(), true, true, false);
+  connection_.SendPacket(1, packet, true, true, false);
   EXPECT_EQ(0u, connection_.NumQueuedPackets());
 }
 
 // TODO(rch): Enable after we get non-blocking sockets.
 TEST_F(QuicConnectionTest, DISABLED_SendSchedulerEAGAIN) {
-  scoped_ptr<QuicPacket> packet(ConstructDataPacket(1, 0));
+  QuicPacket* packet = ConstructDataPacket(1, 0);
   helper_->set_blocked(true);
   EXPECT_CALL(*scheduler_, TimeUntilSend(false)).WillOnce(testing::Return(
       QuicTime::Delta()));
   EXPECT_CALL(*scheduler_, SentPacket(1, _, _)).Times(0);
-  connection_.SendPacket(1, packet.get(), true, false, false);
+  connection_.SendPacket(1, packet, true, false, false);
   EXPECT_EQ(1u, connection_.NumQueuedPackets());
 }
 
 TEST_F(QuicConnectionTest, SendSchedulerDelayThenSend) {
   // Test that if we send a packet with a delay, it ends up queued.
-  scoped_ptr<QuicPacket> packet(ConstructDataPacket(1, 0));
+  QuicPacket* packet = ConstructDataPacket(1, 0);
   EXPECT_CALL(*scheduler_, TimeUntilSend(false)).WillOnce(testing::Return(
       QuicTime::Delta::FromMicroseconds(1)));
-  connection_.SendPacket(1, packet.get(), true, false, false);
+  connection_.SendPacket(1, packet, true, false, false);
   EXPECT_EQ(1u, connection_.NumQueuedPackets());
 
   // Advance the clock to fire the alarm, and configure the scheduler
@@ -962,10 +962,10 @@ TEST_F(QuicConnectionTest, SendSchedulerDelayThenSend) {
 
 TEST_F(QuicConnectionTest, SendSchedulerDelayThenRetransmit) {
   // Test that if we send a retransmit with a delay, it ends up queued.
-  scoped_ptr<QuicPacket> packet(ConstructDataPacket(1, 0));
+  QuicPacket* packet = ConstructDataPacket(1, 0);
   EXPECT_CALL(*scheduler_, TimeUntilSend(true)).WillOnce(testing::Return(
       QuicTime::Delta::FromMicroseconds(1)));
-  connection_.SendPacket(1, packet.get(), true, false, true);
+  connection_.SendPacket(1, packet, true, false, true);
   EXPECT_EQ(1u, connection_.NumQueuedPackets());
 
   // Advance the clock to fire the alarm, and configure the scheduler
@@ -982,22 +982,23 @@ TEST_F(QuicConnectionTest, SendSchedulerDelayThenRetransmit) {
 }
 
 TEST_F(QuicConnectionTest, SendSchedulerDelayAndQueue) {
-  scoped_ptr<QuicPacket> packet(ConstructDataPacket(1, 0));
+  QuicPacket* packet = ConstructDataPacket(1, 0);
   EXPECT_CALL(*scheduler_, TimeUntilSend(false)).WillOnce(testing::Return(
       QuicTime::Delta::FromMicroseconds(1)));
-  connection_.SendPacket(1, packet.get(), true, false, false);
+  connection_.SendPacket(1, packet, true, false, false);
   EXPECT_EQ(1u, connection_.NumQueuedPackets());
 
   // Attempt to send another packet and make sure that it gets queued.
-  connection_.SendPacket(2, packet.get(), true, false, false);
+  packet = ConstructDataPacket(2, 0);
+  connection_.SendPacket(2, packet, true, false, false);
   EXPECT_EQ(2u, connection_.NumQueuedPackets());
 }
 
 TEST_F(QuicConnectionTest, SendSchedulerDelayThenAckAndSend) {
-  scoped_ptr<QuicPacket> packet(ConstructDataPacket(1, 0));
+  QuicPacket* packet = ConstructDataPacket(1, 0);
   EXPECT_CALL(*scheduler_, TimeUntilSend(false)).WillOnce(testing::Return(
       QuicTime::Delta::FromMicroseconds(10)));
-  connection_.SendPacket(1, packet.get(), true, false, false);
+  connection_.SendPacket(1, packet, true, false, false);
   EXPECT_EQ(1u, connection_.NumQueuedPackets());
 
   // Now send non-retransmitting information, that we're not going to resend 3.
@@ -1015,10 +1016,10 @@ TEST_F(QuicConnectionTest, SendSchedulerDelayThenAckAndSend) {
 }
 
 TEST_F(QuicConnectionTest, SendSchedulerDelayThenAckAndHold) {
-  scoped_ptr<QuicPacket> packet(ConstructDataPacket(1, 0));
+  QuicPacket* packet = ConstructDataPacket(1, 0);
   EXPECT_CALL(*scheduler_, TimeUntilSend(false)).WillOnce(testing::Return(
       QuicTime::Delta::FromMicroseconds(10)));
-  connection_.SendPacket(1, packet.get(), true, false, false);
+  connection_.SendPacket(1, packet, true, false, false);
   EXPECT_EQ(1u, connection_.NumQueuedPackets());
 
   // Now send non-resending information, that we're not going to resend 3.
@@ -1032,10 +1033,10 @@ TEST_F(QuicConnectionTest, SendSchedulerDelayThenAckAndHold) {
 }
 
 TEST_F(QuicConnectionTest, SendSchedulerDelayThenOnCanWrite) {
-  scoped_ptr<QuicPacket> packet(ConstructDataPacket(1, 0));
+  QuicPacket* packet = ConstructDataPacket(1, 0);
   EXPECT_CALL(*scheduler_, TimeUntilSend(false)).WillOnce(testing::Return(
       QuicTime::Delta::FromMicroseconds(10)));
-  connection_.SendPacket(1, packet.get(), true, false, false);
+  connection_.SendPacket(1, packet, true, false, false);
   EXPECT_EQ(1u, connection_.NumQueuedPackets());
 
   // OnCanWrite should not send the packet (because of the delay)
diff --git a/net/quic/quic_data_writer.cc b/net/quic/quic_data_writer.cc
index 31ec389..c4f7552 100644
--- a/net/quic/quic_data_writer.cc
+++ b/net/quic/quic_data_writer.cc
@@ -98,36 +98,6 @@ bool QuicDataWriter::WriteBytes(const void* data, size_t data_len) {
   return true;
 }
 
-void QuicDataWriter::WriteUint8ToBuffer(uint8 value, char* buffer) {
-  memcpy(buffer, &value, sizeof(value));
-}
-
-void QuicDataWriter::WriteUint16ToBuffer(uint16 value, char* buffer) {
-  memcpy(buffer, &value, sizeof(value));
-}
-
-void QuicDataWriter::WriteUint32ToBuffer(uint32 value, char* buffer) {
-  memcpy(buffer, &value, sizeof(value));
-}
-
-void QuicDataWriter::WriteUint48ToBuffer(uint64 value, char* buffer) {
-  uint16 hi = value >> 32;
-  uint32 lo = value & 0x00000000FFFFFFFF;
-  WriteUint32ToBuffer(lo, buffer);
-  WriteUint16ToBuffer(hi, buffer + sizeof(lo));
-}
-
-void QuicDataWriter::WriteUint64ToBuffer(uint64 value, char* buffer) {
-  memcpy(buffer, &value, sizeof(value));
-}
-
-void QuicDataWriter::WriteUint128ToBuffer(uint128 value, char* buffer) {
-  uint64 high = Uint128High64(value);
-  uint64 low = Uint128Low64(value);
-  WriteUint64ToBuffer(low, buffer);
-  WriteUint64ToBuffer(high, buffer + sizeof(low));
-}
-
 bool QuicDataWriter::WriteUInt8ToOffset(uint8 value, size_t offset) {
   DCHECK_LT(offset, capacity_);
   int latched_length = length_;
diff --git a/net/quic/quic_data_writer.h b/net/quic/quic_data_writer.h
index d5f9bb6..3ca4fa3 100644
--- a/net/quic/quic_data_writer.h
+++ b/net/quic/quic_data_writer.h
@@ -53,13 +53,6 @@ class NET_EXPORT_PRIVATE QuicDataWriter {
   bool WriteUInt8ToOffset(uint8 value, size_t offset);
   bool WriteUInt48ToOffset(uint64 value, size_t offset);
 
-  static void WriteUint8ToBuffer(uint8 value, char* buffer);
-  static void WriteUint16ToBuffer(uint16 value, char* buffer);
-  static void WriteUint32ToBuffer(uint32 value, char* buffer);
-  static void WriteUint48ToBuffer(uint64 value, char* buffer);
-  static void WriteUint64ToBuffer(uint64 value, char* buffer);
-  static void WriteUint128ToBuffer(uint128 value, char* buffer);
-
   size_t capacity() const {
     return capacity_;
   }
diff --git a/net/quic/quic_framer.cc b/net/quic/quic_framer.cc
index f5ec5e1..cae0b85 100644
--- a/net/quic/quic_framer.cc
+++ b/net/quic/quic_framer.cc
@@ -51,6 +51,7 @@ QuicPacket* QuicFramer::ConstructMaxFrameDataPacket(
     const QuicPacketHeader& header,
     const QuicFrames& frames,
     size_t* num_consumed) {
+  DCHECK(!frames.empty());
   // Compute the length of the packet.  We use "magic numbers" here because
   // sizeof(member_) is not necessarily the same as sizeof(member_wire_format).
   const size_t max_plaintext_size = GetMaxPlaintextSize(kMaxPacketSize);
@@ -89,7 +90,7 @@ QuicPacket* QuicFramer::ConstructMaxFrameDataPacket(
   }
 
   // frame count
-  if (frames.size() > 256u) {
+  if (*num_consumed > 256u) {
     return NULL;
   }
   if (!writer.WriteUInt8(*num_consumed)) {
@@ -608,18 +609,6 @@ bool QuicFramer::ProcessConnectionCloseFrame() {
   return true;
 }
 
-void QuicFramer::WriteSequenceNumber(QuicPacketSequenceNumber sequence_number,
-                                     QuicPacket* packet) {
-  QuicDataWriter::WriteUint48ToBuffer(
-      sequence_number, packet->mutable_data() + kSequenceNumberOffset);
-}
-
-void QuicFramer::WriteFecGroup(QuicFecGroupNumber fec_group,
-                               QuicPacket* packet) {
-  QuicDataWriter::WriteUint8ToBuffer(
-      fec_group, packet->mutable_data() + kFecGroupOffset);
-}
-
 QuicEncryptedPacket* QuicFramer::EncryptPacket(const QuicPacket& packet) {
   scoped_ptr<QuicData> out(encrypter_->Encrypt(packet.AssociatedData(),
                                                packet.Plaintext()));
diff --git a/net/quic/quic_framer.h b/net/quic/quic_framer.h
index f403e14..f749e29 100644
--- a/net/quic/quic_framer.h
+++ b/net/quic/quic_framer.h
@@ -154,11 +154,6 @@ class NET_EXPORT_PRIVATE QuicFramer {
   QuicPacket* ConstructFecPacket(const QuicPacketHeader& header,
                                  const QuicFecData& fec);
 
-  void WriteSequenceNumber(QuicPacketSequenceNumber sequence_number,
-                           QuicPacket* packet);
-
-  void WriteFecGroup(QuicFecGroupNumber fec_group, QuicPacket* packet);
-
   // Returns a new encrypted packet, owned by the caller.
   QuicEncryptedPacket* EncryptPacket(const QuicPacket& packet);
 
diff --git a/net/quic/quic_packet_creator.cc b/net/quic/quic_packet_creator.cc
index d9d6c40..769434d 100644
--- a/net/quic/quic_packet_creator.cc
+++ b/net/quic/quic_packet_creator.cc
@@ -34,7 +34,14 @@ void QuicPacketCreator::OnBuiltFecProtectedPayload(
   }
 }
 
-QuicPacketCreator::PacketPair QuicPacketCreator::SerializeFrames(
+PacketPair QuicPacketCreator::SerializeAllFrames(const QuicFrames& frames) {
+  size_t num_serialized;
+  PacketPair pair = SerializeFrames(frames, &num_serialized);
+  DCHECK_EQ(frames.size(), num_serialized);
+  return pair;
+}
+
+PacketPair QuicPacketCreator::SerializeFrames(
     const QuicFrames& frames, size_t* num_serialized) {
   QuicPacketHeader header;
   FillPacketHeader(0, PACKET_FLAGS_NONE, &header);
@@ -48,7 +55,8 @@ size_t QuicPacketCreator::DataToStream(QuicStreamId id,
                                        StringPiece data,
                                        QuicStreamOffset offset,
                                        bool fin,
-                                       vector<PacketPair>* packets) {
+                                       vector<PacketPair>* packets,
+                                       QuicFrames* packetized_frames) {
   DCHECK_GT(options_.max_packet_length,
             QuicUtils::StreamFramePacketOverhead(1));
   DCHECK_LT(0u, options_.max_num_packets);
@@ -89,8 +97,9 @@ size_t QuicPacketCreator::DataToStream(QuicStreamId id,
       StringPiece data_frame(data.data() + data.size() - unconsumed_bytes,
                                 frame_len);
 
-      QuicStreamFrame frame(id, set_fin, offset, data_frame);
-      frames.push_back(QuicFrame(&frame));
+      QuicStreamFrame* frame = new QuicStreamFrame(
+          id, set_fin, offset, data_frame);
+      frames.push_back(QuicFrame(frame));
       FillPacketHeader(current_fec_group, PACKET_FLAGS_NONE, &header);
       offset += frame_len;
       unconsumed_bytes -= frame_len;
@@ -100,6 +109,7 @@ size_t QuicPacketCreator::DataToStream(QuicStreamId id,
       DCHECK(packet);
       DCHECK_GE(options_.max_packet_length, packet->length());
       packets->push_back(make_pair(header.packet_sequence_number, packet));
+      packetized_frames->push_back(frames[0]);
       frames.clear();
     }
     // If we haven't finished serializing all the data, don't set any final fin.
@@ -111,11 +121,13 @@ size_t QuicPacketCreator::DataToStream(QuicStreamId id,
   // Create a new packet for the fin, if necessary.
   if (fin && data.size() == 0) {
     FillPacketHeader(current_fec_group, PACKET_FLAGS_NONE, &header);
-    QuicStreamFrame frame(id, true, offset, "");
-    frames.push_back(QuicFrame(&frame));
+    QuicStreamFrame* frame = new QuicStreamFrame(id, true, offset, "");
+    frames.push_back(QuicFrame(frame));
     packet = framer_->ConstructFrameDataPacket(header, frames);
     DCHECK(packet);
+    DCHECK_GE(options_.max_packet_length, packet->length());
     packets->push_back(make_pair(header.packet_sequence_number, packet));
+    packetized_frames->push_back(frames[0]);
     frames.clear();
   }
 
@@ -125,6 +137,7 @@ size_t QuicPacketCreator::DataToStream(QuicStreamId id,
     fec_data.redundancy = fec_group_->parity();
     QuicPacket* fec_packet = framer_->ConstructFecPacket(header, fec_data);
     DCHECK(fec_packet);
+    DCHECK_GE(options_.max_packet_length, packet->length());
     packets->push_back(make_pair(header.packet_sequence_number, fec_packet));
     ++fec_group_number_;
   }
@@ -150,22 +163,6 @@ size_t QuicPacketCreator::DataToStream(QuicStreamId id,
   return data.size() - unconsumed_bytes;
 }
 
-QuicPacketCreator::PacketPair QuicPacketCreator::ResetStream(
-    QuicStreamId id,
-    QuicStreamOffset offset,
-    QuicErrorCode error) {
-  QuicPacketHeader header;
-  FillPacketHeader(0, PACKET_FLAGS_NONE, &header);
-
-  QuicRstStreamFrame close_frame(id, offset, error);
-
-  QuicFrames frames;
-  frames.push_back(QuicFrame(&close_frame));
-  QuicPacket* packet = framer_->ConstructFrameDataPacket(header, frames);
-  DCHECK(packet);
-  return make_pair(header.packet_sequence_number, packet);
-}
-
 QuicPacketCreator::PacketPair QuicPacketCreator::CloseConnection(
     QuicConnectionCloseFrame* close_frame) {
 
@@ -179,13 +176,6 @@ QuicPacketCreator::PacketPair QuicPacketCreator::CloseConnection(
   return make_pair(header.packet_sequence_number, packet);
 }
 
-QuicPacketSequenceNumber QuicPacketCreator::SetNewSequenceNumber(
-    QuicPacket* packet) {
-  ++sequence_number_;
-  framer_->WriteSequenceNumber(sequence_number_, packet);
-  return sequence_number_;
-}
-
 void QuicPacketCreator::FillPacketHeader(QuicFecGroupNumber fec_group,
                                          QuicPacketFlags flags,
                                          QuicPacketHeader* header) {
diff --git a/net/quic/quic_packet_creator.h b/net/quic/quic_packet_creator.h
index 62d54c2..4f11348 100644
--- a/net/quic/quic_packet_creator.h
+++ b/net/quic/quic_packet_creator.h
@@ -52,30 +52,27 @@ class NET_EXPORT_PRIVATE QuicPacketCreator : public QuicFecBuilderInterface {
 
   typedef std::pair<QuicPacketSequenceNumber, QuicPacket*> PacketPair;
 
+  // Serializes all frames into a single packet.  All frames must fit into a
+  // single packet.
+  PacketPair SerializeAllFrames(const QuicFrames& frames);
+
   // Serializes as many non-fec frames as can fit into a single packet.
   // num_serialized is set to the number of frames serialized into the packet.
   PacketPair SerializeFrames(const QuicFrames& frames, size_t* num_serialized);
 
-  // Converts a raw payload to a series of QuicPackets.  Returns the number of
-  // bytes consumed from data.
+  // Converts a raw payload to a series of QuicPackets and matching frames in
+  // those packets.  Returns the number of bytes consumed from data.
   // If data is empty and fin is true, the expected behavior is to consume the
   // fin but return 0.
   size_t DataToStream(QuicStreamId id,
                       base::StringPiece data,
                       QuicStreamOffset offset,
                       bool fin,
-                      std::vector<PacketPair>* packets);
-
-  PacketPair ResetStream(QuicStreamId id,
-                         QuicStreamOffset offset,
-                         QuicErrorCode error);
+                      std::vector<PacketPair>* packets,
+                      QuicFrames* packetized_frames);
 
   PacketPair CloseConnection(QuicConnectionCloseFrame* close_frame);
 
-  // Increments the current sequence number in QuicPacketCreator and sets it
-  // into the packet and returns the new sequence number.
-  QuicPacketSequenceNumber SetNewSequenceNumber(QuicPacket* packet);
-
   QuicPacketSequenceNumber sequence_number() const {
     return sequence_number_;
   }
diff --git a/net/quic/quic_packet_creator_test.cc b/net/quic/quic_packet_creator_test.cc
index c03913d..d4835b1 100644
--- a/net/quic/quic_packet_creator_test.cc
+++ b/net/quic/quic_packet_creator_test.cc
@@ -32,6 +32,9 @@ class QuicPacketCreatorTest : public ::testing::Test {
   }
   ~QuicPacketCreatorTest() {
     STLDeleteValues(&packets_);
+    for (QuicFrames::iterator it = frames_.begin(); it != frames_.end(); ++it) {
+      QuicConnection::DeleteEnclosedFrame(&(*it));
+    }
   }
 
   void ProcessPackets() {
@@ -43,6 +46,7 @@ class QuicPacketCreatorTest : public ::testing::Test {
   }
 
   vector<QuicPacketCreator::PacketPair> packets_;
+  QuicFrames frames_;
   QuicFramer framer_;
   testing::StrictMock<MockFramerVisitor> framer_visitor_;
   QuicStreamId id_;
@@ -53,7 +57,8 @@ class QuicPacketCreatorTest : public ::testing::Test {
 };
 
 TEST_F(QuicPacketCreatorTest, DataToStreamBasic) {
-  size_t bytes_consumed = utils_.DataToStream(id_, data_, 0, true, &packets_);
+  size_t bytes_consumed = utils_.DataToStream(
+      id_, data_, 0, true, &packets_, &frames_);
 
   ASSERT_EQ(1u, packets_.size());
   ASSERT_EQ(1u, utils_.sequence_number());
@@ -70,7 +75,8 @@ TEST_F(QuicPacketCreatorTest, DataToStreamBasic) {
 
 TEST_F(QuicPacketCreatorTest, DataToStreamFec) {
   utils_.options()->use_fec = true;
-  size_t bytes_consumed = utils_.DataToStream(id_, data_, 0, true, &packets_);
+  size_t bytes_consumed = utils_.DataToStream(
+      id_, data_, 0, true, &packets_, &frames_);
 
   ASSERT_EQ(2u, packets_.size());
   ASSERT_EQ(2u, utils_.sequence_number());
@@ -93,7 +99,8 @@ TEST_F(QuicPacketCreatorTest, DataToStreamFec) {
 
 TEST_F(QuicPacketCreatorTest, DataToStreamFecHandled) {
   utils_.options()->use_fec = true;
-  size_t bytes_consumed = utils_.DataToStream(id_, data_, 0, true, &packets_);
+  size_t bytes_consumed = utils_.DataToStream(
+      id_, data_, 0, true, &packets_, &frames_);
   ASSERT_EQ(data_.size(), bytes_consumed);
 
   ASSERT_EQ(2u, packets_.size());
@@ -131,7 +138,8 @@ TEST_F(QuicPacketCreatorTest, DataToStreamFecHandled) {
 }
 
 TEST_F(QuicPacketCreatorTest, DataToStreamSkipFin) {
-  size_t bytes_consumed = utils_.DataToStream(id_, data_, 0, false, &packets_);
+  size_t bytes_consumed = utils_.DataToStream(
+      id_, data_, 0, false, &packets_, &frames_);
   ASSERT_EQ(data_.size(), bytes_consumed);
 
   ASSERT_EQ(1u, packets_.size());
@@ -149,7 +157,8 @@ TEST_F(QuicPacketCreatorTest, DataToStreamSkipFin) {
 TEST_F(QuicPacketCreatorTest, NoData) {
   data_ = "";
 
-  size_t bytes_consumed = utils_.DataToStream(id_, data_, 0, true, &packets_);
+  size_t bytes_consumed = utils_.DataToStream(
+      id_, data_, 0, true, &packets_, &frames_);
   ASSERT_EQ(data_.size(), bytes_consumed);
 
   ASSERT_EQ(1u, packets_.size());
@@ -169,7 +178,8 @@ TEST_F(QuicPacketCreatorTest, MultiplePackets) {
   utils_.options()->max_packet_length =
       ciphertext_size + QuicUtils::StreamFramePacketOverhead(1);
 
-  size_t bytes_consumed = utils_.DataToStream(id_, data_, 0, true, &packets_);
+  size_t bytes_consumed = utils_.DataToStream(
+      id_, data_, 0, true, &packets_, &frames_);
   ASSERT_EQ(data_.size(), bytes_consumed);
 
   ASSERT_EQ(2u, packets_.size());
@@ -198,7 +208,8 @@ TEST_F(QuicPacketCreatorTest, MultiplePacketsWithLimits) {
       ciphertext_size + QuicUtils::StreamFramePacketOverhead(1);
   utils_.options()->max_num_packets = 1;
 
-  size_t bytes_consumed = utils_.DataToStream(id_, data_, 0, true, &packets_);
+  size_t bytes_consumed = utils_.DataToStream(
+      id_, data_, 0, true, &packets_, &frames_);
   ASSERT_EQ(kPayloadBytesPerPacket, bytes_consumed);
 
   ASSERT_EQ(1u, packets_.size());
diff --git a/net/quic/quic_protocol.cc b/net/quic/quic_protocol.cc
index 6ba5468..3d0b4d9 100644
--- a/net/quic/quic_protocol.cc
+++ b/net/quic/quic_protocol.cc
@@ -16,7 +16,7 @@ QuicStreamFrame::QuicStreamFrame() {}
 
 QuicStreamFrame::QuicStreamFrame(QuicStreamId stream_id,
                                  bool fin,
-                                 uint64 offset,
+                                 QuicStreamOffset offset,
                                  StringPiece data)
     : stream_id(stream_id),
       fin(fin),
diff --git a/net/quic/quic_protocol.h b/net/quic/quic_protocol.h
index 45d63a2..f2d60a2 100644
--- a/net/quic/quic_protocol.h
+++ b/net/quic/quic_protocol.h
@@ -150,7 +150,7 @@ enum QuicErrorCode {
 
   // Handshake message contained out of order tags.
   QUIC_CRYPTO_TAGS_OUT_OF_ORDER,
-  // Handshake message contained too many entires.
+  // Handshake message contained too many entries.
   QUIC_CRYPTO_TOO_MANY_ENTRIES,
   // Handshake message contained an invalid value length.
   QUIC_CRYPTO_INVALID_VALUE_LENGTH,
@@ -174,12 +174,12 @@ struct NET_EXPORT_PRIVATE QuicStreamFrame {
   QuicStreamFrame();
   QuicStreamFrame(QuicStreamId stream_id,
                   bool fin,
-                  uint64 offset,
+                  QuicStreamOffset offset,
                   base::StringPiece data);
 
   QuicStreamId stream_id;
   bool fin;
-  uint64 offset;
+  QuicStreamOffset offset;  // Location of this data in the stream.
   base::StringPiece data;
 };
 
diff --git a/net/quic/test_tools/quic_test_utils.cc b/net/quic/test_tools/quic_test_utils.cc
index 0badf39..052bd95 100644
--- a/net/quic/test_tools/quic_test_utils.cc
+++ b/net/quic/test_tools/quic_test_utils.cc
@@ -4,6 +4,7 @@
 
 #include "net/quic/test_tools/quic_test_utils.h"
 
+#include "base/stl_util.h"
 #include "net/quic/crypto/crypto_framer.h"
 #include "net/quic/crypto/crypto_utils.h"
 
@@ -97,6 +98,7 @@ PacketSavingConnection::PacketSavingConnection(QuicGuid guid,
 }
 
 PacketSavingConnection::~PacketSavingConnection() {
+  STLDeleteElements(&packets_);
 }
 
 bool PacketSavingConnection::SendPacket(QuicPacketSequenceNumber number,