Add a new NetworkActivityMonitor to track network activity

net/base/network_activity_monitor.h

+// Copyright (c) 2014 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.
+
+#ifndef NET_BASE_NETWORK_ACTIVITY_MONITOR_H_
+#define NET_BASE_NETWORK_ACTIVITY_MONITOR_H_
+
+#include "base/basictypes.h"
+#include "base/synchronization/lock.h"
+#include "net/base/net_export.h"
+
+namespace net {
+
+namespace test {
+
+class NetworkActivityMonitorPeer;
+
+}  // namespace test
+
+// NetworkActivityMonitor tracks network activity across all sockets and
+// provides cumulative statistics about bytes written to and read from
+// the network. It uses locks to ensure thread-safety.

[eroman, 2014/11/13 22:18:39]

I would also emphasize the caveats of what this is measuring:

  * This does not include cache activity
  * Bytes "written" is attempted bytes written (no guarantee they were actually
sent over the network)
  * Whereas bytes read is the actual bytes received from the network.
  * Network activity not initiated directly using client sockets won't be
reflected here (for instance DNS queries issued by getaddrinfo()).

[Ryan Hamilton, 2014/11/13 23:38:17]

Done.

+class NET_EXPORT_PRIVATE NetworkActivityMonitor {
+ public:
+  // Returns the singleton instance of the monitor.
+  static NetworkActivityMonitor* GetInstance();
+
+  void AddBytesRead(uint64 bytes_read);

[eroman, 2014/11/13 22:18:39]

[optional] Consider "Increment" instead of Add

[Ryan Hamilton, 2014/11/13 23:38:17]

Done.

+  void AddBytesWritten(uint64 bytes_written);
+
+  uint64 GetBytesRead();
+  uint64 GetBytesWritten();
+
+ private:
+  friend class test::NetworkActivityMonitorPeer;

[eroman, 2014/11/13 22:18:39]

What about using a friend_test macro? Or at a minimum giving it a name that
makes it obvious it is a test class.

[Ryan Hamilton, 2014/11/13 23:38:18]

Doesn't the test:: prefix do that? Personally, I'm not a fan of the FRIEND_TEST
macros because each time you add a new test you end up having to change the
"real" .h file.  What do you think?

[eroman, 2014/11/14 00:06:15]

Ah, I hadn't noticed the test:: prefix!

+
+  NetworkActivityMonitor();

[eroman, 2014/11/13 22:18:39]

Another interesting API for future consideration would be:
  GetTicksSinceLastRead();
  GetTicksSinceLastWrite()

(Where adding bytes read/written would also update a timestamp).

[Ryan Hamilton, 2014/11/13 23:38:17]

Oo! Interesting. That's a good idea. Done. (Though s/Ticks/Time/ since this
should probably return a TimeDelta).

+  ~NetworkActivityMonitor();
+
+  // Protects bytes_read_.
+  mutable base::Lock bytes_read_lock_;

[eroman, 2014/11/13 22:18:39]

Why is this mutable? Presumably you meant to make GetBytesRead() /
GetBytesWritten() const?

[Ryan Hamilton, 2014/11/13 23:38:18]

Whoops, yes. Done.

+  uint64 bytes_read_;
+
+  // Protects bytes_written_.
+  mutable base::Lock bytes_written_lock_;

[eroman, 2014/11/13 22:18:39]

A single lock to protect all members should suffice, remove this one.

[Ryan Hamilton, 2014/11/13 23:38:17]

Done.

+  uint64 bytes_written_;
+
+  DISALLOW_COPY_AND_ASSIGN(NetworkActivityMonitor);
+};
+
+}  // namespace net
+
+#endif  // NET_BASE_NETWORK_ACTIVITY_MONITOR_H_