Merge recent QUIC changes.

* Add a TODO to fix the in-memory time based fields in the inter arrival congestion feedback struct.  I'd fix them now, but I'm not sure if they'll even be used in their current form.  I'll leave it to patrick to actually implement when the code is ready.

  Merge internal change: 39803339

* Handling truncated ack packets.

  Merge internal change: 39972571

* Various small cleanups to QUIC code found while landing changes in Chrome.

  Merge internal change: 39980760

* Two more fixes for the ever-failing end to end test. Allow truncating connection close frames, since they're short data followed by a truncatable-ack. Check for closed streams in the client's GetStream, rather than closing the connection at an "unexpected" stream. Finally, fast fail in debug mode if we're asked to create a packet and can't: this should never happen since we should break data into small bits, and truncate acks.

  Merge internal change: 39999004

* Test truncated close along with truncate acks

  Merge internal change: 40032708

* Simplify the QuicFramer::Construct* API by returning a QuicPacket*, instead of returning a bool, and setting the packet in an outparam.

  This matches the API of the CryptoFramer.  (Yay, constency :>)

  Merge internal change: 40129101

* Minor cleanup of QuicFramerTest

  Merge internal change: 40130911

BUG=

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

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

24 files changed, 372 insertions, 201 deletions

diff --git a/net/quic/crypto/null_decrypter_test.cc b/net/quic/crypto/null_decrypter_test.cc
index 71f1970..ad73cea 100644
--- a/net/quic/crypto/null_decrypter_test.cc
+++ b/net/quic/crypto/null_decrypter_test.cc
@@ -8,7 +8,6 @@
 using base::StringPiece;
 
 namespace net {
-
 namespace test {
 
 TEST(NullDecrypterTest, Decrypt) {
@@ -67,5 +66,4 @@ TEST(NullDecrypterTest, ShortInput) {
 }
 
 }  // namespace test
-
 }  // namespace net
diff --git a/net/quic/crypto/null_encrypter_test.cc b/net/quic/crypto/null_encrypter_test.cc
index f027af4..9f2cec2 100644
--- a/net/quic/crypto/null_encrypter_test.cc
+++ b/net/quic/crypto/null_encrypter_test.cc
@@ -8,7 +8,6 @@
 using base::StringPiece;
 
 namespace net {
-
 namespace test {
 
 TEST(NullEncrypterTest, Encrypt) {
@@ -46,5 +45,4 @@ TEST(NullEncrypterTest, GetCiphertextSize) {
 }
 
 }  // namespace test
-
 }  // namespace net
diff --git a/net/quic/crypto/quic_random_test.cc b/net/quic/crypto/quic_random_test.cc
index 655ae98..425089a 100644
--- a/net/quic/crypto/quic_random_test.cc
+++ b/net/quic/crypto/quic_random_test.cc
@@ -7,7 +7,6 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
-
 namespace test {
 
 TEST(QuicRandomTest, RandBytes) {
@@ -38,5 +37,4 @@ TEST(QuicRandomTest, Reseed) {
 }
 
 }  // namespace test
-
 }  // namespace net
diff --git a/net/quic/quic_clock_test.cc b/net/quic/quic_clock_test.cc
index 839f011c..00323ba 100644
--- a/net/quic/quic_clock_test.cc
+++ b/net/quic/quic_clock_test.cc
@@ -7,7 +7,6 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
-
 namespace test {
 
 TEST(QuicClockTest, Now) {
@@ -22,5 +21,4 @@ TEST(QuicClockTest, Now) {
 }
 
 }  // namespace test
-
 }  // namespace net
diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc
index e77369a..3bda17d 100644
--- a/net/quic/quic_connection.cc
+++ b/net/quic/quic_connection.cc
@@ -24,9 +24,15 @@ using std::set;
 
 namespace net {
 
+// TODO(pwestin): kDefaultTimeoutUs is in int64.
+int32 kNegotiatedTimeoutUs = kDefaultTimeoutUs;
+
+// The largest gap in packets we'll accept without closing the connection.
+// This will likely have to be tuned.
+const QuicPacketSequenceNumber kMaxPacketGap = 5000;
+
 // The maximum number of nacks which can be transmitted in a single ack packet
-// without exceeding kMaxPacketSize.  Due to nacks, this also sets a limit on
-// the sequence number gap for two consecutive packets.
+// without exceeding kMaxPacketSize.
 const QuicPacketSequenceNumber kMaxUnackedPackets = 192u;
 
 // The amount of time we wait before resending a packet.
@@ -53,7 +59,7 @@ const int kMaxPacketsToSerializeAtOnce = 6;
 
 bool Near(QuicPacketSequenceNumber a, QuicPacketSequenceNumber b) {
   QuicPacketSequenceNumber delta = (a > b) ? a - b : b - a;
-  return delta <= kMaxUnackedPackets;
+  return delta <= kMaxPacketGap;
 }
 
 QuicConnection::QuicConnection(QuicGuid guid,
@@ -125,9 +131,9 @@ void QuicConnection::OnRevivedPacket() {
 bool QuicConnection::OnPacketHeader(const QuicPacketHeader& header) {
   if (!Near(header.packet_sequence_number,
             last_header_.packet_sequence_number)) {
-    DLOG(INFO) << "Packet out of bounds.  Discarding";
-    // TODO(ianswett): Deal with this by truncating the ack packet instead of
-    // discarding the packet entirely.
+    DLOG(INFO) << "Packet " << header.packet_sequence_number
+               << " out of bounds.  Discarding";
+    // TODO(alyssar) close the connection entirely.
     return false;
   }
 
@@ -203,7 +209,7 @@ bool QuicConnection::ValidateAckFrame(const QuicAckFrame& incoming_ack) {
 
   // We can't have too many unacked packets, or our ack frames go over
   // kMaxPacketSize.
-  DCHECK_LT(incoming_ack.received_info.missing_packets.size(),
+  DCHECK_LE(incoming_ack.received_info.missing_packets.size(),
             kMaxUnackedPackets);
 
   if (incoming_ack.sent_info.least_unacked < least_packet_awaiting_ack_) {
@@ -223,17 +229,6 @@ bool QuicConnection::ValidateAckFrame(const QuicAckFrame& incoming_ack) {
     return false;
   }
 
-  for (SequenceSet::const_iterator iter =
-           incoming_ack.received_info.missing_packets.begin();
-       iter != incoming_ack.received_info.missing_packets.end(); ++iter) {
-    if (*iter >= incoming_ack.received_info.largest_received) {
-      DLOG(INFO) << "Cannot nack a packet:" << *iter
-                 << " greater than or equal to largest received:"
-                 << incoming_ack.received_info.largest_received;
-      return false;
-    }
-  }
-
   return true;
 }
 
@@ -514,7 +509,7 @@ void QuicConnection::MaybeResendPacket(
     QuicPacketSequenceNumber new_sequence_number  =
         packet_creator_.SetNewSequenceNumber(packet);
     DVLOG(1) << "Resending unacked packet " << sequence_number << " as "
-               << new_sequence_number;
+             << new_sequence_number;
     // Clear the FEC group.
     framer_.WriteFecGroup(0u, packet);
     unacked_packets_.insert(make_pair(new_sequence_number,
@@ -539,10 +534,6 @@ bool QuicConnection::SendPacket(QuicPacketSequenceNumber sequence_number,
                                 bool should_resend,
                                 bool force,
                                 bool is_retransmit) {
-  DCHECK(packet->length() < kMaxPacketSize)
-      << "Packet " << sequence_number << " will not be read; too large: "
-      << packet->length() << " " << outgoing_ack_;
-
   // If this packet is being forced, don't bother checking to see if we should
   // write, just write.
   if (!force) {
@@ -578,6 +569,10 @@ bool QuicConnection::SendPacket(QuicPacketSequenceNumber sequence_number,
   DLOG(INFO) << "Sending packet : "
              << (should_resend ? "data bearing " : " ack only ")
              << "packet " << sequence_number;
+  DCHECK(encrypted->length() <= kMaxPacketSize)
+      << "Packet " << sequence_number << " will not be read; too large: "
+      << packet->length() << " " << encrypted->length() << " " << outgoing_ack_;
+
   int rv = helper_->WritePacketToWire(*encrypted, &error);
   if (rv == -1) {
     if (error == ERR_IO_PENDING) {
@@ -668,7 +663,6 @@ void QuicConnection::SendConnectionClose(QuicErrorCode error) {
   QuicConnectionCloseFrame frame;
   frame.error_code = error;
   frame.ack_frame = outgoing_ack_;
-  LOG(INFO) << "outgoing_ack_: " << outgoing_ack_;
 
   PacketPair packetpair = packet_creator_.CloseConnection(&frame);
   // There's no point in resending this: we're closing the connection.
diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h
index f98b4e2..761e887 100644
--- a/net/quic/quic_connection.h
+++ b/net/quic/quic_connection.h
@@ -119,6 +119,7 @@ class NET_EXPORT_PRIVATE QuicConnection : public QuicFramerVisitorInterface {
   virtual ~QuicConnection();
 
   // Send the data payload to the peer.
+  // TODO(wtc): document the return value.
   size_t SendStreamData(QuicStreamId id,
                         base::StringPiece data,
                         QuicStreamOffset offset,
@@ -167,6 +168,7 @@ class NET_EXPORT_PRIVATE QuicConnection : public QuicFramerVisitorInterface {
   const IPEndPoint& self_address() const { return self_address_; }
   const IPEndPoint& peer_address() const { return peer_address_; }
   QuicGuid guid() const { return guid_; }
+  const QuicClock* clock() const { return clock_; }
 
   // Updates the internal state concerning which packets have been acked, and
   // sends an ack if new data frames have been received.
@@ -177,8 +179,6 @@ class NET_EXPORT_PRIVATE QuicConnection : public QuicFramerVisitorInterface {
   // scheduled.
   void MaybeResendPacket(QuicPacketSequenceNumber sequence_number);
 
-  QuicGuid guid() { return guid_; }
-
   QuicPacketCreator::Options* options() { return packet_creator_.options(); }
 
   bool connected() { return connected_; }
@@ -201,6 +201,7 @@ class NET_EXPORT_PRIVATE QuicConnection : public QuicFramerVisitorInterface {
   // 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.
+  // TODO(wtc): none of the callers check the return value.
   virtual bool SendPacket(QuicPacketSequenceNumber number,
                           QuicPacket* packet,
                           bool should_resend,
@@ -228,7 +229,6 @@ class NET_EXPORT_PRIVATE QuicConnection : public QuicFramerVisitorInterface {
   // This finds the new least unacked packet and updates the outgoing ack frame.
   void UpdateLeastUnacked(QuicPacketSequenceNumber acked_sequence_number);
 
-protected:
   QuicConnectionHelperInterface* helper() { return helper_; }
 
  private:
@@ -289,7 +289,7 @@ protected:
   QuicFramer framer_;
   const QuicClock* clock_;
 
-  QuicGuid guid_;
+  const QuicGuid guid_;
   IPEndPoint self_address_;
   IPEndPoint peer_address_;
 
diff --git a/net/quic/quic_connection_helper_test.cc b/net/quic/quic_connection_helper_test.cc
index 95068e0..7528869 100644
--- a/net/quic/quic_connection_helper_test.cc
+++ b/net/quic/quic_connection_helper_test.cc
@@ -184,11 +184,9 @@ class QuicConnectionHelperTest : public ::testing::Test {
                                        const QuicFrame& frame) {
     QuicFrames frames;
     frames.push_back(frame);
-    QuicPacket* packet;
-    framer_.ConstructFrameDataPacket(header_, frames, &packet);
-    QuicEncryptedPacket* encrypted = framer_.EncryptPacket(*packet);
-    delete packet;
-    return encrypted;
+    scoped_ptr<QuicPacket> packet(
+        framer_.ConstructFrameDataPacket(header_, frames));
+    return framer_.EncryptPacket(*packet);
   }
 
   QuicGuid guid_;
diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc
index c6700e7..6e14df7 100644
--- a/net/quic/quic_connection_test.cc
+++ b/net/quic/quic_connection_test.cc
@@ -288,18 +288,17 @@ class QuicConnectionTest : public ::testing::Test {
       }
     }
     fec_data.redundancy = data_packet->FecProtectedData();
-    QuicPacket* fec_packet;
-    framer_.ConstructFecPacket(header_, fec_data, &fec_packet);
+    scoped_ptr<QuicPacket> fec_packet(
+        framer_.ConstructFecPacket(header_, fec_data));
     scoped_ptr<QuicEncryptedPacket> encrypted(
         framer_.EncryptPacket(*fec_packet));
 
     connection_.ProcessUdpPacket(IPEndPoint(), IPEndPoint(), *encrypted);
-    delete fec_packet;
   }
 
   void SendStreamDataToPeer(QuicStreamId id, StringPiece data,
-                      QuicStreamOffset offset, bool fin,
-                      QuicPacketSequenceNumber* last_packet) {
+                            QuicStreamOffset offset, bool fin,
+                            QuicPacketSequenceNumber* last_packet) {
     EXPECT_CALL(*scheduler_, SentPacket(_, _, _));
     connection_.SendStreamData(id, data, offset, fin, last_packet);
   }
@@ -332,8 +331,8 @@ class QuicConnectionTest : public ::testing::Test {
     QuicFrames frames;
     QuicFrame frame(&frame1_);
     frames.push_back(frame);
-    QuicPacket* packet = NULL;
-    EXPECT_TRUE(framer_.ConstructFrameDataPacket(header_, frames, &packet));
+    QuicPacket* packet = framer_.ConstructFrameDataPacket(header_, frames);
+    EXPECT_TRUE(packet != NULL);
     return packet;
   }
 
@@ -492,7 +491,8 @@ TEST_F(QuicConnectionTest, LeastUnackedGreaterThanPacketSequenceNumber) {
   ProcessAckPacket(&frame, false);
 }
 
-TEST_F(QuicConnectionTest, NackSequenceNumberGreaterThanLargestReceived) {
+TEST_F(QuicConnectionTest,
+       DISABLED_NackSequenceNumberGreaterThanLargestReceived) {
   SendStreamDataToPeer(1, "foo", 0, false, NULL);
   SendStreamDataToPeer(1, "bar", 3, false, NULL);
   SendStreamDataToPeer(1, "eep", 6, false, NULL);
diff --git a/net/quic/quic_data_writer.cc b/net/quic/quic_data_writer.cc
index 42b5dc4..0261d94 100644
--- a/net/quic/quic_data_writer.cc
+++ b/net/quic/quic_data_writer.cc
@@ -6,6 +6,7 @@
 
 #include <algorithm>
 #include <limits>
+#include <string>
 
 #include "base/basictypes.h"
 #include "base/logging.h"
@@ -81,7 +82,7 @@ char* QuicDataWriter::BeginWrite(size_t length) {
   return buffer_ + length_;
 }
 
-bool QuicDataWriter::WriteBytes(const void* data, uint32 data_len) {
+bool QuicDataWriter::WriteBytes(const void* data, size_t data_len) {
   char* dest = BeginWrite(data_len);
   if (!dest) {
     return false;
@@ -123,4 +124,20 @@ void QuicDataWriter::WriteUint128ToBuffer(uint128 value, char* buffer) {
   WriteUint64ToBuffer(high, buffer + sizeof(low));
 }
 
+bool QuicDataWriter::WriteUInt8ToOffset(uint8 value, size_t offset) {
+  int latched_length = length_;
+  length_ = offset;
+  bool success = WriteUInt8(value);
+  length_ = latched_length;
+  return success;
+}
+
+bool QuicDataWriter::WriteUInt48ToOffset(uint64 value, size_t offset) {
+  int latched_length = length_;
+  length_ = offset;
+  bool success = WriteUInt48(value);
+  length_ = latched_length;
+  return success;
+}
+
 }  // namespace net
diff --git a/net/quic/quic_data_writer.h b/net/quic/quic_data_writer.h
index e03bb83..c5ecd34 100644
--- a/net/quic/quic_data_writer.h
+++ b/net/quic/quic_data_writer.h
@@ -45,7 +45,12 @@ class NET_EXPORT_PRIVATE QuicDataWriter {
   bool WriteUInt64(uint64 value);
   bool WriteUInt128(uint128 value);
   bool WriteStringPiece16(base::StringPiece val);
-  bool WriteBytes(const void* data, uint32 data_len);
+  bool WriteBytes(const void* data, size_t data_len);
+
+  // Methods for editing the payload at a specific offset.
+  // Return true if there is enough space at that offset, false otherwise.
+  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);
diff --git a/net/quic/quic_framer.cc b/net/quic/quic_framer.cc
index 07693fa..36c0153 100644
--- a/net/quic/quic_framer.cc
+++ b/net/quic/quic_framer.cc
@@ -17,6 +17,8 @@ using std::numeric_limits;
 
 namespace net {
 
+bool kQuicAllowOversizedPacketsForTest = false;
+
 QuicFramer::QuicFramer(QuicDecrypter* decrypter, QuicEncrypter* encrypter)
     : visitor_(NULL),
       fec_builder_(NULL),
@@ -27,10 +29,18 @@ QuicFramer::QuicFramer(QuicDecrypter* decrypter, QuicEncrypter* encrypter)
 
 QuicFramer::~QuicFramer() {}
 
-bool QuicFramer::ConstructFrameDataPacket(
+bool CanTruncate(const QuicFrames& frames) {
+  if (frames.size() == 1 && (
+      frames[0].type == ACK_FRAME ||
+      frames[0].type == CONNECTION_CLOSE_FRAME)) {
+    return true;
+  }
+  return false;
+}
+
+QuicPacket* QuicFramer::ConstructFrameDataPacket(
     const QuicPacketHeader& header,
-    const QuicFrames& frames,
-    QuicPacket** packet) {
+    const QuicFrames& frames) {
   // Compute the length of the packet.  We use "magic numbers" here because
   // sizeof(member_) is not necessarily the same as sizeof(member_wire_format).
   size_t len = kPacketHeaderSize;
@@ -40,76 +50,92 @@ bool QuicFramer::ConstructFrameDataPacket(
     len += ComputeFramePayloadLength(frames[i]);
   }
 
+  bool truncating = false;
+  size_t max_plaintext_size = GetMaxPlaintextSize(kMaxPacketSize);
+  if (len > max_plaintext_size) {
+    if (CanTruncate(frames)) {
+      // Truncate the ack frame so the packet will not exceed kMaxPacketSize.
+      // Note that we may not use every byte of the writer in this case.
+      len = max_plaintext_size;
+      truncating = true;
+      DLOG(INFO) << "Truncating large ack";
+    } else {
+      return NULL;
+    }
+  }
+
   QuicDataWriter writer(len);
 
   if (!WritePacketHeader(header, &writer)) {
-    return false;
+    return NULL;
   }
 
   // frame count
   if (frames.size() > 256u) {
-    return false;
+    return NULL;
   }
   if (!writer.WriteUInt8(frames.size())) {
-    return false;
+    return NULL;
   }
 
   for (size_t i = 0; i < frames.size(); ++i) {
     const QuicFrame& frame = frames[i];
     if (!writer.WriteUInt8(frame.type)) {
-          return false;
+          return NULL;
     }
 
     switch (frame.type) {
       case STREAM_FRAME:
         if (!AppendStreamFramePayload(*frame.stream_frame,
                                          &writer)) {
-          return false;
+          return NULL;
         }
         break;
       case PDU_FRAME:
-        return RaiseError(QUIC_INVALID_FRAME_DATA);
+        RaiseError(QUIC_INVALID_FRAME_DATA);
+        return NULL;
       case ACK_FRAME:
         if (!AppendAckFramePayload(*frame.ack_frame, &writer)) {
-          return false;
+          return NULL;
         }
         break;
       case CONGESTION_FEEDBACK_FRAME:
         if (!AppendQuicCongestionFeedbackFramePayload(
                 *frame.congestion_feedback_frame, &writer)) {
-          return false;
+          return NULL;
         }
         break;
       case RST_STREAM_FRAME:
         if (!AppendRstStreamFramePayload(*frame.rst_stream_frame,
                                             &writer)) {
-          return false;
+          return NULL;
         }
         break;
       case CONNECTION_CLOSE_FRAME:
         if (!AppendConnectionCloseFramePayload(
             *frame.connection_close_frame, &writer)) {
-          return false;
+          return NULL;
         }
         break;
       default:
-        return RaiseError(QUIC_INVALID_FRAME_DATA);
+        RaiseError(QUIC_INVALID_FRAME_DATA);
+        return NULL;
     }
   }
 
-  DCHECK_EQ(len, writer.length());
-  *packet = new QuicPacket(writer.take(), len, true, PACKET_FLAGS_NONE);
+  DCHECK(truncating || len == writer.length());
+  QuicPacket* packet = new QuicPacket(writer.take(), len, true,
+                                      PACKET_FLAGS_NONE);
   if (fec_builder_) {
     fec_builder_->OnBuiltFecProtectedPayload(header,
-                                             (*packet)->FecProtectedData());
+                                             packet->FecProtectedData());
   }
 
-  return true;
+  return packet;
 }
 
-bool QuicFramer::ConstructFecPacket(const QuicPacketHeader& header,
-                                    const QuicFecData& fec,
-                                    QuicPacket** packet) {
+QuicPacket* QuicFramer::ConstructFecPacket(const QuicPacketHeader& header,
+                                           const QuicFecData& fec) {
   // Compute the length of the packet.  We use "magic numbers" here because
   // sizeof(member_) is not necessairly the same as sizeof(member_wire_format).
   size_t len = kPacketHeaderSize;
@@ -119,20 +145,18 @@ bool QuicFramer::ConstructFecPacket(const QuicPacketHeader& header,
   QuicDataWriter writer(len);
 
   if (!WritePacketHeader(header, &writer)) {
-    return false;
+    return NULL;
   }
 
   if (!writer.WriteUInt48(fec.min_protected_packet_sequence_number)) {
-    return false;
+    return NULL;
   }
 
   if (!writer.WriteBytes(fec.redundancy.data(), fec.redundancy.length())) {
-    return false;
+    return NULL;
   }
 
-  *packet = new QuicPacket(writer.take(), len, true, PACKET_FLAGS_FEC);
-
-  return true;
+  return new QuicPacket(writer.take(), len, true, PACKET_FLAGS_FEC);
 }
 
 bool QuicFramer::ProcessPacket(const IPEndPoint& self_address,
@@ -372,10 +396,10 @@ bool QuicFramer::ProcessPDUFrame() {
 }
 
 bool QuicFramer::ProcessAckFrame(QuicAckFrame* frame) {
-  if (!ProcessReceivedInfo(&frame->received_info)) {
+  if (!ProcessSentInfo(&frame->sent_info)) {
     return false;
   }
-  if (!ProcessSentInfo(&frame->sent_info)) {
+  if (!ProcessReceivedInfo(&frame->received_info)) {
     return false;
   }
   visitor_->OnAckFrame(*frame);
@@ -641,7 +665,7 @@ size_t QuicFramer::ComputeFramePayloadLength(const QuicFrame& frame) {
       len += 1;  // num missing packets
       len += 6 * ack.received_info.missing_packets.size();
       len += 6;  // least packet sequence number awaiting an ack
-          break;
+      break;
     }
     case CONGESTION_FEEDBACK_FRAME: {
       const QuicCongestionFeedbackFrame& congestion_feedback =
@@ -719,31 +743,75 @@ bool QuicFramer::AppendStreamFramePayload(
   return true;
 }
 
+// TODO(alyssar): revisit the complexity here to rch's satisfaction
+QuicPacketSequenceNumber QuicFramer::CalculateLargestReceived(
+    const SequenceSet& missing_packets,
+    SequenceSet::const_iterator largest_written) {
+  SequenceSet::const_iterator it = largest_written;
+  QuicPacketSequenceNumber previous_missing = *it;
+  ++it;
+
+  // Try to find a gap in the missing packets: any gap indicates a non-missing
+  // packet which we can then return.
+  for (; it != missing_packets.end(); ++it) {
+    if (previous_missing + 1 != *it) {
+      return *it - 1;
+    }
+    previous_missing = *it;
+  }
+
+  // If we've hit the end of the list, and we're not missing any packets, try
+  // finding a gap between the largest written and the beginning of the set.
+  it = largest_written++;
+  previous_missing = *it;
+  do {
+    --it;
+    if (previous_missing - 1 != *it) {
+      return previous_missing - 1;
+    }
+    previous_missing = *it;
+  } while (it != missing_packets.begin());
+
+  // The missing packets are entirely contiguous.  Return the value of the first
+  // missing packet - 1, as that must have been seen.
+  return  *missing_packets.begin() - 1;
+}
+
 // TODO(ianswett): Use varints or another more compact approach for all deltas.
-// TODO(ianswett): Deal with acks which are too large to fit into a packet.
 bool QuicFramer::AppendAckFramePayload(
     const QuicAckFrame& frame,
     QuicDataWriter* writer) {
+  if (!writer->WriteUInt48(frame.sent_info.least_unacked)) {
+    return false;
+  }
+
+  size_t largest_received_offset = writer->length();
   if (!writer->WriteUInt48(frame.received_info.largest_received)) {
     return false;
   }
 
-  DCHECK_GE(numeric_limits<uint8>::max(),
-            frame.received_info.missing_packets.size());
+  // We don't check for overflowing uint8 here, because we only can fit 192 acks
+  // per packet, so if we overflow we will be truncated.
   uint8 num_missing_packets = frame.received_info.missing_packets.size();
+  size_t num_missing_packets_offset = writer->length();
   if (!writer->WriteBytes(&num_missing_packets, 1)) {
     return false;
   }
 
   SequenceSet::const_iterator it = frame.received_info.missing_packets.begin();
+  int num_missing_packets_written = 0;
   for (; it != frame.received_info.missing_packets.end(); ++it) {
     if (!writer->WriteUInt48(*it)) {
-      return false;
+      // We are truncating.  Overwrite largest_received.
+      QuicPacketSequenceNumber largest_received =
+          CalculateLargestReceived(frame.received_info.missing_packets, --it);
+      writer->WriteUInt48ToOffset(largest_received, largest_received_offset);
+      writer->WriteUInt8ToOffset(num_missing_packets_written,
+                                 num_missing_packets_offset);
+      return true;
     }
-  }
-
-  if (!writer->WriteUInt48(frame.sent_info.least_unacked)) {
-    return false;
+    ++num_missing_packets_written;
+    DCHECK_GE(numeric_limits<uint8>::max(), num_missing_packets_written);
   }
 
   return true;
diff --git a/net/quic/quic_framer.h b/net/quic/quic_framer.h
index 2b58165..d2cc378 100644
--- a/net/quic/quic_framer.h
+++ b/net/quic/quic_framer.h
@@ -95,6 +95,13 @@ class NET_EXPORT_PRIVATE QuicFramer {
 
   virtual ~QuicFramer();
 
+  // Calculates the largest received packet to advertise in the case an Ack
+  // Frame was truncated.  last_written in this case is the iterator for the
+  // last missing packet which fit in the outgoing ack.
+  static QuicPacketSequenceNumber CalculateLargestReceived(
+      const SequenceSet& missing_packets,
+      SequenceSet::const_iterator last_written);
+
   // Set callbacks to be called from the framer.  A visitor must be set, or
   // else the framer will likely crash.  It is acceptable for the visitor
   // to do nothing.  If this is called multiple times, only the last visitor
@@ -130,19 +137,15 @@ class NET_EXPORT_PRIVATE QuicFramer {
   bool ProcessRevivedPacket(const QuicPacketHeader& header,
                             base::StringPiece payload);
 
-  // Creates a new QuicPacket populated with the fields in |header| and
-  // |frames|.  Assigns |*packet| to the address of the new object.
-  // Returns true upon success.
-  bool ConstructFrameDataPacket(const QuicPacketHeader& header,
-                                const QuicFrames& frames,
-                                QuicPacket** packet);
-
-  // Creates a new QuicPacket populated with the fields in |header| and
-  // |fec|.  Assigns |*packet| to the address of the new object.
-  // Returns true upon success.
-  bool ConstructFecPacket(const QuicPacketHeader& header,
-                          const QuicFecData& fec,
-                          QuicPacket** packet);
+  // Returns a new QuicPacket, owned by the caller, populated with the fields
+  // in |header| and |frames|, or NULL if the packet could not be created.
+  QuicPacket* ConstructFrameDataPacket(const QuicPacketHeader& header,
+                                       const QuicFrames& frames);
+
+  // Returns a new QuicPacket, owned by the caller, populated with the fields
+  // in |header| and |fec|, or NULL if the packet could not be created.
+  QuicPacket* ConstructFecPacket(const QuicPacketHeader& header,
+                                 const QuicFecData& fec);
 
   void WriteSequenceNumber(QuicPacketSequenceNumber sequence_number,
                            QuicPacket* packet);
diff --git a/net/quic/quic_framer_test.cc b/net/quic/quic_framer_test.cc
index edb6775..25c3f39 100644
--- a/net/quic/quic_framer_test.cc
+++ b/net/quic/quic_framer_test.cc
@@ -24,7 +24,6 @@ using std::string;
 using std::vector;
 
 namespace net {
-
 namespace test {
 
 class TestEncrypter : public QuicEncrypter {
@@ -215,7 +214,8 @@ class QuicFramerTest : public ::testing::Test {
     return reinterpret_cast<char*>(data);
   }
 
-  void CheckProcessingFails(unsigned char* packet, size_t len,
+  void CheckProcessingFails(unsigned char* packet,
+                            size_t len,
                             string expected_error,
                             QuicErrorCode error_code) {
     QuicEncryptedPacket encrypted(AsChars(packet), len, false);
@@ -225,6 +225,15 @@ class QuicFramerTest : public ::testing::Test {
     EXPECT_EQ(error_code, framer_.error()) << "len: " << len;
   }
 
+  void ValidateTruncatedAck(const QuicAckFrame* ack, int keys) {
+    for (int i = 1; i < keys; ++i) {
+      EXPECT_TRUE(ContainsKey(ack->received_info.missing_packets, i)) << i;
+    }
+    // With no gaps in the missing packets, we can't admit to having received
+    // anything.
+    EXPECT_EQ(0u, ack->received_info.largest_received);
+  }
+
   test::TestEncrypter* encrypter_;
   test::TestDecrypter* decrypter_;
   QuicFramer framer_;
@@ -547,6 +556,9 @@ TEST_F(QuicFramerTest, AckFrame) {
     0x01,
     // frame type (ack frame)
     0x02,
+    // least packet sequence number awaiting an ack
+    0xA0, 0x9A, 0x78, 0x56,
+    0x34, 0x12,
     // largest received packet sequence number
     0xBF, 0x9A, 0x78, 0x56,
     0x34, 0x12,
@@ -555,9 +567,6 @@ TEST_F(QuicFramerTest, AckFrame) {
     // missing packet
     0xBE, 0x9A, 0x78, 0x56,
     0x34, 0x12,
-    // least packet sequence number awaiting an ack
-    0xA0, 0x9A, 0x78, 0x56,
-    0x34, 0x12,
   };
 
   QuicEncryptedPacket encrypted(AsChars(packet), arraysize(packet), false);
@@ -578,22 +587,21 @@ TEST_F(QuicFramerTest, AckFrame) {
   EXPECT_EQ(GG_UINT64_C(0x0123456789AA0), frame.sent_info.least_unacked);
 
   // Now test framing boundaries
-  for (size_t i = 0; i < 16; ++i) {
+  for (size_t i = 0; i < 21; ++i) {
     string expected_error;
     if (i < 1) {
       expected_error = "Unable to read frame count.";
     } else if (i < 2) {
       expected_error = "Unable to read frame type.";
     } else if (i < 8) {
+      expected_error = "Unable to read least unacked.";
+    } else if (i < 14) {
       expected_error = "Unable to read largest received.";
-    } else if (i < 9) {
-      expected_error = "Unable to read num missing packets.";
     } else if (i < 15) {
+      expected_error = "Unable to read num missing packets.";
+    } else if (i < 21) {
       expected_error = "Unable to read sequence number in missing packets.";
-    } else if (i < 16) {
-      expected_error = "Unable to read least unacked.";
-    }
-    CheckProcessingFails(packet, i + kPacketHeaderSize, expected_error,
+    }    CheckProcessingFails(packet, i + kPacketHeaderSize, expected_error,
                          QUIC_INVALID_FRAME_DATA);
   }
 }
@@ -945,6 +953,9 @@ TEST_F(QuicFramerTest, ConnectionCloseFrame) {
     'n',
 
     // Ack frame.
+    // least packet sequence number awaiting an ack
+    0xA0, 0x9A, 0x78, 0x56,
+    0x34, 0x12,
     // largest received packet sequence number
     0xBF, 0x9A, 0x78, 0x56,
     0x34, 0x12,
@@ -953,9 +964,6 @@ TEST_F(QuicFramerTest, ConnectionCloseFrame) {
     // missing packet
     0xBE, 0x9A, 0x78, 0x56,
     0x34, 0x12,
-    // least packet sequence number awaiting an ack
-    0xA0, 0x9A, 0x78, 0x56,
-    0x34, 0x12,
     // congestion feedback type (inter arrival)
     0x02,
     // accumulated_number_of_lost_packets
@@ -1086,14 +1094,12 @@ TEST_F(QuicFramerTest, ConstructStreamFramePacket) {
     'r',  'l',  'd',  '!',
   };
 
-  QuicPacket* data;
-  ASSERT_TRUE(framer_.ConstructFrameDataPacket(header, frames, &data));
+  scoped_ptr<QuicPacket> data(framer_.ConstructFrameDataPacket(header, frames));
+  ASSERT_TRUE(data != NULL);
 
   test::CompareCharArraysWithHexError("constructed packet",
                                       data->data(), data->length(),
                                       AsChars(packet), arraysize(packet));
-
-  delete data;
 }
 
 TEST_F(QuicFramerTest, ConstructAckFramePacket) {
@@ -1127,6 +1133,9 @@ TEST_F(QuicFramerTest, ConstructAckFramePacket) {
     0x01,
     // frame type (ack frame)
     0x02,
+    // least packet sequence number awaiting an ack
+    0xA0, 0x9A, 0x78, 0x56,
+    0x34, 0x12,
     // largest received packet sequence number
     0xBF, 0x9A, 0x78, 0x56,
     0x34, 0x12,
@@ -1135,19 +1144,14 @@ TEST_F(QuicFramerTest, ConstructAckFramePacket) {
     // missing packet
     0xBE, 0x9A, 0x78, 0x56,
     0x34, 0x12,
-    // least packet sequence number awaiting an ack
-    0xA0, 0x9A, 0x78, 0x56,
-    0x34, 0x12,
   };
 
-  QuicPacket* data;
-  ASSERT_TRUE(framer_.ConstructFrameDataPacket(header, frames, &data));
+  scoped_ptr<QuicPacket> data(framer_.ConstructFrameDataPacket(header, frames));
+  ASSERT_TRUE(data != NULL);
 
   test::CompareCharArraysWithHexError("constructed packet",
                                       data->data(), data->length(),
                                       AsChars(packet), arraysize(packet));
-
-  delete data;
 }
 
 TEST_F(QuicFramerTest, ConstructCongestionFeedbackFramePacketTCP) {
@@ -1189,14 +1193,12 @@ TEST_F(QuicFramerTest, ConstructCongestionFeedbackFramePacketTCP) {
     0x03, 0x04,
   };
 
-  QuicPacket* data;
-  ASSERT_TRUE(framer_.ConstructFrameDataPacket(header, frames, &data));
+  scoped_ptr<QuicPacket> data(framer_.ConstructFrameDataPacket(header, frames));
+  ASSERT_TRUE(data != NULL);
 
   test::CompareCharArraysWithHexError("constructed packet",
                                       data->data(), data->length(),
                                       AsChars(packet), arraysize(packet));
-
-  delete data;
 }
 
 TEST_F(QuicFramerTest, ConstructCongestionFeedbackFramePacketInterArrival) {
@@ -1264,14 +1266,12 @@ TEST_F(QuicFramerTest, ConstructCongestionFeedbackFramePacketInterArrival) {
     0x02, 0x00, 0x00, 0x00,
   };
 
-  QuicPacket* data;
-  ASSERT_TRUE(framer_.ConstructFrameDataPacket(header, frames, &data));
+  scoped_ptr<QuicPacket> data(framer_.ConstructFrameDataPacket(header, frames));
+  ASSERT_TRUE(data != NULL);
 
   test::CompareCharArraysWithHexError("constructed packet",
                                       data->data(), data->length(),
                                       AsChars(packet), arraysize(packet));
-
-  delete data;
 }
 
 TEST_F(QuicFramerTest, ConstructCongestionFeedbackFramePacketFixRate) {
@@ -1311,14 +1311,12 @@ TEST_F(QuicFramerTest, ConstructCongestionFeedbackFramePacketFixRate) {
     0x01, 0x02, 0x03, 0x04,
   };
 
-  QuicPacket* data;
-  ASSERT_TRUE(framer_.ConstructFrameDataPacket(header, frames, &data));
+  scoped_ptr<QuicPacket> data(framer_.ConstructFrameDataPacket(header, frames));
+  ASSERT_TRUE(data != NULL);
 
   test::CompareCharArraysWithHexError("constructed packet",
                                       data->data(), data->length(),
                                       AsChars(packet), arraysize(packet));
-
-  delete data;
 }
 
 TEST_F(QuicFramerTest, ConstructCongestionFeedbackFramePacketInvalidFeedback) {
@@ -1335,8 +1333,8 @@ TEST_F(QuicFramerTest, ConstructCongestionFeedbackFramePacketInvalidFeedback) {
   QuicFrames frames;
   frames.push_back(QuicFrame(&congestion_feedback_frame));
 
-  QuicPacket* data;
-  EXPECT_FALSE(framer_.ConstructFrameDataPacket(header, frames, &data));
+  scoped_ptr<QuicPacket> data(framer_.ConstructFrameDataPacket(header, frames));
+  ASSERT_TRUE(data == NULL);
 }
 
 TEST_F(QuicFramerTest, ConstructRstFramePacket) {
@@ -1387,14 +1385,12 @@ TEST_F(QuicFramerTest, ConstructRstFramePacket) {
   QuicFrames frames;
   frames.push_back(QuicFrame(&rst_frame));
 
-  QuicPacket* data;
-  ASSERT_TRUE(framer_.ConstructFrameDataPacket(header, frames, &data));
+  scoped_ptr<QuicPacket> data(framer_.ConstructFrameDataPacket(header, frames));
+  ASSERT_TRUE(data != NULL);
 
   test::CompareCharArraysWithHexError("constructed packet",
                                       data->data(), data->length(),
                                       AsChars(packet), arraysize(packet));
-
-  delete data;
 }
 
 TEST_F(QuicFramerTest, ConstructCloseFramePacket) {
@@ -1443,6 +1439,9 @@ TEST_F(QuicFramerTest, ConstructCloseFramePacket) {
     'n',
 
     // Ack frame.
+    // least packet sequence number awaiting an ack
+    0xA0, 0x9A, 0x78, 0x56,
+    0x34, 0x12,
     // largest received packet sequence number
     0xBF, 0x9A, 0x78, 0x56,
     0x34, 0x12,
@@ -1451,20 +1450,14 @@ TEST_F(QuicFramerTest, ConstructCloseFramePacket) {
     // missing packet
     0xBE, 0x9A, 0x78, 0x56,
     0x34, 0x12,
-
-    // least packet sequence number awaiting an ack
-    0xA0, 0x9A, 0x78, 0x56,
-    0x34, 0x12,
   };
 
-  QuicPacket* data;
-  ASSERT_TRUE(framer_.ConstructFrameDataPacket(header, frames, &data));
+  scoped_ptr<QuicPacket> data(framer_.ConstructFrameDataPacket(header, frames));
+  ASSERT_TRUE(data != NULL);
 
   test::CompareCharArraysWithHexError("constructed packet",
                                       data->data(), data->length(),
                                       AsChars(packet), arraysize(packet));
-
-  delete data;
 }
 
 TEST_F(QuicFramerTest, ConstructFecPacket) {
@@ -1501,14 +1494,12 @@ TEST_F(QuicFramerTest, ConstructFecPacket) {
     'm',  'n',  'o',  'p',
   };
 
-  QuicPacket* data;
-  ASSERT_TRUE(framer_.ConstructFecPacket(header, fec_data, &data));
+  scoped_ptr<QuicPacket> data(framer_.ConstructFecPacket(header, fec_data));
+  ASSERT_TRUE(data != NULL);
 
   test::CompareCharArraysWithHexError("constructed packet",
                                       data->data(), data->length(),
                                       AsChars(packet), arraysize(packet));
-
-  delete data;
 }
 
 TEST_F(QuicFramerTest, EncryptPacket) {
@@ -1540,6 +1531,106 @@ TEST_F(QuicFramerTest, EncryptPacket) {
   EXPECT_TRUE(CheckEncryption(StringPiece(AsChars(packet), arraysize(packet))));
 }
 
-}  // namespace test
+TEST_F(QuicFramerTest, CalculateLargestReceived) {
+  SequenceSet missing;
+  missing.insert(1);
+  missing.insert(5);
+  missing.insert(7);
+  missing.insert(8);
+  missing.insert(9);
+
+  // These two we just walk to the next gap, and return the largest seen.
+  EXPECT_EQ(4u, QuicFramer::CalculateLargestReceived(missing, missing.find(1)));
+  EXPECT_EQ(6u, QuicFramer::CalculateLargestReceived(missing, missing.find(5)));
+
+  // 7+ are fun because there is no subsequent gap: we have to walk before
+  // them to find 6.
+  EXPECT_EQ(6u, QuicFramer::CalculateLargestReceived(missing, missing.find(7)));
+  EXPECT_EQ(6u, QuicFramer::CalculateLargestReceived(missing, missing.find(8)));
+  EXPECT_EQ(6u, QuicFramer::CalculateLargestReceived(missing, missing.find(9)));
+
+  // Fill in the gap of 6 to make sure we handle a gap of more than 1 by
+  // returning 4 rather than 3.
+  missing.insert(6);
+  EXPECT_EQ(4u, QuicFramer::CalculateLargestReceived(missing, missing.find(9)));
+
+  // Fill in the rest of the gaps: we should walk to 1 and return 0.
+  missing.insert(4);
+  missing.insert(3);
+  missing.insert(2);
+  EXPECT_EQ(0u, QuicFramer::CalculateLargestReceived(missing, missing.find(7)));
+
+  // If we add a gap after 7-9, we will return that.
+  missing.insert(11);
+  EXPECT_EQ(10u, QuicFramer::CalculateLargestReceived(missing,
+                                                      missing.find(7)));
+  EXPECT_EQ(10u, QuicFramer::CalculateLargestReceived(missing,
+                                                      missing.find(8)));
+  EXPECT_EQ(10u, QuicFramer::CalculateLargestReceived(missing,
+                                                      missing.find(9)));
+}
+
+TEST_F(QuicFramerTest, Truncation) {
+  QuicPacketHeader header;
+  header.guid = GG_UINT64_C(0xFEDCBA9876543210);
+  header.packet_sequence_number = GG_UINT64_C(0x123456789ABC);
+  header.flags = PACKET_FLAGS_NONE;
+  header.fec_group = 0;
+
+  QuicConnectionCloseFrame close_frame;
+  QuicAckFrame* ack_frame = &close_frame.ack_frame;
+  close_frame.error_code = static_cast<QuicErrorCode>(0x05060708);
+  close_frame.error_details = "because I can";
+  ack_frame->received_info.largest_received = 201;
+  for (uint64 i = 1; i < ack_frame->received_info.largest_received; ++i) {
+    ack_frame->received_info.missing_packets.insert(i);
+  }
+
+  // Create a packet with just the ack
+  QuicFrame frame;
+  frame.type = ACK_FRAME;
+  frame.ack_frame = ack_frame;
+  QuicFrames frames;
+  frames.push_back(frame);
+
+  scoped_ptr<QuicPacket> raw_ack_packet(
+      framer_.ConstructFrameDataPacket(header, frames));
+  ASSERT_TRUE(raw_ack_packet != NULL);
+
+  scoped_ptr<QuicEncryptedPacket> ack_packet(
+      framer_.EncryptPacket(*raw_ack_packet));
+
+  // Create a packet with just connection close.
+  frames.clear();
+  frame.type = CONNECTION_CLOSE_FRAME;
+  frame.connection_close_frame = &close_frame;
+  frames.push_back(frame);
+
+  scoped_ptr<QuicPacket> raw_close_packet(
+      framer_.ConstructFrameDataPacket(header, frames));
+  ASSERT_TRUE(raw_close_packet != NULL);
+
+  scoped_ptr<QuicEncryptedPacket> close_packet(
+      framer_.EncryptPacket(*raw_close_packet));
+
+  // Now make sure we can turn our ack packet back into an ack frame
+  ASSERT_TRUE(framer_.ProcessPacket(self_address_, peer_address_, *ack_packet));
+  EXPECT_EQ(QUIC_NO_ERROR, framer_.error());
+  ASSERT_TRUE(visitor_.header_.get());
+  ASSERT_EQ(peer_address_, visitor_.peer_address_);
+  ASSERT_EQ(self_address_, visitor_.self_address_);
+  EXPECT_EQ(1u, visitor_.ack_frames_.size());
+
+  // And do the same for the close frame.
+  ASSERT_TRUE(framer_.ProcessPacket(self_address_, peer_address_,
+                                    *close_packet));
+  EXPECT_EQ(QUIC_NO_ERROR, framer_.error());
+  EXPECT_EQ(0x05060708, visitor_.connection_close_frame_.error_code);
+  EXPECT_EQ("because I can", visitor_.connection_close_frame_.error_details);
 
+  ValidateTruncatedAck(visitor_.ack_frames_[0], 190);
+  ValidateTruncatedAck(&visitor_.connection_close_frame_.ack_frame, 180);
+}
+
+}  // namespace test
 }  // namespace net
diff --git a/net/quic/quic_http_stream.cc b/net/quic/quic_http_stream.cc
index 3ae4c00..4ea35bf 100644
--- a/net/quic/quic_http_stream.cc
+++ b/net/quic/quic_http_stream.cc
@@ -439,6 +439,8 @@ int QuicHttpStream::ParseResponseHeaders() {
 }
 
 void QuicHttpStream::BufferResponseBody(const char* data, int length) {
+  if (length == 0)
+    return;
   IOBufferWithSize* io_buffer = new IOBufferWithSize(length);
   memcpy(io_buffer->data(), data, length);
   response_body_.push_back(make_scoped_refptr(io_buffer));
diff --git a/net/quic/quic_http_stream_test.cc b/net/quic/quic_http_stream_test.cc
index 21d631a..517342e 100644
--- a/net/quic/quic_http_stream_test.cc
+++ b/net/quic/quic_http_stream_test.cc
@@ -225,11 +225,9 @@ class QuicHttpStreamTest : public ::testing::Test {
                                        const QuicFrame& frame) {
     QuicFrames frames;
     frames.push_back(frame);
-    QuicPacket* packet;
-    framer_.ConstructFrameDataPacket(header_, frames, &packet);
-    QuicEncryptedPacket* encrypted = framer_.EncryptPacket(*packet);
-    delete packet;
-    return encrypted;
+    scoped_ptr<QuicPacket> packet(
+        framer_.ConstructFrameDataPacket(header_, frames));
+    return framer_.EncryptPacket(*packet);
   }
 
   const QuicGuid guid_;
diff --git a/net/quic/quic_packet_creator.cc b/net/quic/quic_packet_creator.cc
index cbca68a..95642bd 100644
--- a/net/quic/quic_packet_creator.cc
+++ b/net/quic/quic_packet_creator.cc
@@ -41,7 +41,7 @@ size_t QuicPacketCreator::DataToStream(QuicStreamId id,
                                        vector<PacketPair>* packets) {
   DCHECK_GT(options_.max_packet_length,
             QuicUtils::StreamFramePacketOverhead(1));
-
+  DCHECK_LT(0u, options_.max_num_packets);
   QuicPacketHeader header;
 
   QuicPacket* packet = NULL;
@@ -49,7 +49,7 @@ size_t QuicPacketCreator::DataToStream(QuicStreamId id,
   QuicFecGroupNumber current_fec_group = 0;
   QuicFecData fec_data;
 
-  int num_data_packets = options_.max_num_packets;
+  size_t num_data_packets = options_.max_num_packets;
 
   if (options_.use_fec) {
     DCHECK_LT(1u, options_.max_num_packets);
@@ -86,7 +86,8 @@ size_t QuicPacketCreator::DataToStream(QuicStreamId id,
       unconsumed_bytes -= frame_len;
 
       // Produce the data packet (which might fin the stream).
-      framer_->ConstructFrameDataPacket(header, frames, &packet);
+      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));
       frames.clear();
@@ -102,7 +103,8 @@ size_t QuicPacketCreator::DataToStream(QuicStreamId id,
     FillPacketHeader(current_fec_group, PACKET_FLAGS_NONE, &header);
     QuicStreamFrame frame(id, true, offset, "");
     frames.push_back(QuicFrame(&frame));
-    framer_->ConstructFrameDataPacket(header, frames, &packet);
+    packet = framer_->ConstructFrameDataPacket(header, frames);
+    DCHECK(packet);
     packets->push_back(make_pair(header.packet_sequence_number, packet));
     frames.clear();
   }
@@ -111,8 +113,8 @@ size_t QuicPacketCreator::DataToStream(QuicStreamId id,
   if (current_fec_group != 0) {
     FillPacketHeader(current_fec_group, PACKET_FLAGS_FEC, &header);
     fec_data.redundancy = fec_group_->parity();
-    QuicPacket* fec_packet;
-    framer_->ConstructFecPacket(header, fec_data, &fec_packet);
+    QuicPacket* fec_packet = framer_->ConstructFecPacket(header, fec_data);
+    DCHECK(fec_packet);
     packets->push_back(make_pair(header.packet_sequence_number, fec_packet));
     ++fec_group_number_;
   }
@@ -147,10 +149,10 @@ QuicPacketCreator::PacketPair QuicPacketCreator::ResetStream(
 
   QuicRstStreamFrame close_frame(id, offset, error);
 
-  QuicPacket* packet;
   QuicFrames frames;
   frames.push_back(QuicFrame(&close_frame));
-  framer_->ConstructFrameDataPacket(header, frames, &packet);
+  QuicPacket* packet = framer_->ConstructFrameDataPacket(header, frames);
+  DCHECK(packet);
   return make_pair(header.packet_sequence_number, packet);
 }
 
@@ -160,10 +162,10 @@ QuicPacketCreator::PacketPair QuicPacketCreator::CloseConnection(
   QuicPacketHeader header;
   FillPacketHeader(0, PACKET_FLAGS_NONE, &header);
 
-  QuicPacket* packet;
   QuicFrames frames;
   frames.push_back(QuicFrame(close_frame));
-  framer_->ConstructFrameDataPacket(header, frames, &packet);
+  QuicPacket* packet = framer_->ConstructFrameDataPacket(header, frames);
+  DCHECK(packet);
   return make_pair(header.packet_sequence_number, packet);
 }
 
@@ -173,10 +175,10 @@ QuicPacketCreator::PacketPair QuicPacketCreator::AckPacket(
   QuicPacketHeader header;
   FillPacketHeader(0, PACKET_FLAGS_NONE, &header);
 
-  QuicPacket* packet;
   QuicFrames frames;
   frames.push_back(QuicFrame(ack_frame));
-  framer_->ConstructFrameDataPacket(header, frames, &packet);
+  QuicPacket* packet = framer_->ConstructFrameDataPacket(header, frames);
+  DCHECK(packet);
   return make_pair(header.packet_sequence_number, packet);
 }
 
@@ -186,10 +188,10 @@ QuicPacketCreator::PacketPair QuicPacketCreator::CongestionFeedbackPacket(
   QuicPacketHeader header;
   FillPacketHeader(0, PACKET_FLAGS_NONE, &header);
 
-  QuicPacket* packet;
   QuicFrames frames;
   frames.push_back(QuicFrame(feedback_frame));
-  framer_->ConstructFrameDataPacket(header, frames, &packet);
+  QuicPacket* packet = framer_->ConstructFrameDataPacket(header, frames);
+  DCHECK(packet);
   return make_pair(header.packet_sequence_number, packet);
 }
 
diff --git a/net/quic/quic_packet_creator.h b/net/quic/quic_packet_creator.h
index 8e57ef8..ff43021 100644
--- a/net/quic/quic_packet_creator.h
+++ b/net/quic/quic_packet_creator.h
@@ -28,7 +28,7 @@ class NET_EXPORT_PRIVATE QuicPacketCreator : public QuicFecBuilderInterface {
     void Clear() {
       memset(this, 0, sizeof(Options));
       max_packet_length = kMaxPacketSize;
-      max_num_packets = std::numeric_limits<int>::max();
+      max_num_packets = std::numeric_limits<size_t>::max();
     }
 
     // TODO(alyssar, rch) max frames/packet
@@ -70,7 +70,8 @@ class NET_EXPORT_PRIVATE QuicPacketCreator : public QuicFecBuilderInterface {
 
   PacketPair AckPacket(QuicAckFrame* ack_frame);
 
-  PacketPair CongestionFeedbackPacket(QuicCongestionFeedbackFrame* ack_frame);
+  PacketPair CongestionFeedbackPacket(
+      QuicCongestionFeedbackFrame* feedback_frame);
 
   // Increments the current sequence number in QuicPacketCreator and sets it
   // into the packet and returns the new sequence number.
diff --git a/net/quic/quic_protocol.h b/net/quic/quic_protocol.h
index b947e19..471b797 100644
--- a/net/quic/quic_protocol.h
+++ b/net/quic/quic_protocol.h
@@ -241,6 +241,10 @@ struct NET_EXPORT_PRIVATE CongestionFeedbackMessageInterArrival {
   CongestionFeedbackMessageInterArrival();
   ~CongestionFeedbackMessageInterArrival();
   uint16 accumulated_number_of_lost_packets;
+  // TODO(rch): These times should be QuicTime instances.  We can write
+  // them to the wire in the format specified below, but we should avoid
+  // storing them in memory that way.  As such, we should move this comment
+  // to QuicFramer.
   int16 offset_time;
   uint16 delta_time;  // delta time is described below.
   // The set of received packets since the last feedback was sent, along with
@@ -307,13 +311,16 @@ struct NET_EXPORT_PRIVATE QuicConnectionCloseFrame {
 struct NET_EXPORT_PRIVATE QuicFrame {
   QuicFrame() {}
   explicit QuicFrame(QuicStreamFrame* stream_frame)
-      : type(STREAM_FRAME), stream_frame(stream_frame) {
+      : type(STREAM_FRAME),
+        stream_frame(stream_frame) {
   }
   explicit QuicFrame(QuicAckFrame* frame)
-      : type(ACK_FRAME), ack_frame(frame) {
+      : type(ACK_FRAME),
+        ack_frame(frame) {
   }
   explicit QuicFrame(QuicCongestionFeedbackFrame* frame)
-      : type(CONGESTION_FEEDBACK_FRAME), congestion_feedback_frame(frame) {
+      : type(CONGESTION_FEEDBACK_FRAME),
+        congestion_feedback_frame(frame) {
   }
   explicit QuicFrame(QuicRstStreamFrame* frame)
       : type(RST_STREAM_FRAME),
diff --git a/net/quic/quic_session.cc b/net/quic/quic_session.cc
index 0c04cb7..8e2774a 100644
--- a/net/quic/quic_session.cc
+++ b/net/quic/quic_session.cc
@@ -150,6 +150,10 @@ ReliableQuicStream* QuicSession::GetStream(const QuicStreamId stream_id) {
     return it->second;
   }
 
+  if (IsClosedStream(stream_id)) {
+    return NULL;
+  }
+
   if (stream_id % 2 == next_stream_id_ % 2) {
     // We've received a frame for a locally-created stream that is not
     // currently active.  This is an error.
diff --git a/net/quic/quic_session.h b/net/quic/quic_session.h
index f1375cdb9..6473467 100644
--- a/net/quic/quic_session.h
+++ b/net/quic/quic_session.h
@@ -124,7 +124,7 @@ class NET_EXPORT_PRIVATE QuicSession : public QuicConnectionVisitorInterface {
   // of a stream id larger than the next expected stream id.
   base::hash_set<QuicStreamId> implicitly_created_streams_;
 
-  // A list of packets which need to write more data.
+  // A list of streams which need to write more data.
   std::list<QuicStreamId> write_blocked_streams_;
 
   QuicStreamId largest_peer_created_stream_id_;
diff --git a/net/quic/quic_stream_factory_test.cc b/net/quic/quic_stream_factory_test.cc
index 53f093d..33f5bbd 100644
--- a/net/quic/quic_stream_factory_test.cc
+++ b/net/quic/quic_stream_factory_test.cc
@@ -18,7 +18,6 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
-
 namespace test {
 
 class QuicStreamFactoryTest : public ::testing::Test {
@@ -99,11 +98,9 @@ class QuicStreamFactoryTest : public ::testing::Test {
                       QuicEncrypter::Create(kNULL));
     QuicFrames frames;
     frames.push_back(frame);
-    QuicPacket* packet;
-    framer.ConstructFrameDataPacket(header, frames, &packet);
-    QuicEncryptedPacket* encrypted = framer.EncryptPacket(*packet);
-    delete packet;
-    return scoped_ptr<QuicEncryptedPacket>(encrypted);
+    scoped_ptr<QuicPacket> packet(
+        framer.ConstructFrameDataPacket(header, frames));
+    return scoped_ptr<QuicEncryptedPacket>(framer.EncryptPacket(*packet));
   }
 
   MockHostResolver host_resolver_;
@@ -228,5 +225,4 @@ TEST_F(QuicStreamFactoryTest, CancelCreate) {
 }
 
 }  // namespace test
-
 }  // namespace net
diff --git a/net/quic/reliable_quic_stream.cc b/net/quic/reliable_quic_stream.cc
index 855fbf2..6b284e6 100644
--- a/net/quic/reliable_quic_stream.cc
+++ b/net/quic/reliable_quic_stream.cc
@@ -118,8 +118,7 @@ int ReliableQuicStream::WriteOrBuffer(StringPiece data, bool fin) {
 
 void ReliableQuicStream::OnCanWrite() {
   bool fin = false;
-  bool all_bytes_written = true;
-  while (all_bytes_written && !queued_data_.empty()) {
+  while (!queued_data_.empty()) {
     const string& data = queued_data_.front();
     if (queued_data_.size() == 1 && fin_buffered_) {
       fin = true;
@@ -128,9 +127,9 @@ void ReliableQuicStream::OnCanWrite() {
     if (bytes_written == static_cast<int>(data.size())) {
       queued_data_.pop_front();
     } else {
-      all_bytes_written = false;
       queued_data_.front() = string(data.data() + bytes_written,
                                     data.length() - bytes_written);
+      break;
     }
   }
 }
diff --git a/net/quic/test_tools/quic_test_utils.cc b/net/quic/test_tools/quic_test_utils.cc
index 133f6a6..0971e06 100644
--- a/net/quic/test_tools/quic_test_utils.cc
+++ b/net/quic/test_tools/quic_test_utils.cc
@@ -11,7 +11,6 @@ using std::min;
 using std::string;
 
 namespace net {
-
 namespace test {
 
 MockFramerVisitor::MockFramerVisitor() {
@@ -205,11 +204,8 @@ QuicPacket* ConstructHandshakePacket(QuicGuid guid, CryptoTag tag) {
   QuicFrame frame(&stream_frame);
   QuicFrames frames;
   frames.push_back(frame);
-  QuicPacket* packet;
-  quic_framer.ConstructFrameDataPacket(header, frames, &packet);
-  return packet;
+  return quic_framer.ConstructFrameDataPacket(header, frames);
 }
 
 }  // namespace test
-
 }  // namespace net
diff --git a/net/quic/test_tools/test_task_runner.cc b/net/quic/test_tools/test_task_runner.cc
index a2521fa..695e2c4 100644
--- a/net/quic/test_tools/test_task_runner.cc
+++ b/net/quic/test_tools/test_task_runner.cc
@@ -15,7 +15,6 @@
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace net {
-
 namespace test {
 
 PostedTask::PostedTask(const tracked_objects::Location& location,
@@ -84,5 +83,4 @@ void TestTaskRunner::RunNextTask() {
 }
 
 }  // namespace test
-
 }  // namespace net