Land Recent QUIC Changes

Fix to QUIC's RttStats ExpireSmoothedMetrics where the mean deviation
may increase if the rtt decreased.

Merge internal change: 71665066
https://codereview.chromium.org/406403002/


Allow client to request Reno congestion control.

Disabled BBR tests (will enable after BBR code is checked in).

Merge internal change: 71621257
https://codereview.chromium.org/412753002/


QUIC - Minor cleanup change to keep internal source tree and chromium
codebase in sync.

+ Use DECHEK_EQ instead of DCHECK.

Merge internal change: 71613624
https://codereview.chromium.org/412733002/


Allow QuicConnection to own its QuicPacketWriter

Added a new "bool owns_writer" parameter to the QuicConnection
constructor that specifies whether the QuicConnection has ownership.
Also cleaned up the ownership semantics in a number of tests --
previously, the writer was not actually outliving the connection.

Merge internal change: 71590341
https://codereview.chromium.org/413573003/


Added QuicTime::Infinite() to create infinite time required by the
following internal change.

Change QUIC"s BBR sender to only fall out of slow start when there is a
packet loss if the residual is less than 0.25.

Merge internal change: 71436337
https://codereview.chromium.org/410743003/


Ensure the UnackedPacketMap only keeps one previous transmission less
than largest_observed.

Also improves the tightness of QuicUnackedPacketMapTest.

Merge internal change: 71426216
https://codereview.chromium.org/411723003/


Parameterize the QUIC end-to-end tests to run with FEC both enabled and
disabled.

