Network Quality Estimator: Remove Network Quality Provider class

Network Quality Provider is the base class for network quality
estimator (NQE). It was originally added as a read-only version
of NQE with the goal of passing it to
consumers outside of //net, and preventing them
from calling internal net-specific APIs. With servicification, this is
no longer needed.

In short term, it also makes it simpler to plumb NQE to
H2 session class (see linked bug) which will be done
in a subsequent CL.

This is purely a refactor CL, and does not introduce any
functionality change.

Change-Id: Ibb5bbf6a06ca02d232fbca0c29022422738694f4
Bug: 823322
Reviewed-on: https://chromium-review.googlesource.com/c/1280860Reviewed-by: Matt Menke <mmenke@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#599721}

net/BUILD.gn

@@ -868,8 +868,6 @@ component("net") {
       "nqe/network_quality_observation.h",
       "nqe/network_quality_observation_source.cc",
       "nqe/network_quality_observation_source.h",
-      "nqe/network_quality_provider.cc",
-      "nqe/network_quality_provider.h",
       "nqe/network_quality_store.cc",
       "nqe/network_quality_store.h",
       "nqe/observation_buffer.cc",

net/http/http_network_session.cc

@@ -56,7 +56,7 @@ std::unique_ptr<ClientSocketPoolManager> CreateSocketPoolManager(
       context.client_socket_factory ? context.client_socket_factory
                                     : ClientSocketFactory::GetDefaultFactory(),
       context.socket_performance_watcher_factory,
-      context.network_quality_provider, context.host_resolver,
+      context.network_quality_estimator, context.host_resolver,
       context.cert_verifier, context.channel_id_service,
       context.transport_security_state, context.cert_transparency_verifier,
       context.ct_policy_enforcer, ssl_session_cache_shard,
@@ -165,7 +165,7 @@ HttpNetworkSession::Context::Context()
       http_auth_handler_factory(nullptr),
       net_log(nullptr),
       socket_performance_watcher_factory(nullptr),
-      network_quality_provider(nullptr),
+      network_quality_estimator(nullptr),
       quic_clock(nullptr),
       quic_random(nullptr),
       quic_crypto_client_stream_factory(

net/http/http_network_session.h

@@ -56,7 +56,7 @@ class HttpProxyClientSocketPool;
 class HttpResponseBodyDrainer;
 class HttpServerProperties;
 class NetLog;
-class NetworkQualityProvider;
+class NetworkQualityEstimator;
 class ProxyResolutionService;
 }  // namespace net
 namespace quic {
@@ -257,7 +257,7 @@ class NET_EXPORT HttpNetworkSession {
     HttpServerProperties* http_server_properties;
     NetLog* net_log;
     SocketPerformanceWatcherFactory* socket_performance_watcher_factory;
-    NetworkQualityProvider* network_quality_provider;
+    NetworkQualityEstimator* network_quality_estimator;
 
     // Source of time for QUIC connections.
     quic::QuicClock* quic_clock;

net/http/http_proxy_client_socket_pool.cc

@@ -23,7 +23,7 @@
 #include "net/http/http_proxy_client_socket_wrapper.h"
 #include "net/log/net_log_source_type.h"
 #include "net/log/net_log_with_source.h"
-#include "net/nqe/network_quality_provider.h"
+#include "net/nqe/network_quality_estimator.h"
 #include "net/socket/client_socket_factory.h"
 #include "net/socket/client_socket_handle.h"
 #include "net/socket/client_socket_pool_base.h"
@@ -193,13 +193,14 @@ int HttpProxyConnectJob::HandleConnectResult(int result) {
 }
 
 HttpProxyClientSocketPool::HttpProxyConnectJobFactory::
-    HttpProxyConnectJobFactory(TransportClientSocketPool* transport_pool,
-                               SSLClientSocketPool* ssl_pool,
-                               NetworkQualityProvider* network_quality_provider,
-                               NetLog* net_log)
+    HttpProxyConnectJobFactory(
+        TransportClientSocketPool* transport_pool,
+        SSLClientSocketPool* ssl_pool,
+        NetworkQualityEstimator* network_quality_estimator,
+        NetLog* net_log)
     : transport_pool_(transport_pool),
       ssl_pool_(ssl_pool),
-      network_quality_provider_(network_quality_provider),
+      network_quality_estimator_(network_quality_estimator),
       ssl_http_rtt_multiplier_(GetInt32Param("ssl_http_rtt_multiplier", 10)),
       non_ssl_http_rtt_multiplier_(
           GetInt32Param("non_ssl_http_rtt_multiplier", 5)),
@@ -247,9 +248,9 @@ HttpProxyClientSocketPool::HttpProxyConnectJobFactory::ConnectionTimeout()
 
 base::TimeDelta HttpProxyClientSocketPool::HttpProxyConnectJobFactory::
     ConnectionTimeoutWithConnectionProperty(bool is_secure_connection) const {
-  if (network_quality_provider_) {
+  if (network_quality_estimator_) {
     base::Optional<base::TimeDelta> http_rtt_estimate =
-        network_quality_provider_->GetHttpRTT();
+        network_quality_estimator_->GetHttpRTT();
     if (http_rtt_estimate) {
       int32_t multiplier = is_secure_connection ? ssl_http_rtt_multiplier_
                                                 : non_ssl_http_rtt_multiplier_;
@@ -285,7 +286,7 @@ HttpProxyClientSocketPool::HttpProxyClientSocketPool(
     int max_sockets_per_group,
     TransportClientSocketPool* transport_pool,
     SSLClientSocketPool* ssl_pool,
-    NetworkQualityProvider* network_quality_provider,
+    NetworkQualityEstimator* network_quality_estimator,
     NetLog* net_log)
     : transport_pool_(transport_pool),
       ssl_pool_(ssl_pool),
@@ -296,7 +297,7 @@ HttpProxyClientSocketPool::HttpProxyClientSocketPool(
             ClientSocketPool::used_idle_socket_timeout(),
             new HttpProxyConnectJobFactory(transport_pool,
                                            ssl_pool,
-                                           network_quality_provider,
+                                           network_quality_estimator,
                                            net_log)) {
   // We should always have a |transport_pool_| except in unit tests.
   if (transport_pool_)

net/http/http_proxy_client_socket_pool.h

@@ -32,7 +32,7 @@ class HttpAuthCache;
 class HttpAuthHandlerFactory;
 class HttpProxyClientSocketWrapper;
 class NetLog;
-class NetworkQualityProvider;
+class NetworkQualityEstimator;
 class QuicStreamFactory;
 class SSLClientSocketPool;
 class SSLSocketParams;
@@ -160,7 +160,7 @@ class NET_EXPORT_PRIVATE HttpProxyClientSocketPool
                             int max_sockets_per_group,
                             TransportClientSocketPool* transport_pool,
                             SSLClientSocketPool* ssl_pool,
-                            NetworkQualityProvider* network_quality_provider,
+                            NetworkQualityEstimator* network_quality_estimator,
                             NetLog* net_log);
 
   ~HttpProxyClientSocketPool() override;
@@ -230,10 +230,11 @@ class NET_EXPORT_PRIVATE HttpProxyClientSocketPool
   class NET_EXPORT_PRIVATE HttpProxyConnectJobFactory
       : public PoolBase::ConnectJobFactory {
    public:
-    HttpProxyConnectJobFactory(TransportClientSocketPool* transport_pool,
-                               SSLClientSocketPool* ssl_pool,
-                               NetworkQualityProvider* network_quality_provider,
-                               NetLog* net_log);
+    HttpProxyConnectJobFactory(
+        TransportClientSocketPool* transport_pool,
+        SSLClientSocketPool* ssl_pool,
+        NetworkQualityEstimator* network_quality_estimator,
+        NetLog* net_log);
 
     // ClientSocketPoolBase::ConnectJobFactory methods.
     std::unique_ptr<ConnectJob> NewConnectJob(
@@ -255,7 +256,7 @@ class NET_EXPORT_PRIVATE HttpProxyClientSocketPool
 
     TransportClientSocketPool* const transport_pool_;
     SSLClientSocketPool* const ssl_pool_;
-    NetworkQualityProvider* const network_quality_provider_;
+    NetworkQualityEstimator* const network_quality_estimator_;
 
     // For secure proxies, the connection timeout is set to
     // |ssl_http_rtt_multiplier_| times the HTTP RTT estimate. For insecure

net/nqe/network_quality_estimator.h

@@ -34,7 +34,6 @@
 #include "net/nqe/network_quality_estimator_params.h"
 #include "net/nqe/network_quality_observation.h"
 #include "net/nqe/network_quality_observation_source.h"
-#include "net/nqe/network_quality_provider.h"
 #include "net/nqe/network_quality_store.h"
 #include "net/nqe/observation_buffer.h"
 #include "net/nqe/rtt_throughput_estimates_observer.h"
@@ -65,8 +64,7 @@ class URLRequest;
 // thereby increasing the single NQE instance's accuracy by providing more
 // observed traffic characteristics.
 class NET_EXPORT NetworkQualityEstimator
-    : public NetworkChangeNotifier::ConnectionTypeObserver,
-      public NetworkQualityProvider {
+    : public NetworkChangeNotifier::ConnectionTypeObserver {
  public:
   // Observes measurements of round trip time.
   class NET_EXPORT_PRIVATE RTTObserver {
@@ -120,17 +118,46 @@ class NET_EXPORT NetworkQualityEstimator
   virtual EffectiveConnectionType GetRecentEffectiveConnectionType(
       const base::TimeTicks& start_time) const;
 
-  // NetworkQualityProvider implementation:
-  // Must be called on the IO thread.
-  EffectiveConnectionType GetEffectiveConnectionType() const override;
+  // Returns the current effective connection type.  The effective connection
+  // type is computed by the network quality estimator at regular intervals and
+  // at certain events (e.g., connection change). Virtualized for testing.
+  virtual EffectiveConnectionType GetEffectiveConnectionType() const;
+
+  // Adds |observer| to a list of effective connection type observers.
+  // The observer must register and unregister itself on the same thread.
+  // |observer| would be notified on the thread on which it registered.
+  // |observer| would be notified of the current effective connection
+  // type in the next message pump.
   void AddEffectiveConnectionTypeObserver(
-      EffectiveConnectionTypeObserver* observer) override;
+      EffectiveConnectionTypeObserver* observer);
+
+  // Removes |observer| from a list of effective connection type observers.
   void RemoveEffectiveConnectionTypeObserver(
-      EffectiveConnectionTypeObserver* observer) override;
-  base::Optional<base::TimeDelta> GetHttpRTT() const override;
-  base::Optional<base::TimeDelta> GetTransportRTT() const override;
-  base::Optional<int32_t> GetDownstreamThroughputKbps() const override;
-  base::Optional<int32_t> GetBandwidthDelayProductKbits() const override;
+      EffectiveConnectionTypeObserver* observer);
+
+  // Returns the current HTTP RTT estimate. If the estimate is unavailable,
+  // the returned optional value is null. The RTT at 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.
+  // Virtualized for testing.
+  virtual base::Optional<base::TimeDelta> GetHttpRTT() const;
+
+  // Returns the current transport RTT estimate. If the estimate is
+  // unavailable, the returned optional value is null.  The RTT at the transport
+  // layer provides an aggregate estimate of the transport RTT as computed by
+  // various underlying TCP and QUIC connections. Virtualized for testing.
+  virtual base::Optional<base::TimeDelta> GetTransportRTT() const;
+
+  // Returns the current downstream throughput estimate (in kilobits per
+  // second). If the estimate is unavailable, the returned optional value is
+  // null.
+  base::Optional<int32_t> GetDownstreamThroughputKbps() const;
+
+  // Returns the current bandwidth delay product estimate (in kilobits). If the
+  // estimate is not available, the returned optional value is null. The
+  // bandwidth delay product is calculated from the transport RTT and the
+  // downlink bandwidth estimates. Virtualized for testing.
+  virtual base::Optional<int32_t> GetBandwidthDelayProductKbits() const;
 
   // Adds |observer| to the list of RTT and throughput estimate observers.
   // The observer must register and unregister itself on the same thread.

net/nqe/network_quality_provider.cc

-// Copyright 2017 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_provider.h"
-
-namespace net {
-
-EffectiveConnectionType NetworkQualityProvider::GetEffectiveConnectionType()
-    const {
-  return EFFECTIVE_CONNECTION_TYPE_UNKNOWN;
-}
-
-base::Optional<base::TimeDelta> NetworkQualityProvider::GetHttpRTT() const {
-  return base::Optional<base::TimeDelta>();
-}
-
-base::Optional<base::TimeDelta> NetworkQualityProvider::GetTransportRTT()
-    const {
-  return base::Optional<base::TimeDelta>();
-}
-
-base::Optional<int32_t> NetworkQualityProvider::GetDownstreamThroughputKbps()
-    const {
-  return base::Optional<int32_t>();
-}
-
-base::Optional<int32_t> NetworkQualityProvider::GetBandwidthDelayProductKbits()
-    const {
-  return base ::Optional<int32_t>();
-}
-
-}  // namespace net

net/nqe/network_quality_provider.h

-// Copyright 2017 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_NQE_NETWORK_QUALITY_PROVIDER_H_
-#define NET_NQE_NETWORK_QUALITY_PROVIDER_H_
-
-#include <stdint.h>
-
-#include "base/compiler_specific.h"
-#include "base/gtest_prod_util.h"
-#include "base/macros.h"
-#include "base/optional.h"
-#include "base/time/time.h"
-#include "net/base/net_export.h"
-#include "net/nqe/effective_connection_type.h"
-
-namespace net {
-
-class EffectiveConnectionTypeObserver;
-
-// Provides simple interface to obtain the network quality, and to listen to
-// the changes in the network quality.
-class NET_EXPORT NetworkQualityProvider {
- public:
-  virtual ~NetworkQualityProvider() {}
-
-  // Returns the current effective connection type.  The effective connection
-  // type is computed by the network quality estimator at regular intervals and
-  // at certain events (e.g., connection change).
-  virtual EffectiveConnectionType GetEffectiveConnectionType() const;
-
-  // Adds |observer| to a list of effective connection type observers.
-  // The observer must register and unregister itself on the same thread.
-  // |observer| would be notified on the thread on which it registered.
-  // |observer| would be notified of the current effective connection
-  // type in the next message pump.
-  virtual void AddEffectiveConnectionTypeObserver(
-      EffectiveConnectionTypeObserver* observer) {}
-
-  // Removes |observer| from a list of effective connection type observers.
-  virtual void RemoveEffectiveConnectionTypeObserver(
-      EffectiveConnectionTypeObserver* observer) {}
-
-  // Returns the current HTTP RTT estimate. If the estimate is unavailable,
-  // the returned optional value is null. The RTT at 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 base::Optional<base::TimeDelta> GetHttpRTT() const;
-
-  // Returns the current transport RTT estimate. If the estimate is
-  // unavailable, the returned optional value is null.  The RTT at the transport
-  // layer provides an aggregate estimate of the transport RTT as computed by
-  // various underlying TCP and QUIC connections.
-  virtual base::Optional<base::TimeDelta> GetTransportRTT() const;
-
-  // Returns the current downstream throughput estimate (in kilobits per
-  // second). If the estimate is unavailable, the returned optional value is
-  // null.
-  virtual base::Optional<int32_t> GetDownstreamThroughputKbps() const;
-
-  // Returns the current bandwidth delay product estimate (in kilobits). If the
-  // estimate is not available, the returned optional value is null. The
-  // bandwidth delay product is calculated from the transport RTT and the
-  // downlink bandwidth estimates.
-  virtual base::Optional<int32_t> GetBandwidthDelayProductKbits() const;
-
- protected:
-  NetworkQualityProvider() {}
-
- private:
-  DISALLOW_COPY_AND_ASSIGN(NetworkQualityProvider);
-};
-
-}  // namespace net
-
-#endif  // NET_NQE_NETWORK_QUALITY_PROVIDER_H_

net/nqe/throughput_analyzer.cc

@@ -13,9 +13,9 @@
 #include "net/base/host_port_pair.h"
 #include "net/base/network_activity_monitor.h"
 #include "net/base/url_util.h"
+#include "net/nqe/network_quality_estimator.h"
 #include "net/nqe/network_quality_estimator_params.h"
 #include "net/nqe/network_quality_estimator_util.h"
-#include "net/nqe/network_quality_provider.h"
 #include "net/url_request/url_request.h"
 #include "net/url_request/url_request_context.h"
 
@@ -42,13 +42,13 @@ namespace nqe {
 namespace internal {
 
 ThroughputAnalyzer::ThroughputAnalyzer(
-    const NetworkQualityProvider* network_quality_provider,
+    const NetworkQualityEstimator* network_quality_estimator,
     const NetworkQualityEstimatorParams* params,
     scoped_refptr<base::SingleThreadTaskRunner> task_runner,
     ThroughputObservationCallback throughput_observation_callback,
     const base::TickClock* tick_clock,
     const NetLogWithSource& net_log)
-    : network_quality_provider_(network_quality_provider),
+    : network_quality_estimator_(network_quality_estimator),
       params_(params),
       task_runner_(task_runner),
       throughput_observation_callback_(throughput_observation_callback),
@@ -60,7 +60,7 @@ ThroughputAnalyzer::ThroughputAnalyzer(
       use_localhost_requests_for_tests_(false),
       net_log_(net_log) {
   DCHECK(tick_clock_);
-  DCHECK(network_quality_provider_);
+  DCHECK(network_quality_estimator_);
   DCHECK(params_);
   DCHECK(task_runner_);
   DCHECK(!IsCurrentlyTrackingThroughput());
@@ -234,7 +234,7 @@ bool ThroughputAnalyzer::IsHangingWindow(int64_t bits_received,
   // Scale the |duration| to one HTTP RTT, and compute the number of bits that
   // would be received over a duration of one HTTP RTT.
   size_t bits_received_over_one_http_rtt =
-      bits_received * (network_quality_provider_->GetHttpRTT()
+      bits_received * (network_quality_estimator_->GetHttpRTT()
                            .value_or(base::TimeDelta::FromSeconds(10))
                            .InMillisecondsF() /
                        duration.InMillisecondsF());
@@ -394,7 +394,7 @@ void ThroughputAnalyzer::EraseHangingRequests(const URLRequest& request) {
   const base::TimeTicks now = tick_clock_->NowTicks();
 
   const base::TimeDelta http_rtt =
-      network_quality_provider_->GetHttpRTT().value_or(
+      network_quality_estimator_->GetHttpRTT().value_or(
           base::TimeDelta::FromSeconds(60));
 
   size_t count_request_erased = 0;

net/nqe/throughput_analyzer.h

@@ -28,7 +28,7 @@ class TickClock;
 namespace net {
 
 class NetworkQualityEstimatorParams;
-class NetworkQualityProvider;
+class NetworkQualityEstimator;
 class URLRequest;
 
 namespace nqe {
@@ -59,7 +59,7 @@ class NET_EXPORT_PRIVATE ThroughputAnalyzer {
   // estimation.
   // Virtualized for testing.
   ThroughputAnalyzer(
-      const NetworkQualityProvider* network_quality_provider,
+      const NetworkQualityEstimator* network_quality_estimator,
       const NetworkQualityEstimatorParams* params,
       scoped_refptr<base::SingleThreadTaskRunner> task_runner,
       ThroughputObservationCallback throughput_observation_callback,
@@ -162,7 +162,7 @@ class NET_EXPORT_PRIVATE ThroughputAnalyzer {
   void BoundRequestsSize();
 
   // Guaranteed to be non-null during the duration of |this|.
-  const NetworkQualityProvider* network_quality_provider_;
+  const NetworkQualityEstimator* network_quality_estimator_;
 
   // Guaranteed to be non-null during the duration of |this|.
   const NetworkQualityEstimatorParams* params_;

net/socket/client_socket_pool_manager_impl.cc

@@ -42,7 +42,7 @@ ClientSocketPoolManagerImpl::ClientSocketPoolManagerImpl(
     NetLog* net_log,
     ClientSocketFactory* socket_factory,
     SocketPerformanceWatcherFactory* socket_performance_watcher_factory,
-    NetworkQualityProvider* network_quality_provider,
+    NetworkQualityEstimator* network_quality_estimator,
     HostResolver* host_resolver,
     CertVerifier* cert_verifier,
     ChannelIDService* channel_id_service,
@@ -56,7 +56,7 @@ ClientSocketPoolManagerImpl::ClientSocketPoolManagerImpl(
     : net_log_(net_log),
       socket_factory_(socket_factory),
       socket_performance_watcher_factory_(socket_performance_watcher_factory),
-      network_quality_provider_(network_quality_provider),
+      network_quality_estimator_(network_quality_estimator),
       host_resolver_(host_resolver),
       cert_verifier_(cert_verifier),
       channel_id_service_(channel_id_service),
@@ -304,7 +304,7 @@ ClientSocketPoolManagerImpl::GetSocketPoolForHTTPProxy(
                           sockets_per_proxy_server, sockets_per_group,
                           tcp_http_ret.first->second.get(),
                           ssl_https_ret.first->second.get(),
-                          network_quality_provider_, net_log_)));
+                          network_quality_estimator_, net_log_)));
 
   return ret.first->second.get();
 }

net/socket/client_socket_pool_manager_impl.h

@@ -34,7 +34,7 @@ class CTVerifier;
 class HttpProxyClientSocketPool;
 class HostResolver;
 class NetLog;
-class NetworkQualityProvider;
+class NetworkQualityEstimator;
 class SocketPerformanceWatcherFactory;
 class SOCKSClientSocketPool;
 class SSLClientSocketPool;
@@ -51,7 +51,7 @@ class NET_EXPORT_PRIVATE ClientSocketPoolManagerImpl
       NetLog* net_log,
       ClientSocketFactory* socket_factory,
       SocketPerformanceWatcherFactory* socket_performance_watcher_factory,
-      NetworkQualityProvider* network_quality_provider,
+      NetworkQualityEstimator* network_quality_estimator,
       HostResolver* host_resolver,
       CertVerifier* cert_verifier,
       ChannelIDService* channel_id_service,
@@ -103,7 +103,7 @@ class NET_EXPORT_PRIVATE ClientSocketPoolManagerImpl
   NetLog* const net_log_;
   ClientSocketFactory* const socket_factory_;
   SocketPerformanceWatcherFactory* socket_performance_watcher_factory_;
-  NetworkQualityProvider* network_quality_provider_;
+  NetworkQualityEstimator* network_quality_estimator_;
   HostResolver* const host_resolver_;
   CertVerifier* const cert_verifier_;
   ChannelIDService* const channel_id_service_;

net/url_request/url_request_context_builder.cc

@@ -243,7 +243,7 @@ void URLRequestContextBuilder::SetHttpNetworkSessionComponents(
       request_context->http_server_properties();
   session_context->net_log = request_context->net_log();
   session_context->channel_id_service = request_context->channel_id_service();
-  session_context->network_quality_provider =
+  session_context->network_quality_estimator =
       request_context->network_quality_estimator();
   if (request_context->network_quality_estimator()) {
     session_context->socket_performance_watcher_factory =