Store network quality samples so we can compute percentiles.

net/base/network_quality_estimator_unittest.cc

Index: net/base/network_quality_estimator_unittest.cc
diff --git a/net/base/network_quality_estimator_unittest.cc b/net/base/network_quality_estimator_unittest.cc
index 82e1bbaf3d23e58ce960121170ad5d917e83e6f9..73a868803ba49f10f29c7d9967d93d76c154cc5d 100644
--- a/net/base/network_quality_estimator_unittest.cc
+++ b/net/base/network_quality_estimator_unittest.cc
@@ -32,12 +32,12 @@ TEST(NetworkQualityEstimatorTest, TestPeakKbpsFastestRTTUpdates) {
   // Enable requests to local host to be used for network quality estimation.
   NetworkQualityEstimator estimator(true);
   {
-    NetworkQuality network_quality = estimator.GetEstimate();
-    EXPECT_EQ(network_quality.fastest_rtt_confidence, 0);
-    EXPECT_EQ(network_quality.peak_throughput_kbps_confidence, 0);
+    NetworkQuality network_quality = estimator.GetPeakEstimate();
+    EXPECT_EQ(network_quality.rtt(), base::TimeDelta());
+    EXPECT_EQ(network_quality.downstream_throughput_kbps(), 0);
   }
 
-  TestDelegate d;
+  TestDelegate test_delegate;
   TestURLRequestContext context(false);
 
   uint64_t min_transfer_size_in_bytes =
@@ -46,37 +46,35 @@ TEST(NetworkQualityEstimatorTest, TestPeakKbpsFastestRTTUpdates) {
       NetworkQualityEstimator::kMinRequestDurationMicroseconds);
 
   scoped_ptr<URLRequest> request(context.CreateRequest(
-      test_server_.GetURL("echo.html"), DEFAULT_PRIORITY, &d));
+      test_server_.GetURL("echo.html"), DEFAULT_PRIORITY, &test_delegate));
   request->Start();
 
   base::RunLoop().Run();
 
   base::PlatformThread::Sleep(request_duration);
 
