Add mechanism to add RTT/throughput estimates observer to NQT

Add mechanism to add HTTP RTT, transport RTT, downlink throughout
estimate change observer to NetworkQualityTracker.

In the next CL, an IO object owned by UINetworkQualityEstimatorService
would subscribe to these changes, and then pass them on to the UI
thread.

Bug: 819244
Cq-Include-Trybots: luci.chromium.try:linux_mojo
Change-Id: Ieeb786c9376af7afe9f2e8bff85468067c4c4ecf
Reviewed-on: https://chromium-review.googlesource.com/1144470Reviewed-by: Nick Carter <nick@chromium.org>
Reviewed-by: Matt Menke <mmenke@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#580367}

content/public/browser/network_connection_tracker_unittest.cc

@@ -24,7 +24,8 @@ class TestNetworkConnectionObserver
   explicit TestNetworkConnectionObserver(NetworkConnectionTracker* tracker)
       : num_notifications_(0),
         tracker_(tracker),
-        run_loop_(std::make_unique<base::RunLoop>()),
+        expected_connection_type_(
+            network::mojom::ConnectionType::CONNECTION_UNKNOWN),
         connection_type_(network::mojom::ConnectionType::CONNECTION_UNKNOWN) {
     tracker_->AddNetworkConnectionObserver(this);
   }
@@ -52,13 +53,22 @@ class TestNetworkConnectionObserver
 
     num_notifications_++;
     connection_type_ = type;
-    run_loop_->Quit();
+    if (run_loop_ && expected_connection_type_ == type)
+      run_loop_->Quit();
   }
 
   size_t num_notifications() const { return num_notifications_; }
-  void WaitForNotification() {
+  void WaitForNotification(
+      network::mojom::ConnectionType expected_connection_type) {
+    expected_connection_type_ = expected_connection_type;
+
+    if (connection_type_ == expected_connection_type)
+      return;
+    // WaitForNotification() should not be called twice.
+    EXPECT_EQ(nullptr, run_loop_);
+    run_loop_ = std::make_unique<base::RunLoop>();
     run_loop_->Run();
-    run_loop_.reset(new base::RunLoop());
+    run_loop_.reset();
   }
 
   network::mojom::ConnectionType connection_type() const {
@@ -75,7 +85,9 @@ class TestNetworkConnectionObserver
 
   size_t num_notifications_;
   NetworkConnectionTracker* tracker_;
+  // May be null.
   std::unique_ptr<base::RunLoop> run_loop_;
+  network::mojom::ConnectionType expected_connection_type_;
   network::mojom::ConnectionType connection_type_;
 
   DISALLOW_COPY_AND_ASSIGN(TestNetworkConnectionObserver);
@@ -217,7 +229,8 @@ TEST_F(NetworkConnectionTrackerTest, ObserverNotified) {
   SimulateConnectionTypeChange(
       net::NetworkChangeNotifier::ConnectionType::CONNECTION_3G);
 
-  network_connection_observer()->WaitForNotification();
+  network_connection_observer()->WaitForNotification(
+      network::mojom::ConnectionType::CONNECTION_3G);
   EXPECT_EQ(network::mojom::ConnectionType::CONNECTION_3G,
             network_connection_observer()->connection_type());
   base::RunLoop().RunUntilIdle();
@@ -234,10 +247,12 @@ TEST_F(NetworkConnectionTrackerTest, UnregisteredObserverNotNotified) {
   SimulateConnectionTypeChange(
       net::NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI);
 
-  network_connection_observer2->WaitForNotification();
+  network_connection_observer2->WaitForNotification(
+      network::mojom::ConnectionType::CONNECTION_WIFI);
   EXPECT_EQ(network::mojom::ConnectionType::CONNECTION_WIFI,
             network_connection_observer2->connection_type());
-  network_connection_observer()->WaitForNotification();
+  network_connection_observer()->WaitForNotification(
+      network::mojom::ConnectionType::CONNECTION_WIFI);
   EXPECT_EQ(network::mojom::ConnectionType::CONNECTION_WIFI,
             network_connection_observer()->connection_type());
   base::RunLoop().RunUntilIdle();
@@ -247,7 +262,8 @@ TEST_F(NetworkConnectionTrackerTest, UnregisteredObserverNotNotified) {
   // Simulate an another network change.
   SimulateConnectionTypeChange(
       net::NetworkChangeNotifier::ConnectionType::CONNECTION_2G);
-  network_connection_observer()->WaitForNotification();
+  network_connection_observer()->WaitForNotification(
+      network::mojom::ConnectionType::CONNECTION_2G);
   EXPECT_EQ(network::mojom::ConnectionType::CONNECTION_2G,
             network_connection_observer()->connection_type());
   EXPECT_EQ(2u, network_connection_observer()->num_notifications());

services/network/public/cpp/network_quality_tracker.cc

@@ -4,6 +4,7 @@
 
 #include "services/network/public/cpp/network_quality_tracker.h"
 
+#include <limits>
 #include <utility>
 
 #include "base/logging.h"
@@ -14,7 +15,7 @@ NetworkQualityTracker::NetworkQualityTracker(
     base::RepeatingCallback<network::mojom::NetworkService*()> callback)
     : get_network_service_callback_(callback),
       effective_connection_type_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
-      downlink_bandwidth_kbps_(INT32_MAX),
+      downlink_bandwidth_kbps_(std::numeric_limits<int32_t>::max()),
       binding_(this) {
   InitializeMojoChannel();
   DCHECK(binding_.is_bound());
@@ -57,6 +58,20 @@ void NetworkQualityTracker::RemoveEffectiveConnectionTypeObserver(
   effective_connection_type_observer_list_.RemoveObserver(observer);
 }
 
+void NetworkQualityTracker::AddRTTAndThroughputEstimatesObserver(
+    RTTAndThroughputEstimatesObserver* observer) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  rtt_and_throughput_observer_list_.AddObserver(observer);
+  observer->OnRTTOrThroughputEstimatesComputed(http_rtt_, transport_rtt_,
+                                               downlink_bandwidth_kbps_);
+}
+
+void NetworkQualityTracker::RemoveRTTAndThroughputEstimatesObserver(
+    RTTAndThroughputEstimatesObserver* observer) {
+  DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
+  rtt_and_throughput_observer_list_.RemoveObserver(observer);
+}
+
 void NetworkQualityTracker::OnNetworkQualityChanged(
     net::EffectiveConnectionType effective_connection_type,
     base::TimeDelta http_rtt,
@@ -64,15 +79,23 @@ void NetworkQualityTracker::OnNetworkQualityChanged(
     int32_t bandwidth_kbps) {
   DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
 
-  http_rtt_ = http_rtt;
-  transport_rtt_ = transport_rtt;
-  downlink_bandwidth_kbps_ = bandwidth_kbps;
-
-  if (effective_connection_type == effective_connection_type_)
-    return;
-  effective_connection_type_ = effective_connection_type;
-  for (auto& observer : effective_connection_type_observer_list_)
-    observer.OnEffectiveConnectionTypeChanged(effective_connection_type_);
+  if (http_rtt_ != http_rtt || transport_rtt_ != transport_rtt ||
+      downlink_bandwidth_kbps_ != bandwidth_kbps) {
+    http_rtt_ = http_rtt;
+    transport_rtt_ = transport_rtt;
+    downlink_bandwidth_kbps_ = bandwidth_kbps;
+
+    for (auto& observer : rtt_and_throughput_observer_list_) {
+      observer.OnRTTOrThroughputEstimatesComputed(http_rtt_, transport_rtt_,
+                                                  downlink_bandwidth_kbps_);
+    }
+  }
+
+  if (effective_connection_type != effective_connection_type_) {
+    effective_connection_type_ = effective_connection_type;
+    for (auto& observer : effective_connection_type_observer_list_)
+      observer.OnEffectiveConnectionTypeChanged(effective_connection_type_);
+  }
 }
 
 void NetworkQualityTracker::InitializeMojoChannel() {

services/network/public/cpp/network_quality_tracker.h

@@ -41,6 +41,28 @@ class COMPONENT_EXPORT(NETWORK_CPP) NetworkQualityTracker
     DISALLOW_COPY_AND_ASSIGN(EffectiveConnectionTypeObserver);
   };
 
+  // Observes changes in the HTTP RTT, transport RTT or downstream throughput
+  // estimates.
+  class COMPONENT_EXPORT(NETWORK_CPP) RTTAndThroughputEstimatesObserver {
+   public:
+    // Called when there is a substantial change in either HTTP RTT, transport
+    // RTT or downstream estimate. If either of the RTT estimates are
+    // unavailable, then the value of that estimate is set to base::TimeDelta().
+    // If downstream estimate is unavailable, its value is set to INT32_MAX.
+    virtual void OnRTTOrThroughputEstimatesComputed(
+        base::TimeDelta http_rtt,
+        base::TimeDelta transport_rtt,
+        int32_t downstream_throughput_kbps) = 0;
+
+    virtual ~RTTAndThroughputEstimatesObserver() {}
+
+   protected:
+    RTTAndThroughputEstimatesObserver() {}
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(RTTAndThroughputEstimatesObserver);
+  };
+
   // Running the |callback| returns the network service in use.
   // NetworkQualityTracker does not need to be destroyed before the network
   // service.
@@ -76,12 +98,30 @@ class COMPONENT_EXPORT(NETWORK_CPP) NetworkQualityTracker
   void AddEffectiveConnectionTypeObserver(
       EffectiveConnectionTypeObserver* observer);
 
-  // Unregisters |observer| from receiving notifications.  This must be called
+  // Unregisters |observer| from receiving notifications. This must be called
   // on the same thread on which AddObserver() was called.
   // All observers must be unregistered before |this| is destroyed.
   void RemoveEffectiveConnectionTypeObserver(
       EffectiveConnectionTypeObserver* observer);
 
+  // Registers |observer| to receive notifications of RTT or throughput changes.
+  // This should be called on the same thread as the thread on which |this| is
+  // created. The |observer| would be called back with notifications on that
+  // same thread. The |observer| is notified of the current HTTP RTT, transport
+  // RTT and downstrean bandwidth estimates on the same thread. If either of the
+  // RTT estimate are unavailable, then the value of that estimate is set to
+  // base::TimeDelta(). If downstream estimate is unavailable, its value is set
+  // to INT32_MAX. The |observer| is notified of the current RTT and
+  // throughput estimates synchronously when this method is invoked.
+  void AddRTTAndThroughputEstimatesObserver(
+      RTTAndThroughputEstimatesObserver* observer);
+
+  // Unregisters |observer| from receiving notifications. This must be called
+  // on the same thread on which AddObserver() was called.
+  // All observers must be unregistered before |this| is destroyed.
+  void RemoveRTTAndThroughputEstimatesObserver(
+      RTTAndThroughputEstimatesObserver* observer);
+
  protected:
   // NetworkQualityEstimatorManagerClient implementation. Protected for testing.
   void OnNetworkQualityChanged(
@@ -112,6 +152,9 @@ class COMPONENT_EXPORT(NETWORK_CPP) NetworkQualityTracker
   base::ObserverList<EffectiveConnectionTypeObserver>
       effective_connection_type_observer_list_;
 
+  base::ObserverList<RTTAndThroughputEstimatesObserver>
+      rtt_and_throughput_observer_list_;
+
   mojo::Binding<network::mojom::NetworkQualityEstimatorManagerClient> binding_;
 
   SEQUENCE_CHECKER(sequence_checker_);