Populate EEP estimate in NQE

net/base/network_quality_estimator_unittest.cc

 // Copyright 2015 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 "net/base/network_quality_estimator.h"
 
 #include <stdint.h>
 
 #include <limits>
 #include <map>
@@ skipping 14 matching lines @@
 #include "net/url_request/url_request_test_util.h"
 #include "testing/gtest/include/gtest/gtest.h"
 #include "url/gurl.h"
 
 namespace {
 
 // Helps in setting the current network type and id.
 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.
   // Notifies network quality estimator of change in connection.
   void SimulateNetworkChangeTo(net::NetworkChangeNotifier::ConnectionType type,
                                std::string network_id) {
     current_network_type_ = type;
     current_network_id_ = network_id;
     OnConnectionTypeChanged(type);
   }
@@ skipping 596 matching lines @@
   EXPECT_EQ(100, rtt.InMilliseconds());
 
   int32_t downstream_throughput_kbps;
   EXPECT_FALSE(estimator.GetRecentMedianDownlinkThroughputKbps(
       now + base::TimeDelta::FromSeconds(10), &downstream_throughput_kbps));
   EXPECT_TRUE(estimator.GetRecentMedianDownlinkThroughputKbps(
       now, &downstream_throughput_kbps));
   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