Skip to content

GitLab

T
tangled
Commit bd2ac01e authored by Tarun Bansal's avatar Tarun Bansal Committed by Commit Bot
Browse files
Options

Simplify UI network quality estimator service 

UI network quality estimator service no longer needs to implement
network quality provider since we now have network quality
tracker. This CL simplifies the logic in UI network quality
estimator service by removing the unnecessary code.

Bug: 878844
Change-Id: I7d97bc29452a73d5a6f522b58cca3c67e5368e77
Reviewed-on: https://chromium-review.googlesource.com/1196005Reviewed-by: default avatarRyan Sturm <ryansturm@chromium.org>
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#587798}
parent ef6fd66e
Hide whitespace changes
Inline Side-by-side
Showing with 4 additions and 355 deletions
chrome/browser/net/nqe/ui_network_quality_estimator_service.cc
View file @ bd2ac01e
......@@ -18,12 +18,9 @@
#include "components/prefs/pref_registry.h"
#include "components/prefs/pref_registry_simple.h"
#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"
#include "net/nqe/network_quality_estimator.h"
#include "net/url_request/url_request_context.h"
namespace {
......@@ -84,102 +81,19 @@ void SetNQEOnIOThread(net::NetworkQualitiesPrefsManager* prefs_manager,
} // namespace
// A class that sets itself as an observer of the EffectiveconnectionType for
// the browser IO thread. It reports any change in EffectiveConnectionType back
// to the UI service.
// It is created on the UI thread, but used and deleted on the IO thread.
class UINetworkQualityEstimatorService::IONetworkQualityObserver
: public net::EffectiveConnectionTypeObserver,
public net::RTTAndThroughputEstimatesObserver {
public:
explicit IONetworkQualityObserver(
base::WeakPtr<UINetworkQualityEstimatorService> service)
: service_(service), network_quality_estimator_(nullptr) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
}
~IONetworkQualityObserver() override {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
if (network_quality_estimator_) {
network_quality_estimator_->RemoveEffectiveConnectionTypeObserver(this);
network_quality_estimator_->RemoveRTTAndThroughputEstimatesObserver(this);
}
}
void InitializeOnIOThread(IOThread* io_thread) {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
if (!io_thread->globals()->system_request_context ||
!io_thread->globals()
->system_request_context->network_quality_estimator()) {
return;
}
network_quality_estimator_ =
io_thread->globals()
->system_request_context->network_quality_estimator();
if (!network_quality_estimator_)
return;
network_quality_estimator_->AddEffectiveConnectionTypeObserver(this);
network_quality_estimator_->AddRTTAndThroughputEstimatesObserver(this);
}
// net::NetworkQualityEstimator::EffectiveConnectionTypeObserver
// implementation:
void OnEffectiveConnectionTypeChanged(
net::EffectiveConnectionType type) override {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
content::BrowserThread::PostTask(
content::BrowserThread::UI, FROM_HERE,
base::BindOnce(
&UINetworkQualityEstimatorService::EffectiveConnectionTypeChanged,
service_, type));
}
// net::NetworkQualityEstimator::RTTAndThroughputEstimatesObserver
// implementation:
void OnRTTOrThroughputEstimatesComputed(
base::TimeDelta http_rtt,
base::TimeDelta transport_rtt,
int32_t downstream_throughput_kbps) override {
DCHECK_CURRENTLY_ON(content::BrowserThread::IO);
content::BrowserThread::PostTask(
content::BrowserThread::UI, FROM_HERE,
base::BindOnce(
&UINetworkQualityEstimatorService::RTTOrThroughputComputed,
service_, http_rtt, transport_rtt, downstream_throughput_kbps));
}
private:
base::WeakPtr<UINetworkQualityEstimatorService> service_;
net::NetworkQualityEstimator* network_quality_estimator_;
DISALLOW_COPY_AND_ASSIGN(IONetworkQualityObserver);
};
UINetworkQualityEstimatorService::UINetworkQualityEstimatorService(
Profile* profile)
: type_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
http_rtt_(net::nqe::internal::InvalidRTT()),
transport_rtt_(net::nqe::internal::InvalidRTT()),
downstream_throughput_kbps_(net::nqe::internal::INVALID_RTT_THROUGHPUT),
weak_factory_(this) {
: weak_factory_(this) {
DCHECK(profile);
// If this is running in a context without an IOThread, don't try to create
// the IO object.
if (!g_browser_process->io_thread())
return;
io_observer_ = base::WrapUnique(
new IONetworkQualityObserver(weak_factory_.GetWeakPtr()));
std::unique_ptr<PrefDelegateImpl> pref_delegate(
new PrefDelegateImpl(profile->GetPrefs()));
prefs_manager_ = base::WrapUnique(
new net::NetworkQualitiesPrefsManager(std::move(pref_delegate)));
content::BrowserThread::PostTask(
content::BrowserThread::IO, FROM_HERE,
base::BindOnce(&IONetworkQualityObserver::InitializeOnIOThread,
base::Unretained(io_observer_.get()),
g_browser_process->io_thread()));
content::BrowserThread::PostTask(
content::BrowserThread::IO, FROM_HERE,
base::BindOnce(&SetNQEOnIOThread, prefs_manager_.get(),
......@@ -193,13 +107,6 @@ UINetworkQualityEstimatorService::~UINetworkQualityEstimatorService() {
void UINetworkQualityEstimatorService::Shutdown() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
weak_factory_.InvalidateWeakPtrs();
if (io_observer_) {
bool deleted = content::BrowserThread::DeleteSoon(
content::BrowserThread::IO, FROM_HERE, io_observer_.release());
DCHECK(deleted);
// Silence unused variable warning in release builds.
(void)deleted;
}
if (prefs_manager_) {
prefs_manager_->ShutdownOnPrefSequence();
bool deleted = content::BrowserThread::DeleteSoon(
......@@ -210,83 +117,6 @@ void UINetworkQualityEstimatorService::Shutdown() {
}
}
void UINetworkQualityEstimatorService::EffectiveConnectionTypeChanged(
net::EffectiveConnectionType type) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
type_ = type;
for (auto& observer : effective_connection_type_observer_list_)
observer.OnEffectiveConnectionTypeChanged(type);
}
void UINetworkQualityEstimatorService::RTTOrThroughputComputed(
base::TimeDelta http_rtt,
base::TimeDelta transport_rtt,
int32_t downstream_throughput_kbps) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
http_rtt_ = http_rtt;
transport_rtt_ = transport_rtt;
downstream_throughput_kbps_ = downstream_throughput_kbps;
}
void UINetworkQualityEstimatorService::AddEffectiveConnectionTypeObserver(
net::EffectiveConnectionTypeObserver* observer) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
effective_connection_type_observer_list_.AddObserver(observer);
// Notify the |observer| on the next message pump since |observer| may not
// be completely set up for receiving the callbacks.
content::BrowserThread::PostTask(
content::BrowserThread::UI, FROM_HERE,
base::BindOnce(&UINetworkQualityEstimatorService::
NotifyEffectiveConnectionTypeObserverIfPresent,
weak_factory_.GetWeakPtr(), observer));
}
void UINetworkQualityEstimatorService::RemoveEffectiveConnectionTypeObserver(
net::EffectiveConnectionTypeObserver* observer) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
effective_connection_type_observer_list_.RemoveObserver(observer);
}
base::Optional<base::TimeDelta> UINetworkQualityEstimatorService::GetHttpRTT()
const {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (http_rtt_ == net::nqe::internal::InvalidRTT())
return base::Optional<base::TimeDelta>();
return http_rtt_;
}
base::Optional<base::TimeDelta>
UINetworkQualityEstimatorService::GetTransportRTT() const {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (transport_rtt_ == net::nqe::internal::InvalidRTT())
return base::Optional<base::TimeDelta>();
return transport_rtt_;
}
base::Optional<int32_t>
UINetworkQualityEstimatorService::GetDownstreamThroughputKbps() const {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (downstream_throughput_kbps_ == net::nqe::internal::INVALID_RTT_THROUGHPUT)
return base::Optional<int32_t>();
return downstream_throughput_kbps_;
}
void UINetworkQualityEstimatorService::SetEffectiveConnectionTypeForTesting(
net::EffectiveConnectionType type) {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
EffectiveConnectionTypeChanged(type);
}
net::EffectiveConnectionType
UINetworkQualityEstimatorService::GetEffectiveConnectionType() const {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
return type_;
}
void UINetworkQualityEstimatorService::ClearPrefs() {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (!prefs_manager_)
......@@ -294,18 +124,6 @@ void UINetworkQualityEstimatorService::ClearPrefs() {
prefs_manager_->ClearPrefs();
}
void UINetworkQualityEstimatorService::
NotifyEffectiveConnectionTypeObserverIfPresent(
net::EffectiveConnectionTypeObserver* observer) const {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);
if (!effective_connection_type_observer_list_.HasObserver(observer))
return;
if (type_ == net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN)
return;
observer->OnEffectiveConnectionTypeChanged(type_);
}
// static
void UINetworkQualityEstimatorService::RegisterProfilePrefs(
PrefRegistrySimple* registry) {
......
chrome/browser/net/nqe/ui_network_quality_estimator_service.h
View file @ bd2ac01e
......@@ -12,51 +12,30 @@
#include "base/macros.h"
#include "base/memory/weak_ptr.h"
#include "base/observer_list.h"
#include "base/time/time.h"
#include "components/keyed_service/core/keyed_service.h"
#include "net/nqe/cached_network_quality.h"
#include "net/nqe/effective_connection_type.h"
#include "net/nqe/network_id.h"
#include "net/nqe/network_quality_estimator.h"
class PrefRegistrySimple;
class Profile;
namespace net {
class EffectiveConnectionTypeObserver;
class NetworkQualitiesPrefsManager;
}
// UI service to determine the current EffectiveConnectionType.
class UINetworkQualityEstimatorService
: public KeyedService,
public net::NetworkQualityEstimator::NetworkQualityProvider {
// UI service to manage storage of network quality prefs.
class UINetworkQualityEstimatorService : public KeyedService {
public:
explicit UINetworkQualityEstimatorService(Profile* profile);
~UINetworkQualityEstimatorService() override;
// NetworkQualityProvider implementation:
// Must be called on the UI thread.
net::EffectiveConnectionType GetEffectiveConnectionType() const override;
void AddEffectiveConnectionTypeObserver(
net::EffectiveConnectionTypeObserver* observer) override;
void RemoveEffectiveConnectionTypeObserver(
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;
// Registers the profile-specific network quality estimator prefs.
static void RegisterProfilePrefs(PrefRegistrySimple* registry);
// Clear the network quality estimator prefs.
void ClearPrefs();
// Tests can manually set EffectiveConnectionType, but browser tests should
// expect that the EffectiveConnectionType could change.
void SetEffectiveConnectionTypeForTesting(net::EffectiveConnectionType type);
// Reads the prefs from the disk, parses them into a map of NetworkIDs and
// CachedNetworkQualities, and returns the map.
std::map<net::nqe::internal::NetworkID,
......@@ -64,44 +43,9 @@ class UINetworkQualityEstimatorService
ForceReadPrefsForTesting() const;
private:
class IONetworkQualityObserver;
// Notifies |observer| of the current effective connection type if |observer|
// is still registered as an observer.
void NotifyEffectiveConnectionTypeObserverIfPresent(
net::EffectiveConnectionTypeObserver* observer) const;
// KeyedService implementation:
void Shutdown() override;
// Called when the EffectiveConnectionType has updated to |type|.
// NetworkQualityEstimator::EffectiveConnectionType is an estimate of the
// quality of the network that may differ from the actual network type
// reported by NetworkchangeNotifier::GetConnectionType.
void EffectiveConnectionTypeChanged(net::EffectiveConnectionType type);
// Called when the estimated HTTP RTT, estimated transport RTT or estimated
// downstream throughput is computed.
void RTTOrThroughputComputed(base::TimeDelta http_rtt,
base::TimeDelta transport_rtt,
int32_t downstream_throughput_kbps);
// The current EffectiveConnectionType.
net::EffectiveConnectionType type_;
// Current network quality metrics.
base::TimeDelta http_rtt_;
base::TimeDelta transport_rtt_;
int32_t downstream_throughput_kbps_;
// IO thread based observer that is owned by this service. Created on the UI
// thread, but used and deleted on the IO thread.
std::unique_ptr<IONetworkQualityObserver> io_observer_;
// Observer list for changes in effective connection type.
base::ObserverList<net::EffectiveConnectionTypeObserver>::Unchecked
effective_connection_type_observer_list_;
// Prefs manager that is owned by this service. Created on the UI thread, but
// used and deleted on the IO thread.
std::unique_ptr<net::NetworkQualitiesPrefsManager> prefs_manager_;
......
chrome/browser/net/nqe/ui_network_quality_estimator_service_browsertest.cc
View file @ bd2ac01e
......@@ -23,40 +23,14 @@
#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::EffectiveConnectionTypeObserver {
public:
TestEffectiveConnectionTypeObserver()
: effective_connection_type_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN) {}
~TestEffectiveConnectionTypeObserver() override {}
// net::EffectiveConnectionTypeObserver implementation:
void OnEffectiveConnectionTypeChanged(
net::EffectiveConnectionType type) override {
effective_connection_type_ = type;
}
// The most recently set EffectiveConnectionType.
net::EffectiveConnectionType effective_connection_type() const {
return effective_connection_type_;
}
private:
net::EffectiveConnectionType effective_connection_type_;
DISALLOW_COPY_AND_ASSIGN(TestEffectiveConnectionTypeObserver);
};
class UINetworkQualityEstimatorServiceBrowserTest
: public InProcessBrowserTest {
public:
......@@ -87,8 +61,6 @@ class UINetworkQualityEstimatorServiceBrowserTest
base::HistogramTester histogram_tester;
nqe_test_util::OverrideEffectiveConnectionTypeAndWait(
net::EFFECTIVE_CONNECTION_TYPE_OFFLINE);
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_OFFLINE,
nqe_service->GetEffectiveConnectionType());
// Prefs are written only if persistent caching was enabled.
EXPECT_FALSE(
......@@ -104,9 +76,6 @@ class UINetworkQualityEstimatorServiceBrowserTest
nqe_test_util::OverrideEffectiveConnectionTypeAndWait(
net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G);
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G,
nqe_service->GetEffectiveConnectionType());
// Prefs are written even if the network id was unavailable.
EXPECT_FALSE(
histogram_tester.GetAllSamples("NQE.Prefs.WriteCount").empty());
......@@ -148,88 +117,6 @@ class UINetworkQualityEstimatorServiceBrowserTest
} // namespace
IN_PROC_BROWSER_TEST_F(UINetworkQualityEstimatorServiceBrowserTest,
VerifyNQEState) {
// Verifies that NQE notifying EffectiveConnectionTypeObservers causes the
// UINetworkQualityEstimatorService to receive an updated
// EffectiveConnectionType.
Profile* profile = ProfileManager::GetActiveUserProfile();
UINetworkQualityEstimatorService* nqe_service =
UINetworkQualityEstimatorServiceFactory::GetForProfile(profile);
TestEffectiveConnectionTypeObserver nqe_observer;
nqe_service->AddEffectiveConnectionTypeObserver(&nqe_observer);
nqe_test_util::OverrideEffectiveConnectionTypeAndWait(
net::EFFECTIVE_CONNECTION_TYPE_OFFLINE);
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_OFFLINE,
nqe_service->GetEffectiveConnectionType());
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_OFFLINE,
nqe_observer.effective_connection_type());
nqe_test_util::OverrideEffectiveConnectionTypeAndWait(
net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G);
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G,
nqe_service->GetEffectiveConnectionType());
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G,
nqe_observer.effective_connection_type());
nqe_service->RemoveEffectiveConnectionTypeObserver(&nqe_observer);
nqe_test_util::OverrideEffectiveConnectionTypeAndWait(
net::EFFECTIVE_CONNECTION_TYPE_OFFLINE);
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_OFFLINE,
nqe_service->GetEffectiveConnectionType());
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_SLOW_2G,
nqe_observer.effective_connection_type());
// Observer should be notified on addition.
TestEffectiveConnectionTypeObserver nqe_observer_2;
nqe_service->AddEffectiveConnectionTypeObserver(&nqe_observer_2);
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN,
nqe_observer_2.effective_connection_type());
base::RunLoop().RunUntilIdle();
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_OFFLINE,
nqe_observer_2.effective_connection_type());
// |nqe_observer_3| should be not notified since it unregisters before the
// message loop is run.
TestEffectiveConnectionTypeObserver nqe_observer_3;
nqe_service->AddEffectiveConnectionTypeObserver(&nqe_observer_3);
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN,
nqe_observer_3.effective_connection_type());
nqe_service->RemoveEffectiveConnectionTypeObserver(&nqe_observer_3);
base::RunLoop().RunUntilIdle();
EXPECT_EQ(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN,
nqe_observer_3.effective_connection_type());
}
IN_PROC_BROWSER_TEST_F(UINetworkQualityEstimatorServiceBrowserTest,
VerifyRTTs) {
// Verifies that NQE notifying RTTAndThroughputEstimatesObserver causes the
// UINetworkQualityEstimatorService to receive an update.
Profile* profile = ProfileManager::GetActiveUserProfile();
UINetworkQualityEstimatorService* nqe_service =
UINetworkQualityEstimatorServiceFactory::GetForProfile(profile);
base::TimeDelta rtt_1 = base::TimeDelta::FromMilliseconds(100);
nqe_test_util::OverrideRTTsAndWait(rtt_1);
EXPECT_EQ(rtt_1, nqe_service->GetHttpRTT());
EXPECT_EQ(rtt_1, nqe_service->GetTransportRTT());
EXPECT_FALSE(nqe_service->GetDownstreamThroughputKbps().has_value());
base::TimeDelta rtt_2 = base::TimeDelta::FromMilliseconds(200);
nqe_test_util::OverrideRTTsAndWait(rtt_2);
EXPECT_EQ(rtt_2, nqe_service->GetHttpRTT());
EXPECT_EQ(rtt_2, nqe_service->GetTransportRTT());
EXPECT_FALSE(nqe_service->GetDownstreamThroughputKbps().has_value());
base::TimeDelta rtt_3 = base::TimeDelta::FromMilliseconds(300);
nqe_test_util::OverrideRTTsAndWait(rtt_3);
EXPECT_EQ(rtt_3, nqe_service->GetHttpRTT());
EXPECT_EQ(rtt_3, nqe_service->GetTransportRTT());
EXPECT_FALSE(nqe_service->GetDownstreamThroughputKbps().has_value());
}
// Verify that prefs are writen and read correctly.
IN_PROC_BROWSER_TEST_F(UINetworkQualityEstimatorServiceBrowserTest,
WritingReadingToPrefsEnabled) {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment