Add a NetworkQualityEstimator to TestURLRequestContext.

net/nqe/network_quality_estimator_unittest.cc

 // 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.
 
 #include "net/nqe/network_quality_estimator.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <limits>
@@ skipping 151 matching lines @@
   histogram_tester.ExpectUniqueSample("NQE.CachedNetworkQualityAvailable",
                                       false, 1);
 
   base::TimeDelta rtt;
   int32_t kbps;
   EXPECT_FALSE(estimator.GetRecentHttpRTT(base::TimeTicks(), &rtt));
   EXPECT_FALSE(
       estimator.GetRecentDownlinkThroughputKbps(base::TimeTicks(), &kbps));
 
   TestDelegate test_delegate;
-  TestURLRequestContext context(true);
+  // Theses tests expect the HttpNetworkSession not to use this NQE, but the
+  // higher layers to do so, so create the context, with its own NQE, and then
+  // just set the one on the URLRequestContext, not the one used by the
+  // HttpNetworkSession.
+  // TODO(mmenke):  Fix that?

[tbansal1, 2016/11/14 20:39:27]

can you reassign these to me. As mentioned elsewhere, I will change the tests so
that they override NQE in the params too.

[mmenke, 2016/11/14 20:43:02]

Do you want to do it, or would you rather me do it?  This comment implies the
former, "If you have problems fixing NQE tests, please let me know" implies the
former.

I'm happy to just update expectations according to the errors I get with setting
the NQE in the params as well (I'm reluctant to dig further into the code and
figure if those updates are reasonable, just because I don't think there's much
benefit in me learning the low level details of how NQE works - the higher level
details of how it's hooked into URLRequest may be useful to me in the future,
but not the socket layer stuff).

+  TestURLRequestContext context;
   context.set_network_quality_estimator(&estimator);
