Add in-memory caching of network quality estimates across network changes.

net/base/network_quality_estimator.cc

Index: net/base/network_quality_estimator.cc
diff --git a/net/base/network_quality_estimator.cc b/net/base/network_quality_estimator.cc
index 6415ee6e6d7eb36b47ab4fd568d3537283b27729..4cbd3cbd6ad14c0659a44f5a04c106419cb5bc2a 100644
--- a/net/base/network_quality_estimator.cc
+++ b/net/base/network_quality_estimator.cc
@@ -4,15 +4,27 @@
 
 #include "net/base/network_quality_estimator.h"
 
-#include <string>
-
 #include "base/logging.h"
 #include "base/metrics/histogram.h"
+#include "build/build_config.h"
 #include "net/base/net_util.h"
 #include "net/base/network_quality.h"
 #include "net/url_request/url_request.h"
 #include "url/gurl.h"
 
+#if defined(OS_ANDROID)
+#include "net/android/network_library.h"
+#endif  // OS_ANDROID
+
+namespace {
+
+// Maximum size of the cache that holds network quality estimates.
+// Smaller size may reduce the cache hit rate due to frequent evictions.
+// Larger size may affect performance.
+const uint32_t kMaximumNetworkQualityCacheSize = 10;
+
+}  // namespace
+
 namespace net {
 
 NetworkQualityEstimator::NetworkQualityEstimator()
@@ -28,7 +40,10 @@ NetworkQualityEstimator::NetworkQualityEstimator(
       peak_kbps_since_last_connection_change_(0) {
   static_assert(kMinRequestDurationMicroseconds > 0,
                 "Minimum request duration must be > 0");
+  static_assert(kMaximumNetworkQualityCacheSize > 0,
+                "Size of the network quality cache must be > 0");
   NetworkChangeNotifier::AddConnectionTypeObserver(this);
+  UpdateCurrentNetworkName();
 }
 
 NetworkQualityEstimator::~NetworkQualityEstimator() {
@@ -111,7 +126,8 @@ void NetworkQualityEstimator::OnConnectionTypeChanged(
                             fastest_RTT_since_last_connection_change_);
         break;
       default:
-        NOTREACHED();
+        NOTREACHED() << "Unexpected connection type = "
+                     << current_connection_type_;
         break;
     }
   }
@@ -151,15 +167,32 @@ void NetworkQualityEstimator::OnConnectionTypeChanged(
                              peak_kbps_since_last_connection_change_);
         break;
       default:
-        NOTREACHED();
+        NOTREACHED() << "Unexpected connection type = "
+                     << current_connection_type_;
         break;
     }
   }
 
+  // Write the estimates of the previous network to the cache.
+  CacheNetworkQualityEstimate();
+
   last_connection_change_ = base::TimeTicks::Now();
   bytes_read_since_last_connection_change_ = false;
   peak_kbps_since_last_connection_change_ = 0;
   current_connection_type_ = type;
+  UpdateCurrentNetworkName();
+
+  // Read any cached estimates for the new network.
+  ReadCachedNetworkQualityEstimate();
+}
+
+size_t NetworkQualityEstimator::GetCacheSizeForTests() const {
+  return cached_network_quality_.size();
+}
+
+void NetworkQualityEstimator::SetCurrentNetworkNameForTests(
+    const std::string& network_name) {
+  current_network_name_ = network_name;
 }
 
 NetworkQuality NetworkQualityEstimator::GetEstimate() const {
@@ -177,4 +210,144 @@ NetworkQuality NetworkQualityEstimator::GetEstimate() const {
                         peak_kbps_since_last_connection_change_, 0.1);
 }
 
+void NetworkQualityEstimator::UpdateCurrentNetworkName() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  current_network_name_ = std::string();
+  switch (current_connection_type_) {
+    case NetworkChangeNotifier::ConnectionType::CONNECTION_UNKNOWN:
+    case NetworkChangeNotifier::ConnectionType::CONNECTION_NONE:
+    case NetworkChangeNotifier::ConnectionType::CONNECTION_BLUETOOTH:
+      return;
+    case NetworkChangeNotifier::ConnectionType::CONNECTION_ETHERNET:
+      current_network_name_ = "ethernet";
+      return;
+#if defined(OS_ANDROID)

[bengr, 2015/06/05 20:34:31]

Could we have a NetworkQualityEstimatorAndroid that overrides this method? Maybe
add a TODO.

[tbansal1, 2015/06/06 01:03:14]

Done. Added TODO

+    case NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI:

[mmenke, 2015/06/05 20:37:21]

Erm...WIFI only on Android?

[tbansal1, 2015/06/06 01:03:14]

Added Linux and ChromeOS. GetWiFiSSID() is only implemented
on these 3 platforms.

+      current_network_name_ = GetWifiSSID();
+      return;
+    case NetworkChangeNotifier::ConnectionType::CONNECTION_2G:
+    case NetworkChangeNotifier::ConnectionType::CONNECTION_3G:
+    case NetworkChangeNotifier::ConnectionType::CONNECTION_4G:
+      current_network_name_ = android::GetTelephonyNetworkOperator();
+      return;
+#endif  // OS_ANDROID

[mmenke, 2015/06/05 20:37:20]

Can't ChromeOS have cell connections?  What about iOS?  Heck, at some point, we
may be getting these types on other platforms, too.

[tbansal1, 2015/06/06 01:03:14]

AFAIK, GetTelephonyNetworkOperator() would compile only on Android.
Is there some way to obtain the MCC/MNC for non Android platforms?

Also, e.g., if the connection is cellular on iOS, this code will still
do the caching. It will just be less inaccurate because the network
name (MCC/MNC code) would be empty.

Rearranged the code to make it more obvious.

[mmenke, 2015/06/08 20:34:19]

There's not, but the old code defaulted to NOTREACHED() if we got
NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI on a platform other than
Android, which seemed to be a bug.

+    default:
+#if defined(OS_ANDROID)
+      NOTREACHED() << "Unexpected connection type = "
+                   << current_connection_type_;
+#endif  // OS_ANDROID
+      return;
+  }
+}
+
+bool NetworkQualityEstimator::ReadCachedNetworkQualityEstimate() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  for (const auto& network_quality : cached_network_quality_) {
+    if (!network_quality->MatchesNetwork(current_connection_type_,
+                                         current_network_name_)) {
+      continue;
+    }
+
+    // TOOD(tbansal): Populate these values back into the median computing
+    // algorithm.
+    // Add UMA to record how frequently match happens.
+    // int64 median_kbps = network_quality.median_kbps;
+    // int64 median_rtt_msec = network_quality.median_rtt_milliseconds;
+    // Ensure that the estimates read are non-zero before populating them into
+    // the median computing algorithm.
+    return true;
+  }
+
+  return false;
+}
+
+void NetworkQualityEstimator::CacheNetworkQualityEstimate() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  // TODO(tbansal): Following variables should be initialized using the median
+  /// values reported by NetworkQualityEstimator.
+  int median_kbps = 0;
+  base::TimeDelta median_rtt;
+
+  // If this network is already in the cache, overwrite that entry.
+  for (auto& network_quality : cached_network_quality_) {
+    if (!network_quality->MatchesNetwork(current_connection_type_,
+                                         current_network_name_)) {
+      continue;
+    }
+
+    network_quality->UpdateNetworkQuality(median_kbps, median_rtt);
+    return;
+  }
+
+  if (cached_network_quality_.size() < kMaximumNetworkQualityCacheSize) {
+    cached_network_quality_.push_back(new CachedNetworkQuality(
+        current_connection_type_, current_network_name_, median_kbps,
+        median_rtt));
+    return;
+  }
+
+  DCHECK_EQ(kMaximumNetworkQualityCacheSize, cached_network_quality_.size());
+
+  // Overwrite the oldest entry.
+  ScopedVector<CachedNetworkQuality>::iterator oldest_entry_iterator =
+      cached_network_quality_.begin();
+
+  for (ScopedVector<CachedNetworkQuality>::iterator iterator =
+           cached_network_quality_.begin();
+       iterator != cached_network_quality_.end(); ++iterator) {
+    if ((*oldest_entry_iterator)->GetLastUpdated() >
+        (*iterator)->GetLastUpdated()) {
+      oldest_entry_iterator = iterator;
+    }
+  }
+
+  cached_network_quality_.erase(oldest_entry_iterator);
+  cached_network_quality_.push_back(
+      new CachedNetworkQuality(current_connection_type_, current_network_name_,
+                               median_kbps, median_rtt));
+
+  DCHECK_EQ(kMaximumNetworkQualityCacheSize, cached_network_quality_.size());
+}
+
+NetworkQualityEstimator::CachedNetworkQuality::CachedNetworkQuality(
+    NetworkChangeNotifier::ConnectionType connection_type,
+    const std::string& network_name,
+    uint64_t median_kbps,
+    const base::TimeDelta& median_rtt)
+    : median_kbps_(median_kbps),
+      median_rtt_(median_rtt),
+      last_updated_(base::TimeTicks::Now()),
+      connection_type_(connection_type),
+      network_name_(network_name) {
+  DCHECK_GE(median_kbps_, 0U);
+  DCHECK_GE(median_rtt_, base::TimeDelta());
+}
+
+NetworkQualityEstimator::CachedNetworkQuality::~CachedNetworkQuality() {
+}
+
+void NetworkQualityEstimator::CachedNetworkQuality::UpdateNetworkQuality(
+    uint64_t median_kbps,
+    const base::TimeDelta& median_rtt) {
+  median_kbps_ = median_kbps;
+  median_rtt_ = median_rtt;
+  last_updated_ = base::TimeTicks::Now();
+  DCHECK_GE(median_kbps_, 0U);
+  DCHECK_GE(median_rtt_, base::TimeDelta());
+}
+
+bool NetworkQualityEstimator::CachedNetworkQuality::MatchesNetwork(
+    NetworkChangeNotifier::ConnectionType connection_type,
+    const std::string& network_name) const {
+  return connection_type == connection_type_ && network_name == network_name_;
+}
+
+base::TimeTicks NetworkQualityEstimator::CachedNetworkQuality::GetLastUpdated()

[bengr, 2015/06/05 20:34:31]

Can this be inlined in the .h and renamed last_update_time()? Rename the member
variable last_update_time_

[tbansal1, 2015/06/06 01:03:14]

Done.

+    const {
+  return last_updated_;
+}
+
 }  // namespace net