Add in-memory caching of network quality estimates across network changes.

net/base/network_quality_estimator.h

Index: net/base/network_quality_estimator.h
diff --git a/net/base/network_quality_estimator.h b/net/base/network_quality_estimator.h
index ecfa08af6b8341b3a9bb06897a89baa42651c7b7..45ba34ca4cb839b4a9f560c0dfbb9420730f4506 100644
--- a/net/base/network_quality_estimator.h
+++ b/net/base/network_quality_estimator.h
@@ -7,16 +7,18 @@
 
 #include <stdint.h>
 
+#include <map>
+#include <string>
+
 #include "base/gtest_prod_util.h"
 #include "base/macros.h"
 #include "base/threading/thread_checker.h"
 #include "base/time/time.h"
 #include "net/base/network_change_notifier.h"
+#include "net/base/network_quality.h"
 
 namespace net {
 
-struct NetworkQuality;
-
 // NetworkQualityEstimator provides network quality estimates (quality of the
 // full paths to all origins that have been connected to).
 // The estimates are based on the observed organic traffic.
@@ -43,11 +45,81 @@ class NET_EXPORT_PRIVATE NetworkQualityEstimator
   void NotifyDataReceived(const URLRequest& request,
                           int64_t prefilter_bytes_read);
 
+ protected:
+  // Returns true if the cached network quality estimate was successfully read.
+  bool ReadCachedNetworkQualityEstimate();
+
+  // NetworkChangeNotifier::ConnectionTypeObserver implementation.
+  void OnConnectionTypeChanged(
+      NetworkChangeNotifier::ConnectionType type) override;
+
+  // Returns the number of entries in the cache. Used only for testing.
+  size_t GetCacheSizeForTests() const;
+
  private:
   FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest,
                            TestPeakKbpsFastestRTTUpdates);
+  FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest, TestCaching);
+  FRIEND_TEST_ALL_PREFIXES(NetworkQualityEstimatorTest,
+                           TestLRUCacheMaximumSize);
   FRIEND_TEST_ALL_PREFIXES(URLRequestTestHTTP, NetworkQualityEstimator);
 
+  // NetworkID is used to uniquely identify a network.
+  // For the purpose of network quality estimation and caching, a network is
+  // uniquely identified by a combination of |connection_type_| and
+  // |network_name_|. This approach is unable to distinguish networks with
+  // same name (e.g., different Wi-Fi networks with same SSID).

[bengr, 2015/06/11 00:02:28]

same name -> the same name

Also, could you use BSSID in addition to SSID to disambiguate?

[tbansal1, 2015/06/11 02:20:23]

I believe BSSID is currently exposed only on chromeOS. This is exposed by
Android Java APIs but there is nothing in chromium that exposes it to native
code.

I am not even sure if it is worth indexing this by BSSID. It might
actually cause harm in the sense that we might think that a network
is different from a previously seen network (and not use the cached
estimates), whereas in fact the performance of the two networks is very similar.

[bengr, 2015/06/13 00:28:54]

Acknowledged.

+  struct NetworkID {
+    NetworkChangeNotifier::ConnectionType type;
+    std::string id;
+
+    NetworkID(NetworkChangeNotifier::ConnectionType type, const std::string& id)
+        : type(type), id(id) {}
+
+    ~NetworkID() {}
+
+    NetworkID& operator=(const NetworkID& other) {
+      type = other.type;
+      id = other.id;
+      return *this;
+    }
+
+    // Overloaded because NetworkID is used as key in a map.
+    bool operator<(const NetworkID& other) const {
+      return type < other.type || (type == other.type && id < other.id);
+    }
+  };
+
+  // CachedNetworkQuality stores the quality of a previously seen network.
+  class CachedNetworkQuality {
+   public:
+    explicit CachedNetworkQuality(const NetworkQuality& network_quality);
+
+    ~CachedNetworkQuality();
+
+    // Returns the network quality associated with this cached entry.
+    const NetworkQuality GetNetworkQuality() const;
+
+    // Updates the network quality to the specified |median_kbps| and
+    // |median_rtt|.
+    void UpdateNetworkQuality(uint64_t median_kbps,
+                              const base::TimeDelta& median_rtt);
+
+    // Returns the time when this cached entry was last updated.
+    base::TimeTicks last_update_time() const { return last_update_time_; }
+
+   private:
+    // Time when this cache entry was last updated.
+    base::TimeTicks last_update_time_;
+
+    // Quality of this cached network.
+    NetworkQuality network_quality_;
+
+    DISALLOW_COPY_AND_ASSIGN(CachedNetworkQuality);
+  };
+
+  typedef std::map<NetworkID, scoped_ptr<CachedNetworkQuality>> CachedQualities;

