NQE: Use cached estimates

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 8551bc64ef0367b81dec142000186f442bf8b6bb..587cffb64bb054fc5524f980ae2f1c3cd293fb83 100644
--- a/net/nqe/network_quality_estimator.cc
+++ b/net/nqe/network_quality_estimator.cc
@@ -782,6 +782,8 @@ void NetworkQualityEstimator::OnConnectionTypeChanged(
   effective_connection_type_ = EFFECTIVE_CONNECTION_TYPE_UNKNOWN;
   effective_connection_type_at_last_main_frame_ =
       EFFECTIVE_CONNECTION_TYPE_UNKNOWN;
+  rtt_observations_size_at_last_ect_computation_ = 0;
+  throughput_observations_size_at_last_ect_computation_ = 0;
 
   // Update the local state as part of preparation for the new connection.
   current_network_id_ = GetCurrentNetworkID();
@@ -1384,6 +1386,15 @@ bool NetworkQualityEstimator::ReadCachedNetworkQualityEstimate() {
     rtt_observations_.AddObservation(rtt_observation);
     NotifyObserversOfRTT(rtt_observation);
   }
+
+  if (cached_network_quality.network_quality().transport_rtt() !=
+      nqe::internal::InvalidRTT()) {
+    RttObservation rtt_observation(
+        cached_network_quality.network_quality().transport_rtt(), now,
+        NETWORK_QUALITY_OBSERVATION_SOURCE_TRANSPORT_CACHED_ESTIMATE);
+    rtt_observations_.AddObservation(rtt_observation);
+    NotifyObserversOfRTT(rtt_observation);

[RyanSturm, 2016/12/13 21:08:35]

Can you add a class comment to NotifyObserversOfRTT since it does more than just
notify observers?

[tbansal1, 2016/12/14 01:37:37]

Done.

+  }
   return true;
 }
 
@@ -1594,9 +1605,66 @@ void NetworkQualityEstimator::OnPrefsRead(
     const std::map<nqe::internal::NetworkID,
                    nqe::internal::CachedNetworkQuality> read_prefs) {
   DCHECK(thread_checker_.CalledOnValidThread());
+
   UMA_HISTOGRAM_COUNTS("NQE.Prefs.ReadSize", read_prefs.size());
-  // TODO(tbansal): crbug.com/490870. Incorporate the network quality into the
-  // current estimates.
+  for (std::map<nqe::internal::NetworkID,
+                nqe::internal::CachedNetworkQuality>::const_iterator it =

[RyanSturm, 2016/12/13 21:08:35]

nit: maybe use auto instead since the map is const, auto should give you a
const_iterator anyway.

[tbansal1, 2016/12/14 01:37:37]

Done. This looks much better.

+           read_prefs.begin();
+       it != read_prefs.end(); ++it) {
+    EffectiveConnectionType effective_connection_type =
+        it->second.effective_connection_type();
+    if (effective_connection_type == EFFECTIVE_CONNECTION_TYPE_UNKNOWN ||
+        effective_connection_type == EFFECTIVE_CONNECTION_TYPE_OFFLINE) {
+      continue;
+    }
+
+    // RTT and throughput values are not set in the prefs.
+    DCHECK_EQ(nqe::internal::InvalidRTT(),
+              it->second.network_quality().http_rtt());
+    DCHECK_EQ(nqe::internal::InvalidRTT(),
+              it->second.network_quality().transport_rtt());
+    DCHECK_EQ(nqe::internal::kInvalidThroughput,
+              it->second.network_quality().downstream_throughput_kbps());
+
+    nqe::internal::NetworkQuality network_quality(
+        typical_network_quality_[effective_connection_type].http_rtt(),
+        typical_network_quality_[effective_connection_type].transport_rtt(),
+        typical_network_quality_[effective_connection_type]
+            .downstream_throughput_kbps());
+
+    nqe::internal::CachedNetworkQuality cached_network_quality(
+        base::TimeTicks::Now(), network_quality, effective_connection_type);
+
+    network_quality_store_->Add(it->first, cached_network_quality);

[RyanSturm, 2016/12/13 21:08:35]

talked offline, consider using ListValue instead DictionaryValue for the prefs
to keep LRU behavior in prefs instead of removing the smallest key value
everytime.

[tbansal1, 2016/12/14 01:37:37]

I have changed the behavior in prefs manager to use random deletion. Not using
ListValue for now.

+    if (it->first == current_network_id_) {
+      // Since the cached network quality is for the current network, add it to
+      // the current observations.
+      RttObservation http_rtt_observation(
+          network_quality.http_rtt(), base::TimeTicks::Now(),
+          NETWORK_QUALITY_OBSERVATION_SOURCE_HTTP_CACHED_ESTIMATE);
+      rtt_observations_.AddObservation(http_rtt_observation);
+      NotifyObserversOfRTT(http_rtt_observation);
+
+      RttObservation transport_rtt_observation(
+          network_quality.transport_rtt(), base::TimeTicks::Now(),
+          NETWORK_QUALITY_OBSERVATION_SOURCE_TRANSPORT_CACHED_ESTIMATE);
+      rtt_observations_.AddObservation(transport_rtt_observation);
+      NotifyObserversOfRTT(transport_rtt_observation);
+
+      if (network_quality.downstream_throughput_kbps() !=

[RyanSturm, 2016/12/13 21:08:35]

Can you add a comment referencing the bug to add typical throughput? So this
conditional gets removed when you add that.

[tbansal1, 2016/12/14 01:37:37]

Done.

+          nqe::internal::kInvalidThroughput) {
+        ThroughputObservation throughput_observation(
+            network_quality.downstream_throughput_kbps(),
+            base::TimeTicks::Now(),
+            NETWORK_QUALITY_OBSERVATION_SOURCE_HTTP_CACHED_ESTIMATE);
+        downstream_throughput_kbps_observations_.AddObservation(
+            throughput_observation);
+        NotifyObserversOfThroughput(throughput_observation);
+      }
+
+      ComputeEffectiveConnectionType();
+    }
+  }
 }
 
 }  // namespace net