Skip to content

Commit

Permalink
Replace kInvalidThroughput with INVALID_RTT_THROUGHPUT.
Browse files Browse the repository at this point in the history
As the comment in network_quality.h by tbansal@,
replace kInvalidThroughput with INVALID_RTT_THROUGHPUT.

BUG=
R=tbansal@chromium.org

Cq-Include-Trybots: master.tryserver.chromium.android:android_cronet_tester;master.tryserver.chromium.mac:ios-simulator-cronet

Signed-off-by: zhuoyu.qian <zhuoyu.qian@samsung.com>
Change-Id: I148b149e6787b79a8e7ffda4f7446854d63662bf
Reviewed-on: https://chromium-review.googlesource.com/734926
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Tarun Bansal <tbansal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#512464}
  • Loading branch information
ZhuoyuQian authored and Commit Bot committed Oct 30, 2017
1 parent cc9e05a commit d80590e
Show file tree
Hide file tree
Showing 9 changed files with 51 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ UINetworkQualityEstimatorService::UINetworkQualityEstimatorService(
: type_(net::EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
http_rtt_(net::nqe::internal::InvalidRTT()),
transport_rtt_(net::nqe::internal::InvalidRTT()),
downstream_throughput_kbps_(net::nqe::internal::kInvalidThroughput),
downstream_throughput_kbps_(net::nqe::internal::INVALID_RTT_THROUGHPUT),
weak_factory_(this) {
DCHECK(profile);
// If this is running in a context without an IOThread, don't try to create
Expand Down Expand Up @@ -264,7 +264,7 @@ base::Optional<int32_t>
UINetworkQualityEstimatorService::GetDownstreamThroughputKbps() const {
DCHECK_CURRENTLY_ON(content::BrowserThread::UI);

if (downstream_throughput_kbps_ == net::nqe::internal::kInvalidThroughput)
if (downstream_throughput_kbps_ == net::nqe::internal::INVALID_RTT_THROUGHPUT)
return base::Optional<int32_t>();
return downstream_throughput_kbps_;
}
Expand Down
2 changes: 1 addition & 1 deletion content/common/renderer.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ interface Renderer {
// The downstream throughput is computed in kilobits per second. If an
// estimate of the HTTP or transport RTT is unavailable, it will be set to
// net::nqe::internal::InvalidRTT(). If the throughput estimate is
// unavailable, it will be set to net::nqe::internal::kInvalidThroughput.
// unavailable, it will be set to net::nqe::internal::INVALID_RTT_THROUGHPUT.
OnNetworkQualityChanged(EffectiveConnectionType effective_connection_type,
mojo.common.mojom.TimeDelta http_rtt,
mojo.common.mojom.TimeDelta transport_rtt,
Expand Down
8 changes: 4 additions & 4 deletions net/nqe/network_quality.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,15 @@ base::TimeDelta InvalidRTT() {
}

NetworkQuality::NetworkQuality()
: NetworkQuality(InvalidRTT(), InvalidRTT(), kInvalidThroughput) {}
: NetworkQuality(InvalidRTT(), InvalidRTT(), INVALID_RTT_THROUGHPUT) {}

NetworkQuality::NetworkQuality(const base::TimeDelta& http_rtt,
const base::TimeDelta& transport_rtt,
int32_t downstream_throughput_kbps)
: http_rtt_(http_rtt),
transport_rtt_(transport_rtt),
downstream_throughput_kbps_(downstream_throughput_kbps) {
DCHECK_GE(downstream_throughput_kbps_, kInvalidThroughput);
DCHECK_GE(downstream_throughput_kbps_, INVALID_RTT_THROUGHPUT);
}

NetworkQuality::NetworkQuality(const NetworkQuality& other)
Expand Down Expand Up @@ -50,8 +50,8 @@ bool NetworkQuality::IsFaster(const NetworkQuality& other) const {
(transport_rtt() == InvalidRTT() ||
other.transport_rtt() == InvalidRTT() ||
transport_rtt() <= other.transport_rtt()) &&
(downstream_throughput_kbps() == kInvalidThroughput ||
other.downstream_throughput_kbps() == kInvalidThroughput ||
(downstream_throughput_kbps() == INVALID_RTT_THROUGHPUT ||
other.downstream_throughput_kbps() == INVALID_RTT_THROUGHPUT ||
downstream_throughput_kbps() >= other.downstream_throughput_kbps());
}

Expand Down
7 changes: 0 additions & 7 deletions net/nqe/network_quality.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,6 @@ enum RttThroughputValues {
// |INVALID_RTT_THROUGHPUT|.
NET_EXPORT_PRIVATE base::TimeDelta InvalidRTT();

// Throughput is set to |kInvalidThroughput| if a valid value is
// unavailable. Readers should discard throughput value if it is set to
// |kInvalidThroughput|.
// TODO(tbansal): Remove this variable, and replace all calls by
// |INVALID_RTT_THROUGHPUT|.
const int32_t kInvalidThroughput = INVALID_RTT_THROUGHPUT;

// NetworkQuality is used to cache the quality of a network connection.
class NET_EXPORT_PRIVATE NetworkQuality {
public:
Expand Down
43 changes: 22 additions & 21 deletions net/nqe/network_quality_estimator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -314,7 +314,8 @@ void NetworkQualityEstimator::AddDefaultEstimates() {
}

if (params_->DefaultObservation(current_network_id_.type)
.downstream_throughput_kbps() != nqe::internal::kInvalidThroughput) {
.downstream_throughput_kbps() !=
nqe::internal::INVALID_RTT_THROUGHPUT) {
Observation throughput_observation(
params_->DefaultObservation(current_network_id_.type)
.downstream_throughput_kbps(),
Expand Down Expand Up @@ -475,7 +476,7 @@ void NetworkQualityEstimator::RecordAccuracyAfterMainFrame(

int32_t recent_downstream_throughput_kbps;
if (estimated_quality_at_last_main_frame_.downstream_throughput_kbps() !=
nqe::internal::kInvalidThroughput &&
nqe::internal::INVALID_RTT_THROUGHPUT &&
GetRecentDownlinkThroughputKbps(last_main_frame_request_,
&recent_downstream_throughput_kbps)) {
const int estimated_observed_diff =
Expand Down Expand Up @@ -578,7 +579,7 @@ void NetworkQualityEstimator::RecordCorrelationMetric(const URLRequest& request,
int32_t rtt = 0;

if (estimated_quality_at_last_main_frame_.downstream_throughput_kbps() ==
nqe::internal::kInvalidThroughput) {
nqe::internal::INVALID_RTT_THROUGHPUT) {
return;
}

Expand Down Expand Up @@ -924,7 +925,7 @@ void NetworkQualityEstimator::RecordMetricsOnMainFrameRequest() const {
nqe::internal::InvalidRTT());

if (estimated_quality_at_last_main_frame_.downstream_throughput_kbps() !=
nqe::internal::kInvalidThroughput) {
nqe::internal::INVALID_RTT_THROUGHPUT) {
// Add the 50th percentile value.
UMA_HISTOGRAM_COUNTS_1M(
"NQE.MainFrame.Kbps.Percentile50",
Expand All @@ -933,7 +934,7 @@ void NetworkQualityEstimator::RecordMetricsOnMainFrameRequest() const {
UMA_HISTOGRAM_BOOLEAN(
"NQE.EstimateAvailable.MainFrame.Kbps",
estimated_quality_at_last_main_frame_.downstream_throughput_kbps() !=
nqe::internal::kInvalidThroughput);
nqe::internal::INVALID_RTT_THROUGHPUT);

UMA_HISTOGRAM_ENUMERATION("NQE.MainFrame.EffectiveConnectionType",
effective_connection_type_at_last_main_frame_,
Expand Down Expand Up @@ -962,7 +963,7 @@ void NetworkQualityEstimator::ComputeBandwidthDelayProduct() {

int32_t downlink_throughput_kbps =
GetDownlinkThroughputKbpsEstimateInternal(base::TimeTicks(), 20);
if (downlink_throughput_kbps == nqe::internal::kInvalidThroughput)
if (downlink_throughput_kbps == nqe::internal::INVALID_RTT_THROUGHPUT)
return;

bandwidth_delay_product_kbits_ =
Expand Down Expand Up @@ -1101,7 +1102,7 @@ void NetworkQualityEstimator::ComputeEffectiveConnectionType() {

base::TimeDelta http_rtt = nqe::internal::InvalidRTT();
base::TimeDelta transport_rtt = nqe::internal::InvalidRTT();
int32_t downstream_throughput_kbps = nqe::internal::kInvalidThroughput;
int32_t downstream_throughput_kbps = nqe::internal::INVALID_RTT_THROUGHPUT;

effective_connection_type_ =
GetRecentEffectiveConnectionTypeAndNetworkQuality(
Expand Down Expand Up @@ -1157,7 +1158,7 @@ NetworkQualityEstimator::GetRecentEffectiveConnectionType(

base::TimeDelta http_rtt = nqe::internal::InvalidRTT();
base::TimeDelta transport_rtt = nqe::internal::InvalidRTT();
int32_t downstream_throughput_kbps = nqe::internal::kInvalidThroughput;
int32_t downstream_throughput_kbps = nqe::internal::INVALID_RTT_THROUGHPUT;

return GetRecentEffectiveConnectionTypeAndNetworkQuality(
start_time, &http_rtt, &transport_rtt, &downstream_throughput_kbps);
Expand Down Expand Up @@ -1231,7 +1232,7 @@ NetworkQualityEstimator::GetRecentEffectiveConnectionTypeUsingMetrics(

*http_rtt = nqe::internal::InvalidRTT();
*transport_rtt = nqe::internal::InvalidRTT();
*downstream_throughput_kbps = nqe::internal::kInvalidThroughput;
*downstream_throughput_kbps = nqe::internal::INVALID_RTT_THROUGHPUT;

if (params_->forced_effective_connection_type()) {
*http_rtt = params_
Expand Down Expand Up @@ -1283,7 +1284,7 @@ NetworkQualityEstimator::GetRecentEffectiveConnectionTypeUsingMetrics(
}

if (!GetRecentDownlinkThroughputKbps(start_time, downstream_throughput_kbps))
*downstream_throughput_kbps = nqe::internal::kInvalidThroughput;
*downstream_throughput_kbps = nqe::internal::INVALID_RTT_THROUGHPUT;

if (*http_rtt == nqe::internal::InvalidRTT() &&
http_rtt_metric == NetworkQualityEstimator::MetricUsage::MUST_BE_USED) {
Expand All @@ -1296,15 +1297,15 @@ NetworkQualityEstimator::GetRecentEffectiveConnectionTypeUsingMetrics(
return EFFECTIVE_CONNECTION_TYPE_UNKNOWN;
}

if (*downstream_throughput_kbps == nqe::internal::kInvalidThroughput &&
if (*downstream_throughput_kbps == nqe::internal::INVALID_RTT_THROUGHPUT &&
downstream_throughput_kbps_metric ==
NetworkQualityEstimator::MetricUsage::MUST_BE_USED) {
return EFFECTIVE_CONNECTION_TYPE_UNKNOWN;
}

if (*http_rtt == nqe::internal::InvalidRTT() &&
*transport_rtt == nqe::internal::InvalidRTT() &&
*downstream_throughput_kbps == nqe::internal::kInvalidThroughput) {
*downstream_throughput_kbps == nqe::internal::INVALID_RTT_THROUGHPUT) {
// None of the metrics are available.
return EFFECTIVE_CONNECTION_TYPE_UNKNOWN;
}
Expand Down Expand Up @@ -1335,9 +1336,9 @@ NetworkQualityEstimator::GetRecentEffectiveConnectionTypeUsingMetrics(
const bool estimated_throughput_is_lower_than_threshold =
downstream_throughput_kbps_metric !=
NetworkQualityEstimator::MetricUsage::DO_NOT_USE &&
*downstream_throughput_kbps != nqe::internal::kInvalidThroughput &&
*downstream_throughput_kbps != nqe::internal::INVALID_RTT_THROUGHPUT &&
params_->ConnectionThreshold(type).downstream_throughput_kbps() !=
nqe::internal::kInvalidThroughput &&
nqe::internal::INVALID_RTT_THROUGHPUT &&
*downstream_throughput_kbps <=
params_->ConnectionThreshold(type).downstream_throughput_kbps();

Expand Down Expand Up @@ -1416,7 +1417,7 @@ bool NetworkQualityEstimator::GetRecentDownlinkThroughputKbps(
int32_t* kbps) const {
DCHECK(thread_checker_.CalledOnValidThread());
*kbps = GetDownlinkThroughputKbpsEstimateInternal(start_time, 50);
return (*kbps != nqe::internal::kInvalidThroughput);
return (*kbps != nqe::internal::INVALID_RTT_THROUGHPUT);
}

base::TimeDelta NetworkQualityEstimator::GetRTTEstimateInternal(
Expand Down Expand Up @@ -1536,7 +1537,7 @@ bool NetworkQualityEstimator::ReadCachedNetworkQualityEstimate() {
const base::TimeTicks now = tick_clock_->NowTicks();

if (cached_network_quality.network_quality().downstream_throughput_kbps() !=
nqe::internal::kInvalidThroughput) {
nqe::internal::INVALID_RTT_THROUGHPUT) {
Observation througphput_observation(
cached_network_quality.network_quality().downstream_throughput_kbps(),
now, INT32_MIN,
Expand Down Expand Up @@ -1662,7 +1663,7 @@ void NetworkQualityEstimator::AddAndNotifyObserversOfRTT(
void NetworkQualityEstimator::AddAndNotifyObserversOfThroughput(
const Observation& observation) {
DCHECK(thread_checker_.CalledOnValidThread());
DCHECK_NE(nqe::internal::kInvalidThroughput, observation.value());
DCHECK_NE(nqe::internal::INVALID_RTT_THROUGHPUT, observation.value());
DCHECK_GT(NETWORK_QUALITY_OBSERVATION_SOURCE_MAX, observation.source());

downstream_throughput_kbps_observations_.AddObservation(observation);
Expand Down Expand Up @@ -1692,7 +1693,7 @@ void NetworkQualityEstimator::OnNewThroughputObservationAvailable(
if (downstream_kbps <= 0)
return;

DCHECK_NE(nqe::internal::kInvalidThroughput, downstream_kbps);
DCHECK_NE(nqe::internal::INVALID_RTT_THROUGHPUT, downstream_kbps);

Observation throughput_observation(downstream_kbps, tick_clock_->NowTicks(),
signal_strength_,
Expand Down Expand Up @@ -1810,7 +1811,7 @@ void NetworkQualityEstimator::OnPrefsRead(
it.second.network_quality().http_rtt());
DCHECK_EQ(nqe::internal::InvalidRTT(),
it.second.network_quality().transport_rtt());
DCHECK_EQ(nqe::internal::kInvalidThroughput,
DCHECK_EQ(nqe::internal::INVALID_RTT_THROUGHPUT,
it.second.network_quality().downstream_throughput_kbps());

nqe::internal::CachedNetworkQuality cached_network_quality(
Expand Down Expand Up @@ -1845,7 +1846,7 @@ base::Optional<int32_t> NetworkQualityEstimator::GetDownstreamThroughputKbps()
DCHECK(thread_checker_.CalledOnValidThread());

if (network_quality_.downstream_throughput_kbps() ==
nqe::internal::kInvalidThroughput) {
nqe::internal::INVALID_RTT_THROUGHPUT) {
return base::Optional<int32_t>();
}
return network_quality_.downstream_throughput_kbps();
Expand Down Expand Up @@ -1890,7 +1891,7 @@ void NetworkQualityEstimator::MaybeUpdateNetworkQualityFromCache(

// TODO(tbansal): crbug.com/673977: Remove this check.
if (cached_network_quality.network_quality().downstream_throughput_kbps() !=
nqe::internal::kInvalidThroughput) {
nqe::internal::INVALID_RTT_THROUGHPUT) {
Observation throughput_observation(
cached_network_quality.network_quality().downstream_throughput_kbps(),
base::TimeTicks::Now(), INT32_MIN,
Expand Down
12 changes: 6 additions & 6 deletions net/nqe/network_quality_estimator_params.cc
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ void ObtainDefaultObservations(
DCHECK_EQ(nqe::internal::InvalidRTT(), default_observations[i].http_rtt());
DCHECK_EQ(nqe::internal::InvalidRTT(),
default_observations[i].transport_rtt());
DCHECK_EQ(nqe::internal::kInvalidThroughput,
DCHECK_EQ(nqe::internal::INVALID_RTT_THROUGHPUT,
default_observations[i].downstream_throughput_kbps());
}

Expand Down Expand Up @@ -242,7 +242,7 @@ void ObtainTypicalNetworkQualities(
typical_network_quality[i].http_rtt());
DCHECK_EQ(nqe::internal::InvalidRTT(),
typical_network_quality[i].transport_rtt());
DCHECK_EQ(nqe::internal::kInvalidThroughput,
DCHECK_EQ(nqe::internal::INVALID_RTT_THROUGHPUT,
typical_network_quality[i].downstream_throughput_kbps());
}

Expand Down Expand Up @@ -295,21 +295,21 @@ void ObtainConnectionThresholds(
// Set to the 66th percentile of 2G RTT observations on Android.
base::TimeDelta::FromMilliseconds(2010),
base::TimeDelta::FromMilliseconds(1870),
nqe::internal::kInvalidThroughput);
nqe::internal::INVALID_RTT_THROUGHPUT);

default_effective_connection_type_thresholds[EFFECTIVE_CONNECTION_TYPE_2G] =
nqe::internal::NetworkQuality(
// Set to the 50th percentile of RTT observations on Android.
base::TimeDelta::FromMilliseconds(1420),
base::TimeDelta::FromMilliseconds(1280),
nqe::internal::kInvalidThroughput);
nqe::internal::INVALID_RTT_THROUGHPUT);

default_effective_connection_type_thresholds[EFFECTIVE_CONNECTION_TYPE_3G] =
nqe::internal::NetworkQuality(
// Set to the 50th percentile of 3G RTT observations on Android.
base::TimeDelta::FromMilliseconds(273),
base::TimeDelta::FromMilliseconds(204),
nqe::internal::kInvalidThroughput);
nqe::internal::INVALID_RTT_THROUGHPUT);

// Connection threshold should not be set for 4G effective connection type
// since it is the fastest.
Expand All @@ -325,7 +325,7 @@ void ObtainConnectionThresholds(
DCHECK_EQ(nqe::internal::InvalidRTT(), connection_thresholds[i].http_rtt());
DCHECK_EQ(nqe::internal::InvalidRTT(),
connection_thresholds[i].transport_rtt());
DCHECK_EQ(nqe::internal::kInvalidThroughput,
DCHECK_EQ(nqe::internal::INVALID_RTT_THROUGHPUT,
connection_thresholds[i].downstream_throughput_kbps());
if (effective_connection_type == EFFECTIVE_CONNECTION_TYPE_UNKNOWN)
continue;
Expand Down
7 changes: 4 additions & 3 deletions net/nqe/network_quality_estimator_params_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ TEST(NetworkQualityEstimatorParamsTest, TypicalNetworkQualities) {
// The typical throughput for an effective connection type should not be
// more than the threshold throughput.
if (params.ConnectionThreshold(ect).downstream_throughput_kbps() !=
nqe::internal::kInvalidThroughput) {
nqe::internal::INVALID_RTT_THROUGHPUT) {
EXPECT_LT(params.TypicalNetworkQuality(ect).downstream_throughput_kbps(),
params.ConnectionThreshold(ect).downstream_throughput_kbps());
}
Expand All @@ -97,7 +97,8 @@ TEST(NetworkQualityEstimatorParamsTest, TypicalNetworkQualities) {
.transport_rtt(),
params.ConnectionThreshold(EFFECTIVE_CONNECTION_TYPE_3G).transport_rtt());
if (params.ConnectionThreshold(EFFECTIVE_CONNECTION_TYPE_3G)
.downstream_throughput_kbps() != nqe::internal::kInvalidThroughput) {
.downstream_throughput_kbps() !=
nqe::internal::INVALID_RTT_THROUGHPUT) {
EXPECT_GT(params.TypicalNetworkQuality(EFFECTIVE_CONNECTION_TYPE_4G)
.downstream_throughput_kbps(),
params.ConnectionThreshold(EFFECTIVE_CONNECTION_TYPE_3G)
Expand Down Expand Up @@ -144,4 +145,4 @@ TEST(NetworkQualityEstimatorParamsTest, ObtainAlgorithmToUseFromParams) {

} // namespace nqe

} // namespace net
} // namespace net
Loading

0 comments on commit d80590e

Please sign in to comment.