-  // With smaller transfer, |fastest_rtt| will be updated but not
-  // |peak_throughput_kbps|.
-  estimator.NotifyDataReceived(*(request.get()),
+  // With smaller transfer, RTT will be updated but not the downstream
+  // throughput.
+  estimator.NotifyDataReceived(*(request.get()), min_transfer_size_in_bytes - 1,
                                min_transfer_size_in_bytes - 1);
   {
-    NetworkQuality network_quality = estimator.GetEstimate();
-    EXPECT_GT(network_quality.fastest_rtt_confidence, 0);
-    EXPECT_EQ(network_quality.peak_throughput_kbps_confidence, 0);
+    NetworkQuality network_quality = estimator.GetPeakEstimate();
+    EXPECT_GT(network_quality.rtt(), base::TimeDelta());
+    EXPECT_EQ(network_quality.downstream_throughput_kbps(), 0);
   }
 
-  // With large transfer, both |fastest_rtt| and |peak_throughput_kbps| will be
-  // updated.
-  estimator.NotifyDataReceived(*(request.get()), min_transfer_size_in_bytes);
+  // With large transfer, both RTT and downlink throughput will be updated.
+  estimator.NotifyDataReceived(*(request.get()), min_transfer_size_in_bytes,
+                               min_transfer_size_in_bytes);
   {
-    NetworkQuality network_quality = estimator.GetEstimate();
-    EXPECT_GT(network_quality.fastest_rtt_confidence, 0);
-    EXPECT_GT(network_quality.peak_throughput_kbps_confidence, 0);
-    EXPECT_GE(network_quality.fastest_rtt, request_duration);
-    EXPECT_GT(network_quality.peak_throughput_kbps, uint32_t(0));
+    NetworkQuality network_quality = estimator.GetPeakEstimate();
+    EXPECT_GE(network_quality.rtt(), request_duration);
+    EXPECT_GT(network_quality.downstream_throughput_kbps(), 0);
     EXPECT_LE(
-        network_quality.peak_throughput_kbps,
+        network_quality.downstream_throughput_kbps(),
         min_transfer_size_in_bytes * 8.0 / request_duration.InMilliseconds());
   }
-  EXPECT_EQ(estimator.bytes_read_since_last_connection_change_, true);
+  EXPECT_EQ(estimator.have_received_bytes_since_last_connection_change_, true);
 
   // Check UMA histograms.
   base::HistogramTester histogram_tester;
@@ -88,13 +86,74 @@ TEST(NetworkQualityEstimatorTest, TestPeakKbpsFastestRTTUpdates) {
   histogram_tester.ExpectTotalCount("NQE.PeakKbps.Unknown", 1);
   histogram_tester.ExpectTotalCount("NQE.FastestRTT.Unknown", 1);
   {
-    NetworkQuality network_quality = estimator.GetEstimate();
+    NetworkQuality network_quality = estimator.GetPeakEstimate();
     EXPECT_EQ(estimator.current_connection_type_,
               NetworkChangeNotifier::ConnectionType::CONNECTION_WIFI);
-    EXPECT_EQ(network_quality.fastest_rtt_confidence, 0);
-    EXPECT_EQ(network_quality.peak_throughput_kbps_confidence, 0);
+    EXPECT_EQ(network_quality.rtt(), base::TimeDelta());
+    EXPECT_EQ(network_quality.downstream_throughput_kbps(), 0);
   }
 }
 #endif  // !defined(OS_IOS)
 
+// SpawnedTestServer not supported on iOS (see http://crbug.com/148666).

[mmenke, 2015/06/11 15:11:01]

Can we just use the embedded test server?

[tbansal1, 2015/06/11 19:27:50]

Is it okay if I add a TODO with crbug against myself and do that in a separate
CL?

[mmenke, 2015/06/11 19:51:18]

The embedded test server has a ServeFilesFromDirectory method, so this should be
a trivial change (Unless it has trouble finding the files on Android).  You may
also need to add a slash before echo.html.

[tbansal1, 2015/06/11 21:16:29]

Done.

+#if !defined(OS_IOS)

[mmenke, 2015/06/11 19:51:18]

Rather than if-def-ing out the test, please use:

#if ...
#define MAYBE_BLAH DISABLED_BLAH
#else
#define MAYBE_BLAH BLAH
#endif

Those are much easier to find when one is searching for disabled tests.  Should
fix the previous test, too, if we don't fix this as part of this CL.

[tbansal1, 2015/06/11 21:16:29]

I think the problem was that SpawnedTestServer is not even defined on iOS. So,
it gives compilation error when using this DISABLED_*.

(Apparently DISABLED_* only disables the execution of test. What I was doing was
preventing the compilation also).

+TEST(NetworkQualityEstimatorTest, StoreObservations) {
+  SpawnedTestServer test_server_(
+      SpawnedTestServer::TYPE_HTTP, SpawnedTestServer::kLocalhost,
+      base::FilePath(FILE_PATH_LITERAL("net/data/url_request_unittest")));
+  ASSERT_TRUE(test_server_.Start());
+
+  NetworkQualityEstimator estimator(true);
+  TestDelegate test_delegate;
+  TestURLRequestContext context(false);
+
+  uint64 min_transfer_size_in_bytes =
+      NetworkQualityEstimator::kMinTransferSizeInBytes;
+  base::TimeDelta request_duration = base::TimeDelta::FromMicroseconds(
+      NetworkQualityEstimator::kMinRequestDurationMicroseconds);
+
+  // Push 10 more observations than the maximum buffer size.
+  for (size_t i = 1;
+       i <= NetworkQualityEstimator::ObservationBuffer::kMaximumObservations +
+                10U;
+       ++i) {
+    scoped_ptr<URLRequest> request(context.CreateRequest(
+        test_server_.GetURL("echo.html"), DEFAULT_PRIORITY, &test_delegate));
+    request->Start();
+
+    base::RunLoop().Run();
+
+    base::PlatformThread::Sleep(request_duration);

[mmenke, 2015/06/11 19:51:18]

Do you even need to run the requests here?

[tbansal1, 2015/06/11 21:16:29]

Yes, otherwise the observation will be discarded due to shorter duration.

+
+    estimator.NotifyDataReceived(*(request.get()), min_transfer_size_in_bytes,
+                                 min_transfer_size_in_bytes);
+  }
+
+  EXPECT_EQ(NetworkQualityEstimator::ObservationBuffer::kMaximumObservations,
+            estimator.kbps_observations_.size());
+  EXPECT_EQ(NetworkQualityEstimator::ObservationBuffer::kMaximumObservations,
+            estimator.rtt_msec_observations_.size());
+}
+#endif  // !defined(OS_IOS)
+
+TEST(NetworkQualityEstimatorTest, TestAddObservation) {
+  NetworkQualityEstimator estimator(true);
+
+  EXPECT_EQ(0U, estimator.kbps_observations_.size());
+  estimator.kbps_observations_.AddObservation(0);
+  EXPECT_EQ(1U, estimator.kbps_observations_.size());
+
+  estimator.kbps_observations_.AddObservation(-1);
+  EXPECT_EQ(1U, estimator.kbps_observations_.size());
+
+  estimator.kbps_observations_.AddObservation(INT32_MAX);
+  EXPECT_EQ(2U, estimator.kbps_observations_.size());
+
+  estimator.kbps_observations_.AddObservation(INT32_MIN);
+  EXPECT_EQ(2U, estimator.kbps_observations_.size());
+
+  estimator.kbps_observations_.AddObservation(INT32_MAX + 1);
+  EXPECT_EQ(2U, estimator.kbps_observations_.size());
+}
+
 }  // namespace net