From e481a6cb6e05f3cfac2936c18ec8954c2022b755 Mon Sep 17 00:00:00 2001 From: tbansal Date: Mon, 25 Apr 2016 13:41:13 -0700 Subject: [PATCH] NQE miscellaneous clean ups GetRTTEstimateInternal now takes a list of observation sources as a parameter. Observations from these sources are discarded when computing performance metrics. This will help when adding API that returns median RTT at the transport layer. Condensed histogram code Change some of the code in n_q_e.cc and some tests so that they call NQE's public APIs. BUG=498380 Review URL: https://codereview.chromium.org/1892273002 Cr-Commit-Position: refs/heads/master@{#389555} --- net/base/network_quality_estimator.cc | 181 ++++++----------- net/base/network_quality_estimator.h | 31 ++- .../network_quality_estimator_unittest.cc | 182 +++++++++--------- 3 files changed, 165 insertions(+), 229 deletions(-) diff --git a/net/base/network_quality_estimator.cc b/net/base/network_quality_estimator.cc index 2233333bcc3c8d..d0dd55a44e8df8 100644 --- a/net/base/network_quality_estimator.cc +++ b/net/base/network_quality_estimator.cc @@ -405,9 +405,16 @@ void NetworkQualityEstimator::NotifyHeadersReceived(const URLRequest& request) { // Update |estimated_median_network_quality_| if this is a main frame request. if (request.load_flags() & LOAD_MAIN_FRAME) { - estimated_median_network_quality_ = NetworkQuality( - GetURLRequestRTTEstimateInternal(base::TimeTicks(), 50), - GetDownlinkThroughputKbpsEstimateInternal(base::TimeTicks(), 50)); + base::TimeDelta rtt; + if (!GetURLRequestRTTEstimate(&rtt)) + rtt = InvalidRTT(); + + int32_t downstream_throughput_kbps; + if (!GetDownlinkThroughputKbpsEstimate(&downstream_throughput_kbps)) + downstream_throughput_kbps = kInvalidThroughput; + + estimated_median_network_quality_ = + NetworkQuality(rtt, downstream_throughput_kbps); } base::TimeTicks now = base::TimeTicks::Now(); @@ -600,104 +607,34 @@ void NetworkQualityEstimator::OnConnectionTypeChanged( NetworkChangeNotifier::ConnectionType type) { DCHECK(thread_checker_.CalledOnValidThread()); if (peak_network_quality_.rtt() != InvalidRTT()) { - switch (current_network_id_.type) { - case NetworkChangeNotifier::CONNECTION_UNKNOWN: - UMA_HISTOGRAM_TIMES("NQE.FastestRTT.Unknown", - peak_network_quality_.rtt()); - break; - case NetworkChangeNotifier::CONNECTION_ETHERNET: - UMA_HISTOGRAM_TIMES("NQE.FastestRTT.Ethernet", - peak_network_quality_.rtt()); - break; - case NetworkChangeNotifier::CONNECTION_WIFI: - UMA_HISTOGRAM_TIMES("NQE.FastestRTT.Wifi", peak_network_quality_.rtt()); - break; - case NetworkChangeNotifier::CONNECTION_2G: - UMA_HISTOGRAM_TIMES("NQE.FastestRTT.2G", peak_network_quality_.rtt()); - break; - case NetworkChangeNotifier::CONNECTION_3G: - UMA_HISTOGRAM_TIMES("NQE.FastestRTT.3G", peak_network_quality_.rtt()); - break; - case NetworkChangeNotifier::CONNECTION_4G: - UMA_HISTOGRAM_TIMES("NQE.FastestRTT.4G", peak_network_quality_.rtt()); - break; - case NetworkChangeNotifier::CONNECTION_NONE: - UMA_HISTOGRAM_TIMES("NQE.FastestRTT.None", peak_network_quality_.rtt()); - break; - case NetworkChangeNotifier::CONNECTION_BLUETOOTH: - UMA_HISTOGRAM_TIMES("NQE.FastestRTT.Bluetooth", - peak_network_quality_.rtt()); - break; - default: - NOTREACHED() << "Unexpected connection type = " - << current_network_id_.type; - break; - } + base::HistogramBase* rtt_histogram = + GetHistogram("FastestRTT.", current_network_id_.type, 10 * 1000); + rtt_histogram->Add(peak_network_quality_.rtt().InMilliseconds()); } if (peak_network_quality_.downstream_throughput_kbps() != kInvalidThroughput) { - switch (current_network_id_.type) { - case NetworkChangeNotifier::CONNECTION_UNKNOWN: - UMA_HISTOGRAM_COUNTS( - "NQE.PeakKbps.Unknown", - peak_network_quality_.downstream_throughput_kbps()); - break; - case NetworkChangeNotifier::CONNECTION_ETHERNET: - UMA_HISTOGRAM_COUNTS( - "NQE.PeakKbps.Ethernet", - peak_network_quality_.downstream_throughput_kbps()); - break; - case NetworkChangeNotifier::CONNECTION_WIFI: - UMA_HISTOGRAM_COUNTS( - "NQE.PeakKbps.Wifi", - peak_network_quality_.downstream_throughput_kbps()); - break; - case NetworkChangeNotifier::CONNECTION_2G: - UMA_HISTOGRAM_COUNTS( - "NQE.PeakKbps.2G", - peak_network_quality_.downstream_throughput_kbps()); - break; - case NetworkChangeNotifier::CONNECTION_3G: - UMA_HISTOGRAM_COUNTS( - "NQE.PeakKbps.3G", - peak_network_quality_.downstream_throughput_kbps()); - break; - case NetworkChangeNotifier::CONNECTION_4G: - UMA_HISTOGRAM_COUNTS( - "NQE.PeakKbps.4G", - peak_network_quality_.downstream_throughput_kbps()); - break; - case NetworkChangeNotifier::CONNECTION_NONE: - UMA_HISTOGRAM_COUNTS( - "NQE.PeakKbps.None", - peak_network_quality_.downstream_throughput_kbps()); - break; - case NetworkChangeNotifier::CONNECTION_BLUETOOTH: - UMA_HISTOGRAM_COUNTS( - "NQE.PeakKbps.Bluetooth", - peak_network_quality_.downstream_throughput_kbps()); - break; - default: - NOTREACHED() << "Unexpected connection type = " - << current_network_id_.type; - break; - } + base::HistogramBase* downstream_throughput_histogram = + GetHistogram("PeakKbps.", current_network_id_.type, 1000 * 1000); + downstream_throughput_histogram->Add( + peak_network_quality_.downstream_throughput_kbps()); } - base::TimeDelta rtt = GetURLRequestRTTEstimateInternal(base::TimeTicks(), 50); - if (rtt != InvalidRTT()) { + base::TimeDelta rtt; + if (GetURLRequestRTTEstimate(&rtt)) { // Add the 50th percentile value. base::HistogramBase* rtt_percentile = - GetHistogram("RTT.Percentile50.", current_network_id_.type, - 10 * 1000); // 10 seconds + GetHistogram("RTT.Percentile50.", current_network_id_.type, 10 * 1000); rtt_percentile->Add(rtt.InMilliseconds()); // Add the remaining percentile values. static const int kPercentiles[] = {0, 10, 90, 100}; + std::vector disallowed_observation_sources; + disallowed_observation_sources.push_back(TCP); + disallowed_observation_sources.push_back(QUIC); for (size_t i = 0; i < arraysize(kPercentiles); ++i) { - rtt = - GetURLRequestRTTEstimateInternal(base::TimeTicks(), kPercentiles[i]); + rtt = GetRTTEstimateInternal(disallowed_observation_sources, + base::TimeTicks(), kPercentiles[i]); rtt_percentile = GetHistogram( "RTT.Percentile" + base::IntToString(kPercentiles[i]) + ".", @@ -774,23 +711,17 @@ NetworkQualityEstimator::GetEffectiveConnectionType() const { bool NetworkQualityEstimator::GetURLRequestRTTEstimate( base::TimeDelta* rtt) const { DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(rtt); - if (rtt_observations_.Size() == 0) { - *rtt = InvalidRTT(); - return false; - } - *rtt = GetURLRequestRTTEstimateInternal(base::TimeTicks(), 50); + std::vector disallowed_observation_sources; + disallowed_observation_sources.push_back(TCP); + disallowed_observation_sources.push_back(QUIC); + *rtt = GetRTTEstimateInternal(disallowed_observation_sources, + base::TimeTicks(), 50); return (*rtt != InvalidRTT()); } bool NetworkQualityEstimator::GetDownlinkThroughputKbpsEstimate( int32_t* kbps) const { DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(kbps); - if (downstream_throughput_kbps_observations_.Size() == 0) { - *kbps = kInvalidThroughput; - return false; - } *kbps = GetDownlinkThroughputKbpsEstimateInternal(base::TimeTicks(), 50); return (*kbps != kInvalidThroughput); } @@ -799,8 +730,11 @@ bool NetworkQualityEstimator::GetRecentURLRequestRTTMedian( const base::TimeTicks& begin_timestamp, base::TimeDelta* rtt) const { DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(rtt); - *rtt = GetURLRequestRTTEstimateInternal(begin_timestamp, 50); + std::vector disallowed_observation_sources; + disallowed_observation_sources.push_back(TCP); + disallowed_observation_sources.push_back(QUIC); + *rtt = GetRTTEstimateInternal(disallowed_observation_sources, begin_timestamp, + 50); return (*rtt != InvalidRTT()); } @@ -808,7 +742,6 @@ bool NetworkQualityEstimator::GetRecentMedianDownlinkThroughputKbps( const base::TimeTicks& begin_timestamp, int32_t* kbps) const { DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK(kbps); *kbps = GetDownlinkThroughputKbpsEstimateInternal(begin_timestamp, 50); return (*kbps != kInvalidThroughput); } @@ -826,23 +759,19 @@ NetworkQualityEstimator::ObservationBuffer::ObservationBuffer( template NetworkQualityEstimator::ObservationBuffer::~ObservationBuffer() {} -base::TimeDelta NetworkQualityEstimator::GetURLRequestRTTEstimateInternal( +base::TimeDelta NetworkQualityEstimator::GetRTTEstimateInternal( + const std::vector& disallowed_observation_sources, const base::TimeTicks& begin_timestamp, int percentile) const { DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK_GE(percentile, 0); - DCHECK_LE(percentile, 100); - if (rtt_observations_.Size() == 0) - return InvalidRTT(); // RTT observations are sorted by duration from shortest to longest, thus // a higher percentile RTT will have a longer RTT than a lower percentile. base::TimeDelta rtt = InvalidRTT(); - std::vector disallowed_observation_sources; - disallowed_observation_sources.push_back(TCP); - disallowed_observation_sources.push_back(QUIC); - rtt_observations_.GetPercentile(begin_timestamp, &rtt, percentile, - disallowed_observation_sources); + if (!rtt_observations_.GetPercentile(begin_timestamp, &rtt, percentile, + disallowed_observation_sources)) { + return InvalidRTT(); + } return rtt; } @@ -850,17 +779,15 @@ int32_t NetworkQualityEstimator::GetDownlinkThroughputKbpsEstimateInternal( const base::TimeTicks& begin_timestamp, int percentile) const { DCHECK(thread_checker_.CalledOnValidThread()); - DCHECK_GE(percentile, 0); - DCHECK_LE(percentile, 100); - if (downstream_throughput_kbps_observations_.Size() == 0) - return kInvalidThroughput; // Throughput observations are sorted by kbps from slowest to fastest, // thus a higher percentile throughput will be faster than a lower one. int32_t kbps = kInvalidThroughput; - downstream_throughput_kbps_observations_.GetPercentile( - begin_timestamp, &kbps, 100 - percentile, - std::vector()); + if (!downstream_throughput_kbps_observations_.GetPercentile( + begin_timestamp, &kbps, 100 - percentile, + std::vector())) { + return kInvalidThroughput; + } return kbps; } @@ -909,6 +836,9 @@ bool NetworkQualityEstimator::ObservationBuffer::GetPercentile( const std::vector& disallowed_observation_sources) const { DCHECK(result); + DCHECK_GE(percentile, 0); + DCHECK_LE(percentile, 100); + // Stores WeightedObservation in increasing order of value. std::vector> weighted_observations; @@ -1111,14 +1041,17 @@ void NetworkQualityEstimator::CacheNetworkQualityEstimate() { if (current_network_id_.id.empty()) return; - NetworkQuality network_quality = NetworkQuality( - GetURLRequestRTTEstimateInternal(base::TimeTicks(), 50), - GetDownlinkThroughputKbpsEstimateInternal(base::TimeTicks(), 50)); - if (network_quality.rtt() == InvalidRTT() || - network_quality.downstream_throughput_kbps() == kInvalidThroughput) { + base::TimeDelta rtt = InvalidRTT(); + int32_t downlink_throughput_kbps = kInvalidThroughput; + + if (!GetURLRequestRTTEstimate(&rtt) || + !GetDownlinkThroughputKbpsEstimate(&downlink_throughput_kbps)) { return; } + NetworkQuality network_quality = + NetworkQuality(rtt, downlink_throughput_kbps); + if (cached_network_qualities_.size() == kMaximumNetworkQualityCacheSize) { // Remove the oldest entry. CachedNetworkQualities::iterator oldest_entry_iterator = diff --git a/net/base/network_quality_estimator.h b/net/base/network_quality_estimator.h index 7aecd3265f5270..7a9783446c0f61 100644 --- a/net/base/network_quality_estimator.h +++ b/net/base/network_quality_estimator.h @@ -14,6 +14,7 @@ #include #include +#include "base/compiler_specific.h" #include "base/gtest_prod_util.h" #include "base/macros.h" #include "base/memory/ref_counted.h" @@ -161,7 +162,8 @@ class NET_EXPORT_PRIVATE NetworkQualityEstimator // the HTTP layer measures the time from when the request was sent (this // happens after the connection is established) to the time when the response // headers were received. - virtual bool GetURLRequestRTTEstimate(base::TimeDelta* rtt) const; + virtual bool GetURLRequestRTTEstimate(base::TimeDelta* rtt) const + WARN_UNUSED_RESULT; // Returns true if downlink throughput is available and sets |kbps| to // estimated downlink throughput (in kilobits per second). @@ -184,7 +186,7 @@ class NET_EXPORT_PRIVATE NetworkQualityEstimator // received. virtual bool GetRecentURLRequestRTTMedian( const base::TimeTicks& begin_timestamp, - base::TimeDelta* rtt) const; + base::TimeDelta* rtt) const WARN_UNUSED_RESULT; // Returns true if median downstream throughput is available and sets |kbps| // to the median of downstream throughput (in kilobits per second) @@ -192,7 +194,7 @@ class NET_EXPORT_PRIVATE NetworkQualityEstimator // should not be null. virtual bool GetRecentMedianDownlinkThroughputKbps( const base::TimeTicks& begin_timestamp, - int32_t* kbps) const; + int32_t* kbps) const WARN_UNUSED_RESULT; // Adds |rtt_observer| to the list of round trip time observers. Must be // called on the IO thread. @@ -266,7 +268,6 @@ class NET_EXPORT_PRIVATE NetworkQualityEstimator private: FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest, StoreObservations); - FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest, TestKbpsRTTUpdates); FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest, TestAddObservation); FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest, ObtainOperatingParams); FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest, HalfLifeParam); @@ -416,9 +417,6 @@ class NET_EXPORT_PRIVATE NetworkQualityEstimator static_cast(kMaximumObservationsBufferSize)); } - // Returns the number of observations in this buffer. - size_t Size() const { return observations_.size(); } - // Clears the observations stored in this buffer. void Clear() { observations_.clear(); } @@ -430,16 +428,14 @@ class NET_EXPORT_PRIVATE NetworkQualityEstimator // than |begin_timestamp|. |result| must not be null. // |disallowed_observation_sources| is the list of observation sources that // should be excluded when computing the percentile. - bool GetPercentile(const base::TimeTicks& begin_timestamp, - ValueType* result, - int percentile, - const std::vector& - disallowed_observation_sources) const; + bool GetPercentile( + const base::TimeTicks& begin_timestamp, + ValueType* result, + int percentile, + const std::vector& disallowed_observation_sources) + const WARN_UNUSED_RESULT; 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 @@ -550,13 +546,16 @@ class NET_EXPORT_PRIVATE NetworkQualityEstimator void AddDefaultEstimates(); // Returns an estimate of network quality at the specified |percentile|. + // |disallowed_observation_sources| is the list of observation sources that + // should be excluded when computing the percentile. // Only the observations later than |begin_timestamp| are taken into account. // |percentile| must be between 0 and 100 (both inclusive) with higher // 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 GetURLRequestRTTEstimateInternal( + base::TimeDelta GetRTTEstimateInternal( + const std::vector& disallowed_observation_sources, const base::TimeTicks& begin_timestamp, int percentile) const; int32_t GetDownlinkThroughputKbpsEstimateInternal( diff --git a/net/base/network_quality_estimator_unittest.cc b/net/base/network_quality_estimator_unittest.cc index ba258f9d34f61a..1d9e81d7c19b99 100644 --- a/net/base/network_quality_estimator_unittest.cc +++ b/net/base/network_quality_estimator_unittest.cc @@ -207,11 +207,10 @@ TEST(NetworkQualityEstimatorTest, TestKbpsRTTUpdates) { std::map variation_params; TestNetworkQualityEstimator estimator(variation_params); - EXPECT_EQ(NetworkQualityEstimator::InvalidRTT(), - estimator.GetURLRequestRTTEstimateInternal(base::TimeTicks(), 100)); - EXPECT_EQ(NetworkQualityEstimator::kInvalidThroughput, - estimator.GetDownlinkThroughputKbpsEstimateInternal( - base::TimeTicks(), 100)); + base::TimeDelta rtt; + int32_t kbps; + EXPECT_FALSE(estimator.GetURLRequestRTTEstimate(&rtt)); + EXPECT_FALSE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps)); TestDelegate test_delegate; TestURLRequestContext context(true); @@ -225,23 +224,11 @@ TEST(NetworkQualityEstimatorTest, TestKbpsRTTUpdates) { base::RunLoop().Run(); // Both RTT and downstream throughput should be updated. - EXPECT_NE(NetworkQualityEstimator::InvalidRTT(), - estimator.GetURLRequestRTTEstimateInternal(base::TimeTicks(), 100)); - EXPECT_NE(NetworkQualityEstimator::kInvalidThroughput, - estimator.GetDownlinkThroughputKbpsEstimateInternal( - base::TimeTicks(), 100)); - - base::TimeDelta rtt = NetworkQualityEstimator::InvalidRTT(); - int32_t kbps = NetworkQualityEstimator::kInvalidThroughput; EXPECT_TRUE(estimator.GetURLRequestRTTEstimate(&rtt)); EXPECT_TRUE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps)); - EXPECT_NE(NetworkQualityEstimator::InvalidRTT(), rtt); - EXPECT_NE(NetworkQualityEstimator::kInvalidThroughput, kbps); - EXPECT_NEAR(rtt.InMilliseconds(), - estimator.GetURLRequestRTTEstimateInternal(base::TimeTicks(), 100) - .InMilliseconds(), - 1); + EXPECT_TRUE(estimator.GetURLRequestRTTEstimate(&rtt)); + EXPECT_TRUE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps)); // Check UMA histograms. histogram_tester.ExpectTotalCount("NQE.PeakKbps.Unknown", 0); @@ -269,28 +256,14 @@ TEST(NetworkQualityEstimatorTest, TestKbpsRTTUpdates) { histogram_tester.ExpectTotalCount("NQE.RTT.Percentile90.Unknown", 1); histogram_tester.ExpectTotalCount("NQE.RTT.Percentile100.Unknown", 1); - EXPECT_EQ(NetworkQualityEstimator::InvalidRTT(), - estimator.GetURLRequestRTTEstimateInternal(base::TimeTicks(), 100)); - EXPECT_EQ(NetworkQualityEstimator::kInvalidThroughput, - estimator.GetDownlinkThroughputKbpsEstimateInternal( - base::TimeTicks(), 100)); - EXPECT_FALSE(estimator.GetURLRequestRTTEstimate(&rtt)); EXPECT_FALSE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps)); - EXPECT_EQ(NetworkQualityEstimator::InvalidRTT(), rtt); - EXPECT_EQ(NetworkQualityEstimator::kInvalidThroughput, kbps); estimator.SimulateNetworkChangeTo( NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI, std::string()); histogram_tester.ExpectTotalCount("NQE.PeakKbps.Unknown", 1); histogram_tester.ExpectTotalCount("NQE.FastestRTT.Unknown", 1); - EXPECT_EQ(NetworkQualityEstimator::InvalidRTT(), - estimator.GetURLRequestRTTEstimateInternal(base::TimeTicks(), 100)); - EXPECT_EQ(NetworkQualityEstimator::kInvalidThroughput, - estimator.GetDownlinkThroughputKbpsEstimateInternal( - base::TimeTicks(), 100)); - EXPECT_FALSE(estimator.GetURLRequestRTTEstimate(&rtt)); EXPECT_FALSE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps)); } @@ -299,6 +272,11 @@ TEST(NetworkQualityEstimatorTest, StoreObservations) { std::map variation_params; TestNetworkQualityEstimator estimator(variation_params); + base::TimeDelta rtt; + int32_t kbps; + EXPECT_FALSE(estimator.GetURLRequestRTTEstimate(&rtt)); + EXPECT_FALSE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps)); + TestDelegate test_delegate; TestURLRequestContext context(true); context.set_network_quality_estimator(&estimator); @@ -310,20 +288,15 @@ TEST(NetworkQualityEstimatorTest, StoreObservations) { estimator.GetEchoURL(), DEFAULT_PRIORITY, &test_delegate)); request->Start(); base::RunLoop().Run(); + EXPECT_TRUE(estimator.GetURLRequestRTTEstimate(&rtt)); + EXPECT_TRUE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps)); } - EXPECT_EQ(static_cast( - NetworkQualityEstimator::kMaximumObservationsBufferSize), - estimator.downstream_throughput_kbps_observations_.Size()); - EXPECT_EQ(static_cast( - NetworkQualityEstimator::kMaximumObservationsBufferSize), - estimator.rtt_observations_.Size()); - // Verify that the stored observations are cleared on network change. estimator.SimulateNetworkChangeTo( NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI, "test-2"); - EXPECT_EQ(0U, estimator.downstream_throughput_kbps_observations_.Size()); - EXPECT_EQ(0U, estimator.rtt_observations_.Size()); + EXPECT_FALSE(estimator.GetURLRequestRTTEstimate(&rtt)); + EXPECT_FALSE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps)); } // Verifies that the percentiles are correctly computed. All observations have @@ -390,19 +363,19 @@ TEST(NetworkQualityEstimatorTest, PercentileSameTimestamps) { EXPECT_NEAR(estimator.GetDownlinkThroughputKbpsEstimateInternal( base::TimeTicks(), i), 100 - i, 1); - EXPECT_NEAR(estimator.GetURLRequestRTTEstimateInternal(base::TimeTicks(), i) + std::vector + disallowed_observation_sources; + disallowed_observation_sources.push_back(NetworkQualityEstimator::TCP); + disallowed_observation_sources.push_back(NetworkQualityEstimator::QUIC); + EXPECT_NEAR(estimator + .GetRTTEstimateInternal(disallowed_observation_sources, + base::TimeTicks(), i) .InMilliseconds(), i, 1); } EXPECT_TRUE(estimator.GetURLRequestRTTEstimate(&rtt)); EXPECT_TRUE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps)); - // |network_quality| should be equal to the 50 percentile value. - EXPECT_TRUE(estimator.GetDownlinkThroughputKbpsEstimateInternal( - base::TimeTicks(), 50) > 0); - EXPECT_TRUE( - estimator.GetURLRequestRTTEstimateInternal(base::TimeTicks(), 50) != - NetworkQualityEstimator::InvalidRTT()); } // Verifies that the percentiles are correctly computed. Observations have @@ -432,12 +405,25 @@ TEST(NetworkQualityEstimatorTest, PercentileDifferentTimestamps) { estimator.downstream_throughput_kbps_observations_.AddObservation( NetworkQualityEstimator::ThroughputObservation( i, now, NetworkQualityEstimator::URL_REQUEST)); + + // Insert TCP observation which should not be taken into account when + // computing median RTT at HTTP layer. + estimator.rtt_observations_.AddObservation( + NetworkQualityEstimator::RttObservation( + base::TimeDelta::FromMilliseconds(10000), now, + NetworkQualityEstimator::TCP)); + estimator.rtt_observations_.AddObservation( NetworkQualityEstimator::RttObservation( base::TimeDelta::FromMilliseconds(i), now, NetworkQualityEstimator::URL_REQUEST)); } + std::vector + disallowed_observation_sources; + disallowed_observation_sources.push_back(NetworkQualityEstimator::TCP); + disallowed_observation_sources.push_back(NetworkQualityEstimator::QUIC); + // Older samples have very little weight. So, all percentiles are >= 51 // (lowest value among recent observations). for (int i = 1; i < 100; ++i) { @@ -448,17 +434,12 @@ TEST(NetworkQualityEstimatorTest, PercentileDifferentTimestamps) { EXPECT_NEAR(estimator.GetDownlinkThroughputKbpsEstimateInternal( base::TimeTicks(), i), 51 + 0.49 * (100 - i), 1); - EXPECT_NEAR(estimator.GetURLRequestRTTEstimateInternal(base::TimeTicks(), i) + EXPECT_NEAR(estimator + .GetRTTEstimateInternal(disallowed_observation_sources, + base::TimeTicks(), i) .InMilliseconds(), 51 + 0.49 * i, 1); } - - EXPECT_EQ(NetworkQualityEstimator::InvalidRTT(), - estimator.GetURLRequestRTTEstimateInternal( - base::TimeTicks::Now() + base::TimeDelta::FromMinutes(10), 50)); - EXPECT_EQ(NetworkQualityEstimator::kInvalidThroughput, - estimator.GetDownlinkThroughputKbpsEstimateInternal( - base::TimeTicks::Now() + base::TimeDelta::FromMinutes(10), 50)); } // This test notifies NetworkQualityEstimator of received data. Next, @@ -468,8 +449,14 @@ TEST(NetworkQualityEstimatorTest, ComputedPercentiles) { std::map variation_params; TestNetworkQualityEstimator estimator(variation_params); + std::vector + disallowed_observation_sources; + disallowed_observation_sources.push_back(NetworkQualityEstimator::TCP); + disallowed_observation_sources.push_back(NetworkQualityEstimator::QUIC); + EXPECT_EQ(NetworkQualityEstimator::InvalidRTT(), - estimator.GetURLRequestRTTEstimateInternal(base::TimeTicks(), 100)); + estimator.GetRTTEstimateInternal(disallowed_observation_sources, + base::TimeTicks(), 100)); EXPECT_EQ(NetworkQualityEstimator::kInvalidThroughput, estimator.GetDownlinkThroughputKbpsEstimateInternal( base::TimeTicks(), 100)); @@ -492,7 +479,8 @@ TEST(NetworkQualityEstimatorTest, ComputedPercentiles) { EXPECT_GT(estimator.GetDownlinkThroughputKbpsEstimateInternal( base::TimeTicks(), i), 0); - EXPECT_LT(estimator.GetURLRequestRTTEstimateInternal(base::TimeTicks(), i), + EXPECT_LT(estimator.GetRTTEstimateInternal(disallowed_observation_sources, + base::TimeTicks(), i), base::TimeDelta::Max()); if (i != 0) { @@ -503,9 +491,10 @@ TEST(NetworkQualityEstimatorTest, ComputedPercentiles) { base::TimeTicks(), i - 1)); // RTT percentiles are in increasing order. - EXPECT_GE( - estimator.GetURLRequestRTTEstimateInternal(base::TimeTicks(), i), - estimator.GetURLRequestRTTEstimateInternal(base::TimeTicks(), i - 1)); + EXPECT_GE(estimator.GetRTTEstimateInternal(disallowed_observation_sources, + base::TimeTicks(), i), + estimator.GetRTTEstimateInternal(disallowed_observation_sources, + base::TimeTicks(), i - 1)); } } } @@ -522,8 +511,6 @@ TEST(NetworkQualityEstimatorTest, ObtainOperatingParams) { variation_params["2G.DefaultMedianRTTMsec"] = "-5"; TestNetworkQualityEstimator estimator(variation_params); - EXPECT_EQ(1U, estimator.downstream_throughput_kbps_observations_.Size()); - EXPECT_EQ(1U, estimator.rtt_observations_.Size()); base::TimeDelta rtt; EXPECT_TRUE(estimator.GetURLRequestRTTEstimate(&rtt)); @@ -536,8 +523,6 @@ TEST(NetworkQualityEstimatorTest, ObtainOperatingParams) { // Simulate network change to Wi-Fi. estimator.SimulateNetworkChangeTo( NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI, "test-1"); - EXPECT_EQ(1U, estimator.downstream_throughput_kbps_observations_.Size()); - EXPECT_EQ(1U, estimator.rtt_observations_.Size()); EXPECT_TRUE(estimator.GetURLRequestRTTEstimate(&rtt)); EXPECT_TRUE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps)); @@ -555,8 +540,6 @@ TEST(NetworkQualityEstimatorTest, ObtainOperatingParams) { // available. estimator.SimulateNetworkChangeTo( NetworkChangeNotifier::ConnectionType::CONNECTION_2G, "test-2"); - EXPECT_EQ(1U, estimator.downstream_throughput_kbps_observations_.Size()); - EXPECT_EQ(0U, estimator.rtt_observations_.Size()); EXPECT_FALSE(estimator.GetURLRequestRTTEstimate(&rtt)); EXPECT_TRUE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps)); @@ -888,18 +871,36 @@ TEST(NetworkQualityEstimatorTest, TestGetMedianRTTSince) { base::TimeDelta::FromMilliseconds(100), now, NetworkQualityEstimator::URL_REQUEST)); - base::TimeDelta rtt; - EXPECT_FALSE(estimator.GetRecentURLRequestRTTMedian( - now + base::TimeDelta::FromSeconds(10), &rtt)); - EXPECT_TRUE(estimator.GetRecentURLRequestRTTMedian(now, &rtt)); - 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); + const struct { + base::TimeTicks start_timestamp; + bool expect_network_quality_available; + base::TimeDelta expected_url_request_rtt; + int32_t expected_downstream_throughput; + } tests[] = { + {now + base::TimeDelta::FromSeconds(10), false, + base::TimeDelta::FromMilliseconds(0), 0}, + {now, true, base::TimeDelta::FromMilliseconds(100), 100}, + {now - base::TimeDelta::FromMicroseconds(500), true, + base::TimeDelta::FromMilliseconds(100), 100}, + + }; + + for (const auto& test : tests) { + base::TimeDelta url_request_rtt; + int32_t downstream_throughput_kbps; + EXPECT_EQ(test.expect_network_quality_available, + estimator.GetRecentURLRequestRTTMedian(test.start_timestamp, + &url_request_rtt)); + EXPECT_EQ(test.expect_network_quality_available, + estimator.GetRecentMedianDownlinkThroughputKbps( + test.start_timestamp, &downstream_throughput_kbps)); + + if (test.expect_network_quality_available) { + EXPECT_EQ(test.expected_url_request_rtt, url_request_rtt); + EXPECT_EQ(test.expected_downstream_throughput, + downstream_throughput_kbps); + } + } } // An external estimate provider that does not have a valid RTT or throughput @@ -1124,8 +1125,8 @@ TEST(NetworkQualityEstimatorTest, TestExternalEstimateProvider) { // observations collected from the HTTP requests. TEST(NetworkQualityEstimatorTest, TestExternalEstimateProviderMergeEstimates) { const base::TimeDelta external_estimate_provider_rtt = - base::TimeDelta::FromMilliseconds(1); - const int32_t external_estimate_provider_downstream_throughput = 100; + base::TimeDelta::FromMilliseconds(10 * 1000); + const int32_t external_estimate_provider_downstream_throughput = 100 * 1000; TestExternalEstimateProvider* test_external_estimate_provider = new TestExternalEstimateProvider( external_estimate_provider_rtt, @@ -1147,9 +1148,6 @@ TEST(NetworkQualityEstimatorTest, TestExternalEstimateProviderMergeEstimates) { EXPECT_TRUE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps)); EXPECT_EQ(external_estimate_provider_downstream_throughput, kbps); - EXPECT_EQ(1U, estimator.rtt_observations_.Size()); - EXPECT_EQ(1U, estimator.downstream_throughput_kbps_observations_.Size()); - TestDelegate test_delegate; TestURLRequestContext context(true); context.set_network_quality_estimator(&estimator); @@ -1160,8 +1158,11 @@ TEST(NetworkQualityEstimatorTest, TestExternalEstimateProviderMergeEstimates) { request->Start(); base::RunLoop().Run(); - EXPECT_EQ(2U, estimator.rtt_observations_.Size()); - EXPECT_EQ(2U, estimator.downstream_throughput_kbps_observations_.Size()); + EXPECT_TRUE(estimator.GetURLRequestRTTEstimate(&rtt)); + EXPECT_NE(external_estimate_provider_rtt, rtt); + + EXPECT_TRUE(estimator.GetDownlinkThroughputKbpsEstimate(&kbps)); + EXPECT_NE(external_estimate_provider_downstream_throughput, kbps); } TEST(NetworkQualityEstimatorTest, TestObservers) { @@ -1202,12 +1203,12 @@ TEST(NetworkQualityEstimatorTest, TestObservers) { EXPECT_EQ(2U, rtt_observer.observations().size()); EXPECT_EQ(2U, throughput_observer.observations().size()); - for (auto observation : rtt_observer.observations()) { + for (const auto& observation : rtt_observer.observations()) { EXPECT_LE(0, observation.rtt_ms); EXPECT_LE(0, (observation.timestamp - then).InMilliseconds()); EXPECT_EQ(NetworkQualityEstimator::URL_REQUEST, observation.source); } - for (auto observation : throughput_observer.observations()) { + for (const auto& observation : throughput_observer.observations()) { EXPECT_LE(0, observation.throughput_kbps); EXPECT_LE(0, (observation.timestamp - then).InMilliseconds()); EXPECT_EQ(NetworkQualityEstimator::URL_REQUEST, observation.source); @@ -1268,6 +1269,8 @@ TEST(NetworkQualityEstimatorTest, MAYBE_TestTCPSocketRTT) { context.Init(); EXPECT_EQ(0U, rtt_observer.observations().size()); + base::TimeDelta rtt; + EXPECT_FALSE(estimator.GetURLRequestRTTEstimate(&rtt)); // Send two requests. Verify that the completion of each request generates at // least one TCP RTT observation. @@ -1294,6 +1297,7 @@ TEST(NetworkQualityEstimatorTest, MAYBE_TestTCPSocketRTT) { before_count_tcp_rtt_observations) << i; } + EXPECT_TRUE(estimator.GetURLRequestRTTEstimate(&rtt)); } } // namespace net