Landing Recent QUIC Changes.

Making the (deprecation-in-progress) congestion field write only.

This is a bit of a hack because technically it's "required" for
QUIC_VERSION_23 but as all the internal servers and all of chrome builds
picking up this change consider the field optional despite the version,
it should just Work (TM).

I was hoping to stop sending the congestion fields for v24 clients but
that is not safe until the internal server fleet doesn't consider it a
required field for negotiated()=true.  Bah.

No longer reading the (deprecated) congestion fields for QUIC.

Merge internal change: 85158634
https://codereview.chromium.org/898233003/

Improving junk packet handling for QUIC by ignoring packets with client
port 0.

Merge internal change: 85067984
https://codereview.chromium.org/885713009/

Removing deprecated flag FLAGS_quic_use_initial_rtt_for_stats.

Merge internal change: 84999682
https://codereview.chromium.org/883393008/

Adding more error logging for failed QUIC writes.

Merge internal change: 84989992
https://codereview.chromium.org/898243002/

Further cleanup to QuicAckNotifier and QuicAckNotifierManager.
No functional change.

Merge internal change: 84784925
https://codereview.chromium.org/880403006/

Remove an unneeded hash_set from QuicAckNotifierManager.
No change in behavior.

Merge internal change: 84637544
https://codereview.chromium.org/872403007/

Minor cleanup and optimization of QuicConnection::IsConnectionClose.

Merge internal change: 84624803
https://codereview.chromium.org/903973002/

Rename QuicAckNotifier's AddSequenceNumber to OnSerializedPacket.
No functional change.

Merge internal change: 84624660
https://codereview.chromium.org/867293004/

R=rch@chromium.org

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

