Add Network Quality observer to NQE

net/nqe/network_quality_estimator.h

Index: net/nqe/network_quality_estimator.h
diff --git a/net/nqe/network_quality_estimator.h b/net/nqe/network_quality_estimator.h
index 46b6a8f5017a82ffc38f73aad70945f38fccf93f..7522998c97d121431d0089daba443361904c6201 100644
--- a/net/nqe/network_quality_estimator.h
+++ b/net/nqe/network_quality_estimator.h
@@ -84,6 +84,40 @@ class NET_EXPORT NetworkQualityEstimator
     DISALLOW_COPY_AND_ASSIGN(EffectiveConnectionTypeObserver);
   };
 
+  // Observes changes in the network quality.
+  class NET_EXPORT NetworkQualityObserver {
+   public:
+    // Notifies the observer of a change in the network quality (estimated HTTP
+    // RTT, estimated transport RTT and estimated downstream throughput).
+    // NetworkQualityEstimator computes the network quality once in
+    // every interval of duration
+    // |effective_connection_type_recomputation_interval_|. Additionally, when
+    // there is a change in the connection type of the device, then the
+    // network quality is immediately recomputed. The observer must
+    // register and unregister itself on the IO thread. All the observers would
+    // be notified on the IO thread.

[RyanSturm, 2016/10/11 01:46:26]

nit: I feel like this comment could be reformatted (i.e. register in lline 97
might fit on the previous line). I didn't count characters or anything, so I
might be wrong.

[tbansal1, 2016/10/11 17:46:12]

Done.

+    //
+    // If the computed network quality is different from the previously notified

[RyanSturm, 2016/10/11 01:46:26]

nit: Can you say something like "If any of the three components of network
quality are different" ? I imagine in practice it is rare one changes and
another stays the same, but it sounds like network quality is a metric from
which rtt and kbps are derived as opposed to rtt and kbps being components of
network quality.

[tbansal1, 2016/10/11 17:46:12]

Done.

+    // network quality, then all the registered observers are notified of the
+    // new network quality. |http_rtt|, |transport_rtt| and
+    // |downstream_throughput_kbps| are the current estimates of the HTTP RTT,
+    // transport RTT and downstream throughput (in kilobits per second),
+    // respectively. If an estimate of the HTTP or transport RTT is unavailable,
+    // it will be set to nqe::internal::InvalidRTT(). If the throughput is
+    // unavailable, it will be set to nqe::internal::kInvalidThroughput.
+    virtual void OnNetworkQualityChanged(
+        base::TimeDelta http_rtt,
+        base::TimeDelta transport_rtt,
+        int32_t downstream_throughput_kbps) = 0;
+
+   protected:
+    NetworkQualityObserver() {}
+    virtual ~NetworkQualityObserver() {}
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(NetworkQualityObserver);
+  };
+
   // Observes measurements of round trip time.
   class NET_EXPORT_PRIVATE RTTObserver {
    public:
@@ -185,6 +219,14 @@ class NET_EXPORT NetworkQualityEstimator
   void RemoveEffectiveConnectionTypeObserver(
       EffectiveConnectionTypeObserver* observer);
 
+  // Adds |observer| to the list of network quality observers. Must be
+  // called on the IO thread.
+  void AddNetworkQualityObserver(NetworkQualityObserver* observer);
+
+  // Removes |observer| from the list of network quality observers.
+  // Must be called on the IO thread.
+  void RemoveNetworkQualityObserver(NetworkQualityObserver* observer);
+
   // Notifies NetworkQualityEstimator that the response header of |request| has
   // been received.
   void NotifyHeadersReceived(const URLRequest& request);
@@ -308,6 +350,23 @@ class NET_EXPORT NetworkQualityEstimator
   // Returns a random double in the range [0.0, 1.0). Virtualized for testing.
   virtual double RandDouble() const;
 
+  // Returns the effective type of the current connection based on only the
+  // observations received after |start_time|. May use HTTP RTT, transport RTT
+  // and downstream throughput to compute the effective connection type based on
+  // |http_rtt_metric|, |transport_rtt_metric| and

[RyanSturm, 2016/10/11 01:46:26]

Are the "|*_metric|" variables relevant to this method?

[tbansal1, 2016/10/11 17:46:12]

Done.

+  // |downstream_throughput_kbps_metric|, respectively. |http_rtt|,
+  // |transport_rtt| and |downstream_throughput_kbps| must be non-null.
+  // |http_rtt|, |transport_rtt| and |downstream_throughput_kbps| are
+  // set to the expected HTTP RTT, transport RTT and downstream throughput (in
+  // kilobits per second) based on observations taken since |start_time|.
+  // Virtualized for testing.
+  virtual EffectiveConnectionType
+  GetRecentEffectiveConnectionTypeAndNetworkQuality(
+      const base::TimeTicks& start_time,
+      base::TimeDelta* http_rtt,
+      base::TimeDelta* transport_rtt,
+      int32_t* downstream_throughput_kbps) const;
+
  private:
   FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest,
                            AdaptiveRecomputationEffectiveConnectionType);
