Notify NQE of TCP RTT values

net/socket/tcp_socket_posix.cc

 // Copyright 2013 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/socket/tcp_socket.h"
 
 #include <errno.h>
 #include <netinet/tcp.h>
 #include <sys/socket.h>
 
 #include "base/bind.h"
 #include "base/files/file_path.h"
 #include "base/files/file_util.h"
 #include "base/logging.h"
 #include "base/metrics/histogram_macros.h"
 #include "base/posix/eintr_wrapper.h"
+#include "base/profiler/scoped_tracker.h"
 #include "base/task_runner_util.h"
 #include "base/threading/worker_pool.h"
 #include "net/base/address_list.h"
 #include "net/base/connection_type_histograms.h"
 #include "net/base/io_buffer.h"
 #include "net/base/ip_endpoint.h"
 #include "net/base/net_errors.h"
 #include "net/base/network_activity_monitor.h"
 #include "net/base/network_change_notifier.h"
 #include "net/base/sockaddr_storage.h"
@@ skipping 102 matching lines @@
 void CheckSupportAndMaybeEnableTCPFastOpen(bool user_enabled) {
 #if defined(OS_LINUX) || defined(OS_ANDROID)
   base::PostTaskAndReplyWithResult(
       base::WorkerPool::GetTaskRunner(/*task_is_slow=*/false).get(),
       FROM_HERE,
       base::Bind(SystemSupportsTCPFastOpen),
       base::Bind(RegisterTCPFastOpenIntentAndSupport, user_enabled));
 #endif
 }
 
