NQE: Change GetEffectiveConnectionType to return last ECT

net/nqe/network_quality_estimator.cc

Index: net/nqe/network_quality_estimator.cc
diff --git a/net/nqe/network_quality_estimator.cc b/net/nqe/network_quality_estimator.cc
index 9511ee7594ef9954a4701ca0b5a8e410a11e8c11..a324916f2bbe60af9a2710fd03c513581ff8cd37 100644
--- a/net/nqe/network_quality_estimator.cc
+++ b/net/nqe/network_quality_estimator.cc
@@ -373,7 +373,7 @@ NetworkQualityEstimator::NetworkQualityEstimator(
           EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
       external_estimate_provider_(std::move(external_estimates_provider)),
       effective_connection_type_recomputation_interval_(
-          base::TimeDelta::FromSeconds(15)),
+          base::TimeDelta::FromSeconds(10)),
       rtt_observations_size_at_last_ect_computation_(0),
       throughput_observations_size_at_last_ect_computation_(0),
       effective_connection_type_(EFFECTIVE_CONNECTION_TYPE_UNKNOWN),
@@ -627,6 +627,8 @@ void NetworkQualityEstimator::NotifyHeadersReceived(const URLRequest& request) {
     estimated_quality_at_last_main_frame_ = nqe::internal::NetworkQuality(
         estimated_http_rtt, estimated_transport_rtt,
         downstream_throughput_kbps);
+
+    RecomputeEffectiveConnectionType();
     effective_connection_type_at_last_main_frame_ =
         GetEffectiveConnectionType();
 
@@ -1158,10 +1160,29 @@ void NetworkQualityEstimator::RecordMetricsOnMainFrameRequest() const {
   effective_connection_type_histogram->Add(effective_connection_type);
 }
 
+void NetworkQualityEstimator::RecomputeEffectiveConnectionType() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
+  const base::TimeTicks now = tick_clock_->NowTicks();
+
+  const EffectiveConnectionType past_type = effective_connection_type_;
+  last_effective_connection_type_computation_ = now;
+
+  effective_connection_type_ =
+      GetRecentEffectiveConnectionType(base::TimeTicks());
+
+  if (past_type != effective_connection_type_)
+    NotifyObserversOfEffectiveConnectionTypeChanged();
+
+  rtt_observations_size_at_last_ect_computation_ = rtt_observations_.Size();
+  throughput_observations_size_at_last_ect_computation_ =
+      downstream_throughput_kbps_observations_.Size();
+}
+
 EffectiveConnectionType NetworkQualityEstimator::GetEffectiveConnectionType()
     const {
   DCHECK(thread_checker_.CalledOnValidThread());
-  return GetRecentEffectiveConnectionType(base::TimeTicks());
+  return effective_connection_type_;
 }
 
 EffectiveConnectionType
@@ -1625,26 +1646,16 @@ void NetworkQualityEstimator::MaybeRecomputeEffectiveConnectionType() {
       // effective connection type was unknown.
       effective_connection_type_ != EFFECTIVE_CONNECTION_TYPE_UNKNOWN &&
       // Recompute the effective connection type if the number of samples
-      // available now are more than twice in count than the number of
-      // samples that were available when the effective connection type was
-      // last computed.
-      rtt_observations_size_at_last_ect_computation_ * 2 >=
+      // available now are 50% more in count than the number of samples that

[bengr, 2016/08/25 22:51:24]

remove "in count"

[tbansal1, 2016/09/07 18:33:26]

Done.

+      // were available when the effective connection type was last computed.
+      rtt_observations_size_at_last_ect_computation_ * 1.5 >=

[bengr, 2016/08/25 22:51:24]

Does this mean that there's an exponential backoff on recomputation? If so, I
don't think that's a great idea, since we'll want to be responsive to sudden
changes. 

[tbansal1, 2016/09/07 18:33:26]

There is exponential backoff. Additionally, ECT is recomputed every 10 seconds
(provided there is network traffic), and on every main frame request. And, also
(in future) we may recompute if there is a significant change in the signal
strength.

The goal here is that NQE should detect "sudden changes" and respond to them
instead of callers detecting these changes, and forcing NQE to recompute ECT.

           rtt_observations_.Size() &&
-      throughput_observations_size_at_last_ect_computation_ * 2 >=
+      throughput_observations_size_at_last_ect_computation_ * 1.5 >=
           downstream_throughput_kbps_observations_.Size()) {
     return;
   }
 
-  const EffectiveConnectionType past_type = effective_connection_type_;
-  last_effective_connection_type_computation_ = now;
-  effective_connection_type_ = GetEffectiveConnectionType();
-
-  if (past_type != effective_connection_type_)
-    NotifyObserversOfEffectiveConnectionTypeChanged();
-
-  rtt_observations_size_at_last_ect_computation_ = rtt_observations_.Size();
-  throughput_observations_size_at_last_ect_computation_ =
-      downstream_throughput_kbps_observations_.Size();
+  RecomputeEffectiveConnectionType();

[bengr, 2016/08/25 22:51:24]

Can you just call this "ComputeEffectiveConnectionType()?"

[tbansal1, 2016/09/07 18:33:26]

Done.

 }
 
 void NetworkQualityEstimator::