Expose packet loss counts from QUIC to NetworkQualityEstimator

net/base/network_quality_estimator.h

 // 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.
 
 #ifndef NET_BASE_NETWORK_QUALITY_ESTIMATOR_H_

[bengr, 2016/02/09 15:41:02]

This file is too large. In a follow-up CL, please move each class to a separate
file.

[tbansal1, 2016/02/09 17:54:11]

Acknowledged.

 #define NET_BASE_NETWORK_QUALITY_ESTIMATOR_H_
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <deque>
 #include <map>
 #include <string>
 #include <tuple>
 
@@ skipping 40 matching lines @@
     TCP,
     // The observation is taken at the QUIC layer.
     QUIC,
     // The observation is a previously cached estimate of the metric.
     CACHED_ESTIMATE,
     // The observation is derived from network connection information provided
     // by the platform. For example, typical RTT and throughput values are used
     // for a given type of network connection.
     DEFAULT_FROM_PLATFORM,
     // The observation came from a Chromium-external source.
-    EXTERNAL_ESTIMATE
+    EXTERNAL_ESTIMATE,
+    // Unknown source.

[bengr, 2016/02/09 15:41:03]

Why would the source ever be unknown?

[tbansal1, 2016/02/09 17:54:12]

Fixed the function definition, and eliminated UNKNOWN.

+    UNKNOWN
   };
 
   // Observes measurements of round trip time.
   class NET_EXPORT_PRIVATE RTTObserver {
    public:
     // Will be called when a new RTT observation is available. The round trip
     // time is specified in milliseconds. The time when the observation was
     // taken and the source of the observation are provided.
     virtual void OnRTTObservation(int32_t rtt_ms,
                                   const base::TimeTicks& timestamp,
@@ skipping 17 matching lines @@
                                          ObservationSource source) = 0;
 
    protected:
     ThroughputObserver() {}
     virtual ~ThroughputObserver() {}
 
    private:
     DISALLOW_COPY_AND_ASSIGN(ThroughputObserver);
   };
 
+  // Observes measurements of packet loss.
+  class NET_EXPORT_PRIVATE PacketLossObserver {
+   public:
+    // Will be called when a new packet loss observation is available.
+    // |num_packets_lost| is the number of packets lost.
+    // |num_packets_received_in_order| is the number of packets received in
+    // order. |num_packets_received_not_in_order| is the number of packets
+    // received out of order. All the counts are for the duration since the last
+    // call to the OnPacketLossObservation to |timestamp|.
+    virtual void OnPacketLossObservation(

[bengr, 2016/02/09 15:41:03]

Is num_packets_received_in_order the number of consecutive packets that are
received without a loss? What would the value be if we receive packets
1,2,3,5,6,7,8? Is it 3 or 4? I have no idea what
num_packets_received_no_in_order means, so please explain.

As for the duration, I find it a little strange that an observation is not self
contained. I.e., it doesn't know when it began.

I also find it strange that the observer interface does not expose observations
as you define them below.

[tbansal1, 2016/02/09 17:54:11]

Added an example in socket_performance_watcher_factory.h to make it clearer.
Removed that comment, it was unnecessary and not clear.
Agreed, but I am not sure what's the best way out. It would be ideal for raw
observations and processed observations to have the same format, but it is not
clear how we can do it (or even if it is necessary for them to have the same
format).

+        uint64_t num_packets_lost,
+        uint64_t num_packets_received_in_order,
+        uint64_t num_packets_received_not_in_order,
+        const base::TimeTicks& timestamp,
+        ObservationSource source) = 0;
+
+   protected:
+    PacketLossObserver() {}
+    virtual ~PacketLossObserver() {}
+
+   private:
+    DISALLOW_COPY_AND_ASSIGN(PacketLossObserver);
+  };
+
   // Creates a new NetworkQualityEstimator.
   // |variation_params| is the map containing all field trial parameters
   // related to NetworkQualityEstimator field trial.
   // |external_estimates_provider| may be NULL.
   NetworkQualityEstimator(
       scoped_ptr<ExternalEstimateProvider> external_estimates_provider,
       const std::map<std::string, std::string>& variation_params);
 
   // Construct a NetworkQualityEstimator instance allowing for test
   // configuration. Registers for network type change notifications so estimates
@@ skipping 18 matching lines @@
 
   // Returns true if RTT is available and sets |rtt| to estimated RTT.
   // Virtualized for testing. |rtt| should not be null.
   virtual bool GetRTTEstimate(base::TimeDelta* rtt) const;
 
   // Returns true if downlink throughput is available and sets |kbps| to
   // estimated downlink throughput (in kilobits per second).
   // Virtualized for testing. |kbps| should not be null.
   virtual bool GetDownlinkThroughputKbpsEstimate(int32_t* kbps) const;
 
