Change IsRetransmittable to not check transmission type but instead let

net/quic/quic_connection.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/quic/quic_connection.h"
 
 #include <string.h>
 #include <sys/types.h>
 #include <algorithm>
 #include <iterator>
@@ skipping 1286 matching lines @@
   if (!delay.IsZero()) {
     send_alarm_->Update(now.Add(delay), QuicTime::Delta::FromMilliseconds(1));
     DVLOG(1) << "Delaying sending.";
     return false;
   }
   send_alarm_->Cancel();
   return true;
 }
 
 bool QuicConnection::WritePacket(const QueuedPacket& packet) {
-  QuicPacketSequenceNumber sequence_number =
-      packet.serialized_packet.sequence_number;
-  if (ShouldDiscardPacket(packet.encryption_level,
-                          sequence_number,
-                          IsRetransmittable(packet))) {
+  if (ShouldDiscardPacket(packet)) {
     ++stats_.packets_discarded;
     return true;
   }
   // Connection close packets are encypted and saved, so don't exit early.
   if (writer_->IsWriteBlocked() && !IsConnectionClose(packet)) {
     return false;
   }
 
+  QuicPacketSequenceNumber sequence_number =
+      packet.serialized_packet.sequence_number;
   // Some encryption algorithms require the packet sequence numbers not be
   // repeated.
   DCHECK_LE(sequence_number_of_last_sent_packet_, sequence_number);
   sequence_number_of_last_sent_packet_ = sequence_number;
 
   QuicEncryptedPacket* encrypted = framer_.EncryptPacket(
       packet.encryption_level,
       sequence_number,
       *packet.serialized_packet.packet);
   if (encrypted == NULL) {
@@ skipping 99 matching lines @@
   }
 
   if (result.status == WRITE_STATUS_ERROR) {
     OnWriteError(result.error_code);
     return false;
   }
 
   return true;
 }
 
-bool QuicConnection::ShouldDiscardPacket(
-    EncryptionLevel level,
-    QuicPacketSequenceNumber sequence_number,
-    HasRetransmittableData retransmittable) {
+bool QuicConnection::ShouldDiscardPacket(const QueuedPacket& packet) {
   if (!connected_) {
     DVLOG(1) << ENDPOINT
              << "Not sending packet as connection is disconnected.";
     return true;
   }
 
+  QuicPacketSequenceNumber sequence_number =
+      packet.serialized_packet.sequence_number;
   // If the packet has been discarded before sending, don't send it.
   // This occurs if a packet gets serialized, queued, then discarded.
   if (!sent_packet_manager_.IsUnacked(sequence_number)) {
     DVLOG(1) << ENDPOINT << "Dropping packet before sending: "
              << sequence_number << " since it has already been discarded.";
     return true;
   }
 
   if (encryption_level_ == ENCRYPTION_FORWARD_SECURE &&
-      level == ENCRYPTION_NONE) {
+      packet.encryption_level == ENCRYPTION_NONE) {
     // Drop packets that are NULL encrypted since the peer won't accept them
     // anymore.
     DVLOG(1) << ENDPOINT << "Dropping NULL encrypted packet: "
              << sequence_number << " since the connection is forward secure.";
     LOG_IF(DFATAL,
            sent_packet_manager_.HasRetransmittableFrames(sequence_number))
         << "Once forward secure, all NULL encrypted packets should be "
         << "neutered.";
     return true;
   }
 
-  if (retransmittable == HAS_RETRANSMITTABLE_DATA &&
+  if (packet.transmission_type != NOT_RETRANSMISSION &&
       !sent_packet_manager_.HasRetransmittableFrames(sequence_number)) {
     DVLOG(1) << ENDPOINT << "Dropping unacked packet: " << sequence_number
              << " A previous transmission was acked while write blocked.";
     return true;
   }
 
   return false;
 }
 
 void QuicConnection::OnWriteError(int error_code) {
@@ skipping 490 matching lines @@
   // If we changed the generator's batch state, restore original batch state.
   if (!already_in_batch_mode_) {
     DVLOG(1) << "Leaving Batch Mode.";
     connection_->packet_generator_.FinishBatchOperations();
   }
   DCHECK_EQ(already_in_batch_mode_,
             connection_->packet_generator_.InBatchMode());
 }
 
 HasRetransmittableData QuicConnection::IsRetransmittable(
-    QueuedPacket packet) {
-  // TODO(cyr): Understand why the first check here is necessary. Without it,
-  // DiscardRetransmit test fails.
+    const QueuedPacket& packet) {
+  // Retransmitted packets retransmittable frames are owned by the unacked
+  // packet map, but are not present in the serialized packet.
   if (packet.transmission_type != NOT_RETRANSMISSION ||
       packet.serialized_packet.retransmittable_frames != NULL) {
     return HAS_RETRANSMITTABLE_DATA;
   } else {
     return NO_RETRANSMITTABLE_DATA;
   }
 }
 
 bool QuicConnection::IsConnectionClose(
     QueuedPacket packet) {
   RetransmittableFrames* retransmittable_frames =
       packet.serialized_packet.retransmittable_frames;
   if (!retransmittable_frames) {
     return false;
   }
   for (size_t i = 0; i < retransmittable_frames->frames().size(); ++i) {
     if (retransmittable_frames->frames()[i].type == CONNECTION_CLOSE_FRAME) {
       return true;
     }
   }
   return false;
 }
 
 }  // namespace net