NQE: Use cached estimates

net/nqe/network_quality_estimator.cc

 // Copyright 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "net/nqe/network_quality_estimator.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 #include <utility>
@@ skipping 764 matching lines @@
         min_signal_strength_since_connection_change_ != INT32_MAX &&
             max_signal_strength_since_connection_change_ != INT32_MIN);
   }
 #endif  // OS_ANDROID
   min_signal_strength_since_connection_change_ = INT32_MAX;
   max_signal_strength_since_connection_change_ = INT32_MIN;
   network_quality_ = nqe::internal::NetworkQuality();
   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();
   RecordNetworkIDAvailability();
 
   MaybeQueryExternalEstimateProvider();
 
   // Read any cached estimates for the new network. If cached estimates are
   // unavailable, add the default estimates.
   if (!ReadCachedNetworkQualityEstimate())
@@ skipping 582 matching lines @@
   }
 
   if (cached_network_quality.network_quality().http_rtt() !=
       nqe::internal::InvalidRTT()) {
     RttObservation rtt_observation(
         cached_network_quality.network_quality().http_rtt(), now,
         NETWORK_QUALITY_OBSERVATION_SOURCE_HTTP_CACHED_ESTIMATE);
     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;
 }
 
 void NetworkQualityEstimator::OnUpdatedEstimateAvailable(
     const base::TimeDelta& rtt,
     int32_t downstream_throughput_kbps,
     int32_t upstream_throughput_kbps) {
   DCHECK(thread_checker_.CalledOnValidThread());
   DCHECK(external_estimate_provider_);
 
@@ skipping 190 matching lines @@
     nqe::internal::NetworkQualityStore::NetworkQualitiesCacheObserver*
         observer) {
   DCHECK(thread_checker_.CalledOnValidThread());
   network_quality_store_->RemoveNetworkQualitiesCacheObserver(observer);
 }
 
 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