+  // Returns true if the estimated packet loss rate is available and sets
+  // |packet_loss| to the estimated packet loss rate. Virtualized for testing.
+  // |packet_loss| should not be nullptr. |packet_loss| is always between 0.0

[bengr, 2016/02/09 15:41:02]

In a comment, I would say "should not be null."

[tbansal1, 2016/02/09 17:54:11]

Done.

+  // and 1.0 with a higher value indicating higher packet loss rate.
+  virtual bool GetPacketLossRateEstimate(float* packet_loss) const;
+
   // Notifies NetworkQualityEstimator that the response header of |request| has
   // been received.
   void NotifyHeadersReceived(const URLRequest& request);
 
   // Notifies NetworkQualityEstimator that the response body of |request| has
   // been received.
   void NotifyRequestCompleted(const URLRequest& request);
 
   // Returns true if median RTT is available and sets |rtt| to the median of
   // RTT observations since |begin_timestamp|.
   // Virtualized for testing. |rtt| should not be null.
   virtual bool GetRecentMedianRTT(const base::TimeTicks& begin_timestamp,
                                   base::TimeDelta* rtt) const;
 
   // Returns true if median downstream throughput is available and sets |kbps|
   // to the median of downstream throughput (in kilobits per second)
   // observations since |begin_timestamp|. Virtualized for testing. |kbps|
   // should not be null.
   virtual bool GetRecentMedianDownlinkThroughputKbps(
       const base::TimeTicks& begin_timestamp,
       int32_t* kbps) const;
 
   // SocketPerformanceWatcherFactory implementation:
   scoped_ptr<SocketPerformanceWatcher> CreateSocketPerformanceWatcher(
       const Protocol protocol) override;
   void OnUpdatedRTTAvailable(const Protocol protocol,
                              const base::TimeDelta& rtt) override;
+  void OnUpdatedPacketCountAvailable(
+      const Protocol protocol,
+      uint64_t num_packets_lost,
+      uint64_t num_packets_received_in_order,
+      uint64_t num_packets_received_not_in_order) override;
 
   // Adds |rtt_observer| to the list of round trip time observers. Must be
   // called on the IO thread.
   void AddRTTObserver(RTTObserver* rtt_observer);
 
   // Removes |rtt_observer| from the list of round trip time observers if it
   // is on the list of observers. Must be called on the IO thread.
   void RemoveRTTObserver(RTTObserver* rtt_observer);
 
   // Adds |throughput_observer| to the list of throughput observers. Must be
   // called on the IO thread.
   void AddThroughputObserver(ThroughputObserver* throughput_observer);
 
   // Removes |throughput_observer| from the list of throughput observers if it
   // is on the list of observers. Must be called on the IO thread.
   void RemoveThroughputObserver(ThroughputObserver* throughput_observer);
 
+  // Adds |packet_loss_observer| to the list of packet loss observers. Must be
+  // called on the IO thread.
+  void AddPacketLossObserver(PacketLossObserver* packet_loss_observer);
+
+  // Removes |packet_loss_observer| from the list of packet loss observers if it
+  // is on the list of observers. Must be called on the IO thread.
+  void RemovePacketLossObserver(PacketLossObserver* packet_loss_observer);
+
  protected:
   // NetworkID is used to uniquely identify a network.
   // For the purpose of network quality estimation and caching, a network is
   // uniquely identified by a combination of |type| and
   // |id|. This approach is unable to distinguish networks with
   // same name (e.g., different Wi-Fi networks with same SSID).
   // This is a protected member to expose it to tests.
   struct NET_EXPORT_PRIVATE NetworkID {
     NetworkID(NetworkChangeNotifier::ConnectionType type, const std::string& id)
         : type(type), id(id) {}
@@ skipping 31 matching lines @@
   // NetworkChangeNotifier::ConnectionTypeObserver implementation:
   void OnConnectionTypeChanged(
       NetworkChangeNotifier::ConnectionType type) override;
 
   // ExternalEstimateProvider::UpdatedEstimateObserver implementation.
   void OnUpdatedEstimateAvailable() override;
 
  private:
   FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest, StoreObservations);
   FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest, TestKbpsRTTUpdates);
