From 06878297942a3d632dce62df18b2fe0c32cbd853 Mon Sep 17 00:00:00 2001
From: deadbeef <deadbeef@webrtc.org>
Date: Fri, 21 Apr 2017 14:22:23 -0700
Subject: [PATCH] Fixing SignalSentPacket for TCP connections.

The signal was only being hooked up for incoming connections, not
outgoing connections.

As a result, the bandwidth estimator didn't know when packets were sent
and couldn't calculate delays.

BUG=webrtc:7509

Review-Url: https://codereview.webrtc.org/2834083002
Cr-Commit-Position: refs/heads/master@{#17817}
---
 webrtc/p2p/base/tcpport.cc          |  9 +++++
 webrtc/p2p/base/tcpport_unittest.cc | 55 +++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/webrtc/p2p/base/tcpport.cc b/webrtc/p2p/base/tcpport.cc
index 209f569f5c..79843d82de 100644
--- a/webrtc/p2p/base/tcpport.cc
+++ b/webrtc/p2p/base/tcpport.cc
@@ -157,10 +157,19 @@ Connection* TCPPort::CreateConnection(const Candidate& address,
   TCPConnection* conn = NULL;
   if (rtc::AsyncPacketSocket* socket =
       GetIncoming(address.address(), true)) {
+    // Incoming connection; we already created a socket and connected signals,
+    // so we need to hand off the "read packet" responsibility to
+    // TCPConnection.
     socket->SignalReadPacket.disconnect(this);
     conn = new TCPConnection(this, address, socket);
   } else {
+    // Outgoing connection, which will create a new socket for which we still
+    // need to connect SignalReadyToSend and SignalSentPacket.
     conn = new TCPConnection(this, address);
+    if (conn->socket()) {
+      conn->socket()->SignalReadyToSend.connect(this, &TCPPort::OnReadyToSend);
+      conn->socket()->SignalSentPacket.connect(this, &TCPPort::OnSentPacket);
+    }
   }
   AddOrReplaceConnection(conn);
   return conn;
diff --git a/webrtc/p2p/base/tcpport_unittest.cc b/webrtc/p2p/base/tcpport_unittest.cc
index e33dd433a7..5a232021bb 100644
--- a/webrtc/p2p/base/tcpport_unittest.cc
+++ b/webrtc/p2p/base/tcpport_unittest.cc
@@ -85,3 +85,58 @@ TEST_F(TCPPortTest, TestTCPPortWithLocalhostAddress) {
       lport->CreateConnection(rport->Candidates()[0], Port::ORIGIN_MESSAGE);
   EXPECT_TRUE_WAIT(conn->connected(), kTimeout);
 }
+
+class SentPacketCounter : public sigslot::has_slots<> {
+ public:
+  SentPacketCounter(TCPPort* p) {
+    p->SignalSentPacket.connect(this, &SentPacketCounter::OnSentPacket);
+  }
+
+  int sent_packets() const { return sent_packets_; }
+
+ private:
+  void OnSentPacket(const rtc::SentPacket&) { ++sent_packets_; }
+
+  int sent_packets_ = 0;
+};
+
+// Test that SignalSentPacket is fired when a packet is successfully sent, for
+// both TCP client and server sockets.
+TEST_F(TCPPortTest, SignalSentPacket) {
+  std::unique_ptr<TCPPort> client(CreateTCPPort(kLocalAddr));
+  std::unique_ptr<TCPPort> server(CreateTCPPort(kRemoteAddr));
+  client->SetIceRole(cricket::ICEROLE_CONTROLLING);
+  server->SetIceRole(cricket::ICEROLE_CONTROLLED);
+  client->PrepareAddress();
+  server->PrepareAddress();
+
+  Connection* client_conn =
+      client->CreateConnection(server->Candidates()[0], Port::ORIGIN_MESSAGE);
+  ASSERT_NE(nullptr, client_conn);
+  ASSERT_TRUE_WAIT(client_conn->connected(), kTimeout);
+
+  // Need to get the port of the actual outgoing socket, not the server socket..
+  cricket::Candidate client_candidate = client->Candidates()[0];
+  client_candidate.set_address(static_cast<cricket::TCPConnection*>(client_conn)
+                                   ->socket()
+                                   ->GetLocalAddress());
+  Connection* server_conn =
+      server->CreateConnection(client_candidate, Port::ORIGIN_THIS_PORT);
+  ASSERT_NE(nullptr, server_conn);
+  ASSERT_TRUE_WAIT(server_conn->connected(), kTimeout);
+
+  client_conn->Ping(rtc::TimeMillis());
+  server_conn->Ping(rtc::TimeMillis());
+  ASSERT_TRUE_WAIT(client_conn->writable(), kTimeout);
+  ASSERT_TRUE_WAIT(server_conn->writable(), kTimeout);
+
+  SentPacketCounter client_counter(client.get());
+  SentPacketCounter server_counter(server.get());
+  static const char kData[] = "hello";
+  for (int i = 0; i < 10; ++i) {
+    client_conn->Send(&kData, sizeof(kData), rtc::PacketOptions());
+    server_conn->Send(&kData, sizeof(kData), rtc::PacketOptions());
+  }
+  EXPECT_EQ_WAIT(10, client_counter.sent_packets(), kTimeout);
+  EXPECT_EQ_WAIT(10, server_counter.sent_packets(), kTimeout);
+}