Skip to content

Commit

Permalink
Make NQE a derived class of NetworkQualityProvider
Browse files Browse the repository at this point in the history
NetworkQualityProvider provides functions to get network quality
metrics, and to listen to changes in the network quality.

This CL changes Network Quality Estimator (NQE) to be an implementation
of Network Quality Provider (NQP).

Long term, this allows callers to depend only on NQP when they only
need to read the network quality, while the heavy-weight NQE can be
used in other cases.

BUG=704339
CQ_INCLUDE_TRYBOTS=master.tryserver.chromium.android:android_cronet_tester
TBR=petewil@chromium.org

Review-Url: https://codereview.chromium.org/2927453002
Cr-Commit-Position: refs/heads/master@{#477452}
  • Loading branch information
tbansal authored and Commit Bot committed Jun 6, 2017
1 parent 7cdb0ae commit 1bd4a95
Show file tree
Hide file tree
Showing 20 changed files with 316 additions and 258 deletions.
4 changes: 2 additions & 2 deletions chrome/browser/io_thread.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ class HttpAuthPreferences;
class LoggingNetworkChangeObserver;
class NetworkQualityEstimator;
class ProxyConfigService;
class RTTAndThroughputEstimatesObserver;
class SSLConfigService;
class URLRequestContext;
class URLRequestContextGetter;
Expand Down Expand Up @@ -154,8 +155,7 @@ class IOThread : public content::BrowserThreadDelegate {
#endif
std::unique_ptr<net::HostMappingRules> host_mapping_rules;
std::unique_ptr<net::NetworkQualityEstimator> network_quality_estimator;
std::unique_ptr<
net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver>
std::unique_ptr<net::RTTAndThroughputEstimatesObserver>
network_quality_observer;

// NetErrorTabHelper uses |dns_probe_service| to send DNS probes when a
Expand Down
20 changes: 10 additions & 10 deletions chrome/browser/net/nqe/ui_network_quality_estimator_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,10 @@
#include "components/prefs/pref_service.h"
#include "components/variations/variations_associated_data.h"
#include "content/public/browser/browser_thread.h"
#include "net/nqe/effective_connection_type_observer.h"
#include "net/nqe/network_qualities_prefs_manager.h"
#include "net/nqe/network_quality.h"
#include "net/nqe/rtt_throughput_estimates_observer.h"

namespace {

Expand Down Expand Up @@ -81,8 +83,8 @@ void SetNQEOnIOThread(net::NetworkQualitiesPrefsManager* prefs_manager,
// to the UI service.
// It is created on the UI thread, but used and deleted on the IO thread.
class UINetworkQualityEstimatorService::IONetworkQualityObserver
: public net::NetworkQualityEstimator::EffectiveConnectionTypeObserver,
public net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver {
: public net::EffectiveConnectionTypeObserver,
public net::RTTAndThroughputEstimatesObserver {
public:
explicit IONetworkQualityObserver(
base::WeakPtr<UINetworkQualityEstimatorService> service)
Expand Down Expand Up @@ -221,7 +223,7 @@ void UINetworkQualityEstimatorService::RTTOrThroughputComputed(
}

void UINetworkQualityEstimatorService::AddEffectiveConnectionTypeObserver(
net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer) {
net::EffectiveConnectionTypeObserver* observer) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
effective_connection_type_observer_list_.AddObserver(observer);

Expand All @@ -235,7 +237,7 @@ void UINetworkQualityEstimatorService::AddEffectiveConnectionTypeObserver(
}

void UINetworkQualityEstimatorService::RemoveEffectiveConnectionTypeObserver(
net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer) {
net::EffectiveConnectionTypeObserver* observer) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
effective_connection_type_observer_list_.RemoveObserver(observer);
}
Expand Down Expand Up @@ -268,7 +270,7 @@ UINetworkQualityEstimatorService::GetDownstreamThroughputKbps() const {
}

void UINetworkQualityEstimatorService::AddRTTAndThroughputEstimatesObserver(
net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer) {
net::RTTAndThroughputEstimatesObserver* observer) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
rtt_throughput_observer_list_.AddObserver(observer);

Expand All @@ -284,7 +286,7 @@ void UINetworkQualityEstimatorService::AddRTTAndThroughputEstimatesObserver(
// Removes |observer| from the list of RTT and throughput estimate observers.
// Must be called on the IO thread.
void UINetworkQualityEstimatorService::RemoveRTTAndThroughputEstimatesObserver(
net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer) {
net::RTTAndThroughputEstimatesObserver* observer) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
rtt_throughput_observer_list_.RemoveObserver(observer);
}
Expand All @@ -310,8 +312,7 @@ void UINetworkQualityEstimatorService::ClearPrefs() {

void UINetworkQualityEstimatorService::
NotifyEffectiveConnectionTypeObserverIfPresent(
net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer)
const {
net::EffectiveConnectionTypeObserver* observer) const {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

if (!effective_connection_type_observer_list_.HasObserver(observer))
Expand All @@ -322,8 +323,7 @@ void UINetworkQualityEstimatorService::
}

void UINetworkQualityEstimatorService::NotifyRTTAndThroughputObserverIfPresent(
net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer)
const {
net::RTTAndThroughputEstimatesObserver* observer) const {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

if (!rtt_throughput_observer_list_.HasObserver(observer))
Expand Down
38 changes: 10 additions & 28 deletions chrome/browser/net/nqe/ui_network_quality_estimator_service.h
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ class PrefRegistrySimple;
class Profile;

namespace net {
class EffectiveConnectionTypeObserver;
class NetworkQualitiesPrefsManager;
class RTTAndThroughputEstimatesObserver;
}

// UI service to determine the current EffectiveConnectionType.
Expand All @@ -38,33 +40,17 @@ class UINetworkQualityEstimatorService
// NetworkQualityProvider implementation:
// Must be called on the UI thread.
net::EffectiveConnectionType GetEffectiveConnectionType() const override;
// Must be called on the UI thread. |observer| will be notified on the UI
// thread. |observer| would be notified of the current effective connection
// type in the next message pump.
void AddEffectiveConnectionTypeObserver(
net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer)
override;
// Must be called on the UI thread.
net::EffectiveConnectionTypeObserver* observer) override;
void RemoveEffectiveConnectionTypeObserver(
net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer)
override;
net::EffectiveConnectionTypeObserver* observer) override;
base::Optional<base::TimeDelta> GetHttpRTT() const override;
base::Optional<base::TimeDelta> GetTransportRTT() const override;
base::Optional<int32_t> GetDownstreamThroughputKbps() const override;

// Must be called on the UI thread. |observer| will be notified on the UI
// thread. |observer| would be notified of the changes in the HTTP RTT,
// transport RTT or throughput. |observer| would be notified of the current
// values in the next message pump.
void AddRTTAndThroughputEstimatesObserver(
net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer)
override;

// Removes |observer| from the list of RTT and throughput estimate observers.
// Must be called on the UI thread.
net::RTTAndThroughputEstimatesObserver* observer) override;
void RemoveRTTAndThroughputEstimatesObserver(
net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer)
override;
net::RTTAndThroughputEstimatesObserver* observer) override;

// Registers the profile-specific network quality estimator prefs.
static void RegisterProfilePrefs(PrefRegistrySimple* registry);
Expand All @@ -88,14 +74,12 @@ class UINetworkQualityEstimatorService
// Notifies |observer| of the current effective connection type if |observer|
// is still registered as an observer.
void NotifyEffectiveConnectionTypeObserverIfPresent(
net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer)
const;
net::EffectiveConnectionTypeObserver* observer) const;

// Notifies |observer| of the current effective connection type if |observer|
// is still registered as an observer.
void NotifyRTTAndThroughputObserverIfPresent(
net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer)
const;
net::RTTAndThroughputEstimatesObserver* observer) const;

// KeyedService implementation:
void Shutdown() override;
Expand Down Expand Up @@ -125,13 +109,11 @@ class UINetworkQualityEstimatorService
std::unique_ptr<IONetworkQualityObserver> io_observer_;

// Observer list for changes in effective connection type.
base::ObserverList<
net::NetworkQualityEstimator::EffectiveConnectionTypeObserver>
base::ObserverList<net::EffectiveConnectionTypeObserver>
effective_connection_type_observer_list_;

// Observer list for changes in RTT or throughput values.
base::ObserverList<
net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver>
base::ObserverList<net::RTTAndThroughputEstimatesObserver>
rtt_throughput_observer_list_;

// Prefs manager that is owned by this service. Created on the UI thread, but
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,23 +23,24 @@
#include "net/base/network_change_notifier.h"
#include "net/nqe/cached_network_quality.h"
#include "net/nqe/effective_connection_type.h"
#include "net/nqe/effective_connection_type_observer.h"
#include "net/nqe/network_id.h"
#include "net/nqe/network_quality_estimator.h"
#include "net/nqe/rtt_throughput_estimates_observer.h"
#include "testing/gtest/include/gtest/gtest.h"

class Profile;

namespace {

class TestEffectiveConnectionTypeObserver
: public net::NetworkQualityEstimator::EffectiveConnectionTypeObserver {
: public net::EffectiveConnectionTypeObserver {
public:
TestEffectiveConnectionTypeObserver()
: effective_connection_type_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN) {}
~TestEffectiveConnectionTypeObserver() override {}

// net::NetworkQualityEstimator::EffectiveConnectionTypeObserver
// implementation:
// net::EffectiveConnectionTypeObserver implementation:
void OnEffectiveConnectionTypeChanged(
net::EffectiveConnectionType type) override {
effective_connection_type_ = type;
Expand All @@ -57,16 +58,15 @@ class TestEffectiveConnectionTypeObserver
};

class TestRTTAndThroughputEstimatesObserver
: public net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver {
: public net::RTTAndThroughputEstimatesObserver {
public:
TestRTTAndThroughputEstimatesObserver()
: http_rtt_(base::TimeDelta::FromMilliseconds(-1)),
transport_rtt_(base::TimeDelta::FromMilliseconds(-1)),
downstream_throughput_kbps_(-1) {}
~TestRTTAndThroughputEstimatesObserver() override {}

// net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver
// implementation:
// net::RTTAndThroughputEstimatesObserver implementation:
void OnRTTOrThroughputEstimatesComputed(
base::TimeDelta http_rtt,
base::TimeDelta transport_rtt,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
#include "chrome/test/base/testing_browser_process.h"
#include "components/ukm/test_ukm_recorder.h"
#include "components/ukm/ukm_source.h"
#include "net/nqe/effective_connection_type.h"
#include "net/nqe/network_quality_provider.h"
#include "testing/gmock/include/gmock/gmock.h"

using testing::AnyNumber;
Expand All @@ -23,25 +25,12 @@ namespace {
const char kTestUrl1[] = "https://www.google.com/";
const char kTestUrl2[] = "https://www.example.com/";

class MockNetworkQualityProvider
: public net::NetworkQualityEstimator::NetworkQualityProvider {
class MockNetworkQualityProvider : public net::NetworkQualityProvider {
public:
MOCK_CONST_METHOD0(GetEffectiveConnectionType,
net::EffectiveConnectionType());
MOCK_CONST_METHOD0(GetHttpRTT, base::Optional<base::TimeDelta>());
MOCK_CONST_METHOD0(GetTransportRTT, base::Optional<base::TimeDelta>());
MOCK_METHOD1(
AddEffectiveConnectionTypeObserver,
void(net::NetworkQualityEstimator::EffectiveConnectionTypeObserver*));
MOCK_METHOD1(
RemoveEffectiveConnectionTypeObserver,
void(net::NetworkQualityEstimator::EffectiveConnectionTypeObserver*));
MOCK_METHOD1(
AddRTTAndThroughputEstimatesObserver,
void(net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver*));
MOCK_METHOD1(
RemoveRTTAndThroughputEstimatesObserver,
void(net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver*));
};

} // namespace
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
#include "base/threading/thread.h"
#include "components/prefs/json_pref_store.h"
#include "net/nqe/effective_connection_type.h"
#include "net/nqe/effective_connection_type_observer.h"
#include "net/nqe/network_quality_estimator.h"
#include "net/nqe/network_quality_observation_source.h"
#include "net/nqe/rtt_throughput_estimates_observer.h"

class PrefService;

Expand Down Expand Up @@ -48,8 +50,8 @@ bool CronetUrlRequestContextAdapterRegisterJni(JNIEnv* env);

// Adapter between Java CronetUrlRequestContext and net::URLRequestContext.
class CronetURLRequestContextAdapter
: public net::NetworkQualityEstimator::EffectiveConnectionTypeObserver,
public net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver,
: public net::EffectiveConnectionTypeObserver,
public net::RTTAndThroughputEstimatesObserver,
public net::NetworkQualityEstimator::RTTObserver,
public net::NetworkQualityEstimator::ThroughputObserver {
public:
Expand Down
3 changes: 2 additions & 1 deletion components/metrics/net/network_metrics_provider.cc
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "base/threading/thread_task_runner_handle.h"
#include "build/build_config.h"
#include "net/base/net_errors.h"
#include "net/nqe/effective_connection_type_observer.h"
#include "net/nqe/network_quality_estimator.h"

#if defined(OS_CHROMEOS)
Expand All @@ -45,7 +46,7 @@ void GetAndSetNetworkQualityEstimator(

// Listens to the changes in the effective conection type.
class NetworkMetricsProvider::EffectiveConnectionTypeObserver
: public net::NetworkQualityEstimator::EffectiveConnectionTypeObserver {
: public net::EffectiveConnectionTypeObserver {
public:
// |network_quality_estimator| is used to provide the network quality
// estimates. Guaranteed to be non-null. |callback| is run on
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,6 @@ void NetworkQualityProviderStub::SetUserData(
supports_user_data->SetUserData(&kOfflineNQPKey, std::move(stub));
}

void NetworkQualityProviderStub::AddEffectiveConnectionTypeObserver(
net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer) {}

void NetworkQualityProviderStub::RemoveEffectiveConnectionTypeObserver(
net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer) {}

void NetworkQualityProviderStub::AddRTTAndThroughputEstimatesObserver(
net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer) {
}

void NetworkQualityProviderStub::RemoveRTTAndThroughputEstimatesObserver(
net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer) {
}

net::EffectiveConnectionType
NetworkQualityProviderStub::GetEffectiveConnectionType() const {
return connection_type_;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,14 @@

#include "base/supports_user_data.h"
#include "net/nqe/effective_connection_type.h"
#include "net/nqe/network_quality_estimator.h"
#include "net/nqe/network_quality_provider.h"

namespace offline_pages {

// Test class stubbing out the functionality of NQE::NetworkQualityProvider.
// It is only used for test support.
class NetworkQualityProviderStub
: public net::NetworkQualityEstimator::NetworkQualityProvider,
public base::SupportsUserData::Data {
class NetworkQualityProviderStub : public net::NetworkQualityProvider,
public base::SupportsUserData::Data {
public:
NetworkQualityProviderStub();
~NetworkQualityProviderStub() override;
Expand All @@ -27,22 +26,6 @@ class NetworkQualityProviderStub

net::EffectiveConnectionType GetEffectiveConnectionType() const override;

void AddEffectiveConnectionTypeObserver(
net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer)
override;

void RemoveEffectiveConnectionTypeObserver(
net::NetworkQualityEstimator::EffectiveConnectionTypeObserver* observer)
override;

void AddRTTAndThroughputEstimatesObserver(
net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer)
override;

void RemoveRTTAndThroughputEstimatesObserver(
net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver* observer)
override;

void SetEffectiveConnectionTypeForTest(net::EffectiveConnectionType type) {
connection_type_ = type;
}
Expand Down
3 changes: 2 additions & 1 deletion content/browser/net/network_quality_observer_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "content/public/browser/notification_service.h"
#include "content/public/browser/notification_types.h"
#include "content/public/browser/render_process_host.h"
#include "net/nqe/network_quality_estimator.h"

namespace {

Expand Down Expand Up @@ -213,7 +214,7 @@ void NetworkQualityObserverImpl::OnRTTOrThroughputEstimatesComputed(
last_notified_network_quality_));
}

std::unique_ptr<net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver>
std::unique_ptr<net::RTTAndThroughputEstimatesObserver>
CreateNetworkQualityObserver(
net::NetworkQualityEstimator* network_quality_estimator) {
return base::MakeUnique<NetworkQualityObserverImpl>(
Expand Down
Loading

0 comments on commit 1bd4a95

Please sign in to comment.