Found bug that was fixed in internal change: 71390633
(https://codereview.chromium.org/414523003/)

Merge internal change: 71393479
https://codereview.chromium.org/412543005/


Fix a bug in the QuicReceivedPacketManager where a packet which had
previously been revied was not removed from the revived packet set when
it was eventually received out of order.

Merge internal change: 71390633
https://codereview.chromium.org/414523003/


QuicConnection::ProcessValidatedPacket doesn't need to be public because
it is only called by QuicConnection::OnPacketHeader.

Merge internal change: 71371063
https://codereview.chromium.org/402333003/


Add bandwidth_estimate and server_namespace fields to the source-address
token proto. Not used anywhere, this CL is really just for discussion
about which fields (and fieldnames) we should use.

Merge internal change: 71360060
https://codereview.chromium.org/404343003/

R=rch@chromium.org

Review URL: https://codereview.chromium.org/411823002

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

29 files changed, 342 insertions, 60 deletions

diff --git a/net/quic/congestion_control/rtt_stats.cc b/net/quic/congestion_control/rtt_stats.cc
index e70d2d9..b03686d 100644
--- a/net/quic/congestion_control/rtt_stats.cc
+++ b/net/quic/congestion_control/rtt_stats.cc
@@ -4,6 +4,8 @@
 
 #include "net/quic/congestion_control/rtt_stats.h"
 
+#include <complex>  // std::abs
+
 using std::max;
 
 namespace net {
@@ -40,7 +42,10 @@ void RttStats::SampleNewRecentMinRtt(uint32 num_samples) {
 }
 
 void RttStats::ExpireSmoothedMetrics() {
-  mean_deviation_ = max(mean_deviation_, latest_rtt_.Subtract(smoothed_rtt_));
+  mean_deviation_ =
+      max(mean_deviation_,
+          QuicTime::Delta::FromMicroseconds(
+              std::abs(smoothed_rtt_.Subtract(latest_rtt_).ToMicroseconds())));
   smoothed_rtt_ = max(smoothed_rtt_, latest_rtt_);
 }
 
diff --git a/net/quic/congestion_control/rtt_stats.h b/net/quic/congestion_control/rtt_stats.h
index 8ceaf21..5e01687 100644
--- a/net/quic/congestion_control/rtt_stats.h
+++ b/net/quic/congestion_control/rtt_stats.h
@@ -32,8 +32,9 @@ class NET_EXPORT_PRIVATE RttStats {
                  QuicTime::Delta ack_delay,
                  QuicTime now);
 
-  // Causes the smoothed_rtt to be increased to the latest_rtt and the
-  // mean_variance to be increased to the most recent variance.
+  // Causes the smoothed_rtt to be increased to the latest_rtt if the latest_rtt
+  // is larger. The mean deviation is increased to the most recent deviation if
+  // it's larger.
   void ExpireSmoothedMetrics();
 
   // Forces RttStats to sample a new recent min rtt within the next
diff --git a/net/quic/congestion_control/rtt_stats_test.cc b/net/quic/congestion_control/rtt_stats_test.cc
index a24016c..6b5ff7d 100644
--- a/net/quic/congestion_control/rtt_stats_test.cc
+++ b/net/quic/congestion_control/rtt_stats_test.cc
@@ -172,6 +172,13 @@ TEST_F(RttStatsTest, ExpireSmoothedMetrics) {
   rtt_stats_.ExpireSmoothedMetrics();
   EXPECT_EQ(doubled_rtt, rtt_stats_.SmoothedRtt());
   EXPECT_EQ(initial_rtt.Multiply(0.875), rtt_stats_.mean_deviation());
+
+  // Now go back down to 5ms and expire the smoothed metrics, and ensure the
+  // mean deviation increases to 15ms.
+  QuicTime::Delta half_rtt = initial_rtt.Multiply(0.5);
+  rtt_stats_.UpdateRtt(half_rtt, QuicTime::Delta::Zero(), QuicTime::Zero());
+  EXPECT_GT(doubled_rtt, rtt_stats_.SmoothedRtt());
+  EXPECT_LT(initial_rtt, rtt_stats_.mean_deviation());
 }
 
 }  // namespace test
diff --git a/net/quic/congestion_control/send_algorithm_interface.cc b/net/quic/congestion_control/send_algorithm_interface.cc
index 2401667..5d0e032 100644
--- a/net/quic/congestion_control/send_algorithm_interface.cc
+++ b/net/quic/congestion_control/send_algorithm_interface.cc
@@ -10,8 +10,6 @@
 
 namespace net {
 
-const bool kUseReno = false;
-
 class RttStats;
 
 // Factory for send side congestion control algorithm.
@@ -21,12 +19,14 @@ SendAlgorithmInterface* SendAlgorithmInterface::Create(
     CongestionControlType congestion_control_type,
     QuicConnectionStats* stats) {
   switch (congestion_control_type) {
-    case kTCP:
-      return new TcpCubicSender(clock, rtt_stats, kUseReno,
+    case kCubic:
+      return new TcpCubicSender(clock, rtt_stats,
+                                false /* don't use Reno */,
+                                kMaxTcpCongestionWindow, stats);
+    case kReno:
+      return new TcpCubicSender(clock, rtt_stats,
+                                true /* use Reno */,
                                 kMaxTcpCongestionWindow, stats);
-    case kInterArrival:
-      LOG(DFATAL) << "InterArrivalSendAlgorithm no longer supported.";
-      return NULL;
     case kFixRateCongestionControl:
       return new FixRateSender(rtt_stats);
     case kBBR:
diff --git a/net/quic/congestion_control/send_algorithm_simulator.cc b/net/quic/congestion_control/send_algorithm_simulator.cc
index 5c55f5b..36aa321 100644
--- a/net/quic/congestion_control/send_algorithm_simulator.cc
+++ b/net/quic/congestion_control/send_algorithm_simulator.cc
@@ -73,7 +73,9 @@ void SendAlgorithmSimulator::TransferBytes() {
 
 void SendAlgorithmSimulator::TransferBytes(QuicByteCount max_bytes,
                                            QuicTime::Delta max_time) {
-  const QuicTime end_time = clock_->Now().Add(max_time);
+  const QuicTime end_time = max_time.IsInfinite() ?
+      QuicTime::Zero().Add(QuicTime::Delta::Infinite()) :
+      clock_->Now().Add(max_time);
   QuicByteCount bytes_sent = 0;
   while (!pending_transfers_.empty() &&
          clock_->Now() < end_time &&
diff --git a/net/quic/crypto/crypto_protocol.h b/net/quic/crypto/crypto_protocol.h
index 9e38d4b..5779137 100644
--- a/net/quic/crypto/crypto_protocol.h
+++ b/net/quic/crypto/crypto_protocol.h
@@ -51,6 +51,7 @@ const QuicTag kINAR = TAG('I', 'N', 'A', 'R');   // Inter arrival
 
 // Congestion control options
 const QuicTag kTBBR = TAG('T', 'B', 'B', 'R');   // Reduced Buffer Bloat TCP
+const QuicTag kRENO = TAG('R', 'E', 'N', 'O');   // Reno Congestion Control
 
 // Loss detection algorithm types
 const QuicTag kNACK = TAG('N', 'A', 'C', 'K');   // TCP style nack counting
diff --git a/net/quic/crypto/quic_crypto_server_config.cc b/net/quic/crypto/quic_crypto_server_config.cc
index 3659270..ec88594c 100644
--- a/net/quic/crypto/quic_crypto_server_config.cc
+++ b/net/quic/crypto/quic_crypto_server_config.cc
@@ -466,7 +466,7 @@ bool QuicCryptoServerConfig::SetConfigs(
 
     configs_.swap(new_configs);
     SelectNewPrimaryConfig(now);
-    DCHECK(primary_config_);
+    DCHECK(primary_config_.get());
     DCHECK_EQ(configs_.find(primary_config_->id)->second, primary_config_);
   }
 
@@ -507,8 +507,8 @@ void QuicCryptoServerConfig::ValidateClientHello(
       if (!next_config_promotion_time_.IsZero() &&
           next_config_promotion_time_.IsAfter(now)) {
         SelectNewPrimaryConfig(now);
-        DCHECK(primary_config_);
-        DCHECK(configs_.find(primary_config_->id)->second == primary_config_);
+        DCHECK(primary_config_.get());
+        DCHECK_EQ(configs_.find(primary_config_->id)->second, primary_config_);
       }
 
       memcpy(primary_orbit, primary_config_->orbit, sizeof(primary_orbit));
@@ -580,8 +580,8 @@ QuicErrorCode QuicCryptoServerConfig::ProcessClientHello(
     if (!next_config_promotion_time_.IsZero() &&
         next_config_promotion_time_.IsAfter(now)) {
       SelectNewPrimaryConfig(now);
-      DCHECK(primary_config_);
-      DCHECK(configs_.find(primary_config_->id)->second == primary_config_);
+      DCHECK(primary_config_.get());
+      DCHECK_EQ(configs_.find(primary_config_->id)->second, primary_config_);
     }
 
     // We'll use the config that the client requested in order to do
diff --git a/net/quic/crypto/source_address_token.cc b/net/quic/crypto/source_address_token.cc
index b095e76..f20c343 100644
--- a/net/quic/crypto/source_address_token.cc
+++ b/net/quic/crypto/source_address_token.cc
@@ -14,6 +14,12 @@ using std::vector;
 
 namespace net {
 
+CachedNetworkParameters::CachedNetworkParameters() {
+}
+
+CachedNetworkParameters::~CachedNetworkParameters() {
+}
+
 SourceAddressToken::SourceAddressToken() {
 }
 
@@ -27,6 +33,8 @@ string SourceAddressToken::SerializeAsString() const {
   string time_str = base::Int64ToString(timestamp_);
   out.push_back(time_str.size());
   out.append(time_str);
+  // TODO(rtenneti): Implement serialization of optional CachedNetworkParameters
+  // when they are used.
   return out;
 }
 
@@ -52,6 +60,9 @@ bool SourceAddressToken::ParseFromArray(const char* plaintext,
 
   ip_.assign(&plaintext[1], ip_len);
   timestamp_ = timestamp;
+
+  // TODO(rtenneti): Implement parsing of optional CachedNetworkParameters when
+  // they are used.
   return true;
 }
 
diff --git a/net/quic/crypto/source_address_token.h b/net/quic/crypto/source_address_token.h
index fbb50b1..c719fef 100644
--- a/net/quic/crypto/source_address_token.h
+++ b/net/quic/crypto/source_address_token.h
@@ -13,6 +13,69 @@
 namespace net {
 
 // TODO(rtenneti): sync with server more rationally.
+// CachedNetworkParameters contains data that can be used to choose appropriate
+// connection parameters (initial RTT, initial CWND, etc.) in new connections.
+class CachedNetworkParameters {
+ public:
+  // Describes the state of the connection during which the supplied network
+  // parameters were calculated.
+  enum PreviousConnectionState {
+    SLOW_START = 0,
+    CONGESTION_AVOIDANCE = 1,
+  };
+
+  CachedNetworkParameters();
+  ~CachedNetworkParameters();
+
+  std::string serving_region() const {
+    return serving_region_;
+  }
+  void set_serving_region(base::StringPiece serving_region) {
+    serving_region_ = serving_region.as_string();
+  }
+
+  int32 bandwidth_estimate_bytes_per_second() const {
+    return bandwidth_estimate_bytes_per_second_;
+  }
+  void set_bandwidth_estimate_bytes_per_second(
+      int32 bandwidth_estimate_bytes_per_second) {
+    bandwidth_estimate_bytes_per_second_ = bandwidth_estimate_bytes_per_second;
+  }
+
+  int32 min_rtt_ms() const {
+    return min_rtt_ms_;
+  }
+  void set_min_rtt_ms(int32 min_rtt_ms) {
+    min_rtt_ms_ = min_rtt_ms;
+  }
+
+  int32 previous_connection_state() const {
+    return previous_connection_state_;
+  }
+  void set_previous_connection_state(int32 previous_connection_state) {
+    previous_connection_state_ = previous_connection_state;
+  }
+
+ private:
+  // serving_region_ is used to decide whether or not the bandwidth estimate and
+  // min RTT are reasonable and if they should be used.
+  // For example a group of geographically close servers may share the same
+  // serving_region_ string if they are expected to have similar network
+  // performance.
+  std::string serving_region_;
+  // The server can supply a bandwidth estimate (in bytes/s) which it may re-use
+  // on receipt of a source-address token with this field set.
+  int32 bandwidth_estimate_bytes_per_second_;
+  // The min RTT seen on a previous connection can be used by the server to
+  // inform initial connection parameters for new connections.
+  int32 min_rtt_ms_;
+  // Encodes the PreviousConnectionState enum.
+  int32 previous_connection_state_;
+};
+
+// TODO(rtenneti): sync with server more rationally.
+// A SourceAddressToken is serialised, encrypted and sent to clients so that
+// they can prove ownership of an IP address.
 class SourceAddressToken {
  public:
   SourceAddressToken();
@@ -25,23 +88,37 @@ class SourceAddressToken {
   std::string ip() const {
     return ip_;
   }
-
-  int64 timestamp() const {
-    return timestamp_;
-  }
-
   void set_ip(base::StringPiece ip) {
     ip_ = ip.as_string();
   }
 
+  int64 timestamp() const {
+    return timestamp_;
+  }
   void set_timestamp(int64 timestamp) {
     timestamp_ = timestamp;
   }
 
+  const CachedNetworkParameters& cached_network_parameters() const {
+    return cached_network_parameters_;
+  }
+  void set_cached_network_parameters(
+      const CachedNetworkParameters& cached_network_parameters) {
+    cached_network_parameters_ = cached_network_parameters;
+  }
+
  private:
+  // ip_ contains either 4 (IPv4) or 16 (IPv6) bytes of IP address in network
+  // byte order.
   std::string ip_;
+  // timestamp_ contains a UNIX timestamp value of the time when the token was
+  // created.
   int64 timestamp_;
 
+  // The server can provide estimated network parameters to be used for
+  // initial parameter selection in future connections.
+  CachedNetworkParameters cached_network_parameters_;
+
   DISALLOW_COPY_AND_ASSIGN(SourceAddressToken);
 };
 
diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc
index 7d89917..2a091ca 100644
--- a/net/quic/quic_connection.cc
+++ b/net/quic/quic_connection.cc
@@ -191,12 +191,14 @@ QuicConnection::QuicConnection(QuicConnectionId connection_id,
                                IPEndPoint address,
                                QuicConnectionHelperInterface* helper,
                                QuicPacketWriter* writer,
+                               bool owns_writer,
                                bool is_server,
                                const QuicVersionVector& supported_versions)
     : framer_(supported_versions, helper->GetClock()->ApproximateNow(),
               is_server),
       helper_(helper),
       writer_(writer),
+      owns_writer_(owns_writer),
       encryption_level_(ENCRYPTION_NONE),
       clock_(helper->GetClock()),
       random_generator_(helper->GetRandomGenerator()),
@@ -250,6 +252,9 @@ QuicConnection::QuicConnection(QuicConnectionId connection_id,
 }
 
 QuicConnection::~QuicConnection() {
+  if (owns_writer_) {
+    delete writer_;
+  }
   STLDeleteElements(&undecryptable_packets_);
   STLDeleteValues(&group_map_);
   for (QueuedPacketList::iterator it = queued_packets_.begin();
diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h
index 8c6d2ca..a7e8a7e 100644
--- a/net/quic/quic_connection.h
+++ b/net/quic/quic_connection.h
@@ -223,11 +223,13 @@ class NET_EXPORT_PRIVATE QuicConnection
   };
 
   // Constructs a new QuicConnection for |connection_id| and |address|.
-  // |helper| and |writer| must outlive this connection.
+  // |helper| must outlive this connection, and if |owns_writer| is false, so
+  // must |writer|.
   QuicConnection(QuicConnectionId connection_id,
                  IPEndPoint address,
                  QuicConnectionHelperInterface* helper,
                  QuicPacketWriter* writer,
+                 bool owns_writer,
                  bool is_server,
                  const QuicVersionVector& supported_versions);
   virtual ~QuicConnection();
@@ -303,11 +305,6 @@ class NET_EXPORT_PRIVATE QuicConnection
   // If the socket is not blocked, writes queued packets.
   void WriteIfNotBlocked();
 
-  // Do any work which logically would be done in OnPacket but can not be
-  // safely done until the packet is validated.  Returns true if the packet
-  // can be handled, false otherwise.
-  virtual bool ProcessValidatedPacket();
-
   // The version of the protocol this connection is using.
   QuicVersion version() const { return framer_.version(); }
 
@@ -490,6 +487,11 @@ class NET_EXPORT_PRIVATE QuicConnection
   };
 
  protected:
+  // Do any work which logically would be done in OnPacket but can not be
+  // safely done until the packet is validated.  Returns true if the packet
+  // can be handled, false otherwise.
+  virtual bool ProcessValidatedPacket();
+
   // Send a packet to the peer using encryption |level|. If |sequence_number|
   // is present in the |retransmission_map_|, then contents of this packet will
   // be retransmitted with a new sequence number if it's not acked by the peer.
@@ -632,7 +634,8 @@ class NET_EXPORT_PRIVATE QuicConnection
 
   QuicFramer framer_;
   QuicConnectionHelperInterface* helper_;  // Not owned.
-  QuicPacketWriter* writer_;  // Not owned.
+  QuicPacketWriter* writer_;  // Owned or not depending on |owns_writer_|.
+  bool owns_writer_;
   EncryptionLevel encryption_level_;
   const QuicClock* clock_;
   QuicRandom* random_generator_;
diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc
index a0587b5..b868c98 100644
--- a/net/quic/quic_connection_test.cc
+++ b/net/quic/quic_connection_test.cc
@@ -415,7 +415,12 @@ class TestConnection : public QuicConnection {
                  TestPacketWriter* writer,
                  bool is_server,
                  QuicVersion version)
-      : QuicConnection(connection_id, address, helper, writer, is_server,
+      : QuicConnection(connection_id,
+                       address,
+                       helper,
+                       writer,
+                       false  /* owns_writer */,
+                       is_server,
                        SupportedVersions(version)),
         writer_(writer) {
     // Disable tail loss probes for most tests.
diff --git a/net/quic/quic_dispatcher.cc b/net/quic/quic_dispatcher.cc
index ae964ed..5c6836a 100644
--- a/net/quic/quic_dispatcher.cc
+++ b/net/quic/quic_dispatcher.cc
@@ -377,13 +377,23 @@ QuicConnection* QuicDispatcher::CreateQuicConnection(
   QuicConnection* connection;
   if (FLAGS_enable_quic_connection_flow_control_2) {
     DVLOG(1) << "Creating QuicDispatcher with all versions.";
-    connection = new QuicConnection(connection_id, client_address, helper_,
-                                    writer, true, supported_versions_);
+    connection = new QuicConnection(connection_id,
+                                    client_address,
+                                    helper_,
+                                    writer,
+                                    false  /* owns_writer */,
+                                    true   /* is_server */,
+                                    supported_versions_);
   } else {
     DVLOG(1) << "Connection flow control disabled, creating QuicDispatcher "
              << "WITHOUT version 19 or higher.";
     connection = new QuicConnection(
-        connection_id, client_address, helper_, writer, true,
+        connection_id,
+        client_address,
+        helper_,
+        writer,
+        false  /* owns_writer */,
+        true   /* is_server */,
         supported_versions_no_connection_flow_control_);
   }
   writer->set_connection(connection);
diff --git a/net/quic/quic_http_stream_test.cc b/net/quic/quic_http_stream_test.cc
index 2b85417..5e46258 100644
--- a/net/quic/quic_http_stream_test.cc
+++ b/net/quic/quic_http_stream_test.cc
@@ -59,7 +59,12 @@ class TestQuicConnection : public QuicConnection {
                      IPEndPoint address,
                      QuicConnectionHelper* helper,
                      QuicPacketWriter* writer)
-      : QuicConnection(connection_id, address, helper, writer, false,
+      : QuicConnection(connection_id,
+                       address,
+                       helper,
+                       writer,
+                       false  /* owns_writer */,
+                       false  /* is_server */,
                        versions) {
   }
 
diff --git a/net/quic/quic_received_packet_manager.cc b/net/quic/quic_received_packet_manager.cc
index 3b47687..1a1a1d3 100644
--- a/net/quic/quic_received_packet_manager.cc
+++ b/net/quic/quic_received_packet_manager.cc
@@ -178,6 +178,8 @@ void QuicReceivedPacketManager::RecordPacketReceived(
 
   receive_algorithm_->RecordIncomingPacket(
       bytes, sequence_number, receipt_time);
+
+  received_info_.revived_packets.erase(sequence_number);
 }
 
 void QuicReceivedPacketManager::RecordPacketRevived(
diff --git a/net/quic/quic_received_packet_manager_test.cc b/net/quic/quic_received_packet_manager_test.cc
index b579309..b2a52c8 100644
--- a/net/quic/quic_received_packet_manager_test.cc
+++ b/net/quic/quic_received_packet_manager_test.cc
@@ -202,6 +202,10 @@ class QuicReceivedPacketManagerTest : public ::testing::Test {
     received_manager_.RecordPacketReceived(0u, header, receipt_time);
   }
 
+  void RecordPacketRevived(QuicPacketSequenceNumber sequence_number) {
+    received_manager_.RecordPacketRevived(sequence_number);
+  }
+
   QuicConnectionStats stats_;
   QuicReceivedPacketManager received_manager_;
 };
@@ -328,6 +332,32 @@ TEST_F(QuicReceivedPacketManagerTest, UpdateReceivedConnectionStats) {
   EXPECT_EQ(1u, stats_.packets_reordered);
 }
 
+TEST_F(QuicReceivedPacketManagerTest, RevivedPacket) {
+  RecordPacketReceipt(1, 0);
+  RecordPacketReceipt(3, 0);
+  RecordPacketRevived(2);
+
+  ReceivedPacketInfo info;
+  received_manager_.UpdateReceivedPacketInfo(&info, QuicTime::Zero());
+  EXPECT_EQ(1u, info.missing_packets.size());
+  EXPECT_EQ(2u, *info.missing_packets.begin());
+  EXPECT_EQ(1u, info.revived_packets.size());
+  EXPECT_EQ(2u, *info.missing_packets.begin());
+}
+
+TEST_F(QuicReceivedPacketManagerTest, PacketRevivedThenReceived) {
+  RecordPacketReceipt(1, 0);
+  RecordPacketReceipt(3, 0);
+  RecordPacketRevived(2);
+  RecordPacketReceipt(2, 0);
+
+  ReceivedPacketInfo info;
+  received_manager_.UpdateReceivedPacketInfo(&info, QuicTime::Zero());
+  EXPECT_TRUE(info.missing_packets.empty());
+  EXPECT_TRUE(info.revived_packets.empty());
+}
+
+
 }  // namespace
 }  // namespace test
 }  // namespace net
diff --git a/net/quic/quic_sent_packet_manager.cc b/net/quic/quic_sent_packet_manager.cc
index 30f34e3..c51b744 100644
--- a/net/quic/quic_sent_packet_manager.cc
+++ b/net/quic/quic_sent_packet_manager.cc
@@ -109,6 +109,11 @@ void QuicSentPacketManager::SetFromConfig(const QuicConfig& config) {
     send_algorithm_.reset(
         SendAlgorithmInterface::Create(clock_, &rtt_stats_, kBBR, stats_));
   }
+  if (config.HasReceivedConnectionOptions() &&
+      ContainsQuicTag(config.ReceivedConnectionOptions(), kRENO)) {
+    send_algorithm_.reset(
+        SendAlgorithmInterface::Create(clock_, &rtt_stats_, kReno, stats_));
+  }
   if (config.congestion_feedback() == kPACE ||
       (config.HasReceivedConnectionOptions() &&
        ContainsQuicTag(config.ReceivedConnectionOptions(), kPACE))) {
diff --git a/net/quic/quic_sent_packet_manager_test.cc b/net/quic/quic_sent_packet_manager_test.cc
index 90f94f7..99ed479 100644
--- a/net/quic/quic_sent_packet_manager_test.cc
+++ b/net/quic/quic_sent_packet_manager_test.cc
@@ -1392,6 +1392,29 @@ TEST_F(QuicSentPacketManagerTest, NegotiateTimeLossDetectionFromOptions) {
                 &manager_)->GetLossDetectionType());
 }
 
+TEST_F(QuicSentPacketManagerTest, NegotiateCongestionControlFromOptions) {
+  QuicConfig config;
+  QuicTagVector options;
+
+  options.push_back(kRENO);
+  QuicConfigPeer::SetReceivedConnectionOptions(&config, options);
+  EXPECT_CALL(*network_change_visitor_, OnCongestionWindowChange(_));
+  manager_.SetFromConfig(config);
+  EXPECT_EQ(kReno, QuicSentPacketManagerPeer::GetCongestionControlAlgorithm(
+      manager_)->GetCongestionControlType());
+
+  // TODO(rtenneti): Enable the following code after BBR code is checked in.
+#if 0
+  options.clear();
+  options.push_back(kTBBR);
+  QuicConfigPeer::SetReceivedConnectionOptions(&config, options);
+  EXPECT_CALL(*network_change_visitor_, OnCongestionWindowChange(_));
+  manager_.SetFromConfig(config);
+  EXPECT_EQ(kBBR, QuicSentPacketManagerPeer::GetCongestionControlAlgorithm(
+      manager_)->GetCongestionControlType());
+#endif
+}
+
 TEST_F(QuicSentPacketManagerTest, NegotiatePacingFromOptions) {
   ValueRestore<bool> old_flag(&FLAGS_enable_quic_pacing, true);
   EXPECT_FALSE(manager_.using_pacing());
diff --git a/net/quic/quic_stream_factory.cc b/net/quic/quic_stream_factory.cc
index c399456..503e549 100644
--- a/net/quic/quic_stream_factory.cc
+++ b/net/quic/quic_stream_factory.cc
@@ -833,9 +833,13 @@ int QuicStreamFactory::CreateSession(
         clock_.get(), random_generator_));
   }
 
-  QuicConnection* connection =
-      new QuicConnection(connection_id, addr, helper_.get(), writer.get(),
-                         false, supported_versions_);
+  QuicConnection* connection = new QuicConnection(connection_id,
+                                                  addr,
+                                                  helper_.get(),
+                                                  writer.get(),
+                                                  false  /* owns_writer */,
+                                                  false  /* is_server */,
+                                                  supported_versions_);
   writer->SetConnection(connection);
   connection->set_max_packet_length(max_packet_length_);
 
diff --git a/net/quic/quic_time.cc b/net/quic/quic_time.cc
index d467980..e20a338 100644
--- a/net/quic/quic_time.cc
+++ b/net/quic/quic_time.cc
@@ -90,6 +90,11 @@ QuicTime QuicTime::Zero() {
 }
 
 // static
+QuicTime QuicTime::Infinite() {
+  return QuicTime(base::TimeTicks::FromInternalValue(kQuicInfiniteTimeUs));
+}
+
+// static
 QuicTime QuicTime::Max(QuicTime time1, QuicTime time2) {
   return time1 > time2 ? time1 : time2;
 }
diff --git a/net/quic/quic_time.h b/net/quic/quic_time.h
index 20cc308..5fbe30c 100644
--- a/net/quic/quic_time.h
+++ b/net/quic/quic_time.h
@@ -81,6 +81,9 @@ class NET_EXPORT_PRIVATE QuicTime {
   // will return false for these times.
   static QuicTime Zero();
 
+  // Creates a new QuicTime with an infinite time.
+  static QuicTime Infinite();
+
   // Returns the later time of time1 and time2.
   static QuicTime Max(QuicTime time1, QuicTime time2);
 
diff --git a/net/quic/quic_unacked_packet_map.cc b/net/quic/quic_unacked_packet_map.cc
index 563b53d..a07de8b 100644
--- a/net/quic/quic_unacked_packet_map.cc
+++ b/net/quic/quic_unacked_packet_map.cc
@@ -74,6 +74,15 @@ void QuicUnackedPacketMap::OnRetransmittedPacket(
   // We keep the old packet in the unacked packet list until it, or one of
   // the retransmissions of it are acked.
   transmission_info->retransmittable_frames = NULL;
+  // Only keep one transmission older than largest observed, because only the
+  // most recent is expected to possibly be a spurious retransmission.
+  if (transmission_info->all_transmissions->size() > 1 &&
+      *(++transmission_info->all_transmissions->begin()) < largest_observed_) {
+    QuicPacketSequenceNumber old_transmission =
+        *transmission_info->all_transmissions->begin();
+    transmission_info->all_transmissions->erase(old_transmission);
+    unacked_packets_.erase(old_transmission);
+  }
   unacked_packets_[new_sequence_number] =
       TransmissionInfo(frames,
                        new_sequence_number,
diff --git a/net/quic/quic_unacked_packet_map_test.cc b/net/quic/quic_unacked_packet_map_test.cc
index c5264fd1..87c6ca8 100644
--- a/net/quic/quic_unacked_packet_map_test.cc
+++ b/net/quic/quic_unacked_packet_map_test.cc
@@ -33,8 +33,8 @@ class QuicUnackedPacketMapTest : public ::testing::Test {
         sequence_number, PACKET_1BYTE_SEQUENCE_NUMBER, NULL, 0, NULL);
   }
 
-  void VerifyPendingPackets(QuicPacketSequenceNumber* packets,
-                            size_t num_packets) {
+  void VerifyInFlightPackets(QuicPacketSequenceNumber* packets,
+                             size_t num_packets) {
     if (num_packets == 0) {
       EXPECT_FALSE(unacked_packets_.HasInFlightPackets());
       EXPECT_FALSE(unacked_packets_.HasMultipleInFlightPackets());
@@ -43,11 +43,21 @@ class QuicUnackedPacketMapTest : public ::testing::Test {
     if (num_packets == 1) {
       EXPECT_TRUE(unacked_packets_.HasInFlightPackets());
       EXPECT_FALSE(unacked_packets_.HasMultipleInFlightPackets());
+      ASSERT_TRUE(unacked_packets_.IsUnacked(packets[0]));
+      EXPECT_TRUE(unacked_packets_.GetTransmissionInfo(packets[0]).in_flight);
     }
     for (size_t i = 0; i < num_packets; ++i) {
       ASSERT_TRUE(unacked_packets_.IsUnacked(packets[i]));
       EXPECT_TRUE(unacked_packets_.GetTransmissionInfo(packets[i]).in_flight);
     }
+    size_t in_flight_count = 0;
+    for (QuicUnackedPacketMap::const_iterator it = unacked_packets_.begin();
+         it != unacked_packets_.end(); ++it) {
+      if (it->second.in_flight) {
+        ++in_flight_count;
+      }
+    }
+    EXPECT_EQ(num_packets, in_flight_count);
   }
 
   void VerifyUnackedPackets(QuicPacketSequenceNumber* packets,
@@ -61,6 +71,12 @@ class QuicUnackedPacketMapTest : public ::testing::Test {
     for (size_t i = 0; i < num_packets; ++i) {
       EXPECT_TRUE(unacked_packets_.IsUnacked(packets[i])) << packets[i];
     }
+    size_t unacked_count = 0;
+    for (QuicUnackedPacketMap::const_iterator it = unacked_packets_.begin();
+         it != unacked_packets_.end(); ++it) {
+      ++unacked_count;
+    }
+    EXPECT_EQ(num_packets, unacked_count);
   }
 
   void VerifyRetransmittablePackets(QuicPacketSequenceNumber* packets,
@@ -90,12 +106,12 @@ TEST_F(QuicUnackedPacketMapTest, RttOnly) {
 
   QuicPacketSequenceNumber unacked[] = { 1 };
   VerifyUnackedPackets(unacked, arraysize(unacked));
-  VerifyPendingPackets(NULL, 0);
+  VerifyInFlightPackets(NULL, 0);
   VerifyRetransmittablePackets(NULL, 0);
 
   unacked_packets_.IncreaseLargestObserved(1);
   VerifyUnackedPackets(NULL, 0);
-  VerifyPendingPackets(NULL, 0);
+  VerifyInFlightPackets(NULL, 0);
   VerifyRetransmittablePackets(NULL, 0);
 }
 
@@ -106,22 +122,22 @@ TEST_F(QuicUnackedPacketMapTest, RetransmittableInflightAndRtt) {
 
   QuicPacketSequenceNumber unacked[] = { 1 };
   VerifyUnackedPackets(unacked, arraysize(unacked));
-  VerifyPendingPackets(unacked, arraysize(unacked));
+  VerifyInFlightPackets(unacked, arraysize(unacked));
   VerifyRetransmittablePackets(unacked, arraysize(unacked));
 
   unacked_packets_.RemoveRetransmittability(1);
   VerifyUnackedPackets(unacked, arraysize(unacked));
-  VerifyPendingPackets(unacked, arraysize(unacked));
+  VerifyInFlightPackets(unacked, arraysize(unacked));
   VerifyRetransmittablePackets(NULL, 0);
 
   unacked_packets_.IncreaseLargestObserved(1);
   VerifyUnackedPackets(unacked, arraysize(unacked));
-  VerifyPendingPackets(unacked, arraysize(unacked));
+  VerifyInFlightPackets(unacked, arraysize(unacked));
   VerifyRetransmittablePackets(NULL, 0);
 
   unacked_packets_.RemoveFromInFlight(1);
   VerifyUnackedPackets(NULL, 0);
-  VerifyPendingPackets(NULL, 0);
+  VerifyInFlightPackets(NULL, 0);
   VerifyRetransmittablePackets(NULL, 0);
 }
 
@@ -135,32 +151,78 @@ TEST_F(QuicUnackedPacketMapTest, RetransmittedPacket) {
 
   QuicPacketSequenceNumber unacked[] = { 1, 2 };
   VerifyUnackedPackets(unacked, arraysize(unacked));
-  VerifyPendingPackets(unacked, arraysize(unacked));
+  VerifyInFlightPackets(unacked, arraysize(unacked));
   QuicPacketSequenceNumber retransmittable[] = { 2 };
   VerifyRetransmittablePackets(retransmittable, arraysize(retransmittable));
 
   unacked_packets_.RemoveRetransmittability(1);
   VerifyUnackedPackets(unacked, arraysize(unacked));
-  VerifyPendingPackets(unacked, arraysize(unacked));
+  VerifyInFlightPackets(unacked, arraysize(unacked));
   VerifyRetransmittablePackets(NULL, 0);
 
   unacked_packets_.IncreaseLargestObserved(2);
   VerifyUnackedPackets(unacked, arraysize(unacked));
-  VerifyPendingPackets(unacked, arraysize(unacked));
+  VerifyInFlightPackets(unacked, arraysize(unacked));
   VerifyRetransmittablePackets(NULL, 0);
 
   unacked_packets_.RemoveFromInFlight(2);
   QuicPacketSequenceNumber unacked2[] = { 1 };
   VerifyUnackedPackets(unacked, arraysize(unacked2));
-  VerifyPendingPackets(unacked, arraysize(unacked2));
+  VerifyInFlightPackets(unacked, arraysize(unacked2));
   VerifyRetransmittablePackets(NULL, 0);
 
   unacked_packets_.RemoveFromInFlight(1);
   VerifyUnackedPackets(NULL, 0);
-  VerifyPendingPackets(NULL, 0);
+  VerifyInFlightPackets(NULL, 0);
   VerifyRetransmittablePackets(NULL, 0);
 }
 
+TEST_F(QuicUnackedPacketMapTest, RetransmitTwice) {
+  // Simulate a retransmittable packet being sent and retransmitted twice.
+  unacked_packets_.AddPacket(CreateRetransmittablePacket(1));
+  unacked_packets_.SetSent(1, now_, kDefaultLength, true);
+  unacked_packets_.AddPacket(CreateRetransmittablePacket(2));
+  unacked_packets_.SetSent(2, now_, kDefaultLength, true);
+
+  QuicPacketSequenceNumber unacked[] = { 1, 2 };
+  VerifyUnackedPackets(unacked, arraysize(unacked));
+  VerifyInFlightPackets(unacked, arraysize(unacked));
+  QuicPacketSequenceNumber retransmittable[] = { 1, 2 };
+  VerifyRetransmittablePackets(retransmittable, arraysize(retransmittable));
+
+  // Early retransmit 1 as 3 and send new data as 4.
+  unacked_packets_.IncreaseLargestObserved(2);
+  unacked_packets_.RemoveFromInFlight(2);
+  unacked_packets_.RemoveRetransmittability(2);
+  unacked_packets_.RemoveFromInFlight(1);
+  unacked_packets_.OnRetransmittedPacket(1, 3, LOSS_RETRANSMISSION);
+  unacked_packets_.SetSent(3, now_, kDefaultLength, true);
+  unacked_packets_.AddPacket(CreateRetransmittablePacket(4));
+  unacked_packets_.SetSent(4, now_, kDefaultLength, true);
+
+  QuicPacketSequenceNumber unacked2[] = { 1, 3, 4 };
+  VerifyUnackedPackets(unacked2, arraysize(unacked2));
+  QuicPacketSequenceNumber pending2[] = { 3, 4, };
+  VerifyInFlightPackets(pending2, arraysize(pending2));
+  QuicPacketSequenceNumber retransmittable2[] = { 3, 4 };
+  VerifyRetransmittablePackets(retransmittable2, arraysize(retransmittable2));
+
+  // Early retransmit 3 (formerly 1) as 5, and remove 1 from unacked.
+  unacked_packets_.IncreaseLargestObserved(4);
+  unacked_packets_.RemoveFromInFlight(4);
+  unacked_packets_.RemoveRetransmittability(4);
+  unacked_packets_.RemoveFromInFlight(3);
+  unacked_packets_.OnRetransmittedPacket(3, 5, LOSS_RETRANSMISSION);
+  unacked_packets_.SetSent(5, now_, kDefaultLength, true);
+
+  QuicPacketSequenceNumber unacked3[] = { 3, 5 };
+  VerifyUnackedPackets(unacked3, arraysize(unacked3));
+  QuicPacketSequenceNumber pending3[] = { 5 };
+  VerifyInFlightPackets(pending3, arraysize(pending3));
+  QuicPacketSequenceNumber retransmittable3[] = { 5 };
+  VerifyRetransmittablePackets(retransmittable3, arraysize(retransmittable3));
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace net
diff --git a/net/quic/test_tools/quic_connection_peer.cc b/net/quic/test_tools/quic_connection_peer.cc
index e51fb52..c4c63c7 100644
--- a/net/quic/test_tools/quic_connection_peer.cc
+++ b/net/quic/test_tools/quic_connection_peer.cc
@@ -208,8 +208,13 @@ QuicPacketWriter* QuicConnectionPeer::GetWriter(QuicConnection* connection) {
 
 // static
 void QuicConnectionPeer::SetWriter(QuicConnection* connection,
-                                   QuicPacketWriter* writer) {
+                                   QuicPacketWriter* writer,
+                                   bool owns_writer) {
+  if (connection->owns_writer_) {
+    delete connection->writer_;
+  }
   connection->writer_ = writer;
+  connection->owns_writer_ = owns_writer;
 }
 
 // static
diff --git a/net/quic/test_tools/quic_connection_peer.h b/net/quic/test_tools/quic_connection_peer.h
index 0cc6fbb..11d4814 100644
--- a/net/quic/test_tools/quic_connection_peer.h
+++ b/net/quic/test_tools/quic_connection_peer.h
@@ -106,7 +106,10 @@ class QuicConnectionPeer {
   static QuicAlarm* GetTimeoutAlarm(QuicConnection* connection);
 
   static QuicPacketWriter* GetWriter(QuicConnection* connection);
-  static void SetWriter(QuicConnection* connection, QuicPacketWriter* writer);
+  // If |owns_writer| is true, takes ownership of |writer|.
+  static void SetWriter(QuicConnection* connection,
+                        QuicPacketWriter* writer,
+                        bool owns_writer);
   static void CloseConnection(QuicConnection* connection);
   static QuicEncryptedPacket* GetConnectionClosePacket(
       QuicConnection* connection);
diff --git a/net/quic/test_tools/quic_sent_packet_manager_peer.cc b/net/quic/test_tools/quic_sent_packet_manager_peer.cc
index 2b2fcb9..83a08e0 100644
--- a/net/quic/test_tools/quic_sent_packet_manager_peer.cc
+++ b/net/quic/test_tools/quic_sent_packet_manager_peer.cc
@@ -34,9 +34,9 @@ const LossDetectionInterface* QuicSentPacketManagerPeer::GetLossAlgorithm(
 
 // static
 const SendAlgorithmInterface*
-QuicSentPacketManagerPeer::GetCongestionControlAlgorithm(
-    QuicSentPacketManager* sent_packet_manager) {
-  return sent_packet_manager->send_algorithm_.get();
+    QuicSentPacketManagerPeer::GetCongestionControlAlgorithm(
+    const QuicSentPacketManager& sent_packet_manager) {
+  return sent_packet_manager.send_algorithm_.get();
 }
 
 // static
diff --git a/net/quic/test_tools/quic_sent_packet_manager_peer.h b/net/quic/test_tools/quic_sent_packet_manager_peer.h
index 77fba0b..18eb6be 100644
--- a/net/quic/test_tools/quic_sent_packet_manager_peer.h
+++ b/net/quic/test_tools/quic_sent_packet_manager_peer.h
@@ -26,7 +26,7 @@ class QuicSentPacketManagerPeer {
       QuicSentPacketManager* sent_packet_manager);
 
   static const SendAlgorithmInterface* GetCongestionControlAlgorithm(
-      QuicSentPacketManager* sent_packet_manager);
+      const QuicSentPacketManager& sent_packet_manager);
 
   static void SetLossAlgorithm(QuicSentPacketManager* sent_packet_manager,
                                LossDetectionInterface* loss_detector);
diff --git a/net/quic/test_tools/quic_test_utils.cc b/net/quic/test_tools/quic_test_utils.cc
index 62a891b..9b54f94 100644
--- a/net/quic/test_tools/quic_test_utils.cc
+++ b/net/quic/test_tools/quic_test_utils.cc
@@ -230,8 +230,8 @@ MockConnection::MockConnection(bool is_server)
                      IPEndPoint(TestPeerIPAddress(), kTestPort),
                      new testing::NiceMock<MockHelper>(),
                      new testing::NiceMock<MockPacketWriter>(),
+                     true  /* owns_writer */,
                      is_server, QuicSupportedVersions()),
-      writer_(QuicConnectionPeer::GetWriter(this)),
       helper_(helper()) {
 }
 
@@ -240,8 +240,8 @@ MockConnection::MockConnection(IPEndPoint address,
     : QuicConnection(kTestConnectionId, address,
                      new testing::NiceMock<MockHelper>(),
                      new testing::NiceMock<MockPacketWriter>(),
+                     true  /* owns_writer */,
                      is_server, QuicSupportedVersions()),
-      writer_(QuicConnectionPeer::GetWriter(this)),
       helper_(helper()) {
 }
 
@@ -251,8 +251,8 @@ MockConnection::MockConnection(QuicConnectionId connection_id,
                      IPEndPoint(TestPeerIPAddress(), kTestPort),
                      new testing::NiceMock<MockHelper>(),
                      new testing::NiceMock<MockPacketWriter>(),
+                     true  /* owns_writer */,
                      is_server, QuicSupportedVersions()),
-      writer_(QuicConnectionPeer::GetWriter(this)),
       helper_(helper()) {
 }
 
@@ -262,8 +262,8 @@ MockConnection::MockConnection(bool is_server,
                      IPEndPoint(TestPeerIPAddress(), kTestPort),
                      new testing::NiceMock<MockHelper>(),
                      new testing::NiceMock<MockPacketWriter>(),
+                     true  /* owns_writer */,
                      is_server, supported_versions),
-      writer_(QuicConnectionPeer::GetWriter(this)),
       helper_(helper()) {
 }
 
diff --git a/net/quic/test_tools/quic_test_utils.h b/net/quic/test_tools/quic_test_utils.h
index 631d59f..3bae37f 100644
--- a/net/quic/test_tools/quic_test_utils.h
+++ b/net/quic/test_tools/quic_test_utils.h
@@ -315,7 +315,6 @@ class MockConnection : public QuicConnection {
   }
 
  private:
-  scoped_ptr<QuicPacketWriter> writer_;
   scoped_ptr<QuicConnectionHelperInterface> helper_;
 
   DISALLOW_COPY_AND_ASSIGN(MockConnection);