+  FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest,
+                           TestPacketLossRateUpdates);
   FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest, TestAddObservation);
   FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest, ObtainOperatingParams);
   FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest, HalfLifeParam);
   FRIEND_TEST_ALL_PREFIXES(URLRequestTestHTTP, NetworkQualityEstimator);
   FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest,
                            PercentileSameTimestamps);
   FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest,
                            PercentileDifferentTimestamps);
   FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest, ComputedPercentiles);
   FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest, TestCaching);
@@ skipping 144 matching lines @@
     // buffer is available. Sets |result| to the computed |percentile|
     // value among all observations since |begin_timestamp|. If the value is
     // unavailable, false is returned and |result| is not modified. Percentile
     // value is unavailable if all the values in observation buffer are older
     // than |begin_timestamp|.
     // |result| must not be null.
     bool GetPercentile(const base::TimeTicks& begin_timestamp,
                        ValueType* result,
                        int percentile) const;
 
+    // Returns true iff the weighted average value of the observations in this
+    // buffer is available. Sets |result| to the computed average
+    // value among all observations since |begin_timestamp|. If the value is
+    // unavailable, false is returned and |result| is not modified. Average
+    // value is unavailable if all the values in observation buffer are older
+    // than |begin_timestamp|. |result| must not be nullptr.
+    bool GetWeightedAverage(const base::TimeTicks& begin_timestamp,

[bengr, 2016/02/09 15:41:03]

If this is computed over observations then it is computed over uneven intervals.
Is that representative? E.g., one interval could lose 1 out of 2 packets, and
another could lose 1 out of 1000, but the reported loss rate would be ~1/2.

[tbansal1, 2016/02/09 17:54:11]

I do not understand this comment. Two observations taken at the same time have
same weight. Two observations may differ in weight only if they were taken at
different times. Each observation corresponds to 1 packet received or 1 packet
lost.

+                            ValueType* result) const;
+
    private:
     FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest, StoreObservations);
     FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest,
                              ObtainOperatingParams);
     FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest, HalfLifeParam);
 
     // Computes the weighted observations and stores them in
     // |weighted_observations| sorted by ascending |WeightedObservation.value|.
     // Only the observations with timestamp later than |begin_timestamp| are
     // considered. Also, sets |total_weight| to the total weight of all
@@ skipping 23 matching lines @@
       RttObservation;
   typedef net::NetworkQualityEstimator::ObservationBuffer<base::TimeDelta>
       RttObservationBuffer;
 
   // Value of throughput observations is in kilobits per second.
   typedef net::NetworkQualityEstimator::Observation<int32_t>
       ThroughputObservation;
   typedef net::NetworkQualityEstimator::ObservationBuffer<int32_t>
       ThroughputObservationBuffer;
 
+  // Value of a packet loss observation. A value of 0.0 indicates one packet
+  // was received successfully in-order, while a value of 1.0 indicates one
+  // packet was lost.
+  typedef net::NetworkQualityEstimator::Observation<float>
+      PacketLossObservation;
+
   // This does not use a unordered_map or hash_map for code simplicity (key just
   // implements operator<, rather than hash and equality) and because the map is
   // tiny.
   typedef std::map<NetworkID, CachedNetworkQuality> CachedNetworkQualities;
 
   // Throughput is set to |kInvalidThroughput| if a valid value is
   // unavailable. Readers should discard throughput value if it is set to
   // |kInvalidThroughput|.
   static const int32_t kInvalidThroughput;
 
+  // Packet loss rate is set to |kInvalidPacketLossRate| if a valid value is
+  // unavailable. Readers should discard the packet loss rate value if it is
+  // set to |kInvalidPacketLossRate|.
+  static const float kInvalidPacketLossRate;
+
   // Tiny transfer sizes may give inaccurate throughput results.
   // Minimum size of the transfer over which the throughput is computed.
   static const int kMinTransferSizeInBytes = 10000;
 
   // Minimum duration (in microseconds) of the transfer over which the
   // throughput is computed.
   static const int kMinRequestDurationMicroseconds = 1000;
 
   // Minimum valid value of the variation parameter that holds RTT (in
   // milliseconds) values.
@@ skipping 38 matching lines @@
   // percentiles indicating less performant networks. For example, if
   // |percentile| is 90, then the network is expected to be faster than the
   // returned estimate with 0.9 probability. Similarly, network is expected to
   // be slower than the returned estimate with 0.1 probability.
   base::TimeDelta GetRTTEstimateInternal(const base::TimeTicks& begin_timestamp,
                                          int percentile) const;
   int32_t GetDownlinkThroughputKbpsEstimateInternal(
       const base::TimeTicks& begin_timestamp,
       int percentile) const;
 
