Populate EEP estimate in NQE

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 b1ffdebaeb58e378ad6806ba6c2028f26ae2a33a..c07d0ca2aac2bb2bb1b6a91cb91b4d6f403fefe3 100644
--- a/net/base/network_quality_estimator_unittest.cc
+++ b/net/base/network_quality_estimator_unittest.cc
@@ -32,12 +32,19 @@ namespace {
 class TestNetworkQualityEstimator : public net::NetworkQualityEstimator {
  public:
   TestNetworkQualityEstimator(
-      const std::map<std::string, std::string>& variation_params)
-      : NetworkQualityEstimator(scoped_ptr<net::ExternalEstimateProvider>(),
+      const std::map<std::string, std::string>& variation_params,
+      scoped_ptr<net::ExternalEstimateProvider> external_estimate_provider)
+      : NetworkQualityEstimator(external_estimate_provider.Pass(),
                                 variation_params,
                                 true,
                                 true) {}
 
+  explicit TestNetworkQualityEstimator(
+      const std::map<std::string, std::string>& variation_params)
+      : TestNetworkQualityEstimator(
+            variation_params,
+            scoped_ptr<net::ExternalEstimateProvider>()) {}
+
   ~TestNetworkQualityEstimator() override {}
 
   // Overrides the current network type and id.
@@ -654,4 +661,118 @@ TEST(NetworkQualityEstimatorTest, TestGetMedianRTTSince) {
   EXPECT_EQ(100, downstream_throughput_kbps);
 }
 
+class TestExternalEstimateProvider : public ExternalEstimateProvider {
+ public:
+  TestExternalEstimateProvider()
+      : time_since_last_update_(base::TimeDelta::FromSeconds(1)),
+        time_since_last_update_count_(0),
+        rtt_calls_count_(0),
+        downstream_throughput_kbps_count_(0),
+        request_update_count_(0) {}
+  ~TestExternalEstimateProvider() override {}
+

[mmenke, 2015/09/14 20:07:06]

nit:  Add something along the lines of:

// ExternalEstimateProvider implementation:

[tbansal1, 2015/09/14 23:46:56]

Done.

+  bool GetRTT(base::TimeDelta* rtt) const override {
+    *rtt = base::TimeDelta::FromMilliseconds(1);
+    rtt_calls_count_++;
+    return true;
+  }
+
+  bool GetDownstreamThroughputKbps(
+      int32_t* downstream_throughput_kbps) const override {
+    *downstream_throughput_kbps = 100;
+    downstream_throughput_kbps_count_++;
+    return false;
+  }
+
+  bool GetUpstreamThroughputKbps(
+      int32_t* upstream_throughput_kbps) const override {
+    // NetworkQualityEstimator does not support upstream throughput.
+    NOTIMPLEMENTED();

[mmenke, 2015/09/14 20:07:06]

This should probably be ADD_FAILURE

[tbansal1, 2015/09/14 23:46:57]

Done.

+    return false;
+  }
+
+  bool GetTimeSinceLastUpdate(
+      base::TimeDelta* time_since_last_update) const override {
+    *time_since_last_update = time_since_last_update_;
+    time_since_last_update_count_++;
+    return true;
+  }
+
+  void SetTimeSinceLastUpdate(const base::TimeDelta& time_since_last_update) {

[mmenke, 2015/09/14 20:07:06]

nit:  This should not go in the middle of the override methods.

[mmenke, 2015/09/14 20:07:06]

nit:  set_time_since_last_update(base::TimeDelta time_since_last_update) {}

(Rename it as a simple setter, don't pass TimeDeltas by reference, since they're
so small).

[tbansal1, 2015/09/14 23:46:56]

Done.

[tbansal1, 2015/09/14 23:46:57]

Done.

+    time_since_last_update_ = time_since_last_update;
+  }
+
+  void SetUpdatedEstimateDelegate(UpdatedEstimateDelegate* delegate) override {}
+
+  void RequestUpdate() const override { request_update_count_++; }
+
+  size_t time_since_last_update_count() const {
+    return time_since_last_update_count_;
+  }
+  size_t rtt_calls_count() const { return rtt_calls_count_; }
+  size_t downstream_throughput_kbps_count() const {
+    return downstream_throughput_kbps_count_;
+  }
+  size_t request_update_count() const { return request_update_count_; }
+
+ private:
+  base::TimeDelta time_since_last_update_;
+
+  mutable size_t time_since_last_update_count_;
+  mutable size_t rtt_calls_count_;

[mmenke, 2015/09/14 20:07:06]

nit:  get_rtt_count_, to match request_update_count_?

[tbansal1, 2015/09/14 23:46:57]

Did you mean rtt_count_?

[mmenke, 2015/09/15 15:19:53]

No, I meant get_rtt_count_.  Name of the method it's counting calls to (GetRTT),
and then count.  I'm fine with including the "calls" in the name, though if you
do that, should include it in both the variables to count method calls.

[tbansal1, 2015/09/15 16:48:30]

That makes sense. Changed the metod names to have a suffix of count.

+  mutable size_t downstream_throughput_kbps_count_;
+  mutable size_t request_update_count_;
+};
+
+// Tests if the external estimate provider is called in the constructor and
+// on network change notification.
+TEST(NetworkQualityEstimatorTest, TestExternalEstimateProvider) {
+  TestExternalEstimateProvider* test_external_estimate_provider =
+      new TestExternalEstimateProvider();
+  scoped_ptr<ExternalEstimateProvider> external_estimate_provider(
+      test_external_estimate_provider);
+  std::map<std::string, std::string> variation_params;
+  TestNetworkQualityEstimator estimator(variation_params,
+                                        external_estimate_provider.Pass());
+  EXPECT_EQ(1U,
+            test_external_estimate_provider->time_since_last_update_count());
+  EXPECT_EQ(1U, test_external_estimate_provider->rtt_calls_count());
+  EXPECT_EQ(
+      1U, test_external_estimate_provider->downstream_throughput_kbps_count());
+
+  // Change network type to WiFi. Number of queries to External estimate
+  // provider must increment.
+  estimator.SimulateNetworkChangeTo(
+      NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI, "test-1");
+  EXPECT_EQ(2U,
+            test_external_estimate_provider->time_since_last_update_count());
+  EXPECT_EQ(2U, test_external_estimate_provider->rtt_calls_count());
+  EXPECT_EQ(
+      2U, test_external_estimate_provider->downstream_throughput_kbps_count());
+
+  // Change network type to 2G. Number of queries to External estimate provider
+  // must increment.
+  estimator.SimulateNetworkChangeTo(
+      NetworkChangeNotifier::ConnectionType::CONNECTION_2G, "test-1");
+  EXPECT_EQ(3U,
+            test_external_estimate_provider->time_since_last_update_count());
+  EXPECT_EQ(3U, test_external_estimate_provider->rtt_calls_count());
+  EXPECT_EQ(
+      3U, test_external_estimate_provider->downstream_throughput_kbps_count());
+
+  // Set the external estimate as old. Network Quality estimator should request
+  // an update on connection type change.
+  EXPECT_EQ(0U, test_external_estimate_provider->request_update_count());
+  test_external_estimate_provider->SetTimeSinceLastUpdate(
+      base::TimeDelta::Max());
+  estimator.SimulateNetworkChangeTo(
+      NetworkChangeNotifier::ConnectionType::CONNECTION_2G, "test-1");
+  EXPECT_EQ(4U,
+            test_external_estimate_provider->time_since_last_update_count());
+  EXPECT_EQ(3U, test_external_estimate_provider->rtt_calls_count());
+  EXPECT_EQ(
+      3U, test_external_estimate_provider->downstream_throughput_kbps_count());
+  EXPECT_EQ(1U, test_external_estimate_provider->request_update_count());
+}
+
 }  // namespace net