Clean up style in QuicConnection, et. al. as per Chrome comments

Merge internal change: 37659923


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

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

4 files changed, 93 insertions, 48 deletions

diff --git a/net/quic/quic_connection.cc b/net/quic/quic_connection.cc
index 89fff10..8ab8deb 100644
--- a/net/quic/quic_connection.cc
+++ b/net/quic/quic_connection.cc
@@ -31,7 +31,7 @@ DEFINE_int32(negotiated_timeout_us, net::kDefaultTimeout,
 namespace net {
 
 // An arbitrary number we'll probably want to tune.
-const size_t kMaxUnackedPackets = 5000u;
+const QuicPacketSequenceNumber kMaxUnackedPackets = 5000u;
 
 // The amount of time we wait before resending a packet.
 const int64 kDefaultResendTimeMs = 500;
@@ -50,7 +50,7 @@ QuicConnection::QuicConnection(QuicGuid guid,
       guid_(guid),
       peer_address_(address),
       largest_seen_packet_with_ack_(0),
-      largest_seen_least_packet_awaiting_ack_(0),
+      least_packet_awaiting_ack_(0),
       write_blocked_(false),
       packet_creator_(guid_, &framer_),
       timeout_us_(kDefaultTimeout),
@@ -110,7 +110,6 @@ bool QuicConnection::OnPacketHeader(const QuicPacketHeader& header) {
     return false;
   }
 
-  last_header_ = header;
   ReceivedPacketInfo info = outgoing_ack_.received_info;
   // If this packet has already been seen, or that the sender
   // has told us will not be resent, then stop processing the packet.
@@ -118,6 +117,8 @@ bool QuicConnection::OnPacketHeader(const QuicPacketHeader& header) {
       info.missing_packets.count(header.packet_sequence_number) != 1) {
     return false;
   }
+
+  last_header_ = header;
   return true;
 }
 
@@ -148,8 +149,11 @@ void QuicConnection::OnAckFrame(const QuicAckFrame& incoming_ack) {
   UpdatePacketInformationReceivedByPeer(incoming_ack);
   UpdatePacketInformationSentByPeer(incoming_ack);
   scheduler_->OnIncomingAckFrame(incoming_ack);
+
   // Now the we have received an ack, we might be able to send queued packets.
-  if (!queued_packets_.empty()) {
+  if (queued_packets_.empty()) {
+    return;
+  }
     int delay = scheduler_->TimeUntilSend(false);
     if (delay == 0) {
       helper_->UnregisterSendAlarmIfRegistered();
@@ -160,7 +164,6 @@ void QuicConnection::OnAckFrame(const QuicAckFrame& incoming_ack) {
       helper_->SetSendAlarm(delay);
     }
   }
-}
 
 bool QuicConnection::ValidateAckFrame(const QuicAckFrame& incoming_ack) {
   if (incoming_ack.received_info.largest_received >
@@ -179,7 +182,7 @@ bool QuicConnection::ValidateAckFrame(const QuicAckFrame& incoming_ack) {
 
   if (incoming_ack.sent_info.least_unacked != 0 &&
       incoming_ack.sent_info.least_unacked <
-      largest_seen_least_packet_awaiting_ack_) {
+      least_packet_awaiting_ack_) {
     DLOG(INFO) << "Client sent low least_unacked";
     // We never process old ack frames, so this number should only increase.
     return false;
@@ -205,8 +208,7 @@ void QuicConnection::UpdatePacketInformationReceivedByPeer(
     if ((it->first < incoming_ack.received_info.largest_received &&
          !ContainsKey(incoming_ack.received_info.missing_packets, it->first)) ||
         it->first == incoming_ack.received_info.largest_received) {
-      // This was either explicitly or implicitly acked.  Remove it from our
-      // unacked packet list.
+      // Packet was acked, so remove it from our unacked packet list.
       DVLOG(1) << "Got an ack for " << it->first;
       // TODO(rch): This is inefficient and should be sped up.
       // The acked packet might be queued (if a resend had been attempted).
@@ -268,14 +270,14 @@ void QuicConnection::UpdatePacketInformationSentByPeer(
   }
 
   // Make sure we also don't expect any packets lower than the peer's
-  // last-packet-awaiting-ack
+  // last-packet-awaiting-ack.
   if (incoming_ack.sent_info.least_unacked >
-      largest_seen_least_packet_awaiting_ack_) {
-    for (QuicPacketSequenceNumber i = largest_seen_least_packet_awaiting_ack_;
+      least_packet_awaiting_ack_) {
+    for (QuicPacketSequenceNumber i = least_packet_awaiting_ack_;
          i < incoming_ack.sent_info.least_unacked; ++i) {
       outgoing_ack_.received_info.missing_packets.erase(i);
     }
-    largest_seen_least_packet_awaiting_ack_ =
+    least_packet_awaiting_ack_ =
         incoming_ack.sent_info.least_unacked;
   }
 
diff --git a/net/quic/quic_connection.h b/net/quic/quic_connection.h
index 5a13a61..ac65703 100644
--- a/net/quic/quic_connection.h
+++ b/net/quic/quic_connection.h
@@ -212,6 +212,27 @@ class NET_EXPORT_PRIVATE QuicConnection : public QuicFramerVisitorInterface {
  private:
   friend class QuicConnectionPeer;
   typedef base::hash_set<QuicPacketSequenceNumber> SequenceSet;
+  // Packets which have not been written to the wire.
+  struct QueuedPacket {
+    QueuedPacket(QuicPacketSequenceNumber sequence_number,
+                 QuicPacket* packet,
+                 bool resend,
+                 bool retransmit)
+        : sequence_number(sequence_number),
+          packet(packet),
+          resend(resend),
+          retransmit(retransmit) {
+    }
+
+    QuicPacketSequenceNumber sequence_number;
+    QuicPacket* packet;
+    bool resend;
+    bool retransmit;
+  };
+  typedef std::list<QueuedPacket> QueuedPacketList;
+  typedef base::hash_map<QuicPacketSequenceNumber,
+      QuicPacket*> UnackedPacketMap;
+  typedef std::map<QuicFecGroupNumber, QuicFecGroup*> FecGroupMap;
 
   // Sets up a packet with an QuicAckFrame and sends it out.
   void SendAck();
@@ -237,8 +258,8 @@ class NET_EXPORT_PRIVATE QuicConnection : public QuicFramerVisitorInterface {
   IPEndPoint self_address_;
   IPEndPoint peer_address_;
 
-  bool last_packet_revived_;  // true if the last packet was revived from FEC.
-  size_t last_size_;  // size of the last received packet.
+  bool last_packet_revived_;  // True if the last packet was revived from FEC.
+  size_t last_size_;  // Size of the last received packet.
   QuicPacketHeader last_header_;
   std::vector<QuicStreamFrame> frames_;
 
@@ -246,35 +267,13 @@ class NET_EXPORT_PRIVATE QuicConnection : public QuicFramerVisitorInterface {
 
   // Track some client state so we can do less bookkeeping
   //
-  // The largest packet we've seen which contained an ack frame.
   QuicPacketSequenceNumber largest_seen_packet_with_ack_;
-  // The largest seen value for least_packet_awaiting_ack from the client.
-  QuicPacketSequenceNumber largest_seen_least_packet_awaiting_ack_;
+  QuicPacketSequenceNumber least_packet_awaiting_ack_;
 
-  typedef base::hash_map<QuicPacketSequenceNumber,
-                         QuicPacket*> UnackedPacketMap;
   // When new packets are created which may be resent, they are added
   // to this map, which contains owning pointers.
   UnackedPacketMap unacked_packets_;
 
-  // Packets which have not been written to the wire.
-  struct QueuedPacket {
-    QuicPacketSequenceNumber sequence_number;
-    QuicPacket* packet;
-    bool resend;
-    bool retransmit;
-
-    QueuedPacket(QuicPacketSequenceNumber sequence_number,
-                 QuicPacket* packet,
-                 bool resend,
-                 bool retransmit) {
-      this->sequence_number = sequence_number;
-      this->packet = packet;
-      this->resend = resend;
-      this->retransmit = retransmit;
-    }
-  };
-  typedef std::list<QueuedPacket> QueuedPacketList;
   // When packets could not be sent because the socket was not writable,
   // they are added to this list.  For packets that are not resendable, this
   // list contains owning pointers, since they are not added to
@@ -284,7 +283,6 @@ class NET_EXPORT_PRIVATE QuicConnection : public QuicFramerVisitorInterface {
   // True when the socket becomes unwritable.
   bool write_blocked_;
 
-  typedef std::map<QuicFecGroupNumber, QuicFecGroup*> FecGroupMap;
   FecGroupMap group_map_;
   QuicPacketHeader revived_header_;
   scoped_array<char> revived_payload_;
@@ -309,6 +307,8 @@ class NET_EXPORT_PRIVATE QuicConnection : public QuicFramerVisitorInterface {
   // True by default.  False if we've received or sent an explicit connection
   // close.
   bool connected_;
+
+  DISALLOW_COPY_AND_ASSIGN(QuicConnection);
 };
 
 }  // namespace net
diff --git a/net/quic/quic_connection_test.cc b/net/quic/quic_connection_test.cc
index cd1591e..3f2d7f7 100644
--- a/net/quic/quic_connection_test.cc
+++ b/net/quic/quic_connection_test.cc
@@ -39,6 +39,7 @@ class QuicConnectionPeer {
                            QuicSendScheduler* scheduler) {
     connection->scheduler_.reset(scheduler);
   }
+  DISALLOW_COPY_AND_ASSIGN(QuicConnectionPeer);
 };
 
 namespace test {
@@ -68,6 +69,8 @@ class TestCollector : public QuicReceiptMetricsCollector {
  private:
   MockClock clock_;
   CongestionInfo* info_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestCollector);
 };
 
 class TestConnectionHelper : public QuicConnectionHelperInterface {
@@ -145,6 +148,8 @@ class TestConnectionHelper : public QuicConnectionHelperInterface {
   QuicPacketHeader header_;
   QuicAckFrame frame_;
   bool blocked_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestConnectionHelper);
 };
 
 class TestConnection : public QuicConnection {
@@ -175,6 +180,9 @@ class TestConnection : public QuicConnection {
     return QuicConnection::SendPacket(
         sequence_number, packet, should_resend, force, is_retransmit);
   }
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(TestConnection);
 };
 
 class QuicConnectionTest : public ::testing::Test {
@@ -227,7 +235,7 @@ class QuicConnectionTest : public ::testing::Test {
     connection_.ProcessUdpPacket(IPEndPoint(), IPEndPoint(), *encrypted);
   }
 
-  // Sends an FEC packet that convers the packets that would have been sent.
+  // Sends an FEC packet that covers the packets that would have been sent.
   void ProcessFecPacket(QuicPacketSequenceNumber number,
                         QuicPacketSequenceNumber min_protected_packet,
                         bool expect_revival) {
@@ -288,8 +296,7 @@ class QuicConnectionTest : public ::testing::Test {
   }
 
   bool NonRetransmitting(QuicPacketSequenceNumber number) {
-    return last_frame()->sent_info.non_retransmiting.find(
-        number) !=
+    return last_frame()->sent_info.non_retransmiting.find(number) !=
         last_frame()->sent_info.non_retransmiting.end();
   }
 
@@ -323,6 +330,9 @@ class QuicConnectionTest : public ::testing::Test {
   QuicStreamFrame frame1_;
   QuicStreamFrame frame2_;
   bool accept_packet_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(QuicConnectionTest);
 };
 
 TEST_F(QuicConnectionTest, PacketsInOrder) {
@@ -388,7 +398,10 @@ TEST_F(QuicConnectionTest, LatePacketMarkedWillNotResend) {
   ProcessPacket(5);
   // Now send non-resending information, that we're not going to resend 3.
   // The far end should stop waiting for it.
-  QuicAckFrame frame(0, 0, 1);
+  QuicPacketSequenceNumber largest_received = 0;
+  QuicTransmissionTime time_received = 0;
+  QuicPacketSequenceNumber least_unacked = 1;
+  QuicAckFrame frame(largest_received, time_received, least_unacked);
   frame.sent_info.non_retransmiting.insert(3);
   SendAckPacket(&frame);
   // Force an ack to be sent.
@@ -503,7 +516,7 @@ TEST_F(QuicConnectionTest, LeastUnackedLower) {
 }
 
 TEST_F(QuicConnectionTest, AckUnsentData) {
-  // Ack a packet which has not been sent
+  // Ack a packet which has not been sent.
   EXPECT_CALL(visitor_, ConnectionClose(QUIC_INVALID_ACK_DATA, false));
   QuicAckFrame frame(1, 0, 0);
   SendAckPacket(&frame);
@@ -651,32 +664,32 @@ TEST_F(QuicConnectionTest, MultipleAcks) {
 }
 
 TEST_F(QuicConnectionTest, ReviveMissingPacketAfterFecPacket) {
-  // Don't send missing packet 1
+  // Don't send missing packet 1.
   ProcessFecPacket(2, 1, true);
 }
 
 TEST_F(QuicConnectionTest, ReviveMissingPacketAfterDataPacketThenFecPacket) {
   ProcessFecProtectedPacket(1, false);
-  // Don't send missing packet 2
+  // Don't send missing packet 2.
   ProcessFecPacket(3, 1, true);
 }
 
 TEST_F(QuicConnectionTest, ReviveMissingPacketAfterDataPacketsThenFecPacket) {
   ProcessFecProtectedPacket(1, false);
-  // Don't send missing packet 2
+  // Don't send missing packet 2.
   ProcessFecProtectedPacket(3, false);
   ProcessFecPacket(4, 1, true);
 }
 
 TEST_F(QuicConnectionTest, ReviveMissingPacketAfterDataPacket) {
-  // Don't send missing packet 1
+  // Don't send missing packet 1.
   ProcessFecPacket(3, 1, false);  // out of order
   ProcessFecProtectedPacket(2, true);
 }
 
 TEST_F(QuicConnectionTest, ReviveMissingPacketAfterDataPackets) {
   ProcessFecProtectedPacket(1, false);
-  // Don't send missing packet 2
+  // Don't send missing packet 2.
   ProcessFecPacket(6, 1, false);
   ProcessFecProtectedPacket(3, false);
   ProcessFecProtectedPacket(4, false);
diff --git a/net/quic/test_tools/quic_test_utils.h b/net/quic/test_tools/quic_test_utils.h
index ead8db8..b7dcb6e 100644
--- a/net/quic/test_tools/quic_test_utils.h
+++ b/net/quic/test_tools/quic_test_utils.h
@@ -49,10 +49,15 @@ class MockFramerVisitor : public QuicFramerVisitorInterface {
   MOCK_METHOD1(OnConnectionCloseFrame,
                void(const QuicConnectionCloseFrame& frame));
   MOCK_METHOD0(OnPacketComplete, void());
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockFramerVisitor);
 };
 
 class NoOpFramerVisitor : public QuicFramerVisitorInterface {
  public:
+  NoOpFramerVisitor() {}
+
   virtual void OnError(QuicFramer* framer) OVERRIDE {}
   virtual void OnPacket(const IPEndPoint& self_address,
                         const IPEndPoint& peer_address) OVERRIDE {}
@@ -66,25 +71,34 @@ class NoOpFramerVisitor : public QuicFramerVisitorInterface {
   virtual void OnConnectionCloseFrame(
       const QuicConnectionCloseFrame& frame) OVERRIDE {}
   virtual void OnPacketComplete() OVERRIDE {}
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(NoOpFramerVisitor);
 };
 
 class FramerVisitorCapturingAcks : public NoOpFramerVisitor {
  public:
+  FramerVisitorCapturingAcks() {}
+
   // NoOpFramerVisitor
   virtual bool OnPacketHeader(const QuicPacketHeader& header) OVERRIDE;
   virtual void OnAckFrame(const QuicAckFrame& frame) OVERRIDE;
 
   QuicPacketHeader* header() { return &header_; }
   QuicAckFrame* frame() { return &frame_; }
+
  private:
   QuicPacketHeader header_;
   QuicAckFrame frame_;
+
+  DISALLOW_COPY_AND_ASSIGN(FramerVisitorCapturingAcks);
 };
 
 class MockConnectionVisitor : public QuicConnectionVisitorInterface {
  public:
   MockConnectionVisitor();
   virtual ~MockConnectionVisitor();
+
   MOCK_METHOD4(OnPacket, bool(const IPEndPoint& self_address,
                               const IPEndPoint& peer_address,
                               const QuicPacketHeader& header,
@@ -92,6 +106,9 @@ class MockConnectionVisitor : public QuicConnectionVisitorInterface {
   MOCK_METHOD1(OnRstStream, void(const QuicRstStreamFrame& frame));
   MOCK_METHOD2(ConnectionClose, void(QuicErrorCode error, bool from_peer));
   MOCK_METHOD1(OnAck, void(AckedPackets acked_packets));
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockConnectionVisitor);
 };
 
 class MockScheduler : public QuicSendScheduler {
@@ -102,8 +119,12 @@ class MockScheduler : public QuicSendScheduler {
   MOCK_METHOD1(TimeUntilSend, int(bool));
   MOCK_METHOD1(OnIncomingAckFrame, void(const QuicAckFrame&));
   MOCK_METHOD3(SentPacket, void(QuicPacketSequenceNumber, size_t, bool));
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockScheduler);
 };
 
+
 class MockHelper : public QuicConnectionHelperInterface {
  public:
   MockHelper();
@@ -140,6 +161,9 @@ class MockConnection : public QuicConnection {
                                    QuicStreamOffset offset));
 
   MOCK_METHOD0(OnCanWrite, bool());
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockConnection);
 };
 
 class PacketSavingConnection : public MockConnection {
@@ -154,6 +178,9 @@ class PacketSavingConnection : public MockConnection {
                           bool is_retransmit) OVERRIDE;
 
   std::vector<QuicPacket*> packets_;
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(PacketSavingConnection);
 };
 
 class MockSession : public QuicSession {
@@ -175,6 +202,9 @@ class MockSession : public QuicSession {
   MOCK_METHOD4(WriteData, int(QuicStreamId id, base::StringPiece data,
                               QuicStreamOffset offset, bool fin));
   MOCK_METHOD0(IsHandshakeComplete, bool());
+
+ private:
+  DISALLOW_COPY_AND_ASSIGN(MockSession);
 };
 
 }  // namespace test