+  // Returns an estimate of the packet loss rate. Only the observations later
+  // than |begin_timestamp| are taken into account.
+  float GetPacketLossRateEstimateInternal(
+      const base::TimeTicks& begin_timestamp) const;
+
   // Returns the current network ID checking by calling the platform APIs.
   // Virtualized for testing.
   virtual NetworkID GetCurrentNetworkID() const;
 
   // Writes the estimated quality of the current network to the cache.
   void CacheNetworkQualityEstimate();
 
   void NotifyObserversOfRTT(const RttObservation& observation);
 
   void NotifyObserversOfThroughput(const ThroughputObservation& observation);
 
+  void NotifyObserversOfPacketLoss(uint64_t num_packets_lost,
+                                   uint64_t num_packets_received_in_order,
+                                   uint64_t num_packets_received_not_in_order,
+                                   const base::TimeTicks timestamp,

[bengr, 2016/02/09 15:41:02]

Why not a const base::TimeTicks& ?

[tbansal1, 2016/02/09 17:54:11]

Done.

+                                   ObservationSource source);
+
   // Records the UMA related to RTT.
   void RecordRTTUMA(int32_t estimated_value_msec,
                     int32_t actual_value_msec) const;
 
   // Returns true only if |request| can be used for network quality estimation.
   // Only the requests that go over network are considered to provide useful
   // observations.
   bool RequestProvidesUsefulObservations(const URLRequest& request) const;
 
   // Values of external estimate provider status. This enum must remain
   // synchronized with the enum of the same name in
   // metrics/histograms/histograms.xml.
   enum NQEExternalEstimateProviderStatus {
     EXTERNAL_ESTIMATE_PROVIDER_STATUS_NOT_AVAILABLE,
     EXTERNAL_ESTIMATE_PROVIDER_STATUS_AVAILABLE,
     EXTERNAL_ESTIMATE_PROVIDER_STATUS_QUERIED,
     EXTERNAL_ESTIMATE_PROVIDER_STATUS_QUERY_SUCCESSFUL,
     EXTERNAL_ESTIMATE_PROVIDER_STATUS_CALLBACK,
     EXTERNAL_ESTIMATE_PROVIDER_STATUS_BOUNDARY
   };
 
   // Records the metrics related to external estimate provider.
   void RecordExternalEstimateProviderMetrics(
       NQEExternalEstimateProviderStatus status) const;
 
+  ObservationSource GetObservationSourceForProtocol(
+      const Protocol protocol) const;
+
   // Determines if the requests to local host can be used in estimating the
   // network quality. Set to true only for tests.
   const bool allow_localhost_requests_;
 
   // Determines if the responses smaller than |kMinTransferSizeInBytes|
   // or shorter than |kMinTransferSizeInBytes| can be used in estimating the
   // network quality. Set to true only for tests.
   const bool allow_small_responses_;
 
   // Time when last connection change was observed.
@@ skipping 13 matching lines @@
   // Cache that stores quality of previously seen networks.
   CachedNetworkQualities cached_network_qualities_;
 
   // Buffer that holds throughput observations (in kilobits per second) sorted
   // by timestamp.
   ThroughputObservationBuffer downstream_throughput_kbps_observations_;
 
   // Buffer that holds RTT observations sorted by timestamp.
   RttObservationBuffer rtt_msec_observations_;
 
+  // Buffer that holds packet loss observations sorted by timestamp.
+  ObservationBuffer<float> packet_loss_rate_observations_;
+
   // Default network quality observations obtained from the network quality
   // estimator field trial parameters. The observations are indexed by
   // ConnectionType.
   NetworkQuality
       default_observations_[NetworkChangeNotifier::CONNECTION_LAST + 1];
 
   // Estimated network quality. Updated on mainframe requests.
   NetworkQuality estimated_median_network_quality_;
 
   // ExternalEstimateProvider that provides network quality using operating
   // system APIs. May be NULL.
   const scoped_ptr<ExternalEstimateProvider> external_estimate_provider_;
 
-  // Observer lists for round trip times and throughput measurements.
+  // Observer lists for round trip times, throughput and packet loss
+  // measurements.
   base::ObserverList<RTTObserver> rtt_observer_list_;
   base::ObserverList<ThroughputObserver> throughput_observer_list_;
+  base::ObserverList<PacketLossObserver> packet_loss_observer_list_;
 
   base::ThreadChecker thread_checker_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkQualityEstimator);
 };
 
 }  // namespace net
 
 #endif  // NET_BASE_NETWORK_QUALITY_ESTIMATOR_H_