[bengr, 2015/06/11 00:02:28]

Suggest renaming to CachedNetworkQualities.

[tbansal1, 2015/06/11 02:20:23]

Done.

+
   // Tiny transfer sizes may give inaccurate throughput results.
   // Minimum size of the transfer over which the throughput is computed.
   static const int kMinTransferSizeInBytes = 10000;
@@ -56,6 +128,11 @@ class NET_EXPORT_PRIVATE NetworkQualityEstimator
   // throughput is computed.
   static const int kMinRequestDurationMicroseconds = 1000;
 
+  // Maximum size of the cache that holds network quality estimates.
+  // Smaller size may reduce the cache hit rate due to frequent evictions.
+  // Larger size may affect performance.
+  static const uint32_t kMaximumNetworkQualityCacheSize;
+
   // Construct a NetworkQualityEstimator instance allowing for test
   // configuration.
   // Registers for network type change notifications so estimates can be kept
@@ -65,9 +142,18 @@ class NET_EXPORT_PRIVATE NetworkQualityEstimator
   // used for network quality estimation.
   explicit NetworkQualityEstimator(bool allow_local_host_requests_for_tests);
 
-  // NetworkChangeNotifier::ConnectionTypeObserver implementation.
-  void OnConnectionTypeChanged(
-      NetworkChangeNotifier::ConnectionType type) override;
+  // Updates the current network name to:
+  // WiFi SSID (if the user is connected to a WiFi access point and the SSID
+  // name is available), or
+  // The MCC/MNC code of the cellular carrier if the device is connected to a

[bengr, 2015/06/11 00:02:28]

The -> the

[tbansal1, 2015/06/11 02:20:23]

Done.

+  // cellular network.
+  // Updates the current network name to an empty string in all other cases or
+  // if the network name is not exposed by platform APIs.
+  // Virtualized for testing.
+  virtual std::string GetCurrentNetworkName() const;
+
+  // Write the estimated quality of the current network to the cache.

[bengr, 2015/06/11 00:02:28]

Write -> Writes

[tbansal1, 2015/06/11 02:20:23]

Done.

+  void CacheNetworkQualityEstimate();
 
   // Determines if the requests to local host can be used in estimating the
   // network quality. Set to true only for tests.
@@ -76,10 +162,6 @@ class NET_EXPORT_PRIVATE NetworkQualityEstimator
   // Time when last connection change was observed.
   base::TimeTicks last_connection_change_;
 
-  // Last value passed to |OnConnectionTypeChanged|. This indicates the
-  // current connection type.
-  NetworkChangeNotifier::ConnectionType current_connection_type_;
-
   // Set if any network data has been received since last connectivity change.
   bool bytes_read_since_last_connection_change_;
 
@@ -87,12 +169,18 @@ class NET_EXPORT_PRIVATE NetworkQualityEstimator
   // from URLRequest creation until first byte received.
   base::TimeDelta fastest_RTT_since_last_connection_change_;
 
+  // Cache to store quality of previously seen networks.
+  CachedQualities cached_network_quality_;
+
   // Rough measurement of downlink peak Kbps witnessed since last connectivity
   // change. The accuracy is decreased by ignoring these factors:
   // 1) Multiple URLRequests can occur concurrently.
   // 2) The transfer time includes at least one RTT while no bytes are read.
   uint64_t peak_kbps_since_last_connection_change_;
 
+  // NetworkID of the current network.

[bengr, 2015/06/11 00:02:28]

NetworkID -> ID

[tbansal1, 2015/06/11 02:20:23]

Done.

+  NetworkID current_network_id_;
+
   base::ThreadChecker thread_checker_;
 
   DISALLOW_COPY_AND_ASSIGN(NetworkQualityEstimator);