-TCPSocketPosix::TCPSocketPosix(NetLog* net_log, const NetLog::Source& source)
-    : use_tcp_fastopen_(false),
+TCPSocketPosix::TCPSocketPosix(
+    scoped_ptr<SocketPerformanceWatcher> socket_performance_watcher,
+    NetLog* net_log,
+    const NetLog::Source& source)
+    : socket_performance_watcher_(std::move(socket_performance_watcher)),
+      use_tcp_fastopen_(false),
       tcp_fastopen_write_attempted_(false),
       tcp_fastopen_connected_(false),
       tcp_fastopen_status_(TCP_FASTOPEN_STATUS_UNKNOWN),
       logging_multiple_connect_attempts_(false),
       net_log_(BoundNetLog::Make(net_log, NetLog::SOURCE_SOCKET)) {
   net_log_.BeginEvent(NetLog::TYPE_SOCKET_ALIVE,
                       source.ToEventParametersCallback());
 }
 
 TCPSocketPosix::~TCPSocketPosix() {
@@ skipping 356 matching lines @@
                                         IPEndPoint* address) {
   DCHECK(accept_socket_);
 
   SockaddrStorage storage;
   if (accept_socket_->GetPeerAddress(&storage) != OK ||
       !address->FromSockAddr(storage.addr, storage.addr_len)) {
     accept_socket_.reset();
     return ERR_ADDRESS_INVALID;
   }
 
-  tcp_socket->reset(new TCPSocketPosix(net_log_.net_log(), net_log_.source()));
+  tcp_socket->reset(
+      new TCPSocketPosix(nullptr, net_log_.net_log(), net_log_.source()));

[Wez, 2016/03/04 18:50:26]

IIUC this means that you can't associated a socket-performance-watcher with a
server socket (one received via accept())?  If you passed the factory all the
way down to the TCPSocket ctor then you could allow that to work, since
BuildTcpSocketPosix could just create a new watcher instance.

[tbansal1, 2016/03/09 00:20:03]

SocketPerformanceWatcher only expects RTT values from connections that are
initiated by Chrome since these connections are more likely to be from Chrome to
a web server (that's what Socket Performance Watcher is interested in).

   (*tcp_socket)->socket_.reset(accept_socket_.release());
   return OK;
 }
 
 void TCPSocketPosix::ConnectCompleted(const CompletionCallback& callback,
-                                      int rv) const {
+                                      int rv) {
   DCHECK_NE(ERR_IO_PENDING, rv);
+  NotifySocketPerformanceWatcher();
   callback.Run(HandleConnectCompleted(rv));
 }
 
 int TCPSocketPosix::HandleConnectCompleted(int rv) const {
   // Log the end of this attempt (and any OS error it threw).
   if (rv != OK) {
     net_log_.EndEvent(NetLog::TYPE_TCP_CONNECT_ATTEMPT,
                       NetLog::IntCallback("os_error", errno));
   } else {
     net_log_.EndEvent(NetLog::TYPE_TCP_CONNECT_ATTEMPT);
@@ skipping 33 matching lines @@
 
   net_log_.EndEvent(NetLog::TYPE_TCP_CONNECT,
                     CreateNetLogSourceAddressCallback(storage.addr,
                                                       storage.addr_len));
 }
 
 void TCPSocketPosix::ReadCompleted(const scoped_refptr<IOBuffer>& buf,
                                    const CompletionCallback& callback,
                                    int rv) {
   DCHECK_NE(ERR_IO_PENDING, rv);
+  NotifySocketPerformanceWatcher();
   callback.Run(HandleReadCompleted(buf.get(), rv));
 }
 
 int TCPSocketPosix::HandleReadCompleted(IOBuffer* buf, int rv) {
   if (tcp_fastopen_write_attempted_ && !tcp_fastopen_connected_) {
     // A TCP FastOpen connect-with-write was attempted. This read was a
     // subsequent read, which either succeeded or failed. If the read
     // succeeded, the socket is considered connected via TCP FastOpen.
     // If the read failed, TCP FastOpen is (conservatively) turned off for all
     // subsequent connections. TCP FastOpen status is recorded in both cases.
@@ skipping 17 matching lines @@
                                 buf->data());
   NetworkActivityMonitor::GetInstance()->IncrementBytesReceived(rv);
 
   return rv;
 }
 
 void TCPSocketPosix::WriteCompleted(const scoped_refptr<IOBuffer>& buf,
                                     const CompletionCallback& callback,
                                     int rv) {
   DCHECK_NE(ERR_IO_PENDING, rv);
+  NotifySocketPerformanceWatcher();
   callback.Run(HandleWriteCompleted(buf.get(), rv));
 }
 
 int TCPSocketPosix::HandleWriteCompleted(IOBuffer* buf, int rv) {
   if (rv < 0) {
     if (tcp_fastopen_write_attempted_ && !tcp_fastopen_connected_) {
       // TCP FastOpen connect-with-write was attempted, and the write failed
       // for unknown reasons. Record status and (conservatively) turn off
       // TCP FastOpen for all subsequent connections.
       // TODO (jri): This currently results in conservative behavior, where TCP
@@ skipping 67 matching lines @@
     // turning off TCP FastOpen on more specific errors.
     tcp_fastopen_status_ = TCP_FASTOPEN_ERROR;
     g_tcp_fastopen_has_failed = true;
     return rv;
   }
 
   tcp_fastopen_status_ = TCP_FASTOPEN_SLOW_CONNECT_RETURN;
   return socket_->WaitForWrite(buf, buf_len, callback);
 }
 
+void TCPSocketPosix::NotifySocketPerformanceWatcher() {
+#if defined(TCP_INFO)
+  // TODO(tbansal): Remove ScopedTracker once crbug.com/590254 is fixed.
+  tracked_objects::ScopedTracker tracking_profile(
+      FROM_HERE_WITH_EXPLICIT_FUNCTION(
+          "590254 TCPSocketPosix::NotifySocketPerformanceWatcher"));
+
+  // Check if |socket_performance_watcher_| can be notified of a RTT value.

[Wez, 2016/03/04 18:50:26]

nit: "can" -> "must" or reword to "...watcher_| is interested in..."

[tbansal1, 2016/03/09 00:20:03]

Done.

+  if (!socket_performance_watcher_ ||
+      !socket_performance_watcher_->ShouldNotifyUpdatedRTT()) {
+    return;
+  }
+
+  tcp_info info;
+  if (!GetTcpInfo(socket_->socket_fd(), &info))
+    return;
+
+  // Only notify the SocketPerformanceWatcher if the RTT in |tcp_info| struct
+  // was populated. A value of 0 may be valid, but it is not reported to the
+  // SocketPerformanceWatcher in order to maintain high accuracy.

[Wez, 2016/03/04 18:50:26]

nit: Not clear from the comment why omitting valid data improves accuracy.

[tbansal1, 2016/03/09 00:20:03]

I expanded the comment to make it clearer.

+  if (info.tcpi_rtt > 0) {
+    socket_performance_watcher_->OnUpdatedRTTAvailable(
+        base::TimeDelta::FromMicroseconds(info.tcpi_rtt));
+  }
+#endif  // defined(TCP_INFO)
+}
+
 void TCPSocketPosix::UpdateTCPFastOpenStatusAfterRead() {
   DCHECK(tcp_fastopen_status_ == TCP_FASTOPEN_FAST_CONNECT_RETURN ||
          tcp_fastopen_status_ == TCP_FASTOPEN_SLOW_CONNECT_RETURN);
 
   if (tcp_fastopen_write_attempted_ && !tcp_fastopen_connected_) {
     // TCP FastOpen connect-with-write was attempted, and failed.
     tcp_fastopen_status_ =
         (tcp_fastopen_status_ == TCP_FASTOPEN_FAST_CONNECT_RETURN ?
             TCP_FASTOPEN_FAST_CONNECT_READ_FAILED :
             TCP_FASTOPEN_SLOW_CONNECT_READ_FAILED);
@@ skipping 41 matching lines @@
     if (info.tcpi_rtt > 0) {
       *out_rtt = base::TimeDelta::FromMicroseconds(info.tcpi_rtt);
       return true;
     }
   }
 #endif  // defined(TCP_INFO)
   return false;
 }
 
 }  // namespace net