-  context.Init();
 
   std::unique_ptr<URLRequest> request(context.CreateRequest(
       estimator.GetEchoURL(), DEFAULT_PRIORITY, &test_delegate));
   request->SetLoadFlags(request->load_flags() | LOAD_MAIN_FRAME_DEPRECATED);
   request->Start();
   base::RunLoop().Run();
 
   // Both RTT and downstream throughput should be updated.
   EXPECT_TRUE(estimator.GetRecentHttpRTT(base::TimeTicks(), &rtt));
   EXPECT_TRUE(
@@ skipping 115 matching lines @@
   histogram_tester.ExpectUniqueSample("NQE.CachedNetworkQualityAvailable",
                                       false, 1);
 
   base::TimeDelta rtt;
   int32_t kbps;
   EXPECT_FALSE(estimator.GetRecentHttpRTT(base::TimeTicks(), &rtt));
   EXPECT_FALSE(
       estimator.GetRecentDownlinkThroughputKbps(base::TimeTicks(), &kbps));
 
   TestDelegate test_delegate;
-  TestURLRequestContext context(true);
+  // Theses tests expect the HttpNetworkSession not to use this NQE, but the
+  // higher layers to do so, so create the context, with its own NQE, and then
+  // just set the one on the URLRequestContext, not the one used by the
+  // HttpNetworkSession.
+  // TODO(mmenke):  Fix that?
+  TestURLRequestContext context;
   context.set_network_quality_estimator(&estimator);
-  context.Init();
 
   // Start two requests so that the network quality is added to cache store at
   // the beginning of the second request from the network traffic observed from
   // the first request.
   for (size_t i = 0; i < 2; ++i) {
     std::unique_ptr<URLRequest> request(context.CreateRequest(
         estimator.GetEchoURL(), DEFAULT_PRIORITY, &test_delegate));
     request->SetLoadFlags(request->load_flags() | LOAD_MAIN_FRAME_DEPRECATED);
     request->Start();
     base::RunLoop().Run();
@@ skipping 38 matching lines @@
   std::map<std::string, std::string> variation_params;
   TestNetworkQualityEstimator estimator(variation_params);
 
   base::TimeDelta rtt;
   int32_t kbps;
   EXPECT_FALSE(estimator.GetRecentHttpRTT(base::TimeTicks(), &rtt));
   EXPECT_FALSE(
       estimator.GetRecentDownlinkThroughputKbps(base::TimeTicks(), &kbps));
 
   TestDelegate test_delegate;
-  TestURLRequestContext context(true);
+  // Theses tests expect the HttpNetworkSession not to use this NQE, but the
+  // higher layers to do so, so create the context, with its own NQE, and then
+  // just set the one on the URLRequestContext, not the one used by the
+  // HttpNetworkSession.
+  // TODO(mmenke):  Fix that?
+  TestURLRequestContext context;
   context.set_network_quality_estimator(&estimator);
-  context.Init();
 
   // Push more observations than the maximum buffer size.
   const size_t kMaxObservations = 1000;
   for (size_t i = 0; i < kMaxObservations; ++i) {
     std::unique_ptr<URLRequest> request(context.CreateRequest(
         estimator.GetEchoURL(), DEFAULT_PRIORITY, &test_delegate));
     request->Start();
     base::RunLoop().Run();
     EXPECT_TRUE(estimator.GetRecentHttpRTT(base::TimeTicks(), &rtt));
     EXPECT_TRUE(
@@ skipping 22 matching lines @@
       NETWORK_QUALITY_OBSERVATION_SOURCE_QUIC);
 
   EXPECT_EQ(nqe::internal::InvalidRTT(),
             estimator.GetRTTEstimateInternal(disallowed_observation_sources,
                                              base::TimeTicks(), 100));
   EXPECT_EQ(nqe::internal::kInvalidThroughput,
             estimator.GetDownlinkThroughputKbpsEstimateInternal(
                 base::TimeTicks(), 100));
 
   TestDelegate test_delegate;
-  TestURLRequestContext context(true);
+  // Theses tests expect the HttpNetworkSession not to use this NQE, but the
+  // higher layers to do so, so create the context, with its own NQE, and then
+  // just set the one on the URLRequestContext, not the one used by the
+  // HttpNetworkSession.
+  // TODO(mmenke):  Fix that?
+  TestURLRequestContext context;
   context.set_network_quality_estimator(&estimator);
-  context.Init();
 
   // Number of observations are more than the maximum buffer size.
   for (size_t i = 0; i < 1000U; ++i) {
     std::unique_ptr<URLRequest> request(context.CreateRequest(
         estimator.GetEchoURL(), DEFAULT_PRIORITY, &test_delegate));
     request->Start();
     base::RunLoop().Run();
   }
 
   // Verify the percentiles through simple tests.
@@ skipping 872 matching lines @@
   // provided by external estimate provider.
   EXPECT_TRUE(estimator.GetRecentHttpRTT(base::TimeTicks(), &rtt));
   EXPECT_EQ(external_estimate_provider_rtt, rtt);
 
   int32_t kbps;
   EXPECT_TRUE(
       estimator.GetRecentDownlinkThroughputKbps(base::TimeTicks(), &kbps));
   EXPECT_EQ(external_estimate_provider_downstream_throughput, kbps);
 
   TestDelegate test_delegate;
-  TestURLRequestContext context(true);
+  // Theses tests expect the HttpNetworkSession not to use this NQE, but the
+  // higher layers to do so, so create the context, with its own NQE, and then
+  // just set the one on the URLRequestContext, not the one used by the
+  // HttpNetworkSession.
+  // TODO(mmenke):  Fix that?
+  TestURLRequestContext context;
   context.set_network_quality_estimator(&estimator);
-  context.Init();
 
   std::unique_ptr<URLRequest> request(context.CreateRequest(
       estimator.GetEchoURL(), DEFAULT_PRIORITY, &test_delegate));
   request->Start();
   base::RunLoop().Run();
 
   EXPECT_TRUE(estimator.GetRecentHttpRTT(base::TimeTicks(), &rtt));
   EXPECT_NE(external_estimate_provider_rtt, rtt);
 
   EXPECT_TRUE(
@@ skipping 24 matching lines @@
         test.allow_small_localhost_requests,
         test.allow_small_localhost_requests);
 
     base::TimeDelta rtt;
     EXPECT_FALSE(estimator.GetRecentHttpRTT(base::TimeTicks(), &rtt));
     int32_t kbps;
     EXPECT_FALSE(
         estimator.GetRecentDownlinkThroughputKbps(base::TimeTicks(), &kbps));
 
     TestDelegate test_delegate;
-    TestURLRequestContext context(true);
+    // Theses tests expect the HttpNetworkSession not to use this NQE, but the
+    // higher layers to do so, so create the context, with its own NQE, and then
+    // just set the one on the URLRequestContext, not the one used by the
+    // HttpNetworkSession.
+    // TODO(mmenke):  Fix that?
+    TestURLRequestContext context;
     context.set_network_quality_estimator(&estimator);
-    context.Init();
 
     std::unique_ptr<URLRequest> request(context.CreateRequest(
         estimator.GetEchoURL(), DEFAULT_PRIORITY, &test_delegate));
     request->SetLoadFlags(request->load_flags() | LOAD_MAIN_FRAME_DEPRECATED);
     request->Start();
     base::RunLoop().Run();
 
     EXPECT_EQ(test.allow_small_localhost_requests,
               estimator.GetRecentHttpRTT(base::TimeTicks(), &rtt));
     EXPECT_EQ(
@@ skipping 10 matching lines @@
       new base::SimpleTestTickClock());
   base::SimpleTestTickClock* tick_clock_ptr = tick_clock.get();
 
   TestEffectiveConnectionTypeObserver observer;
   std::map<std::string, std::string> variation_params;
   TestNetworkQualityEstimator estimator(variation_params);
   estimator.AddEffectiveConnectionTypeObserver(&observer);
   estimator.SetTickClockForTesting(std::move(tick_clock));
 
   TestDelegate test_delegate;
-  TestURLRequestContext context(true);
+  // Theses tests expect the HttpNetworkSession not to use this NQE, but the
+  // higher layers to do so, so create the context, with its own NQE, and then
+  // just set the one on the URLRequestContext, not the one used by the
+  // HttpNetworkSession.
+  // TODO(mmenke):  Fix that?
+  TestURLRequestContext context;
   context.set_network_quality_estimator(&estimator);
-  context.Init();
 
   EXPECT_EQ(0U, observer.effective_connection_types().size());
 
   estimator.set_start_time_null_http_rtt(
       base::TimeDelta::FromMilliseconds(1500));
   estimator.set_start_time_null_downlink_throughput_kbps(100000);
 
   tick_clock_ptr->Advance(base::TimeDelta::FromMinutes(60));
 
   std::unique_ptr<URLRequest> request(context.CreateRequest(
@@ skipping 40 matching lines @@
       new base::SimpleTestTickClock());
   base::SimpleTestTickClock* tick_clock_ptr = tick_clock.get();
 
   TestRTTAndThroughputEstimatesObserver observer;
   std::map<std::string, std::string> variation_params;
   TestNetworkQualityEstimator estimator(variation_params);
   estimator.AddRTTAndThroughputEstimatesObserver(&observer);
   estimator.SetTickClockForTesting(std::move(tick_clock));
 
   TestDelegate test_delegate;
-  TestURLRequestContext context(true);
+  // Theses tests expect the HttpNetworkSession not to use this NQE, but the
+  // higher layers to do so, so create the context, with its own NQE, and then
+  // just set the one on the URLRequestContext, not the one used by the
+  // HttpNetworkSession.
+  // TODO(mmenke):  Fix that?
+  TestURLRequestContext context;
   context.set_network_quality_estimator(&estimator);
-  context.Init();
 
   EXPECT_EQ(nqe::internal::InvalidRTT(), observer.http_rtt());
   EXPECT_EQ(nqe::internal::InvalidRTT(), observer.transport_rtt());
   EXPECT_EQ(nqe::internal::kInvalidThroughput,
             observer.downstream_throughput_kbps());
   int notifications_received = observer.notifications_received();
   EXPECT_EQ(0, notifications_received);
 
   base::TimeDelta http_rtt(base::TimeDelta::FromMilliseconds(100));
   base::TimeDelta transport_rtt(base::TimeDelta::FromMilliseconds(200));
@@ skipping 98 matching lines @@
 
   TestEffectiveConnectionTypeObserver observer;
   std::map<std::string, std::string> variation_params;
   TestNetworkQualityEstimator estimator(variation_params);
   estimator.SetTickClockForTesting(std::move(tick_clock));
   estimator.SimulateNetworkChange(NetworkChangeNotifier::CONNECTION_WIFI,
                                   "test");
   estimator.AddEffectiveConnectionTypeObserver(&observer);
 
   TestDelegate test_delegate;
-  TestURLRequestContext context(true);
+  // Theses tests expect the HttpNetworkSession not to use this NQE, but the
+  // higher layers to do so, so create the context, with its own NQE, and then
+  // just set the one on the URLRequestContext, not the one used by the
+  // HttpNetworkSession.
+  // TODO(mmenke):  Fix that?
+  TestURLRequestContext context;
   context.set_network_quality_estimator(&estimator);
-  context.Init();
 
   EXPECT_EQ(0U, observer.effective_connection_types().size());
 
   estimator.set_recent_effective_connection_type(EFFECTIVE_CONNECTION_TYPE_2G);
   tick_clock_ptr->Advance(base::TimeDelta::FromMinutes(60));
 
   std::unique_ptr<URLRequest> request(context.CreateRequest(
       estimator.GetEchoURL(), DEFAULT_PRIORITY, &test_delegate));
   request->SetLoadFlags(request->load_flags() | LOAD_MAIN_FRAME_DEPRECATED);
   request->Start();
@@ skipping 59 matching lines @@
 
 TEST(NetworkQualityEstimatorTest, TestRttThroughputObservers) {
   TestRTTObserver rtt_observer;
   TestThroughputObserver throughput_observer;
   std::map<std::string, std::string> variation_params;
   TestNetworkQualityEstimator estimator(variation_params);
   estimator.AddRTTObserver(&rtt_observer);
   estimator.AddThroughputObserver(&throughput_observer);
 
   TestDelegate test_delegate;
-  TestURLRequestContext context(true);
+  // Theses tests expect the HttpNetworkSession not to use this NQE, but the
+  // higher layers to do so, so create the context, with its own NQE, and then
+  // just set the one on the URLRequestContext, not the one used by the
+  // HttpNetworkSession.
+  // TODO(mmenke):  Fix that?
+  TestURLRequestContext context;
   context.set_network_quality_estimator(&estimator);
-  context.Init();
 
   EXPECT_EQ(0U, rtt_observer.observations().size());
   EXPECT_EQ(0U, throughput_observer.observations().size());
   base::TimeTicks then = base::TimeTicks::Now();
 
   std::unique_ptr<URLRequest> request(context.CreateRequest(
       estimator.GetEchoURL(), DEFAULT_PRIORITY, &test_delegate));
   request->SetLoadFlags(request->load_flags() | LOAD_MAIN_FRAME_DEPRECATED);
   request->Start();
   base::RunLoop().Run();
@@ skipping 650 matching lines @@
     if (expected_count == 1) {
       EffectiveConnectionType last_notified_type =
           observer.effective_connection_types().at(
               observer.effective_connection_types().size() - 1);
       EXPECT_EQ(i, last_notified_type);
     }
   }
 }
 
 }  // namespace net