Cr-Commit-Position: refs/heads/master@{#315086}

net/quic/quic_ack_notifier.cc

@@ -29,21 +29,18 @@ QuicAckNotifier::QuicAckNotifier(DelegateInterface* delegate)
 QuicAckNotifier::~QuicAckNotifier() {
 }
 
-void QuicAckNotifier::AddSequenceNumber(
-    const QuicPacketSequenceNumber& sequence_number,
-    int packet_payload_size) {
+void QuicAckNotifier::OnSerializedPacket() {
   ++unacked_packets_;
-  DVLOG(1) << "AckNotifier waiting for packet: " << sequence_number;
 }
 
-bool QuicAckNotifier::OnAck(QuicPacketSequenceNumber sequence_number,
-                            QuicTime::Delta delta_largest_observed) {
+bool QuicAckNotifier::OnAck(QuicTime::Delta delta_largest_observed) {
   if (unacked_packets_ <= 0) {
-    LOG(DFATAL) << "Acked more packets than were tracked.";
+    LOG(DFATAL) << "Acked more packets than were tracked."
+                << " unacked_packets:" << unacked_packets_;
     return true;
   }
   --unacked_packets_;
-  if (IsEmpty()) {
+  if (!HasUnackedPackets()) {
     // We have seen all the sequence numbers we were waiting for, trigger
     // callback notification.
     delegate_->OnAckNotification(retransmitted_packet_count_,
@@ -54,6 +51,16 @@ bool QuicAckNotifier::OnAck(QuicPacketSequenceNumber sequence_number,
   return false;
 }
 
+bool QuicAckNotifier::OnPacketAbandoned() {
+  if (unacked_packets_ <= 0) {
+    LOG(DFATAL) << "Abandoned more packets than were tracked."
+                << " unacked_packets:" << unacked_packets_;
+    return true;
+  }
+  --unacked_packets_;
+  return unacked_packets_ == 0;
+}
+
 void QuicAckNotifier::OnPacketRetransmitted(int packet_payload_size) {
   ++retransmitted_packet_count_;
   retransmitted_byte_count_ += packet_payload_size;

net/quic/quic_ack_notifier.h

@@ -40,22 +40,22 @@ class NET_EXPORT_PRIVATE QuicAckNotifier {
   explicit QuicAckNotifier(DelegateInterface* delegate);
   virtual ~QuicAckNotifier();
 
-  // Register a sequence number that this AckNotifier should be interested in.
-  void AddSequenceNumber(const QuicPacketSequenceNumber& sequence_number,
-                         int packet_payload_size);
-
-  // Called by the QuicConnection on receipt of new ACK frame, with the sequence
-  // number referenced by the ACK frame.
-  // Deletes the matching sequence number from the stored set of sequence
-  // numbers. If this set is now empty, call the stored delegate's
-  // OnAckNotification method.
+  // Register a serialized packet the notifier should track.
+  void OnSerializedPacket();
+
+  // Called on receipt of new ACK frame for an unacked packet.
+  // Decrements the number of unacked packets and if there are none left, calls
+  // the stored delegate's OnAckNotification method.
   //
-  // Returns true if the provided sequence_number caused the delegate to be
-  // called, false otherwise.
-  bool OnAck(QuicPacketSequenceNumber sequence_number,
-             QuicTime::Delta delta_largest_observed);
+  // Returns true if the delegate was called, false otherwise.
+  bool OnAck(QuicTime::Delta delta_largest_observed);
+
+  // Called when we've given up waiting for a sequence number, typically when
+  // the connection is torn down.
+  // Returns true if there are no more unacked packets being tracked.
+  bool OnPacketAbandoned();
 
-  bool IsEmpty() { return unacked_packets_ == 0; }
+  bool HasUnackedPackets() const { return unacked_packets_ > 0; }
 
   // If a packet is retransmitted by the connection, it will be sent with a
   // different sequence number.

net/quic/quic_ack_notifier_manager.cc

@@ -20,7 +20,13 @@ namespace net {
 AckNotifierManager::AckNotifierManager() {}
 
 AckNotifierManager::~AckNotifierManager() {
-  STLDeleteElements(&ack_notifiers_);
+  for (const auto& pair : ack_notifier_map_) {
+    for (QuicAckNotifier* notifier : pair.second) {
+      if (notifier->OnPacketAbandoned()) {
+        delete notifier;
+      }
+    }
+  }
 }
 
 void AckNotifierManager::OnPacketAcked(QuicPacketSequenceNumber sequence_number,
@@ -34,14 +40,10 @@ void AckNotifierManager::OnPacketAcked(QuicPacketSequenceNumber sequence_number,
 
   // One or more AckNotifiers are registered as interested in this sequence
   // number. Iterate through them and call OnAck on each.
-  AckNotifierList& ack_notifier_list = map_it->second;
-  for (QuicAckNotifier* ack_notifier : ack_notifier_list) {
-    ack_notifier->OnAck(sequence_number, delta_largest_observed);
-
-    // If this has resulted in an empty AckNotifer, erase it.
-    if (ack_notifier->IsEmpty()) {
+  for (QuicAckNotifier* ack_notifier : map_it->second) {
+    if (ack_notifier->OnAck(delta_largest_observed)) {
+      // If this has resulted in an empty AckNotifer, erase it.
       delete ack_notifier;
-      ack_notifiers_.erase(ack_notifier);
     }
   }
 
@@ -75,21 +77,19 @@ void AckNotifierManager::OnPacketRetransmitted(
 void AckNotifierManager::OnSerializedPacket(
     const SerializedPacket& serialized_packet) {
   if (FLAGS_quic_attach_ack_notifiers_to_packets) {
-    // Inform each attached AckNotifier of the packet's sequence number.
+    // Inform each attached AckNotifier of the packet's serialization.
+    AckNotifierList& notifier_list =
+        ack_notifier_map_[serialized_packet.sequence_number];
     for (QuicAckNotifier* notifier : serialized_packet.notifiers) {
       if (notifier == nullptr) {
         LOG(DFATAL) << "AckNotifier should not be nullptr.";
         continue;
       }
-      notifier->AddSequenceNumber(serialized_packet.sequence_number,
-                                  serialized_packet.packet->length());
+      notifier->OnSerializedPacket();
 
       // Update the mapping in the other direction, from sequence number to
       // AckNotifier.
-      ack_notifier_map_[serialized_packet.sequence_number].push_back(notifier);
-
-      // Take ownership of the AckNotifier.
-      ack_notifiers_.insert(notifier);
+      notifier_list.push_back(notifier);
     }
   } else {
     // AckNotifiers can only be attached to retransmittable frames.
@@ -107,15 +107,11 @@ void AckNotifierManager::OnSerializedPacket(
       }
 
       QuicAckNotifier* notifier = quic_frame.stream_frame->notifier;
-      notifier->AddSequenceNumber(serialized_packet.sequence_number,
-                                  serialized_packet.packet->length());
+      notifier->OnSerializedPacket();
 
       // Update the mapping in the other direction, from sequence number to
       // AckNotifier.
       ack_notifier_map_[serialized_packet.sequence_number].push_back(notifier);
-
-      // Take ownership of the AckNotifier.
-      ack_notifiers_.insert(notifier);
     }
   }
 }

net/quic/quic_ack_notifier_manager.h

@@ -48,22 +48,14 @@ class NET_EXPORT_PRIVATE AckNotifierManager {
 
  private:
   typedef std::list<QuicAckNotifier*> AckNotifierList;
-  typedef base::hash_set<QuicAckNotifier*> AckNotifierSet;
   // TODO(ianswett): Further improvement may come from changing this to a deque.
   typedef base::hash_map<QuicPacketSequenceNumber, AckNotifierList>
       AckNotifierMap;
 
-  // On every ACK frame received by the connection, all the ack_notifiers_ will
-  // be told which sequeunce numbers were ACKed.
-  // Once a given QuicAckNotifier has seen all the sequence numbers it is
-  // interested in, it will be deleted, and removed from this set.
-  // Owns the AckNotifiers in this set.
-  AckNotifierSet ack_notifiers_;
-
   // Maps from sequence number to the AckNotifiers which are registered
   // for that sequence number. On receipt of an ACK for a given sequence
   // number, call OnAck for all mapped AckNotifiers.
-  // Does not own the AckNotifiers.
+  // When the last reference is removed from the map, the notifier is deleted.
   AckNotifierMap ack_notifier_map_;
 
   DISALLOW_COPY_AND_ASSIGN(AckNotifierManager);

net/quic/quic_ack_notifier_test.cc

@@ -22,9 +22,9 @@ class QuicAckNotifierTest : public ::testing::Test {
     delegate_ = new MockAckNotifierDelegate;
     notifier_.reset(new QuicAckNotifier(delegate_));
 
-    notifier_->AddSequenceNumber(26, 100);
-    notifier_->AddSequenceNumber(99, 20);
-    notifier_->AddSequenceNumber(1234, 3);
+    notifier_->OnSerializedPacket();
+    notifier_->OnSerializedPacket();
+    notifier_->OnSerializedPacket();
   }
 
   MockAckNotifierDelegate* delegate_;
@@ -35,17 +35,26 @@ class QuicAckNotifierTest : public ::testing::Test {
 // Should trigger callback when we receive acks for all the registered seqnums.
 TEST_F(QuicAckNotifierTest, TriggerCallback) {
   EXPECT_CALL(*delegate_, OnAckNotification(0, 0, zero_)).Times(1);
-  EXPECT_FALSE(notifier_->OnAck(26, zero_));
-  EXPECT_FALSE(notifier_->OnAck(99, zero_));
-  EXPECT_TRUE(notifier_->OnAck(1234, zero_));
+  EXPECT_FALSE(notifier_->OnAck(zero_));
+  EXPECT_FALSE(notifier_->OnAck(zero_));
+  EXPECT_TRUE(notifier_->OnAck(zero_));
 }
 
 // Should not trigger callback if we never provide all the seqnums.
 TEST_F(QuicAckNotifierTest, DoesNotTrigger) {
   // Should not trigger callback as not all packets have been seen.
   EXPECT_CALL(*delegate_, OnAckNotification(_, _, _)).Times(0);
-  EXPECT_FALSE(notifier_->OnAck(26, zero_));
-  EXPECT_FALSE(notifier_->OnAck(99, zero_));
+  EXPECT_FALSE(notifier_->OnAck(zero_));
+  EXPECT_FALSE(notifier_->OnAck(zero_));
+}
+
+// Should not trigger callback if we abandon all three packets.
+TEST_F(QuicAckNotifierTest, AbandonDoesNotTrigger) {
+  // Should not trigger callback as not all packets have been seen.
+  EXPECT_CALL(*delegate_, OnAckNotification(_, _, _)).Times(0);
+  EXPECT_FALSE(notifier_->OnPacketAbandoned());
+  EXPECT_FALSE(notifier_->OnPacketAbandoned());
+  EXPECT_TRUE(notifier_->OnPacketAbandoned());
 }
 
 // Should trigger even after updating sequence numbers and receiving ACKs for
@@ -56,9 +65,9 @@ TEST_F(QuicAckNotifierTest, UpdateSeqNums) {
   notifier_->OnPacketRetransmitted(3);
 
   EXPECT_CALL(*delegate_, OnAckNotification(2, 20 + 3, _)).Times(1);
-  EXPECT_FALSE(notifier_->OnAck(26, zero_));    // original
-  EXPECT_FALSE(notifier_->OnAck(3000, zero_));  // updated
-  EXPECT_TRUE(notifier_->OnAck(3001, zero_));   // updated
+  EXPECT_FALSE(notifier_->OnAck(zero_));  // original
+  EXPECT_FALSE(notifier_->OnAck(zero_));  // updated
+  EXPECT_TRUE(notifier_->OnAck(zero_));   // updated
 }
 
 // Make sure the delegate is called with the delta time from the last ACK.
@@ -68,9 +77,9 @@ TEST_F(QuicAckNotifierTest, DeltaTime) {
   const QuicTime::Delta third_delta = QuicTime::Delta::FromSeconds(10);
 
   EXPECT_CALL(*delegate_, OnAckNotification(0, 0, third_delta)).Times(1);
-  EXPECT_FALSE(notifier_->OnAck(26, first_delta));
-  EXPECT_FALSE(notifier_->OnAck(99, second_delta));
-  EXPECT_TRUE(notifier_->OnAck(1234, third_delta));
+  EXPECT_FALSE(notifier_->OnAck(first_delta));
+  EXPECT_FALSE(notifier_->OnAck(second_delta));
+  EXPECT_TRUE(notifier_->OnAck(third_delta));
 }
 
 }  // namespace

net/quic/quic_config.cc

@@ -594,9 +594,8 @@ bool QuicConfig::negotiated() const {
   // TODO(ianswett): Add the negotiated parameters once and iterate over all
   // of them in negotiated, ToHandshakeMessage, ProcessClientHello, and
   // ProcessServerHello.
-  return congestion_feedback_.negotiated() &&
-      idle_connection_state_lifetime_seconds_.negotiated() &&
-      max_streams_per_connection_.negotiated();
+  return idle_connection_state_lifetime_seconds_.negotiated() &&
+         max_streams_per_connection_.negotiated();
 }
 
 void QuicConfig::SetDefaults() {
@@ -646,10 +645,6 @@ QuicErrorCode QuicConfig::ProcessPeerHello(
   DCHECK(error_details != nullptr);
 
   QuicErrorCode error = QUIC_NO_ERROR;
-  if (error == QUIC_NO_ERROR) {
-    error = congestion_feedback_.ProcessPeerHello(
-        peer_hello,  hello_type, error_details);
-  }
   if (error == QUIC_NO_ERROR) {
     error = idle_connection_state_lifetime_seconds_.ProcessPeerHello(
         peer_hello, hello_type, error_details);

net/quic/quic_connection.cc

@@ -1123,29 +1123,22 @@ void QuicConnection::SendBlocked(QuicStreamId id) {
 }
 
 const QuicConnectionStats& QuicConnection::GetStats() {
-  if (!FLAGS_quic_use_initial_rtt_for_stats) {
-    stats_.min_rtt_us =
-        sent_packet_manager_.GetRttStats()->min_rtt().ToMicroseconds();
-    stats_.srtt_us =
-        sent_packet_manager_.GetRttStats()->smoothed_rtt().ToMicroseconds();
-  } else {
-    const RttStats* rtt_stats = sent_packet_manager_.GetRttStats();
+  const RttStats* rtt_stats = sent_packet_manager_.GetRttStats();
 
-    // Update rtt and estimated bandwidth.
-    QuicTime::Delta min_rtt = rtt_stats->min_rtt();
-    if (min_rtt.IsZero()) {
-      // If min RTT has not been set, use initial RTT instead.
-      min_rtt = QuicTime::Delta::FromMicroseconds(rtt_stats->initial_rtt_us());
-    }
-    stats_.min_rtt_us = min_rtt.ToMicroseconds();
+  // Update rtt and estimated bandwidth.
+  QuicTime::Delta min_rtt = rtt_stats->min_rtt();
+  if (min_rtt.IsZero()) {
+    // If min RTT has not been set, use initial RTT instead.
+    min_rtt = QuicTime::Delta::FromMicroseconds(rtt_stats->initial_rtt_us());
+  }
+  stats_.min_rtt_us = min_rtt.ToMicroseconds();
 
-    QuicTime::Delta srtt = rtt_stats->smoothed_rtt();
-    if (srtt.IsZero()) {
-      // If SRTT has not been set, use initial RTT instead.
-      srtt = QuicTime::Delta::FromMicroseconds(rtt_stats->initial_rtt_us());
-    }
-    stats_.srtt_us = srtt.ToMicroseconds();
+  QuicTime::Delta srtt = rtt_stats->smoothed_rtt();
+  if (srtt.IsZero()) {
+    // If SRTT has not been set, use initial RTT instead.
+    srtt = QuicTime::Delta::FromMicroseconds(rtt_stats->initial_rtt_us());
   }
+  stats_.srtt_us = srtt.ToMicroseconds();
 
   stats_.estimated_bandwidth = sent_packet_manager_.BandwidthEstimate();
   stats_.max_packet_size = packet_generator_.max_packet_length();
@@ -1403,7 +1396,8 @@ bool QuicConnection::WritePacketInner(QueuedPacket* packet) {
     return true;
   }
   // Connection close packets are encrypted and saved, so don't exit early.
-  if (writer_->IsWriteBlocked() && !IsConnectionClose(*packet)) {
+  const bool is_connection_close = IsConnectionClose(*packet);
+  if (writer_->IsWriteBlocked() && !is_connection_close) {
     return false;
   }
 
@@ -1427,7 +1421,7 @@ bool QuicConnection::WritePacketInner(QueuedPacket* packet) {
   // Connection close packets are eventually owned by TimeWaitListManager.
   // Others are deleted at the end of this call.
   scoped_ptr<QuicEncryptedPacket> encrypted_deleter;
-  if (IsConnectionClose(*packet)) {
+  if (is_connection_close) {
     DCHECK(connection_close_packet_.get() == nullptr);
     connection_close_packet_.reset(encrypted);
     // This assures we won't try to write *forced* packets when blocked.
@@ -1542,6 +1536,10 @@ bool QuicConnection::WritePacketInner(QueuedPacket* packet) {
 
   if (result.status == WRITE_STATUS_ERROR) {
     OnWriteError(result.error_code);
+    DLOG(ERROR) << ENDPOINT << "failed writing " << encrypted->length()
+                << "bytes "
+                << " from host " << self_address().ToStringWithoutPort()
+                << " to address " << peer_address().ToString();
     return false;
   }
 
@@ -2104,15 +2102,14 @@ HasRetransmittableData QuicConnection::IsRetransmittable(
   }
 }
 
-bool QuicConnection::IsConnectionClose(
-    QueuedPacket packet) {
-  RetransmittableFrames* retransmittable_frames =
+bool QuicConnection::IsConnectionClose(const QueuedPacket& packet) {
+  const RetransmittableFrames* retransmittable_frames =
       packet.serialized_packet.retransmittable_frames;
-  if (!retransmittable_frames) {
+  if (retransmittable_frames == nullptr) {
     return false;
   }
-  for (size_t i = 0; i < retransmittable_frames->frames().size(); ++i) {
-    if (retransmittable_frames->frames()[i].type == CONNECTION_CLOSE_FRAME) {
+  for (const QuicFrame& frame : retransmittable_frames->frames()) {
+    if (frame.type == CONNECTION_CLOSE_FRAME) {
       return true;
     }
   }

net/quic/quic_connection.h

@@ -670,7 +670,7 @@ class NET_EXPORT_PRIVATE QuicConnection
                                 const IPEndPoint& peer_address);
 
   HasRetransmittableData IsRetransmittable(const QueuedPacket& packet);
-  bool IsConnectionClose(QueuedPacket packet);
+  bool IsConnectionClose(const QueuedPacket& packet);
 
   QuicFramer framer_;
   QuicConnectionHelperInterface* helper_;  // Not owned.

net/quic/quic_dispatcher.cc

@@ -204,6 +204,13 @@ bool QuicDispatcher::OnUnauthenticatedPublicHeader(
     const QuicPacketPublicHeader& header) {
   QuicSession* session = nullptr;
 
+  // Port zero is only allowed for unidirectional UDP, so is disallowed by QUIC.
+  // Given that we can't even send a reply rejecting the packet, just black hole
+  // it.
+  if (current_client_address_.port() == 0) {
+    return false;
+  }
+
   QuicConnectionId connection_id = header.connection_id;
   SessionMap::iterator it = session_map_.find(connection_id);
   if (it == session_map_.end()) {

net/quic/quic_flags.cc

@@ -51,10 +51,6 @@ bool FLAGS_quic_use_std_cbrt = true;
 // store multiple addresses.
 bool FLAGS_quic_use_multiple_address_in_source_tokens = false;
 
-// If true, if min RTT and/or SRTT have not yet been set then initial RTT is
-// used to initialize them in a call to QuicConnection::GetStats.
-bool FLAGS_quic_use_initial_rtt_for_stats = true;
-
 // If true, uses the last sent packet for the RTO timer instead of the earliest.
 bool FLAGS_quic_rto_uses_last_sent = true;
 

net/quic/quic_flags.h

@@ -22,7 +22,6 @@ NET_EXPORT_PRIVATE extern bool FLAGS_quic_enable_pacing;
 NET_EXPORT_PRIVATE extern bool FLAGS_quic_allow_silent_close;
 NET_EXPORT_PRIVATE extern bool FLAGS_quic_use_std_cbrt;
 NET_EXPORT_PRIVATE extern bool FLAGS_quic_use_multiple_address_in_source_tokens;
-NET_EXPORT_PRIVATE extern bool FLAGS_quic_use_initial_rtt_for_stats;
 NET_EXPORT_PRIVATE extern bool FLAGS_quic_rto_uses_last_sent;
 NET_EXPORT_PRIVATE extern bool FLAGS_quic_attach_ack_notifiers_to_packets;
 NET_EXPORT_PRIVATE extern bool FLAGS_quic_ack_notifier_informed_on_serialized;

net/quic/test_tools/quic_config_peer.cc

@@ -45,10 +45,5 @@ void QuicConfigPeer::SetReceivedBytesForConnectionId(QuicConfig* config,
   config->bytes_for_connection_id_.SetReceivedValue(bytes);
 }
 
-// static
-QuicTag QuicConfigPeer::CongestionFeedback(QuicConfig* config) {
-  return config->congestion_feedback_.GetTag();
-}
-
 }  // namespace test
 }  // namespace net

net/quic/test_tools/quic_config_peer.h

@@ -30,8 +30,6 @@ class QuicConfigPeer {
 
   static void SetReceivedBytesForConnectionId(QuicConfig* config, uint32 bytes);
 
-  static QuicTag CongestionFeedback(QuicConfig* config);
-
  private:
   DISALLOW_COPY_AND_ASSIGN(QuicConfigPeer);
 };

net/tools/quic/quic_dispatcher.cc

@@ -209,6 +209,13 @@ bool QuicDispatcher::OnUnauthenticatedPublicHeader(
     const QuicPacketPublicHeader& header) {
   QuicSession* session = nullptr;
 
+  // Port zero is only allowed for unidirectional UDP, so is disallowed by QUIC.
+  // Given that we can't even send a reply rejecting the packet, just black hole
+  // it.
+  if (current_client_address_.port() == 0) {
+    return false;
+  }
+
   QuicConnectionId connection_id = header.connection_id;
   SessionMap::iterator it = session_map_.find(connection_id);
   if (it == session_map_.end()) {

net/tools/quic/quic_dispatcher_test.cc

@@ -97,12 +97,29 @@ QuicSession* CreateSession(QuicDispatcher* dispatcher,
   return *session;
 }
 
+class MockTimeWaitListManager : public QuicTimeWaitListManager {
+ public:
+  MockTimeWaitListManager(QuicPacketWriter* writer,
+                          QuicServerSessionVisitor* visitor,
+                          EpollServer* eps)
+      : QuicTimeWaitListManager(writer, visitor, eps, QuicSupportedVersions()) {
+  }
+
+  MOCK_METHOD5(ProcessPacket,
+               void(const IPEndPoint& server_address,
+                    const IPEndPoint& client_address,
+                    QuicConnectionId connection_id,
+                    QuicPacketSequenceNumber sequence_number,
+                    const QuicEncryptedPacket& packet));
+};
+
 class QuicDispatcherTest : public ::testing::Test {
  public:
   QuicDispatcherTest()
       : crypto_config_(QuicCryptoServerConfig::TESTING,
                        QuicRandom::GetInstance()),
         dispatcher_(config_, crypto_config_, &eps_),
+        time_wait_list_manager_(nullptr),
         session1_(nullptr),
         session2_(nullptr) {
     dispatcher_.Initialize(1);
@@ -133,11 +150,20 @@ class QuicDispatcherTest : public ::testing::Test {
     EXPECT_EQ(data_, packet.AsStringPiece());
   }
 
+  void CreateTimeWaitListManager() {
+    time_wait_list_manager_ = new MockTimeWaitListManager(
+        QuicDispatcherPeer::GetWriter(&dispatcher_), &dispatcher_, &eps_);
+    // dispatcher takes the ownership of time_wait_list_manager.
+    QuicDispatcherPeer::SetTimeWaitListManager(&dispatcher_,
+                                               time_wait_list_manager_);
+  }
+
   EpollServer eps_;
   QuicConfig config_;
   QuicCryptoServerConfig crypto_config_;
   IPEndPoint server_address_;
   TestDispatcher dispatcher_;
+  MockTimeWaitListManager* time_wait_list_manager_;
   MockSession* session1_;
   MockSession* session2_;
   string data_;
@@ -184,28 +210,9 @@ TEST_F(QuicDispatcherTest, Shutdown) {
   dispatcher_.Shutdown();
 }
 
-class MockTimeWaitListManager : public QuicTimeWaitListManager {
- public:
-  MockTimeWaitListManager(QuicPacketWriter* writer,
-                          QuicServerSessionVisitor* visitor,
-                          EpollServer* eps)
-      : QuicTimeWaitListManager(writer, visitor, eps, QuicSupportedVersions()) {
-  }
-
-  MOCK_METHOD5(ProcessPacket, void(const IPEndPoint& server_address,
-                                   const IPEndPoint& client_address,
-                                   QuicConnectionId connection_id,
-                                   QuicPacketSequenceNumber sequence_number,
-                                   const QuicEncryptedPacket& packet));
-};
-
 TEST_F(QuicDispatcherTest, TimeWaitListManager) {
-  MockTimeWaitListManager* time_wait_list_manager =
-      new MockTimeWaitListManager(
-          QuicDispatcherPeer::GetWriter(&dispatcher_), &dispatcher_, &eps_);
-  // dispatcher takes the ownership of time_wait_list_manager.
-  QuicDispatcherPeer::SetTimeWaitListManager(&dispatcher_,
-                                             time_wait_list_manager);
+  CreateTimeWaitListManager();
+
   // Create a new session.
   IPEndPoint client_address(net::test::Loopback4(), 1);
   QuicConnectionId connection_id = 1;
@@ -233,34 +240,44 @@ TEST_F(QuicDispatcherTest, TimeWaitListManager) {
           reinterpret_cast<MockConnection*>(session1_->connection()),
           &MockConnection::ReallyProcessUdpPacket));
   dispatcher_.ProcessPacket(IPEndPoint(), client_address, *encrypted);
-  EXPECT_TRUE(time_wait_list_manager->IsConnectionIdInTimeWait(connection_id));
+  EXPECT_TRUE(time_wait_list_manager_->IsConnectionIdInTimeWait(connection_id));
 
   // Dispatcher forwards subsequent packets for this connection_id to the time
   // wait list manager.
-  EXPECT_CALL(*time_wait_list_manager,
+  EXPECT_CALL(*time_wait_list_manager_,
               ProcessPacket(_, _, connection_id, _, _)).Times(1);
   ProcessPacket(client_address, connection_id, true, "foo");
 }
 
 TEST_F(QuicDispatcherTest, StrayPacketToTimeWaitListManager) {
-  MockTimeWaitListManager* time_wait_list_manager =
-      new MockTimeWaitListManager(
-          QuicDispatcherPeer::GetWriter(&dispatcher_), &dispatcher_, &eps_);
-  // dispatcher takes the ownership of time_wait_list_manager.
-  QuicDispatcherPeer::SetTimeWaitListManager(&dispatcher_,
-                                             time_wait_list_manager);
+  CreateTimeWaitListManager();
 
   IPEndPoint client_address(net::test::Loopback4(), 1);
   QuicConnectionId connection_id = 1;
   // Dispatcher forwards all packets for this connection_id to the time wait
   // list manager.
   EXPECT_CALL(dispatcher_, CreateQuicSession(_, _, _)).Times(0);
-  EXPECT_CALL(*time_wait_list_manager,
+  EXPECT_CALL(*time_wait_list_manager_,
               ProcessPacket(_, _, connection_id, _, _)).Times(1);
   string data = "foo";
   ProcessPacket(client_address, connection_id, false, "foo");
 }
 
+TEST_F(QuicDispatcherTest, ProcessPacketWithBogusPort) {
+  CreateTimeWaitListManager();
+
+  IPEndPoint client_address(net::test::Loopback4(), 0);
+  IPAddressNumber any4;
+  CHECK(net::ParseIPLiteralToNumber("0.0.0.0", &any4));
+  server_address_ = IPEndPoint(any4, 5);
+
+  EXPECT_CALL(dispatcher_, CreateQuicSession(1, _, client_address)).Times(0);
+  EXPECT_CALL(*time_wait_list_manager_, ProcessPacket(_, _, _, _, _)).Times(0);
+  ProcessPacket(client_address, 1, true, "foo");
+  EXPECT_EQ(client_address, dispatcher_.current_client_address());
+  EXPECT_EQ(server_address_, dispatcher_.current_server_address());
+}
+
 class BlockingWriter : public QuicPacketWriterWrapper {
  public:
   BlockingWriter() : write_blocked_(false) {}