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

net/base/network_quality_estimator_unittest.cc

Index: net/base/network_quality_estimator_unittest.cc
diff --git a/net/base/network_quality_estimator_unittest.cc b/net/base/network_quality_estimator_unittest.cc
index 5ec81ab440f4055e4b83d3b283343382c4c5e34f..2c9ef8e1ce0f8e25517255c0e9765b214e97b03c 100644
--- a/net/base/network_quality_estimator_unittest.cc
+++ b/net/base/network_quality_estimator_unittest.cc
@@ -4,9 +4,9 @@
 
 #include "net/base/network_quality_estimator.h"
 
-#include "base/basictypes.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/run_loop.h"
+#include "base/strings/safe_sprintf.h"
 #include "base/test/histogram_tester.h"
 #include "base/threading/platform_thread.h"
 #include "base/time/time.h"
@@ -72,7 +72,7 @@ TEST(NetworkQualityEstimatorTest, DISABLED_TestPeakKbpsFastestRTTUpdates) {
     EXPECT_GT(network_quality.fastest_rtt_confidence, 0);
     EXPECT_GT(network_quality.peak_throughput_kbps_confidence, 0);
     EXPECT_GE(network_quality.fastest_rtt, request_duration);
-    EXPECT_GT(network_quality.peak_throughput_kbps, uint32(0));
+    EXPECT_GT(network_quality.peak_throughput_kbps, 0U);
     EXPECT_LE(
         network_quality.peak_throughput_kbps,
         min_transfer_size_in_bytes * 8.0 / request_duration.InMilliseconds());
@@ -98,4 +98,102 @@ TEST(NetworkQualityEstimatorTest, DISABLED_TestPeakKbpsFastestRTTUpdates) {
 }
 #endif  // !defined(OS_IOS)
 
+// Helps in setting the current network name.
+class TestNetworkQualityEstimator : public NetworkQualityEstimator {
+ public:
+  TestNetworkQualityEstimator() : current_network_name_(std::string()) {}
+
+  ~TestNetworkQualityEstimator() override {}
+
+  void SetCurrentNetworkName(std::string current_network_name) {
+    current_network_name_ = current_network_name;
+    NetworkQualityEstimator::SetCurrentNetworkNameForTests(
+        current_network_name_);
+  }
+
+  using NetworkQualityEstimator::GetCacheSizeForTests;
+  using NetworkQualityEstimator::OnConnectionTypeChanged;
+  using NetworkQualityEstimator::ReadCachedNetworkQualityEstimate;
+
+ private:
+  // NetworkQualityEstimator implementation that updates the current network
+  // name (instead of invoking platform APIs).
+  void UpdateCurrentNetworkName() override {
+    NetworkQualityEstimator::SetCurrentNetworkNameForTests(

[bengr, 2015/05/29 17:38:10]

It would be better, maybe, if you could use a test NCN instead of introducing a
new pathway just for tests.

[tbansal1, 2015/05/29 19:13:37]

NCN only exposes ConnectionType and not 
the SSID name or MCC/MNC code.
SSID/MCCMNC names comes from different codebases
which IIUC do not have mocking/test classes.

[bengr, 2015/06/01 20:39:34]

Uggh. Could you configure NQE to register to listen to a little class you write
that poses as an NCN?

[tbansal1, 2015/06/01 21:56:51]

Not sure....NCN has nothing to do with SSID name or MCC/MNC names.
To find out the current SSID/MCCMNC name, this code calls a 
synchronous network library API (that in turn calls platform specific code). Not
sure how posing an NCN would help.

Mocking the network library API would definitely help but
I am not sure if that design is better than what I have now.

[bengr, 2015/06/01 23:21:10]

My mistake. I guess what you have is fine.

[tbansal1, 2015/06/02 18:18:05]

Done.

+        current_network_name_);
+  }
+
+  std::string current_network_name_;
+};
+
+// Test if the network estimates are cached when network change notification
+// is invoked.
+TEST(NetworkQualityEstimatorTest, TestCaching) {
+  TestNetworkQualityEstimator estimator;
+  EXPECT_EQ(0U, estimator.GetCacheSizeForTests());
+
+  uint32_t cache_size = 0;

[bengr, 2015/05/29 17:38:10]

Chromium style guide says: Do not use unsigned types to mean "this value should
never be < 0". For that, use assertions or run-time checks (as appropriate).
Search for "unsigned" in https://www.chromium.org/developers/coding-style

[tbansal1, 2015/05/29 19:13:37]

Changed to size_t (instead of int to avoid repeated type casting).

+  estimator.OnConnectionTypeChanged(
+      NetworkChangeNotifier::ConnectionType::CONNECTION_2G);
+  EXPECT_EQ(0U, estimator.GetCacheSizeForTests());
+  for (size_t i = 1; i <= 10; ++i) {
+    // Calling the loop multiple times should have no effect on cache size as
+    // the same cache entry should get updated (because the network names do
+    // not change with multiple iterations of the loop).
+    estimator.SetCurrentNetworkName("test-1");
+    EXPECT_EQ(cache_size, estimator.GetCacheSizeForTests()) << i;
+    estimator.OnConnectionTypeChanged(
+        NetworkChangeNotifier::ConnectionType::CONNECTION_2G);
+    if (i == 1)
+      ++cache_size;
+    EXPECT_EQ(cache_size, estimator.GetCacheSizeForTests()) << i;
+
+    estimator.SetCurrentNetworkName("test-2");
+    EXPECT_EQ(cache_size, estimator.GetCacheSizeForTests()) << i;
+    estimator.OnConnectionTypeChanged(
+        NetworkChangeNotifier::ConnectionType::CONNECTION_2G);
+    if (i == 1)
+      ++cache_size;
+    EXPECT_EQ(cache_size, estimator.GetCacheSizeForTests()) << i;
+  }
+
+  // Reading quality of a networks seen before should return true.
+  estimator.SetCurrentNetworkName("test-1");
+  EXPECT_EQ(true, estimator.ReadCachedNetworkQualityEstimate());
+  estimator.SetCurrentNetworkName("test-2");
+  EXPECT_EQ(true, estimator.ReadCachedNetworkQualityEstimate());
+
+  // Reading quality of a network never seen should return false.
+  estimator.SetCurrentNetworkName("test-3");
+  EXPECT_EQ(false, estimator.ReadCachedNetworkQualityEstimate());
+}
+
+// Tests if the cache size remains bounded.
+TEST(NetworkQualityEstimatorTest, TestCacheMaximumSize) {
+  TestNetworkQualityEstimator estimator;
+  estimator.OnConnectionTypeChanged(
+      NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI);
+  EXPECT_EQ(0U, estimator.GetCacheSizeForTests());
+
+  char network_name[20];
+  for (size_t i = 1; i <= 10; ++i) {
+    base::strings::SafeSPrintf(network_name, "%d", i);
+    estimator.SetCurrentNetworkName(network_name);
+    estimator.OnConnectionTypeChanged(
+        NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI);
+  }
+  EXPECT_LT(estimator.GetCacheSizeForTests(), 10U);
+  EXPECT_GT(estimator.GetCacheSizeForTests(), 0U);
+
+  // First network should be evicted from the cache.
+  base::strings::SafeSPrintf(network_name, "%d", 1);
+  estimator.SetCurrentNetworkName(network_name);
+  EXPECT_EQ(false, estimator.ReadCachedNetworkQualityEstimate());
+
+  // Last network should not be evicted from the cache.
+  base::strings::SafeSPrintf(network_name, "%d", 10);
+  estimator.SetCurrentNetworkName(network_name);
+  EXPECT_EQ(true, estimator.ReadCachedNetworkQualityEstimate());
+}
+
 }  // namespace net