Measure bandwidth for chromoting video channel

remoting/base/rate_counter.cc

+// Copyright (c) 2011 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 "remoting/base/rate_counter.h"
+
+namespace remoting {
+
+RateCounter::RateCounter(base::TimeDelta time_window)
+    : time_window_(time_window) {

[simonmorris, 2011/04/04 10:27:34]

Initialise sum_.

[Alpha Left Google, 2011/04/05 00:16:06]

Done.

+}
+
+RateCounter::~RateCounter() {
+}
+
+void RateCounter::Record(int64 value) {

[simonmorris, 2011/04/04 10:27:34]

It might be worth adding a Record(int64, base::Time) and a
Rate(base::Time), so this class can be unit tested.

[Alpha Left Google, 2011/04/05 00:16:06]

How is this related to unit testing? I mean this interface can be unit tested
too, is it?

+  base::Time current_time = base::Time::Now();
+  Evict(current_time);
+
+  base::AutoLock auto_lock(lock_);
+  sum_ += value;
+  data_points_.push(std::make_pair(current_time, value));
+}
+
+double RateCounter::Rate() {
+  Evict(base::Time::Now());
+
+  base::AutoLock auto_lock(lock_);
+  return 1.0 * base::Time::kMillisecondsPerSecond * sum_ /

[simonmorris, 2011/04/04 10:27:34]

Clearer to explicitly cast to a double.

[Alpha Left Google, 2011/04/05 00:16:06]

Done.

+      time_window_.InMilliseconds();

[simonmorris, 2011/04/04 10:27:34]

What if the first data point was taken more recently than
time_window_ ago? Maybe it's OK to give inaccurate rates
during the first time_window_, but if so, it should be
documented.

[Alpha Left Google, 2011/04/05 00:16:06]

Done.

+}
+
+void RateCounter::Evict(base::Time current_time) {
+  base::AutoLock auto_lock(lock_);
+
+  // Remove data points outside of the window.
+  base::Time window_start = current_time - time_window_;
+
+  while (!data_points_.empty()) {
+    if (window_start < data_points_.front().first)

[simonmorris, 2011/04/04 10:27:34]

Clearer to reverse the order, so the non-const is first,
and the const second?

[Alpha Left Google, 2011/04/05 00:16:06]

Done.

+      break;
+
+    sum_ -= data_points_.front().second;
+    data_points_.pop();
+  }
+}
+
+}  // namespace remoting