Notify NQE of TCP RTT values

net/socket/tcp_client_socket_unittest.cc

Index: net/socket/tcp_client_socket_unittest.cc
diff --git a/net/socket/tcp_client_socket_unittest.cc b/net/socket/tcp_client_socket_unittest.cc
index 1c39719f59d421bf821fc05ee29e6adf8903bcde..c477fc7cb243152bd8153f27759b742268dc857f 100644
--- a/net/socket/tcp_client_socket_unittest.cc
+++ b/net/socket/tcp_client_socket_unittest.cc
@@ -8,13 +8,20 @@
 
 #include "net/socket/tcp_client_socket.h"
 
+#include <stddef.h>
+
 #include "net/base/ip_address.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/net_errors.h"
+#include "net/base/socket_performance_watcher.h"
 #include "net/base/test_completion_callback.h"
 #include "net/socket/tcp_server_socket.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
+namespace base {
+class TimeDelta;
+}
+
 namespace net {
 
 namespace {
@@ -29,7 +36,8 @@ TEST(TCPClientSocketTest, BindLoopbackToLoopback) {
   IPEndPoint server_address;
   ASSERT_EQ(OK, server.GetLocalAddress(&server_address));
 
-  TCPClientSocket socket(AddressList(server_address), NULL, NetLog::Source());
+  TCPClientSocket socket(AddressList(server_address), NULL, NULL,
+                         NetLog::Source());
 
   EXPECT_EQ(OK, socket.Bind(IPEndPoint(lo_address, 0)));
 
@@ -61,7 +69,7 @@ TEST(TCPClientSocketTest, BindLoopbackToLoopback) {
 TEST(TCPClientSocketTest, BindLoopbackToExternal) {
   IPAddress external_ip(72, 14, 213, 105);
   TCPClientSocket socket(AddressList::CreateFromIPAddress(external_ip, 80),
-                         NULL, NetLog::Source());
+                         NULL, NULL, NetLog::Source());
 
   EXPECT_EQ(OK, socket.Bind(IPEndPoint(IPAddress::IPv4Localhost(), 0)));
 
@@ -89,7 +97,8 @@ TEST(TCPClientSocketTest, BindLoopbackToIPv6) {
 
   IPEndPoint server_address;
   ASSERT_EQ(OK, server.GetLocalAddress(&server_address));
-  TCPClientSocket socket(AddressList(server_address), NULL, NetLog::Source());
+  TCPClientSocket socket(AddressList(server_address), NULL, NULL,
+                         NetLog::Source());
 
   EXPECT_EQ(OK, socket.Bind(IPEndPoint(IPAddress::IPv4Localhost(), 0)));
 
@@ -101,6 +110,60 @@ TEST(TCPClientSocketTest, BindLoopbackToIPv6) {
   EXPECT_NE(OK, result);
 }
 
+class TestSocketPerformanceWatcher : public SocketPerformanceWatcher {
+ public:
+  TestSocketPerformanceWatcher() : connection_changed_count_(0u) {}
+  ~TestSocketPerformanceWatcher() override {}
+
+  bool ShouldNotifyUpdatedRTT() const override { return true; }
+
+  void OnUpdatedRTTAvailable(const base::TimeDelta& rtt) override {}
+
+  void OnConnectionChanged() override { connection_changed_count_++; }
+
+  size_t connection_changed_count() const { return connection_changed_count_; }
+
+ private:
+  size_t connection_changed_count_;
+
+  DISALLOW_COPY_AND_ASSIGN(TestSocketPerformanceWatcher);
+};
+
+// TestSocketPerformanceWatcher requires kernel support for tcp_info struct, and
+// so it is enabled only on certain platforms.
+#if defined(TCP_INFO) || defined(OS_LINUX)
+#define MAYBE_TestSocketPerformanceWatcher TestSocketPerformanceWatcher
+#else
+#define MAYBE_TestSocketPerformanceWatcher TestSocketPerformanceWatcher
+#endif
+// Tests if the socket performance watcher is notified if the same socket is
+// used for a different connection.
+TEST(TCPClientSocketTest, MAYBE_TestSocketPerformanceWatcher) {
+  const size_t kNumIPs = 2;

[Ryan Sleevi, 2016/04/11 22:06:56]

Seems like this doesn't need to be a constant, given the test. Why would you
change this (to 3, 17, or 63?)

[tbansal1, 2016/04/12 16:14:33]

I kept it as a constant so that the relation between # of IPs pushed (Line 145)
and the number of connection notifications (Line 164) is clearer.

+  IPAddressList ip_list;
+  for (size_t i = 0; i < kNumIPs; ++i)
+    ip_list.push_back(IPAddress(72, 14, 213, 105));

[Ryan Sleevi, 2016/04/11 22:06:56]

Seems like you should make these different IPs to be more reflective of a 'real'
test case.

[tbansal1, 2016/04/12 16:14:33]

Done.

+
+  scoped_ptr<TestSocketPerformanceWatcher> watcher(
+      new TestSocketPerformanceWatcher());
+  TestSocketPerformanceWatcher* watcher_ptr = watcher.get();
+
+  TCPClientSocket socket(
+      AddressList::CreateFromIPAddressList(ip_list, "example.com"),
+      std::move(watcher), NULL, NetLog::Source());
+
+  EXPECT_EQ(OK, socket.Bind(IPEndPoint(IPAddress::IPv4Localhost(), 0)));
+
+  TestCompletionCallback connect_callback;
+  int result = socket.Connect(connect_callback.callback());
+  if (result == ERR_IO_PENDING)
+    result = connect_callback.WaitForResult();

[Ryan Sleevi, 2016/04/11 22:06:56]

This three-line pattern is a bit of an anti-pattern we've been trying to
correct.

int result =
connect_callback.GetResult(socket.Connect(connect_callback.callback()));

[tbansal1, 2016/04/12 16:14:33]

Done.

+
+  ASSERT_NE(OK, result);
+
+  EXPECT_EQ(kNumIPs - 1, watcher_ptr->connection_changed_count());
+}
+
 }  // namespace
 
 }  // namespace net