Merge to M59: 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
[email protected]
NOTRY=True
NOPRESUBMIT=True

Original Review-Url: https://codereview.webrtc.org/2834083002
Original Cr-Commit-Position: refs/heads/master@{#17817}
(cherry picked from commit 0687829)

Review-Url: https://codereview.webrtc.org/2917223002
Cr-Commit-Position: refs/branch-heads/59@{#16}
Cr-Branched-From: 10d095d-refs/heads/master@{#17657}

Author: deadbeef

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;

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);
+}