Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(691)

Unified Diff: net/nqe/network_quality_estimator.cc

Issue 2128793003: Factor out NetworkID and caching mechanism from n_q_e.{h,cc} (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Rebased Created 4 years, 5 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: net/nqe/network_quality_estimator.cc
diff --git a/net/nqe/network_quality_estimator.cc b/net/nqe/network_quality_estimator.cc
index 7750e2faff832b75c6063e3551e24aa924905d94..67693ca70836f99e18e5a85aa14f6fe8814b1b4d 100644
--- a/net/nqe/network_quality_estimator.cc
+++ b/net/nqe/network_quality_estimator.cc
@@ -254,9 +254,9 @@ NetworkQualityEstimator::NetworkQualityEstimator(
effective_connection_type_recomputation_interval_(
base::TimeDelta::FromSeconds(15)),
last_connection_change_(tick_clock_->NowTicks()),
- current_network_id_(
- NetworkID(NetworkChangeNotifier::ConnectionType::CONNECTION_UNKNOWN,
- std::string())),
+ current_network_id_(nqe::internal::NetworkID(
+ NetworkChangeNotifier::ConnectionType::CONNECTION_UNKNOWN,
+ std::string())),
downstream_throughput_kbps_observations_(weight_multiplier_per_second_),
rtt_observations_(weight_multiplier_per_second_),
effective_connection_type_at_last_main_frame_(
@@ -268,12 +268,6 @@ NetworkQualityEstimator::NetworkQualityEstimator(
weak_ptr_factory_(this) {
static_assert(kDefaultHalfLifeSeconds > 0,
"Default half life duration must be > 0");
- static_assert(kMaximumNetworkQualityCacheSize > 0,
- "Size of the network quality cache must be > 0");
- // This limit should not be increased unless the logic for removing the
- // oldest cache entry is rewritten to use a doubly-linked-list LRU queue.
- static_assert(kMaximumNetworkQualityCacheSize <= 10,
- "Size of the network quality cache must <= 10");
// None of the algorithms can have an empty name.
DCHECK(algorithm_name_to_enum_.end() ==
algorithm_name_to_enum_.find(std::string()));
@@ -762,7 +756,10 @@ void NetworkQualityEstimator::OnConnectionTypeChanged(
RecordMetricsOnConnectionTypeChanged();
// Write the estimates of the previous network to the cache.
- CacheNetworkQualityEstimate();
+ network_qualities_manager_.CacheNetworkQualityEstimate(
+ current_network_id_, nqe::internal::CachedNetworkQuality(
+ last_effective_connection_type_computation_,
+ estimated_quality_at_last_main_frame_));
// Clear the local state.
last_connection_change_ = tick_clock_->NowTicks();
@@ -980,7 +977,7 @@ NetworkQualityEstimator::GetRecentEffectiveConnectionTypeUsingMetrics(
// If the device is currently offline, then return
// EFFECTIVE_CONNECTION_TYPE_OFFLINE.
- if (GetCurrentNetworkID().type == NetworkChangeNotifier::CONNECTION_NONE)
+ if (current_network_id_.type == NetworkChangeNotifier::CONNECTION_NONE)
return EFFECTIVE_CONNECTION_TYPE_OFFLINE;
base::TimeDelta http_rtt = nqe::internal::InvalidRTT();
@@ -1165,8 +1162,7 @@ int32_t NetworkQualityEstimator::GetDownlinkThroughputKbpsEstimateInternal(
return kbps;
}
-NetworkQualityEstimator::NetworkID
-NetworkQualityEstimator::GetCurrentNetworkID() const {
+nqe::internal::NetworkID NetworkQualityEstimator::GetCurrentNetworkID() const {
DCHECK(thread_checker_.CalledOnValidThread());
// TODO(tbansal): crbug.com/498068 Add NetworkQualityEstimatorAndroid class
@@ -1179,7 +1175,7 @@ NetworkQualityEstimator::GetCurrentNetworkID() const {
// capture majority of cases, and should not significantly affect estimates
// (that are approximate to begin with).
while (true) {
- NetworkQualityEstimator::NetworkID network_id(
+ nqe::internal::NetworkID network_id(
NetworkChangeNotifier::GetConnectionType(), std::string());
switch (network_id.type) {
@@ -1215,42 +1211,36 @@ NetworkQualityEstimator::GetCurrentNetworkID() const {
bool NetworkQualityEstimator::ReadCachedNetworkQualityEstimate() {
DCHECK(thread_checker_.CalledOnValidThread());
- // If the network name is unavailable, caching should not be performed.
- if (current_network_id_.id.empty())
- return false;
-
- CachedNetworkQualities::const_iterator it =
- cached_network_qualities_.find(current_network_id_);
+ nqe::internal::CachedNetworkQuality cached_network_quality;
- if (it == cached_network_qualities_.end())
+ if (!network_qualities_manager_.GetCachedNetworkQualityEstimate(
+ current_network_id_, &cached_network_quality)) {
+ UMA_HISTOGRAM_BOOLEAN("NQE.CachedNetworkQualityAvailable", false);
RyanSturm 2016/07/12 18:45:38 Can you combine the two NQE.CacheNetworkQualityAva
tbansal1 2016/07/12 19:40:19 Done.
return false;
-
- nqe::internal::NetworkQuality network_quality(it->second.network_quality());
+ }
const base::TimeTicks now = tick_clock_->NowTicks();
- bool read_cached_estimate = false;
- if (network_quality.downstream_throughput_kbps() !=
+ if (cached_network_quality.network_quality().downstream_throughput_kbps() !=
nqe::internal::kInvalidThroughput) {
- read_cached_estimate = true;
ThroughputObservation througphput_observation(
- network_quality.downstream_throughput_kbps(), now,
- NETWORK_QUALITY_OBSERVATION_SOURCE_CACHED_ESTIMATE);
+ cached_network_quality.network_quality().downstream_throughput_kbps(),
+ now, NETWORK_QUALITY_OBSERVATION_SOURCE_CACHED_ESTIMATE);
downstream_throughput_kbps_observations_.AddObservation(
througphput_observation);
NotifyObserversOfThroughput(througphput_observation);
}
- if (network_quality.http_rtt() != nqe::internal::InvalidRTT()) {
- read_cached_estimate = true;
+ if (cached_network_quality.network_quality().http_rtt() !=
+ nqe::internal::InvalidRTT()) {
RttObservation rtt_observation(
- network_quality.http_rtt(), now,
+ cached_network_quality.network_quality().http_rtt(), now,
NETWORK_QUALITY_OBSERVATION_SOURCE_CACHED_ESTIMATE);
rtt_observations_.AddObservation(rtt_observation);
NotifyObserversOfRTT(rtt_observation);
}
-
- return read_cached_estimate;
+ UMA_HISTOGRAM_BOOLEAN("NQE.CachedNetworkQualityAvailable", true);
+ return true;
}
void NetworkQualityEstimator::OnUpdatedEstimateAvailable(
@@ -1337,51 +1327,6 @@ void NetworkQualityEstimator::SetTickClockForTesting(
tick_clock_ = std::move(tick_clock);
}
-void NetworkQualityEstimator::CacheNetworkQualityEstimate() {
- DCHECK(thread_checker_.CalledOnValidThread());
- DCHECK_LE(cached_network_qualities_.size(),
- static_cast<size_t>(kMaximumNetworkQualityCacheSize));
-
- // If the network name is unavailable, caching should not be performed.
- if (current_network_id_.id.empty())
- return;
-
- base::TimeDelta http_rtt = nqe::internal::InvalidRTT();
- int32_t downlink_throughput_kbps = nqe::internal::kInvalidThroughput;
-
- if (!GetHttpRTTEstimate(&http_rtt) ||
- !GetDownlinkThroughputKbpsEstimate(&downlink_throughput_kbps)) {
- return;
- }
-
- // |transport_rtt| is currently not cached.
- nqe::internal::NetworkQuality network_quality = nqe::internal::NetworkQuality(
- http_rtt, nqe::internal::InvalidRTT() /* transport_rtt */,
- downlink_throughput_kbps);
-
- if (cached_network_qualities_.size() == kMaximumNetworkQualityCacheSize) {
- // Remove the oldest entry.
- CachedNetworkQualities::iterator oldest_entry_iterator =
- cached_network_qualities_.begin();
-
- for (CachedNetworkQualities::iterator it =
- cached_network_qualities_.begin();
- it != cached_network_qualities_.end(); ++it) {
- if ((it->second).OlderThan(oldest_entry_iterator->second))
- oldest_entry_iterator = it;
- }
- cached_network_qualities_.erase(oldest_entry_iterator);
- }
- DCHECK_LT(cached_network_qualities_.size(),
- static_cast<size_t>(kMaximumNetworkQualityCacheSize));
-
- cached_network_qualities_.insert(
- std::make_pair(current_network_id_,
- nqe::internal::CachedNetworkQuality(network_quality)));
- DCHECK_LE(cached_network_qualities_.size(),
- static_cast<size_t>(kMaximumNetworkQualityCacheSize));
-}
-
void NetworkQualityEstimator::OnUpdatedRTTAvailable(
SocketPerformanceWatcherFactory::Protocol protocol,
const base::TimeDelta& rtt) {

Powered by Google App Engine
This is Rietveld 408576698