From b56671e0517d4de773faeaa645af40840e863e79 Mon Sep 17 00:00:00 2001
From: deadbeef <deadbeef@webrtc.org>
Date: Fri, 26 May 2017 18:40:05 -0700
Subject: [PATCH] Fix issue with send-side bandwidth estimation over TURN TCP
 connections.

AsyncStunTCPSocket wasn't firing SignalSentPacket, which the bandwidth
estimator requires for every packet in order to look up send times when
feedback arrives. If the signal isn't fired, it always assumes feedback
is arriving extremely late, and decreases the bandwidth by a factor of
2 until it reaches the minimum of 10kbps.

BUG=webrtc:7717
TBR=pthatcher@webrtc.org

Review-Url: https://codereview.webrtc.org/2912523003
Cr-Commit-Position: refs/heads/master@{#18279}
---
 webrtc/p2p/base/asyncstuntcpsocket.cc         |  3 ++
 .../p2p/base/asyncstuntcpsocket_unittest.cc   | 29 +++++++++++++++++++
 2 files changed, 32 insertions(+)

diff --git a/webrtc/p2p/base/asyncstuntcpsocket.cc b/webrtc/p2p/base/asyncstuntcpsocket.cc
index 296a9d413a..3369bc8b48 100644
--- a/webrtc/p2p/base/asyncstuntcpsocket.cc
+++ b/webrtc/p2p/base/asyncstuntcpsocket.cc
@@ -79,6 +79,9 @@ int AsyncStunTCPSocket::Send(const void *pv, size_t cb,
     return res;
   }
 
+  rtc::SentPacket sent_packet(options.packet_id, rtc::TimeMillis());
+  SignalSentPacket(this, sent_packet);
+
   // We claim to have sent the whole thing, even if we only sent partial
   return static_cast<int>(cb);
 }
diff --git a/webrtc/p2p/base/asyncstuntcpsocket_unittest.cc b/webrtc/p2p/base/asyncstuntcpsocket_unittest.cc
index 6c026f6766..61e14117c9 100644
--- a/webrtc/p2p/base/asyncstuntcpsocket_unittest.cc
+++ b/webrtc/p2p/base/asyncstuntcpsocket_unittest.cc
@@ -86,6 +86,8 @@ class AsyncStunTCPSocketTest : public testing::Test,
         kClientAddr.family(), SOCK_STREAM);
     send_socket_.reset(AsyncStunTCPSocket::Create(
         client, kClientAddr, recv_socket_->GetLocalAddress()));
+    send_socket_->SignalSentPacket.connect(
+        this, &AsyncStunTCPSocketTest::OnSentPacket);
     ASSERT_TRUE(send_socket_.get() != NULL);
     vss_->ProcessMessagesUntilIdle();
   }
@@ -96,6 +98,11 @@ class AsyncStunTCPSocketTest : public testing::Test,
     recv_packets_.push_back(std::string(data, len));
   }
 
+  void OnSentPacket(rtc::AsyncPacketSocket* socket,
+                    const rtc::SentPacket& packet) {
+    ++sent_packets_;
+  }
+
   void OnNewConnection(rtc::AsyncPacketSocket* server,
                        rtc::AsyncPacketSocket* new_socket) {
     listen_socket_.reset(new_socket);
@@ -127,6 +134,7 @@ class AsyncStunTCPSocketTest : public testing::Test,
   std::unique_ptr<AsyncStunTCPSocket> recv_socket_;
   std::unique_ptr<rtc::AsyncPacketSocket> listen_socket_;
   std::list<std::string> recv_packets_;
+  int sent_packets_ = 0;
 };
 
 // Testing a stun packet sent/recv properly.
@@ -259,4 +267,25 @@ TEST_F(AsyncStunTCPSocketTest, DISABLED_TestWithSmallSendBuffer) {
                         sizeof(kTurnChannelDataMessageWithOddLength)));
 }
 
+// Test that SignalSentPacket is fired when a packet is sent.
+TEST_F(AsyncStunTCPSocketTest, SignalSentPacketFiredWhenPacketSent) {
+  ASSERT_TRUE(
+      Send(kStunMessageWithZeroLength, sizeof(kStunMessageWithZeroLength)));
+  EXPECT_EQ(1, sent_packets_);
+  // Send another packet for good measure.
+  ASSERT_TRUE(
+      Send(kStunMessageWithZeroLength, sizeof(kStunMessageWithZeroLength)));
+  EXPECT_EQ(2, sent_packets_);
+}
+
+// Test that SignalSentPacket isn't fired when a packet isn't sent (for
+// example, because it's invalid).
+TEST_F(AsyncStunTCPSocketTest, SignalSentPacketNotFiredWhenPacketNotSent) {
+  // Attempt to send a packet that's too small; since it isn't sent,
+  // SignalSentPacket shouldn't fire.
+  char data[1];
+  ASSERT_FALSE(Send(data, sizeof(data)));
+  EXPECT_EQ(0, sent_packets_);
+}
+
 }  // namespace cricket