@@ -468,12 +527,19 @@ class NET_EXPORT NetworkQualityEstimator
   // samples observed after |start_time|. May use HTTP RTT, transport RTT and
   // downstream throughput to compute the effective connection type based on
   // |http_rtt_metric|, |transport_rtt_metric| and
-  // |downstream_throughput_kbps_metric|, respectively.
+  // |downstream_throughput_kbps_metric|, respectively. |http_rtt|,
+  // |transport_rtt| and |downstream_throughput_kbps| must be non-null.
+  // |http_rtt|, |transport_rtt| and |downstream_throughput_kbps| are
+  // set to the expected HTTP RTT, transport RTT and downstream throughput (in
+  // kilobits per second) based on observations taken since |start_time|.
   EffectiveConnectionType GetRecentEffectiveConnectionTypeUsingMetrics(
       const base::TimeTicks& start_time,
       MetricUsage http_rtt_metric,
       MetricUsage transport_rtt_metric,
-      MetricUsage downstream_throughput_kbps_metric) const;
+      MetricUsage downstream_throughput_kbps_metric,
+      base::TimeDelta* http_rtt,
+      base::TimeDelta* transport_rtt,
+      int32_t* downstream_throughput_kbps) const;
 
   // Values of external estimate provider status. This enum must remain
   // synchronized with the enum of the same name in
@@ -590,10 +656,13 @@ class NET_EXPORT NetworkQualityEstimator
   // system APIs. May be NULL.
   const std::unique_ptr<ExternalEstimateProvider> external_estimate_provider_;
 
-  // Observer list for changes in effective connection type.
+  // Observer list for changes in the effective connection type.

[RyanSturm, 2016/10/11 01:46:26]

nit: I don't think adding "the" here improves this comment. I prefer it the way
it was, but I don't think it makes a large difference.

[tbansal1, 2016/10/11 17:46:12]

Done.

   base::ObserverList<EffectiveConnectionTypeObserver>
       effective_connection_type_observer_list_;
 
+  // Observer list for changes in the network quality.
+  base::ObserverList<NetworkQualityObserver> network_quality_observer_list_;
+
   // Observer lists for round trip times and throughput measurements.
   base::ObserverList<RTTObserver> rtt_observer_list_;
   base::ObserverList<ThroughputObserver> throughput_observer_list_;
@@ -618,6 +687,12 @@ class NET_EXPORT NetworkQualityEstimator
   size_t rtt_observations_size_at_last_ect_computation_;
   size_t throughput_observations_size_at_last_ect_computation_;
 
+  // Current estimates of the HTTP RTT, transport RTT and downstream throughput
+  // (in kilobits per second).
+  base::TimeDelta http_rtt_;
+  base::TimeDelta transport_rtt_;
+  int32_t downstream_throughput_kbps_;
+
   // Current effective connection type. It is updated on connection change
   // events. It is also updated every time there is network traffic (provided